Re: [PATCH v1] drm/xe/pcode: stop logging mailbox status as error

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

On Thu, Mar 06, 2025 at 11:13:49AM -0500, Rodrigo Vivi wrote:
> On Thu, Mar 06, 2025 at 12:44:21PM +0200, Raag Jadav wrote:
> > On Thu, Mar 06, 2025 at 03:17:16PM +0530, Aravind Iddamsetty wrote:
> > > On 06-03-2025 14:27, Raag Jadav wrote:
> > > > On Thu, Mar 06, 2025 at 10:08:07AM +0530, Aravind Iddamsetty wrote:
> > > >> Hi Raag,
> > > >>
> > > >> On 04-03-2025 11:47, Raag Jadav wrote:
> > > >>> Since we're expecting many platform specific mailbox commands that
> > > >>> might not be widely supported, stop logging them as errors. With this
> > > >>> we can avoid unnecessary platform checks and use the return value of
> > > >>> mailbox without enraging CI.
> > > >> are you saying there are cases where we send a command that is not supported
> > > >> on a particular platform?
> > > > Yes. First instance on [1], which will be expanded with more commands.
> > > >
> > > > [1] https://lore.kernel.org/r/20250210100515.2205584-1-raag.jadav@intel.com/
> > > 
> > > But why should we expose the interface which is not supported on that HW.
> > 
> > It allows checking unsupported functionality through return code without
> > making CI angry. So it's either this or we can limit each command (current
> > and future) with explicit platform checks and keep revisiting them everytime
> > we add a new platform or command for the foreseeable future, whichever makes
> > more sense.
> 
> I believe what other folks tried to raise here is:
> If the platform doesn't support it, please do not call it to start with.

I thought the point of error codes is to actually use them, but sure.
Platform checks it is.

Raag

> > > >>> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > > >>> ---
> > > >>>  drivers/gpu/drm/xe/xe_pcode.c | 2 +-
> > > >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
> > > >>> index 9333ce776a6e..280f69119ff1 100644
> > > >>> --- a/drivers/gpu/drm/xe/xe_pcode.c
> > > >>> +++ b/drivers/gpu/drm/xe/xe_pcode.c
> > > >>> @@ -46,7 +46,7 @@ static int pcode_mailbox_status(struct xe_tile *tile)
> > > >>>  
> > > >>>  	err = xe_mmio_read32(&tile->mmio, PCODE_MAILBOX) & PCODE_ERROR_MASK;
> > > >>>  	if (err) {
> > > >>> -		drm_err(&tile_to_xe(tile)->drm, "PCODE Mailbox failed: %d %s", err,
> > > >>> +		drm_dbg(&tile_to_xe(tile)->drm, "PCODE Mailbox failed: %d %s", err,
> > > >>>  			err_decode[err].str ?: "Unknown");
> > > >>>  		return err_decode[err].errno ?: -EPROTO;
> > > >>>  	}