[Intel-gfx] [PATCH 2/2] drm/i915/glk: Fix DMC/DC state idleness calculation

[Imre Deak]

On Tue, Oct 03, 2017 at 11:20:10AM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 03, 2017 at 05:47:50PM +0000, Rodrigo Vivi wrote:
> > On Tue, Oct 03, 2017 at 09:51:59AM +0000, Imre Deak wrote:
> > > According to BSpec GLK like BXT needs to ignore the idle state of cores
> > > before starting the DMC firmware's DC state handler.
> > 
> > no mention on CNL there?
> > 
> > Btw I just saw that CNL DMC seems much more like BXT than like SKL.
> > Our code probably needs deeper changes...
> 
> nevermind. I was wrong about this.
> The only difference I see on CNL compared with SKL now
> is the bit 31 of DC_STATE_EN...
> 
> A bit that we should set during modeset and clear after. But there is
> no mention about that bit on the main sequences page and also we
> leave dc states disabled during modeset anyways besides that
> clock gating wa... So I believe we are safe.
> 
> Imre, thoughts?

Yes, that bit would gate CSR_START which sounds redundant if we disable
DC states anyway around a mode set.

Art, could this be clarified under "Sequences for Display C5 and C6" or
the definition DC_STATE_EN?

Thanks,
Imre

> > > 
> > > Fixes: dbb28b5c3d3c ("drm/i915/DMC/GLK: Load DMC on GLK")
> > > Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_csr.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> > > index cdfb624eb82d..ea5d5c9645a4 100644
> > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > @@ -216,7 +216,7 @@ static void gen9_set_dc_state_debugmask(struct drm_i915_private *dev_priv)
> > >  
> > >  	mask = DC_STATE_DEBUG_MASK_MEMORY_UP;
> > >  
> > > -	if (IS_BROXTON(dev_priv))
> > > +	if (IS_GEN9_LP(dev_priv))
> > >  		mask |= DC_STATE_DEBUG_MASK_CORES;
> > 
> > I saw in spec for SKL+: "This field must be set to Mask prior to enabling DC5 or DC6"
> > So I believe this should be INTEL_GEN(dev_priv) >= 9... :/
> > 
> > 
> > >  
> > >  	/* The below bit doesn't need to be cleared ever afterwards */
> > > -- 
> > > 2.13.2
> > >