On Tue, 30 Nov 2021 at 09:21, Maarten Lankhorst
<maarten.lankhorst@xxxxxxxxxxxxxxx> wrote:
>
> We will need the lock to unbind the vma, and wait for bind to complete.
> Remove the special casing for the !ww path, and force ww locking for all.
>
> Changes since v1:
> - Pass err to for_i915_gem_ww handling for -EDEADLK handling.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 ++-----
> drivers/gpu/drm/i915/i915_gem.c | 30 ++++++++++++++++++++++++++----
> 2 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1bfadd9127fc..8322abe194da 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1842,13 +1842,10 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
> const struct i915_ggtt_view *view,
> u64 size, u64 alignment, u64 flags);
>
> -static inline struct i915_vma * __must_check
> +struct i915_vma * __must_check
> i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
> const struct i915_ggtt_view *view,
> - u64 size, u64 alignment, u64 flags)
> -{
> - return i915_gem_object_ggtt_pin_ww(obj, NULL, view, size, alignment, flags);
> -}
> + u64 size, u64 alignment, u64 flags);
>
> int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
> unsigned long flags);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 527228d4da7e..6002045bd194 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -877,6 +877,8 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
> struct i915_vma *vma;
> int ret;
>
> + GEM_WARN_ON(!ww);
Return something here, or GEM_BUG_ON()? I assume it's going to crash
and burn anyway?
> +
> if (flags & PIN_MAPPABLE &&
> (!view || view->type == I915_GGTT_VIEW_NORMAL)) {
> /*
> @@ -936,10 +938,7 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
> return ERR_PTR(ret);
> }
>
> - if (ww)
> - ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);
> - else
> - ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL);
> + ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);
>
> if (ret)
> return ERR_PTR(ret);
> @@ -959,6 +958,29 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj,
> return vma;
> }
>
> +struct i915_vma * __must_check
> +i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
> + const struct i915_ggtt_view *view,
> + u64 size, u64 alignment, u64 flags)
> +{
> + struct i915_gem_ww_ctx ww;
> + struct i915_vma *ret;
> + int err;
> +
> + for_i915_gem_ww(&ww, err, true) {
> + err = i915_gem_object_lock(obj, &ww);
> + if (err)
> + continue;
> +
> + ret = i915_gem_object_ggtt_pin_ww(obj, &ww, view, size,
> + alignment, flags);
> + if (IS_ERR(ret))
> + err = PTR_ERR(ret);
Maybe s/ret/vma/ ? Seeing ret makes me think it's an integer.
Anyway,
Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> + }
> +
> + return err ? ERR_PTR(err) : ret;
> +}
> +
> int
> i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
> --
> 2.34.1
>
