[U-Boot] [RESEND PATCH 5/5] sunxi: video: add LCD support to DE2 driver

[icenowy at aosc.io]

在 2017-09-21 14:51，Maxime Ripard 写道：
> On Thu, Sep 21, 2017 at 05:33:36AM +0000, Vasily Khoruzhick wrote:
>> >>> +     ret = uclass_find_device_by_name(UCLASS_DISPLAY,
>> >>> +                                      "sunxi_lcd", &disp);
>> >>> +     if (!ret) {
>> >>> +             int mux;
>> >>> +
>> >>> +             mux = 0;
>> >>> +
>> >>> +             ret = sunxi_de2_init(dev, plat->base, VIDEO_BPP32, disp, mux,
>> >>> +                                  false);
>> >>> +             if (!ret) {
>> >>> +                     video_set_flush_dcache(dev, 1);
>> >>
>> >> Why do you need to flush the dcache here?
>> >
>> > Copied from HDMI driver init. If it's not necessary why it's here for HDMI?
>> 
>> DE2 is not cache aware, so CPU should flush dcache after updating
>> framebuffer. If I remove this line, dcache isn't flushed when
>> framebuffer is updated, and thus picture is a total mess (black
>> background with some white stripes).
> 
> Ah, so the framebuffer is mapped cacheable. Ok, that's unusual, but
> that works. Can you put that in a comment?

I think currently in U-Boot all DRAM is mapped cacheable.

> 
>> >>> +static int sunxi_lcd_enable(struct udevice *dev, int bpp,
>> >>> +                           const struct display_timing *edid)
>> >>> +{
>> >>> +     struct sunxi_ccm_reg * const ccm =
>> >>> +            (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
>> >>> +     struct sunxi_lcdc_reg * const lcdc =
>> >>> +            (struct sunxi_lcdc_reg *)SUNXI_LCD0_BASE;
>> >>> +     struct sunxi_lcd_priv *priv = dev_get_priv(dev);
>> >>> +     struct udevice *backlight;
>> >>> +     int clk_div, clk_double, ret;
>> >>> +
>> >>> +     /* Reset off */
>> >>> +     setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_LCD0);
>> >>> +
>> >>> +     /* Clock on */
>> >>> +     setbits_le32(&ccm->ahb_gate1, 1 << AHB_GATE_OFFSET_LCD0);
>> >>
>> >> This has nothing to do with using a panel or not, it should be in
>> >> lcdc_init().
>> >
>> > Why? We don't need neither take it out of reset nor turn the clock on
>> > it if LCD is not used (e.g. HDMI-only case).
>> 
>> I'm leaving it here. It's not necessary for HDMI, and it doesn't work
>> without it for LCD.
> 
> I'm pretty sure it still needs to be done for HDMI. LCD0 is the TCON,
> and the TCON is used for HDMI too.

All TCONs come with DE2 are single-channel, either channel0 or channel1.

On A64 TCON0 has channel0 and TCON1 has channel1.

> 
>> >>> +     lcdc_init(lcdc);
>> >>> +     sunxi_lcdc_config_pinmux();
>> >>
>> >> This is already handled in sunxi_lcdc_tcon0_mode_set, why duplicate
>> >> it?
>> >
>> > Because the one that sunxi_lcdc_tcon0_mode_set() calls is
>> > DE1-specific. I don't want to split out that code that won't be used
>> > by DE2 driver.
> 
> Then move out the common code. It's kind of weird though, since the
> DE1 vs DE2 stuff is basically only for the layers part. The TCON is
> always there, and is mostly the same. So you should be able to re-use
> that with minor modifications.
> 
> Maxime