diff mbox

[20/27] drm/i915/icl: Make use of the SW counter field in the new context descriptor

Message ID 20180109232835.11478-11-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Jan. 9, 2018, 11:28 p.m. UTC
From: Oscar Mateo <oscar.mateo@intel.com>

The new context descriptor format in Gen11 contains two assignable fields: the
SW Context ID (technically 11 bits, but practically limited to 2032 entries due
to some being reserved for future use by the GuC) and the SW Counter (6 bits).

We don't want to limit ourselves too much in the maximum number of concurrent
contexts we want to allow, so ideally we want to employ every possible bit
available. Unfortunately, a further limitation in the interface with the GuC
means the combination of SW Context ID + SW Counter has to be unique within the
same engine class (as we use the SW Context ID to index in the GuC stage
descriptor pool, and the Engine Class + SW Counter to index in the 2-dimensional
lrc array). This essentially means we need to somehow encode the engine instance.

Since the BSpec allows 6 bits for engine instance, we use the whole SW counter
for this task. If the limitation of 2032 maximum simultaneous contexts is too
restrictive, we can always squeeze things a bit more (3 extras bits for hw_id,
3 bits for instance) and things will still work (Gen11 does not instance more
than 8 engines of any class).

Another alternative would be to generate the hw_id per HW context instead of per
GEM context, but that has other problems (e.g. maximum number of user-created
contexts would be variable, no relationship between a GuC principal descriptor
and the proxy descriptor it uses, etc...).

Bspec: 12254

v2:
  - Squashed with parts of "Interface changes for GuC fw 22.108" (Daniele)
  - Do not apply the 16 reserved entries limitation to the non-GuC path (Joonas)
v3: Rebased by Rodrigo.
v4: Rebased (s/i915_modparams.enable_guc_submission/USES_GUC_SUBMISSION(dev_priv))

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 11 ++++++++---
 drivers/gpu/drm/i915/i915_gem_context.c |  9 ++++++---
 drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
 drivers/gpu/drm/i915/i915_reg.h         |  2 ++
 drivers/gpu/drm/i915/intel_lrc.c        | 15 ++++++++-------
 5 files changed, 26 insertions(+), 13 deletions(-)

Comments

Daniele Ceraolo Spurio Jan. 11, 2018, 9:10 p.m. UTC | #1
This could potentially be squashed with patch 15, as it doesn't make 
much sense to add a TODO there and solve it here. We might also want to 
update the comment above intel_lr_context_descriptor_update to remove 
the implication that SW context ID == ctx->hw_id (which is still 
technically true after this patch but we're preparing for it not to be 
anymore).

Thanks,
Daniele

