[Intel-gfx] [RFC] drm/i915: Allow the UMD to configure their own power clock state

[Michał Winiarski]

On Wed, May 10, 2017 at 04:47:50PM +0300, Mika Kuoppala wrote:
> Oscar Mateo <oscar.mateo at intel.com> writes:
> 
> > This allows userspace to shutdown slices at will for performance/power reasons
> > (because it doesn't have a use for more slices).
> >
> > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_engine_cs.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index 402769d..17ff88d 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -628,6 +628,7 @@ static int wa_ring_whitelist_reg(struct intel_engine_cs *engine,
> >  static int gen8_init_workarounds(struct intel_engine_cs *engine)
> >  {
> >  	struct drm_i915_private *dev_priv = engine->i915;
> > +	int ret;
> >  
> >  	WA_SET_BIT_MASKED(INSTPM, INSTPM_FORCE_ORDERING);
> >  
> > @@ -673,6 +674,11 @@ static int gen8_init_workarounds(struct intel_engine_cs *engine)
> >  			    GEN6_WIZ_HASHING_MASK,
> >  			    GEN6_WIZ_HASHING_16x4);
> >  
> > +	/* Allow the UMD to configure their own power clock state */
> > +	ret = wa_ring_whitelist_reg(engine, GEN8_R_PWR_CLK_STATE);
> > +	if (ret)
> > +		return ret;
> > +
> 
> How will the different userspace clients coordinate with the slice
> state? I guess in another words, is this part of the context?
> 
> -Mika
> 

Spec says (see IHD-OS-SKL-Vol2c-05.16 page 614):
"This register must be initialized correctly when the context is submitted for
the first time. This register is context save/restored as part of Exec-List
context image in both Exec-List and Ring-Buffer mode of scheduling."

So we're good, right?

Except there's also:
"This register must not be programmed using MI_LOAD_REGISTER_IMM command in ring
buffer or in batch buffer, however programming "NON - SLM Indication" field
through MI_LOAD_REGISTER_IMM is an exception defined below. If a need arises to
change the render configuration for a context being executed in HW, Scheduler
must preempt the context and update the desired render configuration in the
logical render context image in memory and resubmit the context"

So... are we sure that we're fine with giving userspace control over this
register? If so - it would still be a good idea to mention that the info in the
spec is bogus (commit message or just a comment).

-Michał

> 
> >  	return 0;
> >  }
> >  
> > @@ -841,6 +847,11 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine)
> >  	if (ret)
> >  		return ret;
> >  
> > +	/* Allow the UMD to configure their own power clock state */
> > +	ret = wa_ring_whitelist_reg(engine, GEN8_R_PWR_CLK_STATE);
> > +	if (ret)
> > +		return ret;
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx