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

2017-04-11 Thread Chris Wilson 
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 
> >>
> >>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

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2017-04-11 Thread Tvrtko Ursulin 


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 

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

v2: Add MISSING_CASE (Tvrtko)

v3: Rebased

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
Reviewed-by: Tvrtko Ursulin 
Signed-off-by: Oscar Mateo 
---
 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

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

2017-04-11 Thread Chris Wilson 
On Mon, Apr 10, 2017 at 07:34:33AM -0700, Oscar Mateo wrote:
> From: Daniele Ceraolo Spurio 
> 
> Technically speaking, the context size is per engine class, not per
> instance.
> 
> v2: Add MISSING_CASE (Tvrtko)
> 
> v3: Rebased
> 
> Cc: Paulo Zanoni 
> Cc: Rodrigo Vivi 
> Cc: Chris Wilson 
> Signed-off-by: Daniele Ceraolo Spurio 
> Reviewed-by: Tvrtko Ursulin 
> Signed-off-by: Oscar Mateo 
> ---
>  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

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2017-04-10 Thread Oscar Mateo 
From: Daniele Ceraolo Spurio 

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

v2: Add MISSING_CASE (Tvrtko)

v3: Rebased

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
Reviewed-by: Tvrtko Ursulin 
Signed-off-by: Oscar Mateo 
---
 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);
+}
 
 void intel_lr_context_resume(struct drm_i915_private *dev_priv);
 uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
-- 
1.9.1

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

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

2017-04-07 Thread Oscar Mateo 
From: Daniele Ceraolo Spurio 

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

v2: Add MISSING_CASE (Tvrtko)

v3: Rebased

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
Reviewed-by: Tvrtko Ursulin 
Signed-off-by: Oscar Mateo 
---
 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);
+}
 
 void intel_lr_context_resume(struct drm_i915_private *dev_priv);
 uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
-- 
1.9.1

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

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

2017-04-06 Thread Oscar Mateo 
From: Daniele Ceraolo Spurio 

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

v2: Add MISSING_CASE (Tvrtko)

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Reviewed-by: Tvrtko Ursulin 
---
 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..c8abf89 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:
+   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..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,
-- 
1.9.1

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

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

2017-04-06 Thread Oscar Mateo 
From: Daniele Ceraolo Spurio 

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

v2: Add MISSING_CASE (Tvrtko)

Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Reviewed-by: Tvrtko Ursulin 
---
 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..c8abf89 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:
+   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..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,
-- 
1.9.1

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

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

2017-04-06 Thread Tvrtko Ursulin 


On 05/04/2017 10:30, Oscar Mateo wrote:

From: Daniele Ceraolo Spurio 

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 
Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
---
 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2017-04-05 Thread Oscar Mateo 
From: Daniele Ceraolo Spurio 

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

Cc: Tvrtko Ursulin 
Cc: Paulo Zanoni 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
---
 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");
+   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,
-- 
1.9.1

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