linux-kernel - Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug

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]

Date: Mon, 8 Apr 2024 12:43:34 -0700
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Bjorn Andersson <andersson@...nel.org>
CC: <freedreno@...ts.freedesktop.org>, Rob Clark <robdclark@...il.com>,
        "Dmitry Baryshkov" <dmitry.baryshkov@...aro.org>,
        Sean Paul
	<sean@...rly.run>,
        "Marijn Suijten" <marijn.suijten@...ainline.org>,
        David
 Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
        Kuogee Hsieh
	<quic_khsieh@...cinc.com>,
        <dri-devel@...ts.freedesktop.org>, <seanpaul@...omium.org>,
        <swboyd@...omium.org>, <quic_jesszhan@...cinc.com>,
        <quic_bjorande@...cinc.com>, <johan@...nel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle()
 directly for external HPD



On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
> On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
>> From: Kuogee Hsieh <quic_khsieh@...cinc.com>
> [..]
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index d80f89581760..bfb6dfff27e8 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
>>   		return;
>>   
>>   	if (!dp_display->link_ready && status == connector_status_connected)
>> -		dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
>> +		dp_hpd_plug_handle(dp, 0);
> 
> If I read the code correctly, and we get an external connect event
> inbetween a previous disconnect and the related disable call, this
> should result in a PLUG_INT being injected into the queue still.
> 
> Will that not cause the same problem?
> 
> Regards,
> Bjorn
>

Yes, your observation is correct and I had asked the same question to 
kuogee before taking over this change and posting.

We will have to handle that case separately. I don't have a good 
solution yet for it without requiring further rework or we drop the 
below snippet.

         if (state == ST_DISCONNECT_PENDING) {
                 /* wait until ST_DISCONNECTED */
                 dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
                 mutex_unlock(&dp->event_mutex);
                 return 0;
         }

I will need sometime to address that use-case as I need to see if we can 
handle that better and then drop the the DISCONNECT_PENDING state to 
address this fully. But it needs more testing.

But, we will need this patch anyway because without this we will not be 
able to fix even the most regular and commonly seen case of basic 
connect/disconnect receiving complementary events.


>>   	else if (dp_display->link_ready && status == connector_status_disconnected)
>> -		dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
>> +		dp_hpd_unplug_handle(dp, 0);
>>   }
>> -- 
>> 2.43.2
>>