| Message ID | 20210929085950.3063191-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add ww context to intel_dpt_pin, v2. | expand |
On 9/29/21 10:59, Maarten Lankhorst wrote: > Ensure i915_vma_pin_iomap and vma_unpin are done with dpt->obj lock held. > > I don't think there's much of a point in merging intel_dpt_pin() with > intel_pin_fb_obj_dpt(), they touch different objects. > > Changes since v1: > - Fix using the wrong pointer to retrieve error code (Julia) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Julia Lawall <julia.lawall@lip6.fr> LGTM. Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_dpt.c | 40 +++++++++++++++--------- > 1 file changed, 25 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c > index de62bd77b15e..8f7b1f7534a4 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpt.c > +++ b/drivers/gpu/drm/i915/display/intel_dpt.c > @@ -121,32 +121,42 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm) > intel_wakeref_t wakeref; > struct i915_vma *vma; > void __iomem *iomem; > + struct i915_gem_ww_ctx ww; > + int err; > > wakeref = intel_runtime_pm_get(&i915->runtime_pm); > atomic_inc(&i915->gpu_error.pending_fb_pin); > > - vma = i915_gem_object_ggtt_pin(dpt->obj, NULL, 0, 4096, > - HAS_LMEM(i915) ? 0 : PIN_MAPPABLE); > - if (IS_ERR(vma)) > - goto err; > + for_i915_gem_ww(&ww, err, true) { > + err = i915_gem_object_lock(dpt->obj, &ww); > + if (err) > + continue; > > - iomem = i915_vma_pin_iomap(vma); > - i915_vma_unpin(vma); > - if (IS_ERR(iomem)) { > - vma = ERR_CAST(iomem); > - goto err; > - } > + vma = i915_gem_object_ggtt_pin_ww(dpt->obj, &ww, NULL, 0, 4096, > + HAS_LMEM(i915) ? 0 : PIN_MAPPABLE); > + if (IS_ERR(vma)) { > + err = PTR_ERR(vma); > + continue; > + } > + > + iomem = i915_vma_pin_iomap(vma); > + i915_vma_unpin(vma); > > - dpt->vma = vma; > - dpt->iomem = iomem; > + if (IS_ERR(iomem)) { > + err = PTR_ERR(iomem); > + continue; > + } > > - i915_vma_get(vma); > + dpt->vma = vma; > + dpt->iomem = iomem; > + > + i915_vma_get(vma); > + } > > -err: > atomic_dec(&i915->gpu_error.pending_fb_pin); > intel_runtime_pm_put(&i915->runtime_pm, wakeref); > > - return vma; > + return err ? ERR_PTR(err) : vma; > } > > void intel_dpt_unpin(struct i915_address_space *vm)
Op 29-09-2021 om 16:50 schreef Thomas Hellström (Intel): > > On 9/29/21 10:59, Maarten Lankhorst wrote: >> Ensure i915_vma_pin_iomap and vma_unpin are done with dpt->obj lock held. >> >> I don't think there's much of a point in merging intel_dpt_pin() with >> intel_pin_fb_obj_dpt(), they touch different objects. >> >> Changes since v1: >> - Fix using the wrong pointer to retrieve error code (Julia) >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> Reported-by: kernel test robot <lkp@intel.com> >> Reported-by: Julia Lawall <julia.lawall@lip6.fr> > > LGTM. > > Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > >> --- >> drivers/gpu/drm/i915/display/intel_dpt.c | 40 +++++++++++++++--------- >> 1 file changed, 25 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c >> index de62bd77b15e..8f7b1f7534a4 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dpt.c >> +++ b/drivers/gpu/drm/i915/display/intel_dpt.c >> @@ -121,32 +121,42 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm) >> intel_wakeref_t wakeref; >> struct i915_vma *vma; >> void __iomem *iomem; >> + struct i915_gem_ww_ctx ww; >> + int err; >> wakeref = intel_runtime_pm_get(&i915->runtime_pm); >> atomic_inc(&i915->gpu_error.pending_fb_pin); >> - vma = i915_gem_object_ggtt_pin(dpt->obj, NULL, 0, 4096, >> - HAS_LMEM(i915) ? 0 : PIN_MAPPABLE); >> - if (IS_ERR(vma)) >> - goto err; >> + for_i915_gem_ww(&ww, err, true) { >> + err = i915_gem_object_lock(dpt->obj, &ww); >> + if (err) >> + continue; >> - iomem = i915_vma_pin_iomap(vma); >> - i915_vma_unpin(vma); >> - if (IS_ERR(iomem)) { >> - vma = ERR_CAST(iomem); >> - goto err; >> - } >> + vma = i915_gem_object_ggtt_pin_ww(dpt->obj, &ww, NULL, 0, 4096, >> + HAS_LMEM(i915) ? 0 : PIN_MAPPABLE); >> + if (IS_ERR(vma)) { >> + err = PTR_ERR(vma); >> + continue; >> + } >> + >> + iomem = i915_vma_pin_iomap(vma); >> + i915_vma_unpin(vma); >> - dpt->vma = vma; >> - dpt->iomem = iomem; >> + if (IS_ERR(iomem)) { >> + err = PTR_ERR(iomem); >> + continue; >> + } >> - i915_vma_get(vma); >> + dpt->vma = vma; >> + dpt->iomem = iomem; >> + >> + i915_vma_get(vma); >> + } >> -err: >> atomic_dec(&i915->gpu_error.pending_fb_pin); >> intel_runtime_pm_put(&i915->runtime_pm, wakeref); >> - return vma; >> + return err ? ERR_PTR(err) : vma; >> } >> void intel_dpt_unpin(struct i915_address_space *vm) Thanks, pushed
diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c index de62bd77b15e..8f7b1f7534a4 100644 --- a/drivers/gpu/drm/i915/display/intel_dpt.c +++ b/drivers/gpu/drm/i915/display/intel_dpt.c @@ -121,32 +121,42 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space *vm) intel_wakeref_t wakeref; struct i915_vma *vma; void __iomem *iomem; + struct i915_gem_ww_ctx ww; + int err; wakeref = intel_runtime_pm_get(&i915->runtime_pm); atomic_inc(&i915->gpu_error.pending_fb_pin); - vma = i915_gem_object_ggtt_pin(dpt->obj, NULL, 0, 4096, - HAS_LMEM(i915) ? 0 : PIN_MAPPABLE); - if (IS_ERR(vma)) - goto err; + for_i915_gem_ww(&ww, err, true) { + err = i915_gem_object_lock(dpt->obj, &ww); + if (err) + continue; - iomem = i915_vma_pin_iomap(vma); - i915_vma_unpin(vma); - if (IS_ERR(iomem)) { - vma = ERR_CAST(iomem); - goto err; - } + vma = i915_gem_object_ggtt_pin_ww(dpt->obj, &ww, NULL, 0, 4096, + HAS_LMEM(i915) ? 0 : PIN_MAPPABLE); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + continue; + } + + iomem = i915_vma_pin_iomap(vma); + i915_vma_unpin(vma); - dpt->vma = vma; - dpt->iomem = iomem; + if (IS_ERR(iomem)) { + err = PTR_ERR(iomem); + continue; + } - i915_vma_get(vma); + dpt->vma = vma; + dpt->iomem = iomem; + + i915_vma_get(vma); + } -err: atomic_dec(&i915->gpu_error.pending_fb_pin); intel_runtime_pm_put(&i915->runtime_pm, wakeref); - return vma; + return err ? ERR_PTR(err) : vma; } void intel_dpt_unpin(struct i915_address_space *vm)
Ensure i915_vma_pin_iomap and vma_unpin are done with dpt->obj lock held. I don't think there's much of a point in merging intel_dpt_pin() with intel_pin_fb_obj_dpt(), they touch different objects. Changes since v1: - Fix using the wrong pointer to retrieve error code (Julia) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reported-by: kernel test robot <lkp@intel.com> Reported-by: Julia Lawall <julia.lawall@lip6.fr> --- drivers/gpu/drm/i915/display/intel_dpt.c | 40 +++++++++++++++--------- 1 file changed, 25 insertions(+), 15 deletions(-)