Re: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt

2014-08-08 Thread Thierry, Michel

 -Original Message-
 From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
 Of Daniel Vetter
 Sent: Wednesday, August 06, 2014 2:05 PM
 To: Intel Graphics Development
 Cc: Daniel Vetter
 Subject: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing
ppgtt

 A subsequent patch will no longer initialize the aliasing ppgtt if we
 have full ppgtt enabled, since we simply don't need that any more.

 Unfortunately a few places check for the aliasing ppgtt instead of
 checking for ppgtt in general. Fix them up.

 One special case are the gtt offset and size macros, which have some
 code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt
 is _not_ a logical address space, so passing that in as the vm is
 plain and simple a bug. So just WARN about it and carry on - we have a
 gracefully fall-through anyway if we can't find the vma.

 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_cmd_parser.c  | 4 +---
  drivers/gpu/drm/i915/i915_dma.c | 2 +-
  drivers/gpu/drm/i915/i915_gem.c | 8 ++--
  drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
  4 files changed, 5 insertions(+), 13 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
 b/drivers/gpu/drm/i915/i915_cmd_parser.c
 index dea99d92fb4a..c45856bcc8b9 100644
 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
 +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
 @@ -842,8 +842,6 @@ finish:
   */
  bool i915_needs_cmd_parser(struct intel_engine_cs *ring)
  {
 - struct drm_i915_private *dev_priv = ring-dev-dev_private;
 -
   if (!ring-needs_cmd_parser)
   return false;

 @@ -852,7 +850,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs
 *ring)
