Re: [Intel-gfx] [PATCH 4/7] drm/i915: Move the hw semaphore initialisation from GEM to the engine

[Joonas Lahtinen]

On ke, 2016-04-06 at 00:57 +0100, Chris Wilson wrote:
> Since we are setting engine local values that are tied to the hardware,
> move it out of i915_gem_init_seqno() into the intel_ring_init_seqno()
> backend, next to where the other hw semaphore registers are written.
> 
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Joonas Lahtinen 

Comment below, but anyway;

Reviewed-by: Joonas Lahtinen 

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 8 ++--
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3e9e6f9b66f5..65f18f583ae1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2468,7 +2468,7 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_engine_cs *engine;
> - int ret, j;
> + int ret;
>  
>   /* Carefully retire all requests without writing to the rings */
>   for_each_engine(engine, dev_priv) {
> @@ -2479,13 +2479,9 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
>   i915_gem_retire_requests(dev);
>  
>   /* Finally reset hw state */
> - for_each_engine(engine, dev_priv) {
> + for_each_engine(engine, dev_priv)
>   intel_ring_init_seqno(engine, seqno);
>  
> - for (j = 0; j < ARRAY_SIZE(engine->semaphore.sync_seqno); j++)
> - engine->semaphore.sync_seqno[j] = 0;
> - }
> -
>   return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 371f4c1fc33c..fb304df8085d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2552,14 +2552,21 @@ void intel_ring_init_seqno(struct intel_engine_cs 
> *engine, u32 seqno)
>  {
>   struct drm_i915_private *dev_priv = to_i915(engine->dev);
>  
> + /* Semaphores are strictly monotonic, so whenever we reset the seqno,
> +  * so long as we reset the tracking semaphore value to 0, it will
> +  * always be before the next request's seqno.
> +  */

I'd start the comment with; "Our semaphore implementation" to make it
clear it's deliberately engineered to be so, not by hardware.

Regards, Joonas

>   if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
>   I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
>   I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
>   if (HAS_VEBOX(dev_priv))
>   I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
>   }
> + memset(engine->semaphore.sync_seqno, 0,
> +    sizeof(engine->semaphore.sync_seqno));
>  
>   engine->set_seqno(engine, seqno);
> +
>   engine->hangcheck.seqno = seqno;
>  }
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/7] drm/i915: Move the hw semaphore initialisation from GEM to the engine

[Mika Kuoppala]

Chris Wilson  writes:

> [ text/plain ]
> Since we are setting engine local values that are tied to the hardware,
> move it out of i915_gem_init_seqno() into the intel_ring_init_seqno()
> backend, next to where the other hw semaphore registers are written.
>
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Joonas Lahtinen 

Reviewed-by: Mika Kuoppala 

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 8 ++--
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3e9e6f9b66f5..65f18f583ae1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2468,7 +2468,7 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_engine_cs *engine;
> - int ret, j;
> + int ret;
>  
>   /* Carefully retire all requests without writing to the rings */
>   for_each_engine(engine, dev_priv) {
> @@ -2479,13 +2479,9 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
>   i915_gem_retire_requests(dev);
>  
>   /* Finally reset hw state */
> - for_each_engine(engine, dev_priv) {
> + for_each_engine(engine, dev_priv)
>   intel_ring_init_seqno(engine, seqno);
>  
> - for (j = 0; j < ARRAY_SIZE(engine->semaphore.sync_seqno); j++)
> - engine->semaphore.sync_seqno[j] = 0;
> - }
> -
>   return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 371f4c1fc33c..fb304df8085d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2552,14 +2552,21 @@ void intel_ring_init_seqno(struct intel_engine_cs 
> *engine, u32 seqno)
>  {
>   struct drm_i915_private *dev_priv = to_i915(engine->dev);
>  
> + /* Semaphores are strictly monotonic, so whenever we reset the seqno,
> +  * so long as we reset the tracking semaphore value to 0, it will
> +  * always be before the next request's seqno.
> +  */
>   if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
>   I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
>   I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
>   if (HAS_VEBOX(dev_priv))
>   I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
>   }
> + memset(engine->semaphore.sync_seqno, 0,
> +sizeof(engine->semaphore.sync_seqno));
>  
>   engine->set_seqno(engine, seqno);
> +
>   engine->hangcheck.seqno = seqno;
>  }
>  
> -- 
> 2.8.0.rc3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/7] drm/i915: Move the hw semaphore initialisation from GEM to the engine

[Chris Wilson]

Since we are setting engine local values that are tied to the hardware,
move it out of i915_gem_init_seqno() into the intel_ring_init_seqno()
backend, next to where the other hw semaphore registers are written.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_gem.c | 8 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3e9e6f9b66f5..65f18f583ae1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2468,7 +2468,7 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *engine;
-   int ret, j;
+   int ret;
 
/* Carefully retire all requests without writing to the rings */
for_each_engine(engine, dev_priv) {
@@ -2479,13 +2479,9 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
i915_gem_retire_requests(dev);
 
/* Finally reset hw state */
-   for_each_engine(engine, dev_priv) {
+   for_each_engine(engine, dev_priv)
intel_ring_init_seqno(engine, seqno);
 
-   for (j = 0; j < ARRAY_SIZE(engine->semaphore.sync_seqno); j++)
-   engine->semaphore.sync_seqno[j] = 0;
-   }
-
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 371f4c1fc33c..fb304df8085d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2552,14 +2552,21 @@ void intel_ring_init_seqno(struct intel_engine_cs 
*engine, u32 seqno)
 {
struct drm_i915_private *dev_priv = to_i915(engine->dev);
 
+   /* Semaphores are strictly monotonic, so whenever we reset the seqno,
+* so long as we reset the tracking semaphore value to 0, it will
+* always be before the next request's seqno.
+*/
if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
if (HAS_VEBOX(dev_priv))
I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
}
+   memset(engine->semaphore.sync_seqno, 0,
+  sizeof(engine->semaphore.sync_seqno));
 
engine->set_seqno(engine, seqno);
+
engine->hangcheck.seqno = seqno;
 }
 
-- 
2.8.0.rc3

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