Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i8771445c:Fastmail
Date:   Wed, 9 Nov 2022 12:18:43 +0100
From:   Maxime Ripard <maxime@cerno.tech>
To:     kfyatek+publicgit@gmail.com
Cc:     Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Ben Skeggs <bskeggs@redhat.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Samuel Holland <samuel@sholland.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Emma Anholt <emma@anholt.net>,
        Karol Herbst <kherbst@redhat.com>,
        Daniel Vetter <daniel@ffwll.ch>, Chen-Yu Tsai <wens@csie.org>,
        Lyude Paul <lyude@redhat.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@linux.ie>,
        Phil Elwell <phil@raspberrypi.com>,
        Hans de Goede <hdegoede@redhat.com>,
        linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        linux-arm-kernel@lists.infradead.org,
        Dom Cobley <dom@raspberrypi.com>,
        dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
        nouveau@lists.freedesktop.org
Subject: Re: [PATCH v7 15/23] drm/modes: Introduce more named modes
Message-ID: <20221109111843.jkufdneexakofa4p@houat>
References: <20220728-rpi-analog-tv-properties-v7-0-7072a478c6b3@cerno.tech>
 <20220728-rpi-analog-tv-properties-v7-15-7072a478c6b3@cerno.tech>
 <262f0953-1e05-e68e-3e96-2ac2132a1e57@tronnes.org>
 <ba532387-6329-c57a-1fa2-627b2cf40281@tronnes.org>
 <b6b7c689-a50f-cd63-7e48-8739b0d91e61@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <b6b7c689-a50f-cd63-7e48-8739b0d91e61@gmail.com>
Precedence: bulk

On Tue, Nov 08, 2022 at 10:27:17PM +0100, Mateusz Kwiatkowski wrote:
> Hi Noralf,
>=20
> W dniu 8.11.2022 o 10:38, Noralf Tr=F8nnes pisze:
> >
> > Den 07.11.2022 19.03, skrev Noralf Tr=F8nnes:
> >>
> >> Den 07.11.2022 15.16, skrev Maxime Ripard:
> >>> Now that we can easily extend the named modes list, let's add a few m=
ore
> >>> analog TV modes that were used in the wild, and some unit tests to ma=
ke
> >>> sure it works as intended.
> >>>
> >>> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> >>>
> >>> ---
> >>> Changes in v6:
> >>> - Renamed the tests to follow DRM test naming convention
> >>>
> >>> Changes in v5:
> >>> - Switched to KUNIT_ASSERT_NOT_NULL
> >>> ---
> >>>  drivers/gpu/drm/drm_modes.c                     |  2 +
> >>>  drivers/gpu/drm/tests/drm_client_modeset_test.c | 54 +++++++++++++++=
++++++++++
> >>>  2 files changed, 56 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> >>> index 49441cabdd9d..17c5b6108103 100644
> >>> --- a/drivers/gpu/drm/drm_modes.c
> >>> +++ b/drivers/gpu/drm/drm_modes.c
> >>> @@ -2272,7 +2272,9 @@ struct drm_named_mode {
> >>> =20
> >>>  static const struct drm_named_mode drm_named_modes[] =3D {
> >>>  	NAMED_MODE("NTSC", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE, DRM_MO=
DE_TV_MODE_NTSC),
> >>> +	NAMED_MODE("NTSC-J", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE, DRM_=
MODE_TV_MODE_NTSC_J),
> >>>  	NAMED_MODE("PAL", 13500, 720, 576, DRM_MODE_FLAG_INTERLACE, DRM_MOD=
E_TV_MODE_PAL),
> >>> +	NAMED_MODE("PAL-M", 13500, 720, 480, DRM_MODE_FLAG_INTERLACE, DRM_M=
ODE_TV_MODE_PAL_M),
> >>>  };
> >> I'm now having second thoughts about the tv_mode commandline option. C=
an
> >> we just add all the variants to this table and drop the tv_mode option?
> >> IMO this will be more user friendly and less confusing.
> >>
> > One downside of this is that it's not possible to force connector status
> > when using named modes, but I think it would be better to have a force
> > option than a tv_mode option. A lot of userspace treats unknown status
> > as disconnected.
> >
> > Anyone know if it's possible to set the connector status sysfs file
> > using a udev rule?
> >
> > Noralf.
>=20
> I think that leaving named modes only would be a bit limiting. There are =
use
> cases for custom modes, e.g. we might want progressive 240p "NTSC" (like =
80s/90s
> home computers and video game consoles) or the modes with non-13.5MHz pix=
el
> clock that Geert requested with Amiga in mind.

Yeah, it was one of the early requirements that we would be allowed to
fill in any analog mode on the command line, so just having the named
modes with the 480i and 576i modes won't really work for that.

> I'm not sure if the current cmdline-to-drm_mode conversion is flexible en=
ough
> to meaningfully facilitate those, but we're at least getting the syntax d=
own.

It might require a bit of plumbing to get
drm_mode_create_from_cmdline_mode() to add the mode if tv_mode_specified
is set, but it's probably it.

Maxime