Re: [PATCH 1/9] drm/i915: Assign every HW context a unique ID

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 19/04/16 10:04, Chris Wilson wrote:
> On Tue, Apr 19, 2016 at 09:57:14AM +0100, Tvrtko Ursulin wrote:
>>
>> On 19/04/16 07:49, Chris Wilson wrote:
>>> The hardware tracks contexts and expects all live contexts (those active
>>> on the hardware) to have a unique identifier. This is used by the
>>> hardware to assign pagefaults and the like to a particular context.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>   drivers/gpu/drm/i915/i915_debugfs.c     |  2 +-
>>>   drivers/gpu/drm/i915/i915_drv.h         |  4 ++++
>>>   drivers/gpu/drm/i915/i915_gem_context.c | 32 ++++++++++++++++++++++++++++++++
>>>   3 files changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index ffc4734cb2dc..cacbe45b7938 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -2014,7 +2014,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
>>>   		    ctx->legacy_hw_ctx.rcs_state == NULL)
>>>   			continue;
>>>
>>> -		seq_puts(m, "HW context ");
>>> +		seq_printf(m, "HW context %d ", ctx->hw_id);
>>
>> Should be %u.
>>
>>>   		describe_ctx(m, ctx);
>>>   		if (ctx == dev_priv->kernel_context)
>>>   			seq_printf(m, "(kernel context) ");
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index dc3bb5c10957..368fd7090189 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -851,6 +851,7 @@ struct intel_context {
>>>   	struct i915_ctx_hang_stats hang_stats;
>>>   	struct i915_hw_ppgtt *ppgtt;
>>>
>>> +	unsigned hw_id;
>>>   	unsigned flags;
>>>   #define CONTEXT_NO_ZEROMAP		(1<<0)
>>>   #define CONTEXT_NO_ERROR_CAPTURE	(1<<1)
>>> @@ -1810,6 +1811,9 @@ struct drm_i915_private {
>>>   	DECLARE_HASHTABLE(mm_structs, 7);
>>>   	struct mutex mm_lock;
>>>
>>> +	struct ida context_hw_ida;
>>> +#define MAX_CONTEXT_HW_ID (1<<20)
>>
>> BUILD_BUG_ON somewhere if someone in the future is tempted to
>> increase this without realizing ids returned by ida_simple_get are
>> limited to 0x80000000 ?
>
> How do you fancy a BUILD_BUG_ON? (Just not seeing what I should assert.)
> The max-value chosen here is lifted from the ctx-descriptor (as you
> know). A comment is definitely in order to explain the limit.

I thought BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX) is obvious, no?

Probably best at the ida_simple_init call site?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx