在 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
