[Intel-gfx] [PATCH] drm/i915/gt: Reduce log severity on reset prepare.

[Tvrtko Ursulin]

On 05/12/2023 08:50, Nirmoy Das wrote:
> Hi Tvrtko,
> 
> On 12/5/2023 9:34 AM, Tvrtko Ursulin wrote:
>>
>> On 01/12/2023 15:44, Nirmoy Das wrote:
>>> gen8_engine_reset_prepare() can fail when HW fails to set
>>> RESET_CTL_READY_TO_RESET bit. In some cases this is not fatal
>>> error as driver will retry.
>>>
>>> Let the caller of gen8_engine_reset_prepare() decide if a
>>> failure in gen8_engine_reset_prepare is an error or not.
>>
>> No complaints per se but I don't see the caller deciding and it is not 
>> really reducing log level but converting to trace. So commit message 
>> and patch do not align for me which I think should be improved.
> 
> 
> I meant the return value is checked by the caller, gen8_reset_engines(). 
> I will resend with a improved commit message.

Ah okay, maybe my bad for not figuring out that possibility. I guess it 
might be passable as is, but yes, clearer commit text would be better.

Trace is good enough - we are not usually interested in seeing those as 
dbg/info/notice?

Regards,

Tvrtko

> 
> Thanks,
> 
> Nirmoy
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> Cc: John Harrison <John.C.Harrison at Intel.com>
>>> Cc: Andi Shyti <andi.shyti at linux.intel.com>
>>> Cc: Andrzej Hajda <andrzej.hajda at intel.com>
>>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5591
>>> Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/gt/intel_reset.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c 
>>> b/drivers/gpu/drm/i915/gt/intel_reset.c
>>> index d5ed904f355d..e6fbc6202c80 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
>>> @@ -593,10 +593,10 @@ static int gen8_engine_reset_prepare(struct 
>>> intel_engine_cs *engine)
>>>       ret = __intel_wait_for_register_fw(uncore, reg, mask, ack,
>>>                          700, 0, NULL);
>>>       if (ret)
>>> -        gt_err(engine->gt,
>>> -               "%s reset request timed out: {request: %08x, 
>>> RESET_CTL: %08x}\n",
>>> -               engine->name, request,
>>> -               intel_uncore_read_fw(uncore, reg));
>>> +        GT_TRACE(engine->gt,
>>> +             "%s reset request timed out: {request: %08x, RESET_CTL: 
>>> %08x}\n",
>>> +             engine->name, request,
>>> +             intel_uncore_read_fw(uncore, reg));
>>>         return ret;
>>>   }