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

[Dave Gordon]

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.)
>
> 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

So, I tried replacing all instances of "obj->dirty = true" with my new 
function i915_gem_object_mark_dirty(), and added an assertion that it's 
called only when (pages_pin_count > 0) - and found a failure.

Stack is:
	i915_gem_object_mark_dirty
	i915_gem_object_set_to_gtt_domain
	i915_gem_set_domain_ioctl

So is i915_gem_object_set_to_gtt_domain() wrong? It's done a get_pages 
but no pin_pages. Also, i915_gem_object_set_to_cpu_domain() doesn't mark 
the object dirty in the corresponding if(write) clause - is that also wrong?

.Dave.