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