Re: [PATCH 1/7] drm/i915: Fix the offset issue for the stolen GEM objects

Chris Wilson
2014-01-09T11:09:00Z

On Thu, Jan 09, 2014 at 10:58:35AM +0100, Daniel Vetter wrote:
> On Thu, Jan 09, 2014 at 09:45:21AM +0000, Chris Wilson wrote:
> > On Thu, Jan 09, 2014 at 11:00:17AM +0530, akash.goel@intel.com wrote:
> > > From: Akash Goel <akash.goel@intel.com>
> > >
> > > The 'offset' field of the 'scatterlist' structure was wrongly
> > > programmed with the offset value from the base of stolen area,
> > > whereas this field indicates the offset from where the interested
> > > data starts within the PAGE pointed to by the 'page-link' field.
> > > As a result when a new GEM object allocated from the stolen
> > > area is mapped to GTT, it could lead to an overwrite of GTT entries
> > > as the page count calculation will go wrong, refer the function
> > > 'sg_page_count'.
> >
> > This statement is incorrect since my use of sg here predates
> > sg_page_iter.
> >
> > The stolen sg has no page_link, the meaning of offset/length here are
> > relative to the base of the stolen area.
> >
> > However, if you wish to rephrase the above...
>
> Actually we add offset both to sg->offset and adjust the dma_bus_addr
> since this has been introduced in
>
> commit 0104fdbb84d7adb0e377ed05bf75eba97b007544
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Thu Nov 15 11:32:26 2012 +0000
>
> drm/i915: Introduce i915_gem_object_create_stolen()
>
> But only Imre's conversion to the sg_page_iter started to pay any
> attention to sg->offset in
>
> commit 6e995e231a90ce7c5ce2a9eae23c8e22f4388db1
> Author: Imre Deak <imre.deak@intel.com>
> Date: Mon Feb 18 19:28:04 2013 +0200
>
> drm/i915: use for_each_sg_page for setting up the gtt ptes
>
> So with a bit of commit message rewording and these references and cc:
> stable this looks good.

And

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69104
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>

Maybe a reviewed-by when I see a good comment/changelog.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre