Skip to contentSkip to conversation

sunxi: video: Add support for TV (composite) output on H3/H5

PATCH New Jernej Skrabec 4 participants 20 messages May 19, 2017 – Jun 2, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover sunxi: video: Add support for TV (composite) output on H3/H5

This series implements support for composite output. Driver is implemented using DM video framework and heavily reuse code written for older SoCs. Additionally, driver also implements plug detection.

All patches in first version were merged except the last one. Because this version only split one patch into multiple and doesn't do any code change, I didn't write any changelog.

Patch 1 renames tve.c to tve_common.c to better describe it's intention.

Patch 2 adds TVE base address.

Patch 3 adds support for TVE to DE2 driver.

Patch 4 finally adds TVE driver for H3 and H5.

Everything was tested on H3 (OrangePi 2) and H5 (OrangePi PC2) board. Since this series changes video driver for old SoCs, I would kindly ask that someone tests if everything still works as expected.

Best regards,
Jernej

Jernej Skrabec (4):
sunxi: video: Rename tve.c to tve_common.c
sunxi: Add base address for TV encoder
sunxi: video: Add support for CSC and TVE to DE2 driver
sunxi: video: Add H3/H5 TV out driver

 arch/arm/include/asm/arch-sunxi/cpu_sun4i.h |  10 ++
 arch/arm/include/asm/arch-sunxi/display2.h  |  17 +++
 arch/arm/include/asm/arch-sunxi/tve.h       |  17 ++-
 drivers/video/sunxi/Makefile                |   4 +-
 drivers/video/sunxi/sunxi_de2.c             |  60 ++++++++---
 drivers/video/sunxi/sunxi_tve.c             | 156 ++++++++++++++++++++++++++++
 drivers/video/sunxi/{tve.c => tve_common.c} |   6 +-
 7 files changed, 252 insertions(+), 18 deletions(-)
 create mode 100644 drivers/video/sunxi/sunxi_tve.c
 rename drivers/video/sunxi/{tve.c => tve_common.c} (95%)
Signature · 1 line
2.13.0
#1 lore raw

PATCH v2 1/4 sunxi: video: Rename tve.c to tve_common.c

In order to avoid future confusion with similary named files, rename tve.c to tve_common.c. New name better represents the fact that this file holds code which can be and will be shared between multiple drivers.

---

 drivers/video/sunxi/Makefile                | 2 +-
 drivers/video/sunxi/{tve.c => tve_common.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/video/sunxi/{tve.c => tve_common.c} (100%)
drivers/video/sunxi/Makefile +1 -1
index dbaab61b59..0d64c2021f 100644
@@ -5,5 +5,5 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
-obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o lcdc.o tve.o ../videomodes.o
+obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o lcdc.o tve_common.o ../videomodes.o
 obj-$(CONFIG_VIDEO_DE2) += sunxi_de2.o sunxi_dw_hdmi.o lcdc.o ../dw_hdmi.o
drivers/video/sunxi/tve_common.c renamed from drivers/video/sunxi/tve.c +0 -0
similarity index 100%
rename from drivers/video/sunxi/tve.c
rename to drivers/video/sunxi/tve_common.c
Signature · 1 line
2.13.0
#2 lore raw

PATCH v2 1/4 sunxi: video: Rename tve.c to tve_common.c

Quoted 5 lines · Jernej Skrabec

In order to avoid future confusion with similary named files, rename tve.c to tve_common.c. New name better represents the fact that this file holds code which can be and will be shared between multiple drivers.

Thanks!
Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170522/6a25930b/attachment.sig>
#3 lore raw

PATCH v2 1/4 sunxi: video: Rename tve.c to tve_common.c

In reply to Jernej Skrabec
Quoted 4 lines · Jernej Skrabec

In order to avoid future confusion with similary named files, rename tve.c to tve_common.c. New name better represents the fact that this file holds code which can be and will be shared between multiple drivers.

Quoted 8 lines

---

 drivers/video/sunxi/Makefile                | 2 +-
 drivers/video/sunxi/{tve.c => tve_common.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/video/sunxi/{tve.c => tve_common.c} (100%)
#4 lore raw

PATCH v2 1/4 sunxi: video: Rename tve.c to tve_common.c

In reply to Jernej Skrabec

Hi Jernej,

Quoted 11 lines · On Fri, 19 May 2017 17:41:14 +0200 Jernej Skrabec jernej.skrabec at siol.net…

In order to avoid future confusion with similary named files, rename tve.c to tve_common.c. New name better represents the fact that this file holds code which can be and will be shared between multiple drivers.

---

 drivers/video/sunxi/Makefile                | 2 +-
 drivers/video/sunxi/{tve.c => tve_common.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/video/sunxi/{tve.c => tve_common.c} (100%)

applied to u-boot-video/master, thanks!

--
Anatolij

#5 lore raw

PATCH v2 2/4 sunxi: Add base address for TV encoder

In reply to Jernej Skrabec

This commit adds TVE base address for Allwinner H3 and H5 SoCs.

---

 arch/arm/include/asm/arch-sunxi/cpu_sun4i.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm/include/asm/arch-sunxi/cpu_sun4i.h +10 -0
index 6aa5e91ada..2419062d45 100644
@@ -34,7 +34,9 @@
 #define SUNXI_MS_BASE			0x01c07000
 #define SUNXI_TVD_BASE			0x01c08000
 #define SUNXI_CSI0_BASE			0x01c09000
+#ifndef CONFIG_MACH_SUNXI_H3_H5
 #define SUNXI_TVE0_BASE			0x01c0a000
+#endif
 #define SUNXI_EMAC_BASE			0x01c0b000
 #define SUNXI_LCD0_BASE			0x01c0C000
 #define SUNXI_LCD1_BASE			0x01c0d000
@@ -161,10 +163,18 @@ defined(CONFIG_MACH_SUN50I)
 /* module sram */
 #define SUNXI_SRAM_C_BASE		0x01d00000
 
+#ifndef CONFIG_MACH_SUN8I_H3
 #define SUNXI_DE_FE0_BASE		0x01e00000
+#else
+#define SUNXI_TVE0_BASE			0x01e00000
+#endif
 #define SUNXI_DE_FE1_BASE		0x01e20000
 #define SUNXI_DE_BE0_BASE		0x01e60000
+#ifndef CONFIG_MACH_SUN50I_H5
 #define SUNXI_DE_BE1_BASE		0x01e40000
+#else
+#define SUNXI_TVE0_BASE			0x01e40000
+#endif
 #define SUNXI_MP_BASE			0x01e80000
 #define SUNXI_AVG_BASE			0x01ea0000
 
Signature · 1 line
2.13.0
#6 lore raw

PATCH v2 2/4 sunxi: Add base address for TV encoder

Quoted 3 lines · Jernej Skrabec

This commit adds TVE base address for Allwinner H3 and H5 SoCs.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170522/e1973afc/attachment.sig>
#7 lore raw

PATCH v2 2/4 sunxi: Add base address for TV encoder

In reply to Jernej Skrabec
Quoted 7 lines · On Fri, 19 May 2017 17:41:15 +0200 Jernej Skrabec jernej.skrabec at siol.net…

This commit adds TVE base address for Allwinner H3 and H5 SoCs.

---

 arch/arm/include/asm/arch-sunxi/cpu_sun4i.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

applied to u-boot-video/master, thanks!

--
Anatolij

#8 lore raw

PATCH v2 3/4 sunxi: video: Add support for CSC and TVE to DE2 driver

In reply to Jernej Skrabec

Extend DE2 driver with support for TVE driver, which will be added in next commit. TVE unit expects data to be in YUV format, so CSC support is also added here.

Note that HDMI driver has higher priority, so TV out is not probed if HDMI monitor is detected.

---

 arch/arm/include/asm/arch-sunxi/display2.h | 17 +++++++++
 drivers/video/sunxi/sunxi_de2.c            | 60 ++++++++++++++++++++++++------
 2 files changed, 65 insertions(+), 12 deletions(-)
arch/arm/include/asm/arch-sunxi/display2.h +17 -0
index b5875f9605..359cacd90b 100644
@@ -90,6 +90,23 @@ struct de_ui {
 	u32 ovl_size;
 };
 
+struct de_csc {
+	u32 csc_ctl;
+	u8 res[0xc];
+	u32 coef11;
+	u32 coef12;
+	u32 coef13;
+	u32 coef14;
+	u32 coef21;
+	u32 coef22;
+	u32 coef23;
+	u32 coef24;
+	u32 coef31;
+	u32 coef32;
+	u32 coef33;
+	u32 coef34;
+};
+
 /*
  * DE register constants.
  */
drivers/video/sunxi/sunxi_de2.c +48 -12

106 lines not on this page · raw message

Signature · 1 line
2.13.0
#9 lore raw

PATCH v2 3/4 sunxi: video: Add support for CSC and TVE to DE2 driver

Quoted 8 lines · Jernej Skrabec

Extend DE2 driver with support for TVE driver, which will be added in next commit. TVE unit expects data to be in YUV format, so CSC support is also added here.

Note that HDMI driver has higher priority, so TV out is not probed if HDMI monitor is detected.

Thanks!
Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170522/b976b6a6/attachment.sig>
#10 lore raw

PATCH v2 3/4 sunxi: video: Add support for CSC and TVE to DE2 driver

In reply to Jernej Skrabec
Quoted 7 lines · Jernej Skrabec

Extend DE2 driver with support for TVE driver, which will be added in next commit. TVE unit expects data to be in YUV format, so CSC support is also added here.

Note that HDMI driver has higher priority, so TV out is not probed if HDMI monitor is detected.

Quoted 6 lines

---

 arch/arm/include/asm/arch-sunxi/display2.h | 17 +++++++++
 drivers/video/sunxi/sunxi_de2.c            | 60 ++++++++++++++++++++++++------
 2 files changed, 65 insertions(+), 12 deletions(-)
#11 lore raw

PATCH v2 3/4 sunxi: video: Add support for CSC and TVE to DE2 driver

In reply to Jernej Skrabec
Quoted 7 lines · On Fri, 19 May 2017 17:41:16 +0200 Jernej Skrabec jernej.skrabec at siol.net…

Extend DE2 driver with support for TVE driver, which will be added in next commit. TVE unit expects data to be in YUV format, so CSC support is also added here.

Note that HDMI driver has higher priority, so TV out is not probed if HDMI monitor is detected.

Quoted 6 lines

---

 arch/arm/include/asm/arch-sunxi/display2.h | 17 +++++++++
 drivers/video/sunxi/sunxi_de2.c            | 60 ++++++++++++++++++++++++------
 2 files changed, 65 insertions(+), 12 deletions(-)

applied to u-boot-video/master, thanks!

--
Anatolij

#12 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

In reply to Jernej Skrabec

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

---

 arch/arm/include/asm/arch-sunxi/tve.h |  17 +++-
 drivers/video/sunxi/Makefile          |   2 +-
 drivers/video/sunxi/sunxi_tve.c       | 156 ++++++++++++++++++++++++++++++++++
 drivers/video/sunxi/tve_common.c      |   6 +-
 4 files changed, 176 insertions(+), 5 deletions(-)
 create mode 100644 drivers/video/sunxi/sunxi_tve.c
arch/arm/include/asm/arch-sunxi/tve.h +15 -2
index 41a14a68e4..ff34bbbc12 100644
@@ -45,7 +45,9 @@ struct sunxi_tve_reg {
 	u32 csc_reg1;			/* 0x044 */
 	u32 csc_reg2;			/* 0x048 */
 	u32 csc_reg3;			/* 0x04c */
-	u8 res1[0xb0];			/* 0x050 */
+	u8 res1[0xa8];			/* 0x050 */
+	u32 auto_detect_cfg0;		/* 0x0f8 */
+	u32 auto_detect_cfg1;		/* 0x0fc */
 	u32 color_burst;		/* 0x100 */
 	u32 vsync_num;			/* 0x104 */
 	u32 notch_freq;			/* 0x108 */
@@ -62,6 +64,10 @@ struct sunxi_tve_reg {
 	u32 slave_para;			/* 0x134 */
 	u32 cfg1;			/* 0x138 */
 	u32 cfg2;			/* 0x13c */
+	u8 res2[0x1c4];			/* 0x140 */
+	u32 calibration;		/* 0x304 */
+	u8 res3[0x4];			/* 0x308 */
+	u32 unknown3;			/* 0x30c */
 };
 
 /*
@@ -79,12 +85,14 @@ struct sunxi_tve_reg {
 #define SUNXI_TVE_CFG0_PAL			0x07030001
 #define SUNXI_TVE_CFG0_NTSC			0x07030000
 #define SUNXI_TVE_DAC_CFG0_VGA			0x403e1ac7
-#ifdef CONFIG_MACH_SUN5I
+#if defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUNXI_H3_H5)
 #define SUNXI_TVE_DAC_CFG0_COMPOSITE		0x433f0009
 #else
 #define SUNXI_TVE_DAC_CFG0_COMPOSITE		0x403f0008
 #endif
+#define SUNXI_TVE_DAC_CFG0_DETECTION		0x433f0289
 #define SUNXI_TVE_FILTER_COMPOSITE		0x00000120
+#define SUNXI_TVE_CHROMA_FREQ_PAL		0x2a098acb
 #define SUNXI_TVE_CHROMA_FREQ_PAL_M		0x21e6efe3
 #define SUNXI_TVE_CHROMA_FREQ_PAL_NC		0x21f69446
 #define SUNXI_TVE_PORCH_NUM_PAL			0x008a0018
@@ -105,6 +113,8 @@ struct sunxi_tve_reg {
 #define SUNXI_TVE_AUTO_DETECT_STATUS_SHORT_GND	3
 #define SUNXI_TVE_AUTO_DETECT_DEBOUNCE_SHIFT(d)	((d) * 8)
 #define SUNXI_TVE_AUTO_DETECT_DEBOUNCE_MASK(d)	(0xf << ((d) * 8))
+#define SUNXI_TVE_AUTO_DETECT_CFG0		0x00000280
+#define SUNXI_TVE_AUTO_DETECT_CFG1		0x028F00FF
 #define SUNXI_TVE_CSC_REG0_ENABLE		(1 << 31)
 #define SUNXI_TVE_CSC_REG0			0x08440832
 #define SUNXI_TVE_CSC_REG1			0x3b6dace1
@@ -124,6 +134,9 @@ struct sunxi_tve_reg {
 #define SUNXI_TVE_RESYNC_NUM_PAL		0x800d000c
 #define SUNXI_TVE_RESYNC_NUM_NTSC		0x000e000c
 #define SUNXI_TVE_SLAVE_PARA_COMPOSITE		0x00000000
+#define SUNXI_TVE_CALIBRATION_H3		0x02000c00
+#define SUNXI_TVE_CALIBRATION_H5		0x02850000
+#define SUNXI_TVE_UNKNOWN3_H5			0x00101110
 
 void tvencoder_mode_set(struct sunxi_tve_reg * const tve, enum tve_mode mode);
 void tvencoder_enable(struct sunxi_tve_reg * const tve);
drivers/video/sunxi/Makefile +1 -1
index 0d64c2021f..e63c1d65bc 100644
@@ -6,4 +6,4 @@
 #
 
 obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o lcdc.o tve_common.o ../videomodes.o
-obj-$(CONFIG_VIDEO_DE2) += sunxi_de2.o sunxi_dw_hdmi.o lcdc.o ../dw_hdmi.o
+obj-$(CONFIG_VIDEO_DE2) += sunxi_de2.o sunxi_dw_hdmi.o sunxi_tve.o lcdc.o tve_common.o ../dw_hdmi.o
drivers/video/sunxi/sunxi_tve.c new file +156 -0

156 lines not on this page · raw message

drivers/video/sunxi/tve_common.c +4 -2
index adea78a69a..ef99c111e3 100644
@@ -25,8 +25,6 @@ void tvencoder_mode_set(struct sunxi_tve_reg * const tve, enum tve_mode mode)
 		writel(SUNXI_TVE_UNKNOWN1_VGA, &tve->unknown1);
 		break;
 	case tve_mode_composite_pal_nc:
-		writel(SUNXI_TVE_CHROMA_FREQ_PAL_NC, &tve->chroma_freq);
-		/* Fall through */
 	case tve_mode_composite_pal:
 		writel(SUNXI_TVE_GCTRL_DAC_INPUT(0, 1) |
 		       SUNXI_TVE_GCTRL_DAC_INPUT(1, 2) |
@@ -35,6 +33,10 @@ void tvencoder_mode_set(struct sunxi_tve_reg * const tve, enum tve_mode mode)
 		writel(SUNXI_TVE_CFG0_PAL, &tve->cfg0);
 		writel(SUNXI_TVE_DAC_CFG0_COMPOSITE, &tve->dac_cfg0);
 		writel(SUNXI_TVE_FILTER_COMPOSITE, &tve->filter);
+		if (mode == tve_mode_composite_pal)
+			writel(SUNXI_TVE_CHROMA_FREQ_PAL, &tve->chroma_freq);
+		else
+			writel(SUNXI_TVE_CHROMA_FREQ_PAL_NC, &tve->chroma_freq);
 		writel(SUNXI_TVE_PORCH_NUM_PAL, &tve->porch_num);
 		writel(SUNXI_TVE_LINE_NUM_PAL, &tve->line_num);
 		writel(SUNXI_TVE_BLANK_BLACK_LEVEL_PAL,
Signature · 1 line
2.13.0
#13 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

Quoted 4 lines · Jernej Skrabec

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we only support PAL right now and reject any other option), and using composite-pal as monitor.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170522/2a5a8ae4/attachment.sig>
#14 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

Hi Maxime,

Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):

Quoted 10 lines · Jernej Skrabec
Quoted 4 lines

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we only support PAL right now and reject any other option), and using composite-pal as monitor.

I have few arguments against that:

1. Code for parsing that env variable is in videomodes.[c|h], which is clearly a part of an older video framework (ctfb). I didn't want to include any legacy code.

2. Even if this code is used for parsing, it would bring a lot of confusion. For now, we can say that docs/README.video does not apply to H3 and newer SoCs. If we implement this only partially, we would need to describe in details which of each setting is honored with the new driver and which not. Even then, a lot of users would skip that description and complain anyway.

3. If anything is done in this direction, I think that it is better to extend DM video framework so other drivers would benefit from that work too.

Best regards,
Jernej

#15 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

Hi Jernej,

Quoted 18 lines · Jernej Å krabec

Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):

Quoted 10 lines · Jernej Skrabec
Quoted 4 lines

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we only support PAL right now and reject any other option), and using composite-pal as monitor.

I have few arguments against that:

1. Code for parsing that env variable is in videomodes.[c|h], which is clearly a part of an older video framework (ctfb). I didn't want to include any legacy code.

Quoted 5 lines

2. Even if this code is used for parsing, it would bring a lot of confusion. For now, we can say that docs/README.video does not apply to H3 and newer SoCs. If we implement this only partially, we would need to describe in details which of each setting is honored with the new driver and which not. Even then, a lot of users would skip that description and complain anyway.

The issue with this, and we've been bitten very hard on this one with the CHIP, is that you don't really have a clear majority on that one. If you support only PAL, half the world will be left out, and same thing with NTSC (for some reason, we never needed to support the less common ones like PAL-M or NTSC-J, but that just might be because it never really sold that well in those countries, I don't have any numbers on that).

The point is, if you just hardcode PAL for now, you will have half your users complain, and then, when we will introduce NTSC support eventually, we'll have to introduce some mechanism to switch between the two, then we'll probably break the behaviour our users relied on before, making the other half of our users pissed.

I'm not sure this is something we should just discard, or at least the second part. Having the selection mechanism in place, even if we don't support all the settings and just report an error in the logs in such a case address the latter issue.

You'll also need to address how to setup the overscan, since this is really something you want to have very quick.

Quoted 3 lines

3. If anything is done in this direction, I think that it is better to extend DM video framework so other drivers would benefit from that work too.

That makes sense, but again, this is a pre-requisite for me. And it's not that hard to support the video modelines with a device model driver, Linux does it, and you have a string identifier at the beginning of it. It just has to be deterministic, but I don't think this is an issue with U-Boot's DM.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170523/7de354ba/attachment.sig>
#16 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

Hi,

Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):

Quoted 40 lines

Hi Jernej,

Quoted 17 lines · Jernej Å krabec

Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):

Quoted 9 lines · Jernej Skrabec
Quoted 4 lines

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we only support PAL right now and reject any other option), and using composite-pal as monitor.

I have few arguments against that:

1. Code for parsing that env variable is in videomodes.[c|h], which is clearly a part of an older video framework (ctfb). I didn't want to include any legacy code.

Quoted 6 lines

2. Even if this code is used for parsing, it would bring a lot of confusion. For now, we can say that docs/README.video does not apply to H3 and newer SoCs. If we implement this only partially, we would need to describe in details which of each setting is honored with the new driver and which not. Even then, a lot of users would skip that description and complain anyway.

The issue with this, and we've been bitten very hard on this one with the CHIP, is that you don't really have a clear majority on that one. If you support only PAL, half the world will be left out, and same thing with NTSC (for some reason, we never needed to support the less common ones like PAL-M or NTSC-J, but that just might be because it never really sold that well in those countries, I don't have any numbers on that).

The point is, if you just hardcode PAL for now, you will have half your users complain, and then, when we will introduce NTSC support eventually, we'll have to introduce some mechanism to switch between the two, then we'll probably break the behaviour our users relied on before, making the other half of our users pissed.

Quoted 7 lines

I'm not sure this is something we should just discard, or at least the second part. Having the selection mechanism in place, even if we don't support all the settings and just report an error in the logs in such a case address the latter issue.

You'll also need to address how to setup the overscan, since this is really something you want to have very quick.

Ok, I'm prepared to tackle this. Do you think it is worth to delay driver merge?

Quoted 11 lines
Quoted 4 lines

3. If anything is done in this direction, I think that it is better to extend DM video framework so other drivers would benefit from that work too.

That makes sense, but again, this is a pre-requisite for me. And it's not that hard to support the video modelines with a device model driver, Linux does it, and you have a string identifier at the beginning of it. It just has to be deterministic, but I don't think this is an issue with U-Boot's DM.

Ok, so how do you think we should implement this? If only composite modes are supported in the string, someone might try to set hdmi monitor. Should we just ignore such settings and maybe print a warning?

Also the question for Simon, how should I merge code for parsing video string from drivers/video/videomodes.c to DM video in a way to be useful to most drivers?

I guess this is the first time to support analog video output in DM video driver and there are some things missing such as a way to select TV standard and define overscan.

Or alternatively, I could make just quick edit to sunxi_tve.c driver and directly use old functions as they are. That way we could get something working very quickly, but I don't like much such approach.

Best regards,
Jernej

#17 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

Quoted 48 lines · Jernej Škrabec

Hi,

Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):

