lkml.org 
[lkml]   [2024]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
From


On 4/8/2024 2:13 PM, Dmitry Baryshkov wrote:
> On Tue, 9 Apr 2024 at 00:08, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>>
>>
>>
>> On 4/8/2024 1:41 PM, Bjorn Andersson wrote:
>>> On Mon, Apr 08, 2024 at 12:43:34PM -0700, Abhinav Kumar wrote:
>>>>
>>>>
>>>> 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@quicinc.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.
>>>>
>>>
>>> I did some more testing on this patch. Connecting and disconnecting the
>>> cable while in fbcon works reliably, except for:
>>
>> Thanks for the tests !
>>
>>> - edid/modes are not read before we bring up the link so I always end up
>>> with 640x480
>>>
>>
>> hmmm, I wonder why this should be affected due to this patch. We always
>> read the EDID during hpd_connect() and the selected resolution will be
>> programmed with the modeset. We will retry this with our x1e80100 and see.
>
> BTW, why is EDID read during HPD handling? I always supposed that it
> can be read much later, when the DRM framework calls the get_modes /
> get_edid callbacks.
>

Well, dp_panel_read_sink_caps() is in dp_display_process_hpd_high()
currently. We read the edid there.

get_modes(), parses the EDID and adds the modes using drm_add_edid_modes().

>>
>>> - if I run modetest -s <id>:<mode> the link is brought up with the new
>>> resolution and I get my test image on the screen.
>>> But as we're shutting down the link for the resolution chance I end up
>>> in dp_bridge_hpd_notify() with link_ready && state = disconnected.
>>> This triggers an unplug which hangs on the event_mutex, such that as
>>> soon as I get the test image, the state machine enters
>>> DISCONNECT_PENDING. This is immediately followed by another
>>> !link_ready && status = connected, which attempts to perform the plug
>>> operation, but as we're in DISCONNECT_PENDING this is posted on the
>>> event queue. From there I get a log entry from my PLUG_INT, every
>>> 100ms stating that we're still in DISCONNECT_PENDING. If I exit
>>> modetest the screen goes black, and no new mode can be selected,
>>> because we're in DISCONNECT_PENDING. The only way out is to disconnect
>>> the cable to complete the DISCONNECT_PENDING.
>>>
>>
>> I am going to run this test-case and see what we can do.
>>
>>> Regards,
>>> Bjorn
>>>
>>>>
>>>>>> 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
>>>>>>
>
>
>

\
 
 \ /
  Last update: 2024-05-27 16:30    [W:0.046 / U:0.368 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site