Re: [PATCH 5/5] drm/i915: Use the engine class to get the context size

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

On 11/04/2017 11:25, Chris Wilson wrote:
> On Mon, Apr 10, 2017 at 07:34:33AM -0700, Oscar Mateo wrote:
>> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>
>> Technically speaking, the context size is per engine class, not per
>> instance.
>>
>> v2: Add MISSING_CASE (Tvrtko)
>>
>> v3: Rebased
>>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_lrc.c | 33 +++++++++++++++++++++------------
>>  drivers/gpu/drm/i915/intel_lrc.h |  6 +++++-
>>  2 files changed, 26 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 0dc1cc4..1c6672c 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1908,8 +1908,10 @@ static void execlists_init_reg_state(u32 *regs,
>>  }
>>
>>  /**
>> - * intel_lr_context_size() - return the size of the context for an engine
>> - * @engine: which engine to find the context size for
>> + * intel_lr_class_context_size() - return the size of the context for a given
>> + * engine class
>> + * @dev_priv: i915 device private
>> + * @class: which engine class to find the context size for
>>   *
>>   * Each engine may require a different amount of space for a context image,
>>   * so when allocating (or copying) an image, this function can be used to
>> @@ -1921,25 +1923,32 @@ static void execlists_init_reg_state(u32 *regs,
>>   * in LRC mode, but does not include the "shared data page" used with
>>   * GuC submission. The caller should account for this if using the GuC.
>>   */
>> -uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
>> +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class)
>>  {
>>  	int ret = 0;
>>
>> -	WARN_ON(INTEL_GEN(engine->i915) < 8);
>> +	WARN_ON(INTEL_GEN(dev_priv) < 8);
>>
>> -	switch (engine->id) {
>> -	case RCS:
>> -		if (INTEL_GEN(engine->i915) >= 9)
>> +	switch (class) {
>> +	case RENDER_CLASS:
>> +		switch (INTEL_GEN(dev_priv)) {
>> +		default:
>> +			MISSING_CASE(INTEL_GEN(dev_priv));
>> +		case 9:
>>  			ret = GEN9_LR_CONTEXT_RENDER_SIZE;
>> -		else
>> +			break;
>> +		case 8:
>>  			ret = GEN8_LR_CONTEXT_RENDER_SIZE;
>> +			break;
>> +		}
>>  		break;
>> -	case VCS:
>> -	case BCS:
>> -	case VECS:
>> -	case VCS2:
>> +	case VIDEO_DECODE_CLASS:
>> +	case VIDEO_ENHANCEMENT_CLASS:
>> +	case COPY_ENGINE_CLASS:
>>  		ret = GEN8_LR_CONTEXT_OTHER_SIZE;
>>  		break;
>> +	default:
>> +		MISSING_CASE(class);
>>  	}
>>
>>  	return ret;
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
>> index e8015e7..bde2b6e 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.h
>> +++ b/drivers/gpu/drm/i915/intel_lrc.h
>> @@ -78,7 +78,11 @@ enum {
>>  struct drm_i915_private;
>>  struct i915_gem_context;
>>
>> -uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
>> +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class);
>> +static inline uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
>> +{
>> +	return intel_lr_class_context_size(engine->i915, engine->class);
>> +}
>
> I'm not understanding why you want to push this to the caller.
>
> Patches 1-4 are r-b me, and I'll apply once we have put out the fire.

Just to say you can keep my r-b if you change this detail. I wondered 
the same myself but didn't consider it too critical.

Regards,

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