[Intel-gfx] [PATCH 01/15] drm/i915: Add i915_gem_object_write() to i915_gem.c

[Chris Wilson]

On Thu, Jun 18, 2015 at 07:07:46PM +0100, Dave Gordon wrote:
> On 18/06/15 13:10, Chris Wilson wrote:
> > On Thu, Jun 18, 2015 at 12:49:55PM +0100, Dave Gordon wrote:
> >> On 17/06/15 13:02, Daniel Vetter wrote:
> >>> Domain handling is required for all gem objects, and the resulting bugs if
> >>> you don't for one-off objects are absolutely no fun to track down.
> >>
> >> Is it not the case that the new object returned by
> >> i915_gem_alloc_object() is
> >> (a) of a type that can be mapped into the GTT, and
> >> (b) initially in the CPU domain for both reading and writing?
> >>
> >> So AFAICS the allocate-and-fill function I'm describing (to appear in
> >> next patch series respin) doesn't need any further domain handling.
> > 
> > A i915_gem_object_create_from_data() is a reasonable addition, and I
> > suspect it will make the code a bit more succinct.
> 
> I shall adopt this name for it :)
> 
> > Whilst your statement is true today, calling set_domain is then a no-op,
> > and helps document how you use the object and so reduces the likelihood
> > of us introducing bugs in the future.
> > -Chris
> 
> So here's the new function ... where should the set-to-cpu-domain go?
> After the pin_pages and before the sg_copy_from_buffer?

Either, since the domain will not change whilst you have the lock,
but if you do it before get_pages() you will have a slightly easier
error path.

Part of the reason why I want a function like this is so that I can
replace it with a stolen object and so need to write the data through a
temporary GGTT mapping. Speak now if you need more flags to the function
to prevent certain classes of objects being created.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre