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, 21 Sep 2022 17:05:19 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Mateusz Kwiatkowski <kfyatek@gmail.com>
Cc:     Ben Skeggs <bskeggs@redhat.com>, David Airlie <airlied@linux.ie>,
        Chen-Yu Tsai <wens@csie.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Lyude Paul <lyude@redhat.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Samuel Holland <samuel@sholland.org>,
        Karol Herbst <kherbst@redhat.com>,
        Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>,
        Emma Anholt <emma@anholt.net>, Daniel Vetter <daniel@ffwll.ch>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        linux-arm-kernel@lists.infradead.org,
        Phil Elwell <phil@raspberrypi.com>,
        intel-gfx@lists.freedesktop.org,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        dri-devel@lists.freedesktop.org, Dom Cobley <dom@raspberrypi.com>,
        linux-kernel@vger.kernel.org, nouveau@lists.freedesktop.org,
        linux-sunxi@lists.linux.dev,
        Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH v2 10/41] drm/modes: Add a function to generate analog
 display modes
Message-ID: <20220921150519.hijgemsk7o2tdrb3@houat>
References: <20220728-rpi-analog-tv-properties-v2-0-459522d653a7@cerno.tech>
 <20220728-rpi-analog-tv-properties-v2-10-459522d653a7@cerno.tech>
 <242d272b-5b79-986c-9aaf-64e62f6b37ff@gmail.com>
 <20220905133755.gcmmntg3wnecyqjq@houat>
 <10ce686a-d7c8-9ce4-3979-735ad8eab3b5@gmail.com>
 <20220907143421.4iopqwhp3yfircsh@houat>
 <dc1d9499-d4d5-1032-f39f-d4ac4cbb8412@gmail.com>
 <20220909135444.5oi6oh6nqwuke3jl@houat>
 <79ab3fef-fdaa-e191-d839-4af88191e672@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="f6nvjv3uvyvilujx"
Content-Disposition: inline
In-Reply-To: <79ab3fef-fdaa-e191-d839-4af88191e672@gmail.com>
Precedence: bulk


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

Hi,

On Sun, Sep 11, 2022 at 06:48:50AM +0200, Mateusz Kwiatkowski wrote:
> >> Those extra vbp lines will be treated as a black bar at the top of the=
 frame,
> >> and extra vfp lines will be at the bottom of the frame.
> >>
> >> However if someone specifies e.g. 720x604, there's nothing more you co=
uld
> >> remove from vfp, so your only option is to reduce vbp compared to the =
standard
> >> mode, so you'll end up with (vfp=3D=3D4, vsync=3D=3D6, vbp=3D=3D11). T=
he image will not be
> >> centered, the topmost lines will get cropped out, but that's the best =
we can do
> >> and if someone is requesting such resolution, they most likely want to=
 actually
> >> access the VBI to e.g. emit teletext.
> >>
> >> Your current code always starts at (vfp=3D=3D5 or 6, vsync=3D6, vbp=3D=
=3D6) and then
> >> increases both vfp and vbp proportionately. This puts vsync dead cente=
r in the
> >> VBI, which is not how it's supposed to be - and that in turn causes th=
e image
> >> to be significantly shifted upwards.
> >>
> >> I hope this makes more sense to you now.
> >
> > I'm really struggling with this, so thanks for explaining this further
> > (and patiently ;))
> >
> > If I get this right, what you'd like to change is this part of the
> > calculus (simplified a bit, and using PAL, 576i):
> >
> >=A0=A0 vfp_min =3D params->vfp_lines.even + params->vfp_lines.odd; // 5
> >=A0=A0 vbp_min =3D params->vbp_lines.even + params->vbp_lines.odd; // 6
> >=A0=A0 vslen =3D params->vslen_lines.even + params->vslen_lines.odd; // 6
> >
> >=A0=A0 porches =3D params->num_lines - vactive - vslen; // 43
> >=A0=A0 porches_rem =3D porches - vfp_min - vbp_min; // 32
> >
> >=A0=A0 vfp =3D vfp_min + (porches_rem / 2); // 21
> >=A0=A0 vbp =3D porches - vfp; // 22
> >
> > Which is indeed having sync centered.
> >
> > I initially changed it to:
> >
> >=A0=A0 vfp =3D vfp_min; // 6
> >=A0=A0 vbp =3D num_lines - vactive - vslen - vfp; // 38
> >
> > Which is close enough for 576i, but at 480i/50Hz would end up with 134,
> > so still fairly far off.
> >
> > I guess your suggestion would be along the line of:
> >
> >=A0=A0 vfp_min =3D params->vfp_lines.even + params->vfp_lines.odd; // 5
> >=A0=A0 vbp_min =3D params->vbp_lines.even + params->vbp_lines.odd; // 38
> >=A0=A0 vslen =3D params->vslen_lines.even + params->vslen_lines.odd; // 6
> >
> >=A0=A0 porches =3D params->num_lines - vactive - vslen; // 0
> >=A0=A0 porches_rem =3D porches - vfp_min - vbp_min; // 0
> >
> >=A0=A0 vfp =3D vfp_min + (porches_rem / 2); // 5
> >=A0=A0 vbp =3D porches - vfp; // 38
> >
> > Which is still close enough for 576i, but for 480i would end up with:
> >
> >=A0=A0 porches =3D params->num_lines - vactive - vslen; // 139
> >=A0=A0 porches_rem =3D porches - vfp_min - vbp_min; // 96
> >
> >=A0=A0 vfp =3D vfp_min + (porches_rem / 2); // 53
> >=A0=A0 vbp =3D porches - vfp; // 86
> >
> > Right?
>=20
> Yes. And if that's supposed to mean 480i in 50 Hz "PAL" mode, that's also
> "close enough" to the values I suggested above.
>=20
> If you substitute values for true 60 Hz "NTSC" 480i, you should also get =
values
> that are "close enough" to the official spec.
>=20
> The only thing I'd conceptually change is that the 38 lines is not really
> "vbp_min". It's more like "vbp_typ". As I mentioned above, we may want to=
 lower
> this value if someone wants more active lines than the official 486/576.

porches_rem is an int, so if vactive > (num_lines + vslen + vfp_min +
vbp_min), porches_rem is going to be negative and we'll remove equally
between vfp and vbp to match what's been asked

So I believe this should work fine?

Maxime

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

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

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYysoLwAKCRDj7w1vZxhR
xURjAQCbncH6Fc/QtaLetEslcTLx/26P2Sm8OvJplugFXHTJ7QD/dTht2uJh+cg/
SgzXrpjHpRt/MIrMNo1Y+QG6L3LLpgY=
=4e/k
-----END PGP SIGNATURE-----

--f6nvjv3uvyvilujx--