Re: UAPI Re: [PATCH 1/3] drm: Add DRM_MODE_TV_MODE_MONOCHROME

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pekka

Sorry for the slight delay in replying.

On Mon, 26 Feb 2024 at 15:11, Pekka Paalanen
<pekka.paalanen@xxxxxxxxxxxxx> wrote:
>
> On Mon, 26 Feb 2024 15:10:45 +0100
> Maxime Ripard <mripard@xxxxxxxxxx> wrote:
>
> > Hi Pekka,
> >
> > On Wed, Feb 21, 2024 at 11:07:51AM +0200, Pekka Paalanen wrote:
> > > On Fri, 16 Feb 2024 18:48:55 +0000
> > > Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> wrote:
> > >
> > > > From: Nick Hollinghurst <nick.hollinghurst@xxxxxxxxxxxxxxx>
> > > >
> > > > Add this as a value for enum_drm_connector_tv_mode, represented
> > > > by the string "Mono", to generate video with no colour encoding
> > > > or bursts. Define it to have no pedestal (since only NTSC-M calls
> > > > for a pedestal).
> > > >
> > > > Change default mode creation to acommodate the new tv_mode value
> > > > which comprises both 525-line and 625-line formats.
> > > >
> > > > Signed-off-by: Nick Hollinghurst <nick.hollinghurst@xxxxxxxxxxxxxxx>
> > > > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > >
> > > since no-one else commented yet, I'd like to remind of the new UAPI
> > > requirements:
> > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
> > >
> > > AFAIU, adding a new value to an existing enum still counts as new UAPI.
> > >
> > > Maybe there is no need for the full treatment here, or maybe there is,
> > > I'm not sure. I think you should make some statement about how the new
> > > UAPI requirements have been addressed.
> >
> > That property was meant to provide legacy display handling, so I don't
> > expect any reasonably recent codebase like Weston to suppport it, ever
> > :)
> >
> > That being said, from the beginning, that property was meant to be
> > handled as a "mode-setting" property, and thus handled by either the
> > kernel command-line, xrandr, or any similar mechanism.
> >
> > I would expect that new enum variant to be handled under the same terms:
> > it'll probably only show up in distro scripts or configuration files,
> > and never in any actual code base.
> >
> > Is it what you were expecting, or did you mean something else?
>
> Maybe? Let's have it in the commit message and see if DRM maintainers
> agree.

You want the commit text for a patch adding a new enum to state that
the whole property isn't expected to be used through the UAPI? OK, I
can roll a v2 if that is your desire.

> You should expect that all KMS clients will work towards programming
> all exposed KMS properties into known values. That's the only way to
> achieve repeatable KMS behaviour, because there is no KMS reset ioctl.
>
> There are no two tiers of KMS properties AFAIK. You have to be the DRM
> master to change anything. So, people cannot force any settings from
> outside of a KMS client, they have to go through the KMS client, like
> xrandr goes through Xorg (and only Xorg).
>
> I do fully expect Weston to gain support for this property, if anyone
> cares of its value. That goes for all Wayland compositors.

I don't know about Weston, but Wayfire / wlroots / sway have currently
chosen to ignore all interlaced display modes [1].
[2] is the wlroots devs basically calling interlaced output a dead end.

That makes the debate for controlling the colour encoding on a
composite video rather redundant as they're almost always interlaced.

[1] https://github.com/swaywm/sway/issues/3167
[2] https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3038

> You're correct in that a KMS client would probably not know to control
> the value of this property automatically but it needs to come from
> configuration. That would be each KMS client's configuration. I don't
> understand how a script could achieve that.
>
> However, if you feel it is important to have KMS properties that
> display servers must not touch, then they should be documented as such.
> I do not know if that would actually lift the new-UAPI requirements,
> that is for the DRM maintainers to decide and document. Is there such a
> thing already?
>
> What are those "similar to xrandr" mechanisms? I don't think I've heard
> of any,

Boot to the console and run
modetest -w <connector id>:"tv_mode":"NTSC"
There is no reset mechanism for all properties, so that setting
persists after modetest quits.

> and I've also completely missed any kernel command line
> arguments manipulating KMS properties.

"tv_mode" on the command line is handled in
drm_mode_parse_cmdline_options() [3], as are rotate, reflect_x,
reflect_y, margin_[left|right|top|bottom], and panel_orientation all
to set the relevant KMS properties.

Having "video=Composite-1:PAL,tv_mode=Mono" on the kernel command line
will set up connector Composite-1 with the standard 720x576 50Hz
interlaced timings, and DRM_MODE_TV_MODE_MONOCHROME selected on the
tv_mode property. Swap in different tv_mode descriptions as required
(eg PAL,tv_mode=SECAM), although some make little sense.

That's the main route I'm looking at for configuring this property,
for situations such as having a black and white TV connected. You
don't get the opportunity to interrogate a composite display over what
it supports, so it has to be configured manually somewhere in the
system. If your monitor doesn't support the system default, then you
can't see a GUI in order to change the option, and there is no
guaranteed supported configuration so the command line is about the
only option.

The use cases for runtime switching of the "tv_mode" are exceedingly
rare, so IMHO the property doesn't need exposing through the UAPI.
However it was added to the UAPI about 8 years ago for vc4 and sunxi,
and is also now used by other drivers, so can't be reverted. Does that
mean it can now never be changed without jumping through the hoop of
creating some userspace user?

Regards
  Dave

[3] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_modes.c#L2232



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux