[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 <jernej.skrabec at siol.net>
> > > 
> > > 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