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:   Thu, 13 Oct 2022 10:36:38 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>
Cc:     Jernej Skrabec <jernej.skrabec@gmail.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Karol Herbst <kherbst@redhat.com>,
        Samuel Holland <samuel@sholland.org>,
        Lyude Paul <lyude@redhat.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Emma Anholt <emma@anholt.net>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Ben Skeggs <bskeggs@redhat.com>,
        David Airlie <airlied@linux.ie>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        linux-arm-kernel@lists.infradead.org,
        dri-devel@lists.freedesktop.org,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        intel-gfx@lists.freedesktop.org, linux-sunxi@lists.linux.dev,
        Hans de Goede <hdegoede@redhat.com>,
        nouveau@lists.freedesktop.org,
        Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        linux-kernel@vger.kernel.org, Dom Cobley <dom@raspberrypi.com>,
        Phil Elwell <phil@raspberrypi.com>
Subject: Re: [PATCH v4 11/30] drm/modes: Add a function to generate analog
 display modes
Message-ID: <20221013083638.kloiaxervnhii7ew@houat>
References: <20220728-rpi-analog-tv-properties-v4-0-60d38873f782@cerno.tech>
 <20220728-rpi-analog-tv-properties-v4-11-60d38873f782@cerno.tech>
 <0aa690b8-988a-878f-4d4f-d391295bc591@tronnes.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="l3aewuzyqmtjebmf"
Content-Disposition: inline
In-Reply-To: <0aa690b8-988a-878f-4d4f-d391295bc591@tronnes.org>
Precedence: bulk


--l3aewuzyqmtjebmf
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Noralf,

On Sat, Oct 01, 2022 at 02:52:06PM +0200, Noralf Tr=F8nnes wrote:
> Den 29.09.2022 18.31, skrev Maxime Ripard:
> > Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and
> > 625-lines modes in their drivers.
> >=20
> > Since those modes are fairly standard, and that we'll need to use them
> > in more places in the future, it makes sense to move their definition
> > into the core framework.
> >=20
> > However, analog display usually have fairly loose timings requirements,
> > the only discrete parameters being the total number of lines and pixel
> > clock frequency. Thus, we created a function that will create a display
> > mode from the standard, the pixel frequency and the active area.
> >=20
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> >=20
> > ---
> >=20
> > Changes in v4:
> > - Reworded the line length check comment
> > - Switch to HZ_PER_KHZ in tests
> > - Use previous timing to fill our mode
> > - Move the number of lines check earlier
> > ---
> >  drivers/gpu/drm/drm_modes.c            | 474 +++++++++++++++++++++++++=
++++++++
> >  drivers/gpu/drm/tests/Makefile         |   1 +
> >  drivers/gpu/drm/tests/drm_modes_test.c | 144 ++++++++++
> >  include/drm/drm_modes.h                |  17 ++
> >  4 files changed, 636 insertions(+)
> >=20
>=20
> I haven't followed the discussion on this patch, but it seems rather
> excessive to add over 600 lines of code (including tests) to add 2 fixed
> modes. And it's very difficult to see from the code what the actual
> display mode timings really are, which would be useful for other
> developers down the road wanting to use them.
>=20
> Why not just hardcode the modes?

Yeah, I have kind of the same feeling tbh, but it was asked back on the
v1 to ease the transition of old fbdev drivers, since they will need
such a function:
https://lore.kernel.org/dri-devel/CAMuHMdUrwzPYjA0wdR7ADj5Ov6+m03JbnY8fBYzR=
YyWDuNm5=3Dg@mail.gmail.com/

Maxime

--l3aewuzyqmtjebmf
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY0fOFgAKCRDj7w1vZxhR
xRPTAPsHbpjwggNPU38hdpfJ1NG62HfnGiMtgUpIeY78VLwdvAD+ID6+idT6/neO
8RrFdtowwtZdPyNPHMeV6f3x0uD7BgM=
=iymO
-----END PGP SIGNATURE-----

--l3aewuzyqmtjebmf--