diff mbox series

[v1,04/35] drm/modes: Introduce 480i and 576i modes

Message ID 20220728-rpi-analog-tv-properties-v1-4-3d53ae722097@cerno.tech (mailing list archive)
State New, archived
Headers show
Series drm: Analog TV Improvements | expand

Commit Message

Maxime Ripard July 29, 2022, 4:34 p.m. UTC
Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
modes in the drivers.

Since those modes are fairly standards, and that we'll need to use them in
more places in the future, let's move the meson definition into the
framework.

The meson one was chosen because vc4's isn't accurate and doesn't amount to
525 and 625 lines.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Comments

Jani Nikula Aug. 2, 2022, 1:58 p.m. UTC | #1
On Fri, 29 Jul 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> modes in the drivers.
>
> Since those modes are fairly standards, and that we'll need to use them in
> more places in the future, let's move the meson definition into the
> framework.

I think you should always expose interfaces, not data. Data is not an
interface, and I think this sets a bad example that will be cargo
culted.


BR,
Jani.

>
> The meson one was chosen because vc4's isn't accurate and doesn't amount to
> 525 and 625 lines.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 304004fb80aa..a4c1bd688338 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -48,6 +48,24 @@
>  
>  #include "drm_crtc_internal.h"
>  
> +const struct drm_display_mode drm_mode_480i = {
> +	DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
> +		 720, 739, 801, 858, 0,
> +		 480, 488, 494, 525, 0,
> +		 DRM_MODE_FLAG_INTERLACE),
> +	.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> +};
> +EXPORT_SYMBOL_GPL(drm_mode_480i);
> +
> +const struct drm_display_mode drm_mode_576i = {
> +	DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
> +		 720, 732, 795, 864, 0,
> +		 576, 580, 586, 625, 0,
> +		 DRM_MODE_FLAG_INTERLACE),
> +	.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> +};
> +EXPORT_SYMBOL_GPL(drm_mode_576i);
> +
>  /**
>   * drm_mode_debug_printmodeline - print a mode to dmesg
>   * @mode: mode to print
> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> index 8110a6e39320..98ec3e563155 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> @@ -45,21 +45,11 @@ struct meson_encoder_cvbs {
>  struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT] = {
>  	{ /* PAL */
>  		.enci = &meson_cvbs_enci_pal,
> -		.mode = {
> -			DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
> -				 720, 732, 795, 864, 0, 576, 580, 586, 625, 0,
> -				 DRM_MODE_FLAG_INTERLACE),
> -			.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> -		},
> +		.mode = &drm_mode_576i,
>  	},
>  	{ /* NTSC */
>  		.enci = &meson_cvbs_enci_ntsc,
> -		.mode = {
> -			DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
> -				720, 739, 801, 858, 0, 480, 488, 494, 525, 0,
> -				DRM_MODE_FLAG_INTERLACE),
> -			.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> -		},
> +		.mode = &drm_mode_480i,
>  	},
>  };
>  
> @@ -71,7 +61,7 @@ meson_cvbs_get_mode(const struct drm_display_mode *req_mode)
>  	for (i = 0; i < MESON_CVBS_MODES_COUNT; ++i) {
>  		struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>  
> -		if (drm_mode_match(req_mode, &meson_mode->mode,
> +		if (drm_mode_match(req_mode, meson_mode->mode,
>  				   DRM_MODE_MATCH_TIMINGS |
>  				   DRM_MODE_MATCH_CLOCK |
>  				   DRM_MODE_MATCH_FLAGS |
> @@ -104,7 +94,7 @@ static int meson_encoder_cvbs_get_modes(struct drm_bridge *bridge,
>  	for (i = 0; i < MESON_CVBS_MODES_COUNT; ++i) {
>  		struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>  
> -		mode = drm_mode_duplicate(priv->drm, &meson_mode->mode);
> +		mode = drm_mode_duplicate(priv->drm, meson_mode->mode);
>  		if (!mode) {
>  			dev_err(priv->dev, "Failed to create a new display mode\n");
>  			return 0;
> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> index 61d9d183ce7f..26cefb202924 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> @@ -16,7 +16,7 @@
>  
>  struct meson_cvbs_mode {
>  	struct meson_cvbs_enci_mode *enci;
> -	struct drm_display_mode mode;
> +	const struct drm_display_mode *mode;
>  };
>  
>  #define MESON_CVBS_MODES_COUNT	2
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index a80ae9639e96..b4a440e2688c 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -394,6 +394,9 @@ struct drm_display_mode {
>  
>  };
>  
> +extern const struct drm_display_mode drm_mode_480i;
> +extern const struct drm_display_mode drm_mode_576i;
> +
>  /**
>   * DRM_MODE_FMT - printf string for &struct drm_display_mode
>   */
Thomas Zimmermann Aug. 2, 2022, 2:16 p.m. UTC | #2
Hi

Am 02.08.22 um 15:58 schrieb Jani Nikula:
> On Fri, 29 Jul 2022, Maxime Ripard <maxime@cerno.tech> wrote:
>> Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
>> modes in the drivers.
>>
>> Since those modes are fairly standards, and that we'll need to use them in
>> more places in the future, let's move the meson definition into the
>> framework.
> 
> I think you should always expose interfaces, not data. Data is not an
> interface, and I think this sets a bad example that will be cargo
> culted.

Although I wrote the opposite wrt patch 8, I agree with Jani here when 
it comes to 'official' interfaces. The cases I've seen of exported data 
structures often lead to intransparent code.

Best regards
Thomas

> 
> 
> BR,
> Jani.
> 
>>
>> The meson one was chosen because vc4's isn't accurate and doesn't amount to
>> 525 and 625 lines.
>>
>> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>>
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index 304004fb80aa..a4c1bd688338 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -48,6 +48,24 @@
>>   
>>   #include "drm_crtc_internal.h"
>>   
>> +const struct drm_display_mode drm_mode_480i = {
>> +	DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
>> +		 720, 739, 801, 858, 0,
>> +		 480, 488, 494, 525, 0,
>> +		 DRM_MODE_FLAG_INTERLACE),
>> +	.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
>> +};
>> +EXPORT_SYMBOL_GPL(drm_mode_480i);
>> +
>> +const struct drm_display_mode drm_mode_576i = {
>> +	DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
>> +		 720, 732, 795, 864, 0,
>> +		 576, 580, 586, 625, 0,
>> +		 DRM_MODE_FLAG_INTERLACE),
>> +	.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
>> +};
>> +EXPORT_SYMBOL_GPL(drm_mode_576i);
>> +
>>   /**
>>    * drm_mode_debug_printmodeline - print a mode to dmesg
>>    * @mode: mode to print
>> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>> index 8110a6e39320..98ec3e563155 100644
>> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>> @@ -45,21 +45,11 @@ struct meson_encoder_cvbs {
>>   struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT] = {
>>   	{ /* PAL */
>>   		.enci = &meson_cvbs_enci_pal,
>> -		.mode = {
>> -			DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
>> -				 720, 732, 795, 864, 0, 576, 580, 586, 625, 0,
>> -				 DRM_MODE_FLAG_INTERLACE),
>> -			.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
>> -		},
>> +		.mode = &drm_mode_576i,
>>   	},
>>   	{ /* NTSC */
>>   		.enci = &meson_cvbs_enci_ntsc,
>> -		.mode = {
>> -			DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
>> -				720, 739, 801, 858, 0, 480, 488, 494, 525, 0,
>> -				DRM_MODE_FLAG_INTERLACE),
>> -			.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
>> -		},
>> +		.mode = &drm_mode_480i,
>>   	},
>>   };
>>   
>> @@ -71,7 +61,7 @@ meson_cvbs_get_mode(const struct drm_display_mode *req_mode)
>>   	for (i = 0; i < MESON_CVBS_MODES_COUNT; ++i) {
>>   		struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>>   
>> -		if (drm_mode_match(req_mode, &meson_mode->mode,
>> +		if (drm_mode_match(req_mode, meson_mode->mode,
>>   				   DRM_MODE_MATCH_TIMINGS |
>>   				   DRM_MODE_MATCH_CLOCK |
>>   				   DRM_MODE_MATCH_FLAGS |
>> @@ -104,7 +94,7 @@ static int meson_encoder_cvbs_get_modes(struct drm_bridge *bridge,
>>   	for (i = 0; i < MESON_CVBS_MODES_COUNT; ++i) {
>>   		struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>>   
>> -		mode = drm_mode_duplicate(priv->drm, &meson_mode->mode);
>> +		mode = drm_mode_duplicate(priv->drm, meson_mode->mode);
>>   		if (!mode) {
>>   			dev_err(priv->dev, "Failed to create a new display mode\n");
>>   			return 0;
>> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
>> index 61d9d183ce7f..26cefb202924 100644
>> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
>> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
>> @@ -16,7 +16,7 @@
>>   
>>   struct meson_cvbs_mode {
>>   	struct meson_cvbs_enci_mode *enci;
>> -	struct drm_display_mode mode;
>> +	const struct drm_display_mode *mode;
>>   };
>>   
>>   #define MESON_CVBS_MODES_COUNT	2
>> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
>> index a80ae9639e96..b4a440e2688c 100644
>> --- a/include/drm/drm_modes.h
>> +++ b/include/drm/drm_modes.h
>> @@ -394,6 +394,9 @@ struct drm_display_mode {
>>   
>>   };
>>   
>> +extern const struct drm_display_mode drm_mode_480i;
>> +extern const struct drm_display_mode drm_mode_576i;
>> +
>>   /**
>>    * DRM_MODE_FMT - printf string for &struct drm_display_mode
>>    */
>
Geert Uytterhoeven Aug. 12, 2022, 1:18 p.m. UTC | #3
Hi Maxime,

Thanks for your patch!

On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
> Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> modes in the drivers.

Nit: strictly speaking these are not analog modes, but the digital
variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
13.5 MHz sampling frequency for pixels.

In analog modes, the only discrete values are the number of lines, and
the frame/field rate (fixing the horizontal sync rate when combined).

The number of (in)visible pixels per line depends on the available
bandwidth.  In a digital variant (which is anything generated by a
digital computer system), the latter depends on the pixel clock, which
can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).

So I think we probably need some way to generate a PAL/NTSC-compatible
mode based not only on resolution, but also on pixel clock.

>
> Since those modes are fairly standards, and that we'll need to use them in
> more places in the future, let's move the meson definition into the
> framework.
>
> The meson one was chosen because vc4's isn't accurate and doesn't amount to
> 525 and 625 lines.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -48,6 +48,24 @@
>
>  #include "drm_crtc_internal.h"
>
> +const struct drm_display_mode drm_mode_480i = {
> +       DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
> +                720, 739, 801, 858, 0,
> +                480, 488, 494, 525, 0,
> +                DRM_MODE_FLAG_INTERLACE),
> +       .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> +};
> +EXPORT_SYMBOL_GPL(drm_mode_480i);
> +
> +const struct drm_display_mode drm_mode_576i = {
> +       DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
> +                720, 732, 795, 864, 0,
> +                576, 580, 586, 625, 0,
> +                DRM_MODE_FLAG_INTERLACE),
> +       .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> +};
> +EXPORT_SYMBOL_GPL(drm_mode_576i);
> +
>  /**

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
Maxime Ripard Aug. 16, 2022, 11:58 a.m. UTC | #4
On Tue, Aug 02, 2022 at 04:16:29PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 02.08.22 um 15:58 schrieb Jani Nikula:
> > On Fri, 29 Jul 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> > > Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> > > modes in the drivers.
> > > 
> > > Since those modes are fairly standards, and that we'll need to use them in
> > > more places in the future, let's move the meson definition into the
> > > framework.
> > 
> > I think you should always expose interfaces, not data. Data is not an
> > interface, and I think this sets a bad example that will be cargo
> > culted.
> 
> Although I wrote the opposite wrt patch 8, I agree with Jani here when it
> comes to 'official' interfaces. The cases I've seen of exported data
> structures often lead to intransparent code.

Ack. The improvement Geert suggested would involve that change anyway,
so this should be fixed for the next iteration.

Maxime
Maxime Ripard Aug. 16, 2022, 1:26 p.m. UTC | #5
Hi Geert,

On Fri, Aug 12, 2022 at 03:18:58PM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
> 
> Thanks for your patch!
> 
> On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> > modes in the drivers.
> 
> Nit: strictly speaking these are not analog modes, but the digital
> variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
> 13.5 MHz sampling frequency for pixels.
> 
> In analog modes, the only discrete values are the number of lines, and
> the frame/field rate (fixing the horizontal sync rate when combined).
> 
> The number of (in)visible pixels per line depends on the available
> bandwidth.  In a digital variant (which is anything generated by a
> digital computer system), the latter depends on the pixel clock, which
> can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
> Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).
> 
> So I think we probably need some way to generate a PAL/NTSC-compatible
> mode based not only on resolution, but also on pixel clock.

This would also fix the comments made by Jani and Thomas, so I quite
like the idea of it.

I'm struggling a bit to find how would could implement this though.

From what you were saying, I guess the prototype would be something like

struct drm_display_mode *drm_create_analog_mode(unsigned int pixel_clock,
       						unsigned int lines,
						unsigned int frame_rate)

But I have zero idea on what the implementation would be. Do you have
some resources for this you could point me to?

Thanks
Maxime
Geert Uytterhoeven Aug. 16, 2022, 3 p.m. UTC | #6
Hi Maxime,

On Tue, Aug 16, 2022 at 3:26 PM Maxime Ripard <maxime@cerno.tech> wrote:
> On Fri, Aug 12, 2022 at 03:18:58PM +0200, Geert Uytterhoeven wrote:
> > On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> > > modes in the drivers.
> >
> > Nit: strictly speaking these are not analog modes, but the digital
> > variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
> > 13.5 MHz sampling frequency for pixels.
> >
> > In analog modes, the only discrete values are the number of lines, and
> > the frame/field rate (fixing the horizontal sync rate when combined).
> >
> > The number of (in)visible pixels per line depends on the available
> > bandwidth.  In a digital variant (which is anything generated by a
> > digital computer system), the latter depends on the pixel clock, which
> > can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
> > Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).
> >
> > So I think we probably need some way to generate a PAL/NTSC-compatible
> > mode based not only on resolution, but also on pixel clock.
>
> This would also fix the comments made by Jani and Thomas, so I quite
> like the idea of it.
>
> I'm struggling a bit to find how would could implement this though.
>
> From what you were saying, I guess the prototype would be something like
>
> struct drm_display_mode *drm_create_analog_mode(unsigned int pixel_clock,
>                                                 unsigned int lines,
>                                                 unsigned int frame_rate)
>
> But I have zero idea on what the implementation would be. Do you have
> some resources for this you could point me to?

Horizontally, I think you should calculate left/right margins and
hsync length to yield timings that match those for the BT.656 PAL/NTSC
modes.  I.e. when a 640x512 mode with a pixel clock of 14 MHz is
requested, you want to calculate left', right', and hslen' for

| <---- left' ---> | <- 640 pixels -> | <---- right' ---> | <--- hslen' --> |
                        @ 14 MHz

so they match the timings for left, right, and hslen for

| <--- left ---> | <--- 720 pixels ---> | <--- right ---> | <--- hslen ---> |
                        @ 13.5 MHz

As 640 pixels @ 14 MHz are less wide than 720 pixels @ 13.5 MHz,
you want to make sure to align the center of the visible part.

Vertically, it's simpler, as the number of lines is discrete.
You do have to take into account interlace and doublescan, and
progressive modes with 262/312 lines.

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
Maxime Ripard Aug. 17, 2022, 7:53 a.m. UTC | #7
On Tue, Aug 16, 2022 at 05:00:38PM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
> 
> On Tue, Aug 16, 2022 at 3:26 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > On Fri, Aug 12, 2022 at 03:18:58PM +0200, Geert Uytterhoeven wrote:
> > > On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> > > > modes in the drivers.
> > >
> > > Nit: strictly speaking these are not analog modes, but the digital
> > > variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
> > > 13.5 MHz sampling frequency for pixels.
> > >
> > > In analog modes, the only discrete values are the number of lines, and
> > > the frame/field rate (fixing the horizontal sync rate when combined).
> > >
> > > The number of (in)visible pixels per line depends on the available
> > > bandwidth.  In a digital variant (which is anything generated by a
> > > digital computer system), the latter depends on the pixel clock, which
> > > can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
> > > Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).
> > >
> > > So I think we probably need some way to generate a PAL/NTSC-compatible
> > > mode based not only on resolution, but also on pixel clock.
> >
> > This would also fix the comments made by Jani and Thomas, so I quite
> > like the idea of it.
> >
> > I'm struggling a bit to find how would could implement this though.
> >
> > From what you were saying, I guess the prototype would be something like
> >
> > struct drm_display_mode *drm_create_analog_mode(unsigned int pixel_clock,
> >                                                 unsigned int lines,
> >                                                 unsigned int frame_rate)
> >
> > But I have zero idea on what the implementation would be. Do you have
> > some resources for this you could point me to?
> 
> Horizontally, I think you should calculate left/right margins and
> hsync length to yield timings that match those for the BT.656 PAL/NTSC
> modes.  I.e. when a 640x512 mode with a pixel clock of 14 MHz is
> requested, you want to calculate left', right', and hslen' for
> 
> | <---- left' ---> | <- 640 pixels -> | <---- right' ---> | <--- hslen' --> |
>                         @ 14 MHz
> 
> so they match the timings for left, right, and hslen for
> 
> | <--- left ---> | <--- 720 pixels ---> | <--- right ---> | <--- hslen ---> |
>                         @ 13.5 MHz
> 
> As 640 pixels @ 14 MHz are less wide than 720 pixels @ 13.5 MHz,
> you want to make sure to align the center of the visible part.

So I guess in that example if we want to center it, left == right and
left' == right'? What about the sync length?

> Vertically, it's simpler, as the number of lines is discrete.
> You do have to take into account interlace and doublescan, and
> progressive modes with 262/312 lines.

So we only have to deal with 525 and 625 lines total (without taking
interlace and doublescan into account), right?

I guess we still have the same question, we probably want to center it,
so top == bottom, but what about the vsync length?

Maxime
Geert Uytterhoeven Aug. 17, 2022, 8:51 a.m. UTC | #8
Hi Maxime,

On Wed, Aug 17, 2022 at 9:54 AM Maxime Ripard <maxime@cerno.tech> wrote:
> On Tue, Aug 16, 2022 at 05:00:38PM +0200, Geert Uytterhoeven wrote:
> > On Tue, Aug 16, 2022 at 3:26 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > On Fri, Aug 12, 2022 at 03:18:58PM +0200, Geert Uytterhoeven wrote:
> > > > On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> > > > > modes in the drivers.
> > > >
> > > > Nit: strictly speaking these are not analog modes, but the digital
> > > > variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
> > > > 13.5 MHz sampling frequency for pixels.
> > > >
> > > > In analog modes, the only discrete values are the number of lines, and
> > > > the frame/field rate (fixing the horizontal sync rate when combined).
> > > >
> > > > The number of (in)visible pixels per line depends on the available
> > > > bandwidth.  In a digital variant (which is anything generated by a
> > > > digital computer system), the latter depends on the pixel clock, which
> > > > can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
> > > > Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).
> > > >
> > > > So I think we probably need some way to generate a PAL/NTSC-compatible
> > > > mode based not only on resolution, but also on pixel clock.
> > >
> > > This would also fix the comments made by Jani and Thomas, so I quite
> > > like the idea of it.
> > >
> > > I'm struggling a bit to find how would could implement this though.
> > >
> > > From what you were saying, I guess the prototype would be something like
> > >
> > > struct drm_display_mode *drm_create_analog_mode(unsigned int pixel_clock,
> > >                                                 unsigned int lines,
> > >                                                 unsigned int frame_rate)
> > >
> > > But I have zero idea on what the implementation would be. Do you have
> > > some resources for this you could point me to?
> >
> > Horizontally, I think you should calculate left/right margins and
> > hsync length to yield timings that match those for the BT.656 PAL/NTSC
> > modes.  I.e. when a 640x512 mode with a pixel clock of 14 MHz is
> > requested, you want to calculate left', right', and hslen' for
> >
> > | <---- left' ---> | <- 640 pixels -> | <---- right' ---> | <--- hslen' --> |
> >                         @ 14 MHz
> >
> > so they match the timings for left, right, and hslen for
> >
> > | <--- left ---> | <--- 720 pixels ---> | <--- right ---> | <--- hslen ---> |
> >                         @ 13.5 MHz
> >
> > As 640 pixels @ 14 MHz are less wide than 720 pixels @ 13.5 MHz,
> > you want to make sure to align the center of the visible part.
>
> So I guess in that example if we want to center it, left == right and
> left' == right'? What about the sync length?

No, left and right are asymmetrical, cfr. front and back porch in
https://en.wikipedia.org/wiki/PAL#PAL_signal_details
I.e. if the pixel part is reduced, both the left and right margins
should be increased by the same amount.

From the table linked above, hslen should be ca. 4.7µs (fixed).

> > Vertically, it's simpler, as the number of lines is discrete.
> > You do have to take into account interlace and doublescan, and
> > progressive modes with 262/312 lines.
>
> So we only have to deal with 525 and 625 lines total (without taking
> interlace and doublescan into account), right?

Yes.

> I guess we still have the same question, we probably want to center it,
> so top == bottom, but what about the vsync length?

Unfortunately that table does not mention top and bottom margins.
But according to drivers/video/fbdev/amifb.c (see the "Broadcast
video timings" comment block and the definitions of the "ntsc-lace"
and "pal-lace" video modes), they are asymmetrical, too.

Vsync length is 0.576ms, so that's 9 scan lines (I guess I didn't
have that info when I wrote amifb, as I used 4 lines there).

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
Maxime Ripard Aug. 17, 2022, 1:14 p.m. UTC | #9
On Wed, Aug 17, 2022 at 10:51:55AM +0200, Geert Uytterhoeven wrote:
> On Wed, Aug 17, 2022 at 9:54 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > On Tue, Aug 16, 2022 at 05:00:38PM +0200, Geert Uytterhoeven wrote:
> > > On Tue, Aug 16, 2022 at 3:26 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > On Fri, Aug 12, 2022 at 03:18:58PM +0200, Geert Uytterhoeven wrote:
> > > > > On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > > Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> > > > > > modes in the drivers.
> > > > >
> > > > > Nit: strictly speaking these are not analog modes, but the digital
> > > > > variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
> > > > > 13.5 MHz sampling frequency for pixels.
> > > > >
> > > > > In analog modes, the only discrete values are the number of lines, and
> > > > > the frame/field rate (fixing the horizontal sync rate when combined).
> > > > >
> > > > > The number of (in)visible pixels per line depends on the available
> > > > > bandwidth.  In a digital variant (which is anything generated by a
> > > > > digital computer system), the latter depends on the pixel clock, which
> > > > > can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
> > > > > Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).
> > > > >
> > > > > So I think we probably need some way to generate a PAL/NTSC-compatible
> > > > > mode based not only on resolution, but also on pixel clock.
> > > >
> > > > This would also fix the comments made by Jani and Thomas, so I quite
> > > > like the idea of it.
> > > >
> > > > I'm struggling a bit to find how would could implement this though.
> > > >
> > > > From what you were saying, I guess the prototype would be something like
> > > >
> > > > struct drm_display_mode *drm_create_analog_mode(unsigned int pixel_clock,
> > > >                                                 unsigned int lines,
> > > >                                                 unsigned int frame_rate)
> > > >
> > > > But I have zero idea on what the implementation would be. Do you have
> > > > some resources for this you could point me to?
> > >
> > > Horizontally, I think you should calculate left/right margins and
> > > hsync length to yield timings that match those for the BT.656 PAL/NTSC
> > > modes.  I.e. when a 640x512 mode with a pixel clock of 14 MHz is
> > > requested, you want to calculate left', right', and hslen' for
> > >
> > > | <---- left' ---> | <- 640 pixels -> | <---- right' ---> | <--- hslen' --> |
> > >                         @ 14 MHz
> > >
> > > so they match the timings for left, right, and hslen for
> > >
> > > | <--- left ---> | <--- 720 pixels ---> | <--- right ---> | <--- hslen ---> |
> > >                         @ 13.5 MHz
> > >
> > > As 640 pixels @ 14 MHz are less wide than 720 pixels @ 13.5 MHz,
> > > you want to make sure to align the center of the visible part.
> >
> > So I guess in that example if we want to center it, left == right and
> > left' == right'? What about the sync length?
> 
> No, left and right are asymmetrical, cfr. front and back porch in
> https://en.wikipedia.org/wiki/PAL#PAL_signal_details
> I.e. if the pixel part is reduced, both the left and right margins
> should be increased by the same amount.
> 
> From the table linked above, hslen should be ca. 4.7µs (fixed).

each pixel taking 1 / pixel_clock seconds (assuming pixel_clock is in
Hz), and thus hslen (in pixels) = 4.7 * 10 ^ -6 * pixel_clk, right?

> > > Vertically, it's simpler, as the number of lines is discrete.
> > > You do have to take into account interlace and doublescan, and
> > > progressive modes with 262/312 lines.
> >
> > So we only have to deal with 525 and 625 lines total (without taking
> > interlace and doublescan into account), right?
> 
> Yes.
> 
> > I guess we still have the same question, we probably want to center it,
> > so top == bottom, but what about the vsync length?
> 
> Unfortunately that table does not mention top and bottom margins.
> But according to drivers/video/fbdev/amifb.c (see the "Broadcast
> video timings" comment block and the definitions of the "ntsc-lace"
> and "pal-lace" video modes), they are asymmetrical, too.
> 
> Vsync length is 0.576ms, so that's 9 scan lines (I guess I didn't
> have that info when I wrote amifb, as I used 4 lines there).

Thanks, that's some great info already.

It's mentioned though that the settings for NTSC are "straightforward",
but it's definitely not for me :)

I've looked around and it looks like the entire blanking area is
supposed to be 40 pixels in interlaced, but I couldn't find anywhere how
it's supposed to be split between the upper and lower margins and the
sync period.

Thanks!
Maxime
Geert Uytterhoeven Aug. 17, 2022, 2:01 p.m. UTC | #10
Hi Maxime,

On Wed, Aug 17, 2022 at 3:15 PM Maxime Ripard <maxime@cerno.tech> wrote:
> On Wed, Aug 17, 2022 at 10:51:55AM +0200, Geert Uytterhoeven wrote:
> > On Wed, Aug 17, 2022 at 9:54 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > On Tue, Aug 16, 2022 at 05:00:38PM +0200, Geert Uytterhoeven wrote:
> > > > On Tue, Aug 16, 2022 at 3:26 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > On Fri, Aug 12, 2022 at 03:18:58PM +0200, Geert Uytterhoeven wrote:
> > > > > > On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > > > Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> > > > > > > modes in the drivers.
> > > > > >
> > > > > > Nit: strictly speaking these are not analog modes, but the digital
> > > > > > variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
> > > > > > 13.5 MHz sampling frequency for pixels.
> > > > > >
> > > > > > In analog modes, the only discrete values are the number of lines, and
> > > > > > the frame/field rate (fixing the horizontal sync rate when combined).
> > > > > >
> > > > > > The number of (in)visible pixels per line depends on the available
> > > > > > bandwidth.  In a digital variant (which is anything generated by a
> > > > > > digital computer system), the latter depends on the pixel clock, which
> > > > > > can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
> > > > > > Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).
> > > > > >
> > > > > > So I think we probably need some way to generate a PAL/NTSC-compatible
> > > > > > mode based not only on resolution, but also on pixel clock.
> > > > >
> > > > > This would also fix the comments made by Jani and Thomas, so I quite
> > > > > like the idea of it.
> > > > >
> > > > > I'm struggling a bit to find how would could implement this though.
> > > > >
> > > > > From what you were saying, I guess the prototype would be something like
> > > > >
> > > > > struct drm_display_mode *drm_create_analog_mode(unsigned int pixel_clock,
> > > > >                                                 unsigned int lines,
> > > > >                                                 unsigned int frame_rate)
> > > > >
> > > > > But I have zero idea on what the implementation would be. Do you have
> > > > > some resources for this you could point me to?
> > > >
> > > > Horizontally, I think you should calculate left/right margins and
> > > > hsync length to yield timings that match those for the BT.656 PAL/NTSC
> > > > modes.  I.e. when a 640x512 mode with a pixel clock of 14 MHz is
> > > > requested, you want to calculate left', right', and hslen' for
> > > >
> > > > | <---- left' ---> | <- 640 pixels -> | <---- right' ---> | <--- hslen' --> |
> > > >                         @ 14 MHz
> > > >
> > > > so they match the timings for left, right, and hslen for
> > > >
> > > > | <--- left ---> | <--- 720 pixels ---> | <--- right ---> | <--- hslen ---> |
> > > >                         @ 13.5 MHz
> > > >
> > > > As 640 pixels @ 14 MHz are less wide than 720 pixels @ 13.5 MHz,
> > > > you want to make sure to align the center of the visible part.
> > >
> > > So I guess in that example if we want to center it, left == right and
> > > left' == right'? What about the sync length?
> >
> > No, left and right are asymmetrical, cfr. front and back porch in
> > https://en.wikipedia.org/wiki/PAL#PAL_signal_details
> > I.e. if the pixel part is reduced, both the left and right margins
> > should be increased by the same amount.
> >
> > From the table linked above, hslen should be ca. 4.7µs (fixed).
>
> each pixel taking 1 / pixel_clock seconds (assuming pixel_clock is in
> Hz), and thus hslen (in pixels) = 4.7 * 10 ^ -6 * pixel_clk, right?

Exactly.

> > > > Vertically, it's simpler, as the number of lines is discrete.
> > > > You do have to take into account interlace and doublescan, and
> > > > progressive modes with 262/312 lines.
> > >
> > > So we only have to deal with 525 and 625 lines total (without taking
> > > interlace and doublescan into account), right?
> >
> > Yes.
> >
> > > I guess we still have the same question, we probably want to center it,
> > > so top == bottom, but what about the vsync length?
> >
> > Unfortunately that table does not mention top and bottom margins.
> > But according to drivers/video/fbdev/amifb.c (see the "Broadcast
> > video timings" comment block and the definitions of the "ntsc-lace"
> > and "pal-lace" video modes), they are asymmetrical, too.
> >
> > Vsync length is 0.576ms, so that's 9 scan lines (I guess I didn't
> > have that info when I wrote amifb, as I used 4 lines there).
>
> Thanks, that's some great info already.
>
> It's mentioned though that the settings for NTSC are "straightforward",
> but it's definitely not for me :)

As in NTSC just uses different pixel clock and horizontal/vertical sync
rate values...

> I've looked around and it looks like the entire blanking area is
> supposed to be 40 pixels in interlaced, but I couldn't find anywhere how

625 lines - 575[*] visible lines = 50 lines.

[*] BT.656 uses 576 visible lines as that's a multiple of 2, for splitting
     a frame in two fields of equal size.

"visible" is relative, as it includes the overscan region.
Some PAL monitors used with computers had knobs to control width/height
and position of the screen, so you could make use of most or all of
the overscan region, but on a real TV you're limited to ca. 640x512 (on
PAL)  which is what an Amiga used by default (with a 14 MHz pixclock).
> it's supposed to be split between the upper and lower margins and the
> sync period.

"Field Synchronization of PAL System" on http://martin.hinner.info/vga/pal.html
shows the split.

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
Maxime Ripard Aug. 18, 2022, 12:39 p.m. UTC | #11
Hi!

On Wed, Aug 17, 2022 at 04:01:48PM +0200, Geert Uytterhoeven wrote:
> > > > > Vertically, it's simpler, as the number of lines is discrete.
> > > > > You do have to take into account interlace and doublescan, and
> > > > > progressive modes with 262/312 lines.
> > > >
> > > > So we only have to deal with 525 and 625 lines total (without taking
> > > > interlace and doublescan into account), right?
> > >
> > > Yes.
> > >
> > > > I guess we still have the same question, we probably want to center it,
> > > > so top == bottom, but what about the vsync length?
> > >
> > > Unfortunately that table does not mention top and bottom margins.
> > > But according to drivers/video/fbdev/amifb.c (see the "Broadcast
> > > video timings" comment block and the definitions of the "ntsc-lace"
> > > and "pal-lace" video modes), they are asymmetrical, too.
> > >
> > > Vsync length is 0.576ms, so that's 9 scan lines (I guess I didn't
> > > have that info when I wrote amifb, as I used 4 lines there).
> >
> > Thanks, that's some great info already.
> >
> > It's mentioned though that the settings for NTSC are "straightforward",
> > but it's definitely not for me :)
> 
> As in NTSC just uses different pixel clock and horizontal/vertical sync
> rate values...

Oh, so the constants differ but the calculation is the same, ack.

> > I've looked around and it looks like the entire blanking area is
> > supposed to be 40 pixels in interlaced, but I couldn't find anywhere how
> 
> 625 lines - 575[*] visible lines = 50 lines.
> 
> [*] BT.656 uses 576 visible lines as that's a multiple of 2, for splitting
>      a frame in two fields of equal size.
> 
> "visible" is relative, as it includes the overscan region.
> Some PAL monitors used with computers had knobs to control width/height
> and position of the screen, so you could make use of most or all of
> the overscan region

It brings back some memories :)

> but on a real TV you're limited to ca. 640x512 (on PAL) which is what
> an Amiga used by default (with a 14 MHz pixclock).

> > it's supposed to be split between the upper and lower margins and the
> > sync period.
> 
> "Field Synchronization of PAL System" on
> http://martin.hinner.info/vga/pal.html shows the split.

Thanks, that's excellent as well.

I'm mostly done with a function that creates a PAL mode, but I still
have one question.

If I understand well, the blanking period is made up (interlace) of 16
pulses for the first field, 14 for the second, each pulse taking half a
line. That amount to 30 pulses, so 15 lines.

I first assumed that the pre-equalizing pulses would be the back porch,
the long sync pulses the vsync, and the post-equalizing pulses the front
porch. But... we're still missing 35 lines to amount to 625 lines, that
seems to be counted in the field itself (305 lines == (575 + 35) / 2)

So I guess my assumption was wrong to begin with.

You seem to have used a fixed vsync in amifb to 4 lines, and I don't
understand how you come up with the upper and lower margins (or rather,
how they are linked to what's described in that page)

Maxime
Geert Uytterhoeven Aug. 18, 2022, 12:57 p.m. UTC | #12
Hi Maxime,

On Thu, Aug 18, 2022 at 2:39 PM Maxime Ripard <maxime@cerno.tech> wrote:
> On Wed, Aug 17, 2022 at 04:01:48PM +0200, Geert Uytterhoeven wrote:
> > > I've looked around and it looks like the entire blanking area is
> > > supposed to be 40 pixels in interlaced, but I couldn't find anywhere how
> >
> > 625 lines - 575[*] visible lines = 50 lines.
> >
> > [*] BT.656 uses 576 visible lines as that's a multiple of 2, for splitting
> >      a frame in two fields of equal size.
> >
> > "visible" is relative, as it includes the overscan region.
> > Some PAL monitors used with computers had knobs to control width/height
> > and position of the screen, so you could make use of most or all of
> > the overscan region
>
> It brings back some memories :)
>
> > but on a real TV you're limited to ca. 640x512 (on PAL) which is what
> > an Amiga used by default (with a 14 MHz pixclock).
>
> > > it's supposed to be split between the upper and lower margins and the
> > > sync period.
> >
> > "Field Synchronization of PAL System" on
> > http://martin.hinner.info/vga/pal.html shows the split.
>
> Thanks, that's excellent as well.
>
> I'm mostly done with a function that creates a PAL mode, but I still
> have one question.
>
> If I understand well, the blanking period is made up (interlace) of 16
> pulses for the first field, 14 for the second, each pulse taking half a
> line. That amount to 30 pulses, so 15 lines.
>
> I first assumed that the pre-equalizing pulses would be the back porch,
> the long sync pulses the vsync, and the post-equalizing pulses the front
> porch. But... we're still missing 35 lines to amount to 625 lines, that
> seems to be counted in the field itself (305 lines == (575 + 35) / 2)
>
> So I guess my assumption was wrong to begin with.

The back porch is the number of lines between the last "visible" line
and the start of the synchronization pulse, i.e. "l" in the "Field
Synchronization of PAL System" drawing.
Virtual sync length is "m".
The front porch is the number of lines between the end of
the synchronization pulse, and the first "visible" line, i.e.
"j - l - m" (I think you used "n", thus missing lines 6-23 and 319-335).

> You seem to have used a fixed vsync in amifb to 4 lines, and I don't

Actually "m" is 2.5 lines in the first field, and 3 lines in the second field,
so "4" is not that much off of 2.5 + 3.

> understand how you come up with the upper and lower margins (or rather,
> how they are linked to what's described in that page)

These margins probably came from the Amiga hardware reference manual,
for the default 640x512 (PAL) and 640x400 (NTSC) modes.

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
Maxime Ripard Aug. 18, 2022, 1:42 p.m. UTC | #13
On Thu, Aug 18, 2022 at 02:57:55PM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
> 
> On Thu, Aug 18, 2022 at 2:39 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > On Wed, Aug 17, 2022 at 04:01:48PM +0200, Geert Uytterhoeven wrote:
> > > > I've looked around and it looks like the entire blanking area is
> > > > supposed to be 40 pixels in interlaced, but I couldn't find anywhere how
> > >
> > > 625 lines - 575[*] visible lines = 50 lines.
> > >
> > > [*] BT.656 uses 576 visible lines as that's a multiple of 2, for splitting
> > >      a frame in two fields of equal size.
> > >
> > > "visible" is relative, as it includes the overscan region.
> > > Some PAL monitors used with computers had knobs to control width/height
> > > and position of the screen, so you could make use of most or all of
> > > the overscan region
> >
> > It brings back some memories :)
> >
> > > but on a real TV you're limited to ca. 640x512 (on PAL) which is what
> > > an Amiga used by default (with a 14 MHz pixclock).
> >
> > > > it's supposed to be split between the upper and lower margins and the
> > > > sync period.
> > >
> > > "Field Synchronization of PAL System" on
> > > http://martin.hinner.info/vga/pal.html shows the split.
> >
> > Thanks, that's excellent as well.
> >
> > I'm mostly done with a function that creates a PAL mode, but I still
> > have one question.
> >
> > If I understand well, the blanking period is made up (interlace) of 16
> > pulses for the first field, 14 for the second, each pulse taking half a
> > line. That amount to 30 pulses, so 15 lines.
> >
> > I first assumed that the pre-equalizing pulses would be the back porch,
> > the long sync pulses the vsync, and the post-equalizing pulses the front
> > porch. But... we're still missing 35 lines to amount to 625 lines, that
> > seems to be counted in the field itself (305 lines == (575 + 35) / 2)
> >
> > So I guess my assumption was wrong to begin with.
> 
> The back porch is the number of lines between the last "visible" line
> and the start of the synchronization pulse, i.e. "l" in the "Field
> Synchronization of PAL System" drawing.
> Virtual sync length is "m".
> The front porch is the number of lines between the end of
> the synchronization pulse, and the first "visible" line, i.e.
> "j - l - m" (I think you used "n", thus missing lines 6-23 and 319-335).

Ah, yes, that makes sense

> > You seem to have used a fixed vsync in amifb to 4 lines, and I don't
> 
> Actually "m" is 2.5 lines in the first field, and 3 lines in the second field,
> so "4" is not that much off of 2.5 + 3.

Is it? If I'm reading that drawing well, l before the first field starts
on the second half of line 623 and stops at the end of line 625, so 2.5
line, and on the second field starts at the beginning of line 311, and
stops half-way through 313 so 2.5 line again.

Then, for the first field, m starts at the beginning of line 1, stops
half-way through line 3, so 2.5 line indeed, and then on the second
field starts on the second half of 313 and stops at the end of line 315.
So 2.5 again?

Thus, both should be 5?

> > understand how you come up with the upper and lower margins (or rather,
> > how they are linked to what's described in that page)
> 
> These margins probably came from the Amiga hardware reference manual,
> for the default 640x512 (PAL) and 640x400 (NTSC) modes.

Ok.

I started adding more sanity checks to my code, and I just realised I
don't seem to be able to reach 720 pixels over a single line though. If
I understood it properly, and according to [1] the active part of a line
is supposed to be 51.95us, and the blanking period taking 12.05us. [2]
in the timing section has pretty much the same numbers, so it looks
sane.

At 13.5Mhz, a pixel is going to take roughly 74ns, and 51950 / 74 = 702
pixels

It seems we can go push it to 52350 ns, but that still gives us only 706
pixels.

Similarly, if I just choose to ignore that limit and just take the
active time I need, 720 * 74 = 53280ns

That leaves us 10720ns for the blanking period, and that's not enough to
fit even the minimum of the front porch, hsync and back porch (1.55 +
4.5 + 5.5 = 11.55us).

Are those constraints merely recommendations, or am I missing something?

Thanks!
Maxime

1: https://en.wikipedia.org/wiki/PAL#PAL_signal_details
2: http://martin.hinner.info/vga/pal.html
Geert Uytterhoeven Aug. 18, 2022, 3:34 p.m. UTC | #14
Hi Maxime,

On Thu, Aug 18, 2022 at 3:42 PM Maxime Ripard <maxime@cerno.tech> wrote:
> On Thu, Aug 18, 2022 at 02:57:55PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Aug 18, 2022 at 2:39 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > On Wed, Aug 17, 2022 at 04:01:48PM +0200, Geert Uytterhoeven wrote:
> > > > > I've looked around and it looks like the entire blanking area is
> > > > > supposed to be 40 pixels in interlaced, but I couldn't find anywhere how
> > > >
> > > > 625 lines - 575[*] visible lines = 50 lines.
> > > >
> > > > [*] BT.656 uses 576 visible lines as that's a multiple of 2, for splitting
> > > >      a frame in two fields of equal size.
> > > >
> > > > "visible" is relative, as it includes the overscan region.
> > > > Some PAL monitors used with computers had knobs to control width/height
> > > > and position of the screen, so you could make use of most or all of
> > > > the overscan region
> > >
> > > It brings back some memories :)
> > >
> > > > but on a real TV you're limited to ca. 640x512 (on PAL) which is what
> > > > an Amiga used by default (with a 14 MHz pixclock).
> > >
> > > > > it's supposed to be split between the upper and lower margins and the
> > > > > sync period.
> > > >
> > > > "Field Synchronization of PAL System" on
> > > > http://martin.hinner.info/vga/pal.html shows the split.
> > >
> > > Thanks, that's excellent as well.
> > >
> > > I'm mostly done with a function that creates a PAL mode, but I still
> > > have one question.
> > >
> > > If I understand well, the blanking period is made up (interlace) of 16
> > > pulses for the first field, 14 for the second, each pulse taking half a
> > > line. That amount to 30 pulses, so 15 lines.
> > >
> > > I first assumed that the pre-equalizing pulses would be the back porch,
> > > the long sync pulses the vsync, and the post-equalizing pulses the front
> > > porch. But... we're still missing 35 lines to amount to 625 lines, that
> > > seems to be counted in the field itself (305 lines == (575 + 35) / 2)
> > >
> > > So I guess my assumption was wrong to begin with.
> >
> > The back porch is the number of lines between the last "visible" line
> > and the start of the synchronization pulse, i.e. "l" in the "Field
> > Synchronization of PAL System" drawing.
> > Virtual sync length is "m".
> > The front porch is the number of lines between the end of
> > the synchronization pulse, and the first "visible" line, i.e.
> > "j - l - m" (I think you used "n", thus missing lines 6-23 and 319-335).
>
> Ah, yes, that makes sense
>
> > > You seem to have used a fixed vsync in amifb to 4 lines, and I don't
> >
> > Actually "m" is 2.5 lines in the first field, and 3 lines in the second field,
> > so "4" is not that much off of 2.5 + 3.
>
> Is it? If I'm reading that drawing well, l before the first field starts
> on the second half of line 623 and stops at the end of line 625, so 2.5
> line, and on the second field starts at the beginning of line 311, and
> stops half-way through 313 so 2.5 line again.
>
> Then, for the first field, m starts at the beginning of line 1, stops
> half-way through line 3, so 2.5 line indeed, and then on the second
> field starts on the second half of 313 and stops at the end of line 315.
> So 2.5 again?
>
> Thus, both should be 5?

Possibly. Note that this for the official broadcast PAL.

I never looked at these signals with a scope, but I wouldn't be
surprised if some
device on't bother implementing the "half-line-sync", and synchronize
the start and stop of the vertical
sync signal with the start of a horizontal.

> > > understand how you come up with the upper and lower margins (or rather,
> > > how they are linked to what's described in that page)
> >
> > These margins probably came from the Amiga hardware reference manual,
> > for the default 640x512 (PAL) and 640x400 (NTSC) modes.
>
> Ok.
>
> I started adding more sanity checks to my code, and I just realised I
> don't seem to be able to reach 720 pixels over a single line though. If
> I understood it properly, and according to [1] the active part of a line
> is supposed to be 51.95us, and the blanking period taking 12.05us. [2]
> in the timing section has pretty much the same numbers, so it looks
> sane.
>
> At 13.5Mhz, a pixel is going to take roughly 74ns, and 51950 / 74 = 702
> pixels
>
> It seems we can go push it to 52350 ns, but that still gives us only 706
> pixels.
>
> Similarly, if I just choose to ignore that limit and just take the
> active time I need, 720 * 74 = 53280ns
>
> That leaves us 10720ns for the blanking period, and that's not enough to
> fit even the minimum of the front porch, hsync and back porch (1.55 +
> 4.5 + 5.5 = 11.55us).
>
> Are those constraints merely recommendations, or am I missing something?

You are missing that the parts near the borders of the full image are
part of the overscan range, and may or may not be visible, depending
on your actual display.
The full 768x576 image size from BT.656 is not visible on a typical PAL display,
and is more of an "absolute maximum rating", guaranteed to cover more
than analog PAL.

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
Maxime Ripard Aug. 18, 2022, 3:46 p.m. UTC | #15
On Thu, Aug 18, 2022 at 05:34:30PM +0200, Geert Uytterhoeven wrote:
> On Thu, Aug 18, 2022 at 3:42 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > On Thu, Aug 18, 2022 at 02:57:55PM +0200, Geert Uytterhoeven wrote:
> > > On Thu, Aug 18, 2022 at 2:39 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > On Wed, Aug 17, 2022 at 04:01:48PM +0200, Geert Uytterhoeven wrote:
> > > > > > I've looked around and it looks like the entire blanking area is
> > > > > > supposed to be 40 pixels in interlaced, but I couldn't find anywhere how
> > > > >
> > > > > 625 lines - 575[*] visible lines = 50 lines.
> > > > >
> > > > > [*] BT.656 uses 576 visible lines as that's a multiple of 2, for splitting
> > > > >      a frame in two fields of equal size.
> > > > >
> > > > > "visible" is relative, as it includes the overscan region.
> > > > > Some PAL monitors used with computers had knobs to control width/height
> > > > > and position of the screen, so you could make use of most or all of
> > > > > the overscan region
> > > >
> > > > It brings back some memories :)
> > > >
> > > > > but on a real TV you're limited to ca. 640x512 (on PAL) which is what
> > > > > an Amiga used by default (with a 14 MHz pixclock).
> > > >
> > > > > > it's supposed to be split between the upper and lower margins and the
> > > > > > sync period.
> > > > >
> > > > > "Field Synchronization of PAL System" on
> > > > > http://martin.hinner.info/vga/pal.html shows the split.
> > > >
> > > > Thanks, that's excellent as well.
> > > >
> > > > I'm mostly done with a function that creates a PAL mode, but I still
> > > > have one question.
> > > >
> > > > If I understand well, the blanking period is made up (interlace) of 16
> > > > pulses for the first field, 14 for the second, each pulse taking half a
> > > > line. That amount to 30 pulses, so 15 lines.
> > > >
> > > > I first assumed that the pre-equalizing pulses would be the back porch,
> > > > the long sync pulses the vsync, and the post-equalizing pulses the front
> > > > porch. But... we're still missing 35 lines to amount to 625 lines, that
> > > > seems to be counted in the field itself (305 lines == (575 + 35) / 2)
> > > >
> > > > So I guess my assumption was wrong to begin with.
> > >
> > > The back porch is the number of lines between the last "visible" line
> > > and the start of the synchronization pulse, i.e. "l" in the "Field
> > > Synchronization of PAL System" drawing.
> > > Virtual sync length is "m".
> > > The front porch is the number of lines between the end of
> > > the synchronization pulse, and the first "visible" line, i.e.
> > > "j - l - m" (I think you used "n", thus missing lines 6-23 and 319-335).
> >
> > Ah, yes, that makes sense
> >
> > > > You seem to have used a fixed vsync in amifb to 4 lines, and I don't
> > >
> > > Actually "m" is 2.5 lines in the first field, and 3 lines in the second field,
> > > so "4" is not that much off of 2.5 + 3.
> >
> > Is it? If I'm reading that drawing well, l before the first field starts
> > on the second half of line 623 and stops at the end of line 625, so 2.5
> > line, and on the second field starts at the beginning of line 311, and
> > stops half-way through 313 so 2.5 line again.
> >
> > Then, for the first field, m starts at the beginning of line 1, stops
> > half-way through line 3, so 2.5 line indeed, and then on the second
> > field starts on the second half of 313 and stops at the end of line 315.
> > So 2.5 again?
> >
> > Thus, both should be 5?
> 
> Possibly. Note that this for the official broadcast PAL.
> 
> I never looked at these signals with a scope, but I wouldn't be
> surprised if some
> device on't bother implementing the "half-line-sync", and synchronize
> the start and stop of the vertical
> sync signal with the start of a horizontal.

Yeah... official PAL looks like a good enough target to me :)

We'll always be able to tweak it if needed later on.

> > > > understand how you come up with the upper and lower margins (or rather,
> > > > how they are linked to what's described in that page)
> > >
> > > These margins probably came from the Amiga hardware reference manual,
> > > for the default 640x512 (PAL) and 640x400 (NTSC) modes.
> >
> > Ok.
> >
> > I started adding more sanity checks to my code, and I just realised I
> > don't seem to be able to reach 720 pixels over a single line though. If
> > I understood it properly, and according to [1] the active part of a line
> > is supposed to be 51.95us, and the blanking period taking 12.05us. [2]
> > in the timing section has pretty much the same numbers, so it looks
> > sane.
> >
> > At 13.5Mhz, a pixel is going to take roughly 74ns, and 51950 / 74 = 702
> > pixels
> >
> > It seems we can go push it to 52350 ns, but that still gives us only 706
> > pixels.
> >
> > Similarly, if I just choose to ignore that limit and just take the
> > active time I need, 720 * 74 = 53280ns
> >
> > That leaves us 10720ns for the blanking period, and that's not enough to
> > fit even the minimum of the front porch, hsync and back porch (1.55 +
> > 4.5 + 5.5 = 11.55us).
> >
> > Are those constraints merely recommendations, or am I missing something?
> 
> You are missing that the parts near the borders of the full image are
> part of the overscan range, and may or may not be visible, depending
> on your actual display.
> The full 768x576 image size from BT.656 is not visible on a typical PAL display,
> and is more of an "absolute maximum rating", guaranteed to cover more
> than analog PAL.

So the overscan range is not part of the active area, unlike what HDMI
is doing for example?

Is there some minimal timings available somewhere to fit those absolute
maximum ratings?

Maxime
Geert Uytterhoeven Aug. 18, 2022, 3:56 p.m. UTC | #16
Hi Maxime,

On Thu, Aug 18, 2022 at 5:46 PM Maxime Ripard <maxime@cerno.tech> wrote:
> On Thu, Aug 18, 2022 at 05:34:30PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Aug 18, 2022 at 3:42 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > > I started adding more sanity checks to my code, and I just realised I
> > > don't seem to be able to reach 720 pixels over a single line though. If
> > > I understood it properly, and according to [1] the active part of a line
> > > is supposed to be 51.95us, and the blanking period taking 12.05us. [2]
> > > in the timing section has pretty much the same numbers, so it looks
> > > sane.
> > >
> > > At 13.5Mhz, a pixel is going to take roughly 74ns, and 51950 / 74 = 702
> > > pixels
> > >
> > > It seems we can go push it to 52350 ns, but that still gives us only 706
> > > pixels.
> > >
> > > Similarly, if I just choose to ignore that limit and just take the
> > > active time I need, 720 * 74 = 53280ns
> > >
> > > That leaves us 10720ns for the blanking period, and that's not enough to
> > > fit even the minimum of the front porch, hsync and back porch (1.55 +
> > > 4.5 + 5.5 = 11.55us).
> > >
> > > Are those constraints merely recommendations, or am I missing something?
> >
> > You are missing that the parts near the borders of the full image are
> > part of the overscan range, and may or may not be visible, depending
> > on your actual display.
> > The full 768x576 image size from BT.656 is not visible on a typical PAL display,
> > and is more of an "absolute maximum rating", guaranteed to cover more
> > than analog PAL.
>
> So the overscan range is not part of the active area, unlike what HDMI
> is doing for example?

Indeed. DVI-D and HDMI etc. are pure digital (let's ignore they are a
digitized variant of old analog VGA ;-), hence there is a one-to-one
match between pixels in the image and pixels on the screen (ignoring
scaling).  But even when using an analog VGA input on a modern
digital display, you have controls to e.g. move the image.

> Is there some minimal timings available somewhere to fit those absolute
> maximum ratings?

I guess they can be found on the Internet...

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
Mateusz Kwiatkowski Aug. 24, 2022, 4:42 p.m. UTC | #17
Hi Maxime,

W dniu 18.08.2022 o 17:56, Geert Uytterhoeven pisze:
> Hi Maxime,
>
> On Thu, Aug 18, 2022 at 5:46 PM Maxime Ripard <maxime@cerno.tech> wrote:
>> On Thu, Aug 18, 2022 at 05:34:30PM +0200, Geert Uytterhoeven wrote:
>>> On Thu, Aug 18, 2022 at 3:42 PM Maxime Ripard <maxime@cerno.tech> wrote:
>>>> I started adding more sanity checks to my code, and I just realised I
>>>> don't seem to be able to reach 720 pixels over a single line though. If
>>>> I understood it properly, and according to [1] the active part of a line
>>>> is supposed to be 51.95us, and the blanking period taking 12.05us. [2]
>>>> in the timing section has pretty much the same numbers, so it looks
>>>> sane.
>>>>
>>>> At 13.5Mhz, a pixel is going to take roughly 74ns, and 51950 / 74 = 702
>>>> pixels
>>>>
>>>> It seems we can go push it to 52350 ns, but that still gives us only 706
>>>> pixels.
>>>>
>>>> Similarly, if I just choose to ignore that limit and just take the
>>>> active time I need, 720 * 74 = 53280ns
>>>>
>>>> That leaves us 10720ns for the blanking period, and that's not enough to
>>>> fit even the minimum of the front porch, hsync and back porch (1.55 +
>>>> 4.5 + 5.5 = 11.55us).
>>>>
>>>> Are those constraints merely recommendations, or am I missing something?
>>>
>>> You are missing that the parts near the borders of the full image are
>>> part of the overscan range, and may or may not be visible, depending
>>> on your actual display.
>>> The full 768x576 image size from BT.656 is not visible on a typical PAL display,
>>> and is more of an "absolute maximum rating", guaranteed to cover more
>>> than analog PAL.
>>
>> So the overscan range is not part of the active area, unlike what HDMI
>> is doing for example?
>
> Indeed. DVI-D and HDMI etc. are pure digital (let's ignore they are a
> digitized variant of old analog VGA ;-), hence there is a one-to-one
> match between pixels in the image and pixels on the screen (ignoring
> scaling).  But even when using an analog VGA input on a modern
> digital display, you have controls to e.g. move the image.
>
>> Is there some minimal timings available somewhere to fit those absolute
>> maximum ratings?
>
> I guess they can be found on the Internet...

Here are some references that I personally found useful:

- ITU-R BT.601 <https://www.itu.int/rec/R-REC-BT.601/en>
  This is *the* standard that pretty much every modern device that deals with
  analog-style TV signal follows then converting to and from the digital domain.
  For example in the figures on page 10 (12 in the PDF numbering) you can see
  that the "time datum", i.e. start of horizontal sync pulse is canonically
  supposed to happen on sample 732 for 50 Hz or sample 736 for 59.94 Hz modes.

  BT.601 assumes 13.5 MHz sample rate / pixel clock, but you can proportionally
  scale those for other pixel clocks.

- ITU-R BT.1700 <https://www.itu.int/rec/R-REC-BT.1700/en>
  This is *the* standard in force for actual analog composite video signals.
  The vertical sync specs are discrete, so they don't really change between
  analog and digital domains. For horizontal sync, the values in those specs
  are given in microseconds/nanoseconds, but you can multiply those by the
  sampling rate for equivalent pixel counts.

- Pembers' Ponderings
  <https://web.archive.org/web/20160423225838/http://www.pembers.freeserve.co.uk/>
  An old archived website with a ton of resources about analog TV.
  The "Line Standards" article will probably be most interesting to you.

By the way, please note a couple of things:

- The analog standards are very imprecise for modern digital norms, giving
  considerable leeway for just about every timing. The allowed leeways are
  usually equivalent to a couple of pixels at the standard 13.5 MHz sampling
  rate - and those are meant for the transmitting end. Receivers are usually
  much more forgiving to maximize compatibility.

- The 720-pixel standard of BT.601 is considerably wider than the active width
  specified in the analog standards. AFAIK this is intentional, to ensure that
  no part of the actual image is missed during digitization, and to keep the
  number a nice multiply of 16. The picture width given in the analog standards
  is equivalent to somewhere between 702 and 714 pixels (at 13.5 MHz clock),
  depending on the specific standard. And that includes overscan.

- Same goes for the vertical active area. Original analog standards varied
  wildly from country to country, before finally settling on 575 lines for the
  50 Hz standard and 485 lines for the 59.94 Hz standard. Or 576/486, depending
  on how you count. The topmost line of those 576/486 starts at half the screen,
  and the bottommost line ends at half the screen - so they are often combined
  when counting and given as 575/485. The digital 576i50 standard includes
  those half-lines. In the 59.94 Hz regions, 480 active digial lines ended up
  the norm, because 486 does not have nice dividers, and also some of the
  outermost lines which were always overscanned anyway, ended up used for things
  like closed captioning over the years.

- Speaking of closed captioning... a lot of different stuff were put in the
  blanking interval over the years. Like teletext in Europe. There are projects
  like VBIT2 <https://github.com/peterkvt80/vbit2> which intentionally
  reconfigure the Raspberry Pi composite output to include the blanking interval
  in the framebuffer so that teletext can be output by drawing on the edge of
  the "screen" (from the computer point of view).

- A lot of equipment outside the broadcast industry willingly violated those
  standards, and there are real world use cases for that. Film studios used very
  slightly modified TVs to make them sync with 24fps cameras - in that variant,
  "NTSC" could have e.g. 655 lines so that the TV would refresh at 48 Hz with
  the same line frequency. Home computers and video game consoles output
  progressive 262/312-line modes instead of interlaced 525/625 lines. And often
  changed the line frequency slightly as well, for various reasons. Those
  progressive modes are still favored by retro gaming and emulation enthusiasts,
  because they incur a specific look on CRT displays. Even playing back video
  from a tape (especially home-grade, like VHS) could cause timings to go wildly
  out of spec, because of mechanical imprecisions.

- There were multitude of standards predating the ubiquitous 525/60 and 625/50
  modes. The British 405-line and French 819-line standards are the most
  notorious, having lasted well into the 1980s, but there were also a lot of
  wildly varying pre-WW2 television systems. And there are enthusiasts dedicated
  to preserving those.

My point is that the norms for analog TV are rather loose, and I think we
shouldn't limit the drivers to only accepting the "proper" modes as defined in
the spec. Those should of course be the default, but if non-standard modelines
can be generated - there are legitimate use cases why people might want those.

>
> 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

Best regards,
Mateusz Kwiatkowski
Maxime Ripard Aug. 29, 2022, 1:29 p.m. UTC | #18
Hi Mateusz

On Wed, Aug 24, 2022 at 06:42:18PM +0200, Mateusz Kwiatkowski wrote:
> Hi Maxime,
> 
> W dniu 18.08.2022 o 17:56, Geert Uytterhoeven pisze:
> > Hi Maxime,
> >
> > On Thu, Aug 18, 2022 at 5:46 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >> On Thu, Aug 18, 2022 at 05:34:30PM +0200, Geert Uytterhoeven wrote:
> >>> On Thu, Aug 18, 2022 at 3:42 PM Maxime Ripard <maxime@cerno.tech> wrote:
> >>>> I started adding more sanity checks to my code, and I just realised I
> >>>> don't seem to be able to reach 720 pixels over a single line though. If
> >>>> I understood it properly, and according to [1] the active part of a line
> >>>> is supposed to be 51.95us, and the blanking period taking 12.05us. [2]
> >>>> in the timing section has pretty much the same numbers, so it looks
> >>>> sane.
> >>>>
> >>>> At 13.5Mhz, a pixel is going to take roughly 74ns, and 51950 / 74 = 702
> >>>> pixels
> >>>>
> >>>> It seems we can go push it to 52350 ns, but that still gives us only 706
> >>>> pixels.
> >>>>
> >>>> Similarly, if I just choose to ignore that limit and just take the
> >>>> active time I need, 720 * 74 = 53280ns
> >>>>
> >>>> That leaves us 10720ns for the blanking period, and that's not enough to
> >>>> fit even the minimum of the front porch, hsync and back porch (1.55 +
> >>>> 4.5 + 5.5 = 11.55us).
> >>>>
> >>>> Are those constraints merely recommendations, or am I missing something?
> >>>
> >>> You are missing that the parts near the borders of the full image are
> >>> part of the overscan range, and may or may not be visible, depending
> >>> on your actual display.
> >>> The full 768x576 image size from BT.656 is not visible on a typical PAL display,
> >>> and is more of an "absolute maximum rating", guaranteed to cover more
> >>> than analog PAL.
> >>
> >> So the overscan range is not part of the active area, unlike what HDMI
> >> is doing for example?
> >
> > Indeed. DVI-D and HDMI etc. are pure digital (let's ignore they are a
> > digitized variant of old analog VGA ;-), hence there is a one-to-one
> > match between pixels in the image and pixels on the screen (ignoring
> > scaling).  But even when using an analog VGA input on a modern
> > digital display, you have controls to e.g. move the image.
> >
> >> Is there some minimal timings available somewhere to fit those absolute
> >> maximum ratings?
> >
> > I guess they can be found on the Internet...
> 
> Here are some references that I personally found useful:
> 
> - ITU-R BT.601 <https://www.itu.int/rec/R-REC-BT.601/en>
>   This is *the* standard that pretty much every modern device that deals with
>   analog-style TV signal follows then converting to and from the digital domain.
>   For example in the figures on page 10 (12 in the PDF numbering) you can see
>   that the "time datum", i.e. start of horizontal sync pulse is canonically
>   supposed to happen on sample 732 for 50 Hz or sample 736 for 59.94 Hz modes.
> 
>   BT.601 assumes 13.5 MHz sample rate / pixel clock, but you can proportionally
>   scale those for other pixel clocks.
> 
> - ITU-R BT.1700 <https://www.itu.int/rec/R-REC-BT.1700/en>
>   This is *the* standard in force for actual analog composite video signals.
>   The vertical sync specs are discrete, so they don't really change between
>   analog and digital domains. For horizontal sync, the values in those specs
>   are given in microseconds/nanoseconds, but you can multiply those by the
>   sampling rate for equivalent pixel counts.
> 
> - Pembers' Ponderings
>   <https://web.archive.org/web/20160423225838/http://www.pembers.freeserve.co.uk/>
>   An old archived website with a ton of resources about analog TV.
>   The "Line Standards" article will probably be most interesting to you.

Thanks so much for all those resources, it's been super helpful :)

> By the way, please note a couple of things:
> 
> - The analog standards are very imprecise for modern digital norms, giving
>   considerable leeway for just about every timing. The allowed leeways are
>   usually equivalent to a couple of pixels at the standard 13.5 MHz sampling
>   rate - and those are meant for the transmitting end. Receivers are usually
>   much more forgiving to maximize compatibility.

Ok

> - The 720-pixel standard of BT.601 is considerably wider than the active width
>   specified in the analog standards. AFAIK this is intentional, to ensure that
>   no part of the actual image is missed during digitization, and to keep the
>   number a nice multiply of 16. The picture width given in the analog standards
>   is equivalent to somewhere between 702 and 714 pixels (at 13.5 MHz clock),
>   depending on the specific standard. And that includes overscan.

Ok. I think it still makes sense to allow it, if only we were using it so far :)

I've done a first implementation in the v2 I just sent that seems to
work ok, please let me know if I did anything stupid :)

In particular, I chose, if we were between 702 and 720 pixels to disable
all duration checks, and take the missing time from the front and back
porch, in equal proportions.

> - Same goes for the vertical active area. Original analog standards varied
>   wildly from country to country, before finally settling on 575 lines for the
>   50 Hz standard and 485 lines for the 59.94 Hz standard. Or 576/486, depending
>   on how you count. The topmost line of those 576/486 starts at half the screen,
>   and the bottommost line ends at half the screen - so they are often combined
>   when counting and given as 575/485. The digital 576i50 standard includes
>   those half-lines. In the 59.94 Hz regions, 480 active digial lines ended up
>   the norm, because 486 does not have nice dividers, and also some of the
>   outermost lines which were always overscanned anyway, ended up used for things
>   like closed captioning over the years.

Ok

> - Speaking of closed captioning... a lot of different stuff were put in the
>   blanking interval over the years. Like teletext in Europe. There are projects
>   like VBIT2 <https://github.com/peterkvt80/vbit2> which intentionally
>   reconfigure the Raspberry Pi composite output to include the blanking interval
>   in the framebuffer so that teletext can be output by drawing on the edge of
>   the "screen" (from the computer point of view).

I'm not sure how we would support this in KMS to be honest. Asking for a
wider mode and the userspace putting whatever it wants in the margins
seems like a good choice.

> - A lot of equipment outside the broadcast industry willingly violated those
>   standards, and there are real world use cases for that. Film studios used very
>   slightly modified TVs to make them sync with 24fps cameras - in that variant,
>   "NTSC" could have e.g. 655 lines so that the TV would refresh at 48 Hz with
>   the same line frequency. Home computers and video game consoles output
>   progressive 262/312-line modes instead of interlaced 525/625 lines. And often
>   changed the line frequency slightly as well, for various reasons. Those
>   progressive modes are still favored by retro gaming and emulation enthusiasts,
>   because they incur a specific look on CRT displays. Even playing back video
>   from a tape (especially home-grade, like VHS) could cause timings to go wildly
>   out of spec, because of mechanical imprecisions.

Ok

> - There were multitude of standards predating the ubiquitous 525/60 and 625/50
>   modes. The British 405-line and French 819-line standards are the most
>   notorious, having lasted well into the 1980s, but there were also a lot of
>   wildly varying pre-WW2 television systems. And there are enthusiasts dedicated
>   to preserving those.
> 
> My point is that the norms for analog TV are rather loose, and I think we
> shouldn't limit the drivers to only accepting the "proper" modes as defined in
> the spec. Those should of course be the default, but if non-standard modelines
> can be generated - there are legitimate use cases why people might want those.

Yep, that part has been dropped. I'm still wondering if we'd need to
still have a bunch of restrictions (like a total number of lines of 625
with NTSC would be obviously invalid), but that can always be added
later on if such a need comes up

Maxime
Geert Uytterhoeven Aug. 29, 2022, 2:14 p.m. UTC | #19
Hi Maxime,

On Mon, Aug 29, 2022 at 3:30 PM Maxime Ripard <maxime@cerno.tech> wrote:
> On Wed, Aug 24, 2022 at 06:42:18PM +0200, Mateusz Kwiatkowski wrote:
> > - Speaking of closed captioning... a lot of different stuff were put in the
> >   blanking interval over the years. Like teletext in Europe. There are projects
> >   like VBIT2 <https://github.com/peterkvt80/vbit2> which intentionally
> >   reconfigure the Raspberry Pi composite output to include the blanking interval
> >   in the framebuffer so that teletext can be output by drawing on the edge of
> >   the "screen" (from the computer point of view).
>
> I'm not sure how we would support this in KMS to be honest. Asking for a
> wider mode and the userspace putting whatever it wants in the margins
> seems like a good choice.

s/wider/higher/

Teletext is transmitted in the "visible" parts of (horizontal) lines, but during
the vertical blank.

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
Maxime Ripard Aug. 29, 2022, 2:33 p.m. UTC | #20
On Mon, Aug 29, 2022 at 04:14:54PM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
> 
> On Mon, Aug 29, 2022 at 3:30 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > On Wed, Aug 24, 2022 at 06:42:18PM +0200, Mateusz Kwiatkowski wrote:
> > > - Speaking of closed captioning... a lot of different stuff were put in the
> > >   blanking interval over the years. Like teletext in Europe. There are projects
> > >   like VBIT2 <https://github.com/peterkvt80/vbit2> which intentionally
> > >   reconfigure the Raspberry Pi composite output to include the blanking interval
> > >   in the framebuffer so that teletext can be output by drawing on the edge of
> > >   the "screen" (from the computer point of view).
> >
> > I'm not sure how we would support this in KMS to be honest. Asking for a
> > wider mode and the userspace putting whatever it wants in the margins
> > seems like a good choice.
> 
> s/wider/higher/
> 
> Teletext is transmitted in the "visible" parts of (horizontal) lines, but during
> the vertical blank.

Yeah, sorry I meant wider as in larger than the active area, without any
direction in mind. Thanks for the correction :)

Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 304004fb80aa..a4c1bd688338 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -48,6 +48,24 @@ 
 
 #include "drm_crtc_internal.h"
 
+const struct drm_display_mode drm_mode_480i = {
+	DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
+		 720, 739, 801, 858, 0,
+		 480, 488, 494, 525, 0,
+		 DRM_MODE_FLAG_INTERLACE),
+	.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
+};
+EXPORT_SYMBOL_GPL(drm_mode_480i);
+
+const struct drm_display_mode drm_mode_576i = {
+	DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
+		 720, 732, 795, 864, 0,
+		 576, 580, 586, 625, 0,
+		 DRM_MODE_FLAG_INTERLACE),
+	.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
+};
+EXPORT_SYMBOL_GPL(drm_mode_576i);
+
 /**
  * drm_mode_debug_printmodeline - print a mode to dmesg
  * @mode: mode to print
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
index 8110a6e39320..98ec3e563155 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -45,21 +45,11 @@  struct meson_encoder_cvbs {
 struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT] = {
 	{ /* PAL */
 		.enci = &meson_cvbs_enci_pal,
-		.mode = {
-			DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
-				 720, 732, 795, 864, 0, 576, 580, 586, 625, 0,
-				 DRM_MODE_FLAG_INTERLACE),
-			.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
-		},
+		.mode = &drm_mode_576i,
 	},
 	{ /* NTSC */
 		.enci = &meson_cvbs_enci_ntsc,
-		.mode = {
-			DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
-				720, 739, 801, 858, 0, 480, 488, 494, 525, 0,
-				DRM_MODE_FLAG_INTERLACE),
-			.picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
-		},
+		.mode = &drm_mode_480i,
 	},
 };
 
@@ -71,7 +61,7 @@  meson_cvbs_get_mode(const struct drm_display_mode *req_mode)
 	for (i = 0; i < MESON_CVBS_MODES_COUNT; ++i) {
 		struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
 
-		if (drm_mode_match(req_mode, &meson_mode->mode,
+		if (drm_mode_match(req_mode, meson_mode->mode,
 				   DRM_MODE_MATCH_TIMINGS |
 				   DRM_MODE_MATCH_CLOCK |
 				   DRM_MODE_MATCH_FLAGS |
@@ -104,7 +94,7 @@  static int meson_encoder_cvbs_get_modes(struct drm_bridge *bridge,
 	for (i = 0; i < MESON_CVBS_MODES_COUNT; ++i) {
 		struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
 
-		mode = drm_mode_duplicate(priv->drm, &meson_mode->mode);
+		mode = drm_mode_duplicate(priv->drm, meson_mode->mode);
 		if (!mode) {
 			dev_err(priv->dev, "Failed to create a new display mode\n");
 			return 0;
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
index 61d9d183ce7f..26cefb202924 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
@@ -16,7 +16,7 @@ 
 
 struct meson_cvbs_mode {
 	struct meson_cvbs_enci_mode *enci;
-	struct drm_display_mode mode;
+	const struct drm_display_mode *mode;
 };
 
 #define MESON_CVBS_MODES_COUNT	2
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index a80ae9639e96..b4a440e2688c 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -394,6 +394,9 @@  struct drm_display_mode {
 
 };
 
+extern const struct drm_display_mode drm_mode_480i;
+extern const struct drm_display_mode drm_mode_576i;
+
 /**
  * DRM_MODE_FMT - printf string for &struct drm_display_mode
  */