Re: [PATCH] drm/i915/display: Add skip link check quirk

[Jonathan Denose <jdenose@google.com>]

On Wed, Jan 15, 2025 at 9:47 AM Jonathan Denose <jdenose@google.com> wrote:
>
> On Wed, Jan 15, 2025 at 3:10 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> > On Tue, 14 Jan 2025, Jonathan Denose <jdenose@google.com> wrote:
> > > The display on the Advantech UTC124G3PWWW0E-ES worked fine until commit
> > > "drm/i915/dp: Recheck link state after modeset" was introduced. After
> > > this commit the display flickers intermittently as the driver code
> > > initiates the delayed link recheck in an infinite loop.
> > >
> > > To resolve this issue for the Advantech device, add a quirk to skip over
> > > the delayed link recheck.
> >
> > It would be better to try to get at the root cause first, instead of
> > blindly skipping required parts. The code's complicated enough without
> > quirks, and removing them afterwards is very difficult.
> >
> > Please file a bug according to [1], and attach full dmesg with debugs
> > enabled.
>
> I filed a bug with the full dmesg a few weeks ago:
> https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13344
>
> > Cc: Imre
> >
> > BR,
> > Jani.
> >
> >
> >
> > [1] https://drm.pages.freedesktop.org/intel-docs/how-to-file-i915-bugs.html
> >
> >
> >
> > >
> > > Signed-off-by: Jonathan Denose <jdenose@google.com>
> > > ---
> > >
> > >  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 4 +++-
> > >  drivers/gpu/drm/i915/display/intel_quirks.c           | 8 ++++++++
> > >  drivers/gpu/drm/i915/display/intel_quirks.h           | 1 +
> > >  3 files changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > index 397cc4ebae526..7804ad38b00cd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > @@ -32,6 +32,7 @@
> > >  #include "intel_encoder.h"
> > >  #include "intel_hotplug.h"
> > >  #include "intel_panel.h"
> > > +#include "intel_quirks.h"
> > >
> > >  #define LT_MSG_PREFIX                        "[CONNECTOR:%d:%s][ENCODER:%d:%s][%s] "
> > >  #define LT_MSG_ARGS(_intel_dp, _dp_phy)      (_intel_dp)->attached_connector->base.base.id, \
> > > @@ -1622,7 +1623,8 @@ void intel_dp_start_link_train(struct intel_atomic_state *state,
> > >               lt_dbg(intel_dp, DP_PHY_DPRX, "Forcing link training failure\n");
> > >       } else if (passed) {
> > >               intel_dp->link.seq_train_failures = 0;
> > > -             intel_encoder_link_check_queue_work(encoder, 2000);
> > > +             if (!intel_has_quirk(display, QUIRK_SKIP_LINK_CHECK))
> > > +                     intel_encoder_link_check_queue_work(encoder, 2000);
> > >               return;
> > >       }
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c
> > > index 28f497ae785bb..d472a5f21f8b9 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_quirks.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_quirks.c
> > > @@ -78,6 +78,12 @@ static void quirk_fw_sync_len(struct intel_dp *intel_dp)
> > >       drm_info(display->drm, "Applying Fast Wake sync pulse count quirk\n");
> > >  }
> > >
> > > +static void quirk_skip_link_check(struct intel_display *display)
> > > +{
> > > +     intel_set_quirk(display, QUIRK_SKIP_LINK_CHECK);
> > > +     drm_info(display->drm, "Applying skip link check quirk\n");
> > > +}
> > > +
> > >  struct intel_quirk {
> > >       int device;
> > >       int subsystem_vendor;
> > > @@ -229,6 +235,8 @@ static struct intel_quirk intel_quirks[] = {
> > >       { 0x3184, 0x1019, 0xa94d, quirk_increase_ddi_disabled_time },
> > >       /* HP Notebook - 14-r206nv */
> > >       { 0x0f31, 0x103c, 0x220f, quirk_invert_brightness },
> > > +     /* Advantech UTC124G3PWWW0E-ES */
> > > +     {0x5a85, 0x8086, 0x2212, quirk_skip_link_check},
> > >  };
> > >
> > >  static const struct intel_dpcd_quirk intel_dpcd_quirks[] = {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_quirks.h b/drivers/gpu/drm/i915/display/intel_quirks.h
> > > index cafdebda75354..9e8f2816a4fba 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_quirks.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_quirks.h
> > > @@ -20,6 +20,7 @@ enum intel_quirk_id {
> > >       QUIRK_LVDS_SSC_DISABLE,
> > >       QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK,
> > >       QUIRK_FW_SYNC_LEN,
> > > +     QUIRK_SKIP_LINK_CHECK,
> > >  };
> > >
> > >  void intel_init_quirks(struct intel_display *display);
> >
> > --
> > Jani Nikula, Intel
> --
> Jonathan

Hello,

After discussing on the mailing list, Imre found that it was a link
status reporting issue and followed up with some patches at [1]. Are
these patches closer to what you'd be looking for since they more
directly address the root cause?

[1]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13344#note_2773364

-- 
Jonathan