Re: [Intel-gfx] [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms

2014-05-20 Thread Chris Wilson
On Mon, May 19, 2014 at 03:46:57PM -0700, Rodrigo Vivi wrote:
 From: Ben Widawsky benjamin.widaw...@intel.com
 
 Daniel requested in the bug that I use a 3GB fallback size. Since this
 is not in the spec as a valid size, I decided against it. We could
 potentially add a patch to bump it to 3GB on top of this one.
 
 This probably should be CC: stable - but I'll let the powers that be
 decide that one.
 
 Regression from a revert of the revert:
 commit 7907f45bf9f67a1c5e5d4ae05bab428d7c2f43b2
 Author: Ben Widawsky benjamin.widaw...@intel.com
 Date:   Wed Feb 19 22:05:46 2014 -0800
 
 Revert drm/i915/bdw: Limit GTT to 2GB
 
 v2: Change ifdef to 32b, instead of ifndef
 update comment
 
 v3. Update comment to not wrap (Daniel).
 Update commit message
 
 v4: s/CONFIG_32/CONFIG_X86_32 (Jani).
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
 Cc: sta...@vger.kernel.org
 Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
 ---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
 b/drivers/gpu/drm/i915/i915_gem_gtt.c
 index afd4eef..050e47e 100644
 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
 +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
 @@ -1773,6 +1773,13 @@ static inline unsigned int gen8_get_total_gtt_size(u16 
 bdw_gmch_ctl)
   bdw_gmch_ctl = BDW_GMCH_GGMS_MASK;
   if (bdw_gmch_ctl)
   bdw_gmch_ctl = 1  bdw_gmch_ctl;
 +
 +#ifdef CONFIG_X86_32BIT
 + /* Limit 32B platforms to a 2GB GGTT: 4  20 / pte size * PAGE_SIZE */

We might as well be picky and write 32b not 32B.
-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/bdw: Only use 2g GGTT for 32b platforms

2014-05-20 Thread Daniel Vetter
On Mon, May 19, 2014 at 03:46:57PM -0700, Rodrigo Vivi wrote:
 From: Ben Widawsky benjamin.widaw...@intel.com
 
 Daniel requested in the bug that I use a 3GB fallback size. Since this
 is not in the spec as a valid size, I decided against it. We could
 potentially add a patch to bump it to 3GB on top of this one.
 
 This probably should be CC: stable - but I'll let the powers that be
 decide that one.
 
 Regression from a revert of the revert:
 commit 7907f45bf9f67a1c5e5d4ae05bab428d7c2f43b2
 Author: Ben Widawsky benjamin.widaw...@intel.com
 Date:   Wed Feb 19 22:05:46 2014 -0800
 
 Revert drm/i915/bdw: Limit GTT to 2GB
 
 v2: Change ifdef to 32b, instead of ifndef
 update comment
 
 v3. Update comment to not wrap (Daniel).
 Update commit message
 
 v4: s/CONFIG_32/CONFIG_X86_32 (Jani).
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
 Cc: sta...@vger.kernel.org
 Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com

Do we have a tested-by on this? Iirc it doesn't actually work, or at least
it's an incomplete solution apparently ... QA says things still blow up
with this patch applied:

https://bugs.freedesktop.org/show_bug.cgi?id=76619#c30

Cheers, Daniel
 ---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
 b/drivers/gpu/drm/i915/i915_gem_gtt.c
 index afd4eef..050e47e 100644
 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
 +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
 @@ -1773,6 +1773,13 @@ static inline unsigned int gen8_get_total_gtt_size(u16 
 bdw_gmch_ctl)
   bdw_gmch_ctl = BDW_GMCH_GGMS_MASK;
   if (bdw_gmch_ctl)
   bdw_gmch_ctl = 1  bdw_gmch_ctl;
 +
 +#ifdef CONFIG_X86_32BIT
 + /* Limit 32B platforms to a 2GB GGTT: 4  20 / pte size * PAGE_SIZE */
 + if (bdw_gmch_ctl  4)
 + bdw_gmch_ctl = 4;
 +#endif
 +
   return bdw_gmch_ctl  20;
  }
  
 -- 
 1.9.0
 
 ___
 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms

