[PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor

[PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The current comment fails to clarify why we only accept
the "range limits only" variant of the range descriptor.
Reword it to make some actual sense.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 4005dab6147d..ac662495635c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6033,10 +6033,13 @@ void get_monitor_range(const struct detailed_timing *timing, void *c)
 		return;
 
 	/*
-	 * Check for flag range limits only. If flag == 1 then
-	 * no additional timing information provided.
-	 * Default GTF, GTF Secondary curve and CVT are not
-	 * supported
+	 * These limits are used to determine the VRR refresh
+	 * rate range. Only the "range limits only" variant
+	 * of the range descriptor seems to guarantee that
+	 * any and all timings are accepted by the sink, as
+	 * opposed to just timings conforming to the indicated
+	 * formula (GTF/GTF2/CVT). Thus other variants of the
+	 * range descriptor are not accepted here.
 	 */
 	if (range->flags != DRM_EDID_RANGE_LIMITS_ONLY_FLAG)
 		return;
-- 
2.35.1

Re: [PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor

[Navare, Manasi]

On Sat, Aug 27, 2022 at 12:34:52AM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The current comment fails to clarify why we only accept
> the "range limits only" variant of the range descriptor.
> Reword it to make some actual sense.
>

Thanks Ville for adding this description for monitor_range

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 4005dab6147d..ac662495635c 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6033,10 +6033,13 @@ void get_monitor_range(const struct detailed_timing *timing, void *c)
>  		return;
>  
>  	/*
> -	 * Check for flag range limits only. If flag == 1 then
> -	 * no additional timing information provided.
> -	 * Default GTF, GTF Secondary curve and CVT are not
> -	 * supported
> +	 * These limits are used to determine the VRR refresh
> +	 * rate range. Only the "range limits only" variant
> +	 * of the range descriptor seems to guarantee that
> +	 * any and all timings are accepted by the sink, as
> +	 * opposed to just timings conforming to the indicated
> +	 * formula (GTF/GTF2/CVT). Thus other variants of the
> +	 * range descriptor are not accepted here.
>  	 */
>  	if (range->flags != DRM_EDID_RANGE_LIMITS_ONLY_FLAG)
>  		return;
> -- 
> 2.35.1
> 

[PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rename info->monitor_range to info->vrr_range to actually
reflect its usage.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 ++++-----
 drivers/gpu/drm/drm_debugfs.c                 |  4 +--
 drivers/gpu/drm/drm_edid.c                    | 26 +++++++++----------
 drivers/gpu/drm/i915/display/intel_vrr.c      |  6 ++---
 include/drm/drm_connector.h                   |  4 +--
 5 files changed, 26 insertions(+), 26 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 e702f0d72d53..928b5b6541db 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9921,8 +9921,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
 		amdgpu_dm_connector->min_vfreq = 0;
 		amdgpu_dm_connector->max_vfreq = 0;
 		amdgpu_dm_connector->pixel_clock_mhz = 0;
-		connector->display_info.monitor_range.min_vfreq = 0;
-		connector->display_info.monitor_range.max_vfreq = 0;
+		connector->display_info.vrr_range.min_vfreq = 0;
+		connector->display_info.vrr_range.max_vfreq = 0;
 		freesync_capable = false;
 
 		goto update;
@@ -9970,8 +9970,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
 				amdgpu_dm_connector->pixel_clock_mhz =
 					range->pixel_clock_mhz * 10;
 
-				connector->display_info.monitor_range.min_vfreq = range->min_vfreq;
-				connector->display_info.monitor_range.max_vfreq = range->max_vfreq;
+				connector->display_info.vrr_range.min_vfreq = range->min_vfreq;
+				connector->display_info.vrr_range.max_vfreq = range->max_vfreq;
 
 				break;
 			}
@@ -9993,8 +9993,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
 			if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
 				freesync_capable = true;
 
-			connector->display_info.monitor_range.min_vfreq = vsdb_info.min_refresh_rate_hz;
-			connector->display_info.monitor_range.max_vfreq = vsdb_info.max_refresh_rate_hz;
+			connector->display_info.vrr_range.min_vfreq = vsdb_info.min_refresh_rate_hz;
+			connector->display_info.vrr_range.max_vfreq = vsdb_info.max_refresh_rate_hz;
 		}
 	}
 
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 01ee3febb813..1437c798b122 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -377,8 +377,8 @@ static int vrr_range_show(struct seq_file *m, void *data)
 	if (connector->status != connector_status_connected)
 		return -ENODEV;
 
-	seq_printf(m, "Min: %u\n", connector->display_info.monitor_range.min_vfreq);
-	seq_printf(m, "Max: %u\n", connector->display_info.monitor_range.max_vfreq);
+	seq_printf(m, "Min: %u\n", connector->display_info.vrr_range.min_vfreq);
+	seq_printf(m, "Max: %u\n", connector->display_info.vrr_range.max_vfreq);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ac662495635c..4355d73632c3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6020,11 +6020,11 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 }
 
 static
