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

Dmitry Baryshkov posted 7 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH RFC 6/7] drm/panel: lg-sw43408: add missing error handling
Posted by Dmitry Baryshkov 1 year, 7 months ago
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(-)

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;
 }

-- 
2.39.2
Re: [PATCH RFC 6/7] drm/panel: lg-sw43408: add missing error handling
Posted by Doug Anderson 1 year, 7 months ago
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>
Re: [PATCH RFC 6/7] drm/panel: lg-sw43408: add missing error handling
Posted by Dmitry Baryshkov 1 year, 7 months ago
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>

-- 
With best wishes
Dmitry
Re: [PATCH RFC 6/7] drm/panel: lg-sw43408: add missing error handling
Posted by Doug Anderson 1 year, 7 months ago
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