[PATCH] drm/i915: Restrict GPU boost to the RCS engine

[PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Chris Wilson]

Make the assumption that media workloads are not as latency sensitive
for __wait_seqno, and that upclocking the GPU does not affect the BLT
engine. Under that assumption, we only wait to forcibly upclock the GPU
when we are stalling for results from the render pipeline.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5951618a6b08..242b595a0403 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 
 	timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
 
-	if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
+	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
 		if (file_priv)
 			mod_delayed_work(dev_priv->wq,
-- 
2.0.0

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Daniel Vetter]

Adding Deepak for testing, this hopefully alleviates the bad
side-effects of the gpu booster he's seeing.
-Daniel

On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Make the assumption that media workloads are not as latency sensitive
> for __wait_seqno, and that upclocking the GPU does not affect the BLT
> engine. Under that assumption, we only wait to forcibly upclock the GPU
> when we are stalling for results from the render pipeline.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5951618a6b08..242b595a0403 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
>
>         timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
>
> -       if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
> +       if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>                 gen6_rps_boost(dev_priv);
>                 if (file_priv)
>                         mod_delayed_work(dev_priv->wq,
> --
> 2.0.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Deepak S]

On Thursday 12 June 2014 03:02 PM, Daniel Vetter wrote:
> Adding Deepak for testing, this hopefully alleviates the bad
> side-effects of the gpu booster he's seeing.
> -Daniel
>
> On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Make the assumption that media workloads are not as latency sensitive
>> for __wait_seqno, and that upclocking the GPU does not affect the BLT
>> engine. Under that assumption, we only wait to forcibly upclock the GPU
>> when we are stalling for results from the render pipeline.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 5951618a6b08..242b595a0403 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
>>
>>          timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
>>
>> -       if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
>> +       if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>>                  gen6_rps_boost(dev_priv);
>>                  if (file_priv)
>>                          mod_delayed_work(dev_priv->wq,
>> --
>> 2.0.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
Thanks Daniel. Apologies for delayed response.
Yup the changes make sense. I will test and share the results.

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Deepak S]

Hi Chris/Daniel,

The patch is  helping in some of the side-effects due to gpu boost. I still need to get more data. I will keep the thread updated.

Thanks
Deepak

On Thursday 12 June 2014 03:02 PM, Daniel Vetter wrote:
> Adding Deepak for testing, this hopefully alleviates the bad
> side-effects of the gpu booster he's seeing.
> -Daniel
>
> On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Make the assumption that media workloads are not as latency sensitive
>> for __wait_seqno, and that upclocking the GPU does not affect the BLT
>> engine. Under that assumption, we only wait to forcibly upclock the GPU
>> when we are stalling for results from the render pipeline.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 5951618a6b08..242b595a0403 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
>>
>>          timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
>>
>> -       if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
>> +       if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>>                  gen6_rps_boost(dev_priv);
>>                  if (file_priv)
>>                          mod_delayed_work(dev_priv->wq,
>> --
>> 2.0.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Daniel Vetter]

On Tue, Jun 24, 2014 at 05:22:17PM +0530, Deepak S wrote:
> Hi Chris/Daniel,
> 
> The patch is  helping in some of the side-effects due to gpu boost. I
> still need to get more data. I will keep the thread updated.

Ping. Might as well review it, too.
-Daniel

> 
> Thanks
> Deepak
> 
> On Thursday 12 June 2014 03:02 PM, Daniel Vetter wrote:
> >Adding Deepak for testing, this hopefully alleviates the bad
> >side-effects of the gpu booster he's seeing.
> >-Daniel
> >
> >On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >>Make the assumption that media workloads are not as latency sensitive
> >>for __wait_seqno, and that upclocking the GPU does not affect the BLT
> >>engine. Under that assumption, we only wait to forcibly upclock the GPU
> >>when we are stalling for results from the render pipeline.
> >>
> >>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>---
> >>  drivers/gpu/drm/i915/i915_gem.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >>index 5951618a6b08..242b595a0403 100644
> >>--- a/drivers/gpu/drm/i915/i915_gem.c
> >>+++ b/drivers/gpu/drm/i915/i915_gem.c
> >>@@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
> >>
> >>         timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
> >>
> >>-       if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
> >>+       if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
> >>                 gen6_rps_boost(dev_priv);
> >>                 if (file_priv)
> >>                         mod_delayed_work(dev_priv->wq,
> >>--
> >>2.0.0
> >>
> >>_______________________________________________
> >>Intel-gfx mailing list
> >>Intel-gfx@lists.freedesktop.org
> >>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Chris Wilson]

