linux-kernel - Re: [PATCH v1 34/35] drm/modes: Introduce the tv

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]

Date:   Tue, 16 Aug 2022 15:51:05 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Jernej Skrabec <jernej.skrabec@...il.com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Jerome Brunet <jbrunet@...libre.com>,
        Samuel Holland <samuel@...lland.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Daniel Vetter <daniel@...ll.ch>, Emma Anholt <emma@...olt.net>,
        David Airlie <airlied@...ux.ie>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Noralf Trønnes <noralf@...nnes.org>,
        Kevin Hilman <khilman@...libre.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        linux-sunxi@...ts.linux.dev,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Phil Elwell <phil@...pberrypi.com>,
        Mateusz Kwiatkowski <kfyatek+publicgit@...il.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        "open list:ARM/Amlogic Meson..." <linux-amlogic@...ts.infradead.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Dom Cobley <dom@...pberrypi.com>
Subject: Re: [PATCH v1 34/35] drm/modes: Introduce the tv_mode property as a
 command-line option

On Fri, Aug 12, 2022 at 03:31:19PM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
> 
> On Fri, Jul 29, 2022 at 6:37 PM Maxime Ripard <maxime@...no.tech> wrote:
> > Our new tv mode option allows to specify the TV mode from a property.
> > However, it can still be useful, for example to avoid any boot time
> > artifact, to set that property directly from the kernel command line.
> >
> > Let's add some code to allow it, and some unit tests to exercise that code.
> >
> > Signed-off-by: Maxime Ripard <maxime@...no.tech>
> 
> Thanks for your patch!
> 
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1677,6 +1677,80 @@ static int drm_mode_parse_panel_orientation(const char *delim,
> >         return 0;
> >  }
> >
> > +#define TV_OPTION_EQUAL(value, len, option) \
> > +       ((strlen(option) == len) && !strncmp(value, option, len))
> > +
> > +static int drm_mode_parse_tv_mode(const char *delim,
> > +                                 struct drm_cmdline_mode *mode)
> > +{
> > +       const char *value;
> > +       unsigned int len;
> > +
> > +       if (*delim != '=')
> > +               return -EINVAL;
> > +
> > +       value = delim + 1;
> > +       delim = strchr(value, ',');
> > +       if (!delim)
> > +               delim = value + strlen(value);
> > +
> > +       len = delim - value;
> > +       if (TV_OPTION_EQUAL(value, len, "NTSC-443"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_NTSC_443;
> > +       else if (TV_OPTION_EQUAL(value, len, "NTSC-J"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_NTSC_J;
> > +       else if (TV_OPTION_EQUAL(value, len, "NTSC-M"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_NTSC_M;
> 
> [...]
> 
> You already have the array tv_norm_values[] from "[PATCH v1 05/35]
> drm/connector: Add TV standard property". Can't you export that, and
> loop over that array instead?

I'm not sure, the command line doesn't follow the same conventions than
the property names for a number of conventions, but at the same time we
should try to keep it as consistent as possible...

Then again, Jani and Thomas didn't seem too fond about exposing data as
part of the API, so I'm not sure how we could expose that properly.

> > +       else if (TV_OPTION_EQUAL(value, len, "HD480I"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_HD480I;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD480P"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_HD480P;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD576I"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_HD576I;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD576P"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_HD576P;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD720P"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_HD720P;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD1080I"))
> > +               mode->tv_mode = DRM_MODE_TV_NORM_HD1080I;
> 
> The names in tv_norm_values[] use lower-case, while you use upper-case
> here.

Indeed, I'll fix it, thanks!
Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)