2014-05-19 Thread Rodrigo Vivi
From: Ben Widawsky benjamin.widaw...@intel.com

Daniel requested in the bug that I use a 3GB fallback size. Since this
is not in the spec as a valid size, I decided against it. We could
potentially add a patch to bump it to 3GB on top of this one.

This probably should be CC: stable - but I'll let the powers that be
decide that one.

Regression from a revert of the revert:
commit 7907f45bf9f67a1c5e5d4ae05bab428d7c2f43b2
Author: Ben Widawsky benjamin.widaw...@intel.com
Date:   Wed Feb 19 22:05:46 2014 -0800

Revert drm/i915/bdw: Limit GTT to 2GB

v2: Change ifdef to 32b, instead of ifndef
update comment

v3. Update comment to not wrap (Daniel).
Update commit message

v4: s/CONFIG_32/CONFIG_X86_32 (Jani).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
Cc: sta...@vger.kernel.org
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index afd4eef..050e47e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1773,6 +1773,13 @@ static inline unsigned int gen8_get_total_gtt_size(u16 
bdw_gmch_ctl)
bdw_gmch_ctl = BDW_GMCH_GGMS_MASK;
if (bdw_gmch_ctl)
bdw_gmch_ctl = 1  bdw_gmch_ctl;
+
+#ifdef CONFIG_X86_32BIT
+   /* Limit 32B platforms to a 2GB GGTT: 4  20 / pte size * PAGE_SIZE */
+   if (bdw_gmch_ctl  4)
+   bdw_gmch_ctl = 4;
+#endif
+
return bdw_gmch_ctl  20;
 }
 
-- 
1.9.0

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


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms

2014-05-12 Thread Daniel Vetter
On Thu, May 08, 2014 at 09:25:42AM +0300, Jani Nikula wrote:
 On Thu, 08 May 2014, Ben Widawsky b...@bwidawsk.net wrote:
  On Wed, May 07, 2014 at 09:42:57AM +0200, Daniel Vetter wrote:
  Aside: Please add the regression tags when handling bugs, I need those for
  tracking and stats.
  -Daniel
  
 
  I don't know what a regression tag is.
 
 [regression] at the front of the bug summary line.

Also valid are [bisected] (used by QA when they supply the bisect result)
and on kernel bugzilla there's an explicit boolean for this.

Doing this is about the no. 1 job of doing bug scrub ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms

2014-05-08 Thread Jani Nikula
On Thu, 08 May 2014, Ben Widawsky b...@bwidawsk.net wrote:
 On Wed, May 07, 2014 at 09:42:57AM +0200, Daniel Vetter wrote:
 Aside: Please add the regression tags when handling bugs, I need those for
 tracking and stats.
 -Daniel
 

 I don't know what a regression tag is.

[regression] at the front of the bug summary line.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms

2014-05-07 Thread Daniel Vetter
On Tue, May 06, 2014 at 09:58:59PM -0700, Ben Widawsky wrote:
 From: Ben Widawsky benjamin.widaw...@linux.intel.com
 
 Daniel requested in the bug that I use a 3GB fallback size. Since this
 is not in the spec as a valid size, I decided against it. We could
 potentially add a patch to bump it to 3GB on top of this one.
 
 This probably should be CC: stable - but I'll let the powers that be
 decide that one.
 
 v2: Change ifdef to 32b, instead of ifndef
 update comment
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 ---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 8 
  1 file changed, 8 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
 b/drivers/gpu/drm/i915/i915_gem_gtt.c
 index 846b6ee..d03a540 100644
 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
 +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
 @@ -1759,6 +1759,14 @@ static inline unsigned int gen8_get_total_gtt_size(u16 
 bdw_gmch_ctl)
   bdw_gmch_ctl = BDW_GMCH_GGMS_MASK;
   if (bdw_gmch_ctl)
   bdw_gmch_ctl = 1  bdw_gmch_ctl;
 +
 +#ifdef CONFIG_32BIT
 + /* Limit 32B platforms to a 2GB GGTT
 + 4  20 / pte size * PAGE_SIZE */
 + if (bdw_gmch_ctl  4)
 + bdw_gmch_ctl = 4;

