diff mbox series

[RFC,6/7] drm/panel: lg-sw43408: add missing error handling

Message ID 20240510-dsi-panels-upd-api-v1-6-317c78a0dcc8@linaro.org (mailing list archive)
State New, archived
Headers show
Series drm/mipi-dsi: simplify MIPI DSI init/cleanup even more | expand

Commit Message

Dmitry Baryshkov May 9, 2024, 10:37 p.m. UTC 
Add missing error handling for the mipi_dsi_ functions that actually
return error code instead of silently ignoring it.

Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/panel/panel-lg-sw43408.c | 33 ++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

Comments

Doug Anderson May 10, 2024, 9:47 p.m. UTC | #1 
Hi,

On Thu, May 9, 2024 at 3:37 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Add missing error handling for the mipi_dsi_ functions that actually
> return error code instead of silently ignoring it.
>
> Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/panel/panel-lg-sw43408.c | 33 ++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)

Looks right to me. Only slight nit would be that I'd put this as the
first patch in the series to make it obvious to anyone backporting it
to older kernels that it doesn't have any dependencies on the earlier
patches in the series. It's fairly obvious so this isn't a huge deal,
but still could be nice.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Dmitry Baryshkov May 10, 2024, 10:25 p.m. UTC | #2 
On Fri, May 10, 2024 at 02:47:05PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Thu, May 9, 2024 at 3:37 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > Add missing error handling for the mipi_dsi_ functions that actually
> > return error code instead of silently ignoring it.
> >
> > Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/gpu/drm/panel/panel-lg-sw43408.c | 33 ++++++++++++++++++++++++++------
> >  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> Looks right to me. Only slight nit would be that I'd put this as the
> first patch in the series to make it obvious to anyone backporting it
> to older kernels that it doesn't have any dependencies on the earlier
> patches in the series. It's fairly obvious so this isn't a huge deal,
> but still could be nice.

Yes. I wanted to emphasise the _multi stuff rather than this fix. I'll
reorder patches for v2. Maybe I should also rebase the series on top of
patches by Cong Yang. WDYT?

> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
Doug Anderson May 10, 2024, 10:28 p.m. UTC | #3 
Hi,

On Fri, May 10, 2024 at 3:25 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Fri, May 10, 2024 at 02:47:05PM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, May 9, 2024 at 3:37 PM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > Add missing error handling for the mipi_dsi_ functions that actually
> > > return error code instead of silently ignoring it.
> > >
> > > Fixes: 069a6c0e94f9 ("drm: panel: Add LG sw43408 panel driver")
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/gpu/drm/panel/panel-lg-sw43408.c | 33 ++++++++++++++++++++++++++------
> > >  1 file changed, 27 insertions(+), 6 deletions(-)
> >
> > Looks right to me. Only slight nit would be that I'd put this as the
> > first patch in the series to make it obvious to anyone backporting it
> > to older kernels that it doesn't have any dependencies on the earlier
> > patches in the series. It's fairly obvious so this isn't a huge deal,
> > but still could be nice.
>
> Yes. I wanted to emphasise the _multi stuff rather than this fix. I'll
> reorder patches for v2. Maybe I should also rebase the series on top of
> patches by Cong Yang. WDYT?

Sounds good. I think Cong is planning on a V6 to fix the last nit I
had on his patch series [1] but otherwise this plan sounds fine to me.

[1] https://lore.kernel.org/r/CAHwB_NKtw0AyMgFb4rMFNgr4WF10o9_0pYvbKpnDM45aYma9zg@mail.gmail.com

-Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
index 2b3a73696dce..67a98ac508f8 100644
--- a/drivers/gpu/drm/panel/panel-lg-sw43408.c
+++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
@@ -62,16 +62,25 @@  static int sw43408_program(struct drm_panel *panel)
 {
 	struct sw43408_panel *ctx = to_panel_info(panel);
 	struct drm_dsc_picture_parameter_set pps;
+	int ret;
 
 	mipi_dsi_dcs_write_seq(ctx->link, MIPI_DCS_SET_GAMMA_CURVE, 0x02);
 
-	mipi_dsi_dcs_set_tear_on(ctx->link, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+	ret = mipi_dsi_dcs_set_tear_on(ctx->link, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+	if (ret < 0) {
+		dev_err(panel->dev, "Failed to set tearing: %d\n", ret);
+		return ret;
+	}
 
 	mipi_dsi_dcs_write_seq(ctx->link, 0x53, 0x0c, 0x30);
 	mipi_dsi_dcs_write_seq(ctx->link, 0x55, 0x00, 0x70, 0xdf, 0x00, 0x70, 0xdf);
 	mipi_dsi_dcs_write_seq(ctx->link, 0xf7, 0x01, 0x49, 0x0c);
 
-	mipi_dsi_dcs_exit_sleep_mode(ctx->link);
+	ret = mipi_dsi_dcs_exit_sleep_mode(ctx->link);
+	if (ret < 0) {
+		dev_err(panel->dev, "Failed to exit sleep mode: %d\n", ret);
+		return ret;
+	}
 
 	msleep(135);
 
@@ -97,14 +106,22 @@  static int sw43408_program(struct drm_panel *panel)
 	mipi_dsi_dcs_write_seq(ctx->link, 0x55, 0x04, 0x61, 0xdb, 0x04, 0x70, 0xdb);
 	mipi_dsi_dcs_write_seq(ctx->link, 0xb0, 0xca);
 
-	mipi_dsi_dcs_set_display_on(ctx->link);
+	ret = mipi_dsi_dcs_set_display_on(ctx->link);
+	if (ret < 0) {
+		dev_err(panel->dev, "Failed to set display on: %d\n", ret);
+		return ret;
+	}
 
 	msleep(50);
 
 	ctx->link->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
 	drm_dsc_pps_payload_pack(&pps, ctx->link->dsc);
-	mipi_dsi_picture_parameter_set(ctx->link, &pps);
+	ret = mipi_dsi_picture_parameter_set(ctx->link, &pps);
+	if (ret < 0) {
+		dev_err(panel->dev, "Failed to set PPS: %d\n", ret);
+		return ret;
+	}
 
 	ctx->link->mode_flags |= MIPI_DSI_MODE_LPM;
 
@@ -113,8 +130,12 @@  static int sw43408_program(struct drm_panel *panel)
 	 * PPS 1 if pps_identifier is 0
 	 * PPS 2 if pps_identifier is 1
 	 */
-	mipi_dsi_compression_mode_ext(ctx->link, true,
-				      MIPI_DSI_COMPRESSION_DSC, 1);
+	ret = mipi_dsi_compression_mode_ext(ctx->link, true,
+					    MIPI_DSI_COMPRESSION_DSC, 1);
+	if (ret < 0) {
+		dev_err(panel->dev, "Failed to set compression mode: %d\n", ret);
+		return ret;
+	}
 
 	return 0;
 }