[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <pqzvhnrvtb4r3olxkkx2kyhsms3y3cki7cow52au3bug2hqzc6@h7wxgyruzd6n>
Date: Sun, 22 Sep 2024 17:45:07 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Tejas Vipin <tejasvipin76@...il.com>
Cc: neil.armstrong@...aro.org, maarten.lankhorst@...ux.intel.com,
mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, daniel@...ll.ch,
quic_jesszhan@...cinc.com, dianders@...omium.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/panel: elida-kd35t133: transition to mipi_dsi
wrapped functions
On Sun, Sep 22, 2024 at 11:49:49AM GMT, Tejas Vipin wrote:
>
>
> On 9/20/24 9:59 PM, Dmitry Baryshkov wrote:
> > On Tue, Sep 17, 2024 at 12:47:10PM GMT, Tejas Vipin wrote:
> >> Changes the elida-kd35t133 panel to use multi style functions for
> >> improved error handling.
> >>
> >> Signed-off-by: Tejas Vipin <tejasvipin76@...il.com>
> >> ---
> >> drivers/gpu/drm/panel/panel-elida-kd35t133.c | 107 ++++++++-----------
> >> 1 file changed, 45 insertions(+), 62 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/panel/panel-elida-kd35t133.c b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> >> index 00791ea81e90..62abda9559e7 100644
> >> --- a/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> >> +++ b/drivers/gpu/drm/panel/panel-elida-kd35t133.c
> >> @@ -135,25 +127,16 @@ static int kd35t133_prepare(struct drm_panel *panel)
> >>
> >> msleep(20);
> >>
> >> - ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> >> - if (ret < 0) {
> >> - dev_err(ctx->dev, "Failed to exit sleep mode: %d\n", ret);
> >> - goto disable_iovcc;
> >> - }
> >> + mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> >> + mipi_dsi_msleep(&dsi_ctx, 250);
> >>
> >> - msleep(250);
> >> + kd35t133_init_sequence(&dsi_ctx);
> >> + if (!dsi_ctx.accum_err)
> >> + dev_dbg(ctx->dev, "Panel init sequence done\n");
> >>
> >> - ret = kd35t133_init_sequence(ctx);
> >> - if (ret < 0) {
> >> - dev_err(ctx->dev, "Panel init sequence failed: %d\n", ret);
> >> + mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> >> + if (dsi_ctx.accum_err)
> >> goto disable_iovcc;
> >> - }
> >
> > Move this after the last mipi_dsi_msleep(), merge with the error
> > handling.
> >
> >> -
> >> - ret = mipi_dsi_dcs_set_display_on(dsi);
> >> - if (ret < 0) {
> >> - dev_err(ctx->dev, "Failed to set display on: %d\n", ret);
> >> - goto disable_iovcc;
> >> - }
> >>
> >> msleep(50);
> >
> > mipi_dsi_msleep()
>
> Is this necessary though? Converting this msleep to mipi_dsi_msleep and
> moving the previous dsi_ctx.accum_err check to below this seems
> redundant. If the check is placed above msleep, then we need to only
> check for the error once. If its placed below mipi_dsi_msleep, we end up
> checking for the error twice (once as written in the code, once in the
> code generated by the macro) which is unnecessary.
Yes, uniformity. And the compiler will most likely optimize things away.
>
> --
> Tejas Vipin
--
With best wishes
Dmitry
Powered by blists - more mailing lists