[PATCH 4/5] sun7i: add support for Micronova srl s027_3 board

[Andre Przywara]

On Fri, 19 Feb 2021 16:27:41 +0100
Giulio Benetti <giulio.benetti at benettiengineering.com> wrote:

Hi Giulio,

> On 2/19/21 12:37 PM, Andre Przywara wrote:
> > On Fri, 19 Feb 2021 11:12:08 +0100
> > Giulio Benetti <giulio.benetti at benettiengineering.com> wrote:
> > 
> > Hi,
> >   
> >> On 2/19/21 2:44 AM, Andre Przywara wrote:  
> >>> On Fri, 19 Feb 2021 00:26:19 +0100
> >>> Giulio Benetti <giulio.benetti at benettiengineering.com> wrote:
> >>>
> >>> Hi Giulio,
> >>>      
> >>>> From: Giulio Benetti <giulio.benetti at micronovasrl.com>
> >>>>
> >>>> Add support for Micronova srl s027_3 board by adding new vendor/board
> >>>> folders and using in local Makefile the already present
> >>>> board/sunxi/Makefile. The reason is that s027_3 needs a pin to be asserted
> >>>> to enable lcd before setting controller up and sunxi board doesn't provide
> >>>> a way to do this customization.  
> >>>
> >>> Have you tried CONFIG_VIDEO_LCD_POWER?  
> >>
> >> Yes, I already use that for DISPONOFF.  
> > 
> > So what are these pins for, exactly? Backlight? A regulator to provide
> > power to the LCD?  
> 
> It's the power rail for LCD.
> 
> > If you grep for "gpio.CONFIG" in sunxi_display.c, there are more GPIOs
> > that we set, for various reasons. Can you pick some from there, and set
> > them in your defconfig?  
> 
> Unfortunately my LCD_PWR drives a P-MOS so it must be inverted, while 
> LCD0_DISPONOFF can be meant as a lcd reset.
> 
> So I introduce CONFIG_VIDEO_LCD_POWER_ACTIVE_LOW for my board to work
> locally.
> 
> Would it make sense to send a patch for this for supporting my board?
> Or do I wait for you patchset [1] to be applied?
> 
> >>> I am strongly against introducing per-board code files, and we
> >>> definitely won't introduce them just for one GPIO.  
> >>
> >> I knew it was a strange hack, but I have 2 different pins to be set high
> >> before init video controller. Another idea that comes into my mind is to
> >> handle CONFIG_VIDEO_LCD_POWER as a gpio list and iterate/enable, what
> >> about that?  
> > 
> > Do you really need *two* pins to switch the LCD power supply?  
> 
> I've messed up names, I need only one pin for LCD power supply(the on 
> with P-MOS) but another to enable it(DISPONOFF). But here the problem 
> was that LCD power supply is active low.
> 
> > If yes,
> > this might be an option. But I guess at this point we want to go more a
> > DT route.  
> 
> Yes, agree.
> 
> > Speaking of which, I don't see any video output device in the DT, I
> > guess you just rely on simplefb.  
> 
> Yes, simplefb for u-boot while...
> 
> > But given that there is no bridge or
> > it seems to be transparent, at least, you might be able to describe
> > this in the DT as well, so Linux can use its own display driver?  
> 
> for Linux I use 4 different fdt overlays using fe,be,tcon and simple-panel.

I see that the simple-panel binding has a generic enable-gpios
property, so that sounds fine.
Can you share the DT snippets? Just to get an idea what is what and how
we can shortcut it, if needed?
There is also drivers/video/simple_panel.c in U-Boot, which would
handle the power-supply *and* enable-gpios properties already, can you
check if we can use that?
I am a bit reluctant to add just another set of CONFIG_GPIO_... options,
if everything is in the DT already.

How do you plan to handle the overlays? Is the LCD an option, or is it
always connected? You want to always enable it in the U-Boot
defconfig, don't you?

> > Then we might be able to find a more sustainable solution, based on
> > pins in the DT.  
> 
> Yes
> 
> > 
> > Cheers,
> > Andre
> > 
> > P.S. Can you please try to apply  
> 
> [1]:
> 
> > https://patchwork.ozlabs.org/project/uboot/patch/20210205010748.2646-1-andre.przywara@arm.com/
> > This would be merged around the same time, and I am curious if this
> > still works with an LCD.  
> 
> For sure I will, I was waiting for this and I've also started to port it 
> a bit but I have not much time and then you added a patch. I'm going to 
> test it and report. If it doesn't work I'll try to fix it for having 
> s027_3 board added using DT then.

As you have surely noticed, we currently see a warning when compiling
boards with the old display engine, so this patch is needed to stop
that and to avoid removal of video support. I will send a slightly
fixed version soonish and then queue this for the next merge window,
opening in April. So it would be wise if you base any work on that
patch.

Cheers,
Andre