-void get_monitor_range(const struct detailed_timing *timing, void *c)
+void get_vrr_range(const struct detailed_timing *timing, void *c)
 {
 	struct detailed_mode_closure *closure = c;
 	struct drm_display_info *info = &closure->connector->display_info;
-	struct drm_monitor_range_info *monitor_range = &info->monitor_range;
+	struct drm_monitor_range_info *vrr_range = &info->vrr_range;
 	const struct detailed_non_pixel *data = &timing->data.other_data;
 	const struct detailed_data_monitor_range *range = &data->data.range;
 	const struct edid *edid = closure->drm_edid->edid;
@@ -6044,19 +6044,19 @@ void get_monitor_range(const struct detailed_timing *timing, void *c)
 	if (range->flags != DRM_EDID_RANGE_LIMITS_ONLY_FLAG)
 		return;
 
-	monitor_range->min_vfreq = range->min_vfreq;
-	monitor_range->max_vfreq = range->max_vfreq;
+	vrr_range->min_vfreq = range->min_vfreq;
+	vrr_range->max_vfreq = range->max_vfreq;
 
 	if (edid->revision >= 4) {
 		if (data->pad2 & DRM_EDID_RANGE_OFFSET_MIN_VFREQ)
-			monitor_range->min_vfreq += 255;
+			vrr_range->min_vfreq += 255;
 		if (data->pad2 & DRM_EDID_RANGE_OFFSET_MAX_VFREQ)
-			monitor_range->max_vfreq += 255;
+			vrr_range->max_vfreq += 255;
 	}
 }
 
