Re: [PATCH] drm/i915: fix stolen bios_reserved checks

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Paulo Zanoni <paulo.r.zanoni@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: fix stolen bios_reserved checks
Date: Tue, 11 Aug 2015 11:31:46 +0200	[thread overview]
Message-ID: <20150811093146.GL17734@phenom.ffwll.local> (raw)
In-Reply-To: <20150810180644.GA9959@nuc-i3427.alporthouse.com>

On Mon, Aug 10, 2015 at 07:06:44PM +0100, Chris Wilson wrote:
> On Mon, Aug 10, 2015 at 02:57:32PM -0300, Paulo Zanoni wrote:
> > I started digging this when I noticed that the BDW code was just
> > reserving 1mb by coincidence since it was reading reserved fields.
> > Then I noticed we didn't have any values set for SNB and earlier, and
> > that the HSW sizes were wrong. After that, I noticed that the reserved
> > area has a specific start, and may not exactly end where the stolen
> > memory ends. I also noticed the base pointer can be zero. So I decided
> > to just write a single patch fixing everything instead of 20 patches
> > that would be much harder to review.
> > 
> > This patch may solve random stolen memory corruption/problems on
> > almost all platforms. Notice that since this is always dealing with
> > the top of the stolen memory, the problems are not so easy to
> > reproduce - especially since FBC is still disabled by default.
> > 
> > One of the major differences of this patch is that we now look at both
> > the size and base address. By only looking at the size we were
> > assuming that the reserved area was always at the very top of
> > stolen, which is not always true.
> > 
> > After we merge the patch series that allows user space to allocate
> > stolen memory we'll be able to write IGT tests that maybe catch the
> > bugs fixed by this patch.
> > 
> > v2:
> >   - s/BIOS reserved/stolen reserved/g (Chris)
> >   - Don't DRM_ERROR if we can't do anything about it (Chris)
> >   - Improve debug messages (Chris).
> >   - Use the gen7 version instead of gen6 on HSW. Tom found some
> >     documentation problems, so I think with gen7 we're on the safer
> >     side (Tom).
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Looks ok to me, I'd push for DRM_INFO() for the amount of memory
> available (since I think that is interesting as a user).
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx