diff mbox series

[07/21] drm/i915: Drop getparam support for I915_CONTEXT_PARAM_ENGINES

Message ID 20210423223131.879208-8-jason@jlekstrand.net (mailing list archive)
State New, archived
Headers show
Series drm/i915/gem: ioctl clean-ups | expand

Commit Message

Jason Ekstrand April 23, 2021, 10:31 p.m. UTC
This has never been used by any userspace except IGT and provides no
real functionality beyond parroting back parameters userspace passed in
as part of context creation or via setparam.  If the context is in
legacy mode (where you use I915_EXEC_RENDER and friends), it returns
success with zero data so it's not useful for discovering what engines
are in the context.  It's also not a replacement for the recently
removed I915_CONTEXT_CLONE_ENGINES because it doesn't return any of the
balancing or bonding information.

Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 77 +--------------------
 1 file changed, 1 insertion(+), 76 deletions(-)

Comments

Daniel Vetter April 27, 2021, 9:58 a.m. UTC | #1
On Fri, Apr 23, 2021 at 05:31:17PM -0500, Jason Ekstrand wrote:
> This has never been used by any userspace except IGT and provides no
> real functionality beyond parroting back parameters userspace passed in
> as part of context creation or via setparam.  If the context is in
> legacy mode (where you use I915_EXEC_RENDER and friends), it returns
> success with zero data so it's not useful for discovering what engines
> are in the context.  It's also not a replacement for the recently
> removed I915_CONTEXT_CLONE_ENGINES because it doesn't return any of the
> balancing or bonding information.
> 
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 77 +--------------------
>  1 file changed, 1 insertion(+), 76 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index a72c9b256723b..e8179918fa306 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1725,78 +1725,6 @@ set_engines(struct i915_gem_context *ctx,
>  	return 0;
>  }
>  
> -static int
> -get_engines(struct i915_gem_context *ctx,
> -	    struct drm_i915_gem_context_param *args)
> -{
> -	struct i915_context_param_engines __user *user;
> -	struct i915_gem_engines *e;
> -	size_t n, count, size;
> -	bool user_engines;
> -	int err = 0;
> -
> -	e = __context_engines_await(ctx, &user_engines);
> -	if (!e)
> -		return -ENOENT;
> -
> -	if (!user_engines) {
> -		i915_sw_fence_complete(&e->fence);
> -		args->size = 0;
> -		return 0;
> -	}
> -
> -	count = e->num_engines;
> -
> -	/* Be paranoid in case we have an impedance mismatch */
> -	if (!check_struct_size(user, engines, count, &size)) {
> -		err = -EINVAL;
> -		goto err_free;
> -	}
> -	if (overflows_type(size, args->size)) {
> -		err = -EINVAL;
> -		goto err_free;
> -	}
> -
> -	if (!args->size) {
> -		args->size = size;
> -		goto err_free;
> -	}
> -
> -	if (args->size < size) {
> -		err = -EINVAL;
> -		goto err_free;
> -	}
> -
> -	user = u64_to_user_ptr(args->value);
> -	if (put_user(0, &user->extensions)) {
> -		err = -EFAULT;
> -		goto err_free;
> -	}
> -
> -	for (n = 0; n < count; n++) {
> -		struct i915_engine_class_instance ci = {
> -			.engine_class = I915_ENGINE_CLASS_INVALID,
> -			.engine_instance = I915_ENGINE_CLASS_INVALID_NONE,
> -		};
> -
> -		if (e->engines[n]) {
> -			ci.engine_class = e->engines[n]->engine->uabi_class;
> -			ci.engine_instance = e->engines[n]->engine->uabi_instance;
> -		}
> -
> -		if (copy_to_user(&user->engines[n], &ci, sizeof(ci))) {
> -			err = -EFAULT;
> -			goto err_free;
> -		}
> -	}
> -
> -	args->size = size;
> -
> -err_free:
> -	i915_sw_fence_complete(&e->fence);
> -	return err;
> -}
> -
>  static int
>  set_persistence(struct i915_gem_context *ctx,
>  		const struct drm_i915_gem_context_param *args)
> @@ -2127,10 +2055,6 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>  		ret = get_ppgtt(file_priv, ctx, args);
>  		break;
>  
> -	case I915_CONTEXT_PARAM_ENGINES:
> -		ret = get_engines(ctx, args);
> -		break;
> -
>  	case I915_CONTEXT_PARAM_PERSISTENCE:
>  		args->size = 0;
>  		args->value = i915_gem_context_is_persistent(ctx);
> @@ -2138,6 +2062,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>  
>  	case I915_CONTEXT_PARAM_NO_ZEROMAP:
>  	case I915_CONTEXT_PARAM_BAN_PERIOD:
> +	case I915_CONTEXT_PARAM_ENGINES:
>  	case I915_CONTEXT_PARAM_RINGSIZE:

