Message-ID: <20220826082151.h2kt3wca7s3ahl6x@houat>
Date:   Fri, 26 Aug 2022 10:21:51 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Noralf Trønnes <noralf@...nnes.org>
Cc:     Jernej Skrabec <jernej.skrabec@...il.com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Jerome Brunet <jbrunet@...libre.com>,
        Samuel Holland <samuel@...lland.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Daniel Vetter <daniel@...ll.ch>, Emma Anholt <emma@...olt.net>,
        David Airlie <airlied@...ux.ie>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Kevin Hilman <khilman@...libre.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Phil Elwell <phil@...pberrypi.com>,
        Mateusz Kwiatkowski <kfyatek+publicgit@...il.com>,
        linux-arm-kernel@...ts.infradead.org,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        linux-amlogic@...ts.infradead.org, dri-devel@...ts.freedesktop.org,
        Dom Cobley <dom@...pberrypi.com>
Subject: Re: [PATCH v1 05/35] drm/connector: Add TV standard property

On Sun, Aug 21, 2022 at 01:43:40PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 18.08.2022 17.31, skrev Maxime Ripard:
> > On Thu, Aug 18, 2022 at 05:01:38PM +0200, Noralf Trønnes wrote:
> >>
> >>
> >> Den 18.08.2022 01.23, skrev Noralf Trønnes:
> >>>
> >>>
> >>> Den 17.08.2022 15.11, skrev Noralf Trønnes:
> >>>>
> >>>>
> >>>> Den 17.08.2022 13.46, skrev Maxime Ripard:
> >>>>> On Tue, Aug 16, 2022 at 09:35:24PM +0200, Noralf Trønnes wrote:
> >>>>>> Den 16.08.2022 11.49, skrev Maxime Ripard:
> >>>>>>> On Tue, Aug 16, 2022 at 11:42:20AM +0200, Noralf Trønnes wrote:
> >>>>>>>> Den 16.08.2022 10.26, skrev Maxime Ripard:
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> On Mon, Aug 08, 2022 at 02:44:56PM +0200, Noralf Trønnes wrote:
> >>>>>>>>>> Den 29.07.2022 18.34, skrev Maxime Ripard:
> >>>>>>>>>>> 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 <maxime@...no.tech>
> >>>>>>>>>>>
> >>>
> >>>>>> 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.
> >>>>>
> >>>>> Yep, if you want to change from PAL to NTSC, it will require a new mode.
> >>>>>
> >>>>
> >>>> So userspace has to check tv.mode first and then create a display mode
> >>>> the driver will accept if switching to a different display mode is
> >>>> necessary? In other words, userspace can't discover from the kernel
> >>>> which display modes a certain tv.mode/norm provides before it is
> >>>> selected? If so, maybe libdrm should have some function(s) to deal with
> >>>> switching between modes that require a different display mode since
> >>>> knowledge about which display modes a tv.mode supports is needed before
> >>>> hand.
> >>>>
> >>>
> >>> I haven't used vc4 on Pi4 in mainline before and have finally gotten it
> >>> to work.
> >>>
> >>> I see that the connector reports 2 modes that together fit all tv.norms
> >>> so userspace doesn't have to contruct a display mode, but it does need
> >>> to know which display mode belongs to a certain tv.norm.
> >>>
> >>> When I try to use modetest I'm unable to set a mode:
> >>>
> >>> pi@...t:~ $ modetest -M vc4 -s 45:720x480i
> >>> setting mode 720x480i-29.97Hz on connectors 45, crtc 68
> >>> failed to set mode: Function not implemented
> >>>
> >>> The errno is misleading, modetest does a drmModeDirtyFB before checking
> >>> the error returned by drmModeSetCrtc.
> >>>
> >>> Setting the property succeeds, but the modeset still fails:
> >>>
> >>> pi@...t:~ $ modetest -M vc4 -s 45:720x480i -w 45:"tv norm":2
> >>> setting mode 720x480i-29.97Hz on connectors 45, crtc 68
> >>> failed to set mode: Function not implemented
> >>>
> >>> pi@...t:~ $ modetest -M vc4 -c
> >>>         37 tv norm:
> >>>                 flags: bitmask
> >>>                 values: NTSC-443=0x1 NTSC-J=0x2 NTSC-M=0x4 PAL-B=0x10
> >>> PAL-M=0x200 PAL-N=0x400 SECAM-B=0x2000
> >>>                 value: 2
> >>>
> >>> Here's the log, can you see if there's anything obvious in there:
> >>> https://gist.github.com/notro/a079498bf6b64327105752b2bafa8858
> >>>
> >>
> >> I'm one step closer as I now have fbcon working, I had forgotten to add
> >> enable_tvout=1 and I had disable_fw_kms_setup=1 which disables the
> >> video= mode on the kernel commandline.
> >>
> >> modetest still fails though, after alot of printk sprinkling, I've
> >> tracked it down to the drm_mode_equal test in
> >> drm_atomic_helper_connector_tv_check(). The aspect ratios differ:
> >>
> >> [   61.336295] drm_atomic_helper_connector_tv_check:
> >> mode->picture_aspect_ratio=1
> >> [   61.336301] drm_atomic_helper_connector_tv_check:
> >> &crtc_state->mode->picture_aspect_ratio=0
> > 
> > I haven't seen this when testing, but I'll have a look, thanks!
> 
> I have found the cause, the kernel strips off the aspect ratio in
> drm_mode_getconnector() if drm_file->aspect_ratio_allowed is false. So I
> think the drm_mode_equal() test needs to be relaxed for
> legacy/non-atomic userspace to work.

Geert suggested I removed it too, so this check is gone now.

> If I use modetest with atomic commit (-a) it works as is, having the
> drm_mode_equal() test:
> 
> $ modetest -M vc4 -a -P 61@68:720x480 -s 45:720x480i
> 
> I have a problem because the board hangs, either right away or after I
> press <enter> to quit modetest.
> 
> I often get this, sometimes after 10s of seconds:
> 
> [  136.822963] Unhandled fault: asynchronous external abort (0x1211) at
> 0x00000000
> ...
> [  137.248496]  bcm2711_get_temp [bcm2711_thermal] from
> thermal_zone_get_temp+0x54/0x74
> 
> Unloading bcm2711_thermal didn't help, in that case I got nothing, so
> the problem lies elsewhere.
> I have even tried with a fresh SD image and a fresh kernel, but it
> didn't help.

I got this too when working from current next (next-20220825) but it
doesn't seem to happen on drm-misc-next-2022-08-20-1, so it's probably
something unrelated?

> I can switch from NTSC to PAL like this (but it still crashes):
> 
> $ modetest -M vc4 -a -w 45:"tv norm":16 -P 61@68:720x576 -s 45:720x576i

I've tested with my current branch, and it work fine with modetest, I'll
put my commands in my next iteration so that you can compare?

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists