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

[Lionel Landwerlin]

On 01/06/18 20:24, Michel Thierry wrote:
> On 6/1/2018 8:10 AM, Chris Wilson wrote:
>> Quoting Lionel Landwerlin (2018-06-01 10:52:14)
>>> 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).
>>>
>>> v2: Leave the max hw_id computation in i915_gem_context.c (Michel)
>>>
>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>> BSpec: 1237
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h         |  1 +
>>>   drivers/gpu/drm/i915/i915_gem_context.c | 14 +++++++++++---
>>>   drivers/gpu/drm/i915/intel_lrc.c        |  2 +-
>>>   3 files changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index 38157df6ff5c..7088a1c3b6ad 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 */
>>>          } contexts;
>>>   diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
>>> b/drivers/gpu/drm/i915/i915_gem_context.c
>>> index 81f086397d10..fa732592e221 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>>> @@ -208,10 +208,18 @@ static int assign_hw_id(struct 
>>> drm_i915_private *dev_priv, unsigned *out)
>>>          int ret;
>>>          unsigned int max;
>>>   -       if (INTEL_GEN(dev_priv) >= 11)
>>> +       if (INTEL_GEN(dev_priv) >= 11) {
>>>                  max = GEN11_MAX_CONTEXT_HW_ID;
>>> -       else
>>> -               max = MAX_CONTEXT_HW_ID;
>>> +       } else {
>>> +               /*
>>> +                * When using GuC in proxy submission, GuC consumes the
>>> +                * highest bit in the context id to indicate proxy 
>>> submission.
>>> +                */
>>> +               max = USES_GUC_SUBMISSION(dev_priv) ?
>>> +                       MAX_GUC_CONTEXT_HW_ID :
>>> +                       MAX_CONTEXT_HW_ID;
>>
>> I'm just going to say mixing if() and ternary ?: isn't great style.
>>
>> Acked-by: Chris Wilson <chris at chris-wilson.co.uk>
>
> My r-b from v1 still applies, but since I'm already writing this...
>
> Reviewed-by: Michel Thierry <michel.thierry at intel.com>
>
>
>
Thanks, I should have added it.


-Lionel