* disabled. That will cause all of the parser's PPGTT checks to
* fail. For now, disable parsing when PPGTT is off.
*/
 - if (!dev_priv-mm.aliasing_ppgtt)
 + if (USES_PPGTT(ring-dev))
   return false;

   return (i915.enable_cmd_parser == 1);
 diff --git a/drivers/gpu/drm/i915/i915_dma.c
 b/drivers/gpu/drm/i915/i915_dma.c
 index 2e7f03ad5ee2..e5ac1a6e9d26 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -999,7 +999,7 @@ static int i915_getparam(struct drm_device *dev, void
 *data,
   value = HAS_WT(dev);
   break;
   case I915_PARAM_HAS_ALIASING_PPGTT:
 - value = dev_priv-mm.aliasing_ppgtt ||
 USES_FULL_PPGTT(dev);
 + value = USES_PPGTT(dev);
   break;
   case I915_PARAM_HAS_WAIT_TIMEOUT:
   value = 1;
 diff --git a/drivers/gpu/drm/i915/i915_gem.c
 b/drivers/gpu/drm/i915/i915_gem.c
 index d8399ee622b9..a79deb189146 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -5114,9 +5114,7 @@ unsigned long i915_gem_obj_offset(struct
 drm_i915_gem_object *o,
   struct drm_i915_private *dev_priv = o-base.dev-dev_private;
   struct i915_vma *vma;

 - if (!dev_priv-mm.aliasing_ppgtt ||
 - vm == dev_priv-mm.aliasing_ppgtt-base)
 - vm = dev_priv-gtt.base;
 + WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);

   list_for_each_entry(vma, o-vma_list, vma_link) {
   if (vma-vm == vm)
 @@ -5157,9 +5155,7 @@ unsigned long i915_gem_obj_size(struct
 drm_i915_gem_object *o,
   struct drm_i915_private *dev_priv = o-base.dev-dev_private;
   struct i915_vma *vma;

 - if (!dev_priv-mm.aliasing_ppgtt ||
 - vm == dev_priv-mm.aliasing_ppgtt-base)
 - vm = dev_priv-gtt.base;
 + WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);

   BUG_ON(list_empty(o-vma_list));

 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index 05969f03c0c1..390e38325b37 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -1977,9 +1977,7 @@ gen8_ring_dispatch_execbuffer(struct
 intel_engine_cs *ring,
 u64 offset, u32 len,
 unsigned flags)
  {
 - struct drm_i915_private *dev_priv = ring-dev-dev_private;
 - bool ppgtt = dev_priv-mm.aliasing_ppgtt != NULL 
 - !(flags  I915_DISPATCH_SECURE);
 + bool ppgtt = USES_PPGTT(ring-dev)  !(flags 
 I915_DISPATCH_SECURE);
   int ret;

   ret = intel_ring_begin(ring, 4);
 --
 1.9.3

Sorry, I spotted another instance in gen8_ppgtt_info (debugfs). It is more 
gen8_ppgtt_info's fault, so I'm sending a follow up patch for it.
Unless you want to combine them...
Reviewed-by: Michel Thierry michel.thie...@intel.com
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx


smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http

Re: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt

2014-08-08 Thread Daniel Vetter
On Fri, Aug 08, 2014 at 01:03:53PM +, Thierry, Michel wrote:
 
  -Original Message-
  From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
  Of Daniel Vetter
  Sent: Wednesday, August 06, 2014 2:05 PM
  To: Intel Graphics Development
  Cc: Daniel Vetter
  Subject: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing
 ppgtt
 
  A subsequent patch will no longer initialize the aliasing ppgtt if we
  have full ppgtt enabled, since we simply don't need that any more.
 
  Unfortunately a few places check for the aliasing ppgtt instead of
  checking for ppgtt in general. Fix them up.
 
  One special case are the gtt offset and size macros, which have some
  code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt
  is _not_ a logical address space, so passing that in as the vm is
  plain and simple a bug. So just WARN about it and carry on - we have a
  gracefully fall-through anyway if we can't find the vma.
 
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/i915_cmd_parser.c  | 4 +---
   drivers/gpu/drm/i915/i915_dma.c | 2 +-
   drivers/gpu/drm/i915/i915_gem.c | 8 ++--
   drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
   4 files changed, 5 insertions(+), 13 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
  b/drivers/gpu/drm/i915/i915_cmd_parser.c
  index dea99d92fb4a..c45856bcc8b9 100644
  --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
  +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
  @@ -842,8 +842,6 @@ finish:
*/
   bool i915_needs_cmd_parser(struct intel_engine_cs *ring)
   {
  -   struct drm_i915_private *dev_priv = ring-dev-dev_private;
  -
  if (!ring-needs_cmd_parser)
  return false;
 
  @@ -852,7 +850,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs
  *ring)
   * disabled. That will cause all of the parser's PPGTT checks to
   * fail. For now, disable parsing when PPGTT is off.
   */
  -   if (!dev_priv-mm.aliasing_ppgtt)
  +   if (USES_PPGTT(ring-dev))
  return false;
 
  return (i915.enable_cmd_parser == 1);
  diff --git a/drivers/gpu/drm/i915/i915_dma.c
  b/drivers/gpu/drm/i915/i915_dma.c
  index 2e7f03ad5ee2..e5ac1a6e9d26 100644
  --- a/drivers/gpu/drm/i915/i915_dma.c
  +++ b/drivers/gpu/drm/i915/i915_dma.c
  @@ -999,7 +999,7 @@ static int i915_getparam(struct drm_device *dev, void
  *data,
  value = HAS_WT(dev);
  break;
  case I915_PARAM_HAS_ALIASING_PPGTT:
  -   value = dev_priv-mm.aliasing_ppgtt ||
  USES_FULL_PPGTT(dev);
  +   value = USES_PPGTT(dev);
  break;
  case I915_PARAM_HAS_WAIT_TIMEOUT:
  value = 1;
  diff --git a/drivers/gpu/drm/i915/i915_gem.c
  b/drivers/gpu/drm/i915/i915_gem.c
  index d8399ee622b9..a79deb189146 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -5114,9 +5114,7 @@ unsigned long i915_gem_obj_offset(struct
  drm_i915_gem_object *o,
  struct drm_i915_private *dev_priv = o-base.dev-dev_private;
  struct i915_vma *vma;
 
  -   if (!dev_priv-mm.aliasing_ppgtt ||
  -   vm == dev_priv-mm.aliasing_ppgtt-base)
  -   vm = dev_priv-gtt.base;
  +   WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
 
  list_for_each_entry(vma, o-vma_list, vma_link) {
  if (vma-vm == vm)
  @@ -5157,9 +5155,7 @@ unsigned long i915_gem_obj_size(struct
  drm_i915_gem_object *o,
  struct drm_i915_private *dev_priv = o-base.dev-dev_private;
  struct i915_vma *vma;
 
  -   if (!dev_priv-mm.aliasing_ppgtt ||
  -   vm == dev_priv-mm.aliasing_ppgtt-base)
  -   vm = dev_priv-gtt.base;
  +   WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
 
  BUG_ON(list_empty(o-vma_list));
 
  diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
  b/drivers/gpu/drm/i915/intel_ringbuffer.c
  index 05969f03c0c1..390e38325b37 100644
  --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
  +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
  @@ -1977,9 +1977,7 @@ gen8_ring_dispatch_execbuffer(struct
  intel_engine_cs *ring,
u64 offset, u32 len,
unsigned flags)
   {
  -   struct drm_i915_private *dev_priv = ring-dev-dev_private;
  -   bool ppgtt = dev_priv-mm.aliasing_ppgtt != NULL 
  -   !(flags  I915_DISPATCH_SECURE);
  +   bool ppgtt = USES_PPGTT(ring-dev)  !(flags 
  I915_DISPATCH_SECURE);
  int ret;
 
  ret = intel_ring_begin(ring, 4);
  --
  1.9.3
 
 Sorry, I spotted another instance in gen8_ppgtt_info (debugfs). It is more 
 gen8_ppgtt_info's fault, so I'm sending a follow up patch for it.
 Unless you want to combine them...
 Reviewed-by: Michel Thierry michel.thie...@intel.com

Actually I spotted that one and decided that I can't break things worse
than it is - the aliasing ppgtt for full ppgtt is completely unused
(except for these checks), so dumping it doesn't add value.

To check: Does your r-b apply here

Re: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt

2014-08-08 Thread Thierry, Michel


 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
 Vetter
 Sent: Friday, August 08, 2014 2:49 PM
 To: Thierry, Michel
 Cc: Daniel Vetter; Intel Graphics Development
 Subject: Re: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for
aliasing
 ppgtt
 
 On Fri, Aug 08, 2014 at 01:03:53PM +, Thierry, Michel wrote:
 
   -Original Message-
   From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
 Behalf
   Of Daniel Vetter
   Sent: Wednesday, August 06, 2014 2:05 PM
   To: Intel Graphics Development
   Cc: Daniel Vetter
   Subject: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for
aliasing
  ppgtt
  
   A subsequent patch will no longer initialize the aliasing ppgtt if we
   have full ppgtt enabled, since we simply don't need that any more.
  
   Unfortunately a few places check for the aliasing ppgtt instead of
   checking for ppgtt in general. Fix them up.
  
   One special case are the gtt offset and size macros, which have some
   code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt
   is _not_ a logical address space, so passing that in as the vm is
   plain and simple a bug. So just WARN about it and carry on - we have a
   gracefully fall-through anyway if we can't find the vma.
  
   Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
   ---
drivers/gpu/drm/i915/i915_cmd_parser.c  | 4 +---
drivers/gpu/drm/i915/i915_dma.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 8 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
4 files changed, 5 insertions(+), 13 deletions(-)
  
   --
   1.9.3
  
  Sorry, I spotted another instance in gen8_ppgtt_info (debugfs). It is
more
  gen8_ppgtt_info's fault, so I'm sending a follow up patch for it.
  Unless you want to combine them...
  Reviewed-by: Michel Thierry michel.thie...@intel.com
 
 Actually I spotted that one and decided that I can't break things worse
 than it is - the aliasing ppgtt for full ppgtt is completely unused
 (except for these checks), so dumping it doesn't add value.
 
 To check: Does your r-b apply here still even without any fixup for gen8
 ppgtt_info?
 -Daniel
Yes, the r-b still applies. 
And you're also right about gen8_ppgtt_info, it wouldn't print anything
useful in its current state.
-Michel
 
   ___
   Intel-gfx mailing list
   Intel-gfx@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 
 
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch


smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt

2014-08-06 Thread Daniel Vetter
A subsequent patch will no longer initialize the aliasing ppgtt if we
have full ppgtt enabled, since we simply don't need that any more.

Unfortunately a few places check for the aliasing ppgtt instead of
checking for ppgtt in general. Fix them up.

One special case are the gtt offset and size macros, which have some
code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt
is _not_ a logical address space, so passing that in as the vm is
plain and simple a bug. So just WARN about it and carry on - we have a
gracefully fall-through anyway if we can't find the vma.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_cmd_parser.c  | 4 +---
 drivers/gpu/drm/i915/i915_dma.c | 2 +-
 drivers/gpu/drm/i915/i915_gem.c | 8 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
 4 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c 
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index dea99d92fb4a..c45856bcc8b9 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -842,8 +842,6 @@ finish:
  */
 bool i915_needs_cmd_parser(struct intel_engine_cs *ring)
 {
-   struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (!ring-needs_cmd_parser)
return false;
 
@@ -852,7 +850,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs *ring)
 * disabled. That will cause all of the parser's PPGTT checks to
 * fail. For now, disable parsing when PPGTT is off.
 */
-   if (!dev_priv-mm.aliasing_ppgtt)
+   if (USES_PPGTT(ring-dev))
return false;
 
return (i915.enable_cmd_parser == 1);
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2e7f03ad5ee2..e5ac1a6e9d26 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -999,7 +999,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
value = HAS_WT(dev);
break;
case I915_PARAM_HAS_ALIASING_PPGTT:
-   value = dev_priv-mm.aliasing_ppgtt || USES_FULL_PPGTT(dev);
+   value = USES_PPGTT(dev);
break;
case I915_PARAM_HAS_WAIT_TIMEOUT:
value = 1;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d8399ee622b9..a79deb189146 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5114,9 +5114,7 @@ unsigned long i915_gem_obj_offset(struct 
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o-base.dev-dev_private;
struct i915_vma *vma;
 
-   if (!dev_priv-mm.aliasing_ppgtt ||
-   vm == dev_priv-mm.aliasing_ppgtt-base)
-   vm = dev_priv-gtt.base;
+   WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
 
list_for_each_entry(vma, o-vma_list, vma_link) {
if (vma-vm == vm)
@@ -5157,9 +5155,7 @@ unsigned long i915_gem_obj_size(struct 
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o-base.dev-dev_private;
struct i915_vma *vma;
 
-   if (!dev_priv-mm.aliasing_ppgtt ||
-   vm == dev_priv-mm.aliasing_ppgtt-base)
-   vm = dev_priv-gtt.base;
+   WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
 
BUG_ON(list_empty(o-vma_list));
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 05969f03c0c1..390e38325b37 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1977,9 +1977,7 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs 
*ring,
  u64 offset, u32 len,
  unsigned flags)
 {
-   struct drm_i915_private *dev_priv = ring-dev-dev_private;
-   bool ppgtt = dev_priv-mm.aliasing_ppgtt != NULL 
-   !(flags  I915_DISPATCH_SECURE);
+   bool ppgtt = USES_PPGTT(ring-dev)  !(flags  I915_DISPATCH_SECURE);
int ret;
 
ret = intel_ring_begin(ring, 4);
-- 
1.9.3

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