Re: [PATCH v2 1/2] drm/panel: jd9365da: Modify Kingdisplay and Melfas panel timing

[Jessica Zhang <quic_jesszhan@quicinc.com>]

On 9/23/2024 7:13 PM, zhaoxiong lv wrote:
> On Tue, Sep 24, 2024 at 5:14 AM Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>>
>>
>>
>> On 9/23/2024 6:42 AM, Zhaoxiong Lv wrote:
>>> In MTK chips, if the system starts suspending before the DRM runtime
>>> resume has not completed, there is a possibility of a black screen
>>> after waking the machine. Reduce the disable delay resolves this issue,
>>
>> Hi Zhaoxiong,
>>
>> Do you mean "if the system starts suspending before the DRM runtime
>> resume *has* completed" here?
> 
> Hi Jessica
> 
> Sorry, my description may not be clear enough. It should be when the
> DRM runtime resume has not yet completed and the system enters sleep
> mode at the same time.

Got it, yes I think the reworded explanation in your reply is much clearer.

> 
> 
>>
>>>
>>> The "backlight_off_to_display_off_delay_ms" was added between
>>> "backlight off" and "display off"  to prevent "display off" from being
>>> executed when the backlight is not fully powered off, which may cause
>>> a white screen. However, we removed this
>>> "backlight_off_to_display_off_delay_ms" and found that this situation
>>> did not occur. Therefore, in order to solve the problem mentioned
>>> above, we We reduced it from 100ms to 3ms (tCMD_OFF >= 1ms).
>>
>> So from my understanding of this paragraph,
>> `backlight_off_to_display_off_delay_ms` was added to prevent the display
>> powering off before backlight is fully powered off. You recently tested
>> dropping this completely and saw no issue with this.
>>
>> If that's the case, why not drop this delay completely?
>>
>> Thanks,
>>
>> Jessica Zhang
> 
> Yes, there are currently no other issue after removing this delay.
> The reason why I didn't completely remove this delay is because the
> panel spec states that a delay of more than 1ms  (tCMD_OFF >= 1ms) is
> required when entering diaplay_off (0x28H), so I reserved 3ms.

Thanks for the clarification -- I see why you included the "(tCMD_OFF >= 
1ms)" part now. Can you please specify in the commit message that the 
tCMD_OFF delay requirement is from the spec?

Also can you remove the extra "We" in "we We reduced it from 100ms to 3ms"?

Thanks,

Jessica Zhang

> 
>>
>>>
>>> This is the timing specification for the two panels:
>>> 1. Kingdisplay panel timing spec:
>>> https://github.com/KD54183/-JD9365DA_Power-On-Off-Sequence_V0120240923
>>> 2. LMFBX101117480 timing spec: https://github.com/chiohsin-lo/TDY-JD_LIB
>>>
>>>
>>> Fixes: 2b976ad760dc ("drm/panel: jd9365da: Support for kd101ne3-40ti MIPI-DSI panel")
>>> Fixes: c4ce398cf18a ("drm/panel: jd9365da: Support for Melfas lmfbx101117480 MIPI-DSI panel")
>>>
>>> Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com>
>>> ---
>>> Changes between V2 and V1:
>>> -  1. Modify the commit message
>>> -  2. Modify the value of backlight_off_to_display_off_delay_ms.
>>> v1: https://lore.kernel.org/all/20240915080830.11318-2-lvzhaoxiong@huaqin.corp-partner.google.com/
>>> ---
>>>    drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
>>> index 44897e5218a6..486aa20e5518 100644
>>> --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
>>> +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c
>>> @@ -858,7 +858,7 @@ static const struct jadard_panel_desc kingdisplay_kd101ne3_40ti_desc = {
>>>        .reset_before_power_off_vcioo = true,
>>>        .vcioo_to_lp11_delay_ms = 5,
>>>        .lp11_to_reset_delay_ms = 10,
>>> -     .backlight_off_to_display_off_delay_ms = 100,
>>> +     .backlight_off_to_display_off_delay_ms = 3,
>>>        .display_off_to_enter_sleep_delay_ms = 50,
>>>        .enter_sleep_to_reset_down_delay_ms = 100,
>>>    };
>>> @@ -1109,7 +1109,7 @@ static const struct jadard_panel_desc melfas_lmfbx101117480_desc = {
>>>        .reset_before_power_off_vcioo = true,
>>>        .vcioo_to_lp11_delay_ms = 5,
>>>        .lp11_to_reset_delay_ms = 10,
>>> -     .backlight_off_to_display_off_delay_ms = 100,
>>> +     .backlight_off_to_display_off_delay_ms = 3,
>>>        .display_off_to_enter_sleep_delay_ms = 50,
>>>        .enter_sleep_to_reset_down_delay_ms = 100,
>>>    };
>>> --
>>> 2.17.1
>>>
>>