Re: [Intel-gfx] [PATCH 30/31] drm/i915: Finalize contexts in GEM_CONTEXT_CREATE on version 13+

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jun 08, 2021 at 11:36:12PM -0500, Jason Ekstrand wrote:
> All the proto-context stuff for context creation exists to allow older
> userspace drivers to set VMs and engine sets via SET_CONTEXT_PARAM.
> Drivers need to update to use CONTEXT_CREATE_EXT_* for this going
> forward.  Force the issue by blocking the old mechanism on any future
> hardware generations.
> 
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>

With that static added here (same ofc holds for the other one 0day
spotted):

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

But also I think an ack from Jon Bloomfield here would be needed, plus Cc
Carl Zhang and Micheal Mrozek to get their acks too pls.

Also I'm assuming you've tested this with your igt changes (change the
condition to GFX_VER > 11 in a trybot run) and it all works?
-Daniel


> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 33 +++++++++++++++------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index d3c9c42dcae4d..5312142daa0c0 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -1994,9 +1994,22 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  			goto err_pc;
>  	}
>  
> -	ret = proto_context_register(ext_data.fpriv, ext_data.pc, &id);
> -	if (ret < 0)
> -		goto err_pc;
> +	if (GRAPHICS_VER(i915) > 12) {
> +		struct i915_gem_context *ctx;
> +
> +		ctx = i915_gem_create_context(i915, ext_data.pc);
> +		if (IS_ERR(ctx)) {
> +			ret = PTR_ERR(ctx);
> +			goto err_pc;
> +		}
> +
> +		proto_context_close(ext_data.pc);
> +		gem_context_register(ctx, ext_data.fpriv, id);
> +	} else {
> +		ret = proto_context_register(ext_data.fpriv, ext_data.pc, &id);
> +		if (ret < 0)
> +			goto err_pc;
> +	}
>  
>  	args->ctx_id = id;
>  	drm_dbg(&i915->drm, "HW context %d created\n", args->ctx_id);
> @@ -2179,15 +2192,17 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>  	mutex_lock(&file_priv->proto_context_lock);
>  	ctx = __context_lookup(file_priv, args->ctx_id);
>  	if (!ctx) {
> -		/* FIXME: We should consider disallowing SET_CONTEXT_PARAM
> -		 * for most things on future platforms.  Clients should be
> -		 * using CONTEXT_CREATE_EXT_PARAM instead.
> -		 */
>  		pc = xa_load(&file_priv->proto_context_xa, args->ctx_id);
> -		if (pc)
> +		if (pc) {
> +			/* Contexts should be finalized inside
> +			 * GEM_CONTEXT_CREATE starting with graphics
> +			 * version 13.
> +			 */
> +			WARN_ON(GRAPHICS_VER(file_priv->dev_priv) > 12);
>  			ret = set_proto_ctx_param(file_priv, pc, args);
> -		else
> +		} else {
>  			ret = -ENOENT;
> +		}
>  	}
>  	mutex_unlock(&file_priv->proto_context_lock);
>  
> -- 
> 2.31.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch