[Intel-gfx] [PATCH 18/31] drm/i915: Simplify request_alloc by returning the allocated request

[Chris Wilson]

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 certain functions.
> >
> >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 at linux.intel.com>
> >
> >>Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>---
> >> 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.

> 3. Flatten __i915_gem_request_alloc() into i915_gem_request_alloc().
> 
> The description really only covers (3), which is the simplest and
> most mechanical of the changes. So preferably two or three separate
> patches, in whatever order makes the most sense, with a bit more
> description of each.

It mentions 1 explicitly which was the reason why the API was originally
intended to be this.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre