diff mbox series

[07/15] drm/ast: Always validate H/V sync flags

Message ID 20250124080546.9956-8-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/ast: Clean up display-mode detection and validation | expand

Commit Message

Thomas Zimmermann Jan. 24, 2025, 7:57 a.m. UTC 
The ast driver matches DRM display modes against an internal list of
modes supported by the VBIOS. Matching H/V sync flags between modes is
preferred, but optional. If sync flags are not matching, the driver
would program the VBIOS settings to hardware and let the display handle
the difference.

DRM modes are generated from attached displays or standard mode lines.
Therefore differences to the VBIOS modes are not just cosmetical, but
signal possible incompatibility with the display hardware.

Hence make matching H/V sync flags mandatory. If the VBIOS does not
support a certain mode, we should report it as unsupported. Note that
the VBIOS mode tables all appear to refer to standard modes.

(If sync flags really make no difference to the VBIOS, the ast driver
shouldn't match them in the first place.)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_mode.c | 45 ++++++++++++++--------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

Comments

Jocelyn Falempe Jan. 27, 2025, 2:44 p.m. UTC | #1 
On 24/01/2025 08:57, Thomas Zimmermann wrote:
> The ast driver matches DRM display modes against an internal list of
> modes supported by the VBIOS. Matching H/V sync flags between modes is
> preferred, but optional. If sync flags are not matching, the driver
> would program the VBIOS settings to hardware and let the display handle
> the difference.
> 
> DRM modes are generated from attached displays or standard mode lines.
> Therefore differences to the VBIOS modes are not just cosmetical, but
> signal possible incompatibility with the display hardware.
> 
> Hence make matching H/V sync flags mandatory. If the VBIOS does not
> support a certain mode, we should report it as unsupported. Note that
> the VBIOS mode tables all appear to refer to standard modes.
> 
> (If sync flags really make no difference to the VBIOS, the ast driver
> shouldn't match them in the first place.)

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index fca625518a873..48b46dc3a618e 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -113,8 +113,8 @@  static bool ast_get_vbios_mode_info(const struct drm_format_info *format,
 {
 	u32 refresh_rate_index = 0, refresh_rate;
 	const struct ast_vbios_enhtable *best = NULL;
+	const struct ast_vbios_enhtable *loop;
 	u32 hborder, vborder;
-	bool check_sync;
 
 	switch (format->cpp[0] * 8) {
 	case 8:
@@ -176,36 +176,27 @@  static bool ast_get_vbios_mode_info(const struct drm_format_info *format,
 	}
 
 	refresh_rate = drm_mode_vrefresh(mode);
-	check_sync = vbios_mode->enh_table->flags & WideScreenMode;
-
-	while (1) {
-		const struct ast_vbios_enhtable *loop = vbios_mode->enh_table;
-
-		while (loop->refresh_rate != 0xff) {
-			if ((check_sync) &&
-			    (((mode->flags & DRM_MODE_FLAG_NVSYNC)  &&
-			      (loop->flags & PVSync))  ||
-			     ((mode->flags & DRM_MODE_FLAG_PVSYNC)  &&
-			      (loop->flags & NVSync))  ||
-			     ((mode->flags & DRM_MODE_FLAG_NHSYNC)  &&
-			      (loop->flags & PHSync))  ||
-			     ((mode->flags & DRM_MODE_FLAG_PHSYNC)  &&
-			      (loop->flags & NHSync)))) {
-				loop++;
-				continue;
-			}
-			if (loop->refresh_rate <= refresh_rate
-			    && (!best || loop->refresh_rate > best->refresh_rate))
-				best = loop;
+
+	loop = vbios_mode->enh_table;
+
+	while (loop->refresh_rate != 0xff) {
+		if (((mode->flags & DRM_MODE_FLAG_NVSYNC) && (loop->flags & PVSync))  ||
+		    ((mode->flags & DRM_MODE_FLAG_PVSYNC) && (loop->flags & NVSync))  ||
+		    ((mode->flags & DRM_MODE_FLAG_NHSYNC) && (loop->flags & PHSync))  ||
+		    ((mode->flags & DRM_MODE_FLAG_PHSYNC) && (loop->flags & NHSync))) {
 			loop++;
+			continue;
 		}
-		if (best || !check_sync)
-			break;
-		check_sync = 0;
+		if (loop->refresh_rate <= refresh_rate &&
+		    (!best || loop->refresh_rate > best->refresh_rate))
+			best = loop;
+		loop++;
 	}
 
-	if (best)
-		vbios_mode->enh_table = best;
+	if (!best)
+		return false;
+
+	vbios_mode->enh_table = best;
 
 	hborder = (vbios_mode->enh_table->flags & HBorder) ? 8 : 0;
 	vborder = (vbios_mode->enh_table->flags & VBorder) ? 8 : 0;