Hi Maxime,

On Thu, Aug 18, 2022 at 5:34 PM Maxime Ripard [off-list ref] wrote:

On Thu, Aug 18, 2022 at 05:20:42PM +0200, Geert Uytterhoeven wrote:

quoted

On Thu, Aug 18, 2022 at 4:54 PM Maxime Ripard [off-list ref] wrote:

quoted

On Wed, Aug 17, 2022 at 04:04:24PM +0200, Geert Uytterhoeven wrote:

quoted

On Wed, Aug 17, 2022 at 3:19 PM Maxime Ripard [off-list ref] wrote:

quoted

On Wed, Aug 17, 2022 at 03:05:52PM +0200, Geert Uytterhoeven wrote:

quoted

On Wed, Aug 17, 2022 at 1:15 PM Maxime Ripard [off-list ref] wrote:

quoted

On Wed, Aug 17, 2022 at 10:35:07AM +0200, Geert Uytterhoeven wrote:

quoted

On Wed, Aug 17, 2022 at 9:47 AM Maxime Ripard [off-list ref] wrote:

quoted

On Wed, Aug 17, 2022 at 09:31:18AM +0200, Geert Uytterhoeven wrote:

quoted

On Tue, Aug 16, 2022 at 5:50 PM Maxime Ripard [off-list ref] wrote:

quoted

On Tue, Aug 16, 2022 at 04:43:44PM +0200, Geert Uytterhoeven wrote:

quoted

quoted

quoted

quoted

quoted

Either you have to add them here (e.g. "hd720p50" and "hd720p60"), or
handle them through "@<refresh>".  The latter would impact "[PATCH v1
09/35] drm/modes: Move named modes parsing to a separate function", as
currently a named mode and a refresh rate can't be specified both.

I think the former would make more sense. It simplifies a bit the
parser, and we're going to use a named mode anyway.

quoted

As "[PATCH v1 34/35] drm/modes: Introduce the tv_mode property as a
command-line option" uses a separate "tv_mode" option, and not the main
mode name, I think you want to add them here.

It's a separate story I think, we could have a named mode hd720p50,
which would be equivalent to 1280x720,tv_mode=hd720p

So where's the field rate in "1280x720,tv_mode=hd720p"?

Yeah, sorry I meant 1280x720@50,tv_mode=hd720p

Above you said "I think the former would make more sense", so that
should be "1280x720,tv_mode=hd720p50"?

No, 720p at 50Hz would be either hd720p50 or 1280x720@50,tv_mode=hd720p
and 60Hz would be hd720p60 or 1280x720@60,tv_mode=hd720p

I disagree: hd720p50 and hd720p60 are different TV modes.

I agree, and I don't see how that command-line doesn't express that?

Oh, I see what you mean: yes, it expresses that.
But it is inconsistent with the NTSC/PAL/SECAM/hd{480,576}[ip] modes,
where the TV mode specifies both number of lines and frame rate.

Only if we're using a named mode, and naming is hard :)

That's not true: "640x480,tv_mode=PAL-N" would give me a mode with
625 lines and 25 frames/s, "640x480,tv_mode=PAL-M" would give me a
mode with 525 lines and 30 frames/s.

In that series, "640x480,tv_mode=PAL-N" would be rejected as invalid:

https://lore.kernel.org/dri-devel/20220728-rpi-analog-tv-properties-v1-14-3d53ae722097@cerno.tech/ (local)

It would become supported once the ideas from thread "[PATCH v1 04/35]
drm/modes: Introduce 480i and 576i modes" are implemented...

Indeed, but I'm still not sure what your concern is here.
"640x480,tv_mode=PAL-N" and "640x480,tv_mode=PAL-M" are both fairly
obvious.

You were initially saying that you had concern over the inconsistency of
NTSC/PAL/SECAM where the TV mode would specify a number of lines and
frame rate, but hd720p50 also specifies a number of line and frame rate?

My concern is that you want to call the TV mode "hd720p", which
does not dictate the frame rate.
I would like to have both "720p50" and "720p60", as they do dictate
the frame rate, like all the non-hd modes.

But they don't?

The refresh rate is part of the drm_display_mode, whereas that property
is metadata and entirely separate from the display mode.

You can even change it without changing the mode at all

Yes, the refresh rate is part of drm_display_mode.  Vdisplay also
is, but that doesn't mean you can set it to e.g. 700 when using
"tv_mode=PAL-B". Some (combination of) parameters in drm_display_mode
are dictated by the tv_mode.

Perhaps the meaning of "tv_mode" should be clarified? What does it
really mean, and what parameters does it (not) constrain?

For e.g. "PAL-B", I know it's a mode with 625 lines and 30 frames/s
(60 fields/s).
For "hd720p" I know it is an analog mode with 750 lines, but it's still
ambiguous, as I don't know if it is the variant with 60 or 50 frames/s.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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