Re: [PATCH] drm: Only create a cmdline mode if no probed modes match

["Radek Dostál" <rd@radekdostal.com>]

[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]

Hi Chris,

On 04/20/2015 02:26 PM, Chris Wilson wrote:
> The intention of using video=<connector>:<mode: is primarily to select
> the user's preferred resolution at startup. Currently we always create a
> GTF mode irrespective of whether the monitor has a native mode at the
> desired resolution. This has the issue that we may then select the fake
> GTF mode rather the native mode during fb_helper->inital_config() and so
> on a non-GTF monitor we then end up with a loss of signal. Oops. This
> invalid fake mode would also be exported to userspace, who potentially
> may make the same mistake.
> 
> To avoid this issue, we filter out the added command line mode if we
> detect the desired resolution (and clock if specified) amongst the
> probed modes. This fixes the immediate problem of adding a duplicate
> mode, but perhaps more generically we should avoid adding a GTF mode if
> the monitor has an EDID that is not GTF-compatible...
> 
> A second issue sneaked into this patch is to add the cmdline mode mode
> ahead of the absolute fallback 1024x768 mode. That is if the user has
> specified a mode that we create as a fallback, we do not need to add a
> second unused fallback mode.
> 
> Fixes regression from
> 
> commit eaf99c749d43ae74ac7ffece5512f3c73f01dfd2
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Wed Aug 6 10:08:32 2014 +0200
> 
>     drm: Perform cmdline mode parsing during connector initialisation
> 
> that breaks HDMI output on BeagleBone Black with LG TV (model 19LS4R-ZA).
> 
> Reported-by: Radek Dostál <rd@radekdostal.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Radek Dostál <rd@radekdostal.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Julia Lemire <jlemire@matrox.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org

NAKed-by: Radek Dostál <rd@radekdostal.com>

sorry to let you know, but this patch does NOT work :( Attached is the
original.patch which you provided previously and it did work, but one of
the last minute changes had to break it :(

I hope you will find the issue soon,
Radek

[-- Attachment #2: original.patch --]
[-- Type: text/x-patch, Size: 1709 bytes --]

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 6350387..2168be0 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -79,13 +79,30 @@ drm_mode_validate_flag(const struct drm_display_mode *mode,
 
 static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
 {
+	struct drm_cmdline_mode *cmdline_mode;
 	struct drm_display_mode *mode;
 
-	if (!connector->cmdline_mode.specified)
+	cmdline_mode = &connector->cmdline_mode;
+	if (!cmdline_mode->specified)
 		return 0;
 
+        /* Only add a GTF mode if we find no matching probed modes */
+        list_for_each_entry(mode, &connector->probed_modes, head) {
+                if (mode->hdisplay != cmdline_mode->xres ||
+                    mode->vdisplay != cmdline_mode->yres)
+                        continue;
+
+                if (cmdline_mode->refresh_specified) {
+                        if (mode->vrefresh != cmdline_mode->refresh)
+                                continue;
+                }
+
+                return 0;
+        }
+
+
 	mode = drm_mode_create_from_cmdline_mode(connector->dev,
-						 &connector->cmdline_mode);
+						 cmdline_mode);
 	if (mode == NULL)
 		return 0;
 
@@ -195,7 +212,8 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
 		mode_flags |= DRM_MODE_FLAG_3D_MASK;
 
 	list_for_each_entry(mode, &connector->modes, head) {
-		mode->status = drm_mode_validate_basic(mode);
+                if (mode->status == MODE_OK)
+                        mode->status = drm_mode_validate_basic(mode);
 
 		if (mode->status == MODE_OK)
 			mode->status = drm_mode_validate_size(mode, maxX, maxY);