Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Cleanup i915_gem_restore_gtt_mappings()

[David Weinehall]

On Mon, Nov 21, 2016 at 12:30:01PM +, Chris Wilson wrote:
> 
> I was looking at how we get out of the pm_runtime mess. In part, we hurt
> ourselves because we are using wakeref_count to disable asserts, but it
> also seems that pushing the optimisation to pm_runtime is the right
> thing to do.

Any more thoughts on this?


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

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Cleanup i915_gem_restore_gtt_mappings()

[Chris Wilson]

On Mon, Nov 21, 2016 at 02:21:39PM +0200, David Weinehall wrote:
> On Fri, Nov 18, 2016 at 01:58:35PM +, Chris Wilson wrote:
> > On Fri, Nov 18, 2016 at 03:36:45PM +0200, David Weinehall wrote:
> > > On resume we unbind+bind our VMA-mappings. This patch simplifies
> > > this a bit by introducing a restore_vma() helper.  As a nice side-effect
> > > this also makes the resume callgraph self-documenting.
> > > 
> > > v2: move the helper to i915_gem_gtt.c since it's only used by
> > > i915_gem_restore_gtt_mappings(), rename to restore_vma(),
> > > and make static (Chris)
> > > 
> > > Signed-off-by: David Weinehall 
> > > CC: Chris Wilson 
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +++--
> > >  1 file changed, 11 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index b4bde1452f2a..2711044e3bf2 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -3277,6 +3277,16 @@ int i915_ggtt_enable_hw(struct drm_i915_private 
> > > *dev_priv)
> > >   return 0;
> > >  }
> > >  
> > > +static bool restore_vma(struct i915_vma *vma)
> > > +{
> > > + if (i915_vma_is_pinned(vma)) {
> > > + WARN_ON(i915_vma_bind(vma, vma->obj->cache_level, PIN_UPDATE));
> > > + return true;
> > > + } else {
> > > + WARN_ON(i915_vma_unbind(vma));
> > > + return false;
> > > + }
> > > +}
> > 
> > Missed a blank line [checkpatch]
> 
> Dang.
> 
> Will you fix this manually for the merge, or do you want a fixed patch?

I was looking at how we get out of the pm_runtime mess. In part, we hurt
ourselves because we are using wakeref_count to disable asserts, but it
also seems that pushing the optimisation to pm_runtime is the right
thing to do.
-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

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Cleanup i915_gem_restore_gtt_mappings()

[Chris Wilson]

On Fri, Nov 18, 2016 at 03:36:45PM +0200, David Weinehall wrote:
> On resume we unbind+bind our VMA-mappings. This patch simplifies
> this a bit by introducing a restore_vma() helper.  As a nice side-effect
> this also makes the resume callgraph self-documenting.
> 
> v2: move the helper to i915_gem_gtt.c since it's only used by
> i915_gem_restore_gtt_mappings(), rename to restore_vma(),
> and make static (Chris)
> 
> Signed-off-by: David Weinehall 
> CC: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b4bde1452f2a..2711044e3bf2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3277,6 +3277,16 @@ int i915_ggtt_enable_hw(struct drm_i915_private 
> *dev_priv)
>   return 0;
>  }
>  
> +static bool restore_vma(struct i915_vma *vma)
> +{
> + if (i915_vma_is_pinned(vma)) {
> + WARN_ON(i915_vma_bind(vma, vma->obj->cache_level, PIN_UPDATE));
> + return true;
> + } else {
> + WARN_ON(i915_vma_unbind(vma));
> + return false;
> + }
> +}

Missed a blank line [checkpatch]

Reviewed-by: Chris Wilson 
-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

[Intel-gfx] [PATCH v2 1/3] drm/i915: Cleanup i915_gem_restore_gtt_mappings()

[David Weinehall]

On resume we unbind+bind our VMA-mappings. This patch simplifies
this a bit by introducing a restore_vma() helper.  As a nice side-effect
this also makes the resume callgraph self-documenting.

v2: move the helper to i915_gem_gtt.c since it's only used by
i915_gem_restore_gtt_mappings(), rename to restore_vma(),
and make static (Chris)

Signed-off-by: David Weinehall 
CC: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b4bde1452f2a..2711044e3bf2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3277,6 +3277,16 @@ int i915_ggtt_enable_hw(struct drm_i915_private 
*dev_priv)
return 0;
 }
 
+static bool restore_vma(struct i915_vma *vma)
+{
+   if (i915_vma_is_pinned(vma)) {
+   WARN_ON(i915_vma_bind(vma, vma->obj->cache_level, PIN_UPDATE));
+   return true;
+   } else {
+   WARN_ON(i915_vma_unbind(vma));
+   return false;
+   }
+}
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 {
struct i915_ggtt *ggtt = _priv->ggtt;
@@ -3299,12 +3309,7 @@ void i915_gem_restore_gtt_mappings(struct 
drm_i915_private *dev_priv)
if (vma->vm != >base)
continue;
 
-   if (!i915_vma_unbind(vma))
-   continue;
-
-   WARN_ON(i915_vma_bind(vma, obj->cache_level,
- PIN_UPDATE));
-   ggtt_bound = true;
+   ggtt_bound |= restore_vma(vma);
}
 
if (ggtt_bound)
-- 
2.10.2

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