[RFC,05/37] drm: Exynos: Remove mode validation inside mode_fixup

Commit Message

Daniel Stone March 19, 2015, 4:33 a.m. UTC

mode_valid is the callback we already have to check whether or not a
mode is valid. So there's no need to validate again inside mode_fixup,
and there's really very definitely no need to select a totally different
mode.

Apparently the plan was to, if a 1366x768 mode was advertised but
couldn't be expressed in the PHY, silently pick 1024x768 instead and
never tell the user about it, resulting in a good chunk of the display
going missing.

Just remove the entire double-validation-and-find-new-mode code; if we
need to fiddle around the margins to find an acceptable pixel clock,
that should be done at the very least by not completely mangling
[hv]display.

Signed-off-by: Daniel Stone <[e-mail]>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 28 ----------------------------
 1 file changed, 28 deletions(-)

Comments

Daniel Vetter March 20, 2015, 3:59 p.m. UTC | #1

On Thu, Mar 19, 2015 at 04:33:04AM +0000, Daniel Stone wrote:
> mode_valid is the callback we already have to check whether or not a
> mode is valid. So there's no need to validate again inside mode_fixup,
> and there's really very definitely no need to select a totally different
> mode.

There is. mode_fixup is called for modeset, mode_valid by the probe
helpers. But userspace can just ignore the mode list and add their owns.
So yeah you'll need to validate twice.

Fixing that up is somewhere on my todo ...
-Daniel

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 229b361..1593b89 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1155,37 +1155,9 @@  static void hdmi_mode_fixup(struct exynos_drm_display *display,
 				const struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
 {
-	struct drm_display_mode *m;
-	int mode_ok;
-
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
-
-	mode_ok = hdmi_mode_valid(connector, adjusted_mode);
-
-	/* just return if user desired mode exists. */
-	if (mode_ok == MODE_OK)
-		return;
-
-	/*
-	 * otherwise, find the most suitable mode among modes and change it
-	 * to adjusted_mode.
-	 */
-	list_for_each_entry(m, &connector->modes, head) {
-		mode_ok = hdmi_mode_valid(connector, m);
-
-		if (mode_ok == MODE_OK) {
-			DRM_INFO("desired mode doesn't exist so\n");
-			DRM_INFO("use the most suitable mode among modes.\n");
-
-			DRM_DEBUG_KMS("Adjusted Mode: [%d]x[%d] [%d]Hz\n",
-				m->hdisplay, m->vdisplay, m->vrefresh);
-
-			drm_mode_copy(adjusted_mode, m);
-			break;
-		}
-	}
 }
 
 static void hdmi_set_acr(u32 freq, u8 *acr)