* [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
@ 2016-12-14 14:55 ` Paulo Zanoni
0 siblings, 0 replies; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-14 14:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, stable
BSpec got updated and this workaround is now listed as standard
required programming for all subsequent projects. This is confirmed to
fix Skylake screen flickering issues (probably caused by the fact that
we initialized a ring in the first page of stolen, but I didn't 100%
confirm this theory).
v2: this is the patch that fixes the screen flickering, document it.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Cc: stable@vger.kernel.org
Tested-by: Dominik Klementowski <dominik232@gmail.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index efc0e74..b1c8897 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -55,10 +55,9 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
return -ENODEV;
/* See the comment at the drm_mm_init() call for more about this check.
- * WaSkipStolenMemoryFirstPage:bdw,chv,kbl (incomplete)
+ * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
*/
- if (start < 4096 && (IS_GEN8(dev_priv) ||
- IS_KBL_REVID(dev_priv, 0, KBL_REVID_A0)))
+ if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
start = 4096;
mutex_lock(&dev_priv->mm.stolen_lock);
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
@ 2016-12-14 14:55 ` Paulo Zanoni
0 siblings, 0 replies; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-14 14:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, stable
BSpec got updated and this workaround is now listed as standard
required programming for all subsequent projects. This is confirmed to
fix Skylake screen flickering issues (probably caused by the fact that
we initialized a ring in the first page of stolen, but I didn't 100%
confirm this theory).
v2: this is the patch that fixes the screen flickering, document it.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Cc: stable@vger.kernel.org
Tested-by: Dominik Klementowski <dominik232@gmail.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index efc0e74..b1c8897 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -55,10 +55,9 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
return -ENODEV;
/* See the comment at the drm_mm_init() call for more about this check.
- * WaSkipStolenMemoryFirstPage:bdw,chv,kbl (incomplete)
+ * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
*/
- if (start < 4096 && (IS_GEN8(dev_priv) ||
- IS_KBL_REVID(dev_priv, 0, KBL_REVID_A0)))
+ if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
start = 4096;
mutex_lock(&dev_priv->mm.stolen_lock);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-14 14:55 ` Paulo Zanoni
(?)
@ 2016-12-14 14:55 ` Paulo Zanoni
2016-12-14 15:43 ` Chris Wilson
2016-12-14 16:39 ` Ville Syrjälä
-1 siblings, 2 replies; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-14 14:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Reserve the first page of stolen memory right after initializing the
mm allocator. This means that we won't inherit the FB in case the BIOS
decides to put it at the start of stolen. But the BIOS should not be
putting it at the start of stolen since it's going to get corrupted. I
suppose the bug here is that some pixels at the very top of the screen
will be corrupted, so it's not exactly easy to notice.
We have confirmation that the first page of stolen does actually get
corrupted, so I really think we should do this in order to avoid any
possible future headaches, even if that means losing BIOS framebuffer
inheritance. Let's not use the HW in a way it's not supposed to be
used.
v2: don't even put the first page on the mm (Chris).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b1c8897..7435540 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
if (!drm_mm_initialized(&dev_priv->mm.stolen))
return -ENODEV;
- /* See the comment at the drm_mm_init() call for more about this check.
- * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
- */
- if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
- start = 4096;
-
mutex_lock(&dev_priv->mm.stolen_lock);
ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
alignment, start, end,
@@ -410,7 +404,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
{
struct i915_ggtt *ggtt = &dev_priv->ggtt;
unsigned long reserved_total, reserved_base = 0, reserved_size;
- unsigned long stolen_top;
+ unsigned long stolen_start = 0, stolen_top;
mutex_init(&dev_priv->mm.stolen_lock);
@@ -491,18 +485,13 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
- /*
- * Basic memrange allocator for stolen space.
- *
- * TODO: Notice that some platforms require us to not use the first page
- * of the stolen memory but their BIOSes may still put the framebuffer
- * on the first page. So we don't reserve this page for now because of
- * that. Our current solution is to just prevent new nodes from being
- * inserted on the first page - see the check we have at
- * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
- * problem later.
- */
- drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
+ /* WaSkipStolenMemoryFirstPage:bdw+ */
+ if (INTEL_GEN(dev_priv) >= 8)
+ stolen_start = 4096;
+
+ /* Basic memrange allocator for stolen space. */
+ drm_mm_init(&dev_priv->mm.stolen, stolen_start,
+ ggtt->stolen_usable_size);
return 0;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-14 14:55 ` [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage Paulo Zanoni
@ 2016-12-14 15:43 ` Chris Wilson
2016-12-14 16:39 ` Ville Syrjälä
1 sibling, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2016-12-14 15:43 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Wed, Dec 14, 2016 at 12:55:38PM -0200, Paulo Zanoni wrote:
> Reserve the first page of stolen memory right after initializing the
> mm allocator. This means that we won't inherit the FB in case the BIOS
> decides to put it at the start of stolen. But the BIOS should not be
> putting it at the start of stolen since it's going to get corrupted. I
> suppose the bug here is that some pixels at the very top of the screen
> will be corrupted, so it's not exactly easy to notice.
>
> We have confirmation that the first page of stolen does actually get
> corrupted, so I really think we should do this in order to avoid any
> possible future headaches, even if that means losing BIOS framebuffer
> inheritance. Let's not use the HW in a way it's not supposed to be
> used.
>
> v2: don't even put the first page on the mm (Chris).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Mental note: need to add to the list of test cases (insert_in_range >
range of drm_mm).
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
(code, I'll take your word that w/a accurately surmises the situation
and is the blessed strategy)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
2016-12-14 14:55 ` Paulo Zanoni
(?)
(?)
@ 2016-12-14 15:43 ` Chris Wilson
-1 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2016-12-14 15:43 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, stable
On Wed, Dec 14, 2016 at 12:55:37PM -0200, Paulo Zanoni wrote:
> BSpec got updated and this workaround is now listed as standard
> required programming for all subsequent projects. This is confirmed to
> fix Skylake screen flickering issues (probably caused by the fact that
> we initialized a ring in the first page of stolen, but I didn't 100%
> confirm this theory).
>
> v2: this is the patch that fixes the screen flickering, document it.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: stable@vger.kernel.org
> Tested-by: Dominik Klementowski <dominik232@gmail.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
2016-12-14 14:55 ` Paulo Zanoni
` (2 preceding siblings ...)
(?)
@ 2016-12-14 15:53 ` Patchwork
-1 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2016-12-14 15:53 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
URL : https://patchwork.freedesktop.org/series/16800/
State : success
== Summary ==
Series 16800v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16800/revisions/1/mbox/
fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39
fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52
fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20
fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20
fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
24fa971ea77320a76c074a8eb31eca00b321ec73 drm-tip: 2016y-12m-14d-13h-51m-36s UTC integration manifest
02b3767 drm/i915: fully apply WaSkipStolenMemoryFirstPage
d882064 drm/i915: skip the first 4k of stolen memory on everything >= gen8
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3287/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-14 14:55 ` [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage Paulo Zanoni
2016-12-14 15:43 ` Chris Wilson
@ 2016-12-14 16:39 ` Ville Syrjälä
2016-12-14 16:50 ` Chris Wilson
1 sibling, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2016-12-14 16:39 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Wed, Dec 14, 2016 at 12:55:38PM -0200, Paulo Zanoni wrote:
> Reserve the first page of stolen memory right after initializing the
> mm allocator. This means that we won't inherit the FB in case the BIOS
> decides to put it at the start of stolen. But the BIOS should not be
> putting it at the start of stolen since it's going to get corrupted. I
> suppose the bug here is that some pixels at the very top of the screen
> will be corrupted, so it's not exactly easy to notice.
>
> We have confirmation that the first page of stolen does actually get
> corrupted, so I really think we should do this in order to avoid any
> possible future headaches, even if that means losing BIOS framebuffer
> inheritance. Let's not use the HW in a way it's not supposed to be
> used.
>
> v2: don't even put the first page on the mm (Chris).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_stolen.c | 27 ++++++++-------------------
> 1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b1c8897..7435540 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> if (!drm_mm_initialized(&dev_priv->mm.stolen))
> return -ENODEV;
>
> - /* See the comment at the drm_mm_init() call for more about this check.
> - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> - */
> - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> - start = 4096;
> -
> mutex_lock(&dev_priv->mm.stolen_lock);
> ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> alignment, start, end,
> @@ -410,7 +404,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> {
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> unsigned long reserved_total, reserved_base = 0, reserved_size;
> - unsigned long stolen_top;
> + unsigned long stolen_start = 0, stolen_top;
>
> mutex_init(&dev_priv->mm.stolen_lock);
>
> @@ -491,18 +485,13 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>
> ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
>
> - /*
> - * Basic memrange allocator for stolen space.
> - *
> - * TODO: Notice that some platforms require us to not use the first page
> - * of the stolen memory but their BIOSes may still put the framebuffer
> - * on the first page. So we don't reserve this page for now because of
> - * that. Our current solution is to just prevent new nodes from being
> - * inserted on the first page - see the check we have at
> - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
> - * problem later.
> - */
> - drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
> + /* WaSkipStolenMemoryFirstPage:bdw+ */
> + if (INTEL_GEN(dev_priv) >= 8)
> + stolen_start = 4096;
> +
> + /* Basic memrange allocator for stolen space. */
> + drm_mm_init(&dev_priv->mm.stolen, stolen_start,
> + ggtt->stolen_usable_size);
Don't you need to reduce the size by 4k as well?
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-14 16:39 ` Ville Syrjälä
@ 2016-12-14 16:50 ` Chris Wilson
2016-12-14 19:55 ` Paulo Zanoni
0 siblings, 1 reply; 21+ messages in thread
From: Chris Wilson @ 2016-12-14 16:50 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Paulo Zanoni
On Wed, Dec 14, 2016 at 06:39:08PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 14, 2016 at 12:55:38PM -0200, Paulo Zanoni wrote:
> > Reserve the first page of stolen memory right after initializing the
> > mm allocator. This means that we won't inherit the FB in case the BIOS
> > decides to put it at the start of stolen. But the BIOS should not be
> > putting it at the start of stolen since it's going to get corrupted. I
> > suppose the bug here is that some pixels at the very top of the screen
> > will be corrupted, so it's not exactly easy to notice.
> >
> > We have confirmation that the first page of stolen does actually get
> > corrupted, so I really think we should do this in order to avoid any
> > possible future headaches, even if that means losing BIOS framebuffer
> > inheritance. Let's not use the HW in a way it's not supposed to be
> > used.
> >
> > v2: don't even put the first page on the mm (Chris).
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_stolen.c | 27 ++++++++-------------------
> > 1 file changed, 8 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index b1c8897..7435540 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> > if (!drm_mm_initialized(&dev_priv->mm.stolen))
> > return -ENODEV;
> >
> > - /* See the comment at the drm_mm_init() call for more about this check.
> > - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> > - */
> > - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> > - start = 4096;
> > -
> > mutex_lock(&dev_priv->mm.stolen_lock);
> > ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> > alignment, start, end,
> > @@ -410,7 +404,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> > {
> > struct i915_ggtt *ggtt = &dev_priv->ggtt;
> > unsigned long reserved_total, reserved_base = 0, reserved_size;
> > - unsigned long stolen_top;
> > + unsigned long stolen_start = 0, stolen_top;
> >
> > mutex_init(&dev_priv->mm.stolen_lock);
> >
> > @@ -491,18 +485,13 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> >
> > ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
> >
> > - /*
> > - * Basic memrange allocator for stolen space.
> > - *
> > - * TODO: Notice that some platforms require us to not use the first page
> > - * of the stolen memory but their BIOSes may still put the framebuffer
> > - * on the first page. So we don't reserve this page for now because of
> > - * that. Our current solution is to just prevent new nodes from being
> > - * inserted on the first page - see the check we have at
> > - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
> > - * problem later.
> > - */
> > - drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
> > + /* WaSkipStolenMemoryFirstPage:bdw+ */
> > + if (INTEL_GEN(dev_priv) >= 8)
> > + stolen_start = 4096;
> > +
> > + /* Basic memrange allocator for stolen space. */
> > + drm_mm_init(&dev_priv->mm.stolen, stolen_start,
> > + ggtt->stolen_usable_size);
>
> Don't you need to reduce the size by 4k as well?
Sigh. Yes. No more review for me.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-14 16:50 ` Chris Wilson
@ 2016-12-14 19:55 ` Paulo Zanoni
2016-12-15 7:54 ` Daniel Vetter
2016-12-15 8:17 ` Chris Wilson
0 siblings, 2 replies; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-14 19:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Don't even tell the mm allocator to handle the first page of stolen on
the affected platforms. This means that we won't inherit the FB in
case the BIOS decides to put it at the start of stolen. But the BIOS
should not be putting it at the start of stolen since it's going to
get corrupted. I suppose the bug here is that some pixels at the very
top of the screen will be corrupted, so it's not exactly easy to
notice.
We have confirmation that the first page of stolen does actually get
corrupted, so I really think we should do this in order to avoid any
possible future headaches, even if that means losing BIOS framebuffer
inheritance. Let's not use the HW in a way it's not supposed to be
used.
v2: don't even put the first page on the mm (Chris).
v3: drm_mm_init() takes size instead of end as argument (Ville).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
drivers/gpu/drm/i915/i915_gem_stolen.c | 34 +++++++++++++---------------------
drivers/gpu/drm/i915/intel_fbc.c | 6 +++---
3 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8965bbb..aefc968 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -316,6 +316,7 @@ struct i915_ggtt {
struct io_mapping mappable; /* Mapping to our CPU mappable region */
size_t stolen_size; /* Total size of stolen memory */
+ size_t stolen_usable_start; /* First page may be unusable. */
size_t stolen_usable_size; /* Total size minus BIOS reserved */
size_t stolen_reserved_base;
size_t stolen_reserved_size;
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b1c8897..56d7e0b 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
if (!drm_mm_initialized(&dev_priv->mm.stolen))
return -ENODEV;
- /* See the comment at the drm_mm_init() call for more about this check.
- * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
- */
- if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
- start = 4096;
-
mutex_lock(&dev_priv->mm.stolen_lock);
ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
alignment, start, end,
@@ -74,10 +68,11 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
unsigned alignment)
{
struct i915_ggtt *ggtt = &dev_priv->ggtt;
+ u64 start = ggtt->stolen_usable_start;
+ u64 end = start + ggtt->stolen_usable_size;
return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
- alignment, 0,
- ggtt->stolen_usable_size);
+ alignment, start, end);
}
void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
@@ -489,20 +484,17 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
ggtt->stolen_size >> 10,
(ggtt->stolen_size - reserved_total) >> 10);
- ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
+ ggtt->stolen_usable_start = 0;
+ /* WaSkipStolenMemoryFirstPage:bdw+ */
+ if (INTEL_GEN(dev_priv) >= 8)
+ ggtt->stolen_usable_start = 4096;
- /*
- * Basic memrange allocator for stolen space.
- *
- * TODO: Notice that some platforms require us to not use the first page
- * of the stolen memory but their BIOSes may still put the framebuffer
- * on the first page. So we don't reserve this page for now because of
- * that. Our current solution is to just prevent new nodes from being
- * inserted on the first page - see the check we have at
- * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
- * problem later.
- */
- drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
+ ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total -
+ ggtt->stolen_usable_start;
+
+ /* Basic memrange allocator for stolen space. */
+ drm_mm_init(&dev_priv->mm.stolen, ggtt->stolen_usable_start,
+ ggtt->stolen_usable_size);
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 62f215b..534fcb3c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -528,7 +528,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
struct i915_ggtt *ggtt = &dev_priv->ggtt;
int compression_threshold = 1;
int ret;
- u64 end;
+ u64 start = ggtt->stolen_usable_start, end;
/* The FBC hardware for BDW/SKL doesn't have access to the stolen
* reserved range size, so it always assumes the maximum (8mb) is used.
@@ -538,7 +538,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
end = ggtt->stolen_size - 8 * 1024 * 1024;
else
- end = ggtt->stolen_usable_size;
+ end = start + ggtt->stolen_usable_size;
/* HACK: This code depends on what we will do in *_enable_fbc. If that
* code changes, this code needs to change as well.
@@ -549,7 +549,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
/* Try to over-allocate to reduce reallocations and fragmentation. */
ret = i915_gem_stolen_insert_node_in_range(dev_priv, node, size <<= 1,
- 4096, 0, end);
+ 4096, start, end);
if (ret == 0)
return compression_threshold;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 (rev2)
2016-12-14 14:55 ` Paulo Zanoni
` (3 preceding siblings ...)
(?)
@ 2016-12-14 20:22 ` Patchwork
-1 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2016-12-14 20:22 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 (rev2)
URL : https://patchwork.freedesktop.org/series/16800/
State : success
== Summary ==
Series 16800v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16800/revisions/2/mbox/
Test kms_force_connector_basic:
Subgroup force-connector-state:
dmesg-warn -> PASS (fi-snb-2520m)
fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39
fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52
fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20
fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20
fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
24fa971ea77320a76c074a8eb31eca00b321ec73 drm-tip: 2016y-12m-14d-13h-51m-36s UTC integration manifest
c60007e drm/i915: fully apply WaSkipStolenMemoryFirstPage
1e9c3ce drm/i915: skip the first 4k of stolen memory on everything >= gen8
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3291/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-14 19:55 ` Paulo Zanoni
@ 2016-12-15 7:54 ` Daniel Vetter
2016-12-15 8:17 ` Chris Wilson
1 sibling, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-12-15 7:54 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Wed, Dec 14, 2016 at 05:55:39PM -0200, Paulo Zanoni wrote:
> Don't even tell the mm allocator to handle the first page of stolen on
> the affected platforms. This means that we won't inherit the FB in
> case the BIOS decides to put it at the start of stolen. But the BIOS
> should not be putting it at the start of stolen since it's going to
> get corrupted. I suppose the bug here is that some pixels at the very
> top of the screen will be corrupted, so it's not exactly easy to
> notice.
>
> We have confirmation that the first page of stolen does actually get
> corrupted, so I really think we should do this in order to avoid any
> possible future headaches, even if that means losing BIOS framebuffer
> inheritance. Let's not use the HW in a way it's not supposed to be
> used.
>
> v2: don't even put the first page on the mm (Chris).
> v3: drm_mm_init() takes size instead of end as argument (Ville).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
> drivers/gpu/drm/i915/i915_gem_stolen.c | 34 +++++++++++++---------------------
> drivers/gpu/drm/i915/intel_fbc.c | 6 +++---
> 3 files changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 8965bbb..aefc968 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -316,6 +316,7 @@ struct i915_ggtt {
> struct io_mapping mappable; /* Mapping to our CPU mappable region */
>
> size_t stolen_size; /* Total size of stolen memory */
> + size_t stolen_usable_start; /* First page may be unusable. */
> size_t stolen_usable_size; /* Total size minus BIOS reserved */
> size_t stolen_reserved_base;
> size_t stolen_reserved_size;
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b1c8897..56d7e0b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> if (!drm_mm_initialized(&dev_priv->mm.stolen))
> return -ENODEV;
>
> - /* See the comment at the drm_mm_init() call for more about this check.
> - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> - */
> - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> - start = 4096;
> -
> mutex_lock(&dev_priv->mm.stolen_lock);
> ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> alignment, start, end,
> @@ -74,10 +68,11 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> unsigned alignment)
> {
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> + u64 start = ggtt->stolen_usable_start;
> + u64 end = start + ggtt->stolen_usable_size;
>
> return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
> - alignment, 0,
> - ggtt->stolen_usable_size);
> + alignment, start, end);
> }
>
> void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
> @@ -489,20 +484,17 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> ggtt->stolen_size >> 10,
> (ggtt->stolen_size - reserved_total) >> 10);
>
> - ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
> + ggtt->stolen_usable_start = 0;
> + /* WaSkipStolenMemoryFirstPage:bdw+ */
> + if (INTEL_GEN(dev_priv) >= 8)
> + ggtt->stolen_usable_start = 4096;
>
> - /*
> - * Basic memrange allocator for stolen space.
> - *
> - * TODO: Notice that some platforms require us to not use the first page
> - * of the stolen memory but their BIOSes may still put the framebuffer
> - * on the first page. So we don't reserve this page for now because of
> - * that. Our current solution is to just prevent new nodes from being
> - * inserted on the first page - see the check we have at
> - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
> - * problem later.
> - */
> - drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
> + ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total -
> + ggtt->stolen_usable_start;
> +
> + /* Basic memrange allocator for stolen space. */
> + drm_mm_init(&dev_priv->mm.stolen, ggtt->stolen_usable_start,
> + ggtt->stolen_usable_size);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 62f215b..534fcb3c 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -528,7 +528,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> int compression_threshold = 1;
> int ret;
> - u64 end;
> + u64 start = ggtt->stolen_usable_start, end;
>
> /* The FBC hardware for BDW/SKL doesn't have access to the stolen
> * reserved range size, so it always assumes the maximum (8mb) is used.
> @@ -538,7 +538,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> end = ggtt->stolen_size - 8 * 1024 * 1024;
> else
> - end = ggtt->stolen_usable_size;
> + end = start + ggtt->stolen_usable_size;
Funny mix&match of size end end we have :( We might want to switch
drm_mm_init to start/end for consistency with all the _in_range functions.
Let's see whether I'll regret this too ...
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> /* HACK: This code depends on what we will do in *_enable_fbc. If that
> * code changes, this code needs to change as well.
> @@ -549,7 +549,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
>
> /* Try to over-allocate to reduce reallocations and fragmentation. */
> ret = i915_gem_stolen_insert_node_in_range(dev_priv, node, size <<= 1,
> - 4096, 0, end);
> + 4096, start, end);
> if (ret == 0)
> return compression_threshold;
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-14 19:55 ` Paulo Zanoni
2016-12-15 7:54 ` Daniel Vetter
@ 2016-12-15 8:17 ` Chris Wilson
2016-12-15 13:23 ` Paulo Zanoni
1 sibling, 1 reply; 21+ messages in thread
From: Chris Wilson @ 2016-12-15 8:17 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Wed, Dec 14, 2016 at 05:55:39PM -0200, Paulo Zanoni wrote:
> Don't even tell the mm allocator to handle the first page of stolen on
> the affected platforms. This means that we won't inherit the FB in
> case the BIOS decides to put it at the start of stolen. But the BIOS
> should not be putting it at the start of stolen since it's going to
> get corrupted. I suppose the bug here is that some pixels at the very
> top of the screen will be corrupted, so it's not exactly easy to
> notice.
>
> We have confirmation that the first page of stolen does actually get
> corrupted, so I really think we should do this in order to avoid any
> possible future headaches, even if that means losing BIOS framebuffer
> inheritance. Let's not use the HW in a way it's not supposed to be
> used.
>
> v2: don't even put the first page on the mm (Chris).
> v3: drm_mm_init() takes size instead of end as argument (Ville).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
> drivers/gpu/drm/i915/i915_gem_stolen.c | 34 +++++++++++++---------------------
> drivers/gpu/drm/i915/intel_fbc.c | 6 +++---
> 3 files changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 8965bbb..aefc968 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -316,6 +316,7 @@ struct i915_ggtt {
> struct io_mapping mappable; /* Mapping to our CPU mappable region */
>
/* Stolen memory is segmented in hardware with different portions
* offlimits to certain functions.
*
* The drm_mm is initialised to the total accessible range, as found
* from the PCI config. On Broadwell+, this is further restricted to
* avoid the first page! The upper end of stolen memory is reserved for
* hardware functions (such as intermediate encodings and compression)
* and similarly removed from the accessible range.
*/
> size_t stolen_size; /* Total size of stolen memory */
> + size_t stolen_usable_start; /* First page may be unusable. */
> size_t stolen_usable_size; /* Total size minus BIOS reserved */
> size_t stolen_reserved_base;
> size_t stolen_reserved_size;
Aside: Why are these hw values using size_t?
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b1c8897..56d7e0b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> if (!drm_mm_initialized(&dev_priv->mm.stolen))
> return -ENODEV;
>
> - /* See the comment at the drm_mm_init() call for more about this check.
> - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> - */
> - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> - start = 4096;
> -
> mutex_lock(&dev_priv->mm.stolen_lock);
> ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> alignment, start, end,
> @@ -74,10 +68,11 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> unsigned alignment)
> {
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> + u64 start = ggtt->stolen_usable_start;
> + u64 end = start + ggtt->stolen_usable_size;
>
> return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
> - alignment, 0,
> - ggtt->stolen_usable_size);
? The range is already restricted, you cannot get anything before
stolen_usable_start, or anything after usable_size. Just pass 0,
U64_MAX.
> + alignment, start, end);
> }
>
> void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
> @@ -489,20 +484,17 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> ggtt->stolen_size >> 10,
> (ggtt->stolen_size - reserved_total) >> 10);
>
> - ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
> + ggtt->stolen_usable_start = 0;
> + /* WaSkipStolenMemoryFirstPage:bdw+ */
> + if (INTEL_GEN(dev_priv) >= 8)
> + ggtt->stolen_usable_start = 4096;
>
> - /*
> - * Basic memrange allocator for stolen space.
> - *
> - * TODO: Notice that some platforms require us to not use the first page
> - * of the stolen memory but their BIOSes may still put the framebuffer
> - * on the first page. So we don't reserve this page for now because of
> - * that. Our current solution is to just prevent new nodes from being
> - * inserted on the first page - see the check we have at
> - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
> - * problem later.
> - */
> - drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
> + ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total -
> + ggtt->stolen_usable_start;
> +
> + /* Basic memrange allocator for stolen space. */
> + drm_mm_init(&dev_priv->mm.stolen, ggtt->stolen_usable_start,
> + ggtt->stolen_usable_size);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 62f215b..534fcb3c 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -528,7 +528,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> int compression_threshold = 1;
> int ret;
> - u64 end;
> + u64 start = ggtt->stolen_usable_start, end;
>
> /* The FBC hardware for BDW/SKL doesn't have access to the stolen
> * reserved range size, so it always assumes the maximum (8mb) is used.
> @@ -538,7 +538,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> end = ggtt->stolen_size - 8 * 1024 * 1024;
> else
> - end = ggtt->stolen_usable_size;
> + end = start + ggtt->stolen_usable_size;
end = U64_MAX;
No need to set start.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-15 8:17 ` Chris Wilson
@ 2016-12-15 13:23 ` Paulo Zanoni
2016-12-15 13:26 ` Ville Syrjälä
0 siblings, 1 reply; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-15 13:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Don't even tell the mm allocator to handle the first page of stolen on
the affected platforms. This means that we won't inherit the FB in
case the BIOS decides to put it at the start of stolen. But the BIOS
should not be putting it at the start of stolen since it's going to
get corrupted. I suppose the bug here is that some pixels at the very
top of the screen will be corrupted, so it's not exactly easy to
notice.
We have confirmation that the first page of stolen does actually get
corrupted, so I really think we should do this in order to avoid any
possible future headaches, even if that means losing BIOS framebuffer
inheritance. Let's not use the HW in a way it's not supposed to be
used.
Notice that now ggtt->stolen_usable_size won't reflect the ending
address of the stolen usable range anymore, so we have to fix the
places that rely on this. To simplify, we'll just use U64_MAX.
v2: don't even put the first page on the mm (Chris)
v3: drm_mm_init() takes size instead of end as argument (Ville)
v4: add a comment explaining the reserved ranges (Chris)
use 0 for start and U64_MAX for end when possible (Chris)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 10 +++++++++-
drivers/gpu/drm/i915/i915_gem_stolen.c | 36 ++++++++++++----------------------
drivers/gpu/drm/i915/intel_fbc.c | 2 +-
3 files changed, 22 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8965bbb..0055b85 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -315,8 +315,16 @@ struct i915_ggtt {
struct i915_address_space base;
struct io_mapping mappable; /* Mapping to our CPU mappable region */
+ /* Stolen memory is segmented in hardware with different portions
+ * offlimits to certain functions.
+ *
+ * The drm_mm is initialised to the total accessible range, as found
+ * from the PCI config. On Broadwell+, this is further restricted to
+ * avoid the first page! The upper end of stolen memory is reserved for
+ * hardware functions and similarly removed from the accessible range.
+ */
size_t stolen_size; /* Total size of stolen memory */
- size_t stolen_usable_size; /* Total size minus BIOS reserved */
+ size_t stolen_usable_size; /* Total size minus reserved ranges */
size_t stolen_reserved_base;
size_t stolen_reserved_size;
u64 mappable_end; /* End offset that we can CPU map */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index b1c8897..cbbfc64 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
if (!drm_mm_initialized(&dev_priv->mm.stolen))
return -ENODEV;
- /* See the comment at the drm_mm_init() call for more about this check.
- * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
- */
- if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
- start = 4096;
-
mutex_lock(&dev_priv->mm.stolen_lock);
ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
alignment, start, end,
@@ -73,11 +67,8 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
struct drm_mm_node *node, u64 size,
unsigned alignment)
{
- struct i915_ggtt *ggtt = &dev_priv->ggtt;
-
return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
- alignment, 0,
- ggtt->stolen_usable_size);
+ alignment, 0, U64_MAX);
}
void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
@@ -410,7 +401,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
{
struct i915_ggtt *ggtt = &dev_priv->ggtt;
unsigned long reserved_total, reserved_base = 0, reserved_size;
- unsigned long stolen_top;
+ unsigned long stolen_usable_start, stolen_top;
mutex_init(&dev_priv->mm.stolen_lock);
@@ -489,20 +480,17 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
ggtt->stolen_size >> 10,
(ggtt->stolen_size - reserved_total) >> 10);
- ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
+ stolen_usable_start = 0;
+ /* WaSkipStolenMemoryFirstPage:bdw+ */
+ if (INTEL_GEN(dev_priv) >= 8)
+ stolen_usable_start = 4096;
- /*
- * Basic memrange allocator for stolen space.
- *
- * TODO: Notice that some platforms require us to not use the first page
- * of the stolen memory but their BIOSes may still put the framebuffer
- * on the first page. So we don't reserve this page for now because of
- * that. Our current solution is to just prevent new nodes from being
- * inserted on the first page - see the check we have at
- * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
- * problem later.
- */
- drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
+ ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total -
+ stolen_usable_start;
+
+ /* Basic memrange allocator for stolen space. */
+ drm_mm_init(&dev_priv->mm.stolen, stolen_usable_start,
+ ggtt->stolen_usable_size);
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 62f215b..bb5f58c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -538,7 +538,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
end = ggtt->stolen_size - 8 * 1024 * 1024;
else
- end = ggtt->stolen_usable_size;
+ end = U64_MAX;
/* HACK: This code depends on what we will do in *_enable_fbc. If that
* code changes, this code needs to change as well.
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-15 13:23 ` Paulo Zanoni
@ 2016-12-15 13:26 ` Ville Syrjälä
2016-12-15 13:40 ` Chris Wilson
0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2016-12-15 13:26 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Thu, Dec 15, 2016 at 11:23:55AM -0200, Paulo Zanoni wrote:
> Don't even tell the mm allocator to handle the first page of stolen on
> the affected platforms. This means that we won't inherit the FB in
> case the BIOS decides to put it at the start of stolen. But the BIOS
> should not be putting it at the start of stolen since it's going to
> get corrupted. I suppose the bug here is that some pixels at the very
> top of the screen will be corrupted, so it's not exactly easy to
> notice.
>
> We have confirmation that the first page of stolen does actually get
> corrupted, so I really think we should do this in order to avoid any
> possible future headaches, even if that means losing BIOS framebuffer
> inheritance. Let's not use the HW in a way it's not supposed to be
> used.
>
> Notice that now ggtt->stolen_usable_size won't reflect the ending
> address of the stolen usable range anymore, so we have to fix the
> places that rely on this. To simplify, we'll just use U64_MAX.
>
> v2: don't even put the first page on the mm (Chris)
> v3: drm_mm_init() takes size instead of end as argument (Ville)
> v4: add a comment explaining the reserved ranges (Chris)
> use 0 for start and U64_MAX for end when possible (Chris)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 10 +++++++++-
> drivers/gpu/drm/i915/i915_gem_stolen.c | 36 ++++++++++++----------------------
> drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> 3 files changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 8965bbb..0055b85 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -315,8 +315,16 @@ struct i915_ggtt {
> struct i915_address_space base;
> struct io_mapping mappable; /* Mapping to our CPU mappable region */
>
> + /* Stolen memory is segmented in hardware with different portions
> + * offlimits to certain functions.
> + *
> + * The drm_mm is initialised to the total accessible range, as found
> + * from the PCI config. On Broadwell+, this is further restricted to
> + * avoid the first page! The upper end of stolen memory is reserved for
> + * hardware functions and similarly removed from the accessible range.
> + */
> size_t stolen_size; /* Total size of stolen memory */
> - size_t stolen_usable_size; /* Total size minus BIOS reserved */
> + size_t stolen_usable_size; /* Total size minus reserved ranges */
> size_t stolen_reserved_base;
> size_t stolen_reserved_size;
> u64 mappable_end; /* End offset that we can CPU map */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b1c8897..cbbfc64 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> if (!drm_mm_initialized(&dev_priv->mm.stolen))
> return -ENODEV;
>
> - /* See the comment at the drm_mm_init() call for more about this check.
> - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> - */
> - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> - start = 4096;
> -
> mutex_lock(&dev_priv->mm.stolen_lock);
> ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> alignment, start, end,
> @@ -73,11 +67,8 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> struct drm_mm_node *node, u64 size,
> unsigned alignment)
> {
> - struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -
> return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
> - alignment, 0,
> - ggtt->stolen_usable_size);
> + alignment, 0, U64_MAX);
Just i915_gem_stolen_insert_node() then?
> }
>
> void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
> @@ -410,7 +401,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> {
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> unsigned long reserved_total, reserved_base = 0, reserved_size;
> - unsigned long stolen_top;
> + unsigned long stolen_usable_start, stolen_top;
>
> mutex_init(&dev_priv->mm.stolen_lock);
>
> @@ -489,20 +480,17 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> ggtt->stolen_size >> 10,
> (ggtt->stolen_size - reserved_total) >> 10);
>
> - ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
> + stolen_usable_start = 0;
> + /* WaSkipStolenMemoryFirstPage:bdw+ */
> + if (INTEL_GEN(dev_priv) >= 8)
> + stolen_usable_start = 4096;
>
> - /*
> - * Basic memrange allocator for stolen space.
> - *
> - * TODO: Notice that some platforms require us to not use the first page
> - * of the stolen memory but their BIOSes may still put the framebuffer
> - * on the first page. So we don't reserve this page for now because of
> - * that. Our current solution is to just prevent new nodes from being
> - * inserted on the first page - see the check we have at
> - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
> - * problem later.
> - */
> - drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
> + ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total -
> + stolen_usable_start;
> +
> + /* Basic memrange allocator for stolen space. */
> + drm_mm_init(&dev_priv->mm.stolen, stolen_usable_start,
> + ggtt->stolen_usable_size);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 62f215b..bb5f58c 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -538,7 +538,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> end = ggtt->stolen_size - 8 * 1024 * 1024;
> else
> - end = ggtt->stolen_usable_size;
> + end = U64_MAX;
>
> /* HACK: This code depends on what we will do in *_enable_fbc. If that
> * code changes, this code needs to change as well.
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-15 13:26 ` Ville Syrjälä
@ 2016-12-15 13:40 ` Chris Wilson
2016-12-15 13:51 ` Ville Syrjälä
0 siblings, 1 reply; 21+ messages in thread
From: Chris Wilson @ 2016-12-15 13:40 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Paulo Zanoni
On Thu, Dec 15, 2016 at 03:26:59PM +0200, Ville Syrjälä wrote:
> On Thu, Dec 15, 2016 at 11:23:55AM -0200, Paulo Zanoni wrote:
> > Don't even tell the mm allocator to handle the first page of stolen on
> > the affected platforms. This means that we won't inherit the FB in
> > case the BIOS decides to put it at the start of stolen. But the BIOS
> > should not be putting it at the start of stolen since it's going to
> > get corrupted. I suppose the bug here is that some pixels at the very
> > top of the screen will be corrupted, so it's not exactly easy to
> > notice.
> >
> > We have confirmation that the first page of stolen does actually get
> > corrupted, so I really think we should do this in order to avoid any
> > possible future headaches, even if that means losing BIOS framebuffer
> > inheritance. Let's not use the HW in a way it's not supposed to be
> > used.
> >
> > Notice that now ggtt->stolen_usable_size won't reflect the ending
> > address of the stolen usable range anymore, so we have to fix the
> > places that rely on this. To simplify, we'll just use U64_MAX.
> >
> > v2: don't even put the first page on the mm (Chris)
> > v3: drm_mm_init() takes size instead of end as argument (Ville)
> > v4: add a comment explaining the reserved ranges (Chris)
> > use 0 for start and U64_MAX for end when possible (Chris)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.h | 10 +++++++++-
> > drivers/gpu/drm/i915/i915_gem_stolen.c | 36 ++++++++++++----------------------
> > drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> > 3 files changed, 22 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > index 8965bbb..0055b85 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > @@ -315,8 +315,16 @@ struct i915_ggtt {
> > struct i915_address_space base;
> > struct io_mapping mappable; /* Mapping to our CPU mappable region */
> >
> > + /* Stolen memory is segmented in hardware with different portions
> > + * offlimits to certain functions.
> > + *
> > + * The drm_mm is initialised to the total accessible range, as found
> > + * from the PCI config. On Broadwell+, this is further restricted to
> > + * avoid the first page! The upper end of stolen memory is reserved for
> > + * hardware functions and similarly removed from the accessible range.
> > + */
> > size_t stolen_size; /* Total size of stolen memory */
> > - size_t stolen_usable_size; /* Total size minus BIOS reserved */
> > + size_t stolen_usable_size; /* Total size minus reserved ranges */
> > size_t stolen_reserved_base;
> > size_t stolen_reserved_size;
> > u64 mappable_end; /* End offset that we can CPU map */
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index b1c8897..cbbfc64 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> > if (!drm_mm_initialized(&dev_priv->mm.stolen))
> > return -ENODEV;
> >
> > - /* See the comment at the drm_mm_init() call for more about this check.
> > - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> > - */
> > - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> > - start = 4096;
> > -
> > mutex_lock(&dev_priv->mm.stolen_lock);
> > ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> > alignment, start, end,
> > @@ -73,11 +67,8 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> > struct drm_mm_node *node, u64 size,
> > unsigned alignment)
> > {
> > - struct i915_ggtt *ggtt = &dev_priv->ggtt;
> > -
> > return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
> > - alignment, 0,
> > - ggtt->stolen_usable_size);
> > + alignment, 0, U64_MAX);
>
> Just i915_gem_stolen_insert_node() then?
This is stolen_insert_node() :)
Could be turned into an inline later.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
2016-12-15 13:40 ` Chris Wilson
@ 2016-12-15 13:51 ` Ville Syrjälä
0 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjälä @ 2016-12-15 13:51 UTC (permalink / raw)
To: Chris Wilson, Paulo Zanoni, intel-gfx
On Thu, Dec 15, 2016 at 01:40:15PM +0000, Chris Wilson wrote:
> On Thu, Dec 15, 2016 at 03:26:59PM +0200, Ville Syrjälä wrote:
> > On Thu, Dec 15, 2016 at 11:23:55AM -0200, Paulo Zanoni wrote:
> > > Don't even tell the mm allocator to handle the first page of stolen on
> > > the affected platforms. This means that we won't inherit the FB in
> > > case the BIOS decides to put it at the start of stolen. But the BIOS
> > > should not be putting it at the start of stolen since it's going to
> > > get corrupted. I suppose the bug here is that some pixels at the very
> > > top of the screen will be corrupted, so it's not exactly easy to
> > > notice.
> > >
> > > We have confirmation that the first page of stolen does actually get
> > > corrupted, so I really think we should do this in order to avoid any
> > > possible future headaches, even if that means losing BIOS framebuffer
> > > inheritance. Let's not use the HW in a way it's not supposed to be
> > > used.
> > >
> > > Notice that now ggtt->stolen_usable_size won't reflect the ending
> > > address of the stolen usable range anymore, so we have to fix the
> > > places that rely on this. To simplify, we'll just use U64_MAX.
> > >
> > > v2: don't even put the first page on the mm (Chris)
> > > v3: drm_mm_init() takes size instead of end as argument (Ville)
> > > v4: add a comment explaining the reserved ranges (Chris)
> > > use 0 for start and U64_MAX for end when possible (Chris)
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_gem_gtt.h | 10 +++++++++-
> > > drivers/gpu/drm/i915/i915_gem_stolen.c | 36 ++++++++++++----------------------
> > > drivers/gpu/drm/i915/intel_fbc.c | 2 +-
> > > 3 files changed, 22 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > > index 8965bbb..0055b85 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > > @@ -315,8 +315,16 @@ struct i915_ggtt {
> > > struct i915_address_space base;
> > > struct io_mapping mappable; /* Mapping to our CPU mappable region */
> > >
> > > + /* Stolen memory is segmented in hardware with different portions
> > > + * offlimits to certain functions.
> > > + *
> > > + * The drm_mm is initialised to the total accessible range, as found
> > > + * from the PCI config. On Broadwell+, this is further restricted to
> > > + * avoid the first page! The upper end of stolen memory is reserved for
> > > + * hardware functions and similarly removed from the accessible range.
> > > + */
> > > size_t stolen_size; /* Total size of stolen memory */
> > > - size_t stolen_usable_size; /* Total size minus BIOS reserved */
> > > + size_t stolen_usable_size; /* Total size minus reserved ranges */
> > > size_t stolen_reserved_base;
> > > size_t stolen_reserved_size;
> > > u64 mappable_end; /* End offset that we can CPU map */
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > > index b1c8897..cbbfc64 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > > @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> > > if (!drm_mm_initialized(&dev_priv->mm.stolen))
> > > return -ENODEV;
> > >
> > > - /* See the comment at the drm_mm_init() call for more about this check.
> > > - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> > > - */
> > > - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> > > - start = 4096;
> > > -
> > > mutex_lock(&dev_priv->mm.stolen_lock);
> > > ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> > > alignment, start, end,
> > > @@ -73,11 +67,8 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> > > struct drm_mm_node *node, u64 size,
> > > unsigned alignment)
> > > {
> > > - struct i915_ggtt *ggtt = &dev_priv->ggtt;
> > > -
> > > return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
> > > - alignment, 0,
> > > - ggtt->stolen_usable_size);
> > > + alignment, 0, U64_MAX);
> >
> > Just i915_gem_stolen_insert_node() then?
>
> This is stolen_insert_node() :)
/me goes get some coffee...
> Could be turned into an inline later.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 (rev3)
2016-12-14 14:55 ` Paulo Zanoni
` (4 preceding siblings ...)
(?)
@ 2016-12-15 14:45 ` Patchwork
-1 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2016-12-15 14:45 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 (rev3)
URL : https://patchwork.freedesktop.org/series/16800/
State : warning
== Summary ==
Series 16800v3 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16800/revisions/3/mbox/
Test drv_module_reload:
Subgroup basic-reload-inject:
pass -> DMESG-WARN (fi-ilk-650)
fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39
fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-ilk-650 total:247 pass:194 dwarn:1 dfail:0 fail:0 skip:52
fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20
fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20
fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
64353e762935a7ad82867be0e4e80ff2f7bc97e4 drm-tip: 2016y-12m-15d-13h-31m-53s UTC integration manifest
081ddab drm/i915: fully apply WaSkipStolenMemoryFirstPage
f4ae312 drm/i915: skip the first 4k of stolen memory on everything >= gen8
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3294/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
2016-12-14 14:55 ` Paulo Zanoni
` (5 preceding siblings ...)
(?)
@ 2016-12-20 12:57 ` Paulo Zanoni
-1 siblings, 0 replies; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-20 12:57 UTC (permalink / raw)
To: intel-gfx
Em Qua, 2016-12-14 às 12:55 -0200, Paulo Zanoni escreveu:
> BSpec got updated and this workaround is now listed as standard
> required programming for all subsequent projects. This is confirmed
> to
> fix Skylake screen flickering issues (probably caused by the fact
> that
> we initialized a ring in the first page of stolen, but I didn't 100%
> confirm this theory).
>
> v2: this is the patch that fixes the screen flickering, document it.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: stable@vger.kernel.org
> Tested-by: Dominik Klementowski <dominik232@gmail.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Daniel gave me the missing R-B for this patch on IRC, so I pushed both
patches.
Thanks everybody for the reviews.
> ---
> drivers/gpu/drm/i915/i915_gem_stolen.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index efc0e74..b1c8897 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -55,10 +55,9 @@ int i915_gem_stolen_insert_node_in_range(struct
> drm_i915_private *dev_priv,
> return -ENODEV;
>
> /* See the comment at the drm_mm_init() call for more about
> this check.
> - * WaSkipStolenMemoryFirstPage:bdw,chv,kbl (incomplete)
> + * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> */
> - if (start < 4096 && (IS_GEN8(dev_priv) ||
> - IS_KBL_REVID(dev_priv, 0,
> KBL_REVID_A0)))
> + if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> start = 4096;
>
> mutex_lock(&dev_priv->mm.stolen_lock);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
2016-12-13 20:01 ` Paulo Zanoni
@ 2016-12-14 12:52 ` Paulo Zanoni
0 siblings, 0 replies; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-14 12:52 UTC (permalink / raw)
To: intel-gfx; +Cc: dhinakaran.pandiyan
Em Ter, 2016-12-13 às 18:01 -0200, Paulo Zanoni escreveu:
> Em Ter, 2016-12-13 às 17:57 -0200, Paulo Zanoni escreveu:
> >
> > BSpec got updated and this workaround is now listed as standard
> > required programming for all subsequent projects.
> >
Ok, so I got confirmation from the bug reporter that this patch is the
patch that actually fixes his screen flickering bugs. I suppose we're
initializing some ring on the first page and that's what really messes
up things (last time I checked, we did initialize a ring there on my
machine).
The corruption caused by random bugs flipping in the very first pixel
line of the frontbuffer is probably not very noticeable, so we probably
won't get bug reports for patch 2 specifically.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Tested-by: Dominik Klementowski <dominik232@gmail.com>
>
> Cc: stable@vger.kernel.org
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_stolen.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index efc0e74..b1c8897 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -55,10 +55,9 @@ int i915_gem_stolen_insert_node_in_range(struct
> > drm_i915_private *dev_priv,
> > return -ENODEV;
> >
> > /* See the comment at the drm_mm_init() call for more
> > about
> > this check.
> > - * WaSkipStolenMemoryFirstPage:bdw,chv,kbl (incomplete)
> > + * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> > */
> > - if (start < 4096 && (IS_GEN8(dev_priv) ||
> > - IS_KBL_REVID(dev_priv, 0,
> > KBL_REVID_A0)))
> > + if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> > start = 4096;
> >
> > mutex_lock(&dev_priv->mm.stolen_lock);
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
2016-12-13 19:57 Paulo Zanoni
@ 2016-12-13 20:01 ` Paulo Zanoni
2016-12-14 12:52 ` Paulo Zanoni
0 siblings, 1 reply; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-13 20:01 UTC (permalink / raw)
To: intel-gfx; +Cc: dhinakaran.pandiyan
Em Ter, 2016-12-13 às 17:57 -0200, Paulo Zanoni escreveu:
> BSpec got updated and this workaround is now listed as standard
> required programming for all subsequent projects.
>
Cc: stable@vger.kernel.org
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_stolen.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index efc0e74..b1c8897 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -55,10 +55,9 @@ int i915_gem_stolen_insert_node_in_range(struct
> drm_i915_private *dev_priv,
> return -ENODEV;
>
> /* See the comment at the drm_mm_init() call for more about
> this check.
> - * WaSkipStolenMemoryFirstPage:bdw,chv,kbl (incomplete)
> + * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> */
> - if (start < 4096 && (IS_GEN8(dev_priv) ||
> - IS_KBL_REVID(dev_priv, 0,
> KBL_REVID_A0)))
> + if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> start = 4096;
>
> mutex_lock(&dev_priv->mm.stolen_lock);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8
@ 2016-12-13 19:57 Paulo Zanoni
2016-12-13 20:01 ` Paulo Zanoni
0 siblings, 1 reply; 21+ messages in thread
From: Paulo Zanoni @ 2016-12-13 19:57 UTC (permalink / raw)
To: intel-gfx; +Cc: dhinakaran.pandiyan, Paulo Zanoni
BSpec got updated and this workaround is now listed as standard
required programming for all subsequent projects.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index efc0e74..b1c8897 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -55,10 +55,9 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
return -ENODEV;
/* See the comment at the drm_mm_init() call for more about this check.
- * WaSkipStolenMemoryFirstPage:bdw,chv,kbl (incomplete)
+ * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
*/
- if (start < 4096 && (IS_GEN8(dev_priv) ||
- IS_KBL_REVID(dev_priv, 0, KBL_REVID_A0)))
+ if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
start = 4096;
mutex_lock(&dev_priv->mm.stolen_lock);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-12-20 12:57 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14 14:55 [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 Paulo Zanoni
2016-12-14 14:55 ` Paulo Zanoni
2016-12-14 14:55 ` [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage Paulo Zanoni
2016-12-14 15:43 ` Chris Wilson
2016-12-14 16:39 ` Ville Syrjälä
2016-12-14 16:50 ` Chris Wilson
2016-12-14 19:55 ` Paulo Zanoni
2016-12-15 7:54 ` Daniel Vetter
2016-12-15 8:17 ` Chris Wilson
2016-12-15 13:23 ` Paulo Zanoni
2016-12-15 13:26 ` Ville Syrjälä
2016-12-15 13:40 ` Chris Wilson
2016-12-15 13:51 ` Ville Syrjälä
2016-12-14 15:43 ` [Intel-gfx] [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 Chris Wilson
2016-12-14 15:53 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2016-12-14 20:22 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 (rev2) Patchwork
2016-12-15 14:45 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 (rev3) Patchwork
2016-12-20 12:57 ` [PATCH 1/2] drm/i915: skip the first 4k of stolen memory on everything >= gen8 Paulo Zanoni
-- strict thread matches above, loose matches on Subject: below --
2016-12-13 19:57 Paulo Zanoni
2016-12-13 20:01 ` Paulo Zanoni
2016-12-14 12:52 ` Paulo Zanoni
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.