[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20221018080806.mkw4zbzchlatxgwq@houat>
Date: Tue, 18 Oct 2022 10:08:06 +0200
From: Maxime Ripard <maxime@...no.tech>
To: kfyatek+publicgit@...il.com
Cc: Karol Herbst <kherbst@...hat.com>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
Daniel Vetter <daniel@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
David Airlie <airlied@...ux.ie>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Lyude Paul <lyude@...hat.com>, Emma Anholt <emma@...olt.net>,
Chen-Yu Tsai <wens@...e.org>,
Samuel Holland <samuel@...lland.org>,
Ben Skeggs <bskeggs@...hat.com>,
Thomas Zimmermann <tzimmermann@...e.de>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Dom Cobley <dom@...pberrypi.com>, linux-sunxi@...ts.linux.dev,
Dave Stevenson <dave.stevenson@...pberrypi.com>,
Noralf Trønnes <noralf@...nnes.org>,
intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
nouveau@...ts.freedesktop.org,
Geert Uytterhoeven <geert@...ux-m68k.org>,
linux-arm-kernel@...ts.infradead.org,
dri-devel@...ts.freedesktop.org,
Hans de Goede <hdegoede@...hat.com>,
Phil Elwell <phil@...pberrypi.com>
Subject: Re: [PATCH v5 06/22] drm/modes: Add a function to generate analog
display modes
Hi,
On Sun, Oct 16, 2022 at 07:34:12PM +0200, Mateusz Kwiatkowski wrote:
> Hi Maxime & everyone,
>
> Sorry for being inactive in the discussions about this patchset for the last
> couple of weeks.
>
> > +const static struct analog_parameters tv_modes_parameters[] = {
> > + TV_MODE_PARAMETER(DRM_MODE_ANALOG_NTSC,
> > + NTSC_LINES_NUMBER,
> > + NTSC_LINE_DURATION_NS,
> > + PARAM_RANGE(NTSC_HACT_DURATION_MIN_NS,
> > + NTSC_HACT_DURATION_TYP_NS,
> > + NTSC_HACT_DURATION_MAX_NS),
> > + PARAM_RANGE(NTSC_HFP_DURATION_MIN_NS,
> > + NTSC_HFP_DURATION_TYP_NS,
> > + NTSC_HFP_DURATION_MAX_NS),
> > + PARAM_RANGE(NTSC_HSLEN_DURATION_MIN_NS,
> > + NTSC_HSLEN_DURATION_TYP_NS,
> > + NTSC_HSLEN_DURATION_MAX_NS),
> > + PARAM_RANGE(NTSC_HBP_DURATION_MIN_NS,
> > + NTSC_HBP_DURATION_TYP_NS,
> > + NTSC_HBP_DURATION_MAX_NS),
> > + PARAM_RANGE(NTSC_HBLK_DURATION_MIN_NS,
> > + NTSC_HBLK_DURATION_TYP_NS,
> > + NTSC_HBLK_DURATION_MAX_NS),
> > + 16,
> > + PARAM_FIELD(3, 3),
> > + PARAM_FIELD(3, 3),
> > + PARAM_FIELD(16, 17)),
> > + TV_MODE_PARAMETER(DRM_MODE_ANALOG_PAL,
> > + PAL_LINES_NUMBER,
> > + PAL_LINE_DURATION_NS,
> > + PARAM_RANGE(PAL_HACT_DURATION_MIN_NS,
> > + PAL_HACT_DURATION_TYP_NS,
> > + PAL_HACT_DURATION_MAX_NS),
> > + PARAM_RANGE(PAL_HFP_DURATION_MIN_NS,
> > + PAL_HFP_DURATION_TYP_NS,
> > + PAL_HFP_DURATION_MAX_NS),
> > + PARAM_RANGE(PAL_HSLEN_DURATION_MIN_NS,
> > + PAL_HSLEN_DURATION_TYP_NS,
> > + PAL_HSLEN_DURATION_MAX_NS),
> > + PARAM_RANGE(PAL_HBP_DURATION_MIN_NS,
> > + PAL_HBP_DURATION_TYP_NS,
> > + PAL_HBP_DURATION_MAX_NS),
> > + PARAM_RANGE(PAL_HBLK_DURATION_MIN_NS,
> > + PAL_HBLK_DURATION_TYP_NS,
> > + PAL_HBLK_DURATION_MAX_NS),
> > + 12,
> > +
> > + /*
> > + * The front porch is actually 6 short sync
> > + * pulses for the even field, and 5 for the
> > + * odd field. Each sync takes half a life so
> > + * the odd field front porch is shorter by
> > + * half a line.
> > + *
> > + * In progressive, we're supposed to use 6
> > + * pulses, so we're fine there
> > + */
> > + PARAM_FIELD(3, 2),
> > +
> > + /*
> > + * The vsync length is 5 long sync pulses,
> > + * each field taking half a line. We're
> > + * shorter for both fields by half a line.
> > + *
> > + * In progressive, we're supposed to use 5
> > + * pulses, so we're off by half
> > + * a line.
> > + *
> > + * In interlace, we're now off by half a line
> > + * for the even field and one line for the odd
> > + * field.
> > + */
> > + PARAM_FIELD(3, 3),
> > +
> > + /*
> > + * The back porch starts with post-equalizing
> > + * pulses, consisting in 5 short sync pulses
> > + * for the even field, 4 for the odd field. In
> > + * progressive, it's 5 short syncs.
> > + *
> > + * In progressive, we thus have 2.5 lines,
> > + * plus the 0.5 line we were missing
> > + * previously, so we should use 3 lines.
> > + *
> > + * In interlace, the even field is in the
> > + * exact same case than progressive. For the
> > + * odd field, we should be using 2 lines but
> > + * we're one line short, so we'll make up for
> > + * it here by using 3.
> > + *
> > + * The entire blanking area is supposed to
> > + * take 25 lines, so we also need to account
> > + * for the rest of the blanking area that
> > + * can't be in either the front porch or sync
> > + * period.
> > + */
> > + PARAM_FIELD(19, 20)),
> > +};
>
> Nit: setting vbp limits like that makes it impossible to use
> drm_analog_tv_mode() to generate modes that include the VBI for e.g. emitting
> teletext.
>
> This probably doesn't matter, as it can still be created as a custom mode from
> userspace, hence I'm mentioning it as a nit.
Yeah, I think it's out of scope at least for now. Also, the compositor
should probably be aware of the margins being used to put the VBI data,
so expecting userspace to come up with the mode is probably best?
> > + * By convention, NSTC (aka 525/60) systems start with
>
> Typo: s/NSTC/NTSC/
Fixed, thanks
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists