On Thu, Jun 06, 2019 at 10:36:38AM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <[email protected]> > > These functions operate on ggtt so make them take that directly as > parameter.
This patch makes me wonder where we really want and need to go. We need to move out of dev_priv and global i915... but do we need to go and reduce to all minimal stuff used like uncore and ggtt or could we find a middle solution where each group has its own "class"? like this guc stuff would keep the intel_guc, but the i915_gem stuff or intel_gt stuff would have their own structs where we have everything needed for that group? > > At the same time move the USES_GUC conditional down to > intel_guc_reserve_ggtt_top for symmetry with > intel_guc_reserved_gtt_size. > > Signed-off-by: Tvrtko Ursulin <[email protected]> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 14 ++++++-------- > drivers/gpu/drm/i915/intel_guc.c | 18 ++++++++---------- > drivers/gpu/drm/i915/intel_guc.h | 6 +++--- > 3 files changed, 17 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index d3b3676d10f3..d967a4e9ceb0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2912,7 +2912,7 @@ int i915_gem_init_ggtt(struct drm_i915_private > *dev_priv) > * why. > */ > ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE, > - intel_guc_reserved_gtt_size(&dev_priv->guc)); > + intel_guc_reserved_gtt_size(ggtt)); > > ret = intel_vgt_balloon(ggtt); > if (ret) > @@ -2926,11 +2926,9 @@ int i915_gem_init_ggtt(struct drm_i915_private > *dev_priv) > if (ret) > return ret; > > - if (USES_GUC(dev_priv)) { > - ret = intel_guc_reserve_ggtt_top(&dev_priv->guc); > - if (ret) > - goto err_reserve; > - } > + ret = intel_guc_reserve_ggtt_top(ggtt); > + if (ret) > + goto err_reserve; > > /* Clear any non-preallocated blocks */ > drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) { > @@ -2952,7 +2950,7 @@ int i915_gem_init_ggtt(struct drm_i915_private > *dev_priv) > return 0; > > err_appgtt: > - intel_guc_release_ggtt_top(&dev_priv->guc); > + intel_guc_release_ggtt_top(ggtt); > err_reserve: > drm_mm_remove_node(&ggtt->error_capture); > return ret; > @@ -2979,7 +2977,7 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private > *dev_priv) > if (drm_mm_node_allocated(&ggtt->error_capture)) > drm_mm_remove_node(&ggtt->error_capture); > > - intel_guc_release_ggtt_top(&dev_priv->guc); > + intel_guc_release_ggtt_top(ggtt); > > if (drm_mm_initialized(&ggtt->vm.mm)) { > intel_vgt_deballoon(ggtt); > diff --git a/drivers/gpu/drm/i915/intel_guc.c > b/drivers/gpu/drm/i915/intel_guc.c > index b88c349c4fa6..633248b7da25 100644 > --- a/drivers/gpu/drm/i915/intel_guc.c > +++ b/drivers/gpu/drm/i915/intel_guc.c > @@ -719,7 +719,7 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc > *guc, u32 size) > > /** > * intel_guc_reserved_gtt_size() > - * @guc: intel_guc structure > + * @ggtt: Pointer to struct i915_ggtt > * > * The GuC WOPCM mapping shadows the lower part of the GGTT, so if we are > using > * GuC we can't have any objects pinned in that region. This function returns > @@ -729,18 +729,19 @@ struct i915_vma *intel_guc_allocate_vma(struct > intel_guc *guc, u32 size) > * 0 if GuC is not present or not in use. > * Otherwise, the GuC WOPCM size. > */ > -u32 intel_guc_reserved_gtt_size(struct intel_guc *guc) > +u32 intel_guc_reserved_gtt_size(struct i915_ggtt *ggtt) > { > - return guc_to_i915(guc)->wopcm.guc.size; > + return ggtt->vm.i915->wopcm.guc.size; > } > > -int intel_guc_reserve_ggtt_top(struct intel_guc *guc) > +int intel_guc_reserve_ggtt_top(struct i915_ggtt *ggtt) > { > - struct drm_i915_private *i915 = guc_to_i915(guc); > - struct i915_ggtt *ggtt = &i915->ggtt; > u64 size; > int ret; > > + if (!USES_GUC(ggtt->vm.i915)) > + return 0; > + > size = ggtt->vm.total - GUC_GGTT_TOP; > > ret = i915_gem_gtt_reserve(&ggtt->vm, &ggtt->uc_fw, size, > @@ -752,11 +753,8 @@ int intel_guc_reserve_ggtt_top(struct intel_guc *guc) > return ret; > } > > -void intel_guc_release_ggtt_top(struct intel_guc *guc) > +void intel_guc_release_ggtt_top(struct i915_ggtt *ggtt) > { > - struct drm_i915_private *i915 = guc_to_i915(guc); > - struct i915_ggtt *ggtt = &i915->ggtt; > - > if (drm_mm_node_allocated(&ggtt->uc_fw)) > drm_mm_remove_node(&ggtt->uc_fw); > } > diff --git a/drivers/gpu/drm/i915/intel_guc.h > b/drivers/gpu/drm/i915/intel_guc.h > index cbfed7a77c8b..55ea14176c5e 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -173,9 +173,9 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 > rsa_offset); > int intel_guc_suspend(struct intel_guc *guc); > int intel_guc_resume(struct intel_guc *guc); > struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size); > -u32 intel_guc_reserved_gtt_size(struct intel_guc *guc); > -int intel_guc_reserve_ggtt_top(struct intel_guc *guc); > -void intel_guc_release_ggtt_top(struct intel_guc *guc); > +u32 intel_guc_reserved_gtt_size(struct i915_ggtt *ggtt); > +int intel_guc_reserve_ggtt_top(struct i915_ggtt *ggtt); > +void intel_guc_release_ggtt_top(struct i915_ggtt *ggtt); > > static inline bool intel_guc_is_loaded(struct intel_guc *guc) > { > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