I like how this list keeps growing. Same thing as usual about "pls check
igt coverage".

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

>  	default:
>  		ret = -EINVAL;
> -- 
> 2.31.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index a72c9b256723b..e8179918fa306 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1725,78 +1725,6 @@  set_engines(struct i915_gem_context *ctx,
 	return 0;
 }
 
-static int
-get_engines(struct i915_gem_context *ctx,
-	    struct drm_i915_gem_context_param *args)
-{
-	struct i915_context_param_engines __user *user;
-	struct i915_gem_engines *e;
-	size_t n, count, size;
-	bool user_engines;
-	int err = 0;
-
-	e = __context_engines_await(ctx, &user_engines);
-	if (!e)
-		return -ENOENT;
-
-	if (!user_engines) {
-		i915_sw_fence_complete(&e->fence);
-		args->size = 0;
-		return 0;
-	}
-
-	count = e->num_engines;
-
-	/* Be paranoid in case we have an impedance mismatch */
-	if (!check_struct_size(user, engines, count, &size)) {
-		err = -EINVAL;
-		goto err_free;
-	}
-	if (overflows_type(size, args->size)) {
-		err = -EINVAL;
-		goto err_free;
-	}
-
-	if (!args->size) {
-		args->size = size;
-		goto err_free;
-	}
-
-	if (args->size < size) {
-		err = -EINVAL;
-		goto err_free;
-	}
-
-	user = u64_to_user_ptr(args->value);
-	if (put_user(0, &user->extensions)) {
-		err = -EFAULT;
-		goto err_free;
-	}
-
-	for (n = 0; n < count; n++) {
-		struct i915_engine_class_instance ci = {
-			.engine_class = I915_ENGINE_CLASS_INVALID,
-			.engine_instance = I915_ENGINE_CLASS_INVALID_NONE,
-		};
-
-		if (e->engines[n]) {
-			ci.engine_class = e->engines[n]->engine->uabi_class;
-			ci.engine_instance = e->engines[n]->engine->uabi_instance;
-		}
-
-		if (copy_to_user(&user->engines[n], &ci, sizeof(ci))) {
-			err = -EFAULT;
-			goto err_free;
-		}
-	}
-
-	args->size = size;
-
-err_free:
-	i915_sw_fence_complete(&e->fence);
-	return err;
-}
-
 static int
 set_persistence(struct i915_gem_context *ctx,
 		const struct drm_i915_gem_context_param *args)
@@ -2127,10 +2055,6 @@  int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 		ret = get_ppgtt(file_priv, ctx, args);
 		break;
 
-	case I915_CONTEXT_PARAM_ENGINES:
-		ret = get_engines(ctx, args);
-		break;
-
 	case I915_CONTEXT_PARAM_PERSISTENCE:
 		args->size = 0;
 		args->value = i915_gem_context_is_persistent(ctx);
@@ -2138,6 +2062,7 @@  int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 
 	case I915_CONTEXT_PARAM_NO_ZEROMAP:
 	case I915_CONTEXT_PARAM_BAN_PERIOD:
+	case I915_CONTEXT_PARAM_ENGINES:
 	case I915_CONTEXT_PARAM_RINGSIZE:
 	default:
 		ret = -EINVAL;