Hi Colin,
Sorry for any confusion of this code.
I think in this case, it seems like the comment is wrong (but original
implementation is somewhat wrong as well). Probably the original code
implementation makes it unclear.
There are three scenarios:
1. Variable refresh active, targeting a fixed rate
In this case, the min = max = fixed rate
2. Variable refresh active, with a variable range
In this case, the min = minimum refresh rate of the range.
And the max = maximum refresh rate of the range.
3. Variable refresh rate is disabled (The case you are modifying)
In the disabled case, we want to indicate to the display that the refresh rate
is fixed, so we want to program min = max = the base refresh rate.
Today there seems to be an implication that max refresh = base refresh, which
is not necessarily true.
I guess to make the code more clear and correct, the min and max should both be
programmed equal to the base refresh rate (nominal field rate from
mod_freesync_calc_nominal_field_rate)
Does that make sense?
Thanks,
Anthony
-Original Message-
From: Colin King
Sent: Thursday, April 23, 2020 10:03 AM
To: Wentland, Harry ; Li, Sun peng (Leo)
; Deucher, Alexander ; Koenig,
Christian ; Zhou, David(ChunMing)
; David Airlie ; Daniel Vetter
; Koo, Anthony ;
amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
Subject: [PATCH][next] drm/amd/display: fix incorrect assignment due to a typo
From: Colin Ian King
The assignment to infopacket->sb[7] looks incorrect, the comment states it is
the minimum refresh rate yet it is being assigned a value from the maximum
refresh rate max_refresh_in_uhz. Fix this by using min_refresh_in_uhz instead.
Addresses-Coverity: ("Copy-paste error")
Fixes: d2bacc38f6ca ("drm/amd/display: Change infopacket type programming")
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index eb7421e83b86..fe11436536e8 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -587,7 +587,7 @@ static void build_vrr_infopacket_data_v3(const struct
mod_vrr_params *vrr,
} else {
// Non-fs case, program nominal range
/* PB7 = FreeSync Minimum refresh rate (Hz) */
- infopacket->sb[7] = (unsigned char)((vrr->max_refresh_in_uhz +
50) / 100);
+ infopacket->sb[7] = (unsigned char)((vrr->min_refresh_in_uhz +
+50) / 100);
/* PB8 = FreeSync Maximum refresh rate (Hz) */
infopacket->sb[8] = (unsigned char)((vrr->max_refresh_in_uhz +
50) / 100);
}
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
