All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix VCS ring selection after uapi decoupling
@ 2016-01-27 12:38 Tvrtko Ursulin
  2016-01-27 13:00 ` Chris Wilson
  2016-01-28  8:58 ` ✓ Fi.CI.BAT: success for drm/i915: Fix VCS ring selection after uapi decoupling (rev2) Patchwork
  0 siblings, 2 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-01-27 12:38 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Daniel Vetter

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This got broken in:

   commit de1add360522c876c25ef2bbbbab1c94bdb509ab
   Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
   Date:   Fri Jan 15 15:12:50 2016 +0000

       drm/i915: Decouple execbuf uAPI from internal implementation

BSD ring flags need to be shifted before they can be considered
indices into the ring array.

Reported by Zhipeng Gong.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Zhipeng Gong <zhipeng.gong@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +++++++----
 include/uapi/drm/i915_drm.h                | 10 ++++++----
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 2dc08ce1079a..6fc620af14fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1395,12 +1395,15 @@ eb_select_ring(struct drm_i915_private *dev_priv,
 	}
 
 	if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) {
-		unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
+		unsigned int bsd_idx = (args->flags & I915_EXEC_BSD_MASK) >>
+					I915_EXEC_BSD_SHIFT;
 
-		if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
+		if (bsd_idx == (I915_EXEC_BSD_DEFAULT >> I915_EXEC_BSD_SHIFT)) {
 			bsd_idx = gen8_dispatch_bsd_ring(dev_priv, file);
-		} else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
-			   bsd_idx <= I915_EXEC_BSD_RING2) {
+		} else if (bsd_idx >=
+			  (I915_EXEC_BSD_RING1 >> I915_EXEC_BSD_SHIFT) &&
+			  bsd_idx <=
+			  (I915_EXEC_BSD_RING2 >> I915_EXEC_BSD_SHIFT)) {
 			bsd_idx--;
 		} else {
 			DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6a19371391fa..a5524cc95ff8 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -772,10 +772,12 @@ struct drm_i915_gem_execbuffer2 {
 #define I915_EXEC_HANDLE_LUT		(1<<12)
 
 /** Used for switching BSD rings on the platforms with two BSD rings */
-#define I915_EXEC_BSD_MASK		(3<<13)
-#define I915_EXEC_BSD_DEFAULT		(0<<13) /* default ping-pong mode */
-#define I915_EXEC_BSD_RING1		(1<<13)
-#define I915_EXEC_BSD_RING2		(2<<13)
+#define I915_EXEC_BSD_SHIFT	 (13)
+#define I915_EXEC_BSD_MASK	 (3 << I915_EXEC_BSD_SHIFT)
+/* default ping-pong mode */
+#define I915_EXEC_BSD_DEFAULT	 (0 << I915_EXEC_BSD_SHIFT)
+#define I915_EXEC_BSD_RING1	 (1 << I915_EXEC_BSD_SHIFT)
+#define I915_EXEC_BSD_RING2	 (2 << I915_EXEC_BSD_SHIFT)
 
 /** Tell the kernel that the batchbuffer is processed by
  *  the resource streamer.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Fix VCS ring selection after uapi decoupling
  2016-01-27 12:38 [PATCH] drm/i915: Fix VCS ring selection after uapi decoupling Tvrtko Ursulin
@ 2016-01-27 13:00 ` Chris Wilson
  2016-01-27 13:41   ` [PATCH v2] " Tvrtko Ursulin
  2016-01-27 13:42   ` [PATCH] " Tvrtko Ursulin
  2016-01-28  8:58 ` ✓ Fi.CI.BAT: success for drm/i915: Fix VCS ring selection after uapi decoupling (rev2) Patchwork
  1 sibling, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2016-01-27 13:00 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx

On Wed, Jan 27, 2016 at 12:38:43PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This got broken in:
> 
>    commit de1add360522c876c25ef2bbbbab1c94bdb509ab
>    Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>    Date:   Fri Jan 15 15:12:50 2016 +0000
> 
>        drm/i915: Decouple execbuf uAPI from internal implementation
> 
> BSD ring flags need to be shifted before they can be considered
> indices into the ring array.
> 
> Reported by Zhipeng Gong.
> 

This should be hit by gem_busy on appropriate hardware. I need to add
the extra ring to gem_exec_nop and make those all basic test.

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Zhipeng Gong <zhipeng.gong@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +++++++----
>  include/uapi/drm/i915_drm.h                | 10 ++++++----
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 2dc08ce1079a..6fc620af14fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1395,12 +1395,15 @@ eb_select_ring(struct drm_i915_private *dev_priv,
>  	}
>  
>  	if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) {
> -		unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
> +		unsigned int bsd_idx = (args->flags & I915_EXEC_BSD_MASK) >>
> +					I915_EXEC_BSD_SHIFT;
>  
> -		if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
> +		if (bsd_idx == (I915_EXEC_BSD_DEFAULT >> I915_EXEC_BSD_SHIFT)) {
>  			bsd_idx = gen8_dispatch_bsd_ring(dev_priv, file);
> -		} else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
> -			   bsd_idx <= I915_EXEC_BSD_RING2) {
> +		} else if (bsd_idx >=
> +			  (I915_EXEC_BSD_RING1 >> I915_EXEC_BSD_SHIFT) &&
> +			  bsd_idx <=
> +			  (I915_EXEC_BSD_RING2 >> I915_EXEC_BSD_SHIFT)) {

Wouldn't inserting bsd_idx >>= I915_EXEC_BSD_SHIFT; here be simpler ?
>  			bsd_idx--;

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] drm/i915: Fix VCS ring selection after uapi decoupling
  2016-01-27 13:00 ` Chris Wilson
@ 2016-01-27 13:41   ` Tvrtko Ursulin
  2016-01-27 14:58     ` Chris Wilson
  2016-01-27 13:42   ` [PATCH] " Tvrtko Ursulin
  1 sibling, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-01-27 13:41 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Daniel Vetter

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This got broken in:

   commit de1add360522c876c25ef2bbbbab1c94bdb509ab
   Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
   Date:   Fri Jan 15 15:12:50 2016 +0000

       drm/i915: Decouple execbuf uAPI from internal implementation

BSD ring flags need to be shifted before they can be considered
indices into the ring array.

Reported by Zhipeng Gong.

v2: Simplify the code. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Zhipeng Gong <zhipeng.gong@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  1 +
 include/uapi/drm/i915_drm.h                | 10 ++++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 2dc08ce1079a..5cb57f642ac1 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1401,6 +1401,7 @@ eb_select_ring(struct drm_i915_private *dev_priv,
 			bsd_idx = gen8_dispatch_bsd_ring(dev_priv, file);
 		} else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
 			   bsd_idx <= I915_EXEC_BSD_RING2) {
+			bsd_idx >>= I915_EXEC_BSD_SHIFT;
 			bsd_idx--;
 		} else {
 			DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6a19371391fa..a5524cc95ff8 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -772,10 +772,12 @@ struct drm_i915_gem_execbuffer2 {
 #define I915_EXEC_HANDLE_LUT		(1<<12)
 
 /** Used for switching BSD rings on the platforms with two BSD rings */
-#define I915_EXEC_BSD_MASK		(3<<13)
-#define I915_EXEC_BSD_DEFAULT		(0<<13) /* default ping-pong mode */
-#define I915_EXEC_BSD_RING1		(1<<13)
-#define I915_EXEC_BSD_RING2		(2<<13)
+#define I915_EXEC_BSD_SHIFT	 (13)
+#define I915_EXEC_BSD_MASK	 (3 << I915_EXEC_BSD_SHIFT)
+/* default ping-pong mode */
+#define I915_EXEC_BSD_DEFAULT	 (0 << I915_EXEC_BSD_SHIFT)
+#define I915_EXEC_BSD_RING1	 (1 << I915_EXEC_BSD_SHIFT)
+#define I915_EXEC_BSD_RING2	 (2 << I915_EXEC_BSD_SHIFT)
 
 /** Tell the kernel that the batchbuffer is processed by
  *  the resource streamer.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Fix VCS ring selection after uapi decoupling
  2016-01-27 13:00 ` Chris Wilson
  2016-01-27 13:41   ` [PATCH v2] " Tvrtko Ursulin
@ 2016-01-27 13:42   ` Tvrtko Ursulin
  1 sibling, 0 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-01-27 13:42 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx, Tvrtko Ursulin, Daniel Vetter, Zhipeng Gong


On 27/01/16 13:00, Chris Wilson wrote:
> On Wed, Jan 27, 2016 at 12:38:43PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> This got broken in:
>>
>>     commit de1add360522c876c25ef2bbbbab1c94bdb509ab
>>     Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>     Date:   Fri Jan 15 15:12:50 2016 +0000
>>
>>         drm/i915: Decouple execbuf uAPI from internal implementation
>>
>> BSD ring flags need to be shifted before they can be considered
>> indices into the ring array.
>>
>> Reported by Zhipeng Gong.
>>
>
> This should be hit by gem_busy on appropriate hardware. I need to add
> the extra ring to gem_exec_nop and make those all basic test.

Right hardware is the key.

>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Zhipeng Gong <zhipeng.gong@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +++++++----
>>   include/uapi/drm/i915_drm.h                | 10 ++++++----
>>   2 files changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> index 2dc08ce1079a..6fc620af14fa 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> @@ -1395,12 +1395,15 @@ eb_select_ring(struct drm_i915_private *dev_priv,
>>   	}
>>
>>   	if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) {
>> -		unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
>> +		unsigned int bsd_idx = (args->flags & I915_EXEC_BSD_MASK) >>
>> +					I915_EXEC_BSD_SHIFT;
>>
>> -		if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
>> +		if (bsd_idx == (I915_EXEC_BSD_DEFAULT >> I915_EXEC_BSD_SHIFT)) {
>>   			bsd_idx = gen8_dispatch_bsd_ring(dev_priv, file);
>> -		} else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
>> -			   bsd_idx <= I915_EXEC_BSD_RING2) {
>> +		} else if (bsd_idx >=
>> +			  (I915_EXEC_BSD_RING1 >> I915_EXEC_BSD_SHIFT) &&
>> +			  bsd_idx <=
>> +			  (I915_EXEC_BSD_RING2 >> I915_EXEC_BSD_SHIFT)) {
>
> Wouldn't inserting bsd_idx >>= I915_EXEC_BSD_SHIFT; here be simpler ?

Indeed it would, v2 is on the way.

Regards,

Tvrtko


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/i915: Fix VCS ring selection after uapi decoupling
  2016-01-27 13:41   ` [PATCH v2] " Tvrtko Ursulin
@ 2016-01-27 14:58     ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2016-01-27 14:58 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx

On Wed, Jan 27, 2016 at 01:41:09PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This got broken in:
> 
>    commit de1add360522c876c25ef2bbbbab1c94bdb509ab
>    Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>    Date:   Fri Jan 15 15:12:50 2016 +0000
> 
>        drm/i915: Decouple execbuf uAPI from internal implementation
> 
> BSD ring flags need to be shifted before they can be considered
> indices into the ring array.
> 
> Reported by Zhipeng Gong.
> 
> v2: Simplify the code. (Chris Wilson)
 
Testcase: igt/gem_exec_basic # bdw-gt3
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Zhipeng Gong <zhipeng.gong@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

There were a few tests that would have hit this, but nothing existed to
only try and touch the ring so I added gem_exec_basic to exercise the
ring selection.
-Chris

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Fix VCS ring selection after uapi decoupling (rev2)
  2016-01-27 12:38 [PATCH] drm/i915: Fix VCS ring selection after uapi decoupling Tvrtko Ursulin
  2016-01-27 13:00 ` Chris Wilson
@ 2016-01-28  8:58 ` Patchwork
  1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-01-28  8:58 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Summary ==

Built on 430706bace599ea1a908b9a7c6b7ea17535fe17f drm-intel-nightly: 2016y-01m-27d-16h-33m-06s UTC integration manifest

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (ilk-hp8440p)

bdw-nuci7        total:141  pass:132  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:144  pass:138  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:144  pass:120  dwarn:0   dfail:0   fail:0   skip:24 
byt-nuc          total:144  pass:129  dwarn:0   dfail:0   fail:0   skip:15 
hsw-brixbox      total:144  pass:137  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:144  pass:140  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:144  pass:105  dwarn:0   dfail:0   fail:1   skip:38 
ivb-t430s        total:144  pass:138  dwarn:0   dfail:0   fail:0   skip:6  
skl-i5k-2        total:144  pass:135  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps      total:144  pass:130  dwarn:0   dfail:0   fail:0   skip:14 

Results at /archive/results/CI_IGT_test/Patchwork_1275/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-01-28  8:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-27 12:38 [PATCH] drm/i915: Fix VCS ring selection after uapi decoupling Tvrtko Ursulin
2016-01-27 13:00 ` Chris Wilson
2016-01-27 13:41   ` [PATCH v2] " Tvrtko Ursulin
2016-01-27 14:58     ` Chris Wilson
2016-01-27 13:42   ` [PATCH] " Tvrtko Ursulin
2016-01-28  8:58 ` ✓ Fi.CI.BAT: success for drm/i915: Fix VCS ring selection after uapi decoupling (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.