[Intel-gfx] [PATCH] drm/i915: Skip Stolen Memory first page.

[Daniel Vetter]

On Tue, Feb 03, 2015 at 07:40:21PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 03, 2015 at 04:11:05PM +0000, Siluvery, Arun wrote:
> > On 01/08/2014 17:34, Jesse Barnes wrote:
> > > On Thu, 31 Jul 2014 12:08:20 -0700
> > > Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> > >
> > >> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle
> > >>
> > >> v2: Improve variable names and fix allocated size.
> > >>
> > >> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
> > >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > >> ---
> > >>   drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++-----
> > >>   1 file changed, 10 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >> index 21c025a..82035b0 100644
> > >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >> @@ -289,7 +289,8 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
> > >>   int i915_gem_init_stolen(struct drm_device *dev)
> > >>   {
> > >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> > >> -	int bios_reserved = 0;
> > >> +	int start_rsvd = 0;
> > >> +	int end_rsvd = 0;
> > >>
> > >>   #ifdef CONFIG_INTEL_IOMMU
> > >>   	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> > >> @@ -308,15 +309,19 @@ int i915_gem_init_stolen(struct drm_device *dev)
> > >>   	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
> > >>   		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
> > >>
> > >> +	/* WaSkipStolenMemoryFirstPage */
> > >> +	if (INTEL_INFO(dev)->gen >= 8)
> > >> +		start_rsvd = 4096;
> > >> +
> > >>   	if (IS_VALLEYVIEW(dev))
> > >> -		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
> > >> +		end_rsvd = 1024*1024; /* top 1M on VLV/BYT */
> > >>
> > >> -	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
> > >> +	if (WARN_ON((start_rsvd + end_rsvd) > dev_priv->gtt.stolen_size))
> > >>   		return 0;
> > >>
> > >>   	/* Basic memrange allocator for stolen space */
> > >> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> > >> -		    bios_reserved);
> > >> +	drm_mm_init(&dev_priv->mm.stolen, start_rsvd,
> > >> +		    dev_priv->gtt.stolen_size - start_rsvd - end_rsvd);
> > >>
> > >>   	return 0;
> > >>   }
> > >
> > > Beyond the fastboot stuff Ville has already mentioned, the early
> > > allocation of the existing fb from stolen will prevent us from
> > > clobbering the currently displayed buffer with the contents of the
> > > ringbuffers and whatever else we allocate out of stolen at early boot.
> > >
> > > We might be able to avoid that by doing stolen allocations top down, or
> > > by reserving the displayed fb even if we can't allocate an obj for it,
> > > only freeing it after our first mode set.
> > >
> > > Can you file a bug or JIRA for that to make sure we don't lose track of
> > > the fastboot & boot corruption issues after this fix lands?
> > 
> > Reviving an old thread,
> > Any particular reason why this patch is not merged to nightly?
> > Is it known to cause any other regressions?
> 
> It breaks the BIOS fb takeover like I said several times.
> 
> If no one is willing to fix it properly I was thinking we might just
> try to do the BIOS fb takeover, and if it succeeded we do nothing else,
> otherwise we allocate an unused 1 page object to keep the rings/fbc
> buffer/etc. away from the first page.
> 
> The first page corruption supposedly happens only when the CS is doing
> stuff, so if the CS corrupts the fbcon a bit it's no big deal. And
> since we don't accelerate the fbconf hooks the corruption shouldn't
> really happen under normal conditions anyway. You could see it while
> running some igts or something, but that's not a huge problem.

Without fbcon we still reconstruct the fb, but will free it on the first
modeset when userspace provides a real framebuffer. So this approach
doesn't work.

Instead we need to teach the stolen allocation functions to respect the
limit. We can't just restrict the entire drm_mm like in this patch since
the preallocated stolen obj logic must keep on working. Otherwise we break
fastboot.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch