Re: [U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Rask Ingemann Lambertsen]

On Tue, Dec 13, 2016 at 01:36:29AM +0100, Jernej Skrabec wrote:
> This patch adds support for hdmi output. It is designed in the same
> way as video driver for older Allwinner SoCs.
[...]
> diff --git a/drivers/video/sunxi_display2.c b/drivers/video/sunxi_display2.c
> new file mode 100644
> index 000..db376d9
> --- /dev/null
> +++ b/drivers/video/sunxi_display2.c
[...]
> + /* Find a prefilled simpefb node, matching out pipeline config */

s/out/our/ I guess.

-- 
Rask Ingemann Lambertsen
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Maxime Ripard]

Hi,

On Wed, Dec 14, 2016 at 11:11:19PM +0100, Jernej Škrabec wrote:
> > > While I took Rockchip HDMI code for reference, it can't be easily reused.
> > > First of, it uses DT nodes. I guess I could write DT binding or modify
> > > existing driver to work without it.
> > 
> > Like we discussed in the other part of the thread, I think the latter
> > would be easier to deal with.
> 
> I forgot to mention that it also uses driver model. The way I would go with 
> this is to split out common code to dwc_hdmi.c and have platform dependant 
> drivers for Rockchip, Allwinner, etc. Basically the same way as it is done in 
> Linux.

That sounds reasonable.

> > > Thirdly, and in my opinion most annoying, Rockchip driver uses 32
> > > bit aligned registers, but H3 does not. This also means a lot of
> > > work to make it more generic.
> > 
> > How does Linux deal with that? Would just using some kind of accessors
> > that would abstract that away from the driver help, or is it more
> > complicated?
> 
> Yes, Linux driver checks "reg-io-width" property and selects accessors 
> accordingly. I suppose I could do similar, save function pointer in a driver 
> private data. I suppose it is ok that platform specific code initializes this 
> private data?

Something like passing a flag stored in the platform data (and/or read
from DT for the relevant cases) to a generic accessor would also be a
solution, without the need for everyone to implement its accessors.

> > > Actually, H3 is more similar to i.MX6 HDMI in this regard, but
> > > driver's code is scattered throughout multiple files (search for
> > > mxc_hdmi.h inclusion). It is certainly doable, but it will take much
> > > more time.  Basically, U-Boot already has two drivers for DWC HDMI
> > > and with this patch it will get third. Merging all three
> > > implementations into one would be very tedious, but very desirable
> > > goal. I must state that I didn't really try to understand i.MX6 HDMI
> > > code at all, so I don't now how hard it would be to pull it out.
> > 
> > I'm not sure that merging a third and saying that it would be up to
> > the fourth to do the work is reasonable. It's just hiding an issue
> > under the carpet, but I don't see how it will be easier for the next
> > person to work on that. Quite the opposite actually.
> 
> True. I wonder if Renesas already prepared U-Boot DWC HDMI driver...
> 
> Do you think it makes sense to put all files related to Allwinner
> video drivers to sunxi subfolder? There will be at least 4 .c files
> according to my plan and even more in the future (at least for TV
> out driver).

On principle, I don't see anything wrong with that.

Thanks!
Maxime

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


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Jernej Škrabec]

Hi,