On 09/01/18 15:28, Paulo Zanoni wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> The new context descriptor format in Gen11 contains two assignable fields: the
> SW Context ID (technically 11 bits, but practically limited to 2032 entries due
> to some being reserved for future use by the GuC) and the SW Counter (6 bits).
> 
> We don't want to limit ourselves too much in the maximum number of concurrent
> contexts we want to allow, so ideally we want to employ every possible bit
> available. Unfortunately, a further limitation in the interface with the GuC
> means the combination of SW Context ID + SW Counter has to be unique within the
> same engine class (as we use the SW Context ID to index in the GuC stage
> descriptor pool, and the Engine Class + SW Counter to index in the 2-dimensional
> lrc array). This essentially means we need to somehow encode the engine instance.
> 
> Since the BSpec allows 6 bits for engine instance, we use the whole SW counter
> for this task. If the limitation of 2032 maximum simultaneous contexts is too
> restrictive, we can always squeeze things a bit more (3 extras bits for hw_id,
> 3 bits for instance) and things will still work (Gen11 does not instance more
> than 8 engines of any class).
> 
> Another alternative would be to generate the hw_id per HW context instead of per
> GEM context, but that has other problems (e.g. maximum number of user-created
> contexts would be variable, no relationship between a GuC principal descriptor
> and the proxy descriptor it uses, etc...).
> 
> Bspec: 12254
> 
> v2:
>    - Squashed with parts of "Interface changes for GuC fw 22.108" (Daniele)
>    - Do not apply the 16 reserved entries limitation to the non-GuC path (Joonas)
> v3: Rebased by Rodrigo.
> v4: Rebased (s/i915_modparams.enable_guc_submission/USES_GUC_SUBMISSION(dev_priv))
> 
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h         | 11 ++++++++---
>   drivers/gpu/drm/i915/i915_gem_context.c |  9 ++++++---
>   drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
>   drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>   drivers/gpu/drm/i915/intel_lrc.c        | 15 ++++++++-------
>   5 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index aa4f2b178d97..3f1d8c0d2b0a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2079,11 +2079,16 @@ struct drm_i915_private {
>   
>   		/* The hw wants to have a stable context identifier for the
>   		 * lifetime of the context (for OA, PASID, faults, etc).
> -		 * This is limited in execlists to 21 bits.
> +		 * This is limited in execlists to 21 bits. In enhanced execlist
> +		 * (GEN11+) this is limited to 11 bits (the SW Context ID field)
> +		 * but GuC limits it a bit further (11 bits - 16) due to some
> +		 * entries being reserved for future use (so the firmware only
> +		 * supports a GuC stage descriptor pool of 2032 entries).
>   		 */
>   		struct ida hw_ida;
> -#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
> -#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
> +#define MAX_CONTEXT_HW_ID			(1<<21) /* exclusive */
> +#define GEN11_MAX_CONTEXT_HW_ID			(1<<11) /* exclusive */
> +#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC	GEN11_MAX_CONTEXT_HW_ID - 16
>   	} contexts;
>   
>   	u32 fdi_rx_config;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index dbc50b9e18c9..bb5d070083f5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -213,9 +213,12 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
>   	int ret;
>   	unsigned int max;
>   
> -	if (INTEL_GEN(dev_priv) >= 11)
> -		max = GEN11_MAX_CONTEXT_HW_ID;
> -	else
> +	if (INTEL_GEN(dev_priv) >= 11) {
> +		if (USES_GUC_SUBMISSION(dev_priv))
> +			max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
> +		else
> +			max = GEN11_MAX_CONTEXT_HW_ID;
> +	} else
>   		max = MAX_CONTEXT_HW_ID;
>   
>   	ret = ida_simple_get(&dev_priv->contexts.hw_ida,
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 4bfb72f8e1cb..7a39d54e9962 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -156,6 +156,8 @@ struct i915_gem_context {
>   		struct intel_ring *ring;
>   		u32 *lrc_reg_state;
>   		u64 lrc_desc;
> +		u32 sw_context_id;
> +		u32 sw_counter;
>   		int pin_count;
>   	} engine[I915_NUM_ENGINES];
>   
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d8b537570b8e..6d5e2c651580 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3860,6 +3860,8 @@ enum {
>   #define GEN8_CTX_ID_WIDTH 21
>   #define GEN11_SW_CTX_ID_SHIFT 37
>   #define GEN11_SW_CTX_ID_WIDTH 11
> +#define GEN11_SW_COUNTER_SHIFT 55
> +#define GEN11_SW_COUNTER_WIDTH 6
>   #define GEN11_ENGINE_CLASS_SHIFT 61
>   #define GEN11_ENGINE_INSTANCE_SHIFT 48
>   
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d527a79c872c..edf050de8ffe 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -267,15 +267,11 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
>   	if (INTEL_GEN(ctx->i915) >= 11) {
>   		desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
>   								/* bits 61-63 */
> -
> -		/*
> -		 * TODO: use SW counter (bits 60-55) to support more CTXs by
> -		 * combining it with the SW context ID field?
> -		 */
> -
> +		desc |= (u64)ce->sw_counter << GEN11_SW_COUNTER_SHIFT;
> +								/* bits 55-60 */
>   		desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
>   								/* bits 53-48 */
> -		desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
> +		desc |= (u64)ce->sw_context_id << GEN11_SW_CTX_ID_SHIFT;
>   								/* bits 37-47 */
>   	} else {
>   		desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;	/* bits 32-52 */
> @@ -2398,6 +2394,11 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   	ce->ring = ring;
>   	ce->state = vma;
>   
> +	if (INTEL_GEN(ctx->i915) >= 11) {
> +		ce->sw_context_id = ctx->hw_id;
> +		ce->sw_counter = engine->instance;
> +	}
> +
>   	return 0;
>   
>   error_ring_free:
>
oscar.mateo@intel.com Jan. 11, 2018, 10:37 p.m. UTC | #2
On 01/11/2018 01:10 PM, Daniele Ceraolo Spurio wrote:
> This could potentially be squashed with patch 15, as it doesn't make 
> much sense to add a TODO there and solve it here. We might also want 
> to update the comment above intel_lr_context_descriptor_update to 
> remove the implication that SW context ID == ctx->hw_id (which is 
> still technically true after this patch but we're preparing for it not 
> to be anymore).
>

I was actually thinking of a different fate for this patch: leave patch 
15 as is (maybe make the TODO more open, like "TODO: decide what to do 
with sw_counter"), slap a "drm/i915/icl/guc" prefix on this one and 
consider it together with the rest of the GuC patches. At least in the 
meantime, while  we decide how to go about sw_counter (CCing Tvrtko). 
What do you think?

> Thanks,
> Daniele
>
> On 09/01/18 15:28, Paulo Zanoni wrote:
>> From: Oscar Mateo <oscar.mateo@intel.com>
>>
>> The new context descriptor format in Gen11 contains two assignable 
>> fields: the
>> SW Context ID (technically 11 bits, but practically limited to 2032 
>> entries due
>> to some being reserved for future use by the GuC) and the SW Counter 
>> (6 bits).
>>
>> We don't want to limit ourselves too much in the maximum number of 
>> concurrent
>> contexts we want to allow, so ideally we want to employ every 
>> possible bit
>> available. Unfortunately, a further limitation in the interface with 
>> the GuC
>> means the combination of SW Context ID + SW Counter has to be unique 
>> within the
>> same engine class (as we use the SW Context ID to index in the GuC stage
>> descriptor pool, and the Engine Class + SW Counter to index in the 
>> 2-dimensional
>> lrc array). This essentially means we need to somehow encode the 
>> engine instance.
>>
>> Since the BSpec allows 6 bits for engine instance, we use the whole 
>> SW counter
>> for this task. If the limitation of 2032 maximum simultaneous 
>> contexts is too
>> restrictive, we can always squeeze things a bit more (3 extras bits 
>> for hw_id,
>> 3 bits for instance) and things will still work (Gen11 does not 
>> instance more
>> than 8 engines of any class).
>>
>> Another alternative would be to generate the hw_id per HW context 
>> instead of per
>> GEM context, but that has other problems (e.g. maximum number of 
>> user-created
>> contexts would be variable, no relationship between a GuC principal 
>> descriptor
>> and the proxy descriptor it uses, etc...).
>>
>> Bspec: 12254
>>
>> v2:
>>    - Squashed with parts of "Interface changes for GuC fw 22.108" 
>> (Daniele)
>>    - Do not apply the 16 reserved entries limitation to the non-GuC 
>> path (Joonas)
>> v3: Rebased by Rodrigo.
>> v4: Rebased 
>> (s/i915_modparams.enable_guc_submission/USES_GUC_SUBMISSION(dev_priv))
>>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h         | 11 ++++++++---
>>   drivers/gpu/drm/i915/i915_gem_context.c |  9 ++++++---
>>   drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
>>   drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>>   drivers/gpu/drm/i915/intel_lrc.c        | 15 ++++++++-------
>>   5 files changed, 26 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index aa4f2b178d97..3f1d8c0d2b0a 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2079,11 +2079,16 @@ struct drm_i915_private {
>>             /* The hw wants to have a stable context identifier for the
>>            * lifetime of the context (for OA, PASID, faults, etc).
>> -         * This is limited in execlists to 21 bits.
>> +         * This is limited in execlists to 21 bits. In enhanced 
>> execlist
>> +         * (GEN11+) this is limited to 11 bits (the SW Context ID 
>> field)
>> +         * but GuC limits it a bit further (11 bits - 16) due to some
>> +         * entries being reserved for future use (so the firmware only
>> +         * supports a GuC stage descriptor pool of 2032 entries).
>>            */
>>           struct ida hw_ida;
>> -#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
>> -#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
>> +#define MAX_CONTEXT_HW_ID            (1<<21) /* exclusive */
>> +#define GEN11_MAX_CONTEXT_HW_ID            (1<<11) /* exclusive */
>> +#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC GEN11_MAX_CONTEXT_HW_ID - 16
>>       } contexts;
>>         u32 fdi_rx_config;
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
>> b/drivers/gpu/drm/i915/i915_gem_context.c
>> index dbc50b9e18c9..bb5d070083f5 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -213,9 +213,12 @@ static int assign_hw_id(struct drm_i915_private 
>> *dev_priv, unsigned *out)
>>       int ret;
>>       unsigned int max;
>>   -    if (INTEL_GEN(dev_priv) >= 11)
>> -        max = GEN11_MAX_CONTEXT_HW_ID;
>> -    else
>> +    if (INTEL_GEN(dev_priv) >= 11) {
>> +        if (USES_GUC_SUBMISSION(dev_priv))
>> +            max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
>> +        else
>> +            max = GEN11_MAX_CONTEXT_HW_ID;
>> +    } else
>>           max = MAX_CONTEXT_HW_ID;
>>         ret = ida_simple_get(&dev_priv->contexts.hw_ida,
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
>> b/drivers/gpu/drm/i915/i915_gem_context.h
>> index 4bfb72f8e1cb..7a39d54e9962 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.h
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
>> @@ -156,6 +156,8 @@ struct i915_gem_context {
>>           struct intel_ring *ring;
>>           u32 *lrc_reg_state;
>>           u64 lrc_desc;
>> +        u32 sw_context_id;
>> +        u32 sw_counter;
>>           int pin_count;
>>       } engine[I915_NUM_ENGINES];
>>   diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index d8b537570b8e..6d5e2c651580 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3860,6 +3860,8 @@ enum {
>>   #define GEN8_CTX_ID_WIDTH 21
>>   #define GEN11_SW_CTX_ID_SHIFT 37
>>   #define GEN11_SW_CTX_ID_WIDTH 11
>> +#define GEN11_SW_COUNTER_SHIFT 55
>> +#define GEN11_SW_COUNTER_WIDTH 6
>>   #define GEN11_ENGINE_CLASS_SHIFT 61
>>   #define GEN11_ENGINE_INSTANCE_SHIFT 48
>>   diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index d527a79c872c..edf050de8ffe 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -267,15 +267,11 @@ intel_lr_context_descriptor_update(struct 
>> i915_gem_context *ctx,
>>       if (INTEL_GEN(ctx->i915) >= 11) {
>>           desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
>>                                   /* bits 61-63 */
>> -
>> -        /*
>> -         * TODO: use SW counter (bits 60-55) to support more CTXs by
>> -         * combining it with the SW context ID field?
>> -         */
>> -
>> +        desc |= (u64)ce->sw_counter << GEN11_SW_COUNTER_SHIFT;
>> +                                /* bits 55-60 */
>>           desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
>>                                   /* bits 53-48 */
>> -        desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
>> +        desc |= (u64)ce->sw_context_id << GEN11_SW_CTX_ID_SHIFT;
>>                                   /* bits 37-47 */
>>       } else {
>>           desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;    /* bits 
>> 32-52 */
>> @@ -2398,6 +2394,11 @@ static int 
>> execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>>       ce->ring = ring;
>>       ce->state = vma;
>>   +    if (INTEL_GEN(ctx->i915) >= 11) {
>> +        ce->sw_context_id = ctx->hw_id;
>> +        ce->sw_counter = engine->instance;
>> +    }
>> +
>>       return 0;
>>     error_ring_free:
>>
Daniele Ceraolo Spurio Jan. 11, 2018, 11:11 p.m. UTC | #3
On 11/01/18 14:37, Oscar Mateo wrote:
> 
> 
> On 01/11/2018 01:10 PM, Daniele Ceraolo Spurio wrote:
>> This could potentially be squashed with patch 15, as it doesn't make 
>> much sense to add a TODO there and solve it here. We might also want 
>> to update the comment above intel_lr_context_descriptor_update to 
>> remove the implication that SW context ID == ctx->hw_id (which is 
>> still technically true after this patch but we're preparing for it not 
>> to be anymore).
>>
> 
> I was actually thinking of a different fate for this patch: leave patch 
> 15 as is (maybe make the TODO more open, like "TODO: decide what to do 
> with sw_counter"), slap a "drm/i915/icl/guc" prefix on this one and 
> consider it together with the rest of the GuC patches. At least in the 
> meantime, while  we decide how to go about sw_counter (CCing Tvrtko). 
> What do you think?
> 

Sounds good to me, this patch doesn't really impact the !GuC path anyway 
since the number of IDs stays the same. I'll wait to see if there is any 
more feedback and if no one complains I'll send a new revision of patch 15.


>> Thanks,
>> Daniele
>>
>> On 09/01/18 15:28, Paulo Zanoni wrote:
>>> From: Oscar Mateo <oscar.mateo@intel.com>
>>>
>>> The new context descriptor format in Gen11 contains two assignable 
>>> fields: the
>>> SW Context ID (technically 11 bits, but practically limited to 2032 
>>> entries due
>>> to some being reserved for future use by the GuC) and the SW Counter 
>>> (6 bits).
>>>
>>> We don't want to limit ourselves too much in the maximum number of 
>>> concurrent
>>> contexts we want to allow, so ideally we want to employ every 
>>> possible bit
>>> available. Unfortunately, a further limitation in the interface with 
>>> the GuC
>>> means the combination of SW Context ID + SW Counter has to be unique 
>>> within the
>>> same engine class (as we use the SW Context ID to index in the GuC stage
>>> descriptor pool, and the Engine Class + SW Counter to index in the 
>>> 2-dimensional
>>> lrc array). This essentially means we need to somehow encode the 
>>> engine instance.
>>>
>>> Since the BSpec allows 6 bits for engine instance, we use the whole 
>>> SW counter
>>> for this task. If the limitation of 2032 maximum simultaneous 
>>> contexts is too
>>> restrictive, we can always squeeze things a bit more (3 extras bits 
>>> for hw_id,
>>> 3 bits for instance) and things will still work (Gen11 does not 
>>> instance more
>>> than 8 engines of any class).
>>>
>>> Another alternative would be to generate the hw_id per HW context 
>>> instead of per
>>> GEM context, but that has other problems (e.g. maximum number of 
>>> user-created
>>> contexts would be variable, no relationship between a GuC principal 
>>> descriptor
>>> and the proxy descriptor it uses, etc...).
>>>
>>> Bspec: 12254
>>>
>>> v2:
>>>    - Squashed with parts of "Interface changes for GuC fw 22.108" 
>>> (Daniele)
>>>    - Do not apply the 16 reserved entries limitation to the non-GuC 
>>> path (Joonas)
>>> v3: Rebased by Rodrigo.
>>> v4: Rebased 
>>> (s/i915_modparams.enable_guc_submission/USES_GUC_SUBMISSION(dev_priv))
>>>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Michel Thierry <michel.thierry@intel.com>
>>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h         | 11 ++++++++---
>>>   drivers/gpu/drm/i915/i915_gem_context.c |  9 ++++++---
>>>   drivers/gpu/drm/i915/i915_gem_context.h |  2 ++
>>>   drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>>>   drivers/gpu/drm/i915/intel_lrc.c        | 15 ++++++++-------
>>>   5 files changed, 26 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index aa4f2b178d97..3f1d8c0d2b0a 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -2079,11 +2079,16 @@ struct drm_i915_private {
>>>             /* The hw wants to have a stable context identifier for the
>>>            * lifetime of the context (for OA, PASID, faults, etc).
>>> -         * This is limited in execlists to 21 bits.
>>> +         * This is limited in execlists to 21 bits. In enhanced 
>>> execlist
>>> +         * (GEN11+) this is limited to 11 bits (the SW Context ID 
>>> field)
>>> +         * but GuC limits it a bit further (11 bits - 16) due to some
>>> +         * entries being reserved for future use (so the firmware only
>>> +         * supports a GuC stage descriptor pool of 2032 entries).
>>>            */
>>>           struct ida hw_ida;
>>> -#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
>>> -#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
>>> +#define MAX_CONTEXT_HW_ID            (1<<21) /* exclusive */
>>> +#define GEN11_MAX_CONTEXT_HW_ID            (1<<11) /* exclusive */
>>> +#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC GEN11_MAX_CONTEXT_HW_ID - 16
>>>       } contexts;
>>>         u32 fdi_rx_config;
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
>>> b/drivers/gpu/drm/i915/i915_gem_context.c
>>> index dbc50b9e18c9..bb5d070083f5 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>>> @@ -213,9 +213,12 @@ static int assign_hw_id(struct drm_i915_private 
>>> *dev_priv, unsigned *out)
>>>       int ret;
>>>       unsigned int max;
>>>   -    if (INTEL_GEN(dev_priv) >= 11)
>>> -        max = GEN11_MAX_CONTEXT_HW_ID;
>>> -    else
>>> +    if (INTEL_GEN(dev_priv) >= 11) {
>>> +        if (USES_GUC_SUBMISSION(dev_priv))
>>> +            max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
>>> +        else
>>> +            max = GEN11_MAX_CONTEXT_HW_ID;
>>> +    } else
>>>           max = MAX_CONTEXT_HW_ID;
>>>         ret = ida_simple_get(&dev_priv->contexts.hw_ida,
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h 
>>> b/drivers/gpu/drm/i915/i915_gem_context.h
>>> index 4bfb72f8e1cb..7a39d54e9962 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_context.h
>>> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
>>> @@ -156,6 +156,8 @@ struct i915_gem_context {
>>>           struct intel_ring *ring;
>>>           u32 *lrc_reg_state;
>>>           u64 lrc_desc;
>>> +        u32 sw_context_id;
>>> +        u32 sw_counter;
>>>           int pin_count;
>>>       } engine[I915_NUM_ENGINES];
>>>   diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index d8b537570b8e..6d5e2c651580 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -3860,6 +3860,8 @@ enum {
>>>   #define GEN8_CTX_ID_WIDTH 21
>>>   #define GEN11_SW_CTX_ID_SHIFT 37
>>>   #define GEN11_SW_CTX_ID_WIDTH 11
>>> +#define GEN11_SW_COUNTER_SHIFT 55
>>> +#define GEN11_SW_COUNTER_WIDTH 6
>>>   #define GEN11_ENGINE_CLASS_SHIFT 61
>>>   #define GEN11_ENGINE_INSTANCE_SHIFT 48
>>>   diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>> index d527a79c872c..edf050de8ffe 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -267,15 +267,11 @@ intel_lr_context_descriptor_update(struct 
>>> i915_gem_context *ctx,
>>>       if (INTEL_GEN(ctx->i915) >= 11) {
>>>           desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
>>>                                   /* bits 61-63 */
>>> -
>>> -        /*
>>> -         * TODO: use SW counter (bits 60-55) to support more CTXs by
>>> -         * combining it with the SW context ID field?
>>> -         */
>>> -
>>> +        desc |= (u64)ce->sw_counter << GEN11_SW_COUNTER_SHIFT;
>>> +                                /* bits 55-60 */
>>>           desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
>>>                                   /* bits 53-48 */
>>> -        desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
>>> +        desc |= (u64)ce->sw_context_id << GEN11_SW_CTX_ID_SHIFT;
>>>                                   /* bits 37-47 */
>>>       } else {
>>>           desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;    /* bits 
>>> 32-52 */
>>> @@ -2398,6 +2394,11 @@ static int 
>>> execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>>>       ce->ring = ring;
>>>       ce->state = vma;
>>>   +    if (INTEL_GEN(ctx->i915) >= 11) {
>>> +        ce->sw_context_id = ctx->hw_id;
>>> +        ce->sw_counter = engine->instance;
>>> +    }
>>> +
>>>       return 0;
>>>     error_ring_free:
>>>
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index aa4f2b178d97..3f1d8c0d2b0a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2079,11 +2079,16 @@  struct drm_i915_private {
 
 		/* The hw wants to have a stable context identifier for the
 		 * lifetime of the context (for OA, PASID, faults, etc).
-		 * This is limited in execlists to 21 bits.
+		 * This is limited in execlists to 21 bits. In enhanced execlist
+		 * (GEN11+) this is limited to 11 bits (the SW Context ID field)
+		 * but GuC limits it a bit further (11 bits - 16) due to some
+		 * entries being reserved for future use (so the firmware only
+		 * supports a GuC stage descriptor pool of 2032 entries).
 		 */
 		struct ida hw_ida;
-#define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
-#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
+#define MAX_CONTEXT_HW_ID			(1<<21) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID			(1<<11) /* exclusive */
+#define GEN11_MAX_CONTEXT_HW_ID_WITH_GUC	GEN11_MAX_CONTEXT_HW_ID - 16
 	} contexts;
 
 	u32 fdi_rx_config;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index dbc50b9e18c9..bb5d070083f5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -213,9 +213,12 @@  static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
 	int ret;
 	unsigned int max;
 
-	if (INTEL_GEN(dev_priv) >= 11)
-		max = GEN11_MAX_CONTEXT_HW_ID;
-	else
+	if (INTEL_GEN(dev_priv) >= 11) {
+		if (USES_GUC_SUBMISSION(dev_priv))
+			max = GEN11_MAX_CONTEXT_HW_ID_WITH_GUC;
+		else
+			max = GEN11_MAX_CONTEXT_HW_ID;
+	} else
 		max = MAX_CONTEXT_HW_ID;
 
 	ret = ida_simple_get(&dev_priv->contexts.hw_ida,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 4bfb72f8e1cb..7a39d54e9962 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -156,6 +156,8 @@  struct i915_gem_context {
 		struct intel_ring *ring;
 		u32 *lrc_reg_state;
 		u64 lrc_desc;
+		u32 sw_context_id;
+		u32 sw_counter;
 		int pin_count;
 	} engine[I915_NUM_ENGINES];
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d8b537570b8e..6d5e2c651580 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3860,6 +3860,8 @@  enum {
 #define GEN8_CTX_ID_WIDTH 21
 #define GEN11_SW_CTX_ID_SHIFT 37
 #define GEN11_SW_CTX_ID_WIDTH 11
+#define GEN11_SW_COUNTER_SHIFT 55
+#define GEN11_SW_COUNTER_WIDTH 6
 #define GEN11_ENGINE_CLASS_SHIFT 61
 #define GEN11_ENGINE_INSTANCE_SHIFT 48
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d527a79c872c..edf050de8ffe 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -267,15 +267,11 @@  intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
 	if (INTEL_GEN(ctx->i915) >= 11) {
 		desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
 								/* bits 61-63 */
-
-		/*
-		 * TODO: use SW counter (bits 60-55) to support more CTXs by
-		 * combining it with the SW context ID field?
-		 */
-
+		desc |= (u64)ce->sw_counter << GEN11_SW_COUNTER_SHIFT;
+								/* bits 55-60 */
 		desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
 								/* bits 53-48 */
-		desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
+		desc |= (u64)ce->sw_context_id << GEN11_SW_CTX_ID_SHIFT;
 								/* bits 37-47 */
 	} else {
 		desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;	/* bits 32-52 */
@@ -2398,6 +2394,11 @@  static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 	ce->ring = ring;
 	ce->state = vma;
 
+	if (INTEL_GEN(ctx->i915) >= 11) {
+		ce->sw_context_id = ctx->hw_id;
+		ce->sw_counter = engine->instance;
+	}
+
 	return 0;
 
 error_ring_free: