[PATCH v2] drm/xe/gt: Introduce runtime suspend/resume

[Raag Jadav]

On Mon, Aug 25, 2025 at 01:52:59PM +0200, Michal Wajdeczko wrote:
> On 8/25/2025 1:20 PM, Raag Jadav wrote:
> > If power state is retained between suspend/resume cycle, we don't have
> > to perform full gt re-initialization. Introduce runtime helpers for gt
> > which greatly reduce suspend/resume delay.

...

> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > index a3397f04abcc..9004af061af3 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -943,6 +943,31 @@ int xe_gt_suspend(struct xe_gt *gt)
> >  	return err;
> >  }
> >  
> 
> please add kernel-doc for all new public functions

I had planned a separate file-wide patch so that the rest of the helpers
don't feel left out.

> > +int xe_gt_runtime_suspend(struct xe_gt *gt)
> > +{
> > +	unsigned int fw_ref;
> > +	int err = -ETIMEDOUT;
> 
> this is only used in err_msg, maybe just hardcode it there
> or use something else than %pe ?
> 
> note that if fw_get fails then there should be already big:
> 
> xe_gt_WARN(..."Forcewake domain%s %#x failed to acknowledge awake request
> 
> so this log with -ETIMEDOUT seems redundant

Failed attempt at stealing gt_suspend/resume code :D
Makes sense.

> > +	xe_gt_dbg(gt, "runtime suspending\n");
> > +
> > +	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> > +	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL))
> > +		goto err_msg;
> > +
> > +	xe_uc_runtime_suspend(&gt->uc);
> > +
> > +	xe_force_wake_put(gt_to_fw(gt), fw_ref);
> > +	xe_gt_dbg(gt, "runtime suspended\n");
> > +
> > +	return 0;
> > +
> > +err_msg:
> > +	xe_force_wake_put(gt_to_fw(gt), fw_ref);
> > +	xe_gt_err(gt, "runtime suspend failed (%pe)\n", ERR_PTR(err));
> > +
> > +	return err;
> > +}

...

> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index 3f4e6a46ff16..f769245f91ae 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -465,7 +465,15 @@ static void ct_exit_safe_mode(struct xe_guc_ct *ct)
> >  		xe_gt_dbg(ct_to_gt(ct), "GuC CT safe-mode disabled\n");
> >  }
> >  
> > -int xe_guc_ct_enable(struct xe_guc_ct *ct)
> > +void xe_guc_ct_enable(struct xe_guc_ct *ct)
> > +{
> > +	guc_ct_change_state(ct, XE_GUC_CT_STATE_ENABLED);
> > +
> > +	if (ct_needs_safe_mode(ct))
> > +		ct_enter_safe_mode(ct);
> > +}
> > +
> 
> add kernel-doc
> 
> and maybe we should expose
> 
> 	xe_guc_ct_runtime_suspend
> 	xe_guc_ct_runtime_resume
> 
> to make it clear when it should called and all logic related to
> CT will be kept in the ct.c code

With that we'll be down 4 level of abstraction just calling each other
without much really happening underneath other than ct_enable/disable,
but sure if it makes more sense.

Raag