[Intel-gfx] [PATCH v3 17/28] drm/i915: Convert 'trace_irq' to use requests rather than seqnos

[Daniel Vetter]

On Mon, Nov 24, 2014 at 06:49:39PM +0000, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
> 
> Updated the trace_irq code to use requests instead of seqnos. This includes
> reference counting the request object to ensure it sticks around when required.
> Note that getting access to the reference counting functions means moving the
> inline i915_trace_irq_get() function from intel_ringbuffer.h to i915_drv.h.
> 
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> Reviewed-by: Thomas Daniel <Thomas.Daniel at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |    7 +++++++
>  drivers/gpu/drm/i915/i915_gem.c         |    7 ++++---
>  drivers/gpu/drm/i915/i915_trace.h       |    2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.h |    8 +-------
>  4 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8bfdac6..831fae2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3130,4 +3130,11 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
>  	}
>  }
>  
> +static inline void i915_trace_irq_get(struct intel_engine_cs *ring,
> +				      struct drm_i915_gem_request *req)
> +{
> +	if (ring->trace_irq_req == NULL && ring->irq_get(ring))
> +		i915_gem_request_assign(&ring->trace_irq_req, req);

This looks a bit suspiciuos. Thus far ring->trace_irq_req was essentially
ot protected at all by anything. But that was nothing troublesome since we
didn't hang a real resource of it.

But now there's a refcounted request in that pointer, which means if we
race we leak. I'll skip this patch for now.
-Daniel

> +}
> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 69c3e50..69bddcb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2810,10 +2810,11 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
>  		i915_gem_free_request(request);
>  	}
>  
> -	if (unlikely(ring->trace_irq_seqno &&
> -		     i915_seqno_passed(seqno, ring->trace_irq_seqno))) {
> +	if (unlikely(ring->trace_irq_req &&
> +		     i915_seqno_passed(seqno,
> +			 i915_gem_request_get_seqno(ring->trace_irq_req)))) {
>  		ring->irq_put(ring);
> -		ring->trace_irq_seqno = 0;
> +		i915_gem_request_assign(&ring->trace_irq_req, NULL);
>  	}
>  
>  	while (!list_empty(&ring->delayed_free_list)) {
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 2c0327b..2ade958 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -369,7 +369,7 @@ TRACE_EVENT(i915_gem_ring_dispatch,
>  			   __entry->ring = ring->id;
>  			   __entry->seqno = i915_gem_request_get_seqno(req);
>  			   __entry->flags = flags;
> -			   i915_trace_irq_get(ring, __entry->seqno);
> +			   i915_trace_irq_get(ring, req);
>  			   ),
>  
>  	    TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x",
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index cd1a9f9..c8b84de 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -142,7 +142,7 @@ struct  intel_engine_cs {
>  
>  	unsigned irq_refcount; /* protected by dev_priv->irq_lock */
>  	u32		irq_enable_mask;	/* bitmask to enable ring interrupt */
> -	u32		trace_irq_seqno;
> +	struct drm_i915_gem_request *trace_irq_req;
>  	bool __must_check (*irq_get)(struct intel_engine_cs *ring);
>  	void		(*irq_put)(struct intel_engine_cs *ring);
>  
> @@ -444,10 +444,4 @@ intel_ring_get_request(struct intel_engine_cs *ring)
>  	return ring->outstanding_lazy_request;
>  }
>  
> -static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
> -{
> -	if (ring->trace_irq_seqno == 0 && ring->irq_get(ring))
> -		ring->trace_irq_seqno = seqno;
> -}
> -
>  #endif /* _INTEL_RINGBUFFER_H_ */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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