linux-kernel - Re: [PATCHv2] drm: fb_helper: prefer to use mode, which is not DRM_MODE_TYPE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150420110049.GG25451@nuc-i3427.alporthouse.com>
Date:	Mon, 20 Apr 2015 12:00:49 +0100
From:	Chris Wilson <chris@...is-wilson.co.uk>
To:	Radek Dostál <rd@...ekdostal.com>
Cc:	airlied@...ux.ie, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2] drm: fb_helper: prefer to use mode, which is not
 DRM_MODE_TYPE_USERDEF

On Mon, Apr 20, 2015 at 12:57:33PM +0200, Radek Dostál wrote:
> Hi Chris,
> 
> On 04/20/2015 12:48 PM, Chris Wilson wrote:
> >> 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.
> 
> uff, sorry I am not sure that I follow here - I am not that familiar
> with the whole subsystem.
> 
> >> > 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?
> 
> indeed, this was my first idea how to fix this problem, but at the time
> drm_helper_probe_add_cmdline_mode is called connector->modes is empty =>
> the GTF cmdline mode is still added.

Can you do a WARN_ON(list_empty(&connector->modes)) here to see at what
point we set up the invalid GTF mode?
-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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/