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:   Wed, 7 Sep 2022 14:11:24 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Mateusz Kwiatkowski <kfyatek@gmail.com>,
        Ben Skeggs <bskeggs@redhat.com>,
        David Airlie <airlied@linux.ie>, Chen-Yu Tsai <wens@csie.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Lyude Paul <lyude@redhat.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Samuel Holland <samuel@sholland.org>,
        Karol Herbst <kherbst@redhat.com>,
        Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>,
        Emma Anholt <emma@anholt.net>, Daniel Vetter <daniel@ffwll.ch>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Phil Elwell <phil@raspberrypi.com>,
        Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Dom Cobley <dom@raspberrypi.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Nouveau Dev <nouveau@lists.freedesktop.org>,
        linux-sunxi@lists.linux.dev
Subject: Re: [PATCH v2 09/41] drm/connector: Add TV standard property
Message-ID: <20220907121124.kda7wi33b5cmwhcr@houat>
References: <20220728-rpi-analog-tv-properties-v2-0-459522d653a7@cerno.tech>
 <20220728-rpi-analog-tv-properties-v2-9-459522d653a7@cerno.tech>
 <30a9d7cd-d9ff-3177-ac6c-e7c1f966d89a@gmail.com>
 <CAMuHMdU_yfr1ybNM9Dyk6n34Cqv5WJv1wZxQ_ZGJ_T8JCOeB2g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="zkn4o5drgepunzyl"
Content-Disposition: inline
In-Reply-To: <CAMuHMdU_yfr1ybNM9Dyk6n34Cqv5WJv1wZxQ_ZGJ_T8JCOeB2g@mail.gmail.com>
Precedence: bulk


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

On Fri, Sep 02, 2022 at 09:35:20AM +0200, Geert Uytterhoeven wrote:
> On Fri, Sep 2, 2022 at 12:00 AM Mateusz Kwiatkowski <kfyatek@gmail.com> w=
rote:
> > W dniu 29.08.2022 o 15:11, Maxime Ripard pisze:
> > > The TV mode property has been around for a while now to select and ge=
t the
> > > current TV mode output on an analog TV connector.
> > >
> > > Despite that property name being generic, its content isn't and has b=
een
> > > driver-specific which makes it hard to build any generic behaviour on=
 top
> > > of it, both in kernel and user-space.
> > >
> > > Let's create a new bitmask tv norm property, that can contain any of =
the
> > > analog TV standards currently supported by kernel drivers. Each drive=
r can
> > > then pass in a bitmask of the modes it supports.
> >
> > This is not a bitmask property anymore, you've just changed it to an en=
um.
> > The commit message is now misleading.
> >
> > > +static const struct drm_prop_enum_list drm_tv_mode_enum_list[] =3D {
> > > +    { DRM_MODE_TV_MODE_NTSC_443, "NTSC-443" },
> > > +    { DRM_MODE_TV_MODE_NTSC_J, "NTSC-J" },
> > > +    { DRM_MODE_TV_MODE_NTSC_M, "NTSC-M" },
> > > +    { DRM_MODE_TV_MODE_PAL_60, "PAL-60" },
> > > +    { DRM_MODE_TV_MODE_PAL_B, "PAL-B" },
> > > +    { DRM_MODE_TV_MODE_PAL_D, "PAL-D" },
> > > +    { DRM_MODE_TV_MODE_PAL_G, "PAL-G" },
> > > +    { DRM_MODE_TV_MODE_PAL_H, "PAL-H" },
> > > +    { DRM_MODE_TV_MODE_PAL_I, "PAL-I" },
> > > +    { DRM_MODE_TV_MODE_PAL_M, "PAL-M" },
> > > +    { DRM_MODE_TV_MODE_PAL_N, "PAL-N" },
> > > +    { DRM_MODE_TV_MODE_PAL_NC, "PAL-Nc" },
> > > +    { DRM_MODE_TV_MODE_SECAM_60, "SECAM-60" },
> > > +    { DRM_MODE_TV_MODE_SECAM_B, "SECAM-B" },
> > > +    { DRM_MODE_TV_MODE_SECAM_D, "SECAM-D" },
> > > +    { DRM_MODE_TV_MODE_SECAM_G, "SECAM-G" },
> > > +    { DRM_MODE_TV_MODE_SECAM_K, "SECAM-K" },
> > > +    { DRM_MODE_TV_MODE_SECAM_K1, "SECAM-K1" },
> > > +    { DRM_MODE_TV_MODE_SECAM_L, "SECAM-L" },
> > > +};
> >
> > I did not comment on it the last time, but this list looks a little bit=
 random.
> >
> > Compared to the standards defined by V4L2, you also define SECAM-60 (a =
good
> > thing to define, because why not), but don't define PAL-B1, PAL-D1, PAL=
-K,
> > SECAM-H, SECAM-LC (whatever that is - probably just another name for SE=
CAM-L,
> > see my comment about PAL-Nc below), or NTSC-M-KR (a Korean variant of N=
TSC).
> >
> > Like I mentioned previously, I'm personally not a fan of including all =
those
> > CCIR/ITU system variants, as they don't mean any difference to the outp=
ut unless
> > there is an RF modulator involved. But I get it that they have already =
been used
> > and regressing probably wouldn't be a very good idea. But in that case =
keeping
> > it consistent with the set of values used by V4L2 would be wise, I thin=
k.
>=20
> Exactly. Anything outputting RGB (e.g. through a SCART or VGA connector)
> doesn't care about the color subcarrier or modulator parts.  Likewise,
> anything outputting CVBS doesn't care about the modulator part.
>=20
> Perhaps "generic" variants of NSTC and PAL/SECAM should be added, which
> would really just mean 525/60 resp. 625/50.
>=20
> Alternatively, the tv_mode field could be split in two parts (either
> two separate fields, or bitwise), to maintain a clear separation between
> lines/fields versus color encoding and RF modulation (with zero for the
> latter meaning a generic version)? That would also keep the door open
> for TV_MODE_405_50, TV_MODE_819_50, TV_MODE_750_50, TV_MODE_750_60, ...

Again, that property is only about color encoding and RF modulation. The
lines numbers and whether it's interlaced or not is encoded in the mode,
not here. So what you suggest is totally doable today.

Maxime

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

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

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYxiKbAAKCRDj7w1vZxhR
xd9CAP475gMVc9bgPfZRsSpr9ZicWPbCcdtwwB+SIZjyjWZwrgEAipAda0DdLowI
RCIVTz/4+0X5TsSdP1wzG4wph1ePCQY=
=iVLu
-----END PGP SIGNATURE-----

--zkn4o5drgepunzyl--