[Intel-gfx] [PATCH 1/2] drm/i915: drop one bit on the hw_id when using guc

[Lionel Landwerlin]

On 31/05/18 21:33, Michel Thierry wrote:
> On 5/31/2018 12:56 PM, Lionel Landwerlin wrote:
>> We currently using GuC as a proxy to the hardware. When Guc is used in
>> such mode, it consumes the bit 20 of the hw_id to indicate that the
>> workload was submitted by proxy.
>>
>> So far we probably haven't seen the issue because we need to allocate
>> 1048576+ contexts to hit this issue. Still, we should avoid allocating
>> the hw_id on that bit and restriction to bits [0:19] (i.e 20bits
>> instead of 21).
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> BSpec: 1237
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h         | 18 ++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_gem_context.c |  7 +------
>>   drivers/gpu/drm/i915/intel_lrc.c        |  2 +-
>>   3 files changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 64659d4efeda..58ab9259fb73 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1841,6 +1841,7 @@ struct drm_i915_private {
>>            */
>>           struct ida hw_ida;
>>   #define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
>> +#define MAX_GUC_CONTEXT_HW_ID (1<<20) /* exclusive */
>>   #define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
>>             bool dynamic_sseu;
>> @@ -3262,6 +3263,23 @@ i915_gem_context_lookup(struct 
>> drm_i915_file_private *file_priv, u32 id)
>>       return ctx;
>>   }
>>   +static inline u32
>> +i915_gem_context_max_hw_id(struct drm_i915_private *i915)
>> +{
>> +    /* TODO: Confirm the max number on Gen11 with GuC */
>> +    if (INTEL_GEN(i915) >= 11)
>> +        return GEN11_MAX_CONTEXT_HW_ID;
>> +
>> +    /*
>> +     * When using GuC in proxy submission, GuC consumes the highest 
>> bit in
>> +     * the context id to indicate proxy submission.
>> +     */
>> +    if (USES_GUC_SUBMISSION(i915))
>> +        return MAX_GUC_CONTEXT_HW_ID;
>> +
>> +    return MAX_CONTEXT_HW_ID;
>> +}
>> +
>
> What was the reason of moving this out of i915_gem_context.c? I don't 
> see any other user.

Yeah, I was initially thinking I would reuse that in i915_perf.c, but 
that's not the case.
I can send a v2 to put it back in.

>
> Everything else looks good to me so
>
> Reviewed-by: Michel Thierry <michel.thierry at intel.com>
>
>>   int i915_gem_contexts_set_dynamic_sseu(struct drm_i915_private *i915,
>>                          bool allowed);
>>   bool i915_gem_contexts_get_dynamic_sseu(struct drm_i915_private 
>> *i915);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
>> b/drivers/gpu/drm/i915/i915_gem_context.c
>> index ff08515d0c67..b686c04d98a9 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -205,13 +205,8 @@ static void context_close(struct 
>> i915_gem_context *ctx)
>>     static int assign_hw_id(struct drm_i915_private *dev_priv, 
>> unsigned *out)
>>   {
>> +    unsigned int max = i915_gem_context_max_hw_id(dev_priv);
>>       int ret;
>> -    unsigned int max;
>> -
>> -    if (INTEL_GEN(dev_priv) >= 11)
>> -        max = GEN11_MAX_CONTEXT_HW_ID;
>> -    else
>> -        max = MAX_CONTEXT_HW_ID;
>>         ret = ida_simple_get(&dev_priv->contexts.hw_ida,
>>                    0, max, GFP_KERNEL);
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index 26e43eb0ef31..94f9c4795190 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -200,7 +200,7 @@ static inline bool need_preempt(const struct 
>> intel_engine_cs *engine,
>>    *
>>    *      bits  0-11:    flags, GEN8_CTX_* (cached in 
>> ctx->desc_template)
>>    *      bits 12-31:    LRCA, GTT address of (the HWSP of) this context
>> - *      bits 32-52:    ctx ID, a globally unique tag
>> + *      bits 32-52:    ctx ID, a globally unique tag (highest bit 
>> used by GuC)
>>    *      bits 53-54:    mbz, reserved for use by hardware
>>    *      bits 55-63:    group ID, currently unused and set to 0
>>    *
>>
>