[Intel-gfx] [PATCH] drm/i915: Always mark the object as dirty when used by the GPU

[Jani Nikula]

On Wed, 02 Sep 2015, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Mon, Aug 31, 2015 at 03:10:39PM +0100, Chris Wilson wrote:
>> There have been many hard to track down bugs whereby userspace forgot to
>> flag a write buffer and then cause graphics corruption or a hung GPU
>> when that buffer was later purged under memory pressure (as the buffer
>> appeared clean, its pages would have been evicted rather than preserved
>> and any changes more recent than in the backing storage would be lost).
>> In retrospect this is a rare optimisation against memory pressure,
>> already the slow path. If we always mark the buffer as dirty when
>> accessed by the GPU, anything not used can still be evicted cheaply
>> (ideal behaviour for mark-and-sweep eviction) but we do not run the risk
>> of corruption. For correct read serialisation, userspace still has to
>> notify when the GPU writes to an object. However, there are certain
>> situations under which userspace may wish to tell white lies to the
>> kernel...
>> 
>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: Kristian Høgsberg <krh at bitplanet.net>
>> Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
>> Cc: "Goel, Akash" <akash.goel at intel.co>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> Cc: stable at vger.kernel.org
>
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.

>
>> ---
>>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> index 923a3c4bf0b7..a953d4975b8c 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> @@ -1032,6 +1032,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
>>  		u32 old_read = obj->base.read_domains;
>>  		u32 old_write = obj->base.write_domain;
>>  
>> +		obj->dirty = 1; /* be paranoid  */
>>  		obj->base.write_domain = obj->base.pending_write_domain;
>>  		if (obj->base.write_domain == 0)
>>  			obj->base.pending_read_domains |= obj->base.read_domains;
>> @@ -1039,7 +1040,6 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
>>  
>>  		i915_vma_move_to_active(vma, req);
>>  		if (obj->base.write_domain) {
>> -			obj->dirty = 1;
>>  			i915_gem_request_assign(&obj->last_write_req, req);
>>  
>>  			intel_fb_obj_invalidate(obj, ORIGIN_CS);
>> -- 
>> 2.5.1
>> 
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center