* [PATCH 00/11] drm/edid: Range descriptor stuff
@ 2022-08-26 21:34 Ville Syrjala
2022-08-26 21:34 ` [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets Ville Syrjala
` (10 more replies)
0 siblings, 11 replies; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Various improvements (mostly) related to the EDID
range descriptor handling.
Entire series available here:
https://github.com/vsyrjala/linux.git edid_range_descriptor
Ville Syrjälä (11):
drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets
drm/edid: Clarify why we only accept the "range limits only"
descriptor
drm/edid: s/monitor_rage/vrr_range/
drm/edid: Define more flags
drm/edid: Only parse VRR range for continuous frequency displays
drm/edid: Extract drm_gtf2_mode()
drm/edid: Use GTF2 for inferred modes
drm/edid: Use the correct formula for standard timings
drm/edid: Unconfuse preferred timing stuff a bit
drm/edid: Make version checks less convoluted
drm/i915: Infer vrefresh range for eDP if the EDID omits it
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +-
drivers/gpu/drm/drm_debugfs.c | 4 +-
drivers/gpu/drm/drm_edid.c | 231 ++++++++++++------
drivers/gpu/drm/i915/display/intel_dp.c | 45 ++++
drivers/gpu/drm/i915/display/intel_vrr.c | 6 +-
include/drm/drm_connector.h | 8 +-
include/drm/drm_edid.h | 19 +-
7 files changed, 234 insertions(+), 91 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-08-27 1:40 ` Navare, Manasi
2022-08-26 21:34 ` [PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor Ville Syrjala
` (9 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel; +Cc: Jani Nikula, intel-gfx, stable
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
EDID 1.4 introduced some extra flags in the range
descriptor to support min/max h/vfreq >= 255. Consult them
to correctly parse the vfreq limits.
Note that some combinations of the flags are documented
as "reserved" (as are some other values in the descriptor)
but explicitly checking for those doesn't seem particularly
worthwile since we end up with bogus results whether we
decode them or not.
v2: Increase the storage to u16 to make it work (Jani)
Note the "reserved" values situation (Jani)
v3: Document the EDID version number in the defines
Drop some bogus (u8) casts
Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6519
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_debugfs.c | 4 ++--
drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++------
include/drm/drm_connector.h | 4 ++--
include/drm/drm_edid.h | 5 +++++
4 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 493922069c90..01ee3febb813 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", (u8)connector->display_info.monitor_range.min_vfreq);
- seq_printf(m, "Max: %u\n", (u8)connector->display_info.monitor_range.max_vfreq);
+ 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);
return 0;
}
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 90a5e26eafa8..4005dab6147d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6020,12 +6020,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
}
static
-void get_monitor_range(const struct detailed_timing *timing,
- void *info_monitor_range)
+void get_monitor_range(const struct detailed_timing *timing, void *c)
{
- struct drm_monitor_range_info *monitor_range = info_monitor_range;
+ 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;
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;
if (!is_display_descriptor(timing, EDID_DETAIL_MONITOR_RANGE))
return;
@@ -6041,18 +6043,28 @@ void get_monitor_range(const struct detailed_timing *timing,
monitor_range->min_vfreq = range->min_vfreq;
monitor_range->max_vfreq = range->max_vfreq;
+
+ if (edid->revision >= 4) {
+ if (data->pad2 & DRM_EDID_RANGE_OFFSET_MIN_VFREQ)
+ monitor_range->min_vfreq += 255;
+ if (data->pad2 & DRM_EDID_RANGE_OFFSET_MAX_VFREQ)
+ monitor_range->max_vfreq += 255;
+ }
}
static void drm_get_monitor_range(struct drm_connector *connector,
const struct drm_edid *drm_edid)
{
- struct drm_display_info *info = &connector->display_info;
+ const struct drm_display_info *info = &connector->display_info;
+ struct detailed_mode_closure closure = {
+ .connector = connector,
+ .drm_edid = drm_edid,
+ };
if (!version_greater(drm_edid, 1, 1))
return;
- drm_for_each_detailed_block(drm_edid, get_monitor_range,
- &info->monitor_range);
+ drm_for_each_detailed_block(drm_edid, get_monitor_range, &closure);
DRM_DEBUG_KMS("Supported Monitor Refresh rate range is %d Hz - %d Hz\n",
info->monitor_range.min_vfreq,
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 248206bbd975..56aee949c6fa 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -319,8 +319,8 @@ enum drm_panel_orientation {
* EDID's detailed monitor range
*/
struct drm_monitor_range_info {
- u8 min_vfreq;
- u8 max_vfreq;
+ u16 min_vfreq;
+ u16 max_vfreq;
};
/**
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 2181977ae683..1ed61e2b30a4 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -92,6 +92,11 @@ struct detailed_data_string {
u8 str[13];
} __attribute__((packed));
+#define DRM_EDID_RANGE_OFFSET_MIN_VFREQ (1 << 0) /* 1.4 */
+#define DRM_EDID_RANGE_OFFSET_MAX_VFREQ (1 << 1) /* 1.4 */
+#define DRM_EDID_RANGE_OFFSET_MIN_HFREQ (1 << 2) /* 1.4 */
+#define DRM_EDID_RANGE_OFFSET_MAX_HFREQ (1 << 3) /* 1.4 */
+
#define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00
#define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01
#define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
2022-08-26 21:34 ` [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-08-27 1:45 ` Navare, Manasi
2022-08-26 21:34 ` [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/ Ville Syrjala
` (8 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel
Cc: Leo Li, intel-gfx, Rodrigo Siqueira, amd-gfx, Manasi Navare,
Nicholas Kazlauskas
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
2022-08-26 21:34 ` [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets Ville Syrjala
2022-08-26 21:34 ` [PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-08-27 1:47 ` Navare, Manasi
2022-08-29 8:29 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 04/11] drm/edid: Define more flags Ville Syrjala
` (7 subsequent siblings)
10 siblings, 2 replies; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel
Cc: Leo Li, intel-gfx, Rodrigo Siqueira, amd-gfx, Manasi Navare,
Nicholas Kazlauskas
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 04/11] drm/edid: Define more flags
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (2 preceding siblings ...)
2022-08-26 21:34 ` [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/ Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-08-29 8:39 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays Ville Syrjala
` (6 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Replace a bunch of hex constants with proper definitions.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 18 +++++++++---------
include/drm/drm_edid.h | 14 +++++++++-----
2 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 4355d73632c3..856d304a1354 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2984,7 +2984,7 @@ is_rb(const struct detailed_timing *descriptor, void *data)
BUILD_BUG_ON(offsetof(typeof(*descriptor), data.other_data.data.range.formula.cvt.flags) != 15);
if (descriptor->data.other_data.data.range.flags == DRM_EDID_CVT_SUPPORT_FLAG &&
- descriptor->data.other_data.data.range.formula.cvt.flags & 0x10)
+ descriptor->data.other_data.data.range.formula.cvt.flags & DRM_EDID_CVT_FLAGS_REDUCED_BLANKING)
*res = true;
}
@@ -3012,7 +3012,7 @@ find_gtf2(const struct detailed_timing *descriptor, void *data)
BUILD_BUG_ON(offsetof(typeof(*descriptor), data.other_data.data.range.flags) != 10);
- if (descriptor->data.other_data.data.range.flags == 0x02)
+ if (descriptor->data.other_data.data.range.flags == DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG)
*res = descriptor;
}
@@ -3415,7 +3415,7 @@ range_pixel_clock(const struct edid *edid, const u8 *t)
return 0;
/* 1.4 with CVT support gives us real precision, yay */
- if (edid->revision >= 4 && t[10] == 0x04)
+ if (edid->revision >= 4 && t[10] == DRM_EDID_CVT_SUPPORT_FLAG)
return (t[9] * 10000) - ((t[12] >> 2) * 250);
/* 1.3 is pathetic, so fuzz up a bit */
@@ -3441,7 +3441,7 @@ static bool mode_in_range(const struct drm_display_mode *mode,
return false;
/* 1.4 max horizontal check */
- if (edid->revision >= 4 && t[10] == 0x04)
+ if (edid->revision >= 4 && t[10] == DRM_EDID_CVT_SUPPORT_FLAG)
if (t[13] && mode->hdisplay > 8 * (t[13] + (256 * (t[12]&0x3))))
return false;
@@ -3581,13 +3581,13 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
return; /* GTF not defined yet */
switch (range->flags) {
- case 0x02: /* secondary gtf, XXX could do more */
- case 0x00: /* default gtf */
+ case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
+ case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
closure->modes += drm_gtf_modes_for_range(closure->connector,
closure->drm_edid,
timing);
break;
- case 0x04: /* cvt, only in 1.4+ */
+ case DRM_EDID_CVT_SUPPORT_FLAG:
if (!version_greater(closure->drm_edid, 1, 3))
break;
@@ -3595,7 +3595,7 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
closure->drm_edid,
timing);
break;
- case 0x01: /* just the ranges, no formula */
+ case DRM_EDID_RANGE_LIMITS_ONLY_FLAG:
default:
break;
}
@@ -6393,7 +6393,7 @@ static int _drm_edid_connector_update(struct drm_connector *connector,
num_modes += add_cea_modes(connector, drm_edid);
num_modes += add_alternate_cea_modes(connector, drm_edid);
num_modes += add_displayid_detailed_modes(connector, drm_edid);
- if (drm_edid->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
+ if (drm_edid->edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ)
num_modes += add_inferred_modes(connector, drm_edid);
if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 1ed61e2b30a4..429735b91f63 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -97,10 +97,13 @@ struct detailed_data_string {
#define DRM_EDID_RANGE_OFFSET_MIN_HFREQ (1 << 2) /* 1.4 */
#define DRM_EDID_RANGE_OFFSET_MAX_HFREQ (1 << 3) /* 1.4 */
-#define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00
-#define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01
-#define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02
-#define DRM_EDID_CVT_SUPPORT_FLAG 0x04
+#define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00 /* 1.3 */
+#define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01 /* 1.4 */
+#define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02 /* 1.3 */
+#define DRM_EDID_CVT_SUPPORT_FLAG 0x04 /* 1.4 */
+
+#define DRM_EDID_CVT_FLAGS_STANDARD_BLANKING (1 << 3)
+#define DRM_EDID_CVT_FLAGS_REDUCED_BLANKING (1 << 4)
struct detailed_data_monitor_range {
u8 min_vfreq;
@@ -206,7 +209,8 @@ struct detailed_timing {
#define DRM_EDID_DIGITAL_TYPE_DP (5 << 0) /* 1.4 */
#define DRM_EDID_DIGITAL_DFP_1_X (1 << 0) /* 1.3 */
-#define DRM_EDID_FEATURE_DEFAULT_GTF (1 << 0)
+#define DRM_EDID_FEATURE_DEFAULT_GTF (1 << 0) /* 1.2 */
+#define DRM_EDID_FEATURE_CONTINUOUS_FREQ (1 << 0) /* 1.4 */
#define DRM_EDID_FEATURE_PREFERRED_TIMING (1 << 1)
#define DRM_EDID_FEATURE_STANDARD_COLOR (1 << 2)
/* If analog */
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (3 preceding siblings ...)
2022-08-26 21:34 ` [PATCH 04/11] drm/edid: Define more flags Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-08-29 8:58 ` Jani Nikula
2022-08-26 21:34 ` [PATCH 06/11] drm/edid: Extract drm_gtf2_mode() Ville Syrjala
` (5 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel
Cc: Leo Li, intel-gfx, Rodrigo Siqueira, amd-gfx, Manasi Navare,
Nicholas Kazlauskas
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 06/11] drm/edid: Extract drm_gtf2_mode()
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (4 preceding siblings ...)
2022-08-26 21:34 ` [PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-08-29 8:45 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 07/11] drm/edid: Use GTF2 for inferred modes Ville Syrjala
` (4 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Extract the GTF vs. GTF2 logic into a separate function.
We'll have a second user soon.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 47 ++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b459fdf12b58..0c7cbe9b44f5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3113,6 +3113,35 @@ static int drm_mode_hsync(const struct drm_display_mode *mode)
return DIV_ROUND_CLOSEST(mode->clock, mode->htotal);
}
+static struct drm_display_mode *
+drm_gtf2_mode(struct drm_device *dev,
+ const struct drm_edid *drm_edid,
+ int hsize, int vsize, int vrefresh_rate)
+{
+ struct drm_display_mode *mode;
+
+ /*
+ * This is potentially wrong if there's ever a monitor with
+ * more than one ranges section, each claiming a different
+ * secondary GTF curve. Please don't do that.
+ */
+ mode = drm_gtf_mode(dev, hsize, vsize, vrefresh_rate, 0, 0);
+ if (!mode)
+ return NULL;
+
+ if (drm_mode_hsync(mode) > drm_gtf2_hbreak(drm_edid)) {
+ drm_mode_destroy(dev, mode);
+ mode = drm_gtf_mode_complex(dev, hsize, vsize,
+ vrefresh_rate, 0, 0,
+ drm_gtf2_m(drm_edid),
+ drm_gtf2_2c(drm_edid),
+ drm_gtf2_k(drm_edid),
+ drm_gtf2_2j(drm_edid));
+ }
+
+ return mode;
+}
+
/*
* Take the standard timing params (in this case width, aspect, and refresh)
* and convert them into a real mode using CVT/GTF/DMT.
@@ -3201,23 +3230,7 @@ static struct drm_display_mode *drm_mode_std(struct drm_connector *connector,
mode = drm_gtf_mode(dev, hsize, vsize, vrefresh_rate, 0, 0);
break;
case LEVEL_GTF2:
- /*
- * This is potentially wrong if there's ever a monitor with
- * more than one ranges section, each claiming a different
- * secondary GTF curve. Please don't do that.
- */
- mode = drm_gtf_mode(dev, hsize, vsize, vrefresh_rate, 0, 0);
- if (!mode)
- return NULL;
- if (drm_mode_hsync(mode) > drm_gtf2_hbreak(drm_edid)) {
- drm_mode_destroy(dev, mode);
- mode = drm_gtf_mode_complex(dev, hsize, vsize,
- vrefresh_rate, 0, 0,
- drm_gtf2_m(drm_edid),
- drm_gtf2_2c(drm_edid),
- drm_gtf2_k(drm_edid),
- drm_gtf2_2j(drm_edid));
- }
+ mode = drm_gtf2_mode(dev, drm_edid, hsize, vsize, vrefresh_rate);
break;
case LEVEL_CVT:
mode = drm_cvt_mode(dev, hsize, vsize, vrefresh_rate, 0, 0,
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 07/11] drm/edid: Use GTF2 for inferred modes
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (5 preceding siblings ...)
2022-08-26 21:34 ` [PATCH 06/11] drm/edid: Extract drm_gtf2_mode() Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-09-02 12:25 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 08/11] drm/edid: Use the correct formula for standard timings Ville Syrjala
` (3 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
For some resaon we only use the secondary GTF curve for the
standard timings. Use it for inferred modes as well.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0c7cbe9b44f5..fed2bdd55c09 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3546,6 +3546,35 @@ static int drm_gtf_modes_for_range(struct drm_connector *connector,
return modes;
}
+static int drm_gtf2_modes_for_range(struct drm_connector *connector,
+ const struct drm_edid *drm_edid,
+ const struct detailed_timing *timing)
+{
+ int i, modes = 0;
+ struct drm_display_mode *newmode;
+ struct drm_device *dev = connector->dev;
+
+ for (i = 0; i < ARRAY_SIZE(extra_modes); i++) {
+ const struct minimode *m = &extra_modes[i];
+
+ newmode = drm_gtf2_mode(dev, drm_edid, m->w, m->h, m->r);
+ if (!newmode)
+ return modes;
+
+ drm_mode_fixup_1366x768(newmode);
+ if (!mode_in_range(newmode, drm_edid, timing) ||
+ !valid_inferred_mode(connector, newmode)) {
+ drm_mode_destroy(dev, newmode);
+ continue;
+ }
+
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+
+ return modes;
+}
+
static int drm_cvt_modes_for_range(struct drm_connector *connector,
const struct drm_edid *drm_edid,
const struct detailed_timing *timing)
@@ -3594,7 +3623,11 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
return; /* GTF not defined yet */
switch (range->flags) {
- case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
+ case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
+ closure->modes += drm_gtf2_modes_for_range(closure->connector,
+ closure->drm_edid,
+ timing);
+ break;
case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
closure->modes += drm_gtf_modes_for_range(closure->connector,
closure->drm_edid,
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 08/11] drm/edid: Use the correct formula for standard timings
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (6 preceding siblings ...)
2022-08-26 21:34 ` [PATCH 07/11] drm/edid: Use GTF2 for inferred modes Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-09-02 13:41 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 09/11] drm/edid: Unconfuse preferred timing stuff a bit Ville Syrjala
` (2 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Prefer the timing formula indicated by the range
descriptor for generating the non-DMT standard timings.
Previously we just used CVT for all EDID 1.4 continuous
frequency displays without even checking if the range
descriptor indicates otherwise. Now we check the range
descriptor first, and fall back to CVT if nothing else
was indicated. EDID 1.4 more or less deprecates GTF/GTF2
but there are still a lot of 1.4 EDIDs out there that
don't advertise CVT support, so seems safer to use the
formula the EDID actually reports as supported.
For EDID 1.3 we use GTF2 if indicated (as before), and for
EDID 1.2+ we now just use GTF without even checking the
feature flag. There seem to be quite a few EDIDs out there that
don't set the GTF feature flag but still include a GTF range
descriptor and non-DMT standard timings.
This to me seems to be roughly what appendix B of EDID 1.4
suggests should be done.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 49 +++++++++++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fed2bdd55c09..c1c85b9e1208 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3077,20 +3077,53 @@ drm_gtf2_2j(const struct drm_edid *drm_edid)
return descriptor ? descriptor->data.other_data.data.range.formula.gtf2.j : 0;
}
+static void
+get_timing_level(const struct detailed_timing *descriptor, void *data)
+{
+ int *res = data;
+
+ if (!is_display_descriptor(descriptor, EDID_DETAIL_MONITOR_RANGE))
+ return;
+
+ BUILD_BUG_ON(offsetof(typeof(*descriptor), data.other_data.data.range.flags) != 10);
+
+ switch (descriptor->data.other_data.data.range.flags) {
+ case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
+ *res = LEVEL_GTF;
+ break;
+ case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
+ *res = LEVEL_GTF2;
+ break;
+ case DRM_EDID_CVT_SUPPORT_FLAG:
+ *res = LEVEL_CVT;
+ break;
+ default:
+ break;
+ }
+}
+
/* Get standard timing level (CVT/GTF/DMT). */
static int standard_timing_level(const struct drm_edid *drm_edid)
{
const struct edid *edid = drm_edid->edid;
- if (edid->revision >= 2) {
- if (edid->revision >= 4 && (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF))
- return LEVEL_CVT;
- if (drm_gtf2_hbreak(drm_edid))
- return LEVEL_GTF2;
- if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
- return LEVEL_GTF;
+ if (edid->revision >= 4) {
+ /*
+ * If the range descriptor doesn't
+ * indicate otherwise default to CVT
+ */
+ int ret = LEVEL_CVT;
+
+ drm_for_each_detailed_block(drm_edid, get_timing_level, &ret);
+
+ return ret;
+ } else if (edid->revision >= 3 && drm_gtf2_hbreak(drm_edid)) {
+ return LEVEL_GTF2;
+ } else if (edid->revision >= 2) {
+ return LEVEL_GTF;
+ } else {
+ return LEVEL_DMT;
}
- return LEVEL_DMT;
}
/*
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 09/11] drm/edid: Unconfuse preferred timing stuff a bit
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (7 preceding siblings ...)
2022-08-26 21:34 ` [PATCH 08/11] drm/edid: Use the correct formula for standard timings Ville Syrjala
@ 2022-08-26 21:34 ` Ville Syrjala
2022-09-02 12:27 ` Jani Nikula
2022-08-26 21:35 ` [PATCH 10/11] drm/edid: Make version checks less convoluted Ville Syrjala
2022-08-26 21:35 ` [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it Ville Syrjala
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:34 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
For EDID 1.4 the first detailed timing is always preferred,
for older EDIDs there was a feature flag to indicate the same.
While correct, the code setting that up is rather confusing.
Restate it in a slightly more straightforward manner.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c1c85b9e1208..0fe06e5fd6e0 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3952,13 +3952,14 @@ static int add_detailed_modes(struct drm_connector *connector,
struct detailed_mode_closure closure = {
.connector = connector,
.drm_edid = drm_edid,
- .preferred = true,
.quirks = quirks,
};
- if (closure.preferred && !version_greater(drm_edid, 1, 3))
+ if (version_greater(drm_edid, 1, 3))
+ closure.preferred = true; /* first detailed timing is always preferred */
+ else
closure.preferred =
- (drm_edid->edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING);
+ drm_edid->edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING;
drm_for_each_detailed_block(drm_edid, do_detailed_mode, &closure);
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 10/11] drm/edid: Make version checks less convoluted
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (8 preceding siblings ...)
2022-08-26 21:34 ` [PATCH 09/11] drm/edid: Unconfuse preferred timing stuff a bit Ville Syrjala
@ 2022-08-26 21:35 ` Ville Syrjala
2022-09-02 12:31 ` Jani Nikula
2022-08-26 21:35 ` [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it Ville Syrjala
10 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:35 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Get rid of the confusing version_greater() stuff and
simply compare edid->revision directly everwhere. Half
the places already did it this way, and since we actually
reject any EDID with edid->version!=1 it's a perfectly
sane thing to do.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0fe06e5fd6e0..e7f46260dfe7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1572,15 +1572,6 @@ struct drm_edid {
const struct edid *edid;
};
-static bool version_greater(const struct drm_edid *drm_edid,
- u8 version, u8 revision)
-{
- const struct edid *edid = drm_edid->edid;
-
- return edid->version > version ||
- (edid->version == version && edid->revision > revision);
-}
-
static int edid_hfeeodb_extension_block_count(const struct edid *edid);
static int edid_hfeeodb_block_count(const struct edid *edid)
@@ -3652,7 +3643,7 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
closure->drm_edid,
timing);
- if (!version_greater(closure->drm_edid, 1, 1))
+ if (closure->drm_edid->edid->revision < 2)
return; /* GTF not defined yet */
switch (range->flags) {
@@ -3667,7 +3658,7 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
timing);
break;
case DRM_EDID_CVT_SUPPORT_FLAG:
- if (!version_greater(closure->drm_edid, 1, 3))
+ if (closure->drm_edid->edid->revision < 4)
break;
closure->modes += drm_cvt_modes_for_range(closure->connector,
@@ -3688,7 +3679,7 @@ static int add_inferred_modes(struct drm_connector *connector,
.drm_edid = drm_edid,
};
- if (version_greater(drm_edid, 1, 0))
+ if (drm_edid->edid->revision >= 1)
drm_for_each_detailed_block(drm_edid, do_inferred_modes, &closure);
return closure.modes;
@@ -3765,7 +3756,7 @@ static int add_established_modes(struct drm_connector *connector,
}
}
- if (version_greater(drm_edid, 1, 0))
+ if (edid->revision >= 1)
drm_for_each_detailed_block(drm_edid, do_established_modes,
&closure);
@@ -3820,7 +3811,7 @@ static int add_standard_modes(struct drm_connector *connector,
}
}
- if (version_greater(drm_edid, 1, 0))
+ if (drm_edid->edid->revision >= 1)
drm_for_each_detailed_block(drm_edid, do_standard_modes,
&closure);
@@ -3900,7 +3891,7 @@ add_cvt_modes(struct drm_connector *connector, const struct drm_edid *drm_edid)
.drm_edid = drm_edid,
};
- if (version_greater(drm_edid, 1, 2))
+ if (drm_edid->edid->revision >= 3)
drm_for_each_detailed_block(drm_edid, do_cvt_mode, &closure);
/* XXX should also look for CVT codes in VTB blocks */
@@ -3955,7 +3946,7 @@ static int add_detailed_modes(struct drm_connector *connector,
.quirks = quirks,
};
- if (version_greater(drm_edid, 1, 3))
+ if (drm_edid->edid->revision >= 4)
closure.preferred = true; /* first detailed timing is always preferred */
else
closure.preferred =
@@ -6144,7 +6135,7 @@ static void drm_get_vrr_range(struct drm_connector *connector,
.drm_edid = drm_edid,
};
- if (!version_greater(drm_edid, 1, 3))
+ if (drm_edid->edid->revision < 4)
return;
if (!(drm_edid->edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ))
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
` (9 preceding siblings ...)
2022-08-26 21:35 ` [PATCH 10/11] drm/edid: Make version checks less convoluted Ville Syrjala
@ 2022-08-26 21:35 ` Ville Syrjala
2022-08-29 8:56 ` [Intel-gfx] " Jani Nikula
2022-08-29 12:02 ` [PATCH v2 " Ville Syrjala
10 siblings, 2 replies; 28+ messages in thread
From: Ville Syrjala @ 2022-08-26 21:35 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
A bunch of machines seem to have eDP panels where the EDID
indicates continuous frequency support but fails to actually
include the range descirptor. This violates the EDID 1.4
spec, but looks like the Windows driver just hacks around
this by just assuming that the panel supports a continuous
refresh rate range that covers all EDID reported modes.
Do the same so that we get VRR support on these machines.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6323
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 45 +++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 8d1559323412..1f3e4824d316 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5207,6 +5207,49 @@ intel_edp_add_properties(struct intel_dp *intel_dp)
fixed_mode->vdisplay);
}
+/*
+ * Some VRR eDP panels violate the EDID spec and neglect
+ * to include the monitor range descriptor in the EDID.
+ * Cook up the VRR refresh rate limits based on the modes
+ * reported by the panel.
+ */
+static void
+intel_edp_infer_vrr_range(struct intel_connector *connector)
+{
+ struct drm_i915_private *i915 = to_i915(connector->base.dev);
+ struct drm_display_info *info = &connector->base.display_info;
+ const struct edid *edid = connector->edid;
+ const struct drm_display_mode *mode;
+
+ if (!HAS_VRR(i915))
+ return;
+
+ if (!edid || edid->revision < 4 ||
+ !(edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ) ||
+ info->vrr_range.min_vfreq || info->vrr_range.max_vfreq)
+ return;
+
+ if (list_empty(&connector->base.probed_modes))
+ return;
+
+ info->vrr_range.min_vfreq = ~0;
+ info->vrr_range.max_vfreq = 0;
+
+ list_for_each_entry(mode, &connector->base.probed_modes, head) {
+ int vrefresh = drm_mode_vrefresh(mode);
+
+ info->vrr_range.min_vfreq = min_t(int, vrefresh,
+ info->vrr_range.min_vfreq);
+ info->vrr_range.max_vfreq = max_t(int, vrefresh,
+ info->vrr_range.max_vfreq);
+ }
+
+ drm_dbg_kms(&i915->drm,
+ "[CONNECTOR:%d:%s] does not report refresh rate range, assuming: %d Hz - %d Hz\n",
+ connector->base.base.id, connector->base.name,
+ info->vrr_range.min_vfreq, info->vrr_range.max_vfreq);
+}
+
static bool intel_edp_init_connector(struct intel_dp *intel_dp,
struct intel_connector *intel_connector)
{
@@ -5271,6 +5314,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
}
intel_connector->edid = edid;
+ intel_edp_infer_vrr_range(intel_connector);
+
intel_bios_init_panel(dev_priv, &intel_connector->panel,
encoder->devdata, IS_ERR(edid) ? NULL : edid);
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets
2022-08-26 21:34 ` [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets Ville Syrjala
@ 2022-08-27 1:40 ` Navare, Manasi
2022-09-02 13:44 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Navare, Manasi @ 2022-08-27 1:40 UTC (permalink / raw)
To: Ville Syrjala; +Cc: Jani Nikula, intel-gfx, stable, dri-devel
On Sat, Aug 27, 2022 at 12:34:51AM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> EDID 1.4 introduced some extra flags in the range
> descriptor to support min/max h/vfreq >= 255. Consult them
> to correctly parse the vfreq limits.
>
> Note that some combinations of the flags are documented
> as "reserved" (as are some other values in the descriptor)
> but explicitly checking for those doesn't seem particularly
> worthwile since we end up with bogus results whether we
> decode them or not.
>
> v2: Increase the storage to u16 to make it work (Jani)
> Note the "reserved" values situation (Jani)
> v3: Document the EDID version number in the defines
> Drop some bogus (u8) casts
>
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6519
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_debugfs.c | 4 ++--
> drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++------
> include/drm/drm_connector.h | 4 ++--
> include/drm/drm_edid.h | 5 +++++
> 4 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 493922069c90..01ee3febb813 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", (u8)connector->display_info.monitor_range.min_vfreq);
> - seq_printf(m, "Max: %u\n", (u8)connector->display_info.monitor_range.max_vfreq);
> + 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);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 90a5e26eafa8..4005dab6147d 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6020,12 +6020,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
> }
>
> static
> -void get_monitor_range(const struct detailed_timing *timing,
> - void *info_monitor_range)
> +void get_monitor_range(const struct detailed_timing *timing, void *c)
> {
> - struct drm_monitor_range_info *monitor_range = info_monitor_range;
> + 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;
> 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;
>
> if (!is_display_descriptor(timing, EDID_DETAIL_MONITOR_RANGE))
> return;
> @@ -6041,18 +6043,28 @@ void get_monitor_range(const struct detailed_timing *timing,
>
> monitor_range->min_vfreq = range->min_vfreq;
> monitor_range->max_vfreq = range->max_vfreq;
> +
> + if (edid->revision >= 4) {
> + if (data->pad2 & DRM_EDID_RANGE_OFFSET_MIN_VFREQ)
> + monitor_range->min_vfreq += 255;
> + if (data->pad2 & DRM_EDID_RANGE_OFFSET_MAX_VFREQ)
> + monitor_range->max_vfreq += 255;
Yes this makes sense. This looks like added for supporting HRR (high
refresh rate) panels.
Do you think we should mention that in the commit message or in the
comment in the code itself?
Other than that looks good to me:
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> + }
> }
>
> static void drm_get_monitor_range(struct drm_connector *connector,
> const struct drm_edid *drm_edid)
> {
> - struct drm_display_info *info = &connector->display_info;
> + const struct drm_display_info *info = &connector->display_info;
> + struct detailed_mode_closure closure = {
> + .connector = connector,
> + .drm_edid = drm_edid,
> + };
>
> if (!version_greater(drm_edid, 1, 1))
> return;
>
> - drm_for_each_detailed_block(drm_edid, get_monitor_range,
> - &info->monitor_range);
> + drm_for_each_detailed_block(drm_edid, get_monitor_range, &closure);
>
> DRM_DEBUG_KMS("Supported Monitor Refresh rate range is %d Hz - %d Hz\n",
> info->monitor_range.min_vfreq,
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 248206bbd975..56aee949c6fa 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -319,8 +319,8 @@ enum drm_panel_orientation {
> * EDID's detailed monitor range
> */
> struct drm_monitor_range_info {
> - u8 min_vfreq;
> - u8 max_vfreq;
> + u16 min_vfreq;
> + u16 max_vfreq;
> };
>
> /**
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 2181977ae683..1ed61e2b30a4 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -92,6 +92,11 @@ struct detailed_data_string {
> u8 str[13];
> } __attribute__((packed));
>
> +#define DRM_EDID_RANGE_OFFSET_MIN_VFREQ (1 << 0) /* 1.4 */
> +#define DRM_EDID_RANGE_OFFSET_MAX_VFREQ (1 << 1) /* 1.4 */
> +#define DRM_EDID_RANGE_OFFSET_MIN_HFREQ (1 << 2) /* 1.4 */
> +#define DRM_EDID_RANGE_OFFSET_MAX_HFREQ (1 << 3) /* 1.4 */
> +
> #define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00
> #define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01
> #define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor
2022-08-26 21:34 ` [PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor Ville Syrjala
@ 2022-08-27 1:45 ` Navare, Manasi
0 siblings, 0 replies; 28+ messages in thread
From: Navare, Manasi @ 2022-08-27 1:45 UTC (permalink / raw)
To: Ville Syrjala
Cc: Leo Li, intel-gfx, Rodrigo Siqueira, dri-devel, amd-gfx,
Nicholas Kazlauskas
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/
2022-08-26 21:34 ` [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/ Ville Syrjala
@ 2022-08-27 1:47 ` Navare, Manasi
2022-08-29 8:29 ` [Intel-gfx] " Jani Nikula
1 sibling, 0 replies; 28+ messages in thread
From: Navare, Manasi @ 2022-08-27 1:47 UTC (permalink / raw)
To: Ville Syrjala
Cc: Leo Li, intel-gfx, Rodrigo Siqueira, dri-devel, amd-gfx,
Nicholas Kazlauskas
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/
2022-08-26 21:34 ` [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/ Ville Syrjala
2022-08-27 1:47 ` Navare, Manasi
@ 2022-08-29 8:29 ` Jani Nikula
1 sibling, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2022-08-29 8:29 UTC (permalink / raw)
To: Ville Syrjala, dri-devel
Cc: Leo Li, intel-gfx, Rodrigo Siqueira, amd-gfx, Nicholas Kazlauskas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 04/11] drm/edid: Define more flags
2022-08-26 21:34 ` [PATCH 04/11] drm/edid: Define more flags Ville Syrjala
@ 2022-08-29 8:39 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2022-08-29 8:39 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace a bunch of hex constants with proper definitions.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 18 +++++++++---------
> include/drm/drm_edid.h | 14 +++++++++-----
> 2 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 4355d73632c3..856d304a1354 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2984,7 +2984,7 @@ is_rb(const struct detailed_timing *descriptor, void *data)
> BUILD_BUG_ON(offsetof(typeof(*descriptor), data.other_data.data.range.formula.cvt.flags) != 15);
>
> if (descriptor->data.other_data.data.range.flags == DRM_EDID_CVT_SUPPORT_FLAG &&
> - descriptor->data.other_data.data.range.formula.cvt.flags & 0x10)
> + descriptor->data.other_data.data.range.formula.cvt.flags & DRM_EDID_CVT_FLAGS_REDUCED_BLANKING)
> *res = true;
> }
>
> @@ -3012,7 +3012,7 @@ find_gtf2(const struct detailed_timing *descriptor, void *data)
>
> BUILD_BUG_ON(offsetof(typeof(*descriptor), data.other_data.data.range.flags) != 10);
>
> - if (descriptor->data.other_data.data.range.flags == 0x02)
> + if (descriptor->data.other_data.data.range.flags == DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG)
> *res = descriptor;
> }
>
> @@ -3415,7 +3415,7 @@ range_pixel_clock(const struct edid *edid, const u8 *t)
> return 0;
>
> /* 1.4 with CVT support gives us real precision, yay */
> - if (edid->revision >= 4 && t[10] == 0x04)
> + if (edid->revision >= 4 && t[10] == DRM_EDID_CVT_SUPPORT_FLAG)
> return (t[9] * 10000) - ((t[12] >> 2) * 250);
>
> /* 1.3 is pathetic, so fuzz up a bit */
> @@ -3441,7 +3441,7 @@ static bool mode_in_range(const struct drm_display_mode *mode,
> return false;
>
> /* 1.4 max horizontal check */
> - if (edid->revision >= 4 && t[10] == 0x04)
> + if (edid->revision >= 4 && t[10] == DRM_EDID_CVT_SUPPORT_FLAG)
> if (t[13] && mode->hdisplay > 8 * (t[13] + (256 * (t[12]&0x3))))
> return false;
>
> @@ -3581,13 +3581,13 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
> return; /* GTF not defined yet */
>
> switch (range->flags) {
> - case 0x02: /* secondary gtf, XXX could do more */
> - case 0x00: /* default gtf */
> + case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
> + case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
> closure->modes += drm_gtf_modes_for_range(closure->connector,
> closure->drm_edid,
> timing);
> break;
> - case 0x04: /* cvt, only in 1.4+ */
> + case DRM_EDID_CVT_SUPPORT_FLAG:
> if (!version_greater(closure->drm_edid, 1, 3))
> break;
>
> @@ -3595,7 +3595,7 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
> closure->drm_edid,
> timing);
> break;
> - case 0x01: /* just the ranges, no formula */
> + case DRM_EDID_RANGE_LIMITS_ONLY_FLAG:
> default:
> break;
> }
> @@ -6393,7 +6393,7 @@ static int _drm_edid_connector_update(struct drm_connector *connector,
> num_modes += add_cea_modes(connector, drm_edid);
> num_modes += add_alternate_cea_modes(connector, drm_edid);
> num_modes += add_displayid_detailed_modes(connector, drm_edid);
> - if (drm_edid->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
> + if (drm_edid->edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ)
> num_modes += add_inferred_modes(connector, drm_edid);
>
> if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75))
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 1ed61e2b30a4..429735b91f63 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -97,10 +97,13 @@ struct detailed_data_string {
> #define DRM_EDID_RANGE_OFFSET_MIN_HFREQ (1 << 2) /* 1.4 */
> #define DRM_EDID_RANGE_OFFSET_MAX_HFREQ (1 << 3) /* 1.4 */
>
> -#define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00
> -#define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01
> -#define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02
> -#define DRM_EDID_CVT_SUPPORT_FLAG 0x04
> +#define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00 /* 1.3 */
> +#define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01 /* 1.4 */
> +#define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02 /* 1.3 */
> +#define DRM_EDID_CVT_SUPPORT_FLAG 0x04 /* 1.4 */
> +
> +#define DRM_EDID_CVT_FLAGS_STANDARD_BLANKING (1 << 3)
> +#define DRM_EDID_CVT_FLAGS_REDUCED_BLANKING (1 << 4)
>
> struct detailed_data_monitor_range {
> u8 min_vfreq;
> @@ -206,7 +209,8 @@ struct detailed_timing {
> #define DRM_EDID_DIGITAL_TYPE_DP (5 << 0) /* 1.4 */
> #define DRM_EDID_DIGITAL_DFP_1_X (1 << 0) /* 1.3 */
>
> -#define DRM_EDID_FEATURE_DEFAULT_GTF (1 << 0)
> +#define DRM_EDID_FEATURE_DEFAULT_GTF (1 << 0) /* 1.2 */
> +#define DRM_EDID_FEATURE_CONTINUOUS_FREQ (1 << 0) /* 1.4 */
> #define DRM_EDID_FEATURE_PREFERRED_TIMING (1 << 1)
> #define DRM_EDID_FEATURE_STANDARD_COLOR (1 << 2)
> /* If analog */
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 06/11] drm/edid: Extract drm_gtf2_mode()
2022-08-26 21:34 ` [PATCH 06/11] drm/edid: Extract drm_gtf2_mode() Ville Syrjala
@ 2022-08-29 8:45 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2022-08-29 8:45 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract the GTF vs. GTF2 logic into a separate function.
> We'll have a second user soon.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 47 ++++++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b459fdf12b58..0c7cbe9b44f5 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3113,6 +3113,35 @@ static int drm_mode_hsync(const struct drm_display_mode *mode)
> return DIV_ROUND_CLOSEST(mode->clock, mode->htotal);
> }
>
> +static struct drm_display_mode *
> +drm_gtf2_mode(struct drm_device *dev,
> + const struct drm_edid *drm_edid,
> + int hsize, int vsize, int vrefresh_rate)
> +{
> + struct drm_display_mode *mode;
> +
> + /*
> + * This is potentially wrong if there's ever a monitor with
> + * more than one ranges section, each claiming a different
> + * secondary GTF curve. Please don't do that.
> + */
> + mode = drm_gtf_mode(dev, hsize, vsize, vrefresh_rate, 0, 0);
> + if (!mode)
> + return NULL;
> +
> + if (drm_mode_hsync(mode) > drm_gtf2_hbreak(drm_edid)) {
> + drm_mode_destroy(dev, mode);
> + mode = drm_gtf_mode_complex(dev, hsize, vsize,
> + vrefresh_rate, 0, 0,
> + drm_gtf2_m(drm_edid),
> + drm_gtf2_2c(drm_edid),
> + drm_gtf2_k(drm_edid),
> + drm_gtf2_2j(drm_edid));
> + }
> +
> + return mode;
> +}
> +
> /*
> * Take the standard timing params (in this case width, aspect, and refresh)
> * and convert them into a real mode using CVT/GTF/DMT.
> @@ -3201,23 +3230,7 @@ static struct drm_display_mode *drm_mode_std(struct drm_connector *connector,
> mode = drm_gtf_mode(dev, hsize, vsize, vrefresh_rate, 0, 0);
> break;
> case LEVEL_GTF2:
> - /*
> - * This is potentially wrong if there's ever a monitor with
> - * more than one ranges section, each claiming a different
> - * secondary GTF curve. Please don't do that.
> - */
> - mode = drm_gtf_mode(dev, hsize, vsize, vrefresh_rate, 0, 0);
> - if (!mode)
> - return NULL;
> - if (drm_mode_hsync(mode) > drm_gtf2_hbreak(drm_edid)) {
> - drm_mode_destroy(dev, mode);
> - mode = drm_gtf_mode_complex(dev, hsize, vsize,
> - vrefresh_rate, 0, 0,
> - drm_gtf2_m(drm_edid),
> - drm_gtf2_2c(drm_edid),
> - drm_gtf2_k(drm_edid),
> - drm_gtf2_2j(drm_edid));
> - }
> + mode = drm_gtf2_mode(dev, drm_edid, hsize, vsize, vrefresh_rate);
> break;
> case LEVEL_CVT:
> mode = drm_cvt_mode(dev, hsize, vsize, vrefresh_rate, 0, 0,
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it
2022-08-26 21:35 ` [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it Ville Syrjala
@ 2022-08-29 8:56 ` Jani Nikula
2022-08-29 12:02 ` [PATCH v2 " Ville Syrjala
1 sibling, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2022-08-29 8:56 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> A bunch of machines seem to have eDP panels where the EDID
> indicates continuous frequency support but fails to actually
> include the range descirptor. This violates the EDID 1.4
> spec, but looks like the Windows driver just hacks around
> this by just assuming that the panel supports a continuous
> refresh rate range that covers all EDID reported modes.
>
> Do the same so that we get VRR support on these machines.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6323
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 45 +++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8d1559323412..1f3e4824d316 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5207,6 +5207,49 @@ intel_edp_add_properties(struct intel_dp *intel_dp)
> fixed_mode->vdisplay);
> }
>
> +/*
> + * Some VRR eDP panels violate the EDID spec and neglect
> + * to include the monitor range descriptor in the EDID.
> + * Cook up the VRR refresh rate limits based on the modes
> + * reported by the panel.
> + */
> +static void
> +intel_edp_infer_vrr_range(struct intel_connector *connector)
> +{
> + struct drm_i915_private *i915 = to_i915(connector->base.dev);
> + struct drm_display_info *info = &connector->base.display_info;
> + const struct edid *edid = connector->edid;
> + const struct drm_display_mode *mode;
> +
> + if (!HAS_VRR(i915))
> + return;
> +
> + if (!edid || edid->revision < 4 ||
connector->edid is actually an error pointer. I made it so way back when
to differentiate between "broken edid" -EINVAL and "no edid" -ENOENT,
but I don't think we ever use that distinction anywhere.
Either ditch the error pointers (in a separate patch) or check for them
here. With that fixed,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> + !(edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ) ||
> + info->vrr_range.min_vfreq || info->vrr_range.max_vfreq)
> + return;
> +
> + if (list_empty(&connector->base.probed_modes))
> + return;
> +
> + info->vrr_range.min_vfreq = ~0;
> + info->vrr_range.max_vfreq = 0;
> +
> + list_for_each_entry(mode, &connector->base.probed_modes, head) {
> + int vrefresh = drm_mode_vrefresh(mode);
> +
> + info->vrr_range.min_vfreq = min_t(int, vrefresh,
> + info->vrr_range.min_vfreq);
> + info->vrr_range.max_vfreq = max_t(int, vrefresh,
> + info->vrr_range.max_vfreq);
> + }
> +
> + drm_dbg_kms(&i915->drm,
> + "[CONNECTOR:%d:%s] does not report refresh rate range, assuming: %d Hz - %d Hz\n",
> + connector->base.base.id, connector->base.name,
> + info->vrr_range.min_vfreq, info->vrr_range.max_vfreq);
> +}
> +
> static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> struct intel_connector *intel_connector)
> {
> @@ -5271,6 +5314,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> }
> intel_connector->edid = edid;
>
> + intel_edp_infer_vrr_range(intel_connector);
> +
> intel_bios_init_panel(dev_priv, &intel_connector->panel,
> encoder->devdata, IS_ERR(edid) ? NULL : edid);
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays
2022-08-26 21:34 ` [PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays Ville Syrjala
@ 2022-08-29 8:58 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2022-08-29 8:58 UTC (permalink / raw)
To: Ville Syrjala, dri-devel
Cc: Leo Li, intel-gfx, Rodrigo Siqueira, amd-gfx, Manasi Navare,
Nicholas Kazlauskas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it
2022-08-26 21:35 ` [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it Ville Syrjala
2022-08-29 8:56 ` [Intel-gfx] " Jani Nikula
@ 2022-08-29 12:02 ` Ville Syrjala
1 sibling, 0 replies; 28+ messages in thread
From: Ville Syrjala @ 2022-08-29 12:02 UTC (permalink / raw)
To: dri-devel; +Cc: Jani Nikula, intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
A bunch of machines seem to have eDP panels where the EDID
indicates continuous frequency support but fails to actually
include the range descirptor. This violates the EDID 1.4
spec, but looks like the Windows driver just hacks around
this by just assuming that the panel supports a continuous
refresh rate range that covers all EDID reported modes.
Do the same so that we get VRR support on these machines.
v2: connector->edid may be an error pointer (Jani)
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6323
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_dp.c | 45 +++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 8d1559323412..df2b2064ce50 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5207,6 +5207,49 @@ intel_edp_add_properties(struct intel_dp *intel_dp)
fixed_mode->vdisplay);
}
+/*
+ * Some VRR eDP panels violate the EDID spec and neglect
+ * to include the monitor range descriptor in the EDID.
+ * Cook up the VRR refresh rate limits based on the modes
+ * reported by the panel.
+ */
+static void
+intel_edp_infer_vrr_range(struct intel_connector *connector)
+{
+ struct drm_i915_private *i915 = to_i915(connector->base.dev);
+ struct drm_display_info *info = &connector->base.display_info;
+ const struct edid *edid = connector->edid;
+ const struct drm_display_mode *mode;
+
+ if (!HAS_VRR(i915))
+ return;
+
+ if (IS_ERR_OR_NULL(edid) || edid->revision < 4 ||
+ !(edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ) ||
+ info->vrr_range.min_vfreq || info->vrr_range.max_vfreq)
+ return;
+
+ if (list_empty(&connector->base.probed_modes))
+ return;
+
+ info->vrr_range.min_vfreq = ~0;
+ info->vrr_range.max_vfreq = 0;
+
+ list_for_each_entry(mode, &connector->base.probed_modes, head) {
+ int vrefresh = drm_mode_vrefresh(mode);
+
+ info->vrr_range.min_vfreq = min_t(int, vrefresh,
+ info->vrr_range.min_vfreq);
+ info->vrr_range.max_vfreq = max_t(int, vrefresh,
+ info->vrr_range.max_vfreq);
+ }
+
+ drm_dbg_kms(&i915->drm,
+ "[CONNECTOR:%d:%s] does not report refresh rate range, assuming: %d Hz - %d Hz\n",
+ connector->base.base.id, connector->base.name,
+ info->vrr_range.min_vfreq, info->vrr_range.max_vfreq);
+}
+
static bool intel_edp_init_connector(struct intel_dp *intel_dp,
struct intel_connector *intel_connector)
{
@@ -5271,6 +5314,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
}
intel_connector->edid = edid;
+ intel_edp_infer_vrr_range(intel_connector);
+
intel_bios_init_panel(dev_priv, &intel_connector->panel,
encoder->devdata, IS_ERR(edid) ? NULL : edid);
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 07/11] drm/edid: Use GTF2 for inferred modes
2022-08-26 21:34 ` [PATCH 07/11] drm/edid: Use GTF2 for inferred modes Ville Syrjala
@ 2022-09-02 12:25 ` Jani Nikula
2022-09-02 12:45 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2022-09-02 12:25 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> For some resaon we only use the secondary GTF curve for the
> standard timings. Use it for inferred modes as well.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 35 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0c7cbe9b44f5..fed2bdd55c09 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3546,6 +3546,35 @@ static int drm_gtf_modes_for_range(struct drm_connector *connector,
> return modes;
> }
>
> +static int drm_gtf2_modes_for_range(struct drm_connector *connector,
> + const struct drm_edid *drm_edid,
> + const struct detailed_timing *timing)
> +{
> + int i, modes = 0;
> + struct drm_display_mode *newmode;
> + struct drm_device *dev = connector->dev;
> +
> + for (i = 0; i < ARRAY_SIZE(extra_modes); i++) {
> + const struct minimode *m = &extra_modes[i];
> +
> + newmode = drm_gtf2_mode(dev, drm_edid, m->w, m->h, m->r);
> + if (!newmode)
> + return modes;
> +
> + drm_mode_fixup_1366x768(newmode);
> + if (!mode_in_range(newmode, drm_edid, timing) ||
> + !valid_inferred_mode(connector, newmode)) {
> + drm_mode_destroy(dev, newmode);
> + continue;
> + }
> +
> + drm_mode_probed_add(connector, newmode);
> + modes++;
> + }
> +
> + return modes;
> +}
> +
> static int drm_cvt_modes_for_range(struct drm_connector *connector,
> const struct drm_edid *drm_edid,
> const struct detailed_timing *timing)
> @@ -3594,7 +3623,11 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
> return; /* GTF not defined yet */
>
> switch (range->flags) {
> - case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
> + case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
> + closure->modes += drm_gtf2_modes_for_range(closure->connector,
> + closure->drm_edid,
> + timing);
> + break;
> case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
Additionally, per spec:
* Default GTF supported if bit 0 in Feature Support Byte at address 18h = 1
* Secondary GTF supported --- requires support for Default GTF
So I guess both of these would need the edid->features &
DRM_EDID_FEATURE_DEFAULT_GTF check?
Other than that,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> closure->modes += drm_gtf_modes_for_range(closure->connector,
> closure->drm_edid,
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 09/11] drm/edid: Unconfuse preferred timing stuff a bit
2022-08-26 21:34 ` [PATCH 09/11] drm/edid: Unconfuse preferred timing stuff a bit Ville Syrjala
@ 2022-09-02 12:27 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2022-09-02 12:27 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> For EDID 1.4 the first detailed timing is always preferred,
> for older EDIDs there was a feature flag to indicate the same.
> While correct, the code setting that up is rather confusing.
> Restate it in a slightly more straightforward manner.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index c1c85b9e1208..0fe06e5fd6e0 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3952,13 +3952,14 @@ static int add_detailed_modes(struct drm_connector *connector,
> struct detailed_mode_closure closure = {
> .connector = connector,
> .drm_edid = drm_edid,
> - .preferred = true,
> .quirks = quirks,
> };
>
> - if (closure.preferred && !version_greater(drm_edid, 1, 3))
> + if (version_greater(drm_edid, 1, 3))
> + closure.preferred = true; /* first detailed timing is always preferred */
> + else
> closure.preferred =
> - (drm_edid->edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING);
> + drm_edid->edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING;
>
> drm_for_each_detailed_block(drm_edid, do_detailed_mode, &closure);
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 10/11] drm/edid: Make version checks less convoluted
2022-08-26 21:35 ` [PATCH 10/11] drm/edid: Make version checks less convoluted Ville Syrjala
@ 2022-09-02 12:31 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2022-09-02 12:31 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Get rid of the confusing version_greater() stuff and
> simply compare edid->revision directly everwhere. Half
> the places already did it this way, and since we actually
> reject any EDID with edid->version!=1 it's a perfectly
> sane thing to do.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 25 ++++++++-----------------
> 1 file changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0fe06e5fd6e0..e7f46260dfe7 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1572,15 +1572,6 @@ struct drm_edid {
> const struct edid *edid;
> };
>
> -static bool version_greater(const struct drm_edid *drm_edid,
> - u8 version, u8 revision)
> -{
> - const struct edid *edid = drm_edid->edid;
> -
> - return edid->version > version ||
> - (edid->version == version && edid->revision > revision);
> -}
> -
> static int edid_hfeeodb_extension_block_count(const struct edid *edid);
>
> static int edid_hfeeodb_block_count(const struct edid *edid)
> @@ -3652,7 +3643,7 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
> closure->drm_edid,
> timing);
>
> - if (!version_greater(closure->drm_edid, 1, 1))
> + if (closure->drm_edid->edid->revision < 2)
> return; /* GTF not defined yet */
>
> switch (range->flags) {
> @@ -3667,7 +3658,7 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
> timing);
> break;
> case DRM_EDID_CVT_SUPPORT_FLAG:
> - if (!version_greater(closure->drm_edid, 1, 3))
> + if (closure->drm_edid->edid->revision < 4)
> break;
>
> closure->modes += drm_cvt_modes_for_range(closure->connector,
> @@ -3688,7 +3679,7 @@ static int add_inferred_modes(struct drm_connector *connector,
> .drm_edid = drm_edid,
> };
>
> - if (version_greater(drm_edid, 1, 0))
> + if (drm_edid->edid->revision >= 1)
> drm_for_each_detailed_block(drm_edid, do_inferred_modes, &closure);
>
> return closure.modes;
> @@ -3765,7 +3756,7 @@ static int add_established_modes(struct drm_connector *connector,
> }
> }
>
> - if (version_greater(drm_edid, 1, 0))
> + if (edid->revision >= 1)
> drm_for_each_detailed_block(drm_edid, do_established_modes,
> &closure);
>
> @@ -3820,7 +3811,7 @@ static int add_standard_modes(struct drm_connector *connector,
> }
> }
>
> - if (version_greater(drm_edid, 1, 0))
> + if (drm_edid->edid->revision >= 1)
> drm_for_each_detailed_block(drm_edid, do_standard_modes,
> &closure);
>
> @@ -3900,7 +3891,7 @@ add_cvt_modes(struct drm_connector *connector, const struct drm_edid *drm_edid)
> .drm_edid = drm_edid,
> };
>
> - if (version_greater(drm_edid, 1, 2))
> + if (drm_edid->edid->revision >= 3)
> drm_for_each_detailed_block(drm_edid, do_cvt_mode, &closure);
>
> /* XXX should also look for CVT codes in VTB blocks */
> @@ -3955,7 +3946,7 @@ static int add_detailed_modes(struct drm_connector *connector,
> .quirks = quirks,
> };
>
> - if (version_greater(drm_edid, 1, 3))
> + if (drm_edid->edid->revision >= 4)
> closure.preferred = true; /* first detailed timing is always preferred */
> else
> closure.preferred =
> @@ -6144,7 +6135,7 @@ static void drm_get_vrr_range(struct drm_connector *connector,
> .drm_edid = drm_edid,
> };
>
> - if (!version_greater(drm_edid, 1, 3))
> + if (drm_edid->edid->revision < 4)
> return;
>
> if (!(drm_edid->edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ))
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 07/11] drm/edid: Use GTF2 for inferred modes
2022-09-02 12:25 ` [Intel-gfx] " Jani Nikula
@ 2022-09-02 12:45 ` Ville Syrjälä
0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2022-09-02 12:45 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dri-devel
On Fri, Sep 02, 2022 at 03:25:39PM +0300, Jani Nikula wrote:
> On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > For some resaon we only use the secondary GTF curve for the
> > standard timings. Use it for inferred modes as well.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_edid.c | 35 ++++++++++++++++++++++++++++++++++-
> > 1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 0c7cbe9b44f5..fed2bdd55c09 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3546,6 +3546,35 @@ static int drm_gtf_modes_for_range(struct drm_connector *connector,
> > return modes;
> > }
> >
> > +static int drm_gtf2_modes_for_range(struct drm_connector *connector,
> > + const struct drm_edid *drm_edid,
> > + const struct detailed_timing *timing)
> > +{
> > + int i, modes = 0;
> > + struct drm_display_mode *newmode;
> > + struct drm_device *dev = connector->dev;
> > +
> > + for (i = 0; i < ARRAY_SIZE(extra_modes); i++) {
> > + const struct minimode *m = &extra_modes[i];
> > +
> > + newmode = drm_gtf2_mode(dev, drm_edid, m->w, m->h, m->r);
> > + if (!newmode)
> > + return modes;
> > +
> > + drm_mode_fixup_1366x768(newmode);
> > + if (!mode_in_range(newmode, drm_edid, timing) ||
> > + !valid_inferred_mode(connector, newmode)) {
> > + drm_mode_destroy(dev, newmode);
> > + continue;
> > + }
> > +
> > + drm_mode_probed_add(connector, newmode);
> > + modes++;
> > + }
> > +
> > + return modes;
> > +}
> > +
> > static int drm_cvt_modes_for_range(struct drm_connector *connector,
> > const struct drm_edid *drm_edid,
> > const struct detailed_timing *timing)
> > @@ -3594,7 +3623,11 @@ do_inferred_modes(const struct detailed_timing *timing, void *c)
> > return; /* GTF not defined yet */
> >
> > switch (range->flags) {
> > - case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG: /* XXX could do more */
> > + case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
> > + closure->modes += drm_gtf2_modes_for_range(closure->connector,
> > + closure->drm_edid,
> > + timing);
> > + break;
> > case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
>
> Additionally, per spec:
>
> * Default GTF supported if bit 0 in Feature Support Byte at address 18h = 1
>
> * Secondary GTF supported --- requires support for Default GTF
>
> So I guess both of these would need the edid->features &
> DRM_EDID_FEATURE_DEFAULT_GTF check?
There is one actually
if (drm_edid->edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
num_modes += add_inferred_modes(connector, drm_edid);
Though as I think I mentioned in some of these patches a lot of
real world EDIDs don't set the default gtf/continuous frequency
bit but still include a range descriptor. While illegal, I think
a reasonable interpretation might be that they want us to use
the formula specified in the range descriptor for the non-DMT
standard timings, while still indicating that other timings
generated by said formula are not supported.
>
> Other than that,
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>
>
> > closure->modes += drm_gtf_modes_for_range(closure->connector,
> > closure->drm_edid,
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 08/11] drm/edid: Use the correct formula for standard timings
2022-08-26 21:34 ` [PATCH 08/11] drm/edid: Use the correct formula for standard timings Ville Syrjala
@ 2022-09-02 13:41 ` Jani Nikula
2022-09-02 14:02 ` Ville Syrjälä
0 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2022-09-02 13:41 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Prefer the timing formula indicated by the range
> descriptor for generating the non-DMT standard timings.
>
> Previously we just used CVT for all EDID 1.4 continuous
> frequency displays without even checking if the range
> descriptor indicates otherwise. Now we check the range
> descriptor first, and fall back to CVT if nothing else
> was indicated. EDID 1.4 more or less deprecates GTF/GTF2
> but there are still a lot of 1.4 EDIDs out there that
> don't advertise CVT support, so seems safer to use the
> formula the EDID actually reports as supported.
>
> For EDID 1.3 we use GTF2 if indicated (as before), and for
> EDID 1.2+ we now just use GTF without even checking the
> feature flag. There seem to be quite a few EDIDs out there that
> don't set the GTF feature flag but still include a GTF range
> descriptor and non-DMT standard timings.
>
> This to me seems to be roughly what appendix B of EDID 1.4
> suggests should be done.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 49 +++++++++++++++++++++++++++++++-------
> 1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index fed2bdd55c09..c1c85b9e1208 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3077,20 +3077,53 @@ drm_gtf2_2j(const struct drm_edid *drm_edid)
> return descriptor ? descriptor->data.other_data.data.range.formula.gtf2.j : 0;
> }
>
> +static void
> +get_timing_level(const struct detailed_timing *descriptor, void *data)
> +{
> + int *res = data;
> +
> + if (!is_display_descriptor(descriptor, EDID_DETAIL_MONITOR_RANGE))
> + return;
> +
> + BUILD_BUG_ON(offsetof(typeof(*descriptor), data.other_data.data.range.flags) != 10);
> +
> + switch (descriptor->data.other_data.data.range.flags) {
> + case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
> + *res = LEVEL_GTF;
> + break;
> + case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
> + *res = LEVEL_GTF2;
> + break;
> + case DRM_EDID_CVT_SUPPORT_FLAG:
> + *res = LEVEL_CVT;
> + break;
> + default:
> + break;
> + }
> +}
> +
> /* Get standard timing level (CVT/GTF/DMT). */
> static int standard_timing_level(const struct drm_edid *drm_edid)
> {
> const struct edid *edid = drm_edid->edid;
>
> - if (edid->revision >= 2) {
> - if (edid->revision >= 4 && (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF))
> - return LEVEL_CVT;
> - if (drm_gtf2_hbreak(drm_edid))
> - return LEVEL_GTF2;
> - if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
> - return LEVEL_GTF;
> + if (edid->revision >= 4) {
> + /*
> + * If the range descriptor doesn't
> + * indicate otherwise default to CVT
> + */
> + int ret = LEVEL_CVT;
> +
> + drm_for_each_detailed_block(drm_edid, get_timing_level, &ret);
Please remind me why it's okay to iterate all of them and pick the last?
I mean ret gets overwritten by the last block.
Otherwise it all seems okay to me, though I admit my confidence level in
this review is considerably lower than for the other patches.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> +
> + return ret;
> + } else if (edid->revision >= 3 && drm_gtf2_hbreak(drm_edid)) {
> + return LEVEL_GTF2;
> + } else if (edid->revision >= 2) {
> + return LEVEL_GTF;
> + } else {
> + return LEVEL_DMT;
> }
> - return LEVEL_DMT;
> }
>
> /*
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets
2022-08-27 1:40 ` Navare, Manasi
@ 2022-09-02 13:44 ` Ville Syrjälä
0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2022-09-02 13:44 UTC (permalink / raw)
To: Navare, Manasi; +Cc: Jani Nikula, intel-gfx, stable, dri-devel
On Fri, Aug 26, 2022 at 06:40:12PM -0700, Navare, Manasi wrote:
> On Sat, Aug 27, 2022 at 12:34:51AM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > EDID 1.4 introduced some extra flags in the range
> > descriptor to support min/max h/vfreq >= 255. Consult them
> > to correctly parse the vfreq limits.
> >
> > Note that some combinations of the flags are documented
> > as "reserved" (as are some other values in the descriptor)
> > but explicitly checking for those doesn't seem particularly
> > worthwile since we end up with bogus results whether we
> > decode them or not.
> >
> > v2: Increase the storage to u16 to make it work (Jani)
> > Note the "reserved" values situation (Jani)
> > v3: Document the EDID version number in the defines
> > Drop some bogus (u8) casts
> >
> > Cc: stable@vger.kernel.org
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6519
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_debugfs.c | 4 ++--
> > drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++------
> > include/drm/drm_connector.h | 4 ++--
> > include/drm/drm_edid.h | 5 +++++
> > 4 files changed, 27 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> > index 493922069c90..01ee3febb813 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", (u8)connector->display_info.monitor_range.min_vfreq);
> > - seq_printf(m, "Max: %u\n", (u8)connector->display_info.monitor_range.max_vfreq);
> > + 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);
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 90a5e26eafa8..4005dab6147d 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -6020,12 +6020,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
> > }
> >
> > static
> > -void get_monitor_range(const struct detailed_timing *timing,
> > - void *info_monitor_range)
> > +void get_monitor_range(const struct detailed_timing *timing, void *c)
> > {
> > - struct drm_monitor_range_info *monitor_range = info_monitor_range;
> > + 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;
> > 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;
> >
> > if (!is_display_descriptor(timing, EDID_DETAIL_MONITOR_RANGE))
> > return;
> > @@ -6041,18 +6043,28 @@ void get_monitor_range(const struct detailed_timing *timing,
> >
> > monitor_range->min_vfreq = range->min_vfreq;
> > monitor_range->max_vfreq = range->max_vfreq;
> > +
> > + if (edid->revision >= 4) {
> > + if (data->pad2 & DRM_EDID_RANGE_OFFSET_MIN_VFREQ)
> > + monitor_range->min_vfreq += 255;
> > + if (data->pad2 & DRM_EDID_RANGE_OFFSET_MAX_VFREQ)
> > + monitor_range->max_vfreq += 255;
>
> Yes this makes sense. This looks like added for supporting HRR (high
> refresh rate) panels.
> Do you think we should mention that in the commit message or in the
> comment in the code itself?
Didn't really see any need for that. Should be fairly obvious
a single u8 alone can't represent big values.
>
> Other than that looks good to me:
>
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Thanks. Pushed this one to drm-misc-fixes.
>
> Manasi
>
> > + }
> > }
> >
> > static void drm_get_monitor_range(struct drm_connector *connector,
> > const struct drm_edid *drm_edid)
> > {
> > - struct drm_display_info *info = &connector->display_info;
> > + const struct drm_display_info *info = &connector->display_info;
> > + struct detailed_mode_closure closure = {
> > + .connector = connector,
> > + .drm_edid = drm_edid,
> > + };
> >
> > if (!version_greater(drm_edid, 1, 1))
> > return;
> >
> > - drm_for_each_detailed_block(drm_edid, get_monitor_range,
> > - &info->monitor_range);
> > + drm_for_each_detailed_block(drm_edid, get_monitor_range, &closure);
> >
> > DRM_DEBUG_KMS("Supported Monitor Refresh rate range is %d Hz - %d Hz\n",
> > info->monitor_range.min_vfreq,
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 248206bbd975..56aee949c6fa 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -319,8 +319,8 @@ enum drm_panel_orientation {
> > * EDID's detailed monitor range
> > */
> > struct drm_monitor_range_info {
> > - u8 min_vfreq;
> > - u8 max_vfreq;
> > + u16 min_vfreq;
> > + u16 max_vfreq;
> > };
> >
> > /**
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 2181977ae683..1ed61e2b30a4 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -92,6 +92,11 @@ struct detailed_data_string {
> > u8 str[13];
> > } __attribute__((packed));
> >
> > +#define DRM_EDID_RANGE_OFFSET_MIN_VFREQ (1 << 0) /* 1.4 */
> > +#define DRM_EDID_RANGE_OFFSET_MAX_VFREQ (1 << 1) /* 1.4 */
> > +#define DRM_EDID_RANGE_OFFSET_MIN_HFREQ (1 << 2) /* 1.4 */
> > +#define DRM_EDID_RANGE_OFFSET_MAX_HFREQ (1 << 3) /* 1.4 */
> > +
> > #define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00
> > #define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01
> > #define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02
> > --
> > 2.35.1
> >
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Intel-gfx] [PATCH 08/11] drm/edid: Use the correct formula for standard timings
2022-09-02 13:41 ` [Intel-gfx] " Jani Nikula
@ 2022-09-02 14:02 ` Ville Syrjälä
0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2022-09-02 14:02 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dri-devel
On Fri, Sep 02, 2022 at 04:41:36PM +0300, Jani Nikula wrote:
> On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Prefer the timing formula indicated by the range
> > descriptor for generating the non-DMT standard timings.
> >
> > Previously we just used CVT for all EDID 1.4 continuous
> > frequency displays without even checking if the range
> > descriptor indicates otherwise. Now we check the range
> > descriptor first, and fall back to CVT if nothing else
> > was indicated. EDID 1.4 more or less deprecates GTF/GTF2
> > but there are still a lot of 1.4 EDIDs out there that
> > don't advertise CVT support, so seems safer to use the
> > formula the EDID actually reports as supported.
> >
> > For EDID 1.3 we use GTF2 if indicated (as before), and for
> > EDID 1.2+ we now just use GTF without even checking the
> > feature flag. There seem to be quite a few EDIDs out there that
> > don't set the GTF feature flag but still include a GTF range
> > descriptor and non-DMT standard timings.
> >
> > This to me seems to be roughly what appendix B of EDID 1.4
> > suggests should be done.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_edid.c | 49 +++++++++++++++++++++++++++++++-------
> > 1 file changed, 41 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index fed2bdd55c09..c1c85b9e1208 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3077,20 +3077,53 @@ drm_gtf2_2j(const struct drm_edid *drm_edid)
> > return descriptor ? descriptor->data.other_data.data.range.formula.gtf2.j : 0;
> > }
> >
> > +static void
> > +get_timing_level(const struct detailed_timing *descriptor, void *data)
> > +{
> > + int *res = data;
> > +
> > + if (!is_display_descriptor(descriptor, EDID_DETAIL_MONITOR_RANGE))
> > + return;
> > +
> > + BUILD_BUG_ON(offsetof(typeof(*descriptor), data.other_data.data.range.flags) != 10);
> > +
> > + switch (descriptor->data.other_data.data.range.flags) {
> > + case DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG:
> > + *res = LEVEL_GTF;
> > + break;
> > + case DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG:
> > + *res = LEVEL_GTF2;
> > + break;
> > + case DRM_EDID_CVT_SUPPORT_FLAG:
> > + *res = LEVEL_CVT;
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > /* Get standard timing level (CVT/GTF/DMT). */
> > static int standard_timing_level(const struct drm_edid *drm_edid)
> > {
> > const struct edid *edid = drm_edid->edid;
> >
> > - if (edid->revision >= 2) {
> > - if (edid->revision >= 4 && (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF))
> > - return LEVEL_CVT;
> > - if (drm_gtf2_hbreak(drm_edid))
> > - return LEVEL_GTF2;
> > - if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
> > - return LEVEL_GTF;
> > + if (edid->revision >= 4) {
> > + /*
> > + * If the range descriptor doesn't
> > + * indicate otherwise default to CVT
> > + */
> > + int ret = LEVEL_CVT;
> > +
> > + drm_for_each_detailed_block(drm_edid, get_timing_level, &ret);
>
> Please remind me why it's okay to iterate all of them and pick the last?
> I mean ret gets overwritten by the last block.
I think there is an implied assumption that there is only
zero or one of these included in the EDID. While not explicitly
stated in the spec, there is no mention anywhere what it would
mean to have multiple different types of range descriptors.
And so far I didn't come across any EDIDs that would disagree with
that.
>
> Otherwise it all seems okay to me, though I admit my confidence level in
> this review is considerably lower than for the other patches.
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>
> > +
> > + return ret;
> > + } else if (edid->revision >= 3 && drm_gtf2_hbreak(drm_edid)) {
> > + return LEVEL_GTF2;
> > + } else if (edid->revision >= 2) {
> > + return LEVEL_GTF;
> > + } else {
> > + return LEVEL_DMT;
> > }
> > - return LEVEL_DMT;
> > }
> >
> > /*
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-09-02 14:02 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-26 21:34 [PATCH 00/11] drm/edid: Range descriptor stuff Ville Syrjala
2022-08-26 21:34 ` [PATCH 01/11] drm/edid: Handle EDID 1.4 range descriptor h/vfreq offsets Ville Syrjala
2022-08-27 1:40 ` Navare, Manasi
2022-09-02 13:44 ` Ville Syrjälä
2022-08-26 21:34 ` [PATCH 02/11] drm/edid: Clarify why we only accept the "range limits only" descriptor Ville Syrjala
2022-08-27 1:45 ` Navare, Manasi
2022-08-26 21:34 ` [PATCH 03/11] drm/edid: s/monitor_rage/vrr_range/ Ville Syrjala
2022-08-27 1:47 ` Navare, Manasi
2022-08-29 8:29 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 04/11] drm/edid: Define more flags Ville Syrjala
2022-08-29 8:39 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 05/11] drm/edid: Only parse VRR range for continuous frequency displays Ville Syrjala
2022-08-29 8:58 ` Jani Nikula
2022-08-26 21:34 ` [PATCH 06/11] drm/edid: Extract drm_gtf2_mode() Ville Syrjala
2022-08-29 8:45 ` [Intel-gfx] " Jani Nikula
2022-08-26 21:34 ` [PATCH 07/11] drm/edid: Use GTF2 for inferred modes Ville Syrjala
2022-09-02 12:25 ` [Intel-gfx] " Jani Nikula
2022-09-02 12:45 ` Ville Syrjälä
2022-08-26 21:34 ` [PATCH 08/11] drm/edid: Use the correct formula for standard timings Ville Syrjala
2022-09-02 13:41 ` [Intel-gfx] " Jani Nikula
2022-09-02 14:02 ` Ville Syrjälä
2022-08-26 21:34 ` [PATCH 09/11] drm/edid: Unconfuse preferred timing stuff a bit Ville Syrjala
2022-09-02 12:27 ` Jani Nikula
2022-08-26 21:35 ` [PATCH 10/11] drm/edid: Make version checks less convoluted Ville Syrjala
2022-09-02 12:31 ` Jani Nikula
2022-08-26 21:35 ` [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it Ville Syrjala
2022-08-29 8:56 ` [Intel-gfx] " Jani Nikula
2022-08-29 12:02 ` [PATCH v2 " Ville Syrjala
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).