[Intel-gfx] [PATCH 1/2 v2] drm/i915: mark GEM object pages dirty when mapped & written by the CPU

[Chris Wilson]

On Thu, Dec 10, 2015 at 05:24:42PM +0000, Dave Gordon wrote:
> On 10/12/15 13:29, Chris Wilson wrote:
> >On Wed, Dec 09, 2015 at 03:52:51PM +0000, Dave Gordon wrote:
> >>In various places, a single page of a (regular) GEM object is mapped into
> >>CPU address space and updated. In each such case, either the page or the
> >>the object should be marked dirty, to ensure that the modifications are
> >>not discarded if the object is evicted under memory pressure.
> >>
> >>The typical sequence is:
> >>	va = kmap_atomic(i915_gem_object_get_page(obj, pageno));
> >>	*(va+offset) = ...
> >>	kunmap_atomic(va);
> >>
> >>Here we introduce i915_gem_object_get_dirty_page(), which performs the
> >>same operation as i915_gem_object_get_page() but with the side-effect
> >>of marking the returned page dirty in the pagecache.  This will ensure
> >>that if the object is subsequently evicted (due to memory pressure),
> >>the changes are written to backing store rather than discarded.
> >>
> >>Note that it works only for regular (shmfs-backed) GEM objects, but (at
> >>least for now) those are the only ones that are updated in this way --
> >>the objects in question are contexts and batchbuffers, which are always
> >>shmfs-backed.
> >>
> >>A separate patch deals with the case where whole objects are (or may
> >>be) dirtied.
> >>
> >>Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
> >>Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >
> >I like this. There are places were we do both obj->dirty and
> >set_page_dirty(), but this so much more clearly shows what is going on.
> >All of these locations should be infrequent (or at least have patches to
> >make them so), so moving the call out-of-line will also be a benefit.
> 
> I think there was only one place that both called set_page_dirty()
> AND set obj->dirty, which was in populate_lr_context(). You'll see
> that I've eliminated both in favour of a call to get_dirty_page() :)

It was things like all GPU objects have obj->dirty set, so really the
importance of using get_dirty_page() in the relocations and context
pinning is for documentation. Which is a very good reason, nevertheless.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre