linux-kernel - Re: [PATCH v2 3/4] drm/msm/dp: Delete the old 500 ms wait for eDP HPD in aux transfer

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] [day] [month] [year] [list]

Message-ID: <591d1c33-df3d-7ab6-9cb7-1422784a8e51@quicinc.com>
Date: Tue, 19 Mar 2024 11:27:43 -0700
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Doug Anderson <dianders@...omium.org>,
        Dmitry Baryshkov
	<dmitry.baryshkov@...aro.org>
CC: Rob Clark <robdclark@...il.com>, Daniel Vetter <daniel@...ll.ch>,
        "David
 Airlie" <airlied@...il.com>,
        Kuogee Hsieh <quic_khsieh@...cinc.com>,
        "Marijn
 Suijten" <marijn.suijten@...ainline.org>,
        Sean Paul <sean@...rly.run>, <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        "Bjorn
 Andersson" <quic_bjorande@...cinc.com>,
        Johan Hovold <johan@...nel.org>
Subject: Re: [PATCH v2 3/4] drm/msm/dp: Delete the old 500 ms wait for eDP HPD
 in aux transfer



On 3/19/2024 11:15 AM, Doug Anderson wrote:
> Hi,
> 
> On Tue, Mar 19, 2024 at 10:27 AM Dmitry Baryshkov
> <dmitry.baryshkov@...aro.org> wrote:
>>
>> On Tue, 19 Mar 2024 at 19:13, Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>>>
>>>
>>>
>>> On 3/18/2024 5:55 PM, Dmitry Baryshkov wrote:
>>>> On Tue, 19 Mar 2024 at 02:19, Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>>>>>
>>>>> +bjorn, johan as fyi for sc8280xp
>>>>>
>>>>> On 3/15/2024 2:36 PM, Douglas Anderson wrote:
>>>>>> Before the introduction of the wait_hpd_asserted() callback in commit
>>>>>> 841d742f094e ("drm/dp: Add wait_hpd_asserted() callback to struct
>>>>>> drm_dp_aux") the API between panel drivers and DP AUX bus drivers was
>>>>>> that it was up to the AUX bus driver to wait for HPD in the transfer()
>>>>>> function.
>>>>>>
>>>>>> Now wait_hpd_asserted() has been added. The two panel drivers that are
>>>>>> DP AUX endpoints use it. See commit 2327b13d6c47 ("drm/panel-edp: Take
>>>>>> advantage of wait_hpd_asserted() in struct drm_dp_aux") and commit
>>>>>> 3b5765df375c ("drm/panel: atna33xc20: Take advantage of
>>>>>> wait_hpd_asserted() in struct drm_dp_aux"). We've implemented
>>>>>> wait_hpd_asserted() in the MSM DP driver as of commit e2969ee30252
>>>>>> ("drm/msm/dp: move of_dp_aux_populate_bus() to eDP probe()"). There is
>>>>>> no longer any reason for long wait in the AUX transfer() function.
>>>>>> Remove it.
>>>>>>
>>>>>> NOTE: the wait_hpd_asserted() is listed as "optional". That means it's
>>>>>> optional for the DP AUX bus to implement. In the case of the MSM DP
>>>>>> driver we implement it so we can assume it will be called.
>>>>>>
>>>>>
>>>>> How do we enforce that for any new edp panels to be used with MSM, the
>>>>> panels should atleast invoke wait_hpd_asserted()?
>>>>>
>>>>> I agree that since MSM implements it, even though its listed as
>>>>> optional, we can drop this additional wait. So nothing wrong with this
>>>>> patch for current users including sc8280xp, sc7280 and sc7180.
>>>>>
>>>>> But, does there need to be some documentation that the edp panels not
>>>>> using the panel-edp framework should still invoke wait_hpd_asserted()?
>>>>>
>>>>> Since its marked as optional, what happens if the edp panel driver,
>>>>> skips calling wait_hpd_asserted()?
>>>>
>>>> It is optional for the DP AUX implementations, not for the panel to be called.
>>>>
>>>
>>> Yes, I understood that part, but is there anything from the panel side
>>> which mandates calling wait_hpd_asserted()?
>>>
>>> Is this documented somewhere for all edp panels to do:
>>>
>>> if (aux->wait_hpd_asserted)
>>>          aux->wait_hpd_asserted(aux, wait_us);
>>
>> That's obviously not true, e.g. if panel-edp.c handled HPD signal via
>> the GPIO pin.
>>
>> But the documentation explicitly says that the host will be powered up
>> automatically, but the caller must ensure that the panel is powered
>> on. `It's up to the caller of this code to make sure that the panel is
>> powered on if getting an error back is not OK.'
> 
> It wouldn't hurt to send out a documentation patch that makes this
> more explicit. OK, I sent:
> 
> https://lore.kernel.org/r/20240319111432.1.I521dad0693cc24fe4dd14cba0c7048d94f5b6b41@changeid
> 
> -Doug

Thanks, with that in place, this is

Reviewed-by: Abhinav Kumar <quic_abhinavk@...cinc.com>