diff mbox

[3/4,v3] drm/i915: always mark the target of pwrite() as dirty

Message ID 1449773486-30822-4-git-send-email-david.s.gordon@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Gordon Dec. 10, 2015, 6:51 p.m. UTC
Currently, the target object being written *may* be marked dirty, either
in i915_gem_gtt_pwrite_fast() (as a side-effect of setting its domain to
GTT!), or in i915_gem_shmem_pwrite() (if it's a shmfs-backed object).
While these two are the common cases, it's not obvious that they cover
every possible path through the pwrite code, for every possible type
of object (e.g. phys, stolen, etc). So here we move setting-the-mark
to the top level so that it is obvious that it applies no matter which
subsequent path is followed.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Chris Wilson Dec. 10, 2015, 9:09 p.m. UTC | #1
On Thu, Dec 10, 2015 at 06:51:25PM +0000, Dave Gordon wrote:
> Currently, the target object being written *may* be marked dirty, either
> in i915_gem_gtt_pwrite_fast() (as a side-effect of setting its domain to
> GTT!), or in i915_gem_shmem_pwrite() (if it's a shmfs-backed object).
> While these two are the common cases, it's not obvious that they cover
> every possible path through the pwrite code, for every possible type
> of object (e.g. phys, stolen, etc). So here we move setting-the-mark
> to the top level so that it is obvious that it applies no matter which
> subsequent path is followed.
> 
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

I don't like this patch - I feel like it divorces the information that
we are dirtying the pages from the actual copy. Especially as some paths
don't actually dirty the object's backing storage (for extra confusion).
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 936f0a9..81a770f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -937,7 +937,6 @@  i915_gem_shmem_pwrite(struct drm_device *dev,
 	i915_gem_object_pin_pages(obj);
 
 	offset = args->offset;
-	obj->dirty = 1;
 
 	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents,
 			 offset >> PAGE_SHIFT) {
@@ -1074,6 +1073,9 @@  i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 		goto out;
 	}
 
+	/* Object backing store will be out of date hereafter */
+	obj->dirty = 1;
+
 	trace_i915_gem_object_pwrite(obj, args->offset, args->size);
 
 	ret = -EFAULT;