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: Tue, 9 Apr 2024 00:12:28 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc: Bjorn Andersson <andersson@...nel.org>, freedreno@...ts.freedesktop.org, 
	Rob Clark <robdclark@...il.com>, 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 Mon, 8 Apr 2024 at 22:43, Abhinav Kumar <quic_abhinavk@...cinc.com> 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@...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.

Should it then have the Co-developed-by trailers?

> 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.

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.

> >>      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
> >>



-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