On Wed, Jul 09, 2014 at 10:09:00AM +0530, Deepak S wrote:
> 
> On Monday 07 July 2014 02:35 PM, Daniel Vetter wrote:
> >On Tue, Jun 24, 2014 at 05:22:17PM +0530, Deepak S wrote:
> >>Hi Chris/Daniel,
> >>
> >>The patch is  helping in some of the side-effects due to gpu boost. I
> >>still need to get more data. I will keep the thread updated.
> >Ping. Might as well review it, too.
> >-Daniel
> >
> >>Thanks
> >>Deepak
> >>
> >>On Thursday 12 June 2014 03:02 PM, Daniel Vetter wrote:
> >>>Adding Deepak for testing, this hopefully alleviates the bad
> >>>side-effects of the gpu booster he's seeing.
> >>>-Daniel
> >>>
> >>>On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >>>>Make the assumption that media workloads are not as latency sensitive
> >>>>for __wait_seqno, and that upclocking the GPU does not affect the BLT
> >>>>engine. Under that assumption, we only wait to forcibly upclock the GPU
> >>>>when we are stalling for results from the render pipeline.
> >>>>
> >>>>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>---
> >>>>  drivers/gpu/drm/i915/i915_gem.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >>>>index 5951618a6b08..242b595a0403 100644
> >>>>--- a/drivers/gpu/drm/i915/i915_gem.c
> >>>>+++ b/drivers/gpu/drm/i915/i915_gem.c
> >>>>@@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
> >>>>
> >>>>         timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
> >>>>
> >>>>-       if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
> >>>>+       if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
> >>>>                 gen6_rps_boost(dev_priv);
> >>>>                 if (file_priv)
> >>>>                         mod_delayed_work(dev_priv->wq,
> >>>>--
> >>>>2.0.0
> >>>>
> >>>>_______________________________________________
> >>>>Intel-gfx mailing list
> >>>>Intel-gfx@lists.freedesktop.org
> >>>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>>
> Sorry for the delayed response. I am fine with this patch. This is helping us in some of the workload senarios.
> 
> Just one concern here. Since we are avoiding boost for Media related workload, we might end up running GPU at RPn. Which might impact media workload.
> I think we need another patch to run GPU at RPe if the boost is not enabled for media.

That should already be taken care of by the normal upclocking logic.
Except that byt is still using those original ineffectual values for
reclocking....
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Daniel Vetter]

On Wed, Jul 09, 2014 at 10:09:00AM +0530, Deepak S wrote:
> 
> On Monday 07 July 2014 02:35 PM, Daniel Vetter wrote:
> >On Tue, Jun 24, 2014 at 05:22:17PM +0530, Deepak S wrote:
> >>Hi Chris/Daniel,
> >>
> >>The patch is  helping in some of the side-effects due to gpu boost. I
> >>still need to get more data. I will keep the thread updated.
> >Ping. Might as well review it, too.
> >-Daniel
> >
> >>Thanks
> >>Deepak
> >>
> >>On Thursday 12 June 2014 03:02 PM, Daniel Vetter wrote:
> >>>Adding Deepak for testing, this hopefully alleviates the bad
> >>>side-effects of the gpu booster he's seeing.
> >>>-Daniel
> >>>
> >>>On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >>>>Make the assumption that media workloads are not as latency sensitive
> >>>>for __wait_seqno, and that upclocking the GPU does not affect the BLT
> >>>>engine. Under that assumption, we only wait to forcibly upclock the GPU
> >>>>when we are stalling for results from the render pipeline.
> >>>>
> >>>>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>---
> >>>>  drivers/gpu/drm/i915/i915_gem.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >>>>index 5951618a6b08..242b595a0403 100644
> >>>>--- a/drivers/gpu/drm/i915/i915_gem.c
> >>>>+++ b/drivers/gpu/drm/i915/i915_gem.c
> >>>>@@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
> >>>>
> >>>>         timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
> >>>>
> >>>>-       if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
> >>>>+       if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
> >>>>                 gen6_rps_boost(dev_priv);
> >>>>                 if (file_priv)
> >>>>                         mod_delayed_work(dev_priv->wq,
> >>>>--
> >>>>2.0.0
> >>>>
> >>>>_______________________________________________
> >>>>Intel-gfx mailing list
> >>>>Intel-gfx@lists.freedesktop.org
> >>>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>>
> Sorry for the delayed response. I am fine with this patch. This is helping us in some of the workload senarios.
> 
> Just one concern here. Since we are avoiding boost for Media related workload, we might end up running GPU at RPn. Which might impact media workload.
> I think we need another patch to run GPU at RPe if the boost is not enabled for media.
> 
> Reviewed-by: Deepak S<deepak.s@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Deepak S]

