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

From: Daniel Vetter <daniel@ffwll.ch>
To: Dave Gordon <david.s.gordon@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/i915: mark GEM objects as dirty when filled by the CPU
Date: Thu, 10 Dec 2015 11:49:46 +0100	[thread overview]
Message-ID: <20151210104946.GV20822@phenom.ffwll.local> (raw)
In-Reply-To: <56671C32.20008@intel.com>

On Tue, Dec 08, 2015 at 06:06:42PM +0000, Dave Gordon wrote:
> On 08/12/15 17:00, Chris Wilson wrote:
> >On Tue, Dec 08, 2015 at 04:51:16PM +0000, Dave Gordon wrote:
> >>In various places, a GEM object is filled with data by means of CPU
> >>writes. In such cases, the object should be marked dirty, to ensure that
> >>the data is not discarded if the object is evicted under memory
> >>pressure.
> >>
> >>This incorporates and supercedes Alex Dai's earlier patch
> >>[PATCH v1] drm/i915/guc: Fix a fw content lost issue after it is evicted
> >>
> >>Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> >>Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>Cc: Alex Dai <yu.dai@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_cmd_parser.c | 1 +
> >>  drivers/gpu/drm/i915/i915_gem.c        | 1 +
> >>  2 files changed, 2 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> >>index 814d894..292bd5d 100644
> >>--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> >>+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> >>@@ -945,6 +945,7 @@ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj,
> >>  		drm_clflush_virt_range(src, batch_len);
> >>
> >>  	memcpy(dst, src, batch_len);
> >>+	dest_obj->dirty = 1;
> >
> >There is no bug here.
> >-Chris
> 
> Because the shadow batch has been page-pinned by the caller? Two questions,
> then:
> 
> 1. Do we reuse the same shadow batch if the same source batch is
>    resubmitted? If so, can we be sure that it has stayed pinned
>    (one way or another) for the entire intervening period?
> 
> 2. If the shadow batch can never be unpinned, why do we allocate
>    it as a GEM object with backing store which can apparently
>    never be used. We could get rid of the shmfs setup overhead by
>    choosing an object type without backing store.

We don't care about coherency once the shadow batch is retired (we mark it
as purgeable even, which flat-out ignores ->dirty). We use shmem because
we need some memory for it, there's no copying to the backing storage
(like ttm does when evicting from vram to shmem).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx