Re: [PATCH v5 13/22] drm/modes: Introduce the tv_mode property as a command-line option

From: Mateusz Kwiatkowski <kfyatek@gmail.com>
To: "Maxime Ripard" <maxime@cerno.tech>,
	"Karol Herbst" <kherbst@redhat.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"David Airlie" <airlied@linux.ie>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Lyude Paul" <lyude@redhat.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Emma Anholt" <emma@anholt.net>, "Chen-Yu Tsai" <wens@csie.org>,
	"Samuel Holland" <samuel@sholland.org>,
	"Ben Skeggs" <bskeggs@redhat.com>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Noralf Trønnes" <noralf@tronnes.org>
Cc: Dom Cobley <dom@raspberrypi.com>,
	linux-sunxi@lists.linux.dev,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	nouveau@lists.freedesktop.org,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org,
	Hans de Goede <hdegoede@redhat.com>,
	Phil Elwell <phil@raspberrypi.com>
Subject: Re: [PATCH v5 13/22] drm/modes: Introduce the tv_mode property as a command-line option
Date: Sun, 16 Oct 2022 19:51:50 +0200	[thread overview]
Message-ID: <fdeadf0d-8f38-8edf-ae92-e2d9c5aa90b4@gmail.com> (raw)
In-Reply-To: <20220728-rpi-analog-tv-properties-v5-13-d841cc64fe4b@cerno.tech>

Hi Maxime, Noralf & everyone,

I'd like to address Noralf here in particular, and refer to these discussions
from the past:

- https://lore.kernel.org/linux-arm-kernel/2f607c7d-6da1-c8df-1c02-8dd344a92343@gmail.com/
- https://lore.kernel.org/linux-arm-kernel/9e76a508-f469-a54d-ecd7-b5868ca99af4@tronnes.org/

> @@ -2230,20 +2256,22 @@ struct drm_named_mode {
>  	unsigned int xres;
>  	unsigned int yres;
>  	unsigned int flags;
> +	unsigned int tv_mode;
>  };

I saw that you (Noralf) opposed my suggestion about the DRM_MODE_TV_MODE_NONE
enum value in enum drm drm_connector_tv_mode. I get your argumentation, and I'm
not gonna argue, but I still don't like the fact that struct drm_named_mode now
includes a field that is only relevant for analog TV modes, has no "none" value,
and yet the type is supposed to be generic enough to be usable for other types
of outputs as well.

It's true that it can just be ignored (as Maxime mentioned in his response to
my e-mail linked above), and now the value of 0 corresponds to
DRM_MODE_TV_MODE_NTSC, which is a rather sane default, but it still feels messy
to me.

I'm not gonna force my opinion here, but I wanted to bring your attention to
this issue, maybe you have some other solution in mind for this problem. Or if
you don't see that as a problem at all, that's fine, too.

Best regards,
Mateusz Kwiatkowski

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel