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

Icenowy Zheng icenowy at aosc.io
Tue Sep 19 23:26:54 UTC 2017



于 2017年9月20日 GMT+08:00 上午3:00:30, Vasily Khoruzhick <anarsoul at gmail.com> 写到:
>On Tue, Sep 19, 2017 at 1:33 AM, Maxime Ripard
><maxime.ripard at free-electrons.com> wrote:
>> On Mon, Sep 18, 2017 at 10:04:21PM -0700, Vasily Khoruzhick wrote:
>>> Extend DE2 driver with LCD support
>>
>> (All) your commit messages could use a bit more details.
>
>OK, will add in v2.
>
>> Here, for example, explaining the following things would help:
>>   - Why are you creating yet another file
>
>Are you talking about any specific file? I guess adding another driver
>justifies creation of another file.
>
>>   - What is the situation with old Allwinner SoCs that should share
>>     the same code
>
>As far as I can tell, DE2 is present in H3, V3s, A64 and newer. LCD is
>supported in A64 only. I.e. hardware is not present
>in H3 or V3s

The only display output of V3s is LCD.

>
>>   - What are the expected users
>
>Pinebook
>
>>   - Which SoC / board have you tested it on
>
>A64 / Pinebook
>
>>
>> etc...
>>
>>> Signed-off-by: Vasily Khoruzhick <anarsoul at gmail.com>
>>> ---
>>>  arch/arm/mach-sunxi/Kconfig     |   2 +-
>>>  drivers/video/sunxi/Makefile    |   2 +-
>>>  drivers/video/sunxi/sunxi_de2.c |  17 +++++
>>>  drivers/video/sunxi/sunxi_lcd.c | 142
>++++++++++++++++++++++++++++++++++++++++
>>>  4 files changed, 161 insertions(+), 2 deletions(-)
>>>  create mode 100644 drivers/video/sunxi/sunxi_lcd.c
>>>
>>> diff --git a/arch/arm/mach-sunxi/Kconfig
>b/arch/arm/mach-sunxi/Kconfig
>>> index 2309f59999..06d697e3a7 100644
>>> --- a/arch/arm/mach-sunxi/Kconfig
>>> +++ b/arch/arm/mach-sunxi/Kconfig
>>> @@ -680,7 +680,7 @@ config VIDEO_LCD_MODE
>>>
>>>  config VIDEO_LCD_DCLK_PHASE
>>>       int "LCD panel display clock phase"
>>> -     depends on VIDEO
>>> +     depends on VIDEO || DM_VIDEO
>>>       default 1
>>>       ---help---
>>>       Select LCD panel display clock phase shift, range 0-3.
>>> diff --git a/drivers/video/sunxi/Makefile
>b/drivers/video/sunxi/Makefile
>>> index 0d64c2021f..8c91766c24 100644
>>> --- a/drivers/video/sunxi/Makefile
>>> +++ b/drivers/video/sunxi/Makefile
>>> @@ -6,4 +6,4 @@
>>>  #
>>>
>>>  obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o lcdc.o tve_common.o
>../videomodes.o
>>> -obj-$(CONFIG_VIDEO_DE2) += sunxi_de2.o sunxi_dw_hdmi.o lcdc.o
>../dw_hdmi.o
>>> +obj-$(CONFIG_VIDEO_DE2) += sunxi_de2.o sunxi_dw_hdmi.o lcdc.o
>../dw_hdmi.o sunxi_lcd.o
>>> diff --git a/drivers/video/sunxi/sunxi_de2.c
>b/drivers/video/sunxi/sunxi_de2.c
>>> index ee67764ac5..a838bbacd1 100644
>>> --- a/drivers/video/sunxi/sunxi_de2.c
>>> +++ b/drivers/video/sunxi/sunxi_de2.c
>>> @@ -232,6 +232,23 @@ static int sunxi_de2_probe(struct udevice *dev)
>>>       if (!(gd->flags & GD_FLG_RELOC))
>>>               return 0;
>>>
>>> +     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?
>
>>
>>> +                     return 0;
>>> +             }
>>> +     }
>>> +
>>> +     debug("%s: lcd display not found (ret=%d)\n", __func__, ret);
>>> +
>>>       ret = uclass_find_device_by_name(UCLASS_DISPLAY,
>>>                                        "sunxi_dw_hdmi", &disp);
>>>       if (!ret) {
>>> diff --git a/drivers/video/sunxi/sunxi_lcd.c
>b/drivers/video/sunxi/sunxi_lcd.c
>>> new file mode 100644
>>> index 0000000000..154eb5835e
>>> --- /dev/null
>>> +++ b/drivers/video/sunxi/sunxi_lcd.c
>>> @@ -0,0 +1,142 @@
>>> +/*
>>> + * Allwinner LCD driver
>>> + *
>>> + * (C) Copyright 2017 Vasily Khoruzhick <anarsoul at gmail.com>
>>> + *
>>> + * SPDX-License-Identifier:  GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <display.h>
>>> +#include <video_bridge.h>
>>> +#include <backlight.h>
>>> +#include <dm.h>
>>> +#include <edid.h>
>>> +#include <asm/io.h>
>>> +#include <asm/arch/clock.h>
>>> +#include <asm/arch/lcdc.h>
>>> +#include <asm/arch/gpio.h>
>>> +#include <asm/gpio.h>
>>> +
>>> +struct sunxi_lcd_priv {
>>> +     struct display_timing timing;
>>> +     int panel_bpp;
>>> +};
>>> +
>>> +static void sunxi_lcdc_config_pinmux(void)
>>> +{
>>> +     int pin;
>>> +     for (pin = SUNXI_GPD(0); pin <= SUNXI_GPD(21); pin++) {
>>> +             sunxi_gpio_set_cfgpin(pin, SUNXI_GPD_LCD0);
>>> +             sunxi_gpio_set_drv(pin, 3);
>>> +     }
>>> +}
>>> +
>>> +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).
>
>>> +     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.
>
>>> +     lcdc_pll_set(ccm, 0, edid->pixelclock.typ / 1000,
>>> +                  &clk_div, &clk_double);
>>> +     lcdc_tcon0_mode_set(lcdc, edid, clk_div, false,
>>> +                         priv->panel_bpp,
>CONFIG_VIDEO_LCD_DCLK_PHASE);
>>> +     lcdc_enable(lcdc, priv->panel_bpp);
>>> +
>>> +     ret = uclass_get_device(UCLASS_PANEL_BACKLIGHT, 0,
>&backlight);
>>> +     if (!ret)
>>> +             backlight_enable(backlight);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int sunxi_lcd_read_timing(struct udevice *dev,
>>> +                                struct display_timing *timing)
>>> +{
>>> +     struct sunxi_lcd_priv *priv = dev_get_priv(dev);
>>> +     memcpy(timing, &priv->timing, sizeof(struct display_timing));
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int sunxi_lcd_probe(struct udevice *dev)
>>> +{
>>> +     struct udevice *cdev;
>>> +     struct sunxi_lcd_priv *priv = dev_get_priv(dev);
>>> +     int ret;
>>> +
>>> +     /* make sure that clock is active */
>>> +     clock_set_pll10(432000000);
>>
>> Why do you need it active, and why at that rate?
>
>Will check.
>
>>> +
>>> +#ifdef CONFIG_VIDEO_BRIDGE
>>> +     /* Try to get timings from bridge first */
>>> +     ret = uclass_get_device(UCLASS_VIDEO_BRIDGE, 0, &cdev);
>>> +     if (!ret) {
>>> +             u8 edid[EDID_SIZE];
>>> +             int channel_bpp;
>>> +
>>> +             ret = video_bridge_attach(cdev);
>>> +             if (ret) {
>>> +                     debug("video bridge attach failed: %d\n",
>ret);
>>> +                     return ret;
>>> +             }
>>> +             ret = video_bridge_read_edid(cdev, edid, EDID_SIZE);
>>> +             if (ret <= 0) {
>>> +                     debug("video bridge failed to read edid:
>%d\n", ret);
>>> +                     return ret ? ret : -EIO;
>>> +             }
>>> +             ret = edid_get_timing(edid, ret, &priv->timing,
>&channel_bpp);
>>> +             priv->panel_bpp = channel_bpp * 3;
>>> +             return ret;
>>> +     }
>>> +     debug("video bridge not found: %d\n", ret);
>>> +#endif
>>> +     /* Fallback to timings from DT if there's no bridge */
>>> +     ret = uclass_get_device(UCLASS_PANEL, 0, &cdev);
>>> +     if (ret) {
>>> +             debug("video panel not found: %d\n", ret);
>>> +             return ret;
>>> +     }
>>> +
>>> +     if (fdtdec_decode_display_timing(gd->fdt_blob,
>dev_of_offset(cdev),
>>> +                                      0, &priv->timing)) {
>>> +             debug("%s: Failed to decode display timing\n",
>__func__);
>>> +             return -EINVAL;
>>> +     }
>>
>> How does that work with simple-panel style bindings that are most of
>> the panels merged these days?
>
>It should just work, but I haven't even tested this part of the code -
>I don't have a panel with parallel interface nor A64 board to connect
>it to.
>Do you want me to drop it and make this driver dependent on
>CONFIG_VIDEO_BRIDGE
>
>>
>>> +     priv->panel_bpp = 16;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static const struct dm_display_ops sunxi_lcd_ops = {
>>> +       .read_timing = sunxi_lcd_read_timing,
>>> +       .enable = sunxi_lcd_enable,
>>> +};
>>> +
>>> +U_BOOT_DRIVER(sunxi_lcd) = {
>>> +     .name   = "sunxi_lcd",
>>> +     .id     = UCLASS_DISPLAY,
>>> +     .ops    = &sunxi_lcd_ops,
>>> +     .probe  = sunxi_lcd_probe,
>>> +     .priv_auto_alloc_size = sizeof(struct sunxi_lcd_priv),
>>> +};
>>> +
>>> +#ifdef CONFIG_MACH_SUN50I
>>
>> Why do you restrict it to the A64 here?
>
>Because the hardware is present in A64 only.
>
>> Thanks,
>> Maxime
>>
>> --
>> Maxime Ripard, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com


More information about the U-Boot mailing list