diff mbox

[3/5] drm/i915: Extract CSB status read

Message ID 1452018609-10142-4-git-send-email-benjamin.widawsky@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Jan. 5, 2016, 6:30 p.m. UTC
This is a useful thing to have around as a function because the mechanism may
change in the future.

There is a net increase in LOC here, and it will continue to be the case on GEN8
and GEN9 - but future GENs may have an alternate mechanism for doing this.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Comments

Michel Thierry Jan. 6, 2016, 3:09 p.m. UTC | #1
On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> This is a useful thing to have around as a function because the mechanism may
> change in the future.
>
> There is a net increase in LOC here, and it will continue to be the case on GEN8
> and GEN9 - but future GENs may have an alternate mechanism for doing this.
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
>   1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 14affaa..23839ff 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
>          return false;
>   }
>
> +static void get_context_status(struct intel_engine_cs *ring,
> +                              u8 read_pointer,
> +                              u32 *status, u32 *context_id)
> +{
> +       struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +
> +       if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
> +               return;
> +
> +       *status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
> +       *context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
> +}
> +
>   /**
>    * intel_lrc_irq_handler() - handle Context Switch interrupts
>    * @ring: Engine Command Streamer to handle.
> @@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>          spin_lock(&ring->execlist_lock);
>
>          while (read_pointer < write_pointer) {
> -               read_pointer++;
> -               status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
> -               status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
> +
> +               get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> +                                  &status, &status_id);
>
>                  if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
>                          continue;
> --
> 2.6.4

Future GENs may need a completely new lrc_irq_handler anyway.

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Chris Wilson Jan. 6, 2016, 3:15 p.m. UTC | #2
On Wed, Jan 06, 2016 at 03:09:41PM +0000, Michel Thierry wrote:
> On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> >This is a useful thing to have around as a function because the mechanism may
> >change in the future.
> >
> >There is a net increase in LOC here, and it will continue to be the case on GEN8
> >and GEN9 - but future GENs may have an alternate mechanism for doing this.
> >
> >Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 14affaa..23839ff 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
> >         return false;
> >  }
> >
> >+static void get_context_status(struct intel_engine_cs *ring,
> >+                              u8 read_pointer,
> >+                              u32 *status, u32 *context_id)
> >+{
> >+       struct drm_i915_private *dev_priv = ring->dev->dev_private;
> >+
> >+       if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
> >+               return;
> >+
> >+       *status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
> >+       *context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
> >+}
> >+
> >  /**
> >   * intel_lrc_irq_handler() - handle Context Switch interrupts
> >   * @ring: Engine Command Streamer to handle.
> >@@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> >         spin_lock(&ring->execlist_lock);
> >
> >         while (read_pointer < write_pointer) {
> >-               read_pointer++;
> >-               status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
> >-               status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
> >+
> >+               get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> >+                                  &status, &status_id);
> >
> >                 if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
> >                         continue;
> >--
> >2.6.4
> 
> Future GENs may need a completely new lrc_irq_handler anyway.
s/Future/Current/
-Chris
Daniel Vetter Jan. 7, 2016, 2:34 p.m. UTC | #3
On Tue, Jan 05, 2016 at 10:30:07AM -0800, Ben Widawsky wrote:
> This is a useful thing to have around as a function because the mechanism may
> change in the future.
> 
> There is a net increase in LOC here, and it will continue to be the case on GEN8
> and GEN9 - but future GENs may have an alternate mechanism for doing this.
> 
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>

Merged up to this one to dinq.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 14affaa..23839ff 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
>  	return false;
>  }
>  
> +static void get_context_status(struct intel_engine_cs *ring,
> +			       u8 read_pointer,
> +			       u32 *status, u32 *context_id)
> +{
> +	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +
> +	if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
> +		return;
> +
> +	*status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
> +	*context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
> +}
> +
>  /**
>   * intel_lrc_irq_handler() - handle Context Switch interrupts
>   * @ring: Engine Command Streamer to handle.
> @@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>  	spin_lock(&ring->execlist_lock);
>  
>  	while (read_pointer < write_pointer) {
> -		read_pointer++;
> -		status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
> -		status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
> +
> +		get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> +				   &status, &status_id);
>  
>  		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
>  			continue;
> -- 
> 2.6.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 14affaa..23839ff 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -496,6 +496,19 @@  static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 	return false;
 }
 
+static void get_context_status(struct intel_engine_cs *ring,
+			       u8 read_pointer,
+			       u32 *status, u32 *context_id)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+	if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
+		return;
+
+	*status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
+	*context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
+}
+
 /**
  * intel_lrc_irq_handler() - handle Context Switch interrupts
  * @ring: Engine Command Streamer to handle.
@@ -523,9 +536,9 @@  void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 	spin_lock(&ring->execlist_lock);
 
 	while (read_pointer < write_pointer) {
-		read_pointer++;
-		status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
-		status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
+
+		get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
+				   &status, &status_id);
 
 		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
 			continue;