On Tue, Dec 03, 2024 at 06:36:45PM -0800, Abhinav Kumar wrote:
>
>
> On 12/3/2024 5:50 AM, Dmitry Baryshkov wrote:
> > On Mon, Dec 02, 2024 at 04:39:00PM -0800, Abhinav Kumar wrote:
> > > In commit 8ede2ecc3e5ee ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets"),
> > > checks were introduced to avoid handling any plug or irq hpd events in
> > > ST_DISPLAY_OFF state.
> > >
> > > Even if we do get hpd events, after the bridge was disabled,
> > > it should get handled. Moreover, its unclear under what circumstances
> > > these events will fire because ST_DISPLAY_OFF means that the link was
> > > still connected but only the bridge was disabled. If the link was untouched,
> > > then interrupts shouldn't fire.
> >
> > What about the link being untouched, but the monitor being toggled
> > somehow, which might generate HPD / attention events?
> >
>
> To confirm my understanding of this, I tested this case again with and
> without a dongle on sc7180.
>
> Without a dongle - When the monitor is powered off and powered on, without
> physically disturbing the cable, it still generates a hotplug disconnect
> event when powered off and hotplug connect event when its powered on again.
>
> It gets handled the same way as a user would hotplug it using the cable.
>
> With a dongle - When the monitor is powered off , nothing happens and it
> stays in connected state. When the monitor is powered back on, it generates
> a hotplug disable followed by hotplug enable event. This behavior is
> consistent with or without this series with this dongle.
>
> So from the DP driver point of view, for both these cases, its just another
> hotlplug disconnect/connect.
>
> Both these cases still work fine with these changes.
Thanks for the confirmation!
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> > >
> > > Even in the case of the DP compliance equipment, it should be raising these
> > > interrupts during the start of the test which is usually accompanied with either
> > > a HPD pulse or a IRQ HPD but after the bridge is disabled it should be fine
> > > to handle these anyway. In the absence of a better reason to keep these checks,
> > > drop these and if any other issues do arise, it should be handled in a different
> > > way.
> > >
> > > Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/msm/dp/dp_display.c | 10 ----------
> > > 1 file changed, 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index aba925aab7ad..992184cc17e4 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -562,11 +562,6 @@ static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp, u32 data)
> > > drm_dbg_dp(dp->drm_dev, "Before, type=%d hpd_state=%d\n",
> > > dp->msm_dp_display.connector_type, state);
> > > - if (state == ST_DISPLAY_OFF) {
> > > - mutex_unlock(&dp->event_mutex);
> > > - return 0;
> > > - }
> > > -
> > > if (state == ST_MAINLINK_READY || state == ST_CONNECTED) {
> > > mutex_unlock(&dp->event_mutex);
> > > return 0;
> > > @@ -689,11 +684,6 @@ static int msm_dp_irq_hpd_handle(struct msm_dp_display_private *dp, u32 data)
> > > drm_dbg_dp(dp->drm_dev, "Before, type=%d hpd_state=%d\n",
> > > dp->msm_dp_display.connector_type, state);
> > > - if (state == ST_DISPLAY_OFF) {
> > > - mutex_unlock(&dp->event_mutex);
> > > - return 0;
> > > - }
> > > -
> > > if (state == ST_MAINLINK_READY || state == ST_DISCONNECT_PENDING) {
> > > /* wait until ST_CONNECTED */
> > > msm_dp_add_event(dp, EV_IRQ_HPD_INT, 0, 1); /* delay = 1 */
> > >
> > > --
> > > 2.34.1
> > >
> >
--
With best wishes
Dmitry
