[Intel-gfx] [PATCH] drm/i915: Use GGTT batchbuffer selector

2014-09-10 Thread Chris Wilson 
gen6 and earlier conflate address space selection (ppgtt vs ggtt) with
the security bit (i.e. only privileged batches were allowed to run from
ggtt). From Haswell onwards, you are able to select the security bit
separate from the address space - and we always requested to use ppgtt.
This breaks the golden render state batch execution as that is only
present in the global GTT and so we need to disable the use of the ppgtt
selector bit, or else we hang immediately upon boot and thence after
every GPU reset...

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   | 2 +-
 drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c  | 5 +++--
 drivers/gpu/drm/i915/intel_ringbuffer.h  | 5 +++--
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1a0611bb576b..8ff448dc8be4 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1260,7 +1260,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (!file-is_master || !capable(CAP_SYS_ADMIN))
return -EPERM;
 
-   flags |= I915_DISPATCH_SECURE;
+   flags |= I915_DISPATCH_SECURE | I915_DISPATCH_GGTT;
}
if (args-flags  I915_EXEC_IS_PINNED)
flags |= I915_DISPATCH_PINNED;
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c 
b/drivers/gpu/drm/i915/i915_gem_render_state.c
index a9a62d75aa57..a158d610720b 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -165,7 +165,7 @@ int i915_gem_render_state_init(struct intel_engine_cs *ring)
ret = ring-dispatch_execbuffer(ring,
so.ggtt_offset,
so.rodata-batch_items * 4,
-   I915_DISPATCH_SECURE);
+   I915_DISPATCH_GGTT);
if (ret)
goto out;
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 109de2eeb9a8..d053819407da 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2174,7 +2174,7 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs 
*ring,
  u64 offset, u32 len,
  unsigned flags)
 {
-   bool ppgtt = USES_PPGTT(ring-dev)  !(flags  I915_DISPATCH_SECURE);
+   bool ppgtt = USES_PPGTT(ring-dev)  !(flags  I915_DISPATCH_GGTT);
int ret;
 
ret = intel_ring_begin(ring, 4);
@@ -2203,7 +2203,8 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs *ring,
return ret;
 
intel_ring_emit(ring,
-   MI_BATCH_BUFFER_START | MI_BATCH_PPGTT_HSW |
+   MI_BATCH_BUFFER_START |
+   (flags  I915_DISPATCH_GGTT ? 0 : MI_BATCH_PPGTT_HSW) |
(flags  I915_DISPATCH_SECURE ? 0 : 
MI_BATCH_NON_SECURE_HSW));
/* bit0-7 is the length on GEN6+ */
intel_ring_emit(ring, offset);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 96479c89f4bd..755585a6fcc7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -169,8 +169,9 @@ struct  intel_engine_cs {
int (*dispatch_execbuffer)(struct intel_engine_cs *ring,
   u64 offset, u32 length,
   unsigned flags);
-#define I915_DISPATCH_SECURE 0x1
-#define I915_DISPATCH_PINNED 0x2
+#define I915_DISPATCH_GGTT 0x1
+#define I915_DISPATCH_SECURE   0x2
+#define I915_DISPATCH_PINNED   0x4
void(*cleanup)(struct intel_engine_cs *ring);
 
/* GEN8 signal/wait table - never trust comments!
-- 
2.1.0

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

Re: [Intel-gfx] [PATCH] drm/i915: Use GGTT batchbuffer selector

2014-09-10 Thread Chris Wilson 
On Wed, Sep 10, 2014 at 10:26:54AM +0100, Chris Wilson wrote:
 gen6 and earlier conflate address space selection (ppgtt vs ggtt) with
 the security bit (i.e. only privileged batches were allowed to run from
 ggtt). From Haswell onwards, you are able to select the security bit
 separate from the address space - and we always requested to use ppgtt.
 This breaks the golden render state batch execution as that is only
 present in the global GTT and so we need to disable the use of the ppgtt
 selector bit, or else we hang immediately upon boot and thence after
 every GPU reset...
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c   | 2 +-
  drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +-
  drivers/gpu/drm/i915/intel_ringbuffer.c  | 5 +++--
  drivers/gpu/drm/i915/intel_ringbuffer.h  | 5 +++--
  4 files changed, 8 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
 b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 index 1a0611bb576b..8ff448dc8be4 100644
 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 @@ -1260,7 +1260,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
 *data,
   if (!file-is_master || !capable(CAP_SYS_ADMIN))
   return -EPERM;
  
 - flags |= I915_DISPATCH_SECURE;
 + flags |= I915_DISPATCH_SECURE | I915_DISPATCH_GGTT;
   }
   if (args-flags  I915_EXEC_IS_PINNED)
   flags |= I915_DISPATCH_PINNED;
 diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c 
 b/drivers/gpu/drm/i915/i915_gem_render_state.c
 index a9a62d75aa57..a158d610720b 100644
 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
 +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
 @@ -165,7 +165,7 @@ int i915_gem_render_state_init(struct intel_engine_cs 
 *ring)
   ret = ring-dispatch_execbuffer(ring,
   so.ggtt_offset,
   so.rodata-batch_items * 4,
 - I915_DISPATCH_SECURE);
 + I915_DISPATCH_GGTT);
   if (ret)
   goto out;
  
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index 109de2eeb9a8..d053819407da 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -2174,7 +2174,7 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs 
 *ring,
 u64 offset, u32 len,
 unsigned flags)
  {
 - bool ppgtt = USES_PPGTT(ring-dev)  !(flags  I915_DISPATCH_SECURE);
 + bool ppgtt = USES_PPGTT(ring-dev)  !(flags  I915_DISPATCH_GGTT);
   int ret;
  
   ret = intel_ring_begin(ring, 4);
 @@ -2203,7 +2203,8 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs 
 *ring,
   return ret;
  
   intel_ring_emit(ring,
 - MI_BATCH_BUFFER_START | MI_BATCH_PPGTT_HSW |
 + MI_BATCH_BUFFER_START |
 + (flags  I915_DISPATCH_GGTT ? 0 : MI_BATCH_PPGTT_HSW) |
   (flags  I915_DISPATCH_SECURE ? 0 : 
 MI_BATCH_NON_SECURE_HSW));
   /* bit0-7 is the length on GEN6+ */
   intel_ring_emit(ring, offset);

I thought I had also fixed up gen6 like:

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0231144d5ef1..ce90f43cecd0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1761,7 +1761,7 @@ gen6_emit_batchbuffer(struct i915_gem_request *rq,
 
intel_ring_emit(ring,
MI_BATCH_BUFFER_START |
-   (flags  I915_DISPATCH_SECURE ? 0 : 
MI_BATCH_NON_SECURE_I965));
+   ((flags  I915_DISPATCH_SECURE | I915_DISPATCH_GTT) ? 0 
: MI_BATCH_NON_SECURE_I965));
/* bit0-7 is the length on GEN6+ */
intel_ring_emit(ring, offset);
intel_ring_advance(ring);

-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

Re: [Intel-gfx] [PATCH] drm/i915: Use GGTT batchbuffer selector

2014-09-10 Thread Ville Syrjälä 
On Wed, Sep 10, 2014 at 10:26:54AM +0100, Chris Wilson wrote:
 gen6 and earlier conflate address space selection (ppgtt vs ggtt) with
 the security bit (i.e. only privileged batches were allowed to run from
 ggtt). From Haswell onwards, 

Not onwards unfortunately. BDW went back to the single bit approach.

 you are able to select the security bit
 separate from the address space - and we always requested to use ppgtt.
 This breaks the golden render state batch execution as that is only
 present in the global GTT and so we need to disable the use of the ppgtt
 selector bit, or else we hang immediately upon boot and thence after
 every GPU reset...
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c   | 2 +-
  drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +-
  drivers/gpu/drm/i915/intel_ringbuffer.c  | 5 +++--
  drivers/gpu/drm/i915/intel_ringbuffer.h  | 5 +++--
  4 files changed, 8 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
 b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 index 1a0611bb576b..8ff448dc8be4 100644
 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
 @@ -1260,7 +1260,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
 *data,
   if (!file-is_master || !capable(CAP_SYS_ADMIN))
   return -EPERM;
  
 - flags |= I915_DISPATCH_SECURE;
 + flags |= I915_DISPATCH_SECURE | I915_DISPATCH_GGTT;
   }
   if (args-flags  I915_EXEC_IS_PINNED)
   flags |= I915_DISPATCH_PINNED;
 diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c 
 b/drivers/gpu/drm/i915/i915_gem_render_state.c
 index a9a62d75aa57..a158d610720b 100644
 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
 +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
 @@ -165,7 +165,7 @@ int i915_gem_render_state_init(struct intel_engine_cs 
 *ring)
   ret = ring-dispatch_execbuffer(ring,
   so.ggtt_offset,
   so.rodata-batch_items * 4,
 - I915_DISPATCH_SECURE);
 + I915_DISPATCH_GGTT);
   if (ret)
   goto out;
  
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index 109de2eeb9a8..d053819407da 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -2174,7 +2174,7 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs 
 *ring,
 u64 offset, u32 len,
 unsigned flags)
  {
 - bool ppgtt = USES_PPGTT(ring-dev)  !(flags  I915_DISPATCH_SECURE);
 + bool ppgtt = USES_PPGTT(ring-dev)  !(flags  I915_DISPATCH_GGTT);
   int ret;
  
   ret = intel_ring_begin(ring, 4);
 @@ -2203,7 +2203,8 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs 
 *ring,
   return ret;
  
   intel_ring_emit(ring,
 - MI_BATCH_BUFFER_START | MI_BATCH_PPGTT_HSW |
 + MI_BATCH_BUFFER_START |
 + (flags  I915_DISPATCH_GGTT ? 0 : MI_BATCH_PPGTT_HSW) |
   (flags  I915_DISPATCH_SECURE ? 0 : 
 MI_BATCH_NON_SECURE_HSW));

Do we actually want to make a distinction between GGTT and secure given
that HSW is the only one where it makes any difference? Why not just
set both GGGT and secure bits on HSW when I915_DISPATCH_SECURE is set?

   /* bit0-7 is the length on GEN6+ */
   intel_ring_emit(ring, offset);
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
 b/drivers/gpu/drm/i915/intel_ringbuffer.h
 index 96479c89f4bd..755585a6fcc7 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
 @@ -169,8 +169,9 @@ struct  intel_engine_cs {
   int (*dispatch_execbuffer)(struct intel_engine_cs *ring,
  u64 offset, u32 length,
  unsigned flags);
 -#define I915_DISPATCH_SECURE 0x1
 -#define I915_DISPATCH_PINNED 0x2
 +#define I915_DISPATCH_GGTT   0x1
 +#define I915_DISPATCH_SECURE 0x2
 +#define I915_DISPATCH_PINNED 0x4
   void(*cleanup)(struct intel_engine_cs *ring);
  
   /* GEN8 signal/wait table - never trust comments!
 -- 
 2.1.0
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Use GGTT batchbuffer selector

2014-09-10 Thread Chris Wilson 
On Wed, Sep 10, 2014 at 01:30:45PM +0300, Ville Syrjälä wrote:
 Do we actually want to make a distinction between GGTT and secure given
 that HSW is the only one where it makes any difference? Why not just
 set both GGGT and secure bits on HSW when I915_DISPATCH_SECURE is set?

If it is only HSW, then the distinction is moot indeed.
-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