On Wed, Dec 14, 2016 at 11:28:43 CET, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Dec 13, 2016 at 09:13:11PM +0100, Jernej Škrabec wrote:
> > Hi,
> > 
> > On Tue, Dec 13, 2016 at 16:40:55 CET, Maxime Ripard wrote:
> > > Hi,
> > > 
> > > On Tue, Dec 13, 2016 at 01:36:29AM +0100, Jernej Skrabec wrote:
> > > > This patch adds support for hdmi output. It is designed in the same
> > > > way as video driver for older Allwinner SoCs.
> > > > 
> > > > First it checks if monitor is attached. If it is, recommended
> > > > timings are read from EDID. After that, DE2, TCON and HDMI are
> > > > configured according to this timings.
> > > > 
> > > > 32MB of RAM is used for framebuffer. This is just enough to support
> > > > 4K resolution.
> > > > 
> > > > SimpleFB is also supported by this driver.
> > > > 
> > > > Signed-off-by: Jernej Skrabec 
> > > 
> > > From the linux discussion, I recall that you said that the TCON was
> > > still the same, and the HDMI was something that could be shared with
> > > the Rockchip implementation. Did you look into sharing the TCON code
> > > (for example using a small "library" to share the functions) and to
> > > reuse Rockchip's HDMI code?
> > 
> > For now, I only reused one TCON function and some defines. I tought that
> > split would be better done a bit later, when the driver will get support
> > for LCD screens (A64, for example). At that time TCON code would also be
> > refactored to be more generic and properly tested that it can be used
> > with both drivers. Unfortunatelly, I don't have any board with older SoC
> > for testing.
> We can definitely do that. I think I have access to at least of board
> of each generation, so I can totally test whatever you come up with on
> those boards, and help you debugging whatever issue that might show up.
> 
> > While I took Rockchip HDMI code for reference, it can't be easily reused.
> > First of, it uses DT nodes. I guess I could write DT binding or modify
> > existing driver to work without it.
> 
> Like we discussed in the other part of the thread, I think the latter
> would be easier to deal with.

I forgot to mention that it also uses driver model. The way I would go with 
this is to split out common code to dwc_hdmi.c and have platform dependant 
drivers for Rockchip, Allwinner, etc. Basically the same way as it is done in 
Linux.

> 
> > Second issue here is same as in Linux, PHY code is tightly coupled
> > with controller code, so it needs to be decoupled first.
> 
> But it would be easier to deal with than Linux I guess. Can't you just
> create a new weak function to initialise the phy? And then, every
> platform can just overwrite it.

PHY functions are relatively self contained (~3 of them). If I develop my idea 
with shared code further, I could make phy_init() function weak and other two 
functions just public. Rockchip driver will use them, H3 won't. They will come 
handy for A83T and A80 HDMI driver, which uses DWC HDMI PHY. Well, phy_init() 
function needs to be further generalized (mpll table used in phy_init() is SoC 
specific), but I guess we will worry about that later.

> 
> > Thirdly, and in my opinion most annoying, Rockchip driver uses 32
> > bit aligned registers, but H3 does not. This also means a lot of
> > work to make it more generic.
> 
> How does Linux deal with that? Would just using some kind of accessors
> that would abstract that away from the driver help, or is it more
> complicated?

Yes, Linux driver checks "reg-io-width" property and selects accessors 
accordingly. I suppose I could do similar, save function pointer in a driver 
private data. I suppose it is ok that platform specific code initializes this 
private data?

> 
> > Actually, H3 is more similar to i.MX6 HDMI in this regard, but
> > driver's code is scattered throughout multiple files (search for
> > mxc_hdmi.h inclusion). It is certainly doable, but it will take much
> > more time.  Basically, U-Boot already has two drivers for DWC HDMI
> > and with this patch it will get third. Merging all three
> > implementations into one would be very tedious, but very desirable
> > goal. I must state that I didn't really try to understand i.MX6 HDMI
> > code at all, so I don't now how hard it would be to pull it out.
> 
> I'm not sure that merging a third and saying that it would be up to
> the fourth to do the work is reasonable. It's just hiding an issue
> under the carpet, but I don't see how it will be easier for the next
> person to work on that. Quite the opposite actually.

True. I wonder if Renesas already prepared U-Boot DWC HDMI driver...

Do you think it makes sense to put all files related to Allwinner video drivers 
to sunxi subfolder? There will be at least 4 .c files according to my plan and 
even more in the future (at least for TV out driver).

Best regards,
Jernej Škrabec

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

Re: [U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Maxime Ripard]

Hi,

On Tue, Dec 13, 2016 at 09:13:11PM +0100, Jernej Škrabec wrote:
> Hi,
> 
> On Tue, Dec 13, 2016 at 16:40:55 CET, Maxime Ripard wrote:
> > Hi,
> > 
> > On Tue, Dec 13, 2016 at 01:36:29AM +0100, Jernej Skrabec wrote:
> > > This patch adds support for hdmi output. It is designed in the same
> > > way as video driver for older Allwinner SoCs.
> > > 
> > > First it checks if monitor is attached. If it is, recommended
> > > timings are read from EDID. After that, DE2, TCON and HDMI are
> > > configured according to this timings.
> > > 
> > > 32MB of RAM is used for framebuffer. This is just enough to support
> > > 4K resolution.
> > > 
> > > SimpleFB is also supported by this driver.
> > > 
> > > Signed-off-by: Jernej Skrabec 
> > 
> > From the linux discussion, I recall that you said that the TCON was
> > still the same, and the HDMI was something that could be shared with
> > the Rockchip implementation. Did you look into sharing the TCON code
> > (for example using a small "library" to share the functions) and to
> > reuse Rockchip's HDMI code?
> 
> For now, I only reused one TCON function and some defines. I tought that 
> split 
> would be better done a bit later, when the driver will get support for LCD 
> screens (A64, for example). At that time TCON code would also be refactored 
> to 
> be more generic and properly tested that it can be used with both drivers. 
> Unfortunatelly, I don't have any board with older SoC for testing.

We can definitely do that. I think I have access to at least of board
of each generation, so I can totally test whatever you come up with on
those boards, and help you debugging whatever issue that might show up.

> While I took Rockchip HDMI code for reference, it can't be easily reused. 
> First of, it uses DT nodes. I guess I could write DT binding or modify 
> existing driver to work without it.

Like we discussed in the other part of the thread, I think the latter
would be easier to deal with.

> Second issue here is same as in Linux, PHY code is tightly coupled
> with controller code, so it needs to be decoupled first.

But it would be easier to deal with than Linux I guess. Can't you just
create a new weak function to initialise the phy? And then, every
platform can just overwrite it.

> Thirdly, and in my opinion most annoying, Rockchip driver uses 32
> bit aligned registers, but H3 does not. This also means a lot of
> work to make it more generic.

How does Linux deal with that? Would just using some kind of accessors
that would abstract that away from the driver help, or is it more
complicated?

> Actually, H3 is more similar to i.MX6 HDMI in this regard, but
> driver's code is scattered throughout multiple files (search for
> mxc_hdmi.h inclusion). It is certainly doable, but it will take much
> more time.  Basically, U-Boot already has two drivers for DWC HDMI
> and with this patch it will get third. Merging all three
> implementations into one would be very tedious, but very desirable
> goal. I must state that I didn't really try to understand i.MX6 HDMI
> code at all, so I don't now how hard it would be to pull it out.

I'm not sure that merging a third and saying that it would be up to
the fourth to do the work is reasonable. It's just hiding an issue
under the carpet, but I don't see how it will be easier for the next
person to work on that. Quite the opposite actually.

Maxime

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


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Jernej Škrabec]

Hi,

On Tue, Dec 13, 2016 at 16:40:55 CET, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Dec 13, 2016 at 01:36:29AM +0100, Jernej Skrabec wrote:
> > This patch adds support for hdmi output. It is designed in the same
> > way as video driver for older Allwinner SoCs.
> > 
> > First it checks if monitor is attached. If it is, recommended
> > timings are read from EDID. After that, DE2, TCON and HDMI are
> > configured according to this timings.
> > 
> > 32MB of RAM is used for framebuffer. This is just enough to support
> > 4K resolution.
> > 
> > SimpleFB is also supported by this driver.
> > 
> > Signed-off-by: Jernej Skrabec 
> 
> From the linux discussion, I recall that you said that the TCON was
> still the same, and the HDMI was something that could be shared with
> the Rockchip implementation. Did you look into sharing the TCON code
> (for example using a small "library" to share the functions) and to
> reuse Rockchip's HDMI code?

For now, I only reused one TCON function and some defines. I tought that split 
would be better done a bit later, when the driver will get support for LCD 
screens (A64, for example). At that time TCON code would also be refactored to 
be more generic and properly tested that it can be used with both drivers. 
Unfortunatelly, I don't have any board with older SoC for testing.

While I took Rockchip HDMI code for reference, it can't be easily reused. 
First of, it uses DT nodes. I guess I could write DT binding or modify 
existing driver to work without it. Second issue here is same as in Linux, PHY 
code is tightly coupled with controller code, so it needs to be decoupled 
first. Thirdly, and in my opinion most annoying, Rockchip driver uses 32 bit 
aligned registers, but H3 does not. This also means a lot of work to make it 
more generic. Actually, H3 is more similar to i.MX6 HDMI in this regard, but 
driver's code is scattered throughout multiple files (search for mxc_hdmi.h 
inclusion). It is certainly doable, but it will take much more time.
Basically, U-Boot already has two drivers for DWC HDMI and with this patch it 
will get third. Merging all three implementations into one would be very 
tedious, but very desirable goal. I must state that I didn't really try to 
understand i.MX6 HDMI code at all, so I don't now how hard it would be to pull 
it out.

