From: Maxime Ripard <maxime@cerno.tech>
To: Mateusz Kwiatkowski <kfyatek@gmail.com>
Cc: "Karol Herbst" <kherbst@redhat.com>,
"David Airlie" <airlied@linux.ie>,
nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
"Phil Elwell" <phil@raspberrypi.com>,
"Emma Anholt" <emma@anholt.net>,
"Samuel Holland" <samuel@sholland.org>,
"Jernej Skrabec" <jernej.skrabec@gmail.com>,
"Chen-Yu Tsai" <wens@csie.org>,
"Geert Uytterhoeven" <geert@linux-m68k.org>,
"Ben Skeggs" <bskeggs@redhat.com>,
linux-sunxi@lists.linux.dev,
"Thomas Zimmermann" <tzimmermann@suse.de>,
intel-gfx@lists.freedesktop.org,
"Hans de Goede" <hdegoede@redhat.com>,
"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
linux-arm-kernel@lists.infradead.org,
"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
"Dom Cobley" <dom@raspberrypi.com>,
"Dave Stevenson" <dave.stevenson@raspberrypi.com>,
linux-kernel@vger.kernel.org,
"Noralf Trønnes" <noralf@tronnes.org>
Subject: Re: [PATCH v2 10/41] drm/modes: Add a function to generate analog display modes
Date: Fri, 9 Sep 2022 15:54:44 +0200 [thread overview]
Message-ID: <20220909135444.5oi6oh6nqwuke3jl@houat> (raw)
In-Reply-To: <dc1d9499-d4d5-1032-f39f-d4ac4cbb8412@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7126 bytes --]
Hi,
On Wed, Sep 07, 2022 at 11:31:21PM +0200, Mateusz Kwiatkowski wrote:
> W dniu 7.09.2022 o 16:34, Maxime Ripard pisze:
> > On Mon, Sep 05, 2022 at 06:44:42PM +0200, Mateusz Kwiatkowski wrote:
> >> Hi Maxime,
> >>
> >> W dniu 5.09.2022 o 15:37, Maxime Ripard pisze:
> >>>>> + vfp = vfp_min + (porches_rem / 2);
> >>>>> + vbp = porches - vfp;
> >>>>
> >>>> Relative position of the vertical sync within the VBI effectively moves the
> >>>> image up and down. Adding that (porches_rem / 2) moves the image up off center
> >>>> by that many pixels. I'd keep the VFP always at minimum to keep the image
> >>>> centered.
> >>>
> >>> And you would increase the back porch only then?
> >>
> >> Well, increasing vbp only gives a centered image with the default 480i/576i
> >> resolutions. However, only ever changing vbp will cause the image to be always
> >> at the bottom of the screen when the active line count is decreased (e.g.
> >> setting the resolution to 720x480 but for 50Hz "PAL" - like many game consoles
> >> did back in the day).
> >>
> >> I believe that the perfect solution would:
> >>
> >> - Use the canonical / standard-defined blanking line counts for the standard
> >> vertical resolutions (480/486/576)
> >> - Increase vfp and vbp from there by the same number if a smaller number of
> >> active lines is specified, so that the resulting image is centered
> >> - Likewise, decrease vfp and vbp by the same number if the active line number
> >> is larger and there is still leeway (this should allow for seamless handling
> >> of 480i vs. 486i for 60 Hz "NTSC")
> >
> > I'm not sure I understand how that's any different than the code you
> > initially commented on.
> >
> > I would start by taking the entire blanking area, remove the sync
> > period. We only have the two porches now, and I'm starting from the
> > minimum, adding as many pixels in both (unless it's not an even number,
> > in which case the backporch will have the extra pixel).
> >
> > Isn't it the same thing?
> > [...]
> > Unless you only want me to consider the front porch maximum?
>
> I think you're confusing the "post-equalizing pulses" with the "vertical back
> porch" a little bit. The "vertical back porch" includes both the post-equalizing
> pulses and the entire rest of the VBI period, for the standard resolutions at
> least.
>
> The "canonical" modelines (at least for vc4's VEC, see the notes below):
>
> - (vfp==4, vsync==6, vbp==39) for 576i
> - (vfp==7, vsync==6, vbp==32) for 480i
> - (vfp==5, vsync==6, vbp==28) for 486i (full frame NTSC as originally specified)
>
> The numbers for vfp don't exactly match the theoretical values, because:
>
> - VEC actually adds a half-line pulse on top of VFP_ODD, and in the 625-line
> mode also on top of VFP_EVEN (not always, but let's not dwell too much)
> - Conversely, VEC subtracts the half-line pulse from VSYNC_ODD and VSYNC_EVEN
> in the 625-line mode
> - SMPTE S170M (see https://www.itu.int/rec/R-REC-BT.1700-0-200502-I/en) defines
> that active picture for NTSC is on lines 21-263 and 283-525; 263 and 283 are
> half-lines that are represented as full lines in the "486i" spec
It's going to be a bit difficult to match that into a drm_display_mode,
since as far I understand it, all the timings are the sum of the timings
of both fields in interlaced. I guess we'll have to be close enough.
> - SMPTE 314M, which is the spec for DV, defines the 480 active lines as lines
> 23-262 and 285-524; see Table 20 on page 26 in
> https://last.hit.bme.hu/download/firtha/video/SMPTE/SMPTE-314M%20DV25-50.pdf;
> this means that the standard 480i frame shaves off four topmost and two
> bottommost lines (2 and 1 per field) of the 486i full frame
I'm still struggling a bit to match that into front porch, sync period
and back porch. I guess the sync period is easy since it's pretty much
fixed. That line 0-23 is the entire blanking area though, right?
> Note that the half-line pulses in vfp/vsync may be generated in a different way
> on encoders other than vc4's VEC. Maybe we should define some concrete
> semantics for vfp/vsync in analog TV modes, and compensate for that in the
> drivers. But anyway, that's a separate issue.
>
> My point is that, to get a centered image, you can then proportionately add
> values to those "canonical" vfp/vbp values. For example if someone specifies
> 720x480 frame, but 50 Hz PAL, you should set (vfp==52, vsync==6, vbp==87).
In this case, you add 48 both front porches, right? How is that
proportionate?
> 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 could
> remove from vfp, so your only option is to reduce vbp compared to the standard
> mode, so you'll end up with (vfp==4, vsync==6, vbp==11). The 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==5 or 6, vsync=6, vbp==6) and then
> increases both vfp and vbp proportionately. This puts vsync dead center in the
> VBI, which is not how it's supposed to be - and that in turn causes the 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):
vfp_min = params->vfp_lines.even + params->vfp_lines.odd; // 5
vbp_min = params->vbp_lines.even + params->vbp_lines.odd; // 6
vslen = params->vslen_lines.even + params->vslen_lines.odd; // 6
porches = params->num_lines - vactive - vslen; // 43
porches_rem = porches - vfp_min - vbp_min; // 32
vfp = vfp_min + (porches_rem / 2); // 21
vbp = porches - vfp; // 22
Which is indeed having sync centered.
I initially changed it to:
vfp = vfp_min; // 6
vbp = 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:
vfp_min = params->vfp_lines.even + params->vfp_lines.odd; // 5
vbp_min = params->vbp_lines.even + params->vbp_lines.odd; // 38
vslen = params->vslen_lines.even + params->vslen_lines.odd; // 6
porches = params->num_lines - vactive - vslen; // 0
porches_rem = porches - vfp_min - vbp_min; // 0
vfp = vfp_min + (porches_rem / 2); // 5
vbp = porches - vfp; // 38
Which is still close enough for 576i, but for 480i would end up with:
porches = params->num_lines - vactive - vslen; // 139
porches_rem = porches - vfp_min - vbp_min; // 96
vfp = vfp_min + (porches_rem / 2); // 53
vbp = porches - vfp; // 86
Right?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2022-09-09 13:55 UTC|newest]
Thread overview: 128+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-29 13:11 [PATCH v2 00/41] drm: Analog TV Improvements Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 01/41] drm/tests: Order Kunit tests in Makefile Maxime Ripard
2022-08-29 18:46 ` Noralf Trønnes
2022-08-29 19:02 ` Konstantin Ryabitsev
2022-08-30 8:30 ` Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 02/41] drm/tests: Add Kunit Helpers Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 03/41] drm/atomic-helper: Rename drm_atomic_helper_connector_tv_reset to avoid ambiguity Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 04/41] drm/connector: Rename subconnector state variable Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 05/41] drm/atomic: Add TV subconnector property to get/set_property Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 06/41] drm/connector: Rename legacy TV property Maxime Ripard
2022-08-30 19:27 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 07/41] drm/connector: Only register TV mode property if present Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 08/41] drm/connector: Rename drm_mode_create_tv_properties Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 09/41] drm/connector: Add TV standard property Maxime Ripard
2022-09-01 22:00 ` Mateusz Kwiatkowski
2022-09-02 7:35 ` Geert Uytterhoeven
2022-09-07 12:11 ` Maxime Ripard
2022-09-07 12:10 ` Maxime Ripard
2022-09-07 19:52 ` Mateusz Kwiatkowski
2022-09-09 9:46 ` Maxime Ripard
2022-09-11 4:32 ` Mateusz Kwiatkowski
2022-09-05 10:18 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 10/41] drm/modes: Add a function to generate analog display modes Maxime Ripard
2022-08-30 13:01 ` Maíra Canal
2022-09-08 11:10 ` Maxime Ripard
2022-08-31 1:44 ` Mateusz Kwiatkowski
2022-08-31 8:14 ` Geert Uytterhoeven
2022-09-05 13:32 ` Maxime Ripard
2022-09-05 16:32 ` Mateusz Kwiatkowski
2022-09-07 14:38 ` Maxime Ripard
2022-09-05 13:37 ` Maxime Ripard
2022-09-05 16:44 ` Mateusz Kwiatkowski
2022-09-07 14:34 ` Maxime Ripard
2022-09-07 21:31 ` Mateusz Kwiatkowski
2022-09-09 13:54 ` Maxime Ripard [this message]
2022-09-11 4:48 ` Mateusz Kwiatkowski
2022-09-11 4:51 ` Mateusz Kwiatkowski
2022-09-21 15:05 ` Maxime Ripard
2022-09-09 14:00 ` Maxime Ripard
2022-09-11 4:30 ` kFYatek
2022-09-21 14:26 ` Maxime Ripard
2022-09-01 19:09 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 11/41] drm/modes: Only consider bpp and refresh before options Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 12/41] drm/modes: parse_cmdline: Add support for named modes containing dashes Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 13/41] drm/client: Add some tests for drm_connector_pick_cmdline_mode() Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 14/41] drm/modes: Move named modes parsing to a separate function Maxime Ripard
2022-08-30 10:06 ` Geert Uytterhoeven
2022-08-30 10:43 ` Jani Nikula
2022-08-30 12:03 ` Maxime Ripard
2022-08-30 13:36 ` Jani Nikula
2022-09-07 8:39 ` Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 15/41] drm/modes: Switch to named mode descriptors Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 16/41] drm/modes: Fill drm_cmdline mode from named modes Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 17/41] drm/connector: Add pixel clock to cmdline mode Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 18/41] drm/connector: Add a function to lookup a TV mode by its name Maxime Ripard
2022-08-31 19:14 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 19/41] drm/modes: Introduce the tv_mode property as a command-line option Maxime Ripard
2022-08-30 12:34 ` Maíra Canal
2022-08-30 12:44 ` Maíra Canal
2022-09-01 22:46 ` Mateusz Kwiatkowski
2022-09-05 14:28 ` Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 20/41] drm/modes: Properly generate a drm_display_mode from a named mode Maxime Ripard
2022-09-01 22:52 ` Mateusz Kwiatkowski
2022-08-29 13:11 ` [PATCH v2 21/41] drm/modes: Introduce more named modes Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 22/41] drm/atomic-helper: Add a TV properties reset helper Maxime Ripard
2022-08-30 18:40 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 23/41] drm/atomic-helper: Add an analog TV atomic_check implementation Maxime Ripard
2022-08-30 18:49 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 24/41] drm/vc4: vec: Remove empty mode_fixup Maxime Ripard
2022-08-30 15:23 ` Noralf Trønnes
2022-09-07 8:34 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 25/41] drm/vc4: vec: Convert to atomic helpers Maxime Ripard
2022-08-30 15:24 ` Noralf Trønnes
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 26/41] drm/vc4: vec: Refactor VEC TV mode setting Maxime Ripard
2022-08-30 15:29 ` Noralf Trønnes
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 27/41] drm/vc4: vec: Remove redundant atomic_mode_set Maxime Ripard
2022-08-30 15:45 ` Noralf Trønnes
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 28/41] drm/vc4: vec: Fix timings for VEC modes Maxime Ripard
2022-08-30 18:20 ` Noralf Trønnes
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 29/41] drm/vc4: vec: Switch for common modes Maxime Ripard
2022-08-30 18:36 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 30/41] drm/vc4: vec: Fix definition of PAL-M mode Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 31/41] drm/vc4: vec: Use TV Reset implementation Maxime Ripard
2022-08-30 18:51 ` Noralf Trønnes
2022-08-29 13:11 ` [PATCH v2 32/41] drm/vc4: vec: Convert to the new TV mode property Maxime Ripard
2022-08-30 19:01 ` Noralf Trønnes
2022-09-08 11:23 ` Maxime Ripard
2022-09-08 11:31 ` Mateusz Kwiatkowski
2022-09-08 12:16 ` Maxime Ripard
2022-09-08 11:34 ` Noralf Trønnes
2022-08-31 2:23 ` Mateusz Kwiatkowski
2022-09-08 13:18 ` Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 33/41] drm/vc4: vec: Add support for more analog TV standards Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 34/41] drm/sun4i: tv: Remove unused mode_valid Maxime Ripard
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 35/41] drm/sun4i: tv: Convert to atomic hooks Maxime Ripard
2022-09-06 20:02 ` Jernej Škrabec
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 36/41] drm/sun4i: tv: Merge mode_set into atomic_enable Maxime Ripard
2022-09-06 20:04 ` Jernej Škrabec
2022-09-07 7:41 ` Maxime Ripard
2022-09-07 15:09 ` Jernej Škrabec
2022-09-08 14:02 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 37/41] drm/sun4i: tv: Remove useless function Maxime Ripard
2022-09-06 20:06 ` Jernej Škrabec
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 38/41] drm/sun4i: tv: Remove useless destroy function Maxime Ripard
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 39/41] drm/sun4i: tv: Rename error label Maxime Ripard
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 40/41] drm/sun4i: tv: Add missing reset assertion Maxime Ripard
2022-09-07 8:35 ` (subset) " Maxime Ripard
2022-08-29 13:11 ` [PATCH v2 41/41] drm/sun4i: tv: Convert to the new TV mode property Maxime Ripard
2022-09-01 19:35 ` [PATCH v2 00/41] drm: Analog TV Improvements Noralf Trønnes
2022-09-02 11:28 ` Noralf Trønnes
2022-09-05 14:57 ` Maxime Ripard
2022-09-05 15:17 ` Noralf Trønnes
2022-09-07 9:58 ` Maxime Ripard
2022-09-07 10:56 ` Noralf Trønnes
2022-09-07 10:36 ` Stefan Wahren
2022-09-07 16:44 ` Noralf Trønnes
2022-09-10 15:34 ` Noralf Trønnes
2022-09-21 14:03 ` Maxime Ripard
2022-09-24 15:33 ` Noralf Trønnes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220909135444.5oi6oh6nqwuke3jl@houat \
--to=maxime@cerno.tech \
--cc=airlied@linux.ie \
--cc=bskeggs@redhat.com \
--cc=dave.stevenson@raspberrypi.com \
--cc=dom@raspberrypi.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=emma@anholt.net \
--cc=geert@linux-m68k.org \
--cc=hdegoede@redhat.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jernej.skrabec@gmail.com \
--cc=kfyatek@gmail.com \
--cc=kherbst@redhat.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=noralf@tronnes.org \
--cc=nouveau@lists.freedesktop.org \
--cc=phil@raspberrypi.com \
--cc=rodrigo.vivi@intel.com \
--cc=samuel@sholland.org \
--cc=tvrtko.ursulin@linux.intel.com \
--cc=tzimmermann@suse.de \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).