Re: [PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD

[Abhinav Kumar]

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@xxxxxxxxxxx>
> [..]
> > 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