Please don't drop the context of which commit we are talking about.
For further reference, it is 7dd490658627 ('drm/i915: Mark untiled BLT
commands as fenced on gen2/3')
On Mon, 2012-10-08 at 10:49 +0100, Luís Picciochi Oliveira wrote:
> Hi,
>
> Here is the actual diff to the 3.2 kernel:
>
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 2012-07-12
> 04:32:21.000000000 +0100
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c 2012-10-02
> 22:17:56.079302807 +0100
> @@ -573,8 +573,8 @@ i915_gem_execbuffer_reserve(struct intel
> ret = i915_gem_object_put_fence(obj);
> if (ret)
> break;
> + obj->pending_fenced_gpu_access = true;
> }
> - obj->pending_fenced_gpu_access = need_fence;
> }
>
> entry->offset = obj->gtt_offset;
I don't think this is right. Adding more context to the corresponding
hunk in the upstream commit, it looks like:
if (has_fenced_gpu_access) {
if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) {
if (obj->tiling_mode) {
ret = i915_gem_object_get_fence(obj, ring);
if (ret)
goto err_unpin;
entry->flags |= __EXEC_OBJECT_HAS_FENCE;
i915_gem_object_pin_fence(obj);
} else {
ret = i915_gem_object_put_fence(obj);
if (ret)
goto err_unpin;
}
+ obj->pending_fenced_gpu_access = true;
}
- obj->pending_fenced_gpu_access = need_fence;
}
So the condition is changed from has_fenced_gpu_access && need_fence to
has_fenced_gpu_access && (entry->flags & EXEC_OBJECT_NEEDS_FENCE).
But in 3.2 we have:
if (has_fenced_gpu_access) {
if (need_fence) {
ret = i915_gem_object_get_fence(obj,
ring);
if (ret)
break;
} else if (entry->flags &
EXEC_OBJECT_NEEDS_FENCE &&
obj->tiling_mode ==
I915_TILING_NONE) {
/* XXX pipelined! */
ret = i915_gem_object_put_fence(obj);
if (ret)
break;
}
obj->pending_fenced_gpu_access = need_fence;
}
Your suggested hunk will change the condition to has_fenced_gpu_access
&& !need_fence && (entry->flags & EXEC_OBJECT_NEEDS_FENCE) &&
(obj->tiling_mode == I915_TILING_NONE).
Maybe it should be:
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -574,7 +574,7 @@
if (ret)
break;
}
- obj->pending_fenced_gpu_access = need_fence;
+ obj->pending_fenced_gpu_access = true;
}
entry->offset = obj->gtt_offset;
}
--- END ---
Chris, Daniel, can you advise?
Ben.
--
Ben Hutchings
Always try to do things in chronological order;
it's less confusing that way.
signature.asc
Description: This is a digitally signed message part
