Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i8771445c:Fastmail
Date:   Tue, 16 Aug 2022 15:51:05 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Jernej Skrabec <jernej.skrabec@gmail.com>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Jerome Brunet <jbrunet@baylibre.com>,
        Samuel Holland <samuel@sholland.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Daniel Vetter <daniel@ffwll.ch>, Emma Anholt <emma@anholt.net>,
        David Airlie <airlied@linux.ie>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>,
        Kevin Hilman <khilman@baylibre.com>,
        Neil Armstrong <narmstrong@baylibre.com>,
        linux-sunxi@lists.linux.dev,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Phil Elwell <phil@raspberrypi.com>,
        Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        "open list:ARM/Amlogic Meson..." <linux-amlogic@lists.infradead.org>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Dom Cobley <dom@raspberrypi.com>
Subject: Re: [PATCH v1 34/35] drm/modes: Introduce the tv_mode property as a
 command-line option
Message-ID: <20220816135105.goztqjzqqhhigytd@houat>
References: <20220728-rpi-analog-tv-properties-v1-0-3d53ae722097@cerno.tech>
 <20220728-rpi-analog-tv-properties-v1-34-3d53ae722097@cerno.tech>
 <CAMuHMdXizN9OgXgxwdFc1gpnhZof-SZrCH8PczEiJrtYpB62Ow@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="oapqvc3fwa7skyps"
Content-Disposition: inline
In-Reply-To: <CAMuHMdXizN9OgXgxwdFc1gpnhZof-SZrCH8PczEiJrtYpB62Ow@mail.gmail.com>
Precedence: bulk


--oapqvc3fwa7skyps
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Aug 12, 2022 at 03:31:19PM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
>=20
> On Fri, Jul 29, 2022 at 6:37 PM Maxime Ripard <maxime@cerno.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 c=
ode.
> >
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>=20
> Thanks for your patch!
>=20
> > --- 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(cons=
t char *delim,
> >         return 0;
> >  }
> >
> > +#define TV_OPTION_EQUAL(value, len, option) \
> > +       ((strlen(option) =3D=3D 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 !=3D '=3D')
> > +               return -EINVAL;
> > +
> > +       value =3D delim + 1;
> > +       delim =3D strchr(value, ',');
> > +       if (!delim)
> > +               delim =3D value + strlen(value);
> > +
> > +       len =3D delim - value;
> > +       if (TV_OPTION_EQUAL(value, len, "NTSC-443"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_NTSC_443;
> > +       else if (TV_OPTION_EQUAL(value, len, "NTSC-J"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_NTSC_J;
> > +       else if (TV_OPTION_EQUAL(value, len, "NTSC-M"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_NTSC_M;
>=20
> [...]
>=20
> 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 =3D DRM_MODE_TV_NORM_HD480I;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD480P"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_HD480P;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD576I"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_HD576I;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD576P"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_HD576P;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD720P"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_HD720P;
> > +       else if (TV_OPTION_EQUAL(value, len, "HD1080I"))
> > +               mode->tv_mode =3D DRM_MODE_TV_NORM_HD1080I;
>=20
> The names in tv_norm_values[] use lower-case, while you use upper-case
> here.

Indeed, I'll fix it, thanks!
Maxime

--oapqvc3fwa7skyps
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYvugyAAKCRDj7w1vZxhR
xYVoAQDFAMNuU1dbZgCiVBYUf+Y13tqfHCtlOuPYmSb6Gmh3xgEAgXYxzEcBWNLK
j9bSNzYbE9lp+H3MaLuAPoiaBAXIRQE=
=szbm
-----END PGP SIGNATURE-----

--oapqvc3fwa7skyps--