Re: [PATCH 3/4] drm/xe/pcode: Report pcode errors using SIGID

On 8/12/2026 8:07 PM, Umesh Nerlige Ramappa wrote:
> On Wed, Aug 12, 2026 at 07:05:35PM +0530, Nilawar, Badal wrote:
>> Hi Umesh,
>>
>> On 12-08-2026 05:22, Umesh Nerlige Ramappa wrote:
>>> Use xe_log_err() to report pcode errors using SIGID.

hmm, it looks that there is overlap with my series [1]

I guess we should hold on with adding more SIGID usages until
we complete review of the infrastructure patches and first set
of SIGID usages that will act as 'golden' examples

[1] https://patchwork.freedesktop.org/series/171022/#rev4

>>>
>>> Signed-off-by: Umesh Nerlige Ramappa <[email protected]>
>>> ---
>>>  drivers/gpu/drm/xe/xe_pcode.c | 12 +++++++-----
>>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
>>> index e9125341481b..b35c1a77effd 100644
>>> --- a/drivers/gpu/drm/xe/xe_pcode.c
>>> +++ b/drivers/gpu/drm/xe/xe_pcode.c
>>> @@ -14,6 +14,7 @@
>>>  #include "regs/xe_pmt.h"
>>>  #include "xe_assert.h"
>>>  #include "xe_device.h"
>>> +#include "xe_log.h"
>>>  #include "xe_mmio.h"
>>>  #include "xe_pcode_api.h"
>>>  #include "xe_pm.h"
>>> @@ -61,9 +62,7 @@ static int pcode_mailbox_status(struct xe_tile *tile)
>>>      }
>>>      if (err) {
>>> -        drm_err(&tile_to_xe(tile)->drm, "PCODE Mailbox failed: %d %s",
>>> -            err_decode, err_str);
>>> -
>>> +        xe_log_err(tile, PCODE, err_decode, "Mailbox failed: %s\n", err_str);
>>>          return err_decode;
>>>      }
>>> @@ -225,6 +224,9 @@ int xe_pcode_request(struct xe_tile *tile, u32 mbox, u32 request,
>>>                  true, 50 * 1000, true);
>>>      preempt_enable();
>>> +    if (!status && ret)
>>> +        xe_log_err(tile, PCODE, ret, "Request timed out on retry");
>>> +
>>>  out:
>>>      mutex_unlock(&tile->pcode.lock);
>>>      return status ? status : ret;
>>> @@ -319,8 +321,8 @@ int xe_pcode_ready(struct xe_device *xe, bool locked)
>>>          mutex_unlock(&tile->pcode.lock);
>>>      if (ret)
>>> -        drm_err(&xe->drm,
>>> -            "PCODE initialization timedout after: 3 min\n");
>>> +        xe_log_err(xe, PCODE, -ret,
>>> +            "Initialization timed out after: %d min\n", timeout_us / 60000000);
>>
>> Is a SIGID needed here? My understanding is that this failure is translated into survivability mode, at least during probe.

IMO yes, it's a FW error, for which there is SIGID, so we have to report it
what will come next (wedge/survivability) is not to be decided here
we should report per site, not per condition

>> Also is it correct to log CPER_SEV_RECOVERABLE for this error?

initial documentation was reserving FATAL only for real PROBE errors
or for SURVIVABILITY mode, but at least for the latter it is no longer true ;)

> 
> I thought all FW errors should be reported based on this conversation:
> https://patchwork.freedesktop.org/patch/744654/?series=171725&rev=1

that's also my understanding

> 
> fwiu, the chain of errors leading to the eventual failure is still useful, but not entirely clear on what's needed and what's not.

IMO if for given error case there is a matching SIGID we should use xe_log to report it

but we should still check if existing error makes sense ...

that's why in my series I'm splitting almost each new SIGID
usage addition to new patch to have proper discussion

> 
> @Mallesh, thoughts?
> 
> Thanks,
> Umesh
> 
>>
>> Thanks,
>> Badal
>>
>>>      return ret;
>>>  }