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

[Harry Wentland]

+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.

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>