Quoted 45 lines

Hi Jernej,

Quoted 17 lines · Jernej Škrabec

Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):

Quoted 9 lines · Jernej Skrabec
Quoted 4 lines

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we only support PAL right now and reject any other option), and using composite-pal as monitor.

I have few arguments against that:

1. Code for parsing that env variable is in videomodes.[c|h], which is clearly a part of an older video framework (ctfb). I didn't want to include any legacy code.

Quoted 6 lines

2. Even if this code is used for parsing, it would bring a lot of confusion. For now, we can say that docs/README.video does not apply to H3 and newer SoCs. If we implement this only partially, we would need to describe in details which of each setting is honored with the new driver and which not. Even then, a lot of users would skip that description and complain anyway.

The issue with this, and we've been bitten very hard on this one with the CHIP, is that you don't really have a clear majority on that one. If you support only PAL, half the world will be left out, and same thing with NTSC (for some reason, we never needed to support the less common ones like PAL-M or NTSC-J, but that just might be because it never really sold that well in those countries, I don't have any numbers on that).

The point is, if you just hardcode PAL for now, you will have half your users complain, and then, when we will introduce NTSC support eventually, we'll have to introduce some mechanism to switch between the two, then we'll probably break the behaviour our users relied on before, making the other half of our users pissed.

I'm not sure this is something we should just discard, or at least the second part. Having the selection mechanism in place, even if we don't support all the settings and just report an error in the logs in such a case address the latter issue.

Quoted 5 lines
Quoted 2 lines

You'll also need to address how to setup the overscan, since this is really something you want to have very quick.

Ok, I'm prepared to tackle this. Do you think it is worth to delay driver merge?

Not per se, but that should definitely be part of the same release, so if you can make it by then, then we can merge that right now, and merge the rest later. If you can't, then we'll have to postpone it a bit.

Quoted 10 lines
Quoted 9 lines
Quoted 3 lines

3. If anything is done in this direction, I think that it is better to extend DM video framework so other drivers would benefit from that work too.

That makes sense, but again, this is a pre-requisite for me. And it's not that hard to support the video modelines with a device model driver, Linux does it, and you have a string identifier at the beginning of it. It just has to be deterministic, but I don't think this is an issue with U-Boot's DM.

Quoted 4 lines

Ok, so how do you think we should implement this? If only composite modes are supported in the string, someone might try to set hdmi monitor. Should we just ignore such settings and maybe print a warning?

I'm not sure it was your question, but I would just reject any improper configuration, and not display anything in such a case.

Quoted 7 lines

Also the question for Simon, how should I merge code for parsing video string from drivers/video/videomodes.c to DM video in a way to be useful to most drivers?

I guess this is the first time to support analog video output in DM video driver and there are some things missing such as a way to select TV standard and define overscan.

I think it can be done in the same way linux does it here:
http://elixir.free-electrons.com/linux/latest/source/drivers/video/fbdev/core/fb_cmdline.c#L35

You basically check first that there is a string matching what your driver expects, and then get the options.

We could imagine having some extra function to parse that string and give back a structure filled with whatever was set in that command line. We could imagine having the common custom properties to be parsed at that same time.

Quoted 3 lines

Or alternatively, I could make just quick edit to sunxi_tve.c driver and directly use old functions as they are. That way we could get something working very quickly, but I don't like much such approach.

That's another approach, I'm fine with it as a temporary measure, but I'm not sure how Simon feels about it.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170530/43e95896/attachment.sig>
#18 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

Hi Maxime,

Quoted 97 lines · Maxime Ripard
Quoted 48 lines · Jernej Škrabec

Hi,

Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):

