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

Maxime Ripard maxime.ripard at free-electrons.com
Tue Sep 19 08:33:38 UTC 2017


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.

Here, for example, explaining the following things would help:
  - Why are you creating yet another file
  - What is the situation with old Allwinner SoCs that should share
    the same code
  - What are the expected users
  - Which SoC / board have you tested it on

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?

> +			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().

> +	lcdc_init(lcdc);
> +	sunxi_lcdc_config_pinmux();

This is already handled in sunxi_lcdc_tcon0_mode_set, why duplicate
it?

> +	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?

> +
> +#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?

> +	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?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170919/48f31b41/attachment.sig>


More information about the U-Boot mailing list