Re: [PATCH v2] drm/amd/display: Use freesync when `DRM_EDID_FEATURE_CONTINUOUS_FREQ` found

2024-03-08 Thread Harry Wentland

On 2024-03-08 14:40, Mario Limonciello wrote:

The monitor shipped with the Framework 16 supports VRR [1], but it's not
being advertised.

This is because the detailed timing block doesn't contain
`EDID_DETAIL_MONITOR_RANGE` which amdgpu looks for to find min and max
frequencies.  This check however is superfluous for this case because
update_display_info() calls drm_get_monitor_range() to get these ranges
already.

So if the `DRM_EDID_FEATURE_CONTINUOUS_FREQ` EDID feature is found then
turn on freesync without extra checks.

Closes: 
https://www.reddit.com/r/framework/comments/1b4y2i5/no_variable_refresh_rate_on_the_framework_16_on/
Closes: 
https://www.reddit.com/r/framework/comments/1b6vzcy/framework_16_variable_refresh_rate/
Closes: 
https://community.frame.work/t/resolved-no-vrr-freesync-with-amd-version/42338
Link: https://gist.github.com/superm1/e8fbacfa4d0f53150231d3a3e0a13faf
Signed-off-by: Mario Limonciello 


Reviewed-by: Harry Wentland 

Harry


---
v1->v2:
  * Use is_dp_capable_without_timing_msa() as well for new case
  * Move edid checks up a level
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 +++
  1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5b7293da5453..4e1633a18f2c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -11233,18 +11233,21 @@ void amdgpu_dm_update_freesync_caps(struct 
drm_connector *connector,
if (!adev->dm.freesync_module)
goto update;
  
-	if (sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT

-   || sink->sink_signal == SIGNAL_TYPE_EDP) {
+   if (edid && (sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT ||
+sink->sink_signal == SIGNAL_TYPE_EDP)) {
bool edid_check_required = false;
  
-		if (edid) {

-   edid_check_required = is_dp_capable_without_timing_msa(
-   adev->dm.dc,
-   amdgpu_dm_connector);
+   if (is_dp_capable_without_timing_msa(adev->dm.dc,
+amdgpu_dm_connector)) {
+   if (edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ)
+   freesync_capable = true;
+   else
+   edid_check_required = edid->version > 1 ||
+ (edid->version == 1 &&
+  edid->revision > 1);
}
  
-		if (edid_check_required == true && (edid->version > 1 ||

-  (edid->version == 1 && edid->revision > 1))) {
+   if (edid_check_required) {
for (i = 0; i < 4; i++) {
  
  timing	= >detailed_timings[i];


[PATCH v2] drm/amd/display: Use freesync when `DRM_EDID_FEATURE_CONTINUOUS_FREQ` found

2024-03-08 Thread Mario Limonciello
The monitor shipped with the Framework 16 supports VRR [1], but it's not
being advertised.

This is because the detailed timing block doesn't contain
`EDID_DETAIL_MONITOR_RANGE` which amdgpu looks for to find min and max
frequencies.  This check however is superfluous for this case because
update_display_info() calls drm_get_monitor_range() to get these ranges
already.

So if the `DRM_EDID_FEATURE_CONTINUOUS_FREQ` EDID feature is found then
turn on freesync without extra checks.

Closes: 
https://www.reddit.com/r/framework/comments/1b4y2i5/no_variable_refresh_rate_on_the_framework_16_on/
Closes: 
https://www.reddit.com/r/framework/comments/1b6vzcy/framework_16_variable_refresh_rate/
Closes: 
https://community.frame.work/t/resolved-no-vrr-freesync-with-amd-version/42338
Link: https://gist.github.com/superm1/e8fbacfa4d0f53150231d3a3e0a13faf
Signed-off-by: Mario Limonciello 
---
v1->v2:
 * Use is_dp_capable_without_timing_msa() as well for new case
 * Move edid checks up a level
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5b7293da5453..4e1633a18f2c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -11233,18 +11233,21 @@ void amdgpu_dm_update_freesync_caps(struct 
drm_connector *connector,
if (!adev->dm.freesync_module)
goto update;
 
-   if (sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT
-   || sink->sink_signal == SIGNAL_TYPE_EDP) {
+   if (edid && (sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT ||
+sink->sink_signal == SIGNAL_TYPE_EDP)) {
bool edid_check_required = false;
 
-   if (edid) {
-   edid_check_required = is_dp_capable_without_timing_msa(
-   adev->dm.dc,
-   amdgpu_dm_connector);
+   if (is_dp_capable_without_timing_msa(adev->dm.dc,
+amdgpu_dm_connector)) {
+   if (edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ)
+   freesync_capable = true;
+   else
+   edid_check_required = edid->version > 1 ||
+ (edid->version == 1 &&
+  edid->revision > 1);
}
 
-   if (edid_check_required == true && (edid->version > 1 ||
-  (edid->version == 1 && edid->revision > 1))) {
+   if (edid_check_required) {
for (i = 0; i < 4; i++) {
 
timing  = >detailed_timings[i];
-- 
2.34.1