-static void drm_get_monitor_range(struct drm_connector *connector,
-				  const struct drm_edid *drm_edid)
+static void drm_get_vrr_range(struct drm_connector *connector,
+			      const struct drm_edid *drm_edid)
 {
 	const struct drm_display_info *info = &connector->display_info;
 	struct detailed_mode_closure closure = {
@@ -6067,11 +6067,11 @@ static void drm_get_monitor_range(struct drm_connector *connector,
 	if (!version_greater(drm_edid, 1, 1))
 		return;
 
-	drm_for_each_detailed_block(drm_edid, get_monitor_range, &closure);
+	drm_for_each_detailed_block(drm_edid, get_vrr_range, &closure);
 
 	DRM_DEBUG_KMS("Supported Monitor Refresh rate range is %d Hz - %d Hz\n",
-		      info->monitor_range.min_vfreq,
-		      info->monitor_range.max_vfreq);
+		      info->vrr_range.min_vfreq,
+		      info->vrr_range.max_vfreq);
 }
 
 static void drm_parse_vesa_mso_data(struct drm_connector *connector,
@@ -6164,7 +6164,7 @@ static void drm_reset_display_info(struct drm_connector *connector)
 	info->edid_hdmi_ycbcr444_dc_modes = 0;
 
 	info->non_desktop = 0;
-	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
+	memset(&info->vrr_range, 0, sizeof(info->vrr_range));
 	memset(&info->luminance_range, 0, sizeof(info->luminance_range));
 
 	info->mso_stream_count = 0;
@@ -6184,7 +6184,7 @@ static u32 update_display_info(struct drm_connector *connector,
 	info->width_mm = edid->width_cm * 10;
 	info->height_mm = edid->height_cm * 10;
 
-	drm_get_monitor_range(connector, drm_edid);
+	drm_get_vrr_range(connector, drm_edid);
 
 	if (edid->revision < 3)
 		goto out;
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 04250a0fec3c..15bc9b9f2b27 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -38,7 +38,7 @@ bool intel_vrr_is_capable(struct intel_connector *connector)
 	}
 
 	return HAS_VRR(i915) &&
-		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
+		info->vrr_range.max_vfreq - info->vrr_range.min_vfreq > 10;
 }
 
 void
@@ -117,9 +117,9 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 		return;
 
 	vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
-			    adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
+			    adjusted_mode->crtc_htotal * info->vrr_range.max_vfreq);
 	vmax = adjusted_mode->crtc_clock * 1000 /
