On Mon, Apr 08, 2024 at 09:33:01PM -0500, Bjorn Andersson wrote: > On Tue, Apr 09, 2024 at 01:07:57AM +0300, Dmitry Baryshkov wrote: > > On Tue, 9 Apr 2024 at 00:23, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > On 4/8/2024 2:12 PM, Dmitry Baryshkov wrote: > > > > On Mon, 8 Apr 2024 at 22:43, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> 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@xxxxxxxxxxx> > > > >>> [..] > > > >>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > [..] > > > >> > > > >> 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. > > > > > > > > Hmm, no. We need to drop the HPD state machine, not to patch it. Once > > > > the driver has proper detect() callback, there will be no > > > > complementary events. That is a proper way to fix the code, not these > > > > kinds of band-aids patches. > > > > > > > > > > I had discussed this part too :) > > > > > > I totally agree we should fix .detect()'s behavior to just match cable > > > connect/disconnect and not link_ready status. > > > > > > But that alone would not have fixed this issue. If HPD thread does not > > > get scheduled and plug_handle() was not executed, .detect() would have > > > still returned old status as we will update the cable status only in > > > plug_handle() / unplug_handle() to have a common API between internal > > > and external hpd execution. > > > > I think there should be just hpd_notify, which if the HPD is up, > > attempts to read the DPCD. No need for separate plug/unplug_handle. > > The detect() can be as simple as !drm_dp_is_branch() || sink_count != 0. > > > > What is detect() supposed to return in the event that we have external > HPD handler? The link state? While the external HPD bridge would return > the HPD state? It should return the same: there is a sensible display attached. Other drivers (and drm/msm/dp internally) use !branch || (sink_count > 0). > If a driver only drives the link inbetween atomic_enable() and > atomic_disable() will the "connected state" then ever be reported as > "connected"? (I'm sure I'm still missing pieces of this puzzle). I don't probably get the question. Nothing stops the driver from accessing the AUX bus outside of the atomic_enable/disable() brackets. > > Regards, > Bjorn -- With best wishes Dmitry
