[PATCH] drm/xe: Log unreliable MMIO reads during forcewake

[Lin, Shuicheng]

> -----Original Message-----
> From: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>
> Sent: Sunday, October 13, 2024 5:47 AM
> To: Brost, Matthew <matthew.brost at intel.com>; Lin, Shuicheng
> <shuicheng.lin at intel.com>
> Cc: intel-xe at lists.freedesktop.org; Roper, Matthew D
> <matthew.d.roper at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>; Zuo,
> Alex <alex.zuo at intel.com>
> Subject: Re: [PATCH] drm/xe: Log unreliable MMIO reads during forcewake
> 
> 
> 
> On 12.10.2024 07:09, Matthew Brost wrote:
> > On Sat, Oct 12, 2024 at 03:34:45AM +0000, Shuicheng Lin wrote:
> >> In some cases, when the driver attempts to read an MMIO register, the
> >> hardware may return 0xFFFFFFFF. The current force wake path code
> >> treats this as a valid response, as it only checks the BIT.
> >> However, 0xFFFFFFFF should be considered an invalid value, indicating
> >> a potential issue. To address this, we should add a log entry to
> >> highlight this condition.
> >>
> >> Suggested-by: Alex Zuo <alex.zuo at intel.com>
> >> Signed-off-by: Shuicheng Lin <shuicheng.lin at intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_force_wake.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c
> >> b/drivers/gpu/drm/xe/xe_force_wake.c
> >> index a64c14757c84..46f36d05293a 100644
> >> --- a/drivers/gpu/drm/xe/xe_force_wake.c
> >> +++ b/drivers/gpu/drm/xe/xe_force_wake.c
> >> @@ -114,6 +114,10 @@ static int __domain_wait(struct xe_gt *gt, struct
> xe_force_wake_domain *domain,
> >>  	ret = xe_mmio_wait32(&gt->mmio, domain->reg_ack, domain->val,
> wake ? domain->val : 0,
> >>  			     XE_FORCE_WAKE_ACK_TIMEOUT_MS *
> USEC_PER_MSEC,
> >>  			     &value, true);
> >> +	if (value == ~0)
> >> +		xe_gt_notice(gt,
> >> +			     "Force wake domain %d: %s. MMIO unreliable
> (forcewake register returns 0xFFFFFFFF)!\n",
> >> +			     domain->id, str_wake_sleep(wake));
> >
> > Set the ret value (-EIO) to kick the error to upper layers?
> 
> note that in upcoming series [1] there will be no error code returned
> 
> [1] https://patchwork.freedesktop.org/series/137982/#rev9
> 
Thanks for the info.
I checked the patches. The change is upper of function domain_wake_wait()/domain_sleep_wait(), while my change is inside of them.
The return value is checked in xe_force_wake_get()/xe_force_wake_put().
There is no conflict of my patch with this series.

Shuicheng
> >
> >>  	if (ret)
> >
> > Then...
> >
> > if (ret)
> > 	...
> > else if (value == ~0)
> > 	...
> > 	ret = -EIO;
> >
> > Matt
> >
> >>  		xe_gt_notice(gt, "Force wake domain %d failed to ack %s
> (%pe) reg[%#x] = %#x\n",
> >>  			     domain->id, str_wake_sleep(wake), ERR_PTR(ret),
> >> --
> >> 2.25.1
> >>