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:20:40 +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 05/35] drm/connector: Add TV standard property
Message-ID: <20220816132040.uwirtjm5yr6rdd3q@houat>
References: <20220728-rpi-analog-tv-properties-v1-0-3d53ae722097@cerno.tech>
 <20220728-rpi-analog-tv-properties-v1-5-3d53ae722097@cerno.tech>
 <CAMuHMdWYo7M44uLNhTmJenGDreGALBZ9E48oyBDEeAuL=0h=dw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="6ena3yqc72sajvcu"
Content-Disposition: inline
In-Reply-To: <CAMuHMdWYo7M44uLNhTmJenGDreGALBZ9E48oyBDEeAuL=0h=dw@mail.gmail.com>
Precedence: bulk


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

Hi Geert,

On Fri, Aug 12, 2022 at 03:25:18PM +0200, Geert Uytterhoeven wrote:
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1649,11 +1650,40 @@ EXPORT_SYMBOL(drm_mode_create_tv_margin_propert=
ies);
> >   * 0 on success or a negative error code on failure.
> >   */
> >  int drm_mode_create_tv_properties(struct drm_device *dev,
> > +                                 unsigned int supported_tv_norms,
> >                                   unsigned int num_modes,
> >                                   const char * const modes[])
> >  {
> > +       static const struct drm_prop_enum_list tv_norm_values[] =3D {
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_NTSC_443) - 1, "NTSC-4=
43" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_NTSC_J) - 1, "NTSC-J" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_NTSC_M) - 1, "NTSC-M" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_60) - 1, "PAL-60" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_B) - 1, "PAL-B" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_D) - 1, "PAL-D" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_G) - 1, "PAL-G" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_H) - 1, "PAL-H" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_I) - 1, "PAL-I" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_M) - 1, "PAL-M" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_N) - 1, "PAL-N" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_PAL_NC) - 1, "PAL-Nc" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_60) - 1, "SECAM-=
60" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_B) - 1, "SECAM-B=
" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_D) - 1, "SECAM-D=
" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_G) - 1, "SECAM-G=
" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_K) - 1, "SECAM-K=
" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_K1) - 1, "SECAM-=
K1" },
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_L) - 1, "SECAM-L=
" },
>=20
> The above are analog standards, with a variable horizontal resolution.
>=20
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_HD480I) - 1, "hd480i" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_HD480P) - 1, "hd480p" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_HD576I) - 1, "hd576i" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_HD576P) - 1, "hd576p" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_HD720P) - 1, "hd720p" =
},
> > +               { __builtin_ffs(DRM_MODE_TV_NORM_HD1080I) - 1, "hd1080i=
" },
>=20
> The above are digital standards, with a fixed resolution.

Are they?

It's not clear to me from looking at nouveau, but I was under the
impression that they were modes for a component output, so CEA 770.3. I
don't have the spec though, so I can't check.

> You seem to have missed "hd1080p"?

Nobody is using it. If we ever have a driver that uses it I think we can
add it.

> In addition, "hd720p", "hd080i", and "hd1080p" are available in both 50
> and 60 (actually 59.94) Hz, while "hd1080p" can also use 24 or 25 Hz.

It looks like nouveau only exposes modes for 480p at 59.94Hz, 576p at
50Hz, 720p at 60Hz, 1080i at 30Hz.

> Either you have to add them here (e.g. "hd720p50" and "hd720p60"), or
> handle them through "@<refresh>".  The latter would impact "[PATCH v1
> 09/35] drm/modes: Move named modes parsing to a separate function", as
> currently a named mode and a refresh rate can't be specified both.

I think the former would make more sense. It simplifies a bit the
parser, and we're going to use a named mode anyway.

> As "[PATCH v1 34/35] drm/modes: Introduce the tv_mode property as a
> command-line option" uses a separate "tv_mode" option, and not the main
> mode name, I think you want to add them here.

It's a separate story I think, we could have a named mode hd720p50,
which would be equivalent to 1280x720,tv_mode=3Dhd720p

Maxime

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

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

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYvuZqAAKCRDj7w1vZxhR
xRpdAQCdHo+W59CWD9zJ3T1LvoCmG3uMcVuRQ9G79/cpqOxH9wEAhPrjIiW5PuV0
9yKEPpR9rUmMFVvJOYSKOJ0j4md2bQE=
=XiNC
-----END PGP SIGNATURE-----

--6ena3yqc72sajvcu--