Thread (151 messages) 151 messages, 9 authors, 2022-08-30

Re: [PATCH v1 05/35] drm/connector: Add TV standard property

From: Noralf Trønnes <hidden>
Date: 2022-08-16 19:35:40
Also in: dri-devel, linux-amlogic, linux-sunxi, lkml


Den 16.08.2022 11.49, skrev Maxime Ripard:
On Tue, Aug 16, 2022 at 11:42:20AM +0200, Noralf Trønnes wrote:
quoted

Den 16.08.2022 10.26, skrev Maxime Ripard:
quoted
Hi,

On Mon, Aug 08, 2022 at 02:44:56PM +0200, Noralf Trønnes wrote:
quoted
Den 29.07.2022 18.34, skrev Maxime Ripard:
quoted
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.

We'll then be able to phase out the older tv mode property.

Signed-off-by: Maxime Ripard <redacted>
Please also update Documentation/gpu/kms-properties.csv

Requirements for adding a new property is found in
Documentation/gpu/drm-kms.rst
I knew this was going to be raised at some point, so I'm glad it's that
early :)

I really don't know what to do there. If we stick by our usual rules,
then we can't have any of that work merged.

However, I think the status quo is not really satisfactory either.
Indeed, we have a property, that doesn't follow those requirements
either, with a driver-specific content, and that stands in the way of
fixes and further improvements at both the core framework and driver
levels.

So having that new property still seems like a net improvement at the
driver, framework and uAPI levels, even if it's not entirely following
the requirements we have in place.

Even more so since, realistically, those kind of interfaces will never
get any new development on the user-space side of things, it's
considered by everyone as legacy.

This also is something we need to support at some point if we want to
completely deprecate the fbdev drivers and provide decent alternatives
in KMS.

So yeah, strictly speaking, we would not qualify for our requirements
there. I still think we have a strong case for an exception though.
Which requirements would that be? The only one I can see is the
documentation and maybe an IGT test.
This is the one I had in mind
https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
Oh right, I had forgotten about that one.

One benefit of having a userspace implementation is that it increases
the chance of widespread adoption having a working implementation to
look at. I don't think the reason tv.mode is not used anywhere (that I
know of) is because the driver picks the enum values resulting in no
standard names. It's a niche thing and way down on the todo list.
nouveau and ch7006 has a tv_norm module parameter which certainly
doesn't help in moving people/projects over to the DRM property
(downstream rpi also has it now).

mpv[1] is a commandline media player that after a quick look might be a
candidate for implementing the property without too much effort.

How do you test the property? I've used modetest but I can only change
to a tv.mode that matches the current display mode. I can't switch from
ntsc to pal for instance.

I have tried changing mode on rpi-5.15 (which I will switch to for the
gud gadget), but I always end up with flip timeouts when changing the value:

$ cat /proc/cpuinfo | grep Model
Model           : Raspberry Pi 4 Model B Rev 1.1
$ uname -a
Linux pi4t 5.15.56-v7l+ #1575 SMP Fri Jul 22 20:29:46 BST 2022 armv7l
GNU/Linux
$ sudo dmesg -C
$ modetest -M vc4 -s 45:720x480i -w 45:mode:1
setting mode 720x480i-29.97Hz on connectors 45, crtc 73
failed to set gamma: Function not implemented

$ dmesg
$ modetest -M vc4 -s 45:720x480i -w 45:mode:0
setting mode 720x480i-29.97Hz on connectors 45, crtc 73
failed to set gamma: Function not implemented

$ dmesg
[   95.193059] [drm:drm_atomic_helper_wait_for_flip_done
[drm_kms_helper]] *ERROR* [CRTC:73:crtc-1] flip_done timed out
[  105.433112] [drm:drm_crtc_commit_wait [drm]] *ERROR* flip_done timed out
[  105.433519] [drm:drm_atomic_helper_wait_for_dependencies
[drm_kms_helper]] *ERROR* [CRTC:73:crtc-1] commit wait timed out
[  115.673095] [drm:drm_crtc_commit_wait [drm]] *ERROR* flip_done timed out
[  115.673498] [drm:drm_atomic_helper_wait_for_dependencies
[drm_kms_helper]] *ERROR* [PLANE:63:plane-1] commit wait timed out
[  125.913106] [drm:drm_crtc_commit_wait [drm]] *ERROR* flip_done timed out
[  125.913510] vc4-drm gpu: [drm] *ERROR* Timed out waiting for commit
[  136.153411] [drm:drm_atomic_helper_wait_for_flip_done
[drm_kms_helper]] *ERROR* [CRTC:73:crtc-1] flip_done timed out

I doesn't help to reload vc4, I have to reboot to get it working again.
I get this when reloading:
[  776.799784] vc4_vec fec13000.vec: Unbalanced pm_runtime_enable!

I know this was working in rpi-5.10 on the Pi Zero (Pi4 tvout using vc4
did not work at all when I tried).


Noralf.

[1] https://mpv.io/
    https://github.com/mpv-player/mpv/blob/master/video/out/drm_common.c
    https://github.com/mpv-player/mpv/blob/master/video/out/drm_atomic.c

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help