| Message ID | 20191126210732.407496-3-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Handle SDEISR according to PCH rather than platform | expand |
On Tue, 2019-11-26 at 13:07 -0800, Matt Roper wrote: > The bspec tells us 'Program SHPD_FILTER_CNT with the "500 > microseconds > adjusted" value before enabling hotplug detection' on CNP+. We > haven't > been touching this register at all thus far, but we should probably > follow the bspec's guidance. > > The register also exists on LPT and SPT, but there isn't any specific > guidance I can find on how we should be programming it there so let's > leave it be for now. > > Bspec: 4342 > Bspec: 31297 > Bspec: 8407 > Bspec: 49305 > Bspec: 50473 > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 5 +++++ > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c > index dae00f7dd7df..028cb6239c12 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2976,6 +2976,8 @@ static void icp_hpd_irq_setup(struct > drm_i915_private *dev_priv, > hotplug_irqs = sde_ddi_mask | sde_tc_mask; > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, pins); > > + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > + > ibx_display_interrupt_update(dev_priv, hotplug_irqs, > enabled_irqs); > > icp_hpd_detection_setup(dev_priv, ddi_enable_mask, > tc_enable_mask); > @@ -3081,6 +3083,9 @@ static void spt_hpd_irq_setup(struct > drm_i915_private *dev_priv) > { > u32 hotplug_irqs, enabled_irqs; > > + if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) > + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > + > hotplug_irqs = SDE_HOTPLUG_MASK_SPT; > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_spt); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 94d0f593eeb7..74cf45de162e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8110,6 +8110,10 @@ enum { > > #define SHOTPLUG_CTL_TC _MMIO(0xc4034) > #define ICP_TC_HPD_ENABLE(tc_port) (8 << (tc_port) * 4) > + > +#define SHPD_FILTER_CNT _MMIO(0xc4038) > +#define SHPD_FILTER_CNT_500_ADJ 0x001D9 Shouldn't it be 0x1F2? Or I'm missing something? Also this is the default value. > + > /* Icelake DSC Rate Control Range Parameter Registers */ > #define DSCA_RC_RANGE_PARAMETERS_0 _MMIO(0x6B240) > #define DSCA_RC_RANGE_PARAMETERS_0_UDW _MMIO(0x6B240 + > 4)
On Tue, Nov 26, 2019 at 01:41:15PM -0800, Souza, Jose wrote: > On Tue, 2019-11-26 at 13:07 -0800, Matt Roper wrote: > > The bspec tells us 'Program SHPD_FILTER_CNT with the "500 > > microseconds > > adjusted" value before enabling hotplug detection' on CNP+. We > > haven't > > been touching this register at all thus far, but we should probably > > follow the bspec's guidance. > > > > The register also exists on LPT and SPT, but there isn't any specific > > guidance I can find on how we should be programming it there so let's > > leave it be for now. > > > > Bspec: 4342 > > Bspec: 31297 > > Bspec: 8407 > > Bspec: 49305 > > Bspec: 50473 > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 5 +++++ > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index dae00f7dd7df..028cb6239c12 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -2976,6 +2976,8 @@ static void icp_hpd_irq_setup(struct > > drm_i915_private *dev_priv, > > hotplug_irqs = sde_ddi_mask | sde_tc_mask; > > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, pins); > > > > + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > > + > > ibx_display_interrupt_update(dev_priv, hotplug_irqs, > > enabled_irqs); > > > > icp_hpd_detection_setup(dev_priv, ddi_enable_mask, > > tc_enable_mask); > > @@ -3081,6 +3083,9 @@ static void spt_hpd_irq_setup(struct > > drm_i915_private *dev_priv) > > { > > u32 hotplug_irqs, enabled_irqs; > > > > + if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) > > + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > > + > > hotplug_irqs = SDE_HOTPLUG_MASK_SPT; > > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_spt); > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 94d0f593eeb7..74cf45de162e 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -8110,6 +8110,10 @@ enum { > > > > #define SHOTPLUG_CTL_TC _MMIO(0xc4034) > > #define ICP_TC_HPD_ENABLE(tc_port) (8 << (tc_port) * 4) > > + > > +#define SHPD_FILTER_CNT _MMIO(0xc4038) > > +#define SHPD_FILTER_CNT_500_ADJ 0x001D9 > > Shouldn't it be 0x1F2? Or I'm missing something? > Also this is the default value. > 0x1F2 is the "500 microseconds" option, whereas 0x1D9 is the "500 microseconds adjusted" option according to bspec 8407/50473. The programming instructions tell us to use the non-default "adjusted" variant. I'm not sure why they don't just call it "475 microseconds" since that's what this value really works out to... Matt > > > + > > /* Icelake DSC Rate Control Range Parameter Registers */ > > #define DSCA_RC_RANGE_PARAMETERS_0 _MMIO(0x6B240) > > #define DSCA_RC_RANGE_PARAMETERS_0_UDW _MMIO(0x6B240 + > > 4)
On Tue, 2019-11-26 at 13:50 -0800, Matt Roper wrote: > On Tue, Nov 26, 2019 at 01:41:15PM -0800, Souza, Jose wrote: > > On Tue, 2019-11-26 at 13:07 -0800, Matt Roper wrote: > > > The bspec tells us 'Program SHPD_FILTER_CNT with the "500 > > > microseconds > > > adjusted" value before enabling hotplug detection' on CNP+. We > > > haven't > > > been touching this register at all thus far, but we should > > > probably > > > follow the bspec's guidance. > > > > > > The register also exists on LPT and SPT, but there isn't any > > > specific > > > guidance I can find on how we should be programming it there so > > > let's > > > leave it be for now. > > > > > > Bspec: 4342 > > > Bspec: 31297 > > > Bspec: 8407 > > > Bspec: 49305 > > > Bspec: 50473 > > > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_irq.c | 5 +++++ > > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > b/drivers/gpu/drm/i915/i915_irq.c > > > index dae00f7dd7df..028cb6239c12 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -2976,6 +2976,8 @@ static void icp_hpd_irq_setup(struct > > > drm_i915_private *dev_priv, > > > hotplug_irqs = sde_ddi_mask | sde_tc_mask; > > > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, pins); > > > > > > + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > > > + > > > ibx_display_interrupt_update(dev_priv, hotplug_irqs, > > > enabled_irqs); > > > > > > icp_hpd_detection_setup(dev_priv, ddi_enable_mask, > > > tc_enable_mask); > > > @@ -3081,6 +3083,9 @@ static void spt_hpd_irq_setup(struct > > > drm_i915_private *dev_priv) > > > { > > > u32 hotplug_irqs, enabled_irqs; > > > > > > + if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) > > > + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); > > > + > > > hotplug_irqs = SDE_HOTPLUG_MASK_SPT; > > > enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_spt); > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 94d0f593eeb7..74cf45de162e 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -8110,6 +8110,10 @@ enum { > > > > > > #define SHOTPLUG_CTL_TC _MMIO(0xc4034) > > > #define ICP_TC_HPD_ENABLE(tc_port) (8 << (tc_port) > > > * 4) > > > + > > > +#define SHPD_FILTER_CNT _MMIO(0xc4038) > > > +#define SHPD_FILTER_CNT_500_ADJ 0x001D9 > > > > Shouldn't it be 0x1F2? Or I'm missing something? > > Also this is the default value. > > > > 0x1F2 is the "500 microseconds" option, whereas 0x1D9 is the "500 > microseconds adjusted" option according to bspec 8407/50473. The > programming instructions tell us to use the non-default "adjusted" > variant. > > I'm not sure why they don't just call it "475 microseconds" since > that's > what this value really works out to... Oooh "Adjusted" smells like a workaround value Anyways: Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > > Matt > > > > + > > > /* Icelake DSC Rate Control Range Parameter Registers */ > > > #define DSCA_RC_RANGE_PARAMETERS_0 _MMIO(0x6B240) > > > #define DSCA_RC_RANGE_PARAMETERS_0_UDW _MMIO(0x6B240 + > > > 4)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index dae00f7dd7df..028cb6239c12 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2976,6 +2976,8 @@ static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv, hotplug_irqs = sde_ddi_mask | sde_tc_mask; enabled_irqs = intel_hpd_enabled_irqs(dev_priv, pins); + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); + ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs); icp_hpd_detection_setup(dev_priv, ddi_enable_mask, tc_enable_mask); @@ -3081,6 +3083,9 @@ static void spt_hpd_irq_setup(struct drm_i915_private *dev_priv) { u32 hotplug_irqs, enabled_irqs; + if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) + I915_WRITE(SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ); + hotplug_irqs = SDE_HOTPLUG_MASK_SPT; enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_spt); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 94d0f593eeb7..74cf45de162e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8110,6 +8110,10 @@ enum { #define SHOTPLUG_CTL_TC _MMIO(0xc4034) #define ICP_TC_HPD_ENABLE(tc_port) (8 << (tc_port) * 4) + +#define SHPD_FILTER_CNT _MMIO(0xc4038) +#define SHPD_FILTER_CNT_500_ADJ 0x001D9 + /* Icelake DSC Rate Control Range Parameter Registers */ #define DSCA_RC_RANGE_PARAMETERS_0 _MMIO(0x6B240) #define DSCA_RC_RANGE_PARAMETERS_0_UDW _MMIO(0x6B240 + 4)
The bspec tells us 'Program SHPD_FILTER_CNT with the "500 microseconds adjusted" value before enabling hotplug detection' on CNP+. We haven't been touching this register at all thus far, but we should probably follow the bspec's guidance. The register also exists on LPT and SPT, but there isn't any specific guidance I can find on how we should be programming it there so let's leave it be for now. Bspec: 4342 Bspec: 31297 Bspec: 8407 Bspec: 49305 Bspec: 50473 Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 5 +++++ drivers/gpu/drm/i915/i915_reg.h | 4 ++++ 2 files changed, 9 insertions(+)