[PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[Chris Wilson]

During suspend we want to flush out all active contexts and their
rendering. To do so we queue a request from the kernel's context, once
we know that request is done, we know the GPU is completely idle. To
speed up that switch bump the GPU clocks.

Switching to the kernel context prior to idling is also used to enforce
a barrier before changing OA properties, and when evicting active
rendering from the global GTT. All cases where we do want to
race-to-idle.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 58a2a44f88bd..ca1423ad2708 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -895,6 +895,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
 
 	for_each_engine(engine, dev_priv, id) {
 		struct drm_i915_gem_request *req;
+		bool active = false;
 		int ret;
 
 		if (engine_has_kernel_context(engine))
@@ -913,13 +914,17 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
 			prev = i915_gem_active_raw(&tl->last_request,
 						   &dev_priv->drm.struct_mutex);
 			if (prev)
-				i915_sw_fence_await_sw_fence_gfp(&req->submit,
-								 &prev->submit,
-								 GFP_KERNEL);
+				active |= i915_sw_fence_await_sw_fence_gfp(&req->submit,
+									   &prev->submit,
+									   GFP_KERNEL) > 0;
 		}
 
 		ret = i915_switch_context(req);
+
+		if (active)
+			gen6_rps_boost(req, NULL);
 		i915_add_request(req);
+
 		if (ret)
 			return ret;
 	}
-- 
2.14.1

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

✓ Fi.CI.BAT: success for drm/i915: "Race-to-idle" on switching to the kernel context

[Patchwork]

== Series Details ==

Series: drm/i915: "Race-to-idle" on switching to the kernel context
URL   : https://patchwork.freedesktop.org/series/28998/
State : success

== Summary ==

Series 28998v1 drm/i915: "Race-to-idle" on switching to the kernel context
https://patchwork.freedesktop.org/api/1.0/series/28998/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-n2820) fdo#101705
Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-kbl-7260u) fdo#102295

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

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:457s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:440s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:361s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:551s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:523s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:525s
fi-byt-n2820     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:513s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:610s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:444s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:425s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:422s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:494s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-kbl-7260u     total:279  pass:267  dwarn:2   dfail:0   fail:0   skip:10  time:494s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:597s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:605s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:524s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:465s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:481s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:495s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:437s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:484s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:548s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:405s

09654a2769e99c8117fc30942322cbc461e09d6c drm-tip: 2017y-08m-18d-13h-21m-33s UTC integration manifest
0aa5d846af76 drm/i915: "Race-to-idle" on switching to the kernel context

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5439/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> During suspend we want to flush out all active contexts and their
> rendering. To do so we queue a request from the kernel's context, once
> we know that request is done, we know the GPU is completely idle. To
> speed up that switch bump the GPU clocks.
>
> Switching to the kernel context prior to idling is also used to enforce
> a barrier before changing OA properties, and when evicting active
> rendering from the global GTT. All cases where we do want to
> race-to-idle.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 58a2a44f88bd..ca1423ad2708 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -895,6 +895,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
>  
>  	for_each_engine(engine, dev_priv, id) {
>  		struct drm_i915_gem_request *req;
> +		bool active = false;
>  		int ret;
>  
>  		if (engine_has_kernel_context(engine))
> @@ -913,13 +914,17 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
>  			prev = i915_gem_active_raw(&tl->last_request,
>  						   &dev_priv->drm.struct_mutex);
>  			if (prev)
> -				i915_sw_fence_await_sw_fence_gfp(&req->submit,
> -								 &prev->submit,
> -								 GFP_KERNEL);
> +				active |= i915_sw_fence_await_sw_fence_gfp(&req->submit,
> +									   &prev->submit,
> +									   GFP_KERNEL) > 0;

There is no point of kicking the clocks if we are the only request left?

Well logical as the request is empty, just pondering if the actual ctx
save/restore would finish quicker.

-Mika

>  		}
>  
>  		ret = i915_switch_context(req);
> +
> +		if (active)
> +			gen6_rps_boost(req, NULL);
>  		i915_add_request(req);
> +
>  		if (ret)
>  			return ret;
>  	}
> -- 
> 2.14.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

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[Chris Wilson]