Quoted 45 lines

Hi Jernej,

Quoted 17 lines · Jernej Škrabec

Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):

Quoted 9 lines · Jernej Skrabec
Quoted 4 lines

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we only support PAL right now and reject any other option), and using composite-pal as monitor.

I have few arguments against that:

1. Code for parsing that env variable is in videomodes.[c|h], which is clearly a part of an older video framework (ctfb). I didn't want to include any legacy code.

Quoted 6 lines

2. Even if this code is used for parsing, it would bring a lot of confusion. For now, we can say that docs/README.video does not apply to H3 and newer SoCs. If we implement this only partially, we would need to describe in details which of each setting is honored with the new driver and which not. Even then, a lot of users would skip that description and complain anyway.

The issue with this, and we've been bitten very hard on this one with the CHIP, is that you don't really have a clear majority on that one. If you support only PAL, half the world will be left out, and same thing with NTSC (for some reason, we never needed to support the less common ones like PAL-M or NTSC-J, but that just might be because it never really sold that well in those countries, I don't have any numbers on that).

The point is, if you just hardcode PAL for now, you will have half your users complain, and then, when we will introduce NTSC support eventually, we'll have to introduce some mechanism to switch between the two, then we'll probably break the behaviour our users relied on before, making the other half of our users pissed.

I'm not sure this is something we should just discard, or at least the second part. Having the selection mechanism in place, even if we don't support all the settings and just report an error in the logs in such a case address the latter issue.

Quoted 5 lines
Quoted 2 lines

You'll also need to address how to setup the overscan, since this is really something you want to have very quick.

Ok, I'm prepared to tackle this. Do you think it is worth to delay driver merge?

Not per se, but that should definitely be part of the same release, so if you can make it by then, then we can merge that right now, and merge the rest later. If you can't, then we'll have to postpone it a bit.

Quoted 10 lines
Quoted 9 lines
Quoted 3 lines

3. If anything is done in this direction, I think that it is better to extend DM video framework so other drivers would benefit from that work too.

That makes sense, but again, this is a pre-requisite for me. And it's not that hard to support the video modelines with a device model driver, Linux does it, and you have a string identifier at the beginning of it. It just has to be deterministic, but I don't think this is an issue with U-Boot's DM.

Quoted 4 lines

Ok, so how do you think we should implement this? If only composite modes are supported in the string, someone might try to set hdmi monitor. Should we just ignore such settings and maybe print a warning?

I'm not sure it was your question, but I would just reject any improper configuration, and not display anything in such a case.

Quoted 7 lines

Also the question for Simon, how should I merge code for parsing video string from drivers/video/videomodes.c to DM video in a way to be useful to most drivers?

I guess this is the first time to support analog video output in DM video driver and there are some things missing such as a way to select TV standard and define overscan.

I think it can be done in the same way linux does it here:
http://elixir.free-electrons.com/linux/latest/source/drivers/video/fbdev/core/fb_cmdline.c#L35

You basically check first that there is a string matching what your driver expects, and then get the options.

We could imagine having some extra function to parse that string and give back a structure filled with whatever was set in that command line. We could imagine having the common custom properties to be parsed at that same time.

Quoted 6 lines
Quoted 3 lines

Or alternatively, I could make just quick edit to sunxi_tve.c driver and directly use old functions as they are. That way we could get something working very quickly, but I don't like much such approach.

That's another approach, I'm fine with it as a temporary measure, but I'm not sure how Simon feels about it.

Well how about a function that decodes the string into a struct? That function could be called by any drivers that need it.

Regards,
Simon

Quoted 7 lines

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

#19 lore raw

PATCH v2 4/4 sunxi: video: Add H3/H5 TV out driver

Quoted 104 lines · Simon Glass

Hi Maxime,

Quoted 101 lines · Maxime Ripard
Quoted 48 lines · Jernej Škrabec

Hi,

Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):

