Re: [PATCH] drm/xe/guc: Only take actions in CT irq handler if CTs are enabled

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Jan 05, 2024 at 08:59:36PM +0100, Michal Wajdeczko wrote:
> 
> 
> On 02.01.2024 22:43, Matthew Brost wrote:
> > Protect entire IRQ handler by CT being enabled rather than just G2H
> > handler.
> > 
> > Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_guc_ct.h | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.h b/drivers/gpu/drm/xe/xe_guc_ct.h
> > index f15f8a4857e0..05302db02e08 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.h
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.h
> > @@ -24,10 +24,11 @@ void xe_guc_ct_print(struct xe_guc_ct *ct, struct drm_printer *p, bool atomic);
> >  
> >  static inline void xe_guc_ct_irq_handler(struct xe_guc_ct *ct)
> >  {
> > -	wake_up_all(&ct->wq);
> > -	if (ct->enabled)
> > +	if (ct->enabled) {
> 
> nit: to simplify the code I would rather exit early if (!ct->enabled)
> 

I like that too, will repost with your RB.

> > +		wake_up_all(&ct->wq);
> 
> btw, shouldn't we wake up wq _after_ we process a g2h message?
> and maybe this should be done in fast_path or worker?
> 

I'll take an AR to look into this, the ct->wq is used for a lot of
cases. Will do a full audit of this.

Matt

> >  		queue_work(system_unbound_wq, &ct->g2h_worker);
> > -	xe_guc_ct_fast_path(ct);
> > +		xe_guc_ct_fast_path(ct);
> > +	}
> 
> anyway, since we move forward,
> 
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 
> >  }
> >  
> >  /* Basic CT send / receives */