Re: [PATCH 2/3] drm/i915: mark GEM objects as dirty when updated by the CPU

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

On 08/12/15 17:00, Chris Wilson wrote:
> On Tue, Dec 08, 2015 at 04:51:17PM +0000, Dave Gordon wrote:
>> In various places, one or more pages of a GEM object are mapped into CPU
>> address space and updated. In each such case, either the page or the the
>> object should be marked dirty, to ensure that the modifications are not
>> discarded if the object is evicted under memory pressure.
>>
>> Ideally, we would like to mark only the updated pages dirty; but it
>> isn't clear at this point whether this will work for all types of GEM
>> objects (regular/gtt, phys, stolen, userptr, dmabuf, ...). So for now,
>> let's ensure correctness by marking the whole object dirty.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c   | 2 ++
>>   drivers/gpu/drm/i915/i915_gem_render_state.c | 1 +
>>   drivers/gpu/drm/i915/i915_guc_submission.c   | 1 +
>>   drivers/gpu/drm/i915/intel_lrc.c             | 6 +++++-
>>   4 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> index a4c243c..bc28a10 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> @@ -281,6 +281,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
>>   	}
>>
>>   	kunmap_atomic(vaddr);
>> +	obj->dirty = 1;
> Nak. CPU dirtying is a per-page interface.
> -Chris

That's what my commit message said. But let's at least have /correct/ 
behaviour while we work out which object types we (can) support here.

Also, in:

         if (use_cpu_reloc(obj))
                 ret = relocate_entry_cpu(obj, reloc, target_offset);
         else if (obj->map_and_fenceable)
                 ret = relocate_entry_gtt(obj, reloc, target_offset);
         else if (cpu_has_clflush)
                 ret = relocate_entry_clflush(obj, reloc, target_offset);

both the other routines parallel to relocate_entry_cpu() [i.e. 
relocate_entry_gtt() and relocate_entry_clflush()] mark the whole object 
dirty. Why be inconsistent?

Can we be sure that the object in question actually has per-page 
tracking of dirty pages. shmfs objects do, but not phys, which only has 
object-level dirty tracking. Can we guarantee that only the right sort 
of objects will be handled here? And when stolen objects are exposed to 
the user?

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