linux-kernel - Re: [PATCH] drm/panel: elida-kd35t133: transition to mipi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d470e0ef-0193-478c-a858-d6498758aa9a@gmail.com>
Date: Sun, 22 Sep 2024 11:49:49 +0530
From: Tejas Vipin <tejasvipin76@...il.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
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 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.

-- 
Tejas Vipin