[U-Boot] [PATCH 4/8] sunxi: video: Split out TCON code

[Jernej Škrabec]

Hi Maxime,

Dne četrtek, 09. marec 2017 ob 09:33:06 CET je Maxime Ripard napisal(a):
> Hi,
> 
> Thanks for your great work.
> 
> On Thu, Mar 09, 2017 at 12:34:40AM +0100, Jernej Skrabec wrote:
> > -	writel(0, &lcdc->tcon0_io_tristate);
> > +	sunxi_ctfb_mode_to_display_timing(mode, &timing);
> > +	lcdc_tcon0_mode_set(lcdc, &timing, clk_div, for_ext_vga_dac,
> > +			    sunxi_display.depth, CONFIG_VIDEO_LCD_DCLK_PHASE);
> 
> I'm not sure what this sunxi_ctfb_mode_to_display_timing function is
> useful for, but it's introduction and the conversion of the users
> would probably need to be moved to another patch.
> 

I forgot to explain this in commit message.

The thing is that current video display driver for Allwinner SoCs uses older 
framework called cfb console. This framework uses videomodes.h for timing 
related functions. In it, there is a structure called "struct ctfb_res_modes", 
which holds pixel clock, active resolution, sync times, all front/back porch 
values and so on. In contrast, DM video framework uses different structure 
"struct display_timing" which is defined in fdtdec.h and holds exactly the same 
timing informations. It is a bit strange to have two different structures for 
same type of informations, but at least conversion from ctfb timing to display 
timming is pretty straightforward, as you can see from the code.

It made more sense to me to use DM video timing structure because I expect 
that all new drivers will use this framework and I guess that older will be 
converted to use this framework too.

Should I move this change in new patch?

> >  #elif defined CONFIG_VIDEO_VGA_VIA_LCD
> >  
> >  		sunxi_composer_mode_set(mode, address);
> >  		sunxi_lcdc_tcon0_mode_set(mode, true);
> > 
> > -		sunxi_composer_enable();
> > -		sunxi_lcdc_enable();
> > +		lcdc_enable((struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE,
> > +			    sunxi_display.depth);
> > +		lcdc_enable((struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE, 0);
> 
> That one is suspicious. Shouldn't sunxi_composer_enable be left, and
> lcdc_enable called only once?

Uh, missed that. Probably fixup error. It will be fixed in next version.

Best regards,
Jernej Škrabec

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