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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux