Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't allow setting I915_CONTEXT_PARAM_VM twice

Daniel Vetter
2021-07-12T14:30:10Z

On Sat, Jul 10, 2021 at 04:24:46PM -0500, Jason Ekstrand wrote:
> Allowing setting it multiple times brings no real utility to the API, no
> userspace relies on it, and it does make i915 a tiny bit more
> complicated. Let's disallow it for now unless someone comes up with a
> compelling reason to support it.

Maybe mention this is for symmetry with other proto ctx set_param
operations, like set_engines.

>
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 7d6f52d8a8012..5853737cc79f3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -319,7 +319,6 @@ static int set_proto_ctx_vm(struct drm_i915_file_private *fpriv,
> const struct drm_i915_gem_context_param *args)
> {
> struct drm_i915_private *i915 = fpriv->dev_priv;
> - struct i915_address_space *vm;
>
> if (args->size)
> return -EINVAL;
> @@ -327,17 +326,16 @@ static int set_proto_ctx_vm(struct drm_i915_file_private *fpriv,
> if (!HAS_FULL_PPGTT(i915))
> return -ENODEV;
>
> + if (pc->vm)
> + return -EINVAL;
> +
> if (upper_32_bits(args->value))
> return -ENOENT;
>
> - vm = i915_gem_vm_lookup(fpriv, args->value);
> - if (!vm)
> + pc->vm = i915_gem_vm_lookup(fpriv, args->value);
> + if (!pc->vm)
> return -ENOENT;
>
> - if (pc->vm)
> - i915_vm_put(pc->vm);
> - pc->vm = vm;
> -
> return 0;
> }
>
> --
> 2.31.1
>

--
Daniel Vetter
Software Engineer, Intel Corporation