Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/gt: Only kick the signal worker if there's been an update

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Jul 08, 2022 at 04:20:13PM +0200, Karolina Drobnik wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> One impact of commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove
> dma_resv workaround") is that it stores many, many more fences. Whereas
> adding an exclusive fence used to remove the shared fence list, that
> list is now preserved and the write fences included into the list. Not
> just a single write fence, but now a write/read fence per context. That
> causes us to have to track more fences than before (albeit half of those
> are redundant), and we trigger more interrupts for multi-engine
> workloads.
> 
> As part of reducing the impact from handling more signaling, we observe
> we only need to kick the signal worker after adding a fence iff we have

s/iff/if

> good cause to believe that there is work to be done in processing the
> fence i.e. we either need to enable the interrupt or the request is
> already complete but we don't know if we saw the interrupt and so need
> to check signaling.
> 
> References: 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Karolina Drobnik <karolina.drobnik@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> index 9dc9dccf7b09..ecc990ec1b95 100644
> --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> @@ -399,7 +399,8 @@ static void insert_breadcrumb(struct i915_request *rq)
>  	 * the request as it may have completed and raised the interrupt as
>  	 * we were attaching it into the lists.
>  	 */
> -	irq_work_queue(&b->irq_work);
> +	if (!b->irq_armed || __i915_request_is_complete(rq))

would we need the READ_ONCE(irq_armed) ?
would we need to use the irq_lock?

> +		irq_work_queue(&b->irq_work);
>  }
>  
>  bool i915_request_enable_breadcrumb(struct i915_request *rq)
> -- 
> 2.25.1
>