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

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Dave Gordon <david.s.gordon@intel.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] Always mark GEM objects as dirty when written by the CPU
Date: Fri, 11 Dec 2015 17:48:53 +0100	[thread overview]
Message-ID: <20151211164853.GG20822@phenom.ffwll.local> (raw)
In-Reply-To: <20151211122940.GH11596@nuc-i3427.alporthouse.com>

On Fri, Dec 11, 2015 at 12:29:40PM +0000, Chris Wilson wrote:
> On Fri, Dec 11, 2015 at 12:19:09PM +0000, Dave Gordon wrote:
> > On 10/12/15 08:58, Daniel Vetter wrote:
> > >On Mon, Dec 07, 2015 at 12:51:49PM +0000, Dave Gordon wrote:
> > >>I think I missed i915_gem_phys_pwrite().
> > >>
> > >>i915_gem_gtt_pwrite_fast() marks the object dirty for most cases (vit
> > >>set_to_gtt_domain(), but isn't called for all cases (or can return before
> > >>the set_domain). Then we try i915_gem_shmem_pwrite() for non-phys
> > >>objects (no check for stolen!) and that already marks the object dirty
> > >>[aside: we might be able to change that to page-by-page?], but
> > >>i915_gem_phys_pwrite() doesn't mark the object dirty, so we might lose
> > >>updates there?
> > >>
> > >>Or maybe we should move the marking up into i915_gem_pwrite_ioctl() instead.
> > >>The target object is surely going to be dirtied, whatever type it is.
> > >
> > >phys objects are special, and when binding we create allocate new
> > >(contiguous) storage. In put_pages_phys that gets copied back and pages
> > >marked as dirty. While a phys object is pinned it's a kernel bug to look
> > >at the shmem pages and a userspace bug to touch the cpu mmap (since that
> > >data will simply be overwritten whenever the kernel feels like).
> > >
> > >phys objects are only used for cursors on old crap though, so ok if we
> > >don't streamline this fairly quirky old ABI.
> > >-Daniel
> > 
> > So is pread broken already for 'phys' ?
> 
> Yes. A completely unused corner of the API.

I think it would be useful to extract all the phys object stuff into
i915_gem_phys_obj.c, add minimal kerneldoc for the functions, and then an
overview section which explains in detail how fucked up this little bit of
ABI history lore is. I can do the overview section, but the
extraction/basic kerneldoc will probably take a bit longer to get around
to.
-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