| Message ID | 20161118133647.4868-2-david.weinehall@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <david.weinehall@linux.intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > --- > 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.co.uk> -Chris
On Fri, Nov 18, 2016 at 01:58:35PM +0000, 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 <david.weinehall@linux.intel.com> > > CC: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > 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? Kind regards, David
On Mon, Nov 21, 2016 at 02:21:39PM +0200, David Weinehall wrote: > On Fri, Nov 18, 2016 at 01:58:35PM +0000, 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 <david.weinehall@linux.intel.com> > > > CC: Chris Wilson <chris@chris-wilson.co.uk> > > > --- > > > 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
On Mon, Nov 21, 2016 at 12:30:01PM +0000, 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
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 = &dev_priv->ggtt; @@ -3299,12 +3309,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv) if (vma->vm != &ggtt->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)
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 <david.weinehall@linux.intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)