Quoted 45 lines

Hi Jernej,

Quoted 17 lines · Jernej Škrabec

Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):

Quoted 9 lines · Jernej Skrabec
Quoted 4 lines

This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we only support PAL right now and reject any other option), and using composite-pal as monitor.

I have few arguments against that:

1. Code for parsing that env variable is in videomodes.[c|h], which is clearly a part of an older video framework (ctfb). I didn't want to include any legacy code.

Quoted 6 lines

2. Even if this code is used for parsing, it would bring a lot of confusion. For now, we can say that docs/README.video does not apply to H3 and newer SoCs. If we implement this only partially, we would need to describe in details which of each setting is honored with the new driver and which not. Even then, a lot of users would skip that description and complain anyway.

The issue with this, and we've been bitten very hard on this one with the CHIP, is that you don't really have a clear majority on that one. If you support only PAL, half the world will be left out, and same thing with NTSC (for some reason, we never needed to support the less common ones like PAL-M or NTSC-J, but that just might be because it never really sold that well in those countries, I don't have any numbers on that).

The point is, if you just hardcode PAL for now, you will have half your users complain, and then, when we will introduce NTSC support eventually, we'll have to introduce some mechanism to switch between the two, then we'll probably break the behaviour our users relied on before, making the other half of our users pissed.

I'm not sure this is something we should just discard, or at least the second part. Having the selection mechanism in place, even if we don't support all the settings and just report an error in the logs in such a case address the latter issue.

Quoted 5 lines
Quoted 2 lines

You'll also need to address how to setup the overscan, since this is really something you want to have very quick.

Ok, I'm prepared to tackle this. Do you think it is worth to delay driver merge?

Not per se, but that should definitely be part of the same release, so if you can make it by then, then we can merge that right now, and merge the rest later. If you can't, then we'll have to postpone it a bit.

Quoted 10 lines
Quoted 9 lines
Quoted 3 lines

3. If anything is done in this direction, I think that it is better to extend DM video framework so other drivers would benefit from that work too.

That makes sense, but again, this is a pre-requisite for me. And it's not that hard to support the video modelines with a device model driver, Linux does it, and you have a string identifier at the beginning of it. It just has to be deterministic, but I don't think this is an issue with U-Boot's DM.

Quoted 4 lines

Ok, so how do you think we should implement this? If only composite modes are supported in the string, someone might try to set hdmi monitor. Should we just ignore such settings and maybe print a warning?

I'm not sure it was your question, but I would just reject any improper configuration, and not display anything in such a case.

Quoted 7 lines

Also the question for Simon, how should I merge code for parsing video string from drivers/video/videomodes.c to DM video in a way to be useful to most drivers?

I guess this is the first time to support analog video output in DM video driver and there are some things missing such as a way to select TV standard and define overscan.

I think it can be done in the same way linux does it here:
http://elixir.free-electrons.com/linux/latest/source/drivers/video/fbdev/core/fb_cmdline.c#L35

You basically check first that there is a string matching what your driver expects, and then get the options.

We could imagine having some extra function to parse that string and give back a structure filled with whatever was set in that command line. We could imagine having the common custom properties to be parsed at that same time.

Quoted 3 lines

Or alternatively, I could make just quick edit to sunxi_tve.c driver and directly use old functions as they are. That way we could get something working very quickly, but I don't like much such approach.

Quoted 5 lines
Quoted 2 lines

That's another approach, I'm fine with it as a temporary measure, but I'm not sure how Simon feels about it.

Well how about a function that decodes the string into a struct? That function could be called by any drivers that need it.

That was my first suggestion, so I'm fine with that :)

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170601/ec58bb59/attachment.sig>