Re: [PATCH] drm/xe: Always check force_wake_get return code

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Jun 03, 2024 at 02:39:41PM -0500, Lucas De Marchi wrote:
> On Mon, Jun 03, 2024 at 11:56:46AM GMT, Daniele Ceraolo Spurio wrote:
> > The commit title seems a bit too generic, given that you're only fixing
> > a single occurrence. maybe switch to something like "check
> > force_wake_get return code in show_run_ticks" ?
> > 
> > On 6/3/2024 4:30 AM, Tejas Upadhyay wrote:
> > > A force_wake_get failure means that the HW might not be awake for the
> > > access we're doing; this can lead to an immediate error or it can be a
> > > more subtle problem.
> > > 
> > > In this case, user has asked information and in case of forcewake
> > > failure, it might lead to wrong information, so return in case of
> > > error.
> > > 
> > > Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_drm_client.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c
> > > index 4a19b771e3a0..e050fc6d8dd7 100644
> > > --- a/drivers/gpu/drm/xe/xe_drm_client.c
> > > +++ b/drivers/gpu/drm/xe/xe_drm_client.c
> > > @@ -264,7 +264,8 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file)
> > >  		if (!hwe)
> > >  			continue;
> > > -		xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> > > +		if (xe_force_wake_get(gt_to_fw(gt), XE_FW_GT))
> > > +			return;
> > 
> > Since we're not returning any error code, there doesn't seem to be a
> > direct way to know that a forcewake error happened; we can probably
> > infer it by the lack of messages being printed, but there is another
> > (unlikely) exit point that causes no prints to occur. Should we print an
> > error to dmesg, just to be safe?
> 
> see [PATCH v2] drm/xe/client: Check return value of xe_force_wake_get

and https://lore.kernel.org/intel-xe/20240604110225.29495-1-nirmoy.das@intel.com/

> 
> Lucas De Marchi
> 
> > 
> > Daniele
> > 
> > >  		gpu_timestamp = xe_hw_engine_read_timestamp(hwe);
> > >  		xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > >  		break;
> >