Re: [PATCH v3 12/28] drm/i915: Convert mmio_flip::seqno to struct request

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Nov 24, 2014 at 06:49:34PM +0000, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Converted the mmio_flip 'seqno' value to be a request structure as part of the
> on going seqno to request changes. This includes reference counting the request
> being saved away to ensure it can not be retired and freed while the flip code
> is still waiting on it.
> 
> v2: Used the IRQ friendly request dereference call in the notify handler as that
> code is called asynchronously without holding any useful mutex locks.
> 
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Reviewed-by: Thomas Daniel <Thomas.Daniel@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   20 ++++++++++----------
>  drivers/gpu/drm/i915/intel_drv.h     |    3 +--
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 097e8a1..cbf3cb7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9550,18 +9550,19 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
>  {
>  	struct intel_crtc *intel_crtc =
>  		container_of(work, struct intel_crtc, mmio_flip.work);
> -	struct intel_engine_cs *ring;
> -	uint32_t seqno;
> -
> -	seqno = intel_crtc->mmio_flip.seqno;
> -	ring = intel_crtc->mmio_flip.ring;
> +	struct intel_mmio_flip *mmio_flip;
>  
> -	if (seqno)
> -		WARN_ON(__i915_wait_seqno(ring, seqno,
> +	mmio_flip = &intel_crtc->mmio_flip;
> +	if (mmio_flip->req)
> +		WARN_ON(__i915_wait_seqno(i915_gem_request_get_ring(mmio_flip->req),
> +					  i915_gem_request_get_seqno(mmio_flip->req),
>  					  intel_crtc->reset_counter,
>  					  false, NULL, NULL) != 0);
>  
>  	intel_do_mmio_flip(intel_crtc);
> +	if (mmio_flip->req)
> +		i915_gem_request_unreference_irq(mmio_flip->req);

Can't we just grab dev->struct_mutex around here and reuse the normal
request_unref? That would allow us to ditch all the unref_irq complexity.
-Daniel

> +	mmio_flip->req = NULL;
>  }
>  
>  static int intel_queue_mmio_flip(struct drm_device *dev,
> @@ -9573,9 +9574,8 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> -	intel_crtc->mmio_flip.seqno =
> -			     i915_gem_request_get_seqno(obj->last_write_req);
> -	intel_crtc->mmio_flip.ring = obj->ring;
> +	i915_gem_request_assign(&intel_crtc->mmio_flip.req,
> +				obj->last_write_req);
>  
>  	schedule_work(&intel_crtc->mmio_flip.work);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 44b153c5..2755532 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -406,8 +406,7 @@ struct intel_pipe_wm {
>  };
>  
>  struct intel_mmio_flip {
> -	u32 seqno;
> -	struct intel_engine_cs *ring;
> +	struct drm_i915_gem_request *req;
>  	struct work_struct work;
>  };
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx