[PATCH] drm/amdgpu/display: fix dmub invalid register read

[Harry Wentland]

On 2021-03-24 5:13 p.m., Harry Wentland wrote:
> +Nick, Bhawan
> 
> On 2021-03-24 4:39 p.m., Alex Deucher wrote:
>> On Tue, Mar 23, 2021 at 4:18 AM Thomas Lambertz 
>> <mail at thomaslambertz.de> wrote:
>>>
>>> DMCUB_SCRATCH_0 sometimes contains 0xdeadbeef during initialization.
>>> If this is detected, return 0 instead. This prevents wrong bit-flags
>>> from being read.
>>>
>>> The main impact of this bug is in the status check loop in
>>> dmub_srv_wait_for_auto_load. As it is waiting for the device to become
>>> ready, returning too early leads to a race condition. It is usually won
>>> on first boot, but lost when laptop resumes from sleep, breaking screen
>>> brightness control.
>>>
>>> This issue was always present, but previously mitigated by the fact that
>>> the full register was compared to the wanted value. Currently, only the
>>> bottom two bits are tested, which are also set in 0xdeadbeef, thus
>>> returning readiness to early.
>>>
>>> Fixes: 5fe6b98ae00d ("drm/amd/display: Update dmub code")
>>
>> Bug: 
>> https://gitlab.freedesktop.org/drm/amd/-/issues/1518 
>> Harry, Leo, Rodrigo, any ideas?
>>
> 
> When I checked with our DMUB experts yesterday they said they'd never 
> expect to see 0xdeadbeef in SCRATCH0.
> 
> That said based on Thomas's test it does look like we're getting 
> deadbeef at resume so I'm almost inclined to ACK this patch. It doesn't 
> really do any harm.
> 

Apparently an older version of the renoir_dmcub (and likely other 
versions) had a 0xdeadbeef write to SCRATCH0. In order to support these 
older FW this patch is
Reviewed-by: Harry Wentland <harry.wentland at amd.com>

I recommend updating to the latest FW from linux-firmware if any other 
issues are observed.

Harry

> Harry
> 
>> Alex
>>
>>> Signed-off-by: Thomas Lambertz <mail at thomaslambertz.de>
>>> ---
>>>   drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.c | 8 +++++++-
>>>   drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.h | 2 ++
>>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.c 
>>> b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.c
>>> index 8e8e65fa83c0..d6fcae182f68 100644
>>> --- a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.c
>>> +++ b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.c
>>> @@ -323,8 +323,14 @@ uint32_t dmub_dcn20_get_gpint_response(struct 
>>> dmub_srv *dmub)
>>>   union dmub_fw_boot_status dmub_dcn20_get_fw_boot_status(struct 
>>> dmub_srv *dmub)
>>>   {
>>>          union dmub_fw_boot_status status;
>>> +       uint32_t value;
>>> +
>>> +       value = REG_READ(DMCUB_SCRATCH0);
>>> +       if (value == DMCUB_SCRATCH0_INVALID)
>>> +               status.all = 0;
>>> +       else
>>> +               status.all = value;
>>>
>>> -       status.all = REG_READ(DMCUB_SCRATCH0);
>>>          return status;
>>>   }
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.h 
>>> b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.h
>>> index a62be9c0652e..9557e76cf5d4 100644
>>> --- a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.h
>>> +++ b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn20.h
>>> @@ -154,6 +154,8 @@ struct dmub_srv_common_regs {
>>>
>>>   extern const struct dmub_srv_common_regs dmub_srv_dcn20_regs;
>>>
>>> +#define DMCUB_SCRATCH0_INVALID 0xdeadbeef
>>> +
>>>   /* Hardware functions. */
>>>
>>>   void dmub_dcn20_init(struct dmub_srv *dmub);
>>> -- 
>>> 2.31.0
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx 
>>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx 
>