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

[Mateusz Kwiatkowski]

Hi Maxime,

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.

Best regards,
Mateusz Kwiatkowski