By the way, did you get cover letter?

Best regards,
Jernej Skrabec

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


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Chen-Yu Tsai]

On Tue, Dec 13, 2016 at 11:40 PM, Maxime Ripard
 wrote:
> Hi,
>
> On Tue, Dec 13, 2016 at 01:36:29AM +0100, Jernej Skrabec wrote:
>> This patch adds support for hdmi output. It is designed in the same
>> way as video driver for older Allwinner SoCs.
>>
>> First it checks if monitor is attached. If it is, recommended
>> timings are read from EDID. After that, DE2, TCON and HDMI are
>> configured according to this timings.
>>
>> 32MB of RAM is used for framebuffer. This is just enough to support
>> 4K resolution.
>>
>> SimpleFB is also supported by this driver.
>>
>> Signed-off-by: Jernej Skrabec 
>
> From the linux discussion, I recall that you said that the TCON was
> still the same, and the HDMI was something that could be shared with
> the Rockchip implementation. Did you look into sharing the TCON code
> (for example using a small "library" to share the functions) and to
> reuse Rockchip's HDMI code?

I second the first proposal. The TCON structure has not changed since
the A10. Allwinner just removed the unused channels. Channel 1 is
removed from the first TCON, and channel 0 is removed from the second
TCON. So Allwinner is more or less committing to a static mapping for
the display pipeline outputs.

Regards
ChenYu
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Maxime Ripard]

Hi,

On Tue, Dec 13, 2016 at 01:36:29AM +0100, Jernej Skrabec wrote:
> This patch adds support for hdmi output. It is designed in the same
> way as video driver for older Allwinner SoCs.
> 
> First it checks if monitor is attached. If it is, recommended
> timings are read from EDID. After that, DE2, TCON and HDMI are
> configured according to this timings.
> 
> 32MB of RAM is used for framebuffer. This is just enough to support
> 4K resolution.
> 
> SimpleFB is also supported by this driver.
> 
> Signed-off-by: Jernej Skrabec 

From the linux discussion, I recall that you said that the TCON was
still the same, and the HDMI was something that could be shared with
the Rockchip implementation. Did you look into sharing the TCON code
(for example using a small "library" to share the functions) and to
reuse Rockchip's HDMI code?

Thanks,
Maxime

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


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC PATCH 2/3] sunxi: video: Add video driver for H3 SoC

[Jernej Skrabec]

This patch adds support for hdmi output. It is designed in the same
way as video driver for older Allwinner SoCs.

First it checks if monitor is attached. If it is, recommended
timings are read from EDID. After that, DE2, TCON and HDMI are
configured according to this timings.

32MB of RAM is used for framebuffer. This is just enough to support
4K resolution.

SimpleFB is also supported by this driver.

Signed-off-by: Jernej Skrabec 
---

 arch/arm/include/asm/arch-sunxi/cpu_sun4i.h |8 +
 arch/arm/include/asm/arch-sunxi/display2.h  |  377 ++
 board/sunxi/Kconfig |4 +-
 drivers/video/Makefile  |1 +
 drivers/video/sunxi_display2.c  | 1037 +++
 include/configs/sunxi-common.h  |   17 +-
 scripts/config_whitelist.txt|1 +
 7 files changed, 1438 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-sunxi/display2.h
 create mode 100644 drivers/video/sunxi_display2.c

diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h 
b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
index 7232f6d..9df6212 100644
--- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
+++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
@@ -18,6 +18,8 @@
 #define SUNXI_SRAM_D_BASE  0x0001  /* 4 kiB */
 #define SUNXI_SRAM_B_BASE  0x0002  /* 64 kiB (secure) */
 
+#define SUNXI_DE2_BASE 0x0100
+
 #ifdef CONFIG_MACH_SUN8I_A83T
 #define SUNXI_CPUCFG_BASE  0x0170
 #endif
