[Intel-gfx] [PATCH 3/3] drm/i915: Program SHPD_FILTER_CNT on CNP+

[Souza, Jose]

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