-		(adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
+		(adjusted_mode->crtc_htotal * info->vrr_range.min_vfreq);
 
 	vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
 	vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 56aee949c6fa..7ae23d691cd6 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -636,9 +636,9 @@ struct drm_display_info {
 	bool non_desktop;
 
 	/**
-	 * @monitor_range: Frequency range supported by monitor range descriptor
+	 * @vrr_range: Refresh rate range supported by monitor for VRR
 	 */
-	struct drm_monitor_range_info monitor_range;
+	struct drm_monitor_range_info vrr_range;
 
 	/**
 	 * @luminance_range: Luminance range supported by panel
-- 
2.35.1

Re: [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/

[Navare, Manasi]

On Sat, Aug 27, 2022 at 12:34:53AM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rename info->monitor_range to info->vrr_range to actually
> reflect its usage.

Okay makes sense.

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 ++++-----
>  drivers/gpu/drm/drm_debugfs.c                 |  4 +--
>  drivers/gpu/drm/drm_edid.c                    | 26 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_vrr.c      |  6 ++---
>  include/drm/drm_connector.h                   |  4 +--
>  5 files changed, 26 insertions(+), 26 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 e702f0d72d53..928b5b6541db 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9921,8 +9921,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  		amdgpu_dm_connector->min_vfreq = 0;
>  		amdgpu_dm_connector->max_vfreq = 0;
>  		amdgpu_dm_connector->pixel_clock_mhz = 0;
> -		connector->display_info.monitor_range.min_vfreq = 0;
> -		connector->display_info.monitor_range.max_vfreq = 0;
> +		connector->display_info.vrr_range.min_vfreq = 0;
> +		connector->display_info.vrr_range.max_vfreq = 0;
>  		freesync_capable = false;
>  
>  		goto update;
> @@ -9970,8 +9970,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  				amdgpu_dm_connector->pixel_clock_mhz =
>  					range->pixel_clock_mhz * 10;
>  
> -				connector->display_info.monitor_range.min_vfreq = range->min_vfreq;
> -				connector->display_info.monitor_range.max_vfreq = range->max_vfreq;
> +				connector->display_info.vrr_range.min_vfreq = range->min_vfreq;
> +				connector->display_info.vrr_range.max_vfreq = range->max_vfreq;
>  
>  				break;
>  			}
> @@ -9993,8 +9993,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  			if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
>  				freesync_capable = true;
>  
> -			connector->display_info.monitor_range.min_vfreq = vsdb_info.min_refresh_rate_hz;
> -			connector->display_info.monitor_range.max_vfreq = vsdb_info.max_refresh_rate_hz;
> +			connector->display_info.vrr_range.min_vfreq = vsdb_info.min_refresh_rate_hz;
> +			connector->display_info.vrr_range.max_vfreq = vsdb_info.max_refresh_rate_hz;
>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 01ee3febb813..1437c798b122 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -377,8 +377,8 @@ static int vrr_range_show(struct seq_file *m, void *data)
>  	if (connector->status != connector_status_connected)
>  		return -ENODEV;
>  
> -	seq_printf(m, "Min: %u\n", connector->display_info.monitor_range.min_vfreq);
> -	seq_printf(m, "Max: %u\n", connector->display_info.monitor_range.max_vfreq);
> +	seq_printf(m, "Min: %u\n", connector->display_info.vrr_range.min_vfreq);
> +	seq_printf(m, "Max: %u\n", connector->display_info.vrr_range.max_vfreq);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ac662495635c..4355d73632c3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6020,11 +6020,11 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>  }
>  
>  static
> -void get_monitor_range(const struct detailed_timing *timing, void *c)
> +void get_vrr_range(const struct detailed_timing *timing, void *c)
>  {
>  	struct detailed_mode_closure *closure = c;
>  	struct drm_display_info *info = &closure->connector->display_info;
> -	struct drm_monitor_range_info *monitor_range = &info->monitor_range;
> +	struct drm_monitor_range_info *vrr_range = &info->vrr_range;
>  	const struct detailed_non_pixel *data = &timing->data.other_data;
>  	const struct detailed_data_monitor_range *range = &data->data.range;
>  	const struct edid *edid = closure->drm_edid->edid;
> @@ -6044,19 +6044,19 @@ void get_monitor_range(const struct detailed_timing *timing, void *c)
>  	if (range->flags != DRM_EDID_RANGE_LIMITS_ONLY_FLAG)
>  		return;
>  
> -	monitor_range->min_vfreq = range->min_vfreq;
> -	monitor_range->max_vfreq = range->max_vfreq;
> +	vrr_range->min_vfreq = range->min_vfreq;
> +	vrr_range->max_vfreq = range->max_vfreq;
>  
>  	if (edid->revision >= 4) {
>  		if (data->pad2 & DRM_EDID_RANGE_OFFSET_MIN_VFREQ)
> -			monitor_range->min_vfreq += 255;
> +			vrr_range->min_vfreq += 255;
>  		if (data->pad2 & DRM_EDID_RANGE_OFFSET_MAX_VFREQ)
> -			monitor_range->max_vfreq += 255;
> +			vrr_range->max_vfreq += 255;
>  	}
>  }
>  
> -static void drm_get_monitor_range(struct drm_connector *connector,
> -				  const struct drm_edid *drm_edid)
> +static void drm_get_vrr_range(struct drm_connector *connector,
> +			      const struct drm_edid *drm_edid)
>  {
>  	const struct drm_display_info *info = &connector->display_info;
>  	struct detailed_mode_closure closure = {
> @@ -6067,11 +6067,11 @@ static void drm_get_monitor_range(struct drm_connector *connector,
>  	if (!version_greater(drm_edid, 1, 1))
>  		return;
>  
> -	drm_for_each_detailed_block(drm_edid, get_monitor_range, &closure);
> +	drm_for_each_detailed_block(drm_edid, get_vrr_range, &closure);
>  
>  	DRM_DEBUG_KMS("Supported Monitor Refresh rate range is %d Hz - %d Hz\n",
> -		      info->monitor_range.min_vfreq,
> -		      info->monitor_range.max_vfreq);
> +		      info->vrr_range.min_vfreq,
> +		      info->vrr_range.max_vfreq);
>  }
>  
>  static void drm_parse_vesa_mso_data(struct drm_connector *connector,
> @@ -6164,7 +6164,7 @@ static void drm_reset_display_info(struct drm_connector *connector)
>  	info->edid_hdmi_ycbcr444_dc_modes = 0;
>  
>  	info->non_desktop = 0;
> -	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
> +	memset(&info->vrr_range, 0, sizeof(info->vrr_range));
>  	memset(&info->luminance_range, 0, sizeof(info->luminance_range));
>  
>  	info->mso_stream_count = 0;
> @@ -6184,7 +6184,7 @@ static u32 update_display_info(struct drm_connector *connector,
>  	info->width_mm = edid->width_cm * 10;
>  	info->height_mm = edid->height_cm * 10;
>  
> -	drm_get_monitor_range(connector, drm_edid);
> +	drm_get_vrr_range(connector, drm_edid);
>  
>  	if (edid->revision < 3)
>  		goto out;
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 04250a0fec3c..15bc9b9f2b27 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -38,7 +38,7 @@ bool intel_vrr_is_capable(struct intel_connector *connector)
>  	}
>  
>  	return HAS_VRR(i915) &&
> -		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> +		info->vrr_range.max_vfreq - info->vrr_range.min_vfreq > 10;
>  }
>  
>  void
> @@ -117,9 +117,9 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  		return;
>  
>  	vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> -			    adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
> +			    adjusted_mode->crtc_htotal * info->vrr_range.max_vfreq);
>  	vmax = adjusted_mode->crtc_clock * 1000 /
> -		(adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
> +		(adjusted_mode->crtc_htotal * info->vrr_range.min_vfreq);
>  
>  	vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
>  	vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 56aee949c6fa..7ae23d691cd6 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -636,9 +636,9 @@ struct drm_display_info {
>  	bool non_desktop;
>  
>  	/**
> -	 * @monitor_range: Frequency range supported by monitor range descriptor
> +	 * @vrr_range: Refresh rate range supported by monitor for VRR
>  	 */
> -	struct drm_monitor_range_info monitor_range;
> +	struct drm_monitor_range_info vrr_range;
>  
>  	/**
>  	 * @luminance_range: Luminance range supported by panel
> -- 
> 2.35.1
> 

Re: [Intel-gfx] [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/

[Jani Nikula]

On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rename info->monitor_range to info->vrr_range to actually
> reflect its usage.
>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 ++++-----
>  drivers/gpu/drm/drm_debugfs.c                 |  4 +--
>  drivers/gpu/drm/drm_edid.c                    | 26 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_vrr.c      |  6 ++---
>  include/drm/drm_connector.h                   |  4 +--
>  5 files changed, 26 insertions(+), 26 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 e702f0d72d53..928b5b6541db 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9921,8 +9921,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  		amdgpu_dm_connector->min_vfreq = 0;
>  		amdgpu_dm_connector->max_vfreq = 0;
>  		amdgpu_dm_connector->pixel_clock_mhz = 0;
> -		connector->display_info.monitor_range.min_vfreq = 0;
> -		connector->display_info.monitor_range.max_vfreq = 0;
> +		connector->display_info.vrr_range.min_vfreq = 0;
> +		connector->display_info.vrr_range.max_vfreq = 0;
>  		freesync_capable = false;
>  
>  		goto update;
> @@ -9970,8 +9970,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  				amdgpu_dm_connector->pixel_clock_mhz =
>  					range->pixel_clock_mhz * 10;
>  
> -				connector->display_info.monitor_range.min_vfreq = range->min_vfreq;
> -				connector->display_info.monitor_range.max_vfreq = range->max_vfreq;
> +				connector->display_info.vrr_range.min_vfreq = range->min_vfreq;
> +				connector->display_info.vrr_range.max_vfreq = range->max_vfreq;
>  
>  				break;
>  			}
> @@ -9993,8 +9993,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  			if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq > 10)
>  				freesync_capable = true;
>  
> -			connector->display_info.monitor_range.min_vfreq = vsdb_info.min_refresh_rate_hz;
> -			connector->display_info.monitor_range.max_vfreq = vsdb_info.max_refresh_rate_hz;
> +			connector->display_info.vrr_range.min_vfreq = vsdb_info.min_refresh_rate_hz;
> +			connector->display_info.vrr_range.max_vfreq = vsdb_info.max_refresh_rate_hz;
>  		}
>  	}

The amdgpu changes really beg the question why they're filling up
display_info themselves instead of relying on drm_edid.c to do that for
them.

Other than that,

Acked-by: Jani Nikula <jani.nikula@intel.com>


>  
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 01ee3febb813..1437c798b122 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -377,8 +377,8 @@ static int vrr_range_show(struct seq_file *m, void *data)
>  	if (connector->status != connector_status_connected)
>  		return -ENODEV;
>  
> -	seq_printf(m, "Min: %u\n", connector->display_info.monitor_range.min_vfreq);
> -	seq_printf(m, "Max: %u\n", connector->display_info.monitor_range.max_vfreq);
> +	seq_printf(m, "Min: %u\n", connector->display_info.vrr_range.min_vfreq);
> +	seq_printf(m, "Max: %u\n", connector->display_info.vrr_range.max_vfreq);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ac662495635c..4355d73632c3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6020,11 +6020,11 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>  }
>  
>  static
> -void get_monitor_range(const struct detailed_timing *timing, void *c)
> +void get_vrr_range(const struct detailed_timing *timing, void *c)
>  {
>  	struct detailed_mode_closure *closure = c;
>  	struct drm_display_info *info = &closure->connector->display_info;
> -	struct drm_monitor_range_info *monitor_range = &info->monitor_range;
> +	struct drm_monitor_range_info *vrr_range = &info->vrr_range;
>  	const struct detailed_non_pixel *data = &timing->data.other_data;
>  	const struct detailed_data_monitor_range *range = &data->data.range;
>  	const struct edid *edid = closure->drm_edid->edid;
> @@ -6044,19 +6044,19 @@ void get_monitor_range(const struct detailed_timing *timing, void *c)
>  	if (range->flags != DRM_EDID_RANGE_LIMITS_ONLY_FLAG)
>  		return;
>  
> -	monitor_range->min_vfreq = range->min_vfreq;
> -	monitor_range->max_vfreq = range->max_vfreq;
> +	vrr_range->min_vfreq = range->min_vfreq;
> +	vrr_range->max_vfreq = range->max_vfreq;
>  
>  	if (edid->revision >= 4) {
>  		if (data->pad2 & DRM_EDID_RANGE_OFFSET_MIN_VFREQ)
> -			monitor_range->min_vfreq += 255;
> +			vrr_range->min_vfreq += 255;
>  		if (data->pad2 & DRM_EDID_RANGE_OFFSET_MAX_VFREQ)
> -			monitor_range->max_vfreq += 255;
> +			vrr_range->max_vfreq += 255;
>  	}
>  }
>  
> -static void drm_get_monitor_range(struct drm_connector *connector,
> -				  const struct drm_edid *drm_edid)
> +static void drm_get_vrr_range(struct drm_connector *connector,
> +			      const struct drm_edid *drm_edid)
>  {
>  	const struct drm_display_info *info = &connector->display_info;
>  	struct detailed_mode_closure closure = {
> @@ -6067,11 +6067,11 @@ static void drm_get_monitor_range(struct drm_connector *connector,
>  	if (!version_greater(drm_edid, 1, 1))
>  		return;
>  
> -	drm_for_each_detailed_block(drm_edid, get_monitor_range, &closure);
> +	drm_for_each_detailed_block(drm_edid, get_vrr_range, &closure);
>  
>  	DRM_DEBUG_KMS("Supported Monitor Refresh rate range is %d Hz - %d Hz\n",
> -		      info->monitor_range.min_vfreq,
> -		      info->monitor_range.max_vfreq);
> +		      info->vrr_range.min_vfreq,
> +		      info->vrr_range.max_vfreq);
>  }
>  
>  static void drm_parse_vesa_mso_data(struct drm_connector *connector,
> @@ -6164,7 +6164,7 @@ static void drm_reset_display_info(struct drm_connector *connector)
>  	info->edid_hdmi_ycbcr444_dc_modes = 0;
>  
>  	info->non_desktop = 0;
> -	memset(&info->monitor_range, 0, sizeof(info->monitor_range));
> +	memset(&info->vrr_range, 0, sizeof(info->vrr_range));
>  	memset(&info->luminance_range, 0, sizeof(info->luminance_range));
>  
>  	info->mso_stream_count = 0;
> @@ -6184,7 +6184,7 @@ static u32 update_display_info(struct drm_connector *connector,
>  	info->width_mm = edid->width_cm * 10;
>  	info->height_mm = edid->height_cm * 10;
>  
> -	drm_get_monitor_range(connector, drm_edid);
> +	drm_get_vrr_range(connector, drm_edid);
>  
>  	if (edid->revision < 3)
>  		goto out;
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 04250a0fec3c..15bc9b9f2b27 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -38,7 +38,7 @@ bool intel_vrr_is_capable(struct intel_connector *connector)
>  	}
>  
>  	return HAS_VRR(i915) &&
> -		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> +		info->vrr_range.max_vfreq - info->vrr_range.min_vfreq > 10;
>  }
>  
>  void
> @@ -117,9 +117,9 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  		return;
>  
>  	vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> -			    adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
> +			    adjusted_mode->crtc_htotal * info->vrr_range.max_vfreq);
>  	vmax = adjusted_mode->crtc_clock * 1000 /
> -		(adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
> +		(adjusted_mode->crtc_htotal * info->vrr_range.min_vfreq);
>  
>  	vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
>  	vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 56aee949c6fa..7ae23d691cd6 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -636,9 +636,9 @@ struct drm_display_info {
>  	bool non_desktop;
>  
>  	/**
> -	 * @monitor_range: Frequency range supported by monitor range descriptor
> +	 * @vrr_range: Refresh rate range supported by monitor for VRR
>  	 */
> -	struct drm_monitor_range_info monitor_range;
> +	struct drm_monitor_range_info vrr_range;
>  
>  	/**
>  	 * @luminance_range: Luminance range supported by panel

-- 
Jani Nikula, Intel Open Source Graphics Center

[PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since we only use the parsed vrefresh range to determine
if VRR should be supported we should only accept continuous
frequency displays here.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 856d304a1354..b459fdf12b58 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6064,7 +6064,10 @@ static void drm_get_vrr_range(struct drm_connector *connector,
 		.drm_edid = drm_edid,
 	};
 
-	if (!version_greater(drm_edid, 1, 1))
+	if (!version_greater(drm_edid, 1, 3))
+		return;
+
+	if (!(drm_edid->edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ))
 		return;
 
 	drm_for_each_detailed_block(drm_edid, get_vrr_range, &closure);
-- 
2.35.1

Re: [PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays

[Jani Nikula]

On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Since we only use the parsed vrefresh range to determine
> if VRR should be supported we should only accept continuous
> frequency displays here.
>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Makes sense, at least for sane EDIDs. :p

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/drm_edid.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 856d304a1354..b459fdf12b58 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6064,7 +6064,10 @@ static void drm_get_vrr_range(struct drm_connector *connector,
>  		.drm_edid = drm_edid,
>  	};
>  
> -	if (!version_greater(drm_edid, 1, 1))
> +	if (!version_greater(drm_edid, 1, 3))
> +		return;
> +
> +	if (!(drm_edid->edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ))
>  		return;
>  
>  	drm_for_each_detailed_block(drm_edid, get_vrr_range, &closure);

-- 
Jani Nikula, Intel Open Source Graphics Center