Quoting Mika Kuoppala (2017-08-21 10:17:52)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > During suspend we want to flush out all active contexts and their
> > rendering. To do so we queue a request from the kernel's context, once
> > we know that request is done, we know the GPU is completely idle. To
> > speed up that switch bump the GPU clocks.
> >
> > Switching to the kernel context prior to idling is also used to enforce
> > a barrier before changing OA properties, and when evicting active
> > rendering from the global GTT. All cases where we do want to
> > race-to-idle.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_context.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 58a2a44f88bd..ca1423ad2708 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -895,6 +895,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
> >  
> >       for_each_engine(engine, dev_priv, id) {
> >               struct drm_i915_gem_request *req;
> > +             bool active = false;
> >               int ret;
> >  
> >               if (engine_has_kernel_context(engine))
> > @@ -913,13 +914,17 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
> >                       prev = i915_gem_active_raw(&tl->last_request,
> >                                                  &dev_priv->drm.struct_mutex);
> >                       if (prev)
> > -                             i915_sw_fence_await_sw_fence_gfp(&req->submit,
> > -                                                              &prev->submit,
> > -                                                              GFP_KERNEL);
> > +                             active |= i915_sw_fence_await_sw_fence_gfp(&req->submit,
> > +                                                                        &prev->submit,
> > +                                                                        GFP_KERNEL) > 0;
> 
> There is no point of kicking the clocks if we are the only request left?
> 
> Well logical as the request is empty, just pondering if the actual ctx
> save/restore would finish quicker.

I was thinking if it was just the context save itself, it not would be
enough of a difference to justify itself. Just gut feeling and not
measured, I worry about the irony of boosting from idle just to idle.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[Chris Wilson]

Quoting Chris Wilson (2017-08-21 10:28:16)
> Quoting Mika Kuoppala (2017-08-21 10:17:52)
> > Chris Wilson <chris@chris-wilson.co.uk> writes:
> > 
> > > During suspend we want to flush out all active contexts and their
> > > rendering. To do so we queue a request from the kernel's context, once
> > > we know that request is done, we know the GPU is completely idle. To
> > > speed up that switch bump the GPU clocks.
> > >
> > > Switching to the kernel context prior to idling is also used to enforce
> > > a barrier before changing OA properties, and when evicting active
> > > rendering from the global GTT. All cases where we do want to
> > > race-to-idle.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem_context.c | 11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > > index 58a2a44f88bd..ca1423ad2708 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > > @@ -895,6 +895,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
> > >  
> > >       for_each_engine(engine, dev_priv, id) {
> > >               struct drm_i915_gem_request *req;
> > > +             bool active = false;
> > >               int ret;
> > >  
> > >               if (engine_has_kernel_context(engine))
> > > @@ -913,13 +914,17 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
> > >                       prev = i915_gem_active_raw(&tl->last_request,
> > >                                                  &dev_priv->drm.struct_mutex);
> > >                       if (prev)
> > > -                             i915_sw_fence_await_sw_fence_gfp(&req->submit,
> > > -                                                              &prev->submit,
> > > -                                                              GFP_KERNEL);
> > > +                             active |= i915_sw_fence_await_sw_fence_gfp(&req->submit,
> > > +                                                                        &prev->submit,
> > > +                                                                        GFP_KERNEL) > 0;
> > 
> > There is no point of kicking the clocks if we are the only request left?
> > 
> > Well logical as the request is empty, just pondering if the actual ctx
> > save/restore would finish quicker.
> 
> I was thinking if it was just the context save itself, it not would be
> enough of a difference to justify itself. Just gut feeling and not
> measured, I worry about the irony of boosting from idle just to idle.

Hmm, or we could be more precise and just set the clocks high rather
than queue a task. The complication isn't worth it for just a single
callsite, but I am contemplating supplying boost/clocks information
along with the request.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Chris Wilson (2017-08-21 10:28:16)
>> Quoting Mika Kuoppala (2017-08-21 10:17:52)
>> > Chris Wilson <chris@chris-wilson.co.uk> writes:
>> > 
>> > > During suspend we want to flush out all active contexts and their
>> > > rendering. To do so we queue a request from the kernel's context, once
>> > > we know that request is done, we know the GPU is completely idle. To
>> > > speed up that switch bump the GPU clocks.
>> > >
>> > > Switching to the kernel context prior to idling is also used to enforce
>> > > a barrier before changing OA properties, and when evicting active
>> > > rendering from the global GTT. All cases where we do want to
>> > > race-to-idle.
>> > >
>> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > > Cc: David Weinehall <david.weinehall@linux.intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/i915_gem_context.c | 11 ++++++++---
>> > >  1 file changed, 8 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
>> > > index 58a2a44f88bd..ca1423ad2708 100644
>> > > --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> > > @@ -895,6 +895,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
>> > >  
>> > >       for_each_engine(engine, dev_priv, id) {
>> > >               struct drm_i915_gem_request *req;
>> > > +             bool active = false;
>> > >               int ret;
>> > >  
>> > >               if (engine_has_kernel_context(engine))
>> > > @@ -913,13 +914,17 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
>> > >                       prev = i915_gem_active_raw(&tl->last_request,
>> > >                                                  &dev_priv->drm.struct_mutex);
>> > >                       if (prev)
>> > > -                             i915_sw_fence_await_sw_fence_gfp(&req->submit,
>> > > -                                                              &prev->submit,
>> > > -                                                              GFP_KERNEL);
>> > > +                             active |= i915_sw_fence_await_sw_fence_gfp(&req->submit,
>> > > +                                                                        &prev->submit,
>> > > +                                                                        GFP_KERNEL) > 0;
>> > 
>> > There is no point of kicking the clocks if we are the only request left?
>> > 
>> > Well logical as the request is empty, just pondering if the actual ctx
>> > save/restore would finish quicker.
>> 
>> I was thinking if it was just the context save itself, it not would be
>> enough of a difference to justify itself. Just gut feeling and not
>> measured, I worry about the irony of boosting from idle just to idle.
>
> Hmm, or we could be more precise and just set the clocks high rather
> than queue a task. The complication isn't worth it for just a single
> callsite, but I am contemplating supplying boost/clocks information
> along with the request.

For the purposes of suspend, I think the approach is simple and
good enough.

Can David give a Tested-by?

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

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

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[David Weinehall]

On Mon, Aug 21, 2017 at 12:48:03PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Chris Wilson (2017-08-21 10:28:16)
> >> Quoting Mika Kuoppala (2017-08-21 10:17:52)
> >> > Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> > 
> >> > > During suspend we want to flush out all active contexts and their
> >> > > rendering. To do so we queue a request from the kernel's context, once
> >> > > we know that request is done, we know the GPU is completely idle. To
> >> > > speed up that switch bump the GPU clocks.
> >> > >
> >> > > Switching to the kernel context prior to idling is also used to enforce
> >> > > a barrier before changing OA properties, and when evicting active
> >> > > rendering from the global GTT. All cases where we do want to
> >> > > race-to-idle.
> >> > >
> >> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> > > Cc: David Weinehall <david.weinehall@linux.intel.com>
> >> > > ---
> >> > >  drivers/gpu/drm/i915/i915_gem_context.c | 11 ++++++++---
> >> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> >> > >
> >> > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> >> > > index 58a2a44f88bd..ca1423ad2708 100644
> >> > > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> >> > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> >> > > @@ -895,6 +895,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
> >> > >  
> >> > >       for_each_engine(engine, dev_priv, id) {
> >> > >               struct drm_i915_gem_request *req;
> >> > > +             bool active = false;
> >> > >               int ret;
> >> > >  
> >> > >               if (engine_has_kernel_context(engine))
> >> > > @@ -913,13 +914,17 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
> >> > >                       prev = i915_gem_active_raw(&tl->last_request,
> >> > >                                                  &dev_priv->drm.struct_mutex);
> >> > >                       if (prev)
> >> > > -                             i915_sw_fence_await_sw_fence_gfp(&req->submit,
> >> > > -                                                              &prev->submit,
> >> > > -                                                              GFP_KERNEL);
> >> > > +                             active |= i915_sw_fence_await_sw_fence_gfp(&req->submit,
> >> > > +                                                                        &prev->submit,
> >> > > +                                                                        GFP_KERNEL) > 0;
> >> > 
> >> > There is no point of kicking the clocks if we are the only request left?
> >> > 
> >> > Well logical as the request is empty, just pondering if the actual ctx
> >> > save/restore would finish quicker.
> >> 
> >> I was thinking if it was just the context save itself, it not would be
> >> enough of a difference to justify itself. Just gut feeling and not
> >> measured, I worry about the irony of boosting from idle just to idle.
> >
> > Hmm, or we could be more precise and just set the clocks high rather
> > than queue a task. The complication isn't worth it for just a single
> > callsite, but I am contemplating supplying boost/clocks information
> > along with the request.
> 
> For the purposes of suspend, I think the approach is simple and
> good enough.
> 
> Can David give a Tested-by?

Didn't notice this until now, but I'll give it a whirl.

> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> > -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[David Weinehall]

On Fri, Aug 18, 2017 at 03:08:15PM +0100, Chris Wilson wrote:
> During suspend we want to flush out all active contexts and their
> rendering. To do so we queue a request from the kernel's context, once
> we know that request is done, we know the GPU is completely idle. To
> speed up that switch bump the GPU clocks.
> 
> Switching to the kernel context prior to idling is also used to enforce
> a barrier before changing OA properties, and when evicting active
> rendering from the global GTT. All cases where we do want to
> race-to-idle.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@linux.intel.com>

No statistically significant speedup on suspend in our typical
benchmark, but that one doesn't take into account systems in load--it
suspends from idle, and from the description it seems that this patch
would mostly affect systems with load.

But no regression either, so I'm fine with the patch.

Tested-by: David Weinehall <david.weinehall@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 58a2a44f88bd..ca1423ad2708 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -895,6 +895,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
>  
>  	for_each_engine(engine, dev_priv, id) {
>  		struct drm_i915_gem_request *req;
> +		bool active = false;
>  		int ret;
>  
>  		if (engine_has_kernel_context(engine))
> @@ -913,13 +914,17 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
>  			prev = i915_gem_active_raw(&tl->last_request,
>  						   &dev_priv->drm.struct_mutex);
>  			if (prev)
> -				i915_sw_fence_await_sw_fence_gfp(&req->submit,
> -								 &prev->submit,
> -								 GFP_KERNEL);
> +				active |= i915_sw_fence_await_sw_fence_gfp(&req->submit,
> +									   &prev->submit,
> +									   GFP_KERNEL) > 0;
>  		}
>  
>  		ret = i915_switch_context(req);
> +
> +		if (active)
> +			gen6_rps_boost(req, NULL);
>  		i915_add_request(req);
> +
>  		if (ret)
>  			return ret;
>  	}
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[Chris Wilson]

Quoting David Weinehall (2017-08-23 15:54:13)
> On Fri, Aug 18, 2017 at 03:08:15PM +0100, Chris Wilson wrote:
> > During suspend we want to flush out all active contexts and their
> > rendering. To do so we queue a request from the kernel's context, once
> > we know that request is done, we know the GPU is completely idle. To
> > speed up that switch bump the GPU clocks.
> > 
> > Switching to the kernel context prior to idling is also used to enforce
> > a barrier before changing OA properties, and when evicting active
> > rendering from the global GTT. All cases where we do want to
> > race-to-idle.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: David Weinehall <david.weinehall@linux.intel.com>
> 
> No statistically significant speedup on suspend in our typical
> benchmark, but that one doesn't take into account systems in load--it
> suspends from idle, and from the description it seems that this patch
> would mostly affect systems with load.

In terms of everything else, I doubt we ever are significantly waiting
for the GPU upon suspend, the user interface would finish showing its
"going to suspend" screen before starting the suspend, so its only going
to be background tasks still rendering to the gpu oblivious of the
incoming suspend. Rare -- I'm going to squirrel this patch away until we
have a need for it.

Thanks for the review and testing, and if you do come across a workload
which could benefit do let me know. It may well be that userspace isn't
as smart as I expect...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: "Race-to-idle" on switching to the kernel context

[David Weinehall]

On Wed, Aug 23, 2017 at 04:03:44PM +0100, Chris Wilson wrote:
> Quoting David Weinehall (2017-08-23 15:54:13)
> > On Fri, Aug 18, 2017 at 03:08:15PM +0100, Chris Wilson wrote:
> > > During suspend we want to flush out all active contexts and their
> > > rendering. To do so we queue a request from the kernel's context, once
> > > we know that request is done, we know the GPU is completely idle. To
> > > speed up that switch bump the GPU clocks.
> > > 
> > > Switching to the kernel context prior to idling is also used to enforce
> > > a barrier before changing OA properties, and when evicting active
> > > rendering from the global GTT. All cases where we do want to
> > > race-to-idle.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > 
> > No statistically significant speedup on suspend in our typical
> > benchmark, but that one doesn't take into account systems in load--it
> > suspends from idle, and from the description it seems that this patch
> > would mostly affect systems with load.
> 
> In terms of everything else, I doubt we ever are significantly waiting
> for the GPU upon suspend, the user interface would finish showing its
> "going to suspend" screen before starting the suspend, so its only going
> to be background tasks still rendering to the gpu oblivious of the
> incoming suspend. Rare -- I'm going to squirrel this patch away until we
> have a need for it.

I suspect that the most common use-case for suspend is laptops,
triggered by the user closing the lid; ""going to suspend" screens
are gonna go unseen by most users.

> Thanks for the review and testing, and if you do come across a workload
> which could benefit do let me know. It may well be that userspace isn't
> as smart as I expect...

I think the main reason that I didn't see much improvement in our
benchmarks is the way we measure suspend times; to be able to get
numbers that are comparable night after night we "normalise" the load by
running a non-measured suspend/resume right before the one we actually
measure. This means that by the time we benchmark we have already
performed the flushing that your patch achieves, so the benefits of your
patch wouldn't be visible.

With your patch we get more stable results already on the first run,
so there definitely is a benefit.


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