Re: [PATCH v2 09/41] drm/connector: Add TV standard property

[Maxime Ripard]

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@xxxxxxxxx> wrote:
> > 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 get the
> > > current TV mode output on an analog TV connector.
> > >
> > > Despite that property name being generic, its content isn't and has been
> > > 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 driver 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 enum.
> > The commit message is now misleading.
> >
> > > +static const struct drm_prop_enum_list drm_tv_mode_enum_list[] = {
> > > +    { 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 SECAM-L,
> > see my comment about PAL-Nc below), or NTSC-M-KR (a Korean variant of NTSC).
> >
> > 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 output 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 think.
> 
> 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.
> 
> Perhaps "generic" variants of NSTC and PAL/SECAM should be added, which
> would really just mean 525/60 resp. 625/50.
> 
> 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

Attachment: signature.asc
Description: PGP signature