Re: [PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

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

On Mon, Aug 25, 2025 at 04:48:28PM +0200, Raag Jadav wrote:
> On Mon, Aug 25, 2025 at 04:31:23PM +0200, Andi Shyti wrote:
> > Hi Rodrigo,
> > 
> > On Fri, Aug 22, 2025 at 12:30:02PM -0400, Rodrigo Vivi wrote:
> > > On Tue, Aug 19, 2025 at 04:55:29PM -0100, Andi Shyti wrote:
> > > > From: Andi Shyti <andi.shyti@linux.intel.com>
> > > > 
> > > > Coverity warns that 'cap' may be used uninitialised. If pcode
> > > > is disabled there is no need to go through the hassle of a
> > > > pcode read or taking a PM reference.
> > > 
> > > Please mark it as false positive!
> > 
> > this patch is not for fixing the Coverity warning, but I saw it
> > useless to step any further if there is skip pcode.
> > 
> > The same check is done later in the function, but in the meantime
> > we have done a few things that we could have spared.
> 
> I tried something similar a few days ago, but perhaps not very convincingly
> I presume.
> 
> [1] https://lore.kernel.org/intel-xe/20250806152256.748057-1-raag.jadav@intel.com/

Cc a bunch of folks...

Well, we have a mess to fix here indeed.

We should not be mixing platform checks with info checks that are coming from
platform definition...

Look comment in xe_info_init_early:
"Initialize device info content that only depends on static driver_data
passed to the driver at probe time from PCI ID table."

However this thread made me to realize that we are not respecting that
and we are indeed changing the info at runtime and not only based
on the platform:

sriov_update_device_info()

Hence these kind of patches poping up.

Jani, I remember you did a very good organization in i915 with the static
info vs info that can change in runtime. Any advice, guidance here?

Thanks,
Rodrigo.

> 
> Raag
> 
> > > We will only get here for BMG which has pcode for sure.
> > > 
> > > > 
> > > > Check skip_pcode early in the function and return if it is set.
> > > > 
> > > > No change for platforms where pcode is enabled.
> > > > 
> > > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/xe/xe_device_sysfs.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > > index bd9015761aa0..3a083c215891 100644
> > > > --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > > +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > > @@ -156,6 +156,9 @@ static int late_bind_create_files(struct device *dev)
> > > >  	u32 cap;
> > > >  	int ret;
> > > >  
> > > > +	if (xe->info.skip_pcode)
> > > > +		return 0;
> > > > +
> > > >  	xe_pm_runtime_get(xe);
> > > >  
> > > >  	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
> > > > -- 
> > > > 2.50.0
> > > >