| Message ID | 20240619033351.230929-1-tejasvipin76@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/panel: raydium-rm692e5: transition to mipi_dsi wrapped functions | expand |
On Wed, Jun 19, 2024 at 09:03:49AM GMT, Tejas Vipin wrote: > Use functions introduced in commit 966e397e4f60 ("drm/mipi-dsi: Introduce > mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe > ("drm/mipi-dsi: wrap more functions for streamline handling") for the > raydium rm692e5 panel. > > Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> > --- > Changes in v2: > - Change rm692e5_on to return void and take mipi_dsi_multi_context > as an argument. > - Remove unnecessary warnings. > - More efficient error handling in rm692e5_prepare > > v1: https://lore.kernel.org/all/20240615093758.65431-1-tejasvipin76@gmail.com/ > --- > drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 237 ++++++++---------- > 1 file changed, 99 insertions(+), 138 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > index 21d97f6b8a2f..9936bda61af2 100644 > --- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > +++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > static int rm692e5_prepare(struct drm_panel *panel) > { > struct rm692e5_panel *ctx = to_rm692e5_panel(panel); > struct drm_dsc_picture_parameter_set pps; > - struct device *dev = &ctx->dsi->dev; > - int ret; > + struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi }; > > - ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > - if (ret < 0) { > - dev_err(dev, "Failed to enable regulators: %d\n", ret); > - return ret; > - } > + dsi_ctx.accum_err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > + if (dsi_ctx.accum_err) > + return dsi_ctx.accum_err; int ret, please. Let's leave dsi_ctx.accum_err for DSI errors only. LGTM otherwise. > > rm692e5_reset(ctx); > > - ret = rm692e5_on(ctx); > - if (ret < 0) { > - dev_err(dev, "Failed to initialize panel: %d\n", ret); > - gpiod_set_value_cansleep(ctx->reset_gpio, 1); > - regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > - return ret; > - } > + rm692e5_on(&dsi_ctx); >
On 6/19/24 12:06 PM, Dmitry Baryshkov wrote: > On Wed, Jun 19, 2024 at 09:03:49AM GMT, Tejas Vipin wrote: >> Use functions introduced in commit 966e397e4f60 ("drm/mipi-dsi: Introduce >> mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe >> ("drm/mipi-dsi: wrap more functions for streamline handling") for the >> raydium rm692e5 panel. >> >> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> >> --- >> Changes in v2: >> - Change rm692e5_on to return void and take mipi_dsi_multi_context >> as an argument. >> - Remove unnecessary warnings. >> - More efficient error handling in rm692e5_prepare >> >> v1: https://lore.kernel.org/all/20240615093758.65431-1-tejasvipin76@gmail.com/ >> --- >> drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 237 ++++++++---------- >> 1 file changed, 99 insertions(+), 138 deletions(-) >> >> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c >> index 21d97f6b8a2f..9936bda61af2 100644 >> --- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c >> +++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > >> static int rm692e5_prepare(struct drm_panel *panel) >> { >> struct rm692e5_panel *ctx = to_rm692e5_panel(panel); >> struct drm_dsc_picture_parameter_set pps; >> - struct device *dev = &ctx->dsi->dev; >> - int ret; >> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi }; >> >> - ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); >> - if (ret < 0) { >> - dev_err(dev, "Failed to enable regulators: %d\n", ret); >> - return ret; >> - } >> + dsi_ctx.accum_err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); >> + if (dsi_ctx.accum_err) >> + return dsi_ctx.accum_err; > > int ret, please. Let's leave dsi_ctx.accum_err for DSI errors only. > LGTM otherwise. Is this really necessary seeing how regulator_bulk_enable returns 0 on success anyways? It saves creating a new variable for a single check. In case you do think its necessary, should it be changed in himax_hx83102 too? > >> >> rm692e5_reset(ctx); >> >> - ret = rm692e5_on(ctx); >> - if (ret < 0) { >> - dev_err(dev, "Failed to initialize panel: %d\n", ret); >> - gpiod_set_value_cansleep(ctx->reset_gpio, 1); >> - regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); >> - return ret; >> - } >> + rm692e5_on(&dsi_ctx); >> > >
Hi, On Wed, Jun 19, 2024 at 12:23 AM Tejas Vipin <tejasvipin76@gmail.com> wrote: > > > > On 6/19/24 12:06 PM, Dmitry Baryshkov wrote: > > On Wed, Jun 19, 2024 at 09:03:49AM GMT, Tejas Vipin wrote: > >> Use functions introduced in commit 966e397e4f60 ("drm/mipi-dsi: Introduce > >> mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe > >> ("drm/mipi-dsi: wrap more functions for streamline handling") for the > >> raydium rm692e5 panel. > >> > >> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> > >> --- > >> Changes in v2: > >> - Change rm692e5_on to return void and take mipi_dsi_multi_context > >> as an argument. > >> - Remove unnecessary warnings. > >> - More efficient error handling in rm692e5_prepare > >> > >> v1: https://lore.kernel.org/all/20240615093758.65431-1-tejasvipin76@gmail.com/ > >> --- > >> drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 237 ++++++++---------- > >> 1 file changed, 99 insertions(+), 138 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > >> index 21d97f6b8a2f..9936bda61af2 100644 > >> --- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > >> +++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > > > >> static int rm692e5_prepare(struct drm_panel *panel) > >> { > >> struct rm692e5_panel *ctx = to_rm692e5_panel(panel); > >> struct drm_dsc_picture_parameter_set pps; > >> - struct device *dev = &ctx->dsi->dev; > >> - int ret; > >> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi }; > >> > >> - ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > >> - if (ret < 0) { > >> - dev_err(dev, "Failed to enable regulators: %d\n", ret); > >> - return ret; > >> - } > >> + dsi_ctx.accum_err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > >> + if (dsi_ctx.accum_err) > >> + return dsi_ctx.accum_err; > > > > int ret, please. Let's leave dsi_ctx.accum_err for DSI errors only. > > LGTM otherwise. > > Is this really necessary seeing how regulator_bulk_enable returns > 0 on success anyways? It saves creating a new variable for a single > check. In case you do think its necessary, should it be changed in > himax_hx83102 too? Right. I made the same choice as Tejas did when I wrote commit a2ab7cb169da ("drm/panel: himax-hx83102: use wrapped MIPI DCS functions"). In that commit message, I wrote: It can also be noted that hx83102_prepare() has a mix of things that can take advantage of _multi calls and things that can't. The cleanest seemed to be to use the multi_ctx still but consistently use the "accum_err" variable for error returns, though that's definitely a style decision with pros and cons. In my mind trying to juggle half the cases having the error in "ret" and half in the DSI context was a recipe for getting mixed up and returning the wrong error. On the other hand, it felt awkward using the "dsi_ctx.accum_err". In the end I felt that the extra awkwardness was worth it if it meant that I was less likely to "return ret" when the error code was actually in "dsi_ctx.accum_err"... -Doug
Hi, On Tue, Jun 18, 2024 at 8:34 PM Tejas Vipin <tejasvipin76@gmail.com> wrote: > > Use functions introduced in commit 966e397e4f60 ("drm/mipi-dsi: Introduce > mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe > ("drm/mipi-dsi: wrap more functions for streamline handling") for the > raydium rm692e5 panel. It should be noted in the patch notes that as part of this patch there is a small functionality change which is arguably a bugfix. Specifically if some of the initialization commands in rm692e5_prepare() fail we'll now properly power the panel back off. IMO it's not a big enough deal to add a "Fixes" line since it's unlikely anyone is actually hitting this. If you want to add a Fixes tag (or someone else feels strongly that there should be one), the right way would probably to make this a 2-patch series and have _just_ the bugfix first and then have the conversion be a no-op. > Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> > --- > Changes in v2: > - Change rm692e5_on to return void and take mipi_dsi_multi_context > as an argument. > - Remove unnecessary warnings. > - More efficient error handling in rm692e5_prepare > > v1: https://lore.kernel.org/all/20240615093758.65431-1-tejasvipin76@gmail.com/ > --- > drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 237 ++++++++---------- > 1 file changed, 99 insertions(+), 138 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > index 21d97f6b8a2f..9936bda61af2 100644 > --- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > +++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > @@ -40,176 +40,137 @@ static void rm692e5_reset(struct rm692e5_panel *ctx) > usleep_range(10000, 11000); > } > > -static int rm692e5_on(struct rm692e5_panel *ctx) > +static void rm692e5_on(struct mipi_dsi_multi_context *dsi_ctx) > { > - struct mipi_dsi_device *dsi = ctx->dsi; > - struct device *dev = &dsi->dev; > - int ret; > + dsi_ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x41); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd6, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x16); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x8a, 0x87); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x71); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x82, 0x01); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc6, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc7, 0x2c); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc8, 0x64); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc9, 0x3c); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xca, 0x80); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xcb, 0x02); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xcc, 0x02); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x38); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x18, 0x13); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xf4); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x00, 0xff); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x01, 0xff); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x02, 0xcf); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x03, 0xbc); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x04, 0xb9); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x05, 0x99); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x06, 0x02); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x07, 0x0a); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x08, 0xe0); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x09, 0x4c); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0a, 0xeb); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0b, 0xe8); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0c, 0x32); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0d, 0x07); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xf4); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0d, 0xc0); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0e, 0xff); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0f, 0xff); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x10, 0x33); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x11, 0x6f); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x12, 0x6e); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x13, 0xa6); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x14, 0x80); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x15, 0x02); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x16, 0x38); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x17, 0xd3); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x18, 0x3a); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x19, 0xba); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x1a, 0xcc); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x1b, 0x01); > + > + mipi_dsi_dcs_nop_multi(dsi_ctx); > + > + mipi_dsi_msleep(dsi_ctx, 32); > + > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x38); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x18, 0x13); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xd1); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd3, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd0, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd2, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd4, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xb4, 0x01); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xf9); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x00, 0xaf); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x1d, 0x37); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x44, 0x0a, 0x7b); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfa, 0x01); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc2, 0x08); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x35, 0x00); > + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x51, 0x05, 0x42); > + > + mipi_dsi_dcs_exit_sleep_mode_multi(dsi_ctx); > + mipi_dsi_msleep(dsi_ctx, 100); > + mipi_dsi_dcs_set_display_on_multi(dsi_ctx); > nit: Delete the extra blank line above so there's not a blank like before the closing brace of the function. Hopefully you can post a v3 with the blank line removed and adjust the commit message. Then feel free to add: Reviewed-by: Douglas Anderson <dianders@chromium.org>
On Thu, 20 Jun 2024 at 17:42, Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Wed, Jun 19, 2024 at 12:23 AM Tejas Vipin <tejasvipin76@gmail.com> wrote: > > > > > > > > On 6/19/24 12:06 PM, Dmitry Baryshkov wrote: > > > On Wed, Jun 19, 2024 at 09:03:49AM GMT, Tejas Vipin wrote: > > >> Use functions introduced in commit 966e397e4f60 ("drm/mipi-dsi: Introduce > > >> mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe > > >> ("drm/mipi-dsi: wrap more functions for streamline handling") for the > > >> raydium rm692e5 panel. > > >> > > >> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> > > >> --- > > >> Changes in v2: > > >> - Change rm692e5_on to return void and take mipi_dsi_multi_context > > >> as an argument. > > >> - Remove unnecessary warnings. > > >> - More efficient error handling in rm692e5_prepare > > >> > > >> v1: https://lore.kernel.org/all/20240615093758.65431-1-tejasvipin76@gmail.com/ > > >> --- > > >> drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 237 ++++++++---------- > > >> 1 file changed, 99 insertions(+), 138 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > > >> index 21d97f6b8a2f..9936bda61af2 100644 > > >> --- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > > >> +++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c > > > > > >> static int rm692e5_prepare(struct drm_panel *panel) > > >> { > > >> struct rm692e5_panel *ctx = to_rm692e5_panel(panel); > > >> struct drm_dsc_picture_parameter_set pps; > > >> - struct device *dev = &ctx->dsi->dev; > > >> - int ret; > > >> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi }; > > >> > > >> - ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > > >> - if (ret < 0) { > > >> - dev_err(dev, "Failed to enable regulators: %d\n", ret); > > >> - return ret; > > >> - } > > >> + dsi_ctx.accum_err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > > >> + if (dsi_ctx.accum_err) > > >> + return dsi_ctx.accum_err; > > > > > > int ret, please. Let's leave dsi_ctx.accum_err for DSI errors only. > > > LGTM otherwise. > > > > Is this really necessary seeing how regulator_bulk_enable returns > > 0 on success anyways? It saves creating a new variable for a single > > check. In case you do think its necessary, should it be changed in > > himax_hx83102 too? > > Right. I made the same choice as Tejas did when I wrote commit > a2ab7cb169da ("drm/panel: himax-hx83102: use wrapped MIPI DCS > functions"). In that commit message, I wrote: > > It can also be noted that hx83102_prepare() has a mix of things that > can take advantage of _multi calls and things that can't. The cleanest > seemed to be to use the multi_ctx still but consistently use the > "accum_err" variable for error returns, though that's definitely a > style decision with pros and cons. > > In my mind trying to juggle half the cases having the error in "ret" > and half in the DSI context was a recipe for getting mixed up and > returning the wrong error. On the other hand, it felt awkward using > the "dsi_ctx.accum_err". In the end I felt that the extra awkwardness > was worth it if it meant that I was less likely to "return ret" when > the error code was actually in "dsi_ctx.accum_err"... Fair point. Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
diff --git a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c index 21d97f6b8a2f..9936bda61af2 100644 --- a/drivers/gpu/drm/panel/panel-raydium-rm692e5.c +++ b/drivers/gpu/drm/panel/panel-raydium-rm692e5.c @@ -40,176 +40,137 @@ static void rm692e5_reset(struct rm692e5_panel *ctx) usleep_range(10000, 11000); } -static int rm692e5_on(struct rm692e5_panel *ctx) +static void rm692e5_on(struct mipi_dsi_multi_context *dsi_ctx) { - struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - int ret; + dsi_ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x41); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd6, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x16); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x8a, 0x87); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x71); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x82, 0x01); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc6, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc7, 0x2c); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc8, 0x64); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc9, 0x3c); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xca, 0x80); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xcb, 0x02); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xcc, 0x02); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x38); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x18, 0x13); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xf4); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x00, 0xff); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x01, 0xff); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x02, 0xcf); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x03, 0xbc); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x04, 0xb9); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x05, 0x99); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x06, 0x02); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x07, 0x0a); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x08, 0xe0); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x09, 0x4c); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0a, 0xeb); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0b, 0xe8); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0c, 0x32); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0d, 0x07); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xf4); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0d, 0xc0); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0e, 0xff); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x0f, 0xff); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x10, 0x33); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x11, 0x6f); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x12, 0x6e); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x13, 0xa6); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x14, 0x80); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x15, 0x02); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x16, 0x38); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x17, 0xd3); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x18, 0x3a); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x19, 0xba); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x1a, 0xcc); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x1b, 0x01); + + mipi_dsi_dcs_nop_multi(dsi_ctx); + + mipi_dsi_msleep(dsi_ctx, 32); + + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x38); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x18, 0x13); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xd1); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd3, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd0, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd2, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xd4, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xb4, 0x01); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0xf9); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x00, 0xaf); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x1d, 0x37); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x44, 0x0a, 0x7b); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfe, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xfa, 0x01); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0xc2, 0x08); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x35, 0x00); + mipi_dsi_generic_write_seq_multi(dsi_ctx, 0x51, 0x05, 0x42); + + mipi_dsi_dcs_exit_sleep_mode_multi(dsi_ctx); + mipi_dsi_msleep(dsi_ctx, 100); + mipi_dsi_dcs_set_display_on_multi(dsi_ctx); - dsi->mode_flags |= MIPI_DSI_MODE_LPM; - - mipi_dsi_generic_write_seq(dsi, 0xfe, 0x41); - mipi_dsi_generic_write_seq(dsi, 0xd6, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0x16); - mipi_dsi_generic_write_seq(dsi, 0x8a, 0x87); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0x71); - mipi_dsi_generic_write_seq(dsi, 0x82, 0x01); - mipi_dsi_generic_write_seq(dsi, 0xc6, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xc7, 0x2c); - mipi_dsi_generic_write_seq(dsi, 0xc8, 0x64); - mipi_dsi_generic_write_seq(dsi, 0xc9, 0x3c); - mipi_dsi_generic_write_seq(dsi, 0xca, 0x80); - mipi_dsi_generic_write_seq(dsi, 0xcb, 0x02); - mipi_dsi_generic_write_seq(dsi, 0xcc, 0x02); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0x38); - mipi_dsi_generic_write_seq(dsi, 0x18, 0x13); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0xf4); - mipi_dsi_generic_write_seq(dsi, 0x00, 0xff); - mipi_dsi_generic_write_seq(dsi, 0x01, 0xff); - mipi_dsi_generic_write_seq(dsi, 0x02, 0xcf); - mipi_dsi_generic_write_seq(dsi, 0x03, 0xbc); - mipi_dsi_generic_write_seq(dsi, 0x04, 0xb9); - mipi_dsi_generic_write_seq(dsi, 0x05, 0x99); - mipi_dsi_generic_write_seq(dsi, 0x06, 0x02); - mipi_dsi_generic_write_seq(dsi, 0x07, 0x0a); - mipi_dsi_generic_write_seq(dsi, 0x08, 0xe0); - mipi_dsi_generic_write_seq(dsi, 0x09, 0x4c); - mipi_dsi_generic_write_seq(dsi, 0x0a, 0xeb); - mipi_dsi_generic_write_seq(dsi, 0x0b, 0xe8); - mipi_dsi_generic_write_seq(dsi, 0x0c, 0x32); - mipi_dsi_generic_write_seq(dsi, 0x0d, 0x07); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0xf4); - mipi_dsi_generic_write_seq(dsi, 0x0d, 0xc0); - mipi_dsi_generic_write_seq(dsi, 0x0e, 0xff); - mipi_dsi_generic_write_seq(dsi, 0x0f, 0xff); - mipi_dsi_generic_write_seq(dsi, 0x10, 0x33); - mipi_dsi_generic_write_seq(dsi, 0x11, 0x6f); - mipi_dsi_generic_write_seq(dsi, 0x12, 0x6e); - mipi_dsi_generic_write_seq(dsi, 0x13, 0xa6); - mipi_dsi_generic_write_seq(dsi, 0x14, 0x80); - mipi_dsi_generic_write_seq(dsi, 0x15, 0x02); - mipi_dsi_generic_write_seq(dsi, 0x16, 0x38); - mipi_dsi_generic_write_seq(dsi, 0x17, 0xd3); - mipi_dsi_generic_write_seq(dsi, 0x18, 0x3a); - mipi_dsi_generic_write_seq(dsi, 0x19, 0xba); - mipi_dsi_generic_write_seq(dsi, 0x1a, 0xcc); - mipi_dsi_generic_write_seq(dsi, 0x1b, 0x01); - - ret = mipi_dsi_dcs_nop(dsi); - if (ret < 0) { - dev_err(dev, "Failed to nop: %d\n", ret); - return ret; - } - msleep(32); - - mipi_dsi_generic_write_seq(dsi, 0xfe, 0x38); - mipi_dsi_generic_write_seq(dsi, 0x18, 0x13); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0xd1); - mipi_dsi_generic_write_seq(dsi, 0xd3, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xd0, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xd2, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xd4, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xb4, 0x01); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0xf9); - mipi_dsi_generic_write_seq(dsi, 0x00, 0xaf); - mipi_dsi_generic_write_seq(dsi, 0x1d, 0x37); - mipi_dsi_generic_write_seq(dsi, 0x44, 0x0a, 0x7b); - mipi_dsi_generic_write_seq(dsi, 0xfe, 0x00); - mipi_dsi_generic_write_seq(dsi, 0xfa, 0x01); - mipi_dsi_generic_write_seq(dsi, 0xc2, 0x08); - mipi_dsi_generic_write_seq(dsi, 0x35, 0x00); - mipi_dsi_generic_write_seq(dsi, 0x51, 0x05, 0x42); - - ret = mipi_dsi_dcs_exit_sleep_mode(dsi); - if (ret < 0) { - dev_err(dev, "Failed to exit sleep mode: %d\n", ret); - return ret; - } - msleep(100); - - ret = mipi_dsi_dcs_set_display_on(dsi); - if (ret < 0) { - dev_err(dev, "Failed to set display on: %d\n", ret); - return ret; - } - - return 0; } static int rm692e5_disable(struct drm_panel *panel) { struct rm692e5_panel *ctx = to_rm692e5_panel(panel); struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - int ret; + struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; - mipi_dsi_generic_write_seq(dsi, 0xfe, 0x00); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xfe, 0x00); - 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); - ret = mipi_dsi_dcs_enter_sleep_mode(dsi); - if (ret < 0) { - dev_err(dev, "Failed to enter sleep mode: %d\n", ret); - return ret; - } - msleep(100); + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx); - return 0; + mipi_dsi_msleep(&dsi_ctx, 100); + + return dsi_ctx.accum_err; } static int rm692e5_prepare(struct drm_panel *panel) { struct rm692e5_panel *ctx = to_rm692e5_panel(panel); struct drm_dsc_picture_parameter_set pps; - struct device *dev = &ctx->dsi->dev; - int ret; + struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi }; - ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); - if (ret < 0) { - dev_err(dev, "Failed to enable regulators: %d\n", ret); - return ret; - } + dsi_ctx.accum_err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies); + if (dsi_ctx.accum_err) + return dsi_ctx.accum_err; rm692e5_reset(ctx); - ret = rm692e5_on(ctx); - if (ret < 0) { - dev_err(dev, "Failed to initialize panel: %d\n", ret); - gpiod_set_value_cansleep(ctx->reset_gpio, 1); - regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); - return ret; - } + rm692e5_on(&dsi_ctx); drm_dsc_pps_payload_pack(&pps, &ctx->dsc); - ret = mipi_dsi_picture_parameter_set(ctx->dsi, &pps); - if (ret < 0) { - dev_err(panel->dev, "failed to transmit PPS: %d\n", ret); - return ret; - } - - ret = mipi_dsi_compression_mode(ctx->dsi, true); - if (ret < 0) { - dev_err(dev, "failed to enable compression mode: %d\n", ret); - return ret; - } - - msleep(28); + mipi_dsi_picture_parameter_set_multi(&dsi_ctx, &pps); + mipi_dsi_compression_mode_ext_multi(&dsi_ctx, true, MIPI_DSI_COMPRESSION_DSC, 0); + mipi_dsi_msleep(&dsi_ctx, 28); - mipi_dsi_generic_write_seq(ctx->dsi, 0xfe, 0x40); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xfe, 0x40); /* 0x05 -> 90Hz, 0x00 -> 60Hz */ - mipi_dsi_generic_write_seq(ctx->dsi, 0xbd, 0x05); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xbd, 0x05); - mipi_dsi_generic_write_seq(ctx->dsi, 0xfe, 0x00); + mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xfe, 0x00); - return 0; + if (dsi_ctx.accum_err) { + gpiod_set_value_cansleep(ctx->reset_gpio, 1); + regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); + } + + return dsi_ctx.accum_err; } static int rm692e5_unprepare(struct drm_panel *panel)
Use functions introduced in commit 966e397e4f60 ("drm/mipi-dsi: Introduce mipi_dsi_*_write_seq_multi()") and commit f79d6d28d8fe ("drm/mipi-dsi: wrap more functions for streamline handling") for the raydium rm692e5 panel. Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com> --- Changes in v2: - Change rm692e5_on to return void and take mipi_dsi_multi_context as an argument. - Remove unnecessary warnings. - More efficient error handling in rm692e5_prepare v1: https://lore.kernel.org/all/20240615093758.65431-1-tejasvipin76@gmail.com/ --- drivers/gpu/drm/panel/panel-raydium-rm692e5.c | 237 ++++++++---------- 1 file changed, 99 insertions(+), 138 deletions(-)