[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <qmp3ofpoynff2r66ye6ryv65lztfgbz2gsuoserfsuh3zgeglw@rlibvmp2serx>
Date: Thu, 13 Feb 2025 23:27:49 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Anusha Srivatsa <asrivats@...hat.com>
Cc: Neil Armstrong <neil.armstrong@...aro.org>,
Jessica Zhang <quic_jesszhan@...cinc.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Joel Selvaraj <jo@...amily.in>,
Ondrej Jirman <megi@....cz>, Javier Martinez Canillas <javierm@...hat.com>,
Jianhua Lu <lujianhua000@...il.com>, Robert Chiras <robert.chiras@....com>,
Artur Weber <aweber.kernel@...il.com>, Jonathan Corbet <corbet@....net>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH 04/20] drm/panel/boe-bf060y8m-aj0: Move to using
mipi_dsi_*_multi() variants
On Thu, Feb 13, 2025 at 03:44:21PM -0500, Anusha Srivatsa wrote:
> Stop using deprecated API.
> Used Coccinelle to make the change.
>
> @rule_3@
> identifier dsi_var;
> identifier r;
> identifier func;
> type t;
> position p;
> expression dsi_device;
> expression list es;
> @@
> t func(...) {
> ...
> struct mipi_dsi_device *dsi_var = dsi_device;
> +struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var };
> <+...
> (
> -mipi_dsi_dcs_write_seq(dsi_var,es);
> +mipi_dsi_dcs_write_seq_multi(&dsi_ctx,es);
> |
> -r = mipi_dsi_dcs_exit_sleep_mode(dsi_var)@p;
> +mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> |
> -r = mipi_dsi_dcs_enter_sleep_mode(dsi_var)@p;
> +mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> |
> -r = mipi_dsi_dcs_set_display_off(dsi_var)@p;
> +mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> |
> .....//rest of the mipi APIs with _multi variant
> )
> <+...
> -if(r < 0) {
> -...
> -}
> ...+>
> }
Granted the amount of issues, I'd kindly ask you to manually review your
patches after coccinelle before sending them to the mailing list.
I'd really repeat Doug's suggestion: please perform those conversions
one by one, etc.
I'm not going to review the rest of the series, Coccinelle most likely
can not catch all of this.
Another note, this is version 2 of your series, so it should have been
marked with v2.
>
> Signed-off-by: Anusha Srivatsa <asrivats@...hat.com>
> ---
> drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c | 75 +++++++++++---------------
> 1 file changed, 30 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c b/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c
> index 7e66db4a88bbed27920107458d01efd9cf4986df..8903a6c889794330fa1f54a30e779c7d5fbc4b14 100644
> --- a/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c
> +++ b/drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c
> @@ -55,42 +55,34 @@ static void boe_bf060y8m_aj0_reset(struct boe_bf060y8m_aj0 *boe)
> static int boe_bf060y8m_aj0_on(struct boe_bf060y8m_aj0 *boe)
> {
> struct mipi_dsi_device *dsi = boe->dsi;
> - struct device *dev = &dsi->dev;
> - int ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> - mipi_dsi_dcs_write_seq(dsi, 0xb0, 0xa5, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, 0xb2, 0x00, 0x4c);
> - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_SET_3D_CONTROL, 0x10);
> - mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_POWER_SAVE, DCS_ALLOW_HBM_RANGE);
> - mipi_dsi_dcs_write_seq(dsi, 0xf8,
> - 0x00, 0x08, 0x10, 0x00, 0x22, 0x00, 0x00, 0x2d);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb0, 0xa5, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb2, 0x00, 0x4c);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_SET_3D_CONTROL, 0x10);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE,
> + DCS_ALLOW_HBM_RANGE);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xf8, 0x00, 0x08, 0x10, 0x00,
> + 0x22, 0x00, 0x00, 0x2d);
>
> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> - if (ret < 0) {
> - dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
> - return ret;
> - }
> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> msleep(30);
mipi_dsi_msleep();
>
> - mipi_dsi_dcs_write_seq(dsi, 0xb0, 0xa5, 0x00);
> - mipi_dsi_dcs_write_seq(dsi, 0xc0,
> - 0x08, 0x48, 0x65, 0x33, 0x33, 0x33,
> - 0x2a, 0x31, 0x39, 0x20, 0x09);
> - mipi_dsi_dcs_write_seq(dsi, 0xc1, 0x00, 0x00, 0x00, 0x1f, 0x1f,
> - 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f,
> - 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f);
> - mipi_dsi_dcs_write_seq(dsi, 0xe2, 0x20, 0x04, 0x10, 0x12, 0x92,
> - 0x4f, 0x8f, 0x44, 0x84, 0x83, 0x83, 0x83,
> - 0x5c, 0x5c, 0x5c);
> - mipi_dsi_dcs_write_seq(dsi, 0xde, 0x01, 0x2c, 0x00, 0x77, 0x3e);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb0, 0xa5, 0x00);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc0, 0x08, 0x48, 0x65, 0x33,
> + 0x33, 0x33, 0x2a, 0x31, 0x39, 0x20, 0x09);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc1, 0x00, 0x00, 0x00, 0x1f,
> + 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f,
> + 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xe2, 0x20, 0x04, 0x10, 0x12,
> + 0x92, 0x4f, 0x8f, 0x44, 0x84, 0x83, 0x83,
> + 0x83, 0x5c, 0x5c, 0x5c);
> + mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xde, 0x01, 0x2c, 0x00, 0x77,
> + 0x3e);
>
> msleep(30);
mipi_dsi_msleep()
>
> - ret = mipi_dsi_dcs_set_display_on(dsi);
> - if (ret < 0) {
> - dev_err(dev, "Failed to set display on: %d\n", ret);
> - return ret;
> - }
> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> msleep(50);
mipi_dsi_msleep()
>
> return 0;
return dsi_ctx.accum_err
> @@ -99,24 +91,18 @@ static int boe_bf060y8m_aj0_on(struct boe_bf060y8m_aj0 *boe)
> static int boe_bf060y8m_aj0_off(struct boe_bf060y8m_aj0 *boe)
> {
> struct mipi_dsi_device *dsi = boe->dsi;
> - struct device *dev = &dsi->dev;
> - int ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
> /* OFF commands sent in HS mode */
> dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> - ret = mipi_dsi_dcs_set_display_off(dsi);
> - if (ret < 0) {
> - dev_err(dev, "Failed to set display off: %d\n", ret);
> - return ret;
> - }
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +
> msleep(20);
mipi_dsi_msleep()
>
> - ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> - if (ret < 0) {
> - dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
> - return ret;
> - }
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +
> usleep_range(1000, 2000);
mipi_dsi_usleep_range()
> +
> dsi->mode_flags |= MIPI_DSI_MODE_LPM;
>
> return 0;
return dsi_ctx.accum_err;
> @@ -233,12 +219,10 @@ static const struct drm_panel_funcs boe_bf060y8m_aj0_panel_funcs = {
> static int boe_bf060y8m_aj0_bl_update_status(struct backlight_device *bl)
> {
> struct mipi_dsi_device *dsi = bl_get_data(bl);
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> u16 brightness = backlight_get_brightness(bl);
> - int ret;
>
> - ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
> - if (ret < 0)
> - return ret;
> + mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, brightness);
>
> return 0;
return dsi_ctx.accum_err;
But why? There is little point in coverting this function.
> }
> @@ -246,6 +230,7 @@ static int boe_bf060y8m_aj0_bl_update_status(struct backlight_device *bl)
> static int boe_bf060y8m_aj0_bl_get_brightness(struct backlight_device *bl)
> {
> struct mipi_dsi_device *dsi = bl_get_data(bl);
> +
> u16 brightness;
> int ret;
--
With best wishes
Dmitry
Powered by blists - more mailing lists