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

[Nirmoy Das <nirmoy.das@linux.intel.com>]

Hi Tvrtko,

On 12/5/2023 11:05 AM, Tvrtko Ursulin wrote:
>
> 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.

I sent a v2 already :)


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


Idea is that all the GT related events are recorded in trace and dmesg 
could be noisy some times.


Regards,

Nirmoy

>
> Regards,
>
> Tvrtko
>
>>
>> Thanks,
>>
>> Nirmoy
>>
>>>
>>> Regards,
>>>
>>> Tvrtko
>>>
>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
>>>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>>>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5591
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@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;
>>>>   }