Message ID | 20190613133539.12620-21-tvrtko.ursulin@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Implicit dev_priv removal and GT compartmentalization | expand |
Quoting Tvrtko Ursulin (2019-06-13 14:35:31) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Having made start to better code compartmentalization by introducing > struct intel_gt, continue the theme elsewhere in code by making functions > take parameters take what logically makes most sense for them instead of > the global struct drm_i915_private. So I am debating whether this is better off as part of i915_ggtt and not i915_address_space. -Chris
On 13/06/2019 15:08, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-06-13 14:35:31) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Having made start to better code compartmentalization by introducing >> struct intel_gt, continue the theme elsewhere in code by making functions >> take parameters take what logically makes most sense for them instead of >> the global struct drm_i915_private. > > So I am debating whether this is better off as part of i915_ggtt and not > i915_address_space. In terms of what object is passed in? They mostly operate on but, but end with ggtt->invalidate(ggtt). And while we could move that vfunc to vm (and have it NULL for !ggtt) would it make sense? Regards, Tvrtko
Quoting Tvrtko Ursulin (2019-06-14 10:51:59) > > On 13/06/2019 15:08, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2019-06-13 14:35:31) > >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >> Having made start to better code compartmentalization by introducing > >> struct intel_gt, continue the theme elsewhere in code by making functions > >> take parameters take what logically makes most sense for them instead of > >> the global struct drm_i915_private. > > > > So I am debating whether this is better off as part of i915_ggtt and not > > i915_address_space. > > In terms of what object is passed in? No, I was thinking placement of the intel_gt backpointer. Just trying to assess whether i915_address_space is an abstract concept... But it needs to be tied to a struct device for dma_mapping, so I think I am talking rubbish. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 0bc75c963955..516ffc4a521a 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2323,23 +2323,28 @@ static bool needs_idle_maps(struct drm_i915_private *dev_priv) return IS_GEN(dev_priv, 5) && IS_MOBILE(dev_priv) && intel_vtd_active(); } -void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv) +static void ggtt_suspend_mappings(struct i915_ggtt *ggtt) { - struct i915_ggtt *ggtt = &dev_priv->ggtt; + struct drm_i915_private *i915 = ggtt->vm.i915; /* Don't bother messing with faults pre GEN6 as we have little * documentation supporting that it's a good idea. */ - if (INTEL_GEN(dev_priv) < 6) + if (INTEL_GEN(i915) < 6) return; - intel_gt_check_and_clear_faults(&dev_priv->gt); + intel_gt_check_and_clear_faults(ggtt->vm.gt); ggtt->vm.clear_range(&ggtt->vm, 0, ggtt->vm.total); ggtt->invalidate(ggtt); } +void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv) +{ + ggtt_suspend_mappings(&dev_priv->ggtt); +} + int i915_gem_gtt_prepare_pages(struct drm_i915_gem_object *obj, struct sg_table *pages) { @@ -3674,12 +3679,11 @@ void i915_ggtt_disable_guc(struct drm_i915_private *i915) ggtt->invalidate(ggtt); } -void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv) +static void ggtt_restore_mappings(struct i915_ggtt *ggtt) { - struct i915_ggtt *ggtt = &dev_priv->ggtt; struct i915_vma *vma, *vn; - intel_gt_check_and_clear_faults(&dev_priv->gt); + intel_gt_check_and_clear_faults(ggtt->vm.gt); mutex_lock(&ggtt->vm.mutex); @@ -3716,6 +3720,11 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv) ggtt->invalidate(ggtt); mutex_unlock(&ggtt->vm.mutex); +} + +void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv) +{ + ggtt_restore_mappings(&dev_priv->ggtt); if (INTEL_GEN(dev_priv) >= 8) { struct intel_ppat *ppat = &dev_priv->ppat;