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

[Chris Wilson]

On Mon, Apr 20, 2015 at 10:46:08AM +0100, Chris Wilson wrote:
> On Mon, Apr 20, 2015 at 11:36:04AM +0200, Radek Dostál wrote:
> > Hi Chris,
> > 
> > On 04/20/2015 11:09 AM, Chris Wilson wrote:
> > > The EDID modes should be earlier in the list, and so higher priority
> > > than the cmdline mode. The only instance I see that breaking down is if
> > > the mode gets created by drm_pick_cmdline_mode, i.e.
> > 
> > indeed at the beginning the command line mode is added to the end of the
> > list, but later on it seems to me that all modes are reordered based on
> > the resolution and clock and than mode generated by
> > drm_helper_probe_add_cmdline_mode gets upper in the list as it has
> > higher clock value. Please see attached output of dmesg.
> 
> Ah thanks, drm_mode_sort()!
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 213b11e..9c8357f 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1127,6 +1127,7 @@ static int drm_mode_compare(void *priv, struct list_head *lh_a, struct list_head
>                 ((a->type & DRM_MODE_TYPE_PREFERRED) != 0);
>         if (diff)
>                 return diff;
> +
>         diff = b->hdisplay * b->vdisplay - a->hdisplay * a->vdisplay;
>         if (diff)
>                 return diff;
> @@ -1136,7 +1137,16 @@ static int drm_mode_compare(void *priv, struct list_head *lh_a, struct list_head
>                 return diff;
>  
>         diff = b->clock - a->clock;
> -       return diff;
> +       if (diff)
> +              return diff;
> +
> +       /* sort user-defined modes after reported modes of same resolution */
> +       diff = ((a->type & DRM_MODE_TYPE_USERDEF) != 0) -
> +               ((b->type & DRM_MODE_TYPE_USERDEF) != 0);
> +       if (diff)
> +               return diff;

Hmm, so that should be before the clock comparison as well to fix your
example. Not as neat.

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.
-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/