Re: [PATCH v1 1/4] drm/xe/pcode: Introduce xe_pcode_read_probe()

[Raag Jadav <raag.jadav@intel.com>]

On Fri, Nov 21, 2025 at 09:33:42AM +0100, Raag Jadav wrote:
> On Thu, Nov 20, 2025 at 09:02:29AM -0600, Lucas De Marchi wrote:
> > On Tue, Nov 18, 2025 at 04:38:46PM +0100, Raag Jadav wrote:

...

> > > > xe_pcode_read_if_supported
> > > > (Explicit about conditional support.)
> > > 
> > > I don't even begin to qualify here so it's upto you all.
> > 
> > xe_pcode_read_if_supported() would be ok IMO, documenting it to mask
> > not-supported errors.
> > 
> > But the the way this is implemented with the extra flag seems weird.
> > By "having the caller check" I think it's about handling
> > the return code from this function and treating it as a fatal or normal
> > case depending on the command being sent, if there's a fallback etc.
> > This patch seems to add a function and not used it, but I may be missing
> > something.
> 
> Forgot to doc. I had an impression that -ENXIO could be used for the
> fallback since we already have it here but ...
> 
> > I'd rather have this:
> > 
> > 1) Caller should handle errors and treat it as fatal or normal,
> >    depending on having a fallback or not. Emit an err there if
> >    appropriate rather than here. It seems we are already emitting
> >    additional dbgs in the caller for when pcode_read fails
> > 
> > 2) What is the command/subcommand triggering this error? We could have a
> >    helper like xe_pcode_strerr() that users could call if needed (but
> >    then we'd need to return the undecoded error), or we could change
> >    this specific return code to -ENOTSUPP.
> 
> ... converting to -ENOTSUPP makes much more sense, considering the undecoded
> return will be inconsistent with other pcode helpers.

On second thought, looking at it from caller standpoint, I'm wondering why
this shouldn't be the expectation with xe_pcode_read() itself?

I'm okay with the new helper but if we're claiming that something happens
*only* if supported, I'd expect it to be the default behaviour instead of
having extra bells and whistles. The only difference here is how the caller
chooses to treat it (-ENOTSUPP) anyway.

Thoughts?

Raag