diff mbox

[03/10] drm/i915: Fix gen8 semaphores id for legacy mode

Message ID 1461860672-12623-3-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson April 28, 2016, 4:24 p.m. UTC 
With the introduction of a distinct engine->id vs the hardware id, we need
to fix up the value we use for selecting the target engine when signaling
a semaphore. Note that these values can be merged with engine->guc_id.

Fixes: de1add360522c876c25ef2bbbbab1c94bdb509ab
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++-
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Tvrtko Ursulin April 29, 2016, 8:36 a.m. UTC | #1 
On 28/04/16 17:24, Chris Wilson wrote:
> With the introduction of a distinct engine->id vs the hardware id, we need
> to fix up the value we use for selecting the target engine when signaling
> a semaphore. Note that these values can be merged with engine->guc_id.

So I broke something more with the decoupling, did not realize. I 
suppose it was still worth it. This at least wasn't being used.

Regards,

Tvrtko


> Fixes: de1add360522c876c25ef2bbbbab1c94bdb509ab
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++--
>   drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++-
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 138afed82682..9761443bcfdf 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1308,7 +1308,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
>   		intel_ring_emit(signaller, seqno);
>   		intel_ring_emit(signaller, 0);
>   		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
> -					   MI_SEMAPHORE_TARGET(waiter->id));
> +					   MI_SEMAPHORE_TARGET(waiter->hw_id));
>   		intel_ring_emit(signaller, 0);
>   	}
>
> @@ -1348,7 +1348,7 @@ static int gen8_xcs_signal(struct drm_i915_gem_request *signaller_req,
>   		intel_ring_emit(signaller, upper_32_bits(gtt_offset));
>   		intel_ring_emit(signaller, seqno);
>   		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
> -					   MI_SEMAPHORE_TARGET(waiter->id));
> +					   MI_SEMAPHORE_TARGET(waiter->hw_id));
>   		intel_ring_emit(signaller, 0);
>   	}
>
> @@ -2759,6 +2759,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   	engine->name = "render ring";
>   	engine->id = RCS;
>   	engine->exec_id = I915_EXEC_RENDER;
> +	engine->hw_id = 0;
>   	engine->mmio_base = RENDER_RING_BASE;
>
>   	if (INTEL_INFO(dev)->gen >= 8) {
> @@ -2909,6 +2910,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>   	engine->name = "bsd ring";
>   	engine->id = VCS;
>   	engine->exec_id = I915_EXEC_BSD;
> +	engine->hw_id = 1;
>
>   	engine->write_tail = ring_write_tail;
>   	if (INTEL_INFO(dev)->gen >= 6) {
> @@ -2987,6 +2989,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
>   	engine->name = "bsd2 ring";
>   	engine->id = VCS2;
>   	engine->exec_id = I915_EXEC_BSD;
> +	engine->hw_id = 4;
>
>   	engine->write_tail = ring_write_tail;
>   	engine->mmio_base = GEN8_BSD2_RING_BASE;
> @@ -3019,6 +3022,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>   	engine->name = "blitter ring";
>   	engine->id = BCS;
>   	engine->exec_id = I915_EXEC_BLT;
> +	engine->hw_id = 2;
>
>   	engine->mmio_base = BLT_RING_BASE;
>   	engine->write_tail = ring_write_tail;
> @@ -3078,6 +3082,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
>   	engine->name = "video enhancement ring";
>   	engine->id = VECS;
>   	engine->exec_id = I915_EXEC_VEBOX;
> +	engine->hw_id = 3;
>
>   	engine->mmio_base = VEBOX_RING_BASE;
>   	engine->write_tail = ring_write_tail;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 7023e88531b5..2651fd5263eb 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -153,7 +153,8 @@ struct  intel_engine_cs {
>   #define I915_NUM_ENGINES 5
>   #define _VCS(n) (VCS + (n))
>   	unsigned int exec_id;
> -	unsigned int guc_id;
> +	unsigned int hw_id;
> +	unsigned int guc_id; /* XXX same as hw_id? */
>   	u32		mmio_base;
>   	struct		drm_device *dev;
>   	struct intel_ringbuffer *buffer;
>
Chris Wilson April 29, 2016, 8:49 a.m. UTC | #2 
On Fri, Apr 29, 2016 at 09:36:37AM +0100, Tvrtko Ursulin wrote:
> 
> On 28/04/16 17:24, Chris Wilson wrote:
> >With the introduction of a distinct engine->id vs the hardware id, we need
> >to fix up the value we use for selecting the target engine when signaling
> >a semaphore. Note that these values can be merged with engine->guc_id.
> 
> So I broke something more with the decoupling, did not realize. I
> suppose it was still worth it. This at least wasn't being used.

A consolation prize: wean the guc over to a common hw_id :)
-Chris
Dave Gordon May 4, 2016, 11:35 a.m. UTC | #3 
On 29/04/2016 09:49, Chris Wilson wrote:
> On Fri, Apr 29, 2016 at 09:36:37AM +0100, Tvrtko Ursulin wrote:
>> On 28/04/16 17:24, Chris Wilson wrote:
>>> With the introduction of a distinct engine->id vs the hardware id, we need
>>> to fix up the value we use for selecting the target engine when signaling
>>> a semaphore. Note that these values can be merged with engine->guc_id.
>> So I broke something more with the decoupling, did not realize. I
>> suppose it was still worth it. This at least wasn't being used.
> A consolation prize: wean the guc over to a common hw_id :)
> -Chris
As in, use the GuC's concept of the engine ID for other purposes too?

.Dave.
Chris Wilson May 4, 2016, 11:44 a.m. UTC | #4 
On Wed, May 04, 2016 at 12:35:09PM +0100, Dave Gordon wrote:
>    On 29/04/2016 09:49, Chris Wilson wrote:
> 
>  On Fri, Apr 29, 2016 at 09:36:37AM +0100, Tvrtko Ursulin wrote:
> 
>  On 28/04/16 17:24, Chris Wilson wrote:
> 
>  With the introduction of a distinct engine->id vs the hardware id, we need
>  to fix up the value we use for selecting the target engine when signaling
>  a semaphore. Note that these values can be merged with engine->guc_id.
> 
>  So I broke something more with the decoupling, did not realize. I
>  suppose it was still worth it. This at least wasn't being used.
> 
>  A consolation prize: wean the guc over to a common hw_id :)
>  -Chris
> 
> 
>    As in, use the GuC's concept of the engine ID for other purposes too?

