[PATCH v5 1/2] drm/xe: modify GuC CTB enable communication

[Matthew Brost]

On Thu, Aug 01, 2024 at 08:34:59PM +0000, Matthew Brost wrote:
> On Thu, Aug 01, 2024 at 11:29:40AM +0530, Riana Tauro wrote:
> > Modify GuC CTB enable communication function to only set the
> > CTB state if registration is not required.
> > This will be used while resuming from D3Hot where
> > registration of CTB is not necessary.
> > 
> > v2: split patches (Rodrigo)
> > 
> > v3: do not expose internals of CT layer (Michal)
> > 
> > Signed-off-by: Riana Tauro <riana.tauro at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_guc.c | 19 +++++++++++++++----
> >  drivers/gpu/drm/xe/xe_guc.h |  2 +-
> >  drivers/gpu/drm/xe/xe_uc.c  |  4 ++--
> >  3 files changed, 18 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> > index de0fe9e65746..1ff49318f001 100644
> > --- a/drivers/gpu/drm/xe/xe_guc.c
> > +++ b/drivers/gpu/drm/xe/xe_guc.c
> > @@ -755,7 +755,7 @@ static int vf_guc_min_load_for_hwconfig(struct xe_guc *guc)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = xe_guc_enable_communication(guc);
> > +	ret = xe_guc_enable_communication(guc, true);
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -800,7 +800,7 @@ int xe_guc_min_load_for_hwconfig(struct xe_guc *guc)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = xe_guc_enable_communication(guc);
> > +	ret = xe_guc_enable_communication(guc, true);
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -854,7 +854,16 @@ static void guc_enable_irq(struct xe_guc *guc)
> >  	xe_mmio_rmw32(gt, GUC_SG_INTR_MASK, events, 0);
> >  }
> >  
> > -int xe_guc_enable_communication(struct xe_guc *guc)
> > +/**
> > + * xe_guc_enable_communication - Enable GuC CTB communication
> > + * @guc: GuC object
> > + * @register_ctb: true to register CTB, false otherwise
> > + *
> > + * Enables GuC CTB Communication and IRQ
> > + *
> > + * Return: 0 on success, negative error code otherwise
> > + */
> > +int xe_guc_enable_communication(struct xe_guc *guc, bool register_ctb)
> >  {
> >  	struct xe_device *xe = guc_to_xe(guc);
> >  	int err;
> > @@ -870,7 +879,9 @@ int xe_guc_enable_communication(struct xe_guc *guc)
> >  		guc_enable_irq(guc);
> >  	}
> >  
> > -	err = xe_guc_ct_enable(&guc->ct);
> > +	if (register_ctb)
> > +		err = xe_guc_ct_enable(&guc->ct);
> > +
> >  	if (err)
> 
> err could be unitialized here and I believe some of kernel test robot
> scripts will likely complain about this even though it is impossible to
> hit in this patch or once the following patch lands.
> 

Scratch it being impossible to hit once the following patch lands, it is
indeed possible as the 'else' clause will call
'xe_guc_ct_runtime_resume' which doesn't set the 'err'.

Matt 

> Matt 
> 
> >  		return err;
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_guc.h b/drivers/gpu/drm/xe/xe_guc.h
> > index e0bbf98f849d..13e75573f7d0 100644
> > --- a/drivers/gpu/drm/xe/xe_guc.h
> > +++ b/drivers/gpu/drm/xe/xe_guc.h
> > @@ -20,7 +20,7 @@ int xe_guc_post_load_init(struct xe_guc *guc);
> >  int xe_guc_reset(struct xe_guc *guc);
> >  int xe_guc_upload(struct xe_guc *guc);
> >  int xe_guc_min_load_for_hwconfig(struct xe_guc *guc);
> > -int xe_guc_enable_communication(struct xe_guc *guc);
> > +int xe_guc_enable_communication(struct xe_guc *guc, bool register_ctb);
> >  int xe_guc_suspend(struct xe_guc *guc);
> >  void xe_guc_notify(struct xe_guc *guc);
> >  int xe_guc_auth_huc(struct xe_guc *guc, u32 rsa_addr);
> > diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
> > index 0d073a9987c2..fa98e9f22631 100644
> > --- a/drivers/gpu/drm/xe/xe_uc.c
> > +++ b/drivers/gpu/drm/xe/xe_uc.c
> > @@ -154,7 +154,7 @@ static int vf_uc_init_hw(struct xe_uc *uc)
> >  	if (err)
> >  		return err;
> >  
> > -	err = xe_guc_enable_communication(&uc->guc);
> > +	err = xe_guc_enable_communication(&uc->guc, true);
> >  	if (err)
> >  		return err;
> >  
> > @@ -194,7 +194,7 @@ int xe_uc_init_hw(struct xe_uc *uc)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = xe_guc_enable_communication(&uc->guc);
> > +	ret = xe_guc_enable_communication(&uc->guc, true);
> >  	if (ret)
> >  		return ret;
> >  
> > -- 
> > 2.40.0
> >