Re: [PATCH v2 32/41] drm/vc4: vec: Convert to the new TV mode property

[Maxime Ripard]

On Thu, Sep 08, 2022 at 01:31:34PM +0200, Mateusz Kwiatkowski wrote:
> W dniu 08.09.2022 o 13:23, Maxime Ripard pisze:
> > Hi Noralf,
> >
> > On Tue, Aug 30, 2022 at 09:01:08PM +0200, Noralf Trønnes wrote:
> >>> +static const struct drm_prop_enum_list tv_mode_names[] = {
> >>
> >> Maybe call it legacy_tv_mode_enums?
> >>
> >>>
> >>> +    { VC4_VEC_TV_MODE_NTSC, "NTSC", },
> >>>
> >>> +    { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", },
> >>>
> >>> +    { VC4_VEC_TV_MODE_PAL, "PAL", },
> >>>
> >>> +    { VC4_VEC_TV_MODE_PAL_M, "PAL-M", },
> >>
> >> If you use DRM_MODE_TV_MODE_* here you don't need to translate the value
> >> using the switch statement in get/set property, you can use the value
> >> directly to get/set tv.mode.
> >
> > I'm sorry, I'm not quite sure what you mean by that. If we expose the
> > DRM_MODE_TV_MODE_* properties there, won't that change the values the
> > userspace will need to use to set that property?
> 
> I'd just like to point out that if numerical values of these enums are your
> concern, then you're (or perhaps I am ;) already breaking this by adding new
> modes in patch 33/41 in this series.
> 
> And the values (and names!) added by that patch (33/41) don't match those
> currently used by the downstream version
> (https://github.com/raspberrypi/linux/blob/rpi-5.15.y/drivers/gpu/drm/vc4/vc4_vec.c).
> If any userspace software is manipulating this property, it's most likely
> targeting the downstream code. But since you're not aiming for consistency with
> that, I was under the impression that compatibility isn't a concern.

I'm not really concerned about the compatibility with the downstream
tree, if only because you already broke that compatibility with your
patch :)

So you're right, I'll reorganize that patch to keep the backward
compatibility.

Maxime

Attachment: signature.asc
Description: PGP signature