[PATCH] modesetting: Add common widescreen resolutions

[Daniel Martin]

On 21 February 2016 at 12:08, Adel Gadllah <adel.gadllah at gmail.com> wrote:
> Stolen from the intel ddx driver, see:
> https://cgit.freedesktop.org/xorg/driver/xf86-video-intel/commit/?id=26fd6bec
> https://bugs.freedesktop.org/show_bug.cgi?id=37858
>
> it makes those common modes selectable when using the modesetting driver.
>
> Signed-off-by: Adel Gadllah <adel.gadllah at gmail.com>
> ---
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 107 ++++++++++++++++++++++-
>  1 file changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 0d34ca1..078b497 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -953,6 +953,111 @@ has_panel_fitter(xf86OutputPtr output)
>      return FALSE;
>  }
>
> +static struct pixel_count {
> +       int16_t width, height;
> +} common_16_9[] = {
> +       { 640, 360 },
> +       { 720, 405 },
> +       { 864, 486 },
> +       { 960, 540 },
> +       { 1024, 576 },
> +       { 1280, 720 },
> +       { 1366, 768 },
> +       { 1600, 900 },
> +       { 1920, 1080 },
> +       { 2048, 1152 },
> +       { 2560, 1440 },
> +       { 2880, 1620 },
> +       { 3200, 1800 },
> +       { 3840, 2160 },
> +       { 4096, 2304 },
> +       { 5120, 2880 },
> +       { 7680, 4320 },
> +       { 15360, 8640 },
> +}, common_16_10[] = {
> +       { 1280, 800 },
> +       { 1400, 900 },
> +       { 1680, 1050 },
> +       { 1920, 1200 },
> +       { 2560, 1600 },
> +};
> +
> +static bool
> +drmmode_output_is_duplicate_mode(DisplayModePtr modes, DisplayModePtr m)
> +{
> +       if (m == NULL)
> +               return false;
> +
> +       while (modes) {
> +               if (xf86ModesEqual(modes, m))
> +                       return true;
> +
> +               modes = modes->next;
> +       }
> +
> +       return false;
> +}
> +
> +static DisplayModePtr
> +drmmode_output_get_default_modes(DisplayModePtr preferred)
> +{
> +    DisplayModePtr modes;
> +    int n;
> +
> +    modes = xf86GetDefaultModes();
> +    if (preferred) {

I would prefer to invert this check. Then you don't need to indent the
following block.

> +               DisplayModePtr m;
> +
> +               /* Add a half-resolution mode useful for large panels */
> +               m = xf86GTFMode(preferred->HDisplay / 2,
> +                               preferred->VDisplay / 2,
> +                               xf86ModeVRefresh(preferred),
> +                               FALSE, FALSE);
> +               if (!drmmode_output_is_duplicate_mode(modes, m))
> +                       modes = xf86ModesAdd(modes, m);
> +               else
> +                       free(m);
> +
> +               if (preferred->VDisplay * 16 > preferred->HDisplay * 9 - preferred->HDisplay / 32 &&
> +                   preferred->VDisplay * 16 < preferred->HDisplay * 9 + preferred->HDisplay / 32) {
> +                       for (n = 0; n < ARRAY_SIZE(common_16_9); n++) {
> +                               if (preferred->HDisplay <= common_16_9[n].width ||
> +                                   preferred->VDisplay <= common_16_9[n].height)
> +                                       break;
> +
> +                               m = xf86GTFMode(common_16_9[n].width,
> +                                               common_16_9[n].height,
> +                                               xf86ModeVRefresh(preferred),
> +                                               FALSE, FALSE);
> +                               if (!drmmode_output_is_duplicate_mode(modes, m))
> +                                       modes = xf86ModesAdd(modes, m);
> +                               else
> +                                       free(m);
> +                       }
> +               }
> +
> +               if (preferred->VDisplay * 16 > preferred->HDisplay * 10 - preferred->HDisplay / 32 &&
> +                   preferred->VDisplay * 16 < preferred->HDisplay * 10 + preferred->HDisplay / 32) {
> +                       for (n = 0; n < ARRAY_SIZE(common_16_10); n++) {
> +                               if (preferred->HDisplay <= common_16_10[n].width ||
> +                                   preferred->VDisplay <= common_16_10[n].height)
> +                                       break;
> +
> +                               m = xf86GTFMode(common_16_10[n].width,
> +                                               common_16_10[n].height,
> +                                               xf86ModeVRefresh(preferred),
> +                                               FALSE, FALSE);
> +                               if (!drmmode_output_is_duplicate_mode(modes, m))
> +                                       modes = xf86ModesAdd(modes, m);
> +                               else
> +                                       free(m);
> +                       }
> +               }
> +       }
> +
> +       return modes;
> +}

We can and should avoid duplicating the code pretty easy. Something like:

    struct pixel_count *common;

    if ($aspect == 16:9)
        common = common_16_9;
    else
    if ($aspect == 16:10)
        common = common_16_10;

    ... iterate over common[i]

> +
>  static DisplayModePtr
>  drmmode_output_add_gtf_modes(xf86OutputPtr output, DisplayModePtr Modes)
>  {
> @@ -978,7 +1083,7 @@ drmmode_output_add_gtf_modes(xf86OutputPtr output, DisplayModePtr Modes)
>      max_vrefresh = max(max_vrefresh, 60.0);
>      max_vrefresh *= (1 + SYNC_TOLERANCE);
>
> -    m = xf86GetDefaultModes();
> +    m = drmmode_output_get_default_modes();
>      xf86ValidateModesSize(output->scrn, m, max_x, max_y, 0);
>
>      for (i = m; i; i = i->next) {
> --
> 2.5.0
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel