| Message ID | 20240917052307.760662-2-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reduce SHPD_FILTER_CNT value | expand |
On Tue, Sep 17, 2024 at 10:53:06AM +0530, Suraj Kandpal wrote: > Reduce SHPD_CNT to 250us for Display12 to implement WA 14013120569 > in a alternative way. Its not what the Wa asks to do but has the same > effect which would be detecting shpd when it is less than 250us and > this would be okay as it lines up with DP1.4a(Table3-4) spec. > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c > index 2c4e946d5575..05a9e82cac75 100644 > --- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c > +++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c > @@ -849,7 +849,12 @@ static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv) > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->display.hotplug.pch_hpd); > hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->display.hotplug.pch_hpd); > > - if (INTEL_PCH_TYPE(dev_priv) <= PCH_TGP) > + /* > + * As an alternative to WA_14013120569 we reduce the value to 250us to be able to If I remember correctly, when the SHPD_FILTER_CNT programming was first added in commit 4948738e296c ("drm/i915/hotplug: Reduce SHPD_FILTER to 250us") the justification was that we thought it could serve as a cleaner alternative to the steps recommended by Wa_14013120569. However when it was later discussed with the hardware teams in July 2023, they said that this was _not_ a valid replacement for Wa_14013120569; the steps given on the workaround are necessary due to the specific nature of the clock signals on some of these platforms. However changing the SHPD_FILTER_CNT value to 250us is good/desirable for different reasons (to align with the DP source requirements in the DP spec). So as far as the workaround is concerned, I think we need to either: - Go back and actually implement the workaround using the original steps the hardware team settled on. - Decide that we just won't implement the workaround because the recommended implementation is too ugly and Linux use cases aren't suffering any adverse effects by not implementing it. Either way, I think we should avoid mentioning that workaround in the comments here (and in the commit message) because it turns out that this is not actually a valid alternative. The justification given for switching to use a 250us on platforms with a TGL PCH should be focused specifically on alignment with the DP spec to avoid any confusion. Matt > + * detect SHPD when an external display is connected. This is also expected of > + * us as stated in DP1.4a Table 3-4. > + */ > + if (INTEL_PCH_TYPE(dev_priv) < PCH_TGP) > intel_uncore_write(&dev_priv->uncore, SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > else > intel_uncore_write(&dev_priv->uncore, SHPD_FILTER_CNT, SHPD_FILTER_CNT_250); > -- > 2.43.2 >
> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Saturday, September 21, 2024 2:56 AM > To: Kandpal, Suraj <suraj.kandpal@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH 1/2] drm/i915/hotplug: Reduce SHPD_FLITER_CNT for > Display12 > > On Tue, Sep 17, 2024 at 10:53:06AM +0530, Suraj Kandpal wrote: > > Reduce SHPD_CNT to 250us for Display12 to implement WA 14013120569 > in > > a alternative way. Its not what the Wa asks to do but has the same > > effect which would be detecting shpd when it is less than 250us and > > this would be okay as it lines up with DP1.4a(Table3-4) spec. > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c > > b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c > > index 2c4e946d5575..05a9e82cac75 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c > > @@ -849,7 +849,12 @@ static void icp_hpd_irq_setup(struct > drm_i915_private *dev_priv) > > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv- > >display.hotplug.pch_hpd); > > hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, > > dev_priv->display.hotplug.pch_hpd); > > > > - if (INTEL_PCH_TYPE(dev_priv) <= PCH_TGP) > > + /* > > + * As an alternative to WA_14013120569 we reduce the value to > 250us > > +to be able to > > If I remember correctly, when the SHPD_FILTER_CNT programming was first > added in commit 4948738e296c ("drm/i915/hotplug: Reduce SHPD_FILTER > to > 250us") the justification was that we thought it could serve as a cleaner > alternative to the steps recommended by Wa_14013120569. However when > it was later discussed with the hardware teams in July 2023, they said that > this was _not_ a valid replacement for Wa_14013120569; the steps given on > the workaround are necessary due to the specific nature of the clock signals > on some of these platforms. However changing the SHPD_FILTER_CNT value > to 250us is good/desirable for different reasons (to align with the DP source > requirements in the DP spec). > > So as far as the workaround is concerned, I think we need to either: > - Go back and actually implement the workaround using the original > steps the hardware team settled on. > - Decide that we just won't implement the workaround because the > recommended implementation is too ugly and Linux use cases aren't > suffering any adverse effects by not implementing it. So we wont be implementing this wa and ill remove the reference of the wa from Commit and comments and only keep the dp spec reason for implementing this change Regards, Suraj Kandpal > > Either way, I think we should avoid mentioning that workaround in the > comments here (and in the commit message) because it turns out that this > is not actually a valid alternative. The justification given for switching to use > a 250us on platforms with a TGL PCH should be focused specifically on > alignment with the DP spec to avoid any confusion. > > > Matt > > > + * detect SHPD when an external display is connected. This is also > expected of > > + * us as stated in DP1.4a Table 3-4. > > + */ > > + if (INTEL_PCH_TYPE(dev_priv) < PCH_TGP) > > intel_uncore_write(&dev_priv->uncore, SHPD_FILTER_CNT, > SHPD_FILTER_CNT_500_ADJ); > > else > > intel_uncore_write(&dev_priv->uncore, SHPD_FILTER_CNT, > > SHPD_FILTER_CNT_250); > > -- > > 2.43.2 > > > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c index 2c4e946d5575..05a9e82cac75 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug_irq.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug_irq.c @@ -849,7 +849,12 @@ static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv) enabled_irqs = intel_hpd_enabled_irqs(dev_priv, dev_priv->display.hotplug.pch_hpd); hotplug_irqs = intel_hpd_hotplug_irqs(dev_priv, dev_priv->display.hotplug.pch_hpd); - if (INTEL_PCH_TYPE(dev_priv) <= PCH_TGP) + /* + * As an alternative to WA_14013120569 we reduce the value to 250us to be able to + * detect SHPD when an external display is connected. This is also expected of + * us as stated in DP1.4a Table 3-4. + */ + if (INTEL_PCH_TYPE(dev_priv) < PCH_TGP) intel_uncore_write(&dev_priv->uncore, SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); else intel_uncore_write(&dev_priv->uncore, SHPD_FILTER_CNT, SHPD_FILTER_CNT_250);
Reduce SHPD_CNT to 250us for Display12 to implement WA 14013120569 in a alternative way. Its not what the Wa asks to do but has the same effect which would be detecting shpd when it is less than 250us and this would be okay as it lines up with DP1.4a(Table3-4) spec. Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_hotplug_irq.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)