On 05/04/2017 10:30, Oscar Mateo wrote:
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> Technically speaking, the context size is per engine class, not per instance.
It is very nice to have the code match the documentation!
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> --- drivers/gpu/drm/i915/intel_lrc.c | 34 ++++++++++++++++++++++------------ drivers/gpu/drm/i915/intel_lrc.h | 7 ++++++- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 0dc1cc4..6b1fc4a 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,33 @@ 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, + enum intel_engine_class 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: + DRM_ERROR("Unknown context size for GEN\n");
MISSING_CASE I think, otherwise it is too easy to miss in the noise. And it is MISSING_CASE for the class below which looks appropriate to me for this layer.
+ 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..b3a4331 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -78,7 +78,12 @@ 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, + enum intel_engine_class class); +static inline uint32_t intel_lr_context_size(struct intel_engine_cs *engine) +{ + return intel_lr_class_context_size(engine->i915, engine->class); +} void intel_lr_context_resume(struct drm_i915_private *dev_priv); uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
Otherwise looks fine to me. I also really hope to be able to use the class/instance cleanup in scope of the better VCS load balancing which is currently being looked at.
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx