| Message ID | 1491834873-9345-6-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -Chris
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
On Tue, Apr 11, 2017 at 12:32:14PM +0100, Tvrtko Ursulin wrote: > > 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. > >> > >>--- > >>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. I've pushed the first 4 patches. Patch 5 is good to go once we have clarification why you want to expose the per-class lookup to context_size (or restore the interface back to hiding the class lookup). -Chris
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); +} void intel_lr_context_resume(struct drm_i915_private *dev_priv); uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,