All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] drm/i915: Allow the UMD to configure their own power clock state
@ 2017-05-02 15:07 Oscar Mateo
  2017-05-02 22:24 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Oscar Mateo @ 2017-05-02 15:07 UTC (permalink / raw)
  To: intel-gfx

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@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Oscar Mateo <oscar.mateo@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;
+
 	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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Allow the UMD to configure their own power clock state
  2017-05-02 15:07 [RFC] drm/i915: Allow the UMD to configure their own power clock state Oscar Mateo
@ 2017-05-02 22:24 ` Patchwork
  2017-05-10 12:59 ` [RFC] " Joonas Lahtinen
  2017-05-10 13:47 ` Mika Kuoppala
  2 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-05-02 22:24 UTC (permalink / raw)
  To: Oscar Mateo; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Allow the UMD to configure their own power clock state
URL   : https://patchwork.freedesktop.org/series/23846/
State : success

== Summary ==

Series 23846v1 drm/i915: Allow the UMD to configure their own power clock state
https://patchwork.freedesktop.org/api/1.0/series/23846/revisions/1/mbox/

Test drv_module_reload:
        Subgroup basic-reload-final:
                dmesg-warn -> PASS       (fi-skl-6770hq) fdo#100248

fdo#100248 https://bugs.freedesktop.org/show_bug.cgi?id=100248

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:437s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:427s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:581s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:508s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:562s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:491s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:491s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:409s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:405s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:493s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:464s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:458s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:568s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:450s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:579s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:461s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:490s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:431s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:400s

7f027554339633cb4e6a2d3974510aaef1a9e24c drm-tip: 2017y-05m-02d-18h-48m-28s UTC integration manifest
7a90a9b drm/i915: Allow the UMD to configure their own power clock state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4602/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-10 14:09   ` Michał Winiarski
@ 2017-05-10  8:29     ` Oscar Mateo
  2017-05-10 14:26     ` Chris Wilson
  1 sibling, 0 replies; 10+ messages in thread
From: Oscar Mateo @ 2017-05-10  8:29 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx




On 05/10/2017 02:09 PM, Michał Winiarski wrote:
> On Wed, May 10, 2017 at 04:47:50PM +0300, Mika Kuoppala wrote:
>> Oscar Mateo <oscar.mateo@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@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Signed-off-by: Oscar Mateo <oscar.mateo@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

As Michal said, this is context-saved/restored, so no need to coordinate 
between clients.

> 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?

Ditto :)

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

This approach has been double-checked with HW and they believe there 
should be no problem, as long as the update is done as:

- PIPECONTROL - Stalling flish, flush all caches (color, depth, DC$)
- LOAD_REGISTER_IMMEDIATE - R_PWR_CLK_STATE
- Reprogram complete state

I'm trying to get them to change the BSpec accordingly.

>>>   	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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-10 13:28   ` Daniel Vetter
@ 2017-05-10  8:33     ` Oscar Mateo
  2017-05-11 13:09       ` Joonas Lahtinen
  0 siblings, 1 reply; 10+ messages in thread
From: Oscar Mateo @ 2017-05-10  8:33 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx




On 05/10/2017 01:28 PM, Daniel Vetter wrote:
> On Wed, May 10, 2017 at 2:59 PM, Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com> wrote:
>>> @@ -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;
>> This is not a workaround, so it should be part of the cmd parser and
>> have an userspace.

cmd parser? This is Gen8+, so the cmd parser shouldn't even be active. 
Or am I missing something?

> Yeah, the "This allows userspace ..." start of the commit message
> should have been a dead giveaway that we do indeed need the userspace
> patch for this too.
> -Daniel

Yes, I got the memo :)
Dmitry has opened a ticket against intel-vaapi-driver 
(https://github.com/01org/intel-vaapi-driver/issues/152) so hopefully we 
will have a userspace soon.

-- Oscar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-02 15:07 [RFC] drm/i915: Allow the UMD to configure their own power clock state Oscar Mateo
  2017-05-02 22:24 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-05-10 12:59 ` Joonas Lahtinen
  2017-05-10 13:28   ` Daniel Vetter
  2017-05-10 13:47 ` Mika Kuoppala
  2 siblings, 1 reply; 10+ messages in thread
From: Joonas Lahtinen @ 2017-05-10 12:59 UTC (permalink / raw)
  To: Oscar Mateo, intel-gfx; +Cc: Daniel Vetter

On ti, 2017-05-02 at 15:07 +0000, Oscar Mateo wrote:
> 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@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>

<SNIP>

> @@ -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;
> +
>  	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;

