On 28/07/16 16:10, Chris Wilson wrote:
On Thu, Jul 28, 2016 at 01:48:32PM +0100, Dave Gordon wrote:
On 27/07/16 16:28, Chris Wilson wrote:
On Wed, Jul 27, 2016 at 12:08:35PM +0100, Dave Gordon wrote:
On 25/07/16 10:18, Joonas Lahtinen wrote:
On ma, 2016-07-25 at 08:44 +0100, Chris Wilson wrote:
If is simpler and leads to more readable code through the callstack if
the allocation returns the allocated struct through the return value.
The importance of this is that it no longer looks like we accidentally
allocate requests as side-effect of calling .
Dave seems to have expressed to wish to review this around January, so
CC'ing him here.
>From me,
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_gem.c | 75 ++++++++----------------------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 12 ++---
drivers/gpu/drm/i915/i915_gem_request.c | 58 ++++++++---------------
drivers/gpu/drm/i915/i915_trace.h | 13 +++---
drivers/gpu/drm/i915/intel_display.c | 36 ++++++--------
drivers/gpu/drm/i915/intel_lrc.c | 2 +-
drivers/gpu/drm/i915/intel_overlay.c | 20 ++++----
8 files changed, 79 insertions(+), 140 deletions(-)
The actual code looks fine, but the patch amalgamates several
different changes, only one of which is mentioned in the
description above.
1. Change the signature of i915_gem_object_sync() and its internal
subfunction __i915_gem_object_sync() to always require a request
as input, with corresponding rework of the callsites. This enables
the removal of the internal call to i915_gem_request_alloc().
2. Change API of i915_gem_request_alloc() not to allow NULL ctx,
which involves changing various of the remaining callsites.
Reverts the unwanted the change.
No, 2 and 3 together do NOT revert the change 268270883 because
that's what originally changed the API of i915_gem_request_alloc()
to return a pointer directly, rather than via an output parameter.
Which everyone agrees is a good idea :)
Allowing NULL here was a useful step in the elimination of the
per-ring default context (which was after all your idea, IIRC). Now
that there aren't so many callsites it's not so ugly to have those
callers mention the (now-unified) kernel context.
I'm referring to the change that took it from the API here to what was
merged, and so reverting back to what I originally wrote.
-Chris
Whatever. Item 1 still needs to be a separate patch from 2&|3.
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
