Message-ID: <0db1c833-eeee-cadd-b050-837d79a3d691@tronnes.org>
Date:   Sat, 1 Oct 2022 15:12:06 +0200
From:   Noralf Trønnes <noralf@...nnes.org>
To:     Maxime Ripard <maxime@...no.tech>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Karol Herbst <kherbst@...hat.com>,
        Samuel Holland <samuel@...lland.org>,
        Lyude Paul <lyude@...hat.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Emma Anholt <emma@...olt.net>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Ben Skeggs <bskeggs@...hat.com>,
        David Airlie <airlied@...ux.ie>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>
Cc:     linux-arm-kernel@...ts.infradead.org,
        dri-devel@...ts.freedesktop.org,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        intel-gfx@...ts.freedesktop.org, linux-sunxi@...ts.linux.dev,
        Hans de Goede <hdegoede@...hat.com>,
        nouveau@...ts.freedesktop.org,
        Mateusz Kwiatkowski <kfyatek+publicgit@...il.com>,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        linux-kernel@...r.kernel.org, Dom Cobley <dom@...pberrypi.com>,
        Phil Elwell <phil@...pberrypi.com>,
        Noralf Trønnes <noralf@...nnes.org>
Subject: Re: [PATCH v4 00/30] drm: Analog TV Improvements



Den 29.09.2022 18.30, skrev Maxime Ripard:
> Hi,
> 
> Here's a series aiming at improving the command line named modes support,
> and more importantly how we deal with all the analog TV variants.
> 
> The named modes support were initially introduced to allow to specify the
> analog TV mode to be used.
> 
> However, this was causing multiple issues:
> 
>   * The mode name parsed on the command line was passed directly to the
>     driver, which had to figure out which mode it was suppose to match;
> 
>   * Figuring that out wasn't really easy, since the video= argument or what
>     the userspace might not even have a name in the first place, but
>     instead could have passed a mode with the same timings;
> 
>   * The fallback to matching on the timings was mostly working as long as
>     we were supporting one 525 lines (most likely NSTC) and one 625 lines
>     (PAL), but couldn't differentiate between two modes with the same
>     timings (NTSC vs PAL-M vs NSTC-J for example);
> 
>   * There was also some overlap with the tv mode property registered by
>     drm_mode_create_tv_properties(), but named modes weren't interacting
>     with that property at all.
> 
>   * Even though that property was generic, its possible values were
>     specific to each drivers, which made some generic support difficult.
> 
> Thus, I chose to tackle in multiple steps:
> 
>   * A new TV mode property was introduced, with generic values, each driver
>     reporting through a bitmask what standard it supports to the userspace;
> 
>   * This option was added to the command line parsing code to be able to
>     specify it on the kernel command line, and new atomic_check and reset
>     helpers were created to integrate properly into atomic KMS;
> 
>   * The named mode parsing code is now creating a proper display mode for
>     the given named mode, and the TV standard will thus be part of the
>     connector state;
> 
>   * Two drivers were converted and tested for now (vc4 and sun4i), with
>     some backward compatibility code to translate the old TV mode to the
>     new TV mode;
> 
> Unit tests were created along the way.
> 
> One can switch from NTSC to PAL now using (on vc4)
> 
> modetest -M vc4  -s 53:720x480i -w 53:'TV mode':1 # NTSC
> modetest -M vc4  -s 53:720x576i -w 53:'TV mode':4 # PAL
> 
> Let me know what you think,
> Maxime
> 

I suggest that you apply the patches that are reviewed, have merrit on
their own and are not tied to the TV mode property.
This will help in keeping this rather big patchset focused and ease the
task for reviewers.

The following seems to be in that group:

  drm/tests: Order Kunit tests in Makefile
  drm/atomic-helper: Rename drm_atomic_helper_connector_tv_reset to
avoid ambiguity
  drm/connector: Rename subconnector state variable
  drm/atomic: Add TV subconnector property to get/set_property
  drm/modes: Only consider bpp and refresh before options
  drm/modes: parse_cmdline: Add support for named modes containing dashes
  drm/vc4: vec: Fix definition of PAL-M mode

Noralf.

Powered by blists - more mailing lists