| Message ID | 20190613151904.16256-2-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 16:19:01) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Continuing on the theme of better logical organization of our code, make > the first step towards making the ggtt code better isolated from wider > struct drm_i915_private. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 40 +++++++++++++++++++---------- > 1 file changed, 26 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 285a7a02c015..ea276ed9021a 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2873,7 +2873,13 @@ static void ggtt_release_guc_top(struct i915_ggtt *ggtt) > drm_mm_remove_node(&ggtt->uc_fw); > } > > -int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) > +static void cleanup_init_ggtt(struct i915_ggtt *ggtt) > +{ > + ggtt_release_guc_top(ggtt); > + drm_mm_remove_node(&ggtt->error_capture); > +} > + > +static int init_ggtt(struct i915_ggtt *ggtt) > { > /* Let GEM Manage all of the aperture. > * > @@ -2884,7 +2890,6 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) > * aperture. One page should be enough to keep any prefetching inside > * of the aperture. > */ > - struct i915_ggtt *ggtt = &dev_priv->ggtt; > struct i915_address_space *vm = &ggtt->vm; > unsigned long hole_start, hole_end; > struct drm_mm_node *entry; > @@ -2897,7 +2902,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) > * why. > */ > ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE, > - intel_wopcm_guc_size(&dev_priv->wopcm)); > + intel_wopcm_guc_size(&vm->i915->wopcm)); > > ret = intel_vgt_balloon(ggtt); > if (ret) > @@ -2917,8 +2922,10 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) > * GGTT as it can comfortably hold GuC/HuC firmware images. > */ > ret = ggtt_reserve_guc_top(ggtt); > - if (ret) > - goto err_reserve; > + if (ret) { > + cleanup_init_ggtt(ggtt); Joonas would be turning in his grave, can you not hear the haunting call of onions? > + return ret; > + } > > /* Clear any non-preallocated blocks */ > drm_mm_for_each_hole(entry, &vm->mm, hole_start, hole_end) { > @@ -2930,19 +2937,24 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) > /* And finally clear the reserved guard page */ > vm->clear_range(vm, vm->total - PAGE_SIZE, PAGE_SIZE); > > - if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) { > - ret = init_aliasing_ppgtt(dev_priv); > + return 0; > +} > + > +int i915_gem_init_ggtt(struct drm_i915_private *i915) Less with the i915_gem; this is not about gem/ i915->gem or the GEM API :) -Chris
On 13/06/2019 16:38, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-06-13 16:19:01) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Continuing on the theme of better logical organization of our code, make >> the first step towards making the ggtt code better isolated from wider >> struct drm_i915_private. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 40 +++++++++++++++++++---------- >> 1 file changed, 26 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> index 285a7a02c015..ea276ed9021a 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> @@ -2873,7 +2873,13 @@ static void ggtt_release_guc_top(struct i915_ggtt *ggtt) >> drm_mm_remove_node(&ggtt->uc_fw); >> } >> >> -int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) >> +static void cleanup_init_ggtt(struct i915_ggtt *ggtt) >> +{ >> + ggtt_release_guc_top(ggtt); >> + drm_mm_remove_node(&ggtt->error_capture); >> +} >> + >> +static int init_ggtt(struct i915_ggtt *ggtt) >> { >> /* Let GEM Manage all of the aperture. >> * >> @@ -2884,7 +2890,6 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) >> * aperture. One page should be enough to keep any prefetching inside >> * of the aperture. >> */ >> - struct i915_ggtt *ggtt = &dev_priv->ggtt; >> struct i915_address_space *vm = &ggtt->vm; >> unsigned long hole_start, hole_end; >> struct drm_mm_node *entry; >> @@ -2897,7 +2902,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) >> * why. >> */ >> ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE, >> - intel_wopcm_guc_size(&dev_priv->wopcm)); >> + intel_wopcm_guc_size(&vm->i915->wopcm)); >> >> ret = intel_vgt_balloon(ggtt); >> if (ret) >> @@ -2917,8 +2922,10 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) >> * GGTT as it can comfortably hold GuC/HuC firmware images. >> */ >> ret = ggtt_reserve_guc_top(ggtt); >> - if (ret) >> - goto err_reserve; >> + if (ret) { >> + cleanup_init_ggtt(ggtt); > > Joonas would be turning in his grave, can you not hear the haunting call > of onions? Wasn't happy with how that under-developed onion looked, but I am also not happy without it. :/ >> + return ret; >> + } >> >> /* Clear any non-preallocated blocks */ >> drm_mm_for_each_hole(entry, &vm->mm, hole_start, hole_end) { >> @@ -2930,19 +2937,24 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) >> /* And finally clear the reserved guard page */ >> vm->clear_range(vm, vm->total - PAGE_SIZE, PAGE_SIZE); >> >> - if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) { >> - ret = init_aliasing_ppgtt(dev_priv); >> + return 0; >> +} >> + >> +int i915_gem_init_ggtt(struct drm_i915_private *i915) > > Less with the i915_gem; this is not about gem/ i915->gem or the GEM API :) Sometimes I try to minimize the diff, and sometimes I create a big one for no reason. Go figure. :) i915_init_ggtt I suppose? Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 285a7a02c015..ea276ed9021a 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2873,7 +2873,13 @@ static void ggtt_release_guc_top(struct i915_ggtt *ggtt) drm_mm_remove_node(&ggtt->uc_fw); } -int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) +static void cleanup_init_ggtt(struct i915_ggtt *ggtt) +{ + ggtt_release_guc_top(ggtt); + drm_mm_remove_node(&ggtt->error_capture); +} + +static int init_ggtt(struct i915_ggtt *ggtt) { /* Let GEM Manage all of the aperture. * @@ -2884,7 +2890,6 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) * aperture. One page should be enough to keep any prefetching inside * of the aperture. */ - struct i915_ggtt *ggtt = &dev_priv->ggtt; struct i915_address_space *vm = &ggtt->vm; unsigned long hole_start, hole_end; struct drm_mm_node *entry; @@ -2897,7 +2902,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) * why. */ ggtt->pin_bias = max_t(u32, I915_GTT_PAGE_SIZE, - intel_wopcm_guc_size(&dev_priv->wopcm)); + intel_wopcm_guc_size(&vm->i915->wopcm)); ret = intel_vgt_balloon(ggtt); if (ret) @@ -2917,8 +2922,10 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) * GGTT as it can comfortably hold GuC/HuC firmware images. */ ret = ggtt_reserve_guc_top(ggtt); - if (ret) - goto err_reserve; + if (ret) { + cleanup_init_ggtt(ggtt); + return ret; + } /* Clear any non-preallocated blocks */ drm_mm_for_each_hole(entry, &vm->mm, hole_start, hole_end) { @@ -2930,19 +2937,24 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) /* And finally clear the reserved guard page */ vm->clear_range(vm, vm->total - PAGE_SIZE, PAGE_SIZE); - if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) { - ret = init_aliasing_ppgtt(dev_priv); + return 0; +} + +int i915_gem_init_ggtt(struct drm_i915_private *i915) +{ + int ret; + + ret = init_ggtt(&i915->ggtt); + if (ret) + return ret; + + if (INTEL_PPGTT(i915) == INTEL_PPGTT_ALIASING) { + ret = init_aliasing_ppgtt(i915); if (ret) - goto err_appgtt; + cleanup_init_ggtt(&i915->ggtt); } return 0; - -err_appgtt: - ggtt_release_guc_top(ggtt); -err_reserve: - drm_mm_remove_node(&ggtt->error_capture); - return ret; } static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)