All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
@ 2014-05-07  4:58 Ben Widawsky
  2014-05-07  7:42 ` Daniel Vetter
  2014-05-08  2:17 ` [PATCH] [v3] " Ben Widawsky
  0 siblings, 2 replies; 11+ messages in thread
From: Ben Widawsky @ 2014-05-07  4:58 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky

From: Ben Widawsky <benjamin.widawsky@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 <ben@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

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

* Re: [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-07  4:58 [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms Ben Widawsky
@ 2014-05-07  7:42 ` Daniel Vetter
  2014-05-08  1:21   ` Ben Widawsky
  2014-05-08  2:17 ` [PATCH] [v3] " Ben Widawsky
  1 sibling, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2014-05-07  7:42 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: Ben Widawsky, Intel GFX, Ben Widawsky

On Tue, May 06, 2014 at 09:58:59PM -0700, Ben Widawsky wrote:
> From: Ben Widawsky <benjamin.widawsky@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 <ben@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.vetter@ffwll.ch>o

And yes most definitely Cc: stable@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

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

* Re: [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-07  7:42 ` Daniel Vetter
@ 2014-05-08  1:21   ` Ben Widawsky
  2014-05-08  6:25     ` Jani Nikula
  0 siblings, 1 reply; 11+ messages in thread
From: Ben Widawsky @ 2014-05-08  1:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Ben Widawsky, Intel GFX, 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.widawsky@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 <ben@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.vetter@ffwll.ch>o
> 
> And yes most definitely Cc: stable@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

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

* [PATCH] [v3] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-07  4:58 [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms Ben Widawsky
  2014-05-07  7:42 ` Daniel Vetter
@ 2014-05-08  2:17 ` Ben Widawsky
  2014-05-13  7:41   ` [Intel-gfx] " Jani Nikula
  1 sibling, 1 reply; 11+ messages in thread
From: Ben Widawsky @ 2014-05-08  2:17 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky, stable, Ben Widawsky

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.widawsky@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

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
Cc: stable@vger.kernel.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 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 4d87bf2..8d6d531 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2640,6 +2640,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_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

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

* Re: [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-08  1:21   ` Ben Widawsky
@ 2014-05-08  6:25     ` Jani Nikula
  2014-05-12 16:42       ` Daniel Vetter
  0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2014-05-08  6:25 UTC (permalink / raw)
  To: Ben Widawsky, Daniel Vetter; +Cc: Ben Widawsky, Intel GFX, Ben Widawsky

On Thu, 08 May 2014, Ben Widawsky <ben@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

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

* Re: [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-08  6:25     ` Jani Nikula
@ 2014-05-12 16:42       ` Daniel Vetter
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-05-12 16:42 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Ben Widawsky, Ben Widawsky, Intel GFX, Ben Widawsky

On Thu, May 08, 2014 at 09:25:42AM +0300, Jani Nikula wrote:
> On Thu, 08 May 2014, Ben Widawsky <ben@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

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

* Re: [Intel-gfx] [PATCH] [v3] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-08  2:17 ` [PATCH] [v3] " Ben Widawsky
@ 2014-05-13  7:41   ` Jani Nikula
  2014-05-19 15:13     ` Jesse Barnes
  0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2014-05-13  7:41 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky, stable, Ben Widawsky

On Thu, 08 May 2014, Ben Widawsky <benjamin.widawsky@intel.com> wrote:
> 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.widawsky@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
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
> Cc: stable@vger.kernel.org
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  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 4d87bf2..8d6d531 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2640,6 +2640,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_32BIT

There's no such config for x86.

There's CONFIG_X86_32 though. Which I think matches the original
approach of #ifndef CONFIG_64BIT in the referenced bug. However, "with
this patch only, the system still hung while booting." [1].

I want a new patch with Tested-by on it.


BR,
Jani.


[1] https://bugs.freedesktop.org/show_bug.cgi?id=76619#c16



> +	/* 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

-- 
Jani Nikula, Intel Open Source Technology Center

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

* Re: [PATCH] [v3] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-13  7:41   ` [Intel-gfx] " Jani Nikula
@ 2014-05-19 15:13     ` Jesse Barnes
  2014-05-19 22:46       ` [PATCH] " Rodrigo Vivi
  0 siblings, 1 reply; 11+ messages in thread
From: Jesse Barnes @ 2014-05-19 15:13 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Intel GFX, Ben Widawsky, stable, Ben Widawsky

On Tue, 13 May 2014 10:41:58 +0300
Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Thu, 08 May 2014, Ben Widawsky <benjamin.widawsky@intel.com> wrote:
> > 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.widawsky@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
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76619
> > Cc: stable@vger.kernel.org
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  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 4d87bf2..8d6d531 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2640,6 +2640,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_32BIT
> 
> There's no such config for x86.
> 
> There's CONFIG_X86_32 though. Which I think matches the original
> approach of #ifndef CONFIG_64BIT in the referenced bug. However, "with
> this patch only, the system still hung while booting." [1].
> 
> I want a new patch with Tested-by on it.
> 
> 
> BR,
> Jani.
> 
> 
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=76619#c16

Ben, will you re-post this or can you find someone else who can?

-- 
Jesse Barnes, Intel Open Source Technology Center

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

* [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-19 15:13     ` Jesse Barnes
@ 2014-05-19 22:46       ` Rodrigo Vivi
  2014-05-20  7:15         ` Chris Wilson
  2014-05-20  7:55         ` Daniel Vetter
  0 siblings, 2 replies; 11+ messages in thread
From: Rodrigo Vivi @ 2014-05-19 22:46 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, stable, Ben Widawsky

From: Ben Widawsky <benjamin.widawsky@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.widawsky@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: stable@vger.kernel.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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

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

* Re: [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-19 22:46       ` [PATCH] " Rodrigo Vivi
@ 2014-05-20  7:15         ` Chris Wilson
  2014-05-20  7:55         ` Daniel Vetter
  1 sibling, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2014-05-20  7:15 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Ben Widawsky, stable, Ben Widawsky

On Mon, May 19, 2014 at 03:46:57PM -0700, Rodrigo Vivi wrote:
> From: Ben Widawsky <benjamin.widawsky@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.widawsky@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: stable@vger.kernel.org
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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

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

* Re: [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms
  2014-05-19 22:46       ` [PATCH] " Rodrigo Vivi
  2014-05-20  7:15         ` Chris Wilson
@ 2014-05-20  7:55         ` Daniel Vetter
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-05-20  7:55 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Ben Widawsky, stable, Ben Widawsky

On Mon, May 19, 2014 at 03:46:57PM -0700, Rodrigo Vivi wrote:
> From: Ben Widawsky <benjamin.widawsky@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.widawsky@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: stable@vger.kernel.org
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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

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

end of thread, other threads:[~2014-05-20  7:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-07  4:58 [PATCH] drm/i915/bdw: Only use 2g GGTT for 32b platforms Ben Widawsky
2014-05-07  7:42 ` Daniel Vetter
2014-05-08  1:21   ` Ben Widawsky
2014-05-08  6:25     ` Jani Nikula
2014-05-12 16:42       ` Daniel Vetter
2014-05-08  2:17 ` [PATCH] [v3] " Ben Widawsky
2014-05-13  7:41   ` [Intel-gfx] " Jani Nikula
2014-05-19 15:13     ` Jesse Barnes
2014-05-19 22:46       ` [PATCH] " Rodrigo Vivi
2014-05-20  7:15         ` Chris Wilson
2014-05-20  7:55         ` Daniel Vetter

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.