Re: [PATCHv2] drm: fb_helper: prefer to use mode, which is not DRM_MODE_TYPE_USERDEF

[Chris Wilson]

On Mon, Apr 20, 2015 at 12:38:25PM +0200, Radek Dostál wrote:
> Hi Chris,
> 
> On 04/20/2015 11:58 AM, Chris Wilson wrote:
> > Hmm, so that should be before the clock comparison as well to fix your
> > example. Not as neat.
> 
> indeed that is required.
> 
> > The other idea I was considering was not adding the GTF cmdline mode if
> > the probed modes already contain one of a matching resolution. The goal
> > here is to also not present a mode to userspace that is invalid.
> 
> Unfortunately you can not do that. I already tried. At the time when
> drm_helper_probe_add_cmdline_mode is called EDID informations are not
> yet available.

My understanding is that it should be. fb_helper.initial_config does a
probe first, and the intel_fb_initial_config() should only keep the
active mode.
 
> Only option would be to remove the GTF cmdline mode, when matching mode
> is found in EDID.

So basically

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 6350387..9212bec 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -79,13 +79,29 @@ 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->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;
 


is not sufficient?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/