@@ -46,7 +48,9 @@
 #define SUNXI_USB1_BASE0x01c14000
 #endif
 #define SUNXI_SS_BASE  0x01c15000
+#ifndef CONFIG_MACH_SUN8I_H3
 #define SUNXI_HDMI_BASE0x01c16000
+#endif
 #define SUNXI_SPI2_BASE0x01c17000
 #define SUNXI_SATA_BASE0x01c18000
 #ifdef CONFIG_SUNXI_GEN_SUN4I
@@ -163,6 +167,10 @@ defined(CONFIG_MACH_SUN50I)
 #define SUNXI_MP_BASE  0x01e8
 #define SUNXI_AVG_BASE 0x01ea
 
+#ifdef CONFIG_MACH_SUN8I_H3
+#define SUNXI_HDMI_BASE0x01ee
+#endif
+
 #define SUNXI_RTC_BASE 0x01f0
 #define SUNXI_PRCM_BASE0x01f01400
 
diff --git a/arch/arm/include/asm/arch-sunxi/display2.h 
b/arch/arm/include/asm/arch-sunxi/display2.h
new file mode 100644
index 000..b1d99d7
--- /dev/null
+++ b/arch/arm/include/asm/arch-sunxi/display2.h
@@ -0,0 +1,377 @@
+/*
+ * Sunxi platform display controller register and constant defines
+ *
+ * (C) Copyright 2016 Jernej Skrabec 
+ *
+ * Based on Linux DRM driver defines:
+ * Copyright (C) 2016 Jean-Francois Moine 
+ * Copyright (c) 2016 Allwinnertech Co., Ltd.
+ *
+ * Based on display.h:
+ * (C) Copyright 2014 Hans de Goede 
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#ifndef _SUNXI_DISPLAY2_H
+#define _SUNXI_DISPLAY2_H
+
+struct sunxi_lcdc_reg {
+   u32 ctrl;   /* 0x00 */
+   u32 int0;   /* 0x04 */
+   u32 int1;   /* 0x08 */
+   u8 res0[0x04];  /* 0x0c */
+   u32 tcon0_frm_ctrl; /* 0x10 */
+   u32 tcon0_frm_seed[6];  /* 0x14 */
+   u32 tcon0_frm_table[4]; /* 0x2c */
+   u8 res1[4]; /* 0x3c */
+   u32 tcon0_ctrl; /* 0x40 */
+   u32 tcon0_dclk; /* 0x44 */
+   u32 tcon0_timing_active;/* 0x48 */
+   u32 tcon0_timing_h; /* 0x4c */
+   u32 tcon0_timing_v; /* 0x50 */
+   u32 tcon0_timing_sync;  /* 0x54 */
+   u32 tcon0_hv_intf;  /* 0x58 */
+   u8 res2[0x04];  /* 0x5c */
+   u32 tcon0_cpu_intf; /* 0x60 */
+   u32 tcon0_cpu_wr_dat;   /* 0x64 */
+   u32 tcon0_cpu_rd_dat0;  /* 0x68 */
+   u32 tcon0_cpu_rd_dat1;  /* 0x6c */
+   u32 tcon0_ttl_timing0;  /* 0x70 */
+   u32 tcon0_ttl_timing1;  /* 0x74 */
+   u32 tcon0_ttl_timing2;  /* 0x78 */
+   u32 tcon0_ttl_timing3;  /* 0x7c */
+   u32 tcon0_ttl_timing4;  /* 0x80 */
+   u32 tcon0_lvds_intf;/* 0x84 */
+   u32 tcon0_io_polarity;  /* 0x88 */
+   u32 tcon0_io_tristate;  /* 0x8c */
+   u32 tcon1_ctrl; /* 0x90 */
+   u32 tcon1_timing_source;/* 0x94 */
+   u32 tcon1_timing_scale; /* 0x98 */
+   u32 tcon1_timing_out;   /* 0x9c */
+   u32 tcon1_timing_h; /* 0xa0 */
+   u32 tcon1_timing_v; /* 0xa4 */
+   u32 tcon1_timing_sync;  /* 0xa8 */
+   u8 res3[0x44];  /* 0xac */
+   u32 tcon1_io_polarity;  /* 0xf0 */
+   u32 tcon1_io_tristate;