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

On Wed, Aug 12, 2026 at 09:45:40PM +0200, Michal Wajdeczko wrote:
>
>
>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

For PCODE, then let's just continue the review in the below (Michal's) 
series since the latest one has all relevant errors.

https://patchwork.freedesktop.org/series/171022/#rev4

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