[Intel-gfx] [RFC, 1/4] drm/i915: Convert requests to use struct fence

[Dave Gordon]

On 07/04/15 12:18, Maarten Lankhorst wrote:
> Hey,
> 
> Op 07-04-15 om 12:59 schreef John Harrison:
>> On 07/04/2015 10:18, Maarten Lankhorst wrote:
>>> Hey,
>>>
>>> Op 20-03-15 om 18:48 schreef John.C.Harrison at Intel.com:
>>>> From: John Harrison <John.C.Harrison at Intel.com>
>>>>
>>>> There is a construct in the linux kernel called 'struct fence' that is intended
>>>> to keep track of work that is executed on hardware. I.e. it solves the basic
>>>> problem that the drivers 'struct drm_i915_gem_request' is trying to address. The
>>>> request structure does quite a lot more than simply track the execution progress
>>>> so is very definitely still required. However, the basic completion status side
>>>> could be updated to use the ready made fence implementation and gain all the
>>>> advantages that provides.
>>>>
>>>> This patch makes the first step of integrating a struct fence into the request.
>>>> It replaces the explicit reference count with that of the fence. It also
>>>> replaces the 'is completed' test with the fence's equivalent. Currently, that
>>>> simply chains on to the original request implementation. A future patch will
>>>> improve this.
>>>>
>>>> For: VIZ-5190
>>>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>>>>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_drv.h         |   37 +++++++++------------
>>>>   drivers/gpu/drm/i915/i915_gem.c         |   55 ++++++++++++++++++++++++++++---
>>>>   drivers/gpu/drm/i915/intel_lrc.c        |    1 +
>>>>   drivers/gpu/drm/i915/intel_ringbuffer.c |    1 +
>>>>   drivers/gpu/drm/i915/intel_ringbuffer.h |    3 ++
>>>>   5 files changed, 70 insertions(+), 27 deletions(-)

Since Maarten provided i915_gem_request_unreference__unlocked() in the
interval since this was first posted, you'll need to convert that too:

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4213dfb..97073fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2218,14 +2218,8 @@ void i915_gem_request_unreference_irq(struct drm_i915_gem_request *req);
>  static inline void
>  i915_gem_request_unreference__unlocked(struct drm_i915_gem_request *req)
>  {
> -       struct drm_device *dev;
> -
> -       if (!req)
> -               return;
> -
> -       dev = req->ring->dev;
> -       if (kref_put_mutex(&req->ref, i915_gem_request_free, &dev->struct_mutex))
> -               mutex_unlock(&dev->struct_mutex);
> +       if (req)
> +               fence_put_mutex(&req->fence, &req->ring->dev->struct_mutex);
>  }
>  static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,

... and here's the implementation of fence_put_mutex() ...

> diff --git a/include/linux/fence.h b/include/linux/fence.h
> index 39efee1..c4fbb25 100644
> --- a/include/linux/fence.h
> +++ b/include/linux/fence.h
> @@ -218,6 +218,24 @@ static inline void fence_put(struct fence *fence)
>                 kref_put(&fence->refcount, fence_release);
>  }
>  
> +/**
> + * fence_put_mutex - decrement refcount for object.
> + * @fence: object.
> + * @lock: lock to take in release case
> + *
> + * A version of fence_put() that doesn't require the caller to hold the
> + * associated lock already. If the refcount doesn't go to zero, the lock
> + * is not needed; if it does, the lock will automatically be acquired and
> + * released around the call to fence_release().
> + */
> +static inline void fence_put_mutex(struct fence *fence,
> +                                  struct mutex *lock)
> +{
> +       if (fence)
> +               if (kref_put_mutex(&fence->refcount, fence_release, lock))
> +                       mutex_unlock(lock);
> +}
> +
>  int fence_signal(struct fence *fence);
>  int fence_signal_locked(struct fence *fence);
>  signed long fence_default_wait(struct fence *fence, bool intr, signed long timeout);

.Dave.