Looks like the HW engine ID predates the GuC and so if they continued
to use the same values for the GuC (which for the engines that exist up
to and including gen9 they have), the GuC can use the older id and we
can have one fewer special GuC value.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 138afed82682..9761443bcfdf 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1308,7 +1308,7 @@  static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
 		intel_ring_emit(signaller, seqno);
 		intel_ring_emit(signaller, 0);
 		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
-					   MI_SEMAPHORE_TARGET(waiter->id));
+					   MI_SEMAPHORE_TARGET(waiter->hw_id));
 		intel_ring_emit(signaller, 0);
 	}
 
@@ -1348,7 +1348,7 @@  static int gen8_xcs_signal(struct drm_i915_gem_request *signaller_req,
 		intel_ring_emit(signaller, upper_32_bits(gtt_offset));
 		intel_ring_emit(signaller, seqno);
 		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
-					   MI_SEMAPHORE_TARGET(waiter->id));
+					   MI_SEMAPHORE_TARGET(waiter->hw_id));
 		intel_ring_emit(signaller, 0);
 	}
 
@@ -2759,6 +2759,7 @@  int intel_init_render_ring_buffer(struct drm_device *dev)
 	engine->name = "render ring";
 	engine->id = RCS;
 	engine->exec_id = I915_EXEC_RENDER;
+	engine->hw_id = 0;
 	engine->mmio_base = RENDER_RING_BASE;
 
 	if (INTEL_INFO(dev)->gen >= 8) {
@@ -2909,6 +2910,7 @@  int intel_init_bsd_ring_buffer(struct drm_device *dev)
 	engine->name = "bsd ring";
 	engine->id = VCS;
 	engine->exec_id = I915_EXEC_BSD;
+	engine->hw_id = 1;
 
 	engine->write_tail = ring_write_tail;
 	if (INTEL_INFO(dev)->gen >= 6) {
@@ -2987,6 +2989,7 @@  int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->name = "bsd2 ring";
 	engine->id = VCS2;
 	engine->exec_id = I915_EXEC_BSD;
+	engine->hw_id = 4;
 
 	engine->write_tail = ring_write_tail;
 	engine->mmio_base = GEN8_BSD2_RING_BASE;
@@ -3019,6 +3022,7 @@  int intel_init_blt_ring_buffer(struct drm_device *dev)
 	engine->name = "blitter ring";
 	engine->id = BCS;
 	engine->exec_id = I915_EXEC_BLT;
+	engine->hw_id = 2;
 
 	engine->mmio_base = BLT_RING_BASE;
 	engine->write_tail = ring_write_tail;
@@ -3078,6 +3082,7 @@  int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	engine->name = "video enhancement ring";
 	engine->id = VECS;
 	engine->exec_id = I915_EXEC_VEBOX;
+	engine->hw_id = 3;
 
 	engine->mmio_base = VEBOX_RING_BASE;
 	engine->write_tail = ring_write_tail;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 7023e88531b5..2651fd5263eb 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -153,7 +153,8 @@  struct  intel_engine_cs {
 #define I915_NUM_ENGINES 5
 #define _VCS(n) (VCS + (n))
 	unsigned int exec_id;
-	unsigned int guc_id;
+	unsigned int hw_id;
+	unsigned int guc_id; /* XXX same as hw_id? */
 	u32		mmio_base;
 	struct		drm_device *dev;
 	struct intel_ringbuffer *buffer;