Re: [PATCH] Always mark GEM objects as dirty when written by the CPU

From: Daniel Vetter <daniel@ffwll.ch>
To: Dave Gordon <david.s.gordon@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] Always mark GEM objects as dirty when written by the CPU
Date: Fri, 4 Dec 2015 10:57:30 +0100	[thread overview]
Message-ID: <20151204095730.GK10243@phenom.ffwll.local> (raw)
In-Reply-To: <565D9EC3.5020109@intel.com>

On Tue, Dec 01, 2015 at 01:21:07PM +0000, Dave Gordon wrote:
> On 01/12/15 13:04, Chris Wilson wrote:
> >On Tue, Dec 01, 2015 at 12:42:02PM +0000, Dave Gordon wrote:
> >>In various places, one or more pages of a GEM object are mapped into CPU
> >>address space and updated. In each such case, the object should be
> >>marked dirty, to ensure that the modifications are not discarded if the
> >>object is evicted under memory pressure.
> >>
> >>This is similar to commit
> >>	commit 51bc140431e233284660b1d22c47dec9ecdb521e
> >>	Author: Chris Wilson <chris@chris-wilson.co.uk>
> >>	Date:   Mon Aug 31 15:10:39 2015 +0100
> >>	drm/i915: Always mark the object as dirty when used by the GPU
> >>
> >>in which Chris ensured that updates by the GPU were not lost due to
> >>eviction, but this patch applies instead to the multiple places where
> >>object content is updated by the host CPU.
> >
> >Apart from that commit was to mask userspace bugs, here we are under
> >control of when the pages are marked and have chosen a different
> >per-page interface for CPU writes as opposed to per-object.
> >-Chris
> >
> 
> The pattern
> 	get_pages();
> 	kmap(get_page())
> 	write
> 	kunmap()
> occurs often enough that it might be worth providing a common function to do
> that and mark only the specific page dirty (other cases touch the whole
> object, so for those we can just set the obj->dirty flag and let put_pages()
> take care of propagating that to all the individual pages).
> 
> But can we be sure that all the functions touched by this patch will operate
> only on regular (default) GEM objects (i.e. not phys, stolen, etc) 'cos some
> of those don't support per-page tracking. What about objects with no backing
> store -- can/should we mark those as dirty (which would prevent eviction)?

I thought our special objects do clear obj->dirty on put_pages? Can you
please elaborate on your concern?

While we discuss all this: A patch at the end to document dirty (maybe
even as a first stab at kerneldoc for i915_drm_gem_buffer_object) would be
awesome.
-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