diff mbox

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

Message ID 1491384637-971-6-git-send-email-oscar.mateo@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

oscar.mateo@intel.com April 5, 2017, 9:30 a.m. UTC 
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Technically speaking, the context size is per engine class, not per
instance.

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
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>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 34 ++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_lrc.h |  7 ++++++-
 2 files changed, 28 insertions(+), 13 deletions(-)

Comments

Tvrtko Ursulin April 6, 2017, 6:19 p.m. UTC | #1 
On 05/04/2017 10:30, Oscar Mateo wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> 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@intel.com>
> 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>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  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
diff mbox

Patch

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");
+		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,