Re: [Intel-gfx] [PATCH 11/18] drm/i915: switch to default context on idle
On Fri, Mar 30, 2012 at 02:17:20PM -0700, Ben Widawsky wrote:
> On Thu, 29 Mar 2012 21:29:05 +0200
> Daniel Vetter wrote:
>
> > On Sun, Mar 18, 2012 at 01:39:51PM -0700, Ben Widawsky wrote:
> > > To keep things as sane as possible, switch to the default context before
> > > idling. This should help free context objects, as well as put things in
> > > a more well defined state before suspending.
> > >
> > > Signed-off-by: Ben Widawsky
> >
> > Context are yet another thing that will nicely fragment our global gtt
> > space. You don't pin them to mappable, which is a start, but I wonder
> > whether we should integrate this into our evict_something code? Although
> > that project is more involved with the current evict code, so maybe just
> > add a fixme.
> >
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c |6 ++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > b/drivers/gpu/drm/i915/i915_gem.c
> > > index 0985aa5..c1aab45 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3685,6 +3685,8 @@ int
> > > i915_gem_idle(struct drm_device *dev)
> > > {
> > > drm_i915_private_t *dev_priv = dev->dev_private;
> > > + struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
> > > + u32 seqno;
> > > int ret;
> > >
> > > mutex_lock(&dev->struct_mutex);
> > > @@ -3694,6 +3696,10 @@ i915_gem_idle(struct drm_device *dev)
> > > return 0;
> > > }
> > >
> > > + seqno = i915_gem_next_request_seqno(ring);
> > > + i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID, seqno, 0);
> > > + WARN_ON(i915_wait_request(ring, seqno, false));
> >
> > This can block and potentially return early due to a signal, i.e. you need
> > to properly handle this (like the i915_gpu_idle below).
>
> Can you elaborate? Whether or not the context switch fails, I want to
> try to idle the GPU. So I *thought* this is doing just that.
wait_request can return error codes, you need to handle them. E.g. gpu
hang, interrupt, out of memory, whatnotelse.
-Daniel
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/18] drm/i915: switch to default context on idle
On Thu, 29 Mar 2012 21:29:05 +0200
Daniel Vetter wrote:
> On Sun, Mar 18, 2012 at 01:39:51PM -0700, Ben Widawsky wrote:
> > To keep things as sane as possible, switch to the default context before
> > idling. This should help free context objects, as well as put things in
> > a more well defined state before suspending.
> >
> > Signed-off-by: Ben Widawsky
>
> Context are yet another thing that will nicely fragment our global gtt
> space. You don't pin them to mappable, which is a start, but I wonder
> whether we should integrate this into our evict_something code? Although
> that project is more involved with the current evict code, so maybe just
> add a fixme.
>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c |6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 0985aa5..c1aab45 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3685,6 +3685,8 @@ int
> > i915_gem_idle(struct drm_device *dev)
> > {
> > drm_i915_private_t *dev_priv = dev->dev_private;
> > + struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
> > + u32 seqno;
> > int ret;
> >
> > mutex_lock(&dev->struct_mutex);
> > @@ -3694,6 +3696,10 @@ i915_gem_idle(struct drm_device *dev)
> > return 0;
> > }
> >
> > + seqno = i915_gem_next_request_seqno(ring);
> > + i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID, seqno, 0);
> > + WARN_ON(i915_wait_request(ring, seqno, false));
>
> This can block and potentially return early due to a signal, i.e. you need
> to properly handle this (like the i915_gpu_idle below).
Can you elaborate? Whether or not the context switch fails, I want to
try to idle the GPU. So I *thought* this is doing just that.
>
> > +
> > ret = i915_gpu_idle(dev, true);
> > if (ret) {
> > mutex_unlock(&dev->struct_mutex);
> > --
> > 1.7.9.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/18] drm/i915: switch to default context on idle
On Thu, 29 Mar 2012 21:29:05 +0200, Daniel Vetter wrote: > On Sun, Mar 18, 2012 at 01:39:51PM -0700, Ben Widawsky wrote: > > To keep things as sane as possible, switch to the default context before > > idling. This should help free context objects, as well as put things in > > a more well defined state before suspending. > > > > Signed-off-by: Ben Widawsky > > Context are yet another thing that will nicely fragment our global gtt > space. You don't pin them to mappable, which is a start, but I wonder > whether we should integrate this into our evict_something code? Although > that project is more involved with the current evict code, so maybe just > add a fixme. My suggestion for when we want to step beyond having the context pinned all the time was for a hook into unbind that checked to see if this was a "pinned" object and to fixup any registers to point back to the default. In the case of contexts, it would check a flag to see if it was indeed the active context and then schedule a change back to the default context prior to unbinding. From another viewpoint, this sounds alot like fences, and maybe we should add the tracking to the CXTID register in a similar manner. -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: [Intel-gfx] [PATCH 11/18] drm/i915: switch to default context on idle
On Sun, Mar 18, 2012 at 01:39:51PM -0700, Ben Widawsky wrote:
> To keep things as sane as possible, switch to the default context before
> idling. This should help free context objects, as well as put things in
> a more well defined state before suspending.
>
> Signed-off-by: Ben Widawsky
Context are yet another thing that will nicely fragment our global gtt
space. You don't pin them to mappable, which is a start, but I wonder
whether we should integrate this into our evict_something code? Although
that project is more involved with the current evict code, so maybe just
add a fixme.
> ---
> drivers/gpu/drm/i915/i915_gem.c |6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0985aa5..c1aab45 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3685,6 +3685,8 @@ int
> i915_gem_idle(struct drm_device *dev)
> {
> drm_i915_private_t *dev_priv = dev->dev_private;
> + struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
> + u32 seqno;
> int ret;
>
> mutex_lock(&dev->struct_mutex);
> @@ -3694,6 +3696,10 @@ i915_gem_idle(struct drm_device *dev)
> return 0;
> }
>
> + seqno = i915_gem_next_request_seqno(ring);
> + i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID, seqno, 0);
> + WARN_ON(i915_wait_request(ring, seqno, false));
This can block and potentially return early due to a signal, i.e. you need
to properly handle this (like the i915_gpu_idle below).
> +
> ret = i915_gpu_idle(dev, true);
> if (ret) {
> mutex_unlock(&dev->struct_mutex);
> --
> 1.7.9.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/18] drm/i915: switch to default context on idle
To keep things as sane as possible, switch to the default context before
idling. This should help free context objects, as well as put things in
a more well defined state before suspending.
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_gem.c |6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0985aa5..c1aab45 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3685,6 +3685,8 @@ int
i915_gem_idle(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
+ struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
+ u32 seqno;
int ret;
mutex_lock(&dev->struct_mutex);
@@ -3694,6 +3696,10 @@ i915_gem_idle(struct drm_device *dev)
return 0;
}
+ seqno = i915_gem_next_request_seqno(ring);
+ i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID, seqno, 0);
+ WARN_ON(i915_wait_request(ring, seqno, false));
+
ret = i915_gpu_idle(dev, true);
if (ret) {
mutex_unlock(&dev->struct_mutex);
--
1.7.9.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
