[PATCH 1/2] drm/xe: make xe_gt_idle_disable_c6() handle the forcewake internally

[Vivi, Rodrigo]

On Mon, 2025-08-25 at 18:28 +0000, Wang, X wrote:
> 
> 
> > -----Original Message-----
> > From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> > Sent: Monday, August 25, 2025 11:15
> > To: Wang, X <x.wang at intel.com>
> > Cc: intel-xe at lists.freedesktop.org; Poosa, Karthik
> > <karthik.poosa at intel.com>;
> > Xiong, James <james.xiong at intel.com>; Summers, Stuart
> > <stuart.summers at intel.com>; Lin, Shuicheng
> > <shuicheng.lin at intel.com>;
> > Belgaumkar, Vinay <vinay.belgaumkar at intel.com>
> > Subject: Re: [PATCH 1/2] drm/xe: make xe_gt_idle_disable_c6()
> > handle the
> > forcewake internally
> > 
> > On Mon, Aug 25, 2025 at 09:57:58AM -0700, Xin Wang wrote:
> > > Move forcewake_get() into xe_gt_idle_enable_c6() to streamline
> > > the
> > > code and make it easier to use.
> > > 
> > > Suggested-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Signed-off-by: Xin Wang <x.wang at intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_gt_idle.c | 23 ++++++++++++++---------
> > > drivers/gpu/drm/xe/xe_gt_idle.h |  2 +-
> > > drivers/gpu/drm/xe/xe_guc_pc.c  | 13 +------------
> > >  3 files changed, 16 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_idle.c
> > > b/drivers/gpu/drm/xe/xe_gt_idle.c index
> > > ffb210216aa9..8556b3ff1561
> > > 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_idle.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt_idle.c
> > > @@ -322,15 +322,11 @@ static void gt_idle_fini(void *arg)  {
> > >  	struct kobject *kobj = arg;
> > >  	struct xe_gt *gt = kobj_to_gt(kobj->parent);
> > > -	unsigned int fw_ref;
> > > 
> > >  	xe_gt_idle_disable_pg(gt);
> > > 
> > > -	if (gt_to_xe(gt)->info.skip_guc_pc) {
> > > -		fw_ref = xe_force_wake_get(gt_to_fw(gt),
> > > XE_FW_GT);
> > > +	if (gt_to_xe(gt)->info.skip_guc_pc)
> > >  		xe_gt_idle_disable_c6(gt);
> > > -		xe_force_wake_put(gt_to_fw(gt), fw_ref);
> > > -	}
> > > 
> > >  	sysfs_remove_files(kobj, gt_idle_attrs);
> > >  	kobject_put(kobj);
> > > @@ -390,14 +386,23 @@ void xe_gt_idle_enable_c6(struct xe_gt *gt)
> > >  			RC_CTL_HW_ENABLE | RC_CTL_TO_MODE |
> > RC_CTL_RC6_ENABLE);  }
> > > 
> > > -void xe_gt_idle_disable_c6(struct xe_gt *gt)
> > > +int xe_gt_idle_disable_c6(struct xe_gt *gt)
> > >  {
> > > -	xe_device_assert_mem_access(gt_to_xe(gt));
> > 
> > I'm afraid you missed my comment about this line in the previous
> > review.
> > 
> > This line has nothing to do with forcewake, so it should remain.
> > or if you really want to remove, please send it in a separate patch
> > with proper
> > explanation.
> > 
> > Thanks,
> > Rodrigo.
> > 
> > > -	xe_force_wake_assert_held(gt_to_fw(gt), XE_FW_GT);
> > > +	unsigned int fw_ref;
> > > 
> > >  	if (IS_SRIOV_VF(gt_to_xe(gt)))
> > > -		return;
> > > +		return 0;
> > > +
> > > +	xe_device_assert_mem_access(gt_to_xe(gt));
> Sorry I missed your previous comments; I did not remove this line. 
> Move it here since we could directly return if Xe is running in SRIOV
> mode.

only if it is a separate patch please.

> > > +
> > > +	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> > > +	if (!fw_ref)
> > > +		return -ETIMEDOUT;
> > > 
> > >  	xe_mmio_write32(&gt->mmio, RC_CONTROL, 0);
> > >  	xe_mmio_write32(&gt->mmio, RC_STATE, 0);
> > > +
> > > +	xe_force_wake_put(gt_to_fw(gt), fw_ref);
> > > +
> > > +	return 0;
> > >  }
> > > diff --git a/drivers/gpu/drm/xe/xe_gt_idle.h
> > > b/drivers/gpu/drm/xe/xe_gt_idle.h index
> > > 591a01e181bc..9c34a155e102
> > > 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt_idle.h
> > > +++ b/drivers/gpu/drm/xe/xe_gt_idle.h
> > > @@ -13,7 +13,7 @@ struct xe_gt;
> > > 
> > >  int xe_gt_idle_init(struct xe_gt_idle *gtidle);  void
> > > xe_gt_idle_enable_c6(struct xe_gt *gt); -void
> > > xe_gt_idle_disable_c6(struct xe_gt *gt);
> > > +int xe_gt_idle_disable_c6(struct xe_gt *gt);
> > >  void xe_gt_idle_enable_pg(struct xe_gt *gt);  void
> > > xe_gt_idle_disable_pg(struct xe_gt *gt);  int
> > > xe_gt_idle_pg_print(struct xe_gt *gt, struct drm_printer *p);
> > > diff
> > > --git a/drivers/gpu/drm/xe/xe_guc_pc.c
> > > b/drivers/gpu/drm/xe/xe_guc_pc.c index 0f8430acd16d..88557e86d637
> > > 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> > > @@ -1076,7 +1076,6 @@ int xe_guc_pc_gucrc_disable(struct
> > > xe_guc_pc
> > > *pc)  {
> > >  	struct xe_device *xe = pc_to_xe(pc);
> > >  	struct xe_gt *gt = pc_to_gt(pc);
> > > -	unsigned int fw_ref;
> > >  	int ret = 0;
> > > 
> > >  	if (xe->info.skip_guc_pc)
> > > @@ -1086,17 +1085,7 @@ int xe_guc_pc_gucrc_disable(struct
> > > xe_guc_pc *pc)
> > >  	if (ret)
> > >  		return ret;
> > > 
> > > -	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)) {
> > > -		xe_force_wake_put(gt_to_fw(gt), fw_ref);
> > > -		return -ETIMEDOUT;
> > > -	}
> > > -
> > > -	xe_gt_idle_disable_c6(gt);
> > > -
> > > -	xe_force_wake_put(gt_to_fw(gt), fw_ref);
> > > -
> > > -	return 0;
> > > +	return xe_gt_idle_disable_c6(gt);
> > >  }
> > > 
> > >  /**
> > > --
> > > 2.43.0
> > >