Re: [PATCH] drm/xe/gsc: Make GSCCS disabling message less alarming

[John Harrison <john.c.harrison@intel.com>]

On 12/19/2024 13:06, Daniele Ceraolo Spurio wrote:
> On 12/18/2024 5:51 PM, John Harrison wrote:
>> On 12/18/2024 17:09, Daniele Ceraolo Spurio wrote:
>>> The GSCCS is only used to transport messages from the driver to the GSC
>>> FW; therefore, we can disable it if we're not using the FW, which is
>>> the case on both BMG and PTL. However, the current wording of the 
>>> logged
>>> message could be interpreted as a problem, so reword to make it clearer
>>> it is not an error and lower it debug verbosity as users don't really
>>> need to know about it.
>>>
>>> Reported-by: Eero Tamminen <eero.t.tamminen@intel.com>
>>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/3866
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_hw_engine.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c 
>>> b/drivers/gpu/drm/xe/xe_hw_engine.c
>>> index 4294aa600192..ac9c666a9652 100644
>>> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
>>> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
>>> @@ -775,7 +775,7 @@ static void check_gsc_availability(struct xe_gt 
>>> *gt)
>>>           xe_mmio_write32(&gt->mmio, GUNIT_GSC_INTR_ENABLE, 0);
>>>           xe_mmio_write32(&gt->mmio, GUNIT_GSC_INTR_MASK, ~0);
>>>   -        drm_info(&xe->drm, "gsccs disabled due to lack of FW\n");
>>> +        drm_dbg(&xe->drm, "GSC FW not used, disabling gsccs\n");
>> This message would also be hit in the case where the firmware is 
>> supposed to be present but isn't. E.g. on a LNL but without the right 
>> firmware package installed? But presumably in that case, there would 
>> already have been an earlier message about missing firmware files?
>
> Right now if the firmware is missing we abort the driver load, so we 
> don't even get this far. If that were to change (and the code here 
> does for this purpose support the case where the FW is defined but 
> missing) I'd expect we'd get an earlier error about the missing blob.
>
> Daniele
Sounds good.

Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

>
>>
>> John.
>>
>>>       }
>>>   }
>>
>