This is not a workaround, so it should be part of the cmd parser and
have an userspace.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-10 12:59 ` [RFC] " Joonas Lahtinen
@ 2017-05-10 13:28   ` Daniel Vetter
  2017-05-10  8:33     ` Oscar Mateo
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2017-05-10 13:28 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: intel-gfx

On Wed, May 10, 2017 at 2:59 PM, Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
>> @@ -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;
>
> This is not a workaround, so it should be part of the cmd parser and
> have an userspace.

Yeah, the "This allows userspace ..." start of the commit message
should have been a dead giveaway that we do indeed need the userspace
patch for this too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-02 15:07 [RFC] drm/i915: Allow the UMD to configure their own power clock state Oscar Mateo
  2017-05-02 22:24 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-05-10 12:59 ` [RFC] " Joonas Lahtinen
@ 2017-05-10 13:47 ` Mika Kuoppala
  2017-05-10 14:09   ` Michał Winiarski
  2 siblings, 1 reply; 10+ messages in thread
From: Mika Kuoppala @ 2017-05-10 13:47 UTC (permalink / raw)
  To: Oscar Mateo, intel-gfx

Oscar Mateo <oscar.mateo@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@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Oscar Mateo <oscar.mateo@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


>  	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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-10 13:47 ` Mika Kuoppala
@ 2017-05-10 14:09   ` Michał Winiarski
  2017-05-10  8:29     ` Oscar Mateo
  2017-05-10 14:26     ` Chris Wilson
  0 siblings, 2 replies; 10+ messages in thread
From: Michał Winiarski @ 2017-05-10 14:09 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Wed, May 10, 2017 at 04:47:50PM +0300, Mika Kuoppala wrote:
> Oscar Mateo <oscar.mateo@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@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Oscar Mateo <oscar.mateo@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-10 14:09   ` Michał Winiarski
  2017-05-10  8:29     ` Oscar Mateo
@ 2017-05-10 14:26     ` Chris Wilson
  1 sibling, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2017-05-10 14:26 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

On Wed, May 10, 2017 at 04:09:34PM +0200, Michał Winiarski wrote:
> On Wed, May 10, 2017 at 04:47:50PM +0300, Mika Kuoppala wrote:
> > Oscar Mateo <oscar.mateo@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@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Oscar Mateo <oscar.mateo@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).

Doesn't sound like it. So we are back to doing the preempt and changing
the context image as suggested in the other patch. Still we have the
issue that OA config conflicts with changing the slice eu config.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] drm/i915: Allow the UMD to configure their own power clock state
  2017-05-10  8:33     ` Oscar Mateo
@ 2017-05-11 13:09       ` Joonas Lahtinen
  0 siblings, 0 replies; 10+ messages in thread
From: Joonas Lahtinen @ 2017-05-11 13:09 UTC (permalink / raw)
  To: Oscar Mateo, Daniel Vetter; +Cc: intel-gfx

On ke, 2017-05-10 at 08:33 +0000, Oscar Mateo wrote:
> 
> 
> On 05/10/2017 01:28 PM, Daniel Vetter wrote:
> > 
> > On Wed, May 10, 2017 at 2:59 PM, Joonas Lahtinen
> > <joonas.lahtinen@linux.intel.com> wrote:
> > > 
> > > > 
> > > > @@ -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;
> > > This is not a workaround, so it should be part of the cmd parser
> > > and
> > > have an userspace.
> 
> cmd parser? This is Gen8+, so the cmd parser shouldn't even be
> active. 
> Or am I missing something?

That's correct, my bad.

> 
> > 
> > Yeah, the "This allows userspace ..." start of the commit message
> > should have been a dead giveaway that we do indeed need the
> > userspace
> > patch for this too.
> > -Daniel
> 
> Yes, I got the memo :)
> Dmitry has opened a ticket against intel-vaapi-driver 
> (https://github.com/01org/intel-vaapi-driver/issues/152) so hopefully
> we 
> will have a userspace soon.

So with the userspace, we can merge.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-11 13:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-02 15:07 [RFC] drm/i915: Allow the UMD to configure their own power clock state Oscar Mateo
2017-05-02 22:24 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-05-10 12:59 ` [RFC] " Joonas Lahtinen
2017-05-10 13:28   ` Daniel Vetter
2017-05-10  8:33     ` Oscar Mateo
2017-05-11 13:09       ` Joonas Lahtinen
2017-05-10 13:47 ` Mika Kuoppala
2017-05-10 14:09   ` Michał Winiarski
2017-05-10  8:29     ` Oscar Mateo
2017-05-10 14:26     ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.