On Monday 07 July 2014 02:35 PM, Daniel Vetter wrote:
> On Tue, Jun 24, 2014 at 05:22:17PM +0530, Deepak S wrote:
>> Hi Chris/Daniel,
>>
>> The patch is  helping in some of the side-effects due to gpu boost. I
>> still need to get more data. I will keep the thread updated.
> Ping. Might as well review it, too.
> -Daniel
>
>> Thanks
>> Deepak
>>
>> On Thursday 12 June 2014 03:02 PM, Daniel Vetter wrote:
>>> Adding Deepak for testing, this hopefully alleviates the bad
>>> side-effects of the gpu booster he's seeing.
>>> -Daniel
>>>
>>> On Thu, Jun 12, 2014 at 11:28 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>>> Make the assumption that media workloads are not as latency sensitive
>>>> for __wait_seqno, and that upclocking the GPU does not affect the BLT
>>>> engine. Under that assumption, we only wait to forcibly upclock the GPU
>>>> when we are stalling for results from the render pipeline.
>>>>
>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_gem.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>>> index 5951618a6b08..242b595a0403 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>>> @@ -1409,7 +1409,7 @@ static int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
>>>>
>>>>          timeout_expire = timeout ? jiffies + timespec_to_jiffies_timeout(timeout) : 0;
>>>>
>>>> -       if (INTEL_INFO(dev)->gen >= 6 && can_wait_boost(file_priv)) {
>>>> +       if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>>>>                  gen6_rps_boost(dev_priv);
>>>>                  if (file_priv)
>>>>                          mod_delayed_work(dev_priv->wq,
>>>> --
>>>> 2.0.0
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
Sorry for the delayed response. I am fine with this patch. This is helping us in some of the workload senarios.

Just one concern here. Since we are avoiding boost for Media related workload, we might end up running GPU at RPn. Which might impact media workload.
I think we need another patch to run GPU at RPe if the boost is not enabled for media.

Reviewed-by: Deepak S<deepak.s@linux.intel.com>  

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Chris Wilson]

On Thu, Jun 12, 2014 at 10:28:55AM +0100, Chris Wilson wrote:
> Make the assumption that media workloads are not as latency sensitive
> for __wait_seqno, and that upclocking the GPU does not affect the BLT
> engine. Under that assumption, we only wait to forcibly upclock the GPU
> when we are stalling for results from the render pipeline.

For reference, the BLT does depend upon the gpu frequency (and acts as
a media source of busyness), and to lesser extent the ring/memory bw,
also controlled as a side-effect of gpu frequency.

Tested on ivb, i7-3720QM
-Chris

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

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Daniel Vetter]

On Wed, Dec 17, 2014 at 03:12:05PM +0000, Chris Wilson wrote:
> On Thu, Jun 12, 2014 at 10:28:55AM +0100, Chris Wilson wrote:
> > Make the assumption that media workloads are not as latency sensitive
> > for __wait_seqno, and that upclocking the GPU does not affect the BLT
> > engine. Under that assumption, we only wait to forcibly upclock the GPU
> > when we are stalling for results from the render pipeline.
> 
> For reference, the BLT does depend upon the gpu frequency (and acts as
> a media source of busyness), and to lesser extent the ring/memory bw,
> also controlled as a side-effect of gpu frequency.
> 
> Tested on ivb, i7-3720QM

Since the patch landed already, can you please submit a fixup to add the
|| ring == BLT with Deepak on cc so he can review it?

Thanks, 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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Restrict GPU boost to the RCS engine

[Chris Wilson]

On Wed, Dec 17, 2014 at 06:02:06PM +0100, Daniel Vetter wrote:
> On Wed, Dec 17, 2014 at 03:12:05PM +0000, Chris Wilson wrote:
> > On Thu, Jun 12, 2014 at 10:28:55AM +0100, Chris Wilson wrote:
> > > Make the assumption that media workloads are not as latency sensitive
> > > for __wait_seqno, and that upclocking the GPU does not affect the BLT
> > > engine. Under that assumption, we only wait to forcibly upclock the GPU
> > > when we are stalling for results from the render pipeline.
> > 
> > For reference, the BLT does depend upon the gpu frequency (and acts as
> > a media source of busyness), and to lesser extent the ring/memory bw,
> > also controlled as a side-effect of gpu frequency.
> > 
> > Tested on ivb, i7-3720QM
> 
> Since the patch landed already, can you please submit a fixup to add the
> || ring == BLT with Deepak on cc so he can review it?

I am not overly concerned as I don't have a pure BLT benchmark that
isn't quickly ramped to maximum by the more responsive RPS tuning we
have in IVB/HSW (and patches exist for VLV). I also prefer the concept
of detecting dropped frames for boosting, irrespective of waits. That is
the best I have for making gnome-shell snappy.
-Chris

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