Comment needs CodingStyle polish and we might as well return the right
value directly instead of adjusting bdw_gmch_ctl. With that polish applied
this is Reviewed-by: Daniel Vetter daniel.vet...@ffwll.cho

And yes most definitely Cc: sta...@vger.kernel.org since it's a
regression.

Aside: Please add the regression tags when handling bugs, I need those for
tracking and stats.
-Daniel

 +#endif
 +
   return bdw_gmch_ctl  20;
  }
  
 -- 
 1.9.2
 
 ___
 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms

2014-05-07 Thread Ben Widawsky
On Wed, May 07, 2014 at 09:42:57AM +0200, Daniel Vetter wrote:
 On Tue, May 06, 2014 at 09:58:59PM -0700, Ben Widawsky wrote:
  From: Ben Widawsky benjamin.widaw...@linux.intel.com
  
  Daniel requested in the bug that I use a 3GB fallback size. Since this
  is not in the spec as a valid size, I decided against it. We could
  potentially add a patch to bump it to 3GB on top of this one.
  
  This probably should be CC: stable - but I'll let the powers that be
  decide that one.
  
  v2: Change ifdef to 32b, instead of ifndef
  update comment
  
  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
  ---
   drivers/gpu/drm/i915/i915_gem_gtt.c | 8 
   1 file changed, 8 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
  b/drivers/gpu/drm/i915/i915_gem_gtt.c
  index 846b6ee..d03a540 100644
  --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
  +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
  @@ -1759,6 +1759,14 @@ static inline unsigned int 
  gen8_get_total_gtt_size(u16 bdw_gmch_ctl)
  bdw_gmch_ctl = BDW_GMCH_GGMS_MASK;
  if (bdw_gmch_ctl)
  bdw_gmch_ctl = 1  bdw_gmch_ctl;
  +
  +#ifdef CONFIG_32BIT
  +   /* Limit 32B platforms to a 2GB GGTT
  +   4  20 / pte size * PAGE_SIZE */
  +   if (bdw_gmch_ctl  4)
  +   bdw_gmch_ctl = 4;
 
 Comment needs CodingStyle polish and we might as well return the right
 value directly instead of adjusting bdw_gmch_ctl. With that polish applied
 this is Reviewed-by: Daniel Vetter daniel.vet...@ffwll.cho
 
 And yes most definitely Cc: sta...@vger.kernel.org since it's a
 regression.
 
 Aside: Please add the regression tags when handling bugs, I need those for
 tracking and stats.
 -Daniel
 

I don't know what a regression tag is.

  +#endif
  +
  return bdw_gmch_ctl  20;
   }
   
  -- 
  1.9.2
  
  ___
  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
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms

2014-05-06 Thread Ben Widawsky
From: Ben Widawsky benjamin.widaw...@linux.intel.com

Daniel requested in the bug that I use a 3GB fallback size. Since this
is not in the spec as a valid size, I decided against it. We could
potentially add a patch to bump it to 3GB on top of this one.

This probably should be CC: stable - but I'll let the powers that be
decide that one.

v2: Change ifdef to 32b, instead of ifndef
update comment

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 846b6ee..d03a540 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1759,6 +1759,14 @@ static inline unsigned int gen8_get_total_gtt_size(u16 
bdw_gmch_ctl)
bdw_gmch_ctl = BDW_GMCH_GGMS_MASK;
if (bdw_gmch_ctl)
bdw_gmch_ctl = 1  bdw_gmch_ctl;
+
+#ifdef CONFIG_32BIT
+   /* Limit 32B platforms to a 2GB GGTT
+   4  20 / pte size * PAGE_SIZE */
+   if (bdw_gmch_ctl  4)
+   bdw_gmch_ctl = 4;
+#endif
+
return bdw_gmch_ctl  20;
 }
 
-- 
1.9.2

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