Re: [PATCH 1/2] drm/i915: Shadow default engine context image in the context

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Feb 21, 2024 at 02:22:44PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> To enable adding override of the default engine context image let us start
> shadowing the per engine state in the context.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Carlos Santa <carlos.santa@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_context_types.h   | 2 ++
>  drivers/gpu/drm/i915/gt/intel_lrc.c             | 7 ++++---
>  drivers/gpu/drm/i915/gt/intel_ring_submission.c | 7 ++++---
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
> index 7eccbd70d89f..b179178680a5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> @@ -99,6 +99,8 @@ struct intel_context {
>  	struct i915_address_space *vm;
>  	struct i915_gem_context __rcu *gem_context;
>  
> +	struct file *default_state;
> +
>  	/*
>  	 * @signal_lock protects the list of requests that need signaling,
>  	 * @signals. While there are any requests that need signaling,
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 7c367ba8d9dc..d4eb822d20ae 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1060,9 +1060,8 @@ void lrc_init_state(struct intel_context *ce,
>  
>  	set_redzone(state, engine);
>  
> -	if (engine->default_state) {
> -		shmem_read(engine->default_state, 0,
> -			   state, engine->context_size);
> +	if (ce->default_state) {
> +		shmem_read(ce->default_state, 0, state, engine->context_size);
>  		__set_bit(CONTEXT_VALID_BIT, &ce->flags);
>  		inhibit = false;
>  	}
> @@ -1174,6 +1173,8 @@ int lrc_alloc(struct intel_context *ce, struct intel_engine_cs *engine)
>  
>  	GEM_BUG_ON(ce->state);
>  
> +	ce->default_state = engine->default_state;
> +
>  	vma = __lrc_alloc_state(ce, engine);
>  	if (IS_ERR(vma))
>  		return PTR_ERR(vma);
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> index 92085ffd23de..8625e88e785f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> @@ -474,8 +474,7 @@ static int ring_context_init_default_state(struct intel_context *ce,
>  	if (IS_ERR(vaddr))
>  		return PTR_ERR(vaddr);
>  
> -	shmem_read(ce->engine->default_state, 0,
> -		   vaddr, ce->engine->context_size);
> +	shmem_read(ce->default_state, 0, vaddr, ce->engine->context_size);
>  
>  	i915_gem_object_flush_map(obj);
>  	__i915_gem_object_release_map(obj);
> @@ -491,7 +490,7 @@ static int ring_context_pre_pin(struct intel_context *ce,
>  	struct i915_address_space *vm;
>  	int err = 0;
>  
> -	if (ce->engine->default_state &&
> +	if (ce->default_state &&
>  	    !test_bit(CONTEXT_VALID_BIT, &ce->flags)) {
>  		err = ring_context_init_default_state(ce, ww);
>  		if (err)
> @@ -570,6 +569,8 @@ static int ring_context_alloc(struct intel_context *ce)
>  {
>  	struct intel_engine_cs *engine = ce->engine;
>  
> +	ce->default_state = engine->default_state;
> +
>  	/* One ringbuffer to rule them all */
>  	GEM_BUG_ON(!engine->legacy.ring);
>  	ce->ring = engine->legacy.ring;
> -- 
> 2.40.1
>