Re: [PATCH 2/2] drm/i915: Mark obj->mapping as dirtying the backing storage

[Dave Gordon <david.s.gordon@intel.com>]

On 12/04/16 16:18, Chris Wilson wrote:
> On Tue, Apr 12, 2016 at 02:32:31PM +0100, Chris Wilson wrote:
>> When reviewing some of Tvrtko's usage for i915_gem_object_pin_map(), he
>> suggested replacing some use of kmap(i915_gem_object_get_dirty_page())
>> with a plain i915_gem_object_pin_map(). This raised the question of who
>> should mark the page as dirty (or the mapping case, the object).
>> We can write simpler, safer code if we mark the entire object as dirty
>> upon obtaining the obj->mapping. (The counter-argument is that the
>> caller should be marking the object as dirty itself, or we should be
>> passing in a direction parameter.)

Hmm, didn't I say that back in December?

> What I particularly dislike about the current obj->dirty is that it is
> strictly only valid inside a pin_pages/unpin_pages section. That isn't
> clear from the API atm.
> -Chris

But no-one ever reads it until after the pincount goes to zero! I don't 
even think reading it is part of the public API at all; so we probably 
should replace 'obj->dirty = true' with i915_gem_object_mark_dirty(obj) 
everywhere.

As for existing callers of pin-and-map:
. populate-context() marks the object dirty
. context pinning already marks the whole object dirty
. context reset already marks the whole object dirty
. the LRC HWSP code doesn't mark it (but it's part of the context)
but
. pin-and-map ringbuffer doesn't mark anything - we could add it
. the dmabuf code doesn't mark anything - should it?

All the above places that set obj->dirty are either touching multiple 
pages themselves or have mapped the whole object for access by the GPU, 
so in all these cases multiple pages are (potentially) dirty.

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx