| From c41a47b5c46a4a0b4344807ec246418e6e7347c0 Mon Sep 17 00:00:00 2001 |
| From: Douglas Anderson <dianders@chromium.org> |
| Date: Tue, 14 May 2024 10:20:54 -0700 |
| Subject: [PATCH] FROMGIT: drm/mipi-dsi: Reduce driver bloat of |
| mipi_dsi_*_write_seq() |
| |
| Through a cooperative effort between Hsin-Yi Wang and Dmitry |
| Baryshkov, we have realized the dev_err() in the |
| mipi_dsi_*_write_seq() macros was causing quite a bit of bloat to the |
| kernel. Let's hoist this call into drm_mipi_dsi.c by adding a "chatty" |
| version of the functions that includes the print. While doing this, |
| add a bit more comments to these macros making it clear that they |
| print errors and also that they return out of _the caller's_ function. |
| |
| Without any changes to clients this gives a nice savings. Specifically |
| the macro was inlined and thus the error report call was inlined into |
| every call to mipi_dsi_dcs_write_seq() and |
| mipi_dsi_generic_write_seq(). By using a call to a "chatty" function, |
| the usage is reduced to one call in the chatty function and a function |
| call at the invoking site. |
| |
| Building with my build system shows one example: |
| |
| $ scripts/bloat-o-meter \ |
| .../before/panel-novatek-nt36672e.ko \ |
| .../after/panel-novatek-nt36672e.ko |
| add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-4404 (-4404) |
| Function old new delta |
| nt36672e_1080x2408_60hz_init 10640 6236 -4404 |
| Total: Before=15055, After=10651, chg -29.25% |
| |
| Note that given the change in location of the print it's harder to |
| include the "cmd" in the printout for mipi_dsi_dcs_write_seq() since, |
| theoretically, someone could call the new chatty function with a |
| zero-size array and it would be illegal to dereference data[0]. |
| There's a printk format to print the whole buffer and this is probably |
| more useful for debugging anyway. Given that we're doing this for |
| mipi_dsi_dcs_write_seq(), let's also print the buffer for |
| mipi_dsi_generic_write_seq() in the error case. |
| |
| It should be noted that the current consensus of DRM folks is that the |
| mipi_dsi_*_write_seq() should be deprecated due to the non-intuitive |
| return behavior. A future patch will formally mark them as deprecated |
| and provide an alternative. |
| |
| Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> |
| Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> |
| Reviewed-by: Linus Walleij <linus.walleij@linaro.org> |
| Signed-off-by: Douglas Anderson <dianders@chromium.org> |
| Link: https://lore.kernel.org/r/20240514102056.v5.4.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid |
| Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> |
| Link: https://patchwork.freedesktop.org/patch/msgid/20240514102056.v5.4.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid |
| (cherry picked from commit 3b724909a380fddb44dfa0072fc459c698a52658 |
| https://anongit.freedesktop.org/git/drm/drm-misc.git drm-misc-next) |
| |
| BUG=b:319025360 |
| TEST=emerge-geralt sys-kernel/chromeos-kernel-6_1 |
| |
| Change-Id: Id15fae80582bc74a0d4f1338987fa375738f45b9 |
| Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com> |
| Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/5547926 |
| Reviewed-by: Fei Shao <fshao@chromium.org> |
| Reviewed-by: Sean Paul <sean@poorly.run> |
| Commit-Queue: Fei Shao <fshao@chromium.org> |
| Reviewed-by: Xuxin Xiong <xuxinxiong@huaqin.corp-partner.google.com> |
| Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/5553096 |
| --- |
| drivers/gpu/drm/drm_mipi_dsi.c | 56 ++++++++++++++++++++++++++++++++++ |
| include/drm/drm_mipi_dsi.h | 39 ++++++++++++----------- |
| 2 files changed, 77 insertions(+), 18 deletions(-) |
| |
| diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c |
| index 795001bb7ff1a24cb97844f5ba24980f8497545d..4d2685d5a6e03f97f18120c2a995e6316a318fd0 100644 |
| --- a/drivers/gpu/drm/drm_mipi_dsi.c |
| +++ b/drivers/gpu/drm/drm_mipi_dsi.c |
| @@ -764,6 +764,34 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload, |
| } |
| EXPORT_SYMBOL(mipi_dsi_generic_write); |
| |
| +/** |
| + * mipi_dsi_generic_write_chatty() - mipi_dsi_generic_write() w/ an error log |
| + * @dsi: DSI peripheral device |
| + * @payload: buffer containing the payload |
| + * @size: size of payload buffer |
| + * |
| + * Like mipi_dsi_generic_write() but includes a dev_err() |
| + * call for you and returns 0 upon success, not the number of bytes sent. |
| + * |
| + * Return: 0 on success or a negative error code on failure. |
| + */ |
| +int mipi_dsi_generic_write_chatty(struct mipi_dsi_device *dsi, |
| + const void *payload, size_t size) |
| +{ |
| + struct device *dev = &dsi->dev; |
| + ssize_t ret; |
| + |
| + ret = mipi_dsi_generic_write(dsi, payload, size); |
| + if (ret < 0) { |
| + dev_err(dev, "sending generic data %*ph failed: %zd\n", |
| + (int)size, payload, ret); |
| + return ret; |
| + } |
| + |
| + return 0; |
| +} |
| +EXPORT_SYMBOL(mipi_dsi_generic_write_chatty); |
| + |
| /** |
| * mipi_dsi_generic_read() - receive data using a generic read packet |
| * @dsi: DSI peripheral device |
| @@ -852,6 +880,34 @@ ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi, |
| } |
| EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer); |
| |
| +/** |
| + * mipi_dsi_dcs_write_buffer_chatty - mipi_dsi_dcs_write_buffer() w/ an error log |
| + * @dsi: DSI peripheral device |
| + * @data: buffer containing data to be transmitted |
| + * @len: size of transmission buffer |
| + * |
| + * Like mipi_dsi_dcs_write_buffer() but includes a dev_err() |
| + * call for you and returns 0 upon success, not the number of bytes sent. |
| + * |
| + * Return: 0 on success or a negative error code on failure. |
| + */ |
| +int mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi, |
| + const void *data, size_t len) |
| +{ |
| + struct device *dev = &dsi->dev; |
| + ssize_t ret; |
| + |
| + ret = mipi_dsi_dcs_write_buffer(dsi, data, len); |
| + if (ret < 0) { |
| + dev_err(dev, "sending dcs data %*ph failed: %zd\n", |
| + (int)len, data, ret); |
| + return ret; |
| + } |
| + |
| + return 0; |
| +} |
| +EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer_chatty); |
| + |
| /** |
| * mipi_dsi_dcs_write() - send DCS write command |
| * @dsi: DSI peripheral device |
| diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h |
| index 82b1cc434ea3fbede3bb8e878e5a8919a3a3e951..6d68d9927f4664bc48205b099a7bf07057193952 100644 |
| --- a/include/drm/drm_mipi_dsi.h |
| +++ b/include/drm/drm_mipi_dsi.h |
| @@ -256,6 +256,8 @@ int mipi_dsi_picture_parameter_set(struct mipi_dsi_device *dsi, |
| |
| ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload, |
| size_t size); |
| +int mipi_dsi_generic_write_chatty(struct mipi_dsi_device *dsi, |
| + const void *payload, size_t size); |
| ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params, |
| size_t num_params, void *data, size_t size); |
| |
| @@ -279,6 +281,8 @@ enum mipi_dsi_dcs_tear_mode { |
| |
| ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi, |
| const void *data, size_t len); |
| +int mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi, |
| + const void *data, size_t len); |
| ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd, |
| const void *data, size_t len); |
| ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data, |
| @@ -311,40 +315,39 @@ int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi, |
| |
| /** |
| * mipi_dsi_generic_write_seq - transmit data using a generic write packet |
| + * |
| + * This macro will print errors for you and will RETURN FROM THE CALLING |
| + * FUNCTION (yes this is non-intuitive) upon error. |
| + * |
| * @dsi: DSI peripheral device |
| * @seq: buffer containing the payload |
| */ |
| #define mipi_dsi_generic_write_seq(dsi, seq...) \ |
| do { \ |
| static const u8 d[] = { seq }; \ |
| - struct device *dev = &dsi->dev; \ |
| int ret; \ |
| - ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)); \ |
| - if (ret < 0) { \ |
| - dev_err_ratelimited(dev, "transmit data failed: %d\n", \ |
| - ret); \ |
| + ret = mipi_dsi_generic_write_chatty(dsi, d, ARRAY_SIZE(d)); \ |
| + if (ret < 0) \ |
| return ret; \ |
| - } \ |
| } while (0) |
| |
| /** |
| * mipi_dsi_dcs_write_seq - transmit a DCS command with payload |
| + * |
| + * This macro will print errors for you and will RETURN FROM THE CALLING |
| + * FUNCTION (yes this is non-intuitive) upon error. |
| + * |
| * @dsi: DSI peripheral device |
| * @cmd: Command |
| * @seq: buffer containing data to be transmitted |
| */ |
| -#define mipi_dsi_dcs_write_seq(dsi, cmd, seq...) \ |
| - do { \ |
| - static const u8 d[] = { cmd, seq }; \ |
| - struct device *dev = &dsi->dev; \ |
| - int ret; \ |
| - ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \ |
| - if (ret < 0) { \ |
| - dev_err_ratelimited( \ |
| - dev, "sending command %#02x failed: %d\n", \ |
| - cmd, ret); \ |
| - return ret; \ |
| - } \ |
| +#define mipi_dsi_dcs_write_seq(dsi, cmd, seq...) \ |
| + do { \ |
| + static const u8 d[] = { cmd, seq }; \ |
| + int ret; \ |
| + ret = mipi_dsi_dcs_write_buffer_chatty(dsi, d, ARRAY_SIZE(d)); \ |
| + if (ret < 0) \ |
| + return ret; \ |
| } while (0) |
| |
| /** |
| -- |
| 2.45.1.288.g0e0cd299f1-goog |
| |