[U-Boot] [PATCH v4 3/9] rockchip: video: refactor rk_vop and split RK3328-specific code off

Eric eric.gao at rock-chips.com
Fri Jun 2 03:10:53 UTC 2017


headline  is wrong. it  should be  "rockchip: video: refactor rk_vop and 
split RK3288-specific code off


On 2017年05月31日 23:59, Philipp Tomsich wrote:
> To prepare for adding the RK3399 VOP driver (which shares most of its
> registers and config logic with the RK3228 VOP), this change refactors
> the driver and splits the RK3288-specific driver off.
>
> The changes in detail are:
> - introduces a data-structure for chip-specific drivers to register
>    features/callbacks with the common driver: at this time, this is
>    limited to a callback for setting the pin polarities (between the
>    VOP and the encoder modules) and a flag to signal 10bit RGB
>    capability
> - refactors the probing of regulators into a helper function that
>    can take a list of regulator names to probe and autoset
> - moves the priv data-structure into a (common) header file to be
>    used by the chip-specific drivers to provide base addresses to
>    the common driver
> - uses a callback into the chip-specific driver to set pin polarities
>    (replacing the direct register accesses previously used)
> - splits enabling the output (towards an encoder) into a separate
>    help function withint the common driver
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>
> ---
>
> Changes in v4:
> - added to break down into smaller changes
>
> Changes in v3: None
> Changes in v2: None
>
>   arch/arm/include/asm/arch-rockchip/vop_rk3288.h |   1 +
>   drivers/video/rockchip/Makefile                 |   1 +
>   drivers/video/rockchip/rk3288_vop.c             |  95 +++++++++++++
>   drivers/video/rockchip/rk_vop.c                 | 182 ++++++++++++------------
>   drivers/video/rockchip/rk_vop.h                 |  32 +++++
>   5 files changed, 217 insertions(+), 94 deletions(-)
>   create mode 100644 drivers/video/rockchip/rk3288_vop.c
>   create mode 100644 drivers/video/rockchip/rk_vop.h
>
> diff --git a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
> index d5599ec..049f192 100644
> --- a/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
> +++ b/arch/arm/include/asm/arch-rockchip/vop_rk3288.h
> @@ -197,6 +197,7 @@ enum vop_modes {
>   #define V_DSP_DEN_POL(x)               (((x) & 1) << 6)
>   #define V_DSP_VSYNC_POL(x)             (((x) & 1) << 5)
>   #define V_DSP_HSYNC_POL(x)             (((x) & 1) << 4)
> +#define V_DSP_PIN_POL(x)               (((x) & 0xf) << 4)
>   #define V_DSP_OUT_MODE(x)              ((x) & 0xf)
>   
>   /* VOP_DSP_CTRL1 */
> diff --git a/drivers/video/rockchip/Makefile b/drivers/video/rockchip/Makefile
> index cd54b12..9477ad0 100644
> --- a/drivers/video/rockchip/Makefile
> +++ b/drivers/video/rockchip/Makefile
> @@ -7,6 +7,7 @@
>   
>   ifdef CONFIG_VIDEO_ROCKCHIP
>   obj-y += rk_vop.o
> +obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288_vop.o
>   obj-$(CONFIG_DISPLAY_ROCKCHIP_EDP) += rk_edp.o
>   obj-$(CONFIG_DISPLAY_ROCKCHIP_LVDS) += rk_lvds.o
>   obj-$(CONFIG_DISPLAY_ROCKCHIP_HDMI) += rk_hdmi.o
> diff --git a/drivers/video/rockchip/rk3288_vop.c b/drivers/video/rockchip/rk3288_vop.c
> new file mode 100644
> index 0000000..e3e1ec7
> --- /dev/null
> +++ b/drivers/video/rockchip/rk3288_vop.c
> @@ -0,0 +1,95 @@
> +/*
> + * Copyright (c) 2017 Theobroma Systems Design und Consulting GmbH
> + * Copyright (c) 2015 Google, Inc
> + * Copyright 2014 Rockchip Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <display.h>
> +#include <dm.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <video.h>
> +#include <asm/hardware.h>
> +#include <asm/io.h>
> +#include <asm/arch/clock.h>
> +#include <asm/arch/grf_rk3288.h>
> +#include "rk_vop.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void rk3288_set_pin_polarity(struct udevice *dev,
> +				    enum vop_modes mode, u32 polarity)
> +{
> +	struct rk_vop_priv *priv = dev_get_priv(dev);
> +	struct rk3288_vop *regs = priv->regs;
> +
> +	/* The RK3328 VOP (v3.1) has its polarity configuration in ctrl0 */
> +	clrsetbits_le32(&regs->dsp_ctrl0,
> +			M_DSP_DCLK_POL | M_DSP_DEN_POL |
> +			M_DSP_VSYNC_POL | M_DSP_HSYNC_POL,
> +			V_DSP_PIN_POL(polarity));
> +}
> +
> +static void rk3288_set_io_vsel(struct udevice *dev)
> +{
> +	struct rk3288_grf *grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +
> +	/* lcdc(vop) iodomain select 1.8V */
> +	rk_setreg(&grf->io_vsel, 1 << 0);
> +}
> +
> +/*
> + * Try some common regulators. We should really get these from the
> + * device tree somehow.
> + */
> +static const char * const rk3288_regulator_names[] = {
> +	"vcc18_lcd",
> +	"VCC18_LCD",
> +	"vdd10_lcd_pwren_h",
> +	"vdd10_lcd",
> +	"VDD10_LCD",
> +	"vcc33_lcd"
> +};
> +
> +static int rk3288_vop_probe(struct udevice *dev)
> +{
> +	/* Before relocation we don't need to do anything */
> +	if (!(gd->flags & GD_FLG_RELOC))
> +		return 0;
> +
> +	/* Set the LCDC(vop) iodomain to 1.8V */
> +	rk3288_set_io_vsel(dev);
> +
> +	/* Probe regulators required for the RK3288 VOP */
> +	rk_vop_probe_regulators(dev, rk3288_regulator_names,
> +				ARRAY_SIZE(rk3288_regulator_names));
> +
> +	return rk_vop_probe(dev);
> +}
> +
> +struct rkvop_driverdata rk3288_driverdata = {
> +	.features = VOP_FEATURE_OUTPUT_10BIT,
> +	.set_pin_polarity = rk3288_set_pin_polarity,
> +};
> +
> +static const struct udevice_id rk3288_vop_ids[] = {
> +	{ .compatible = "rockchip,rk3288-vop",
> +	  .data = (ulong)&rk3288_driverdata },
> +	{ }
> +};
> +
> +static const struct video_ops rk3288_vop_ops = {
> +};
> +
> +U_BOOT_DRIVER(rk_vop) = {
> +	.name	= "rk3288_vop",
> +	.id	= UCLASS_VIDEO,
> +	.of_match = rk3288_vop_ids,
> +	.ops	= &rk3288_vop_ops,
> +	.bind	= rk_vop_bind,
> +	.probe	= rk3288_vop_probe,
> +	.priv_auto_alloc_size	= sizeof(struct rk_vop_priv),
> +};
> diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c
> index 16d1484..c15f93d 100644
> --- a/drivers/video/rockchip/rk_vop.c
> +++ b/drivers/video/rockchip/rk_vop.c
> @@ -17,24 +17,25 @@
>   #include <asm/hardware.h>
>   #include <asm/io.h>
>   #include <asm/arch/clock.h>
> -#include <asm/arch/cru_rk3288.h>
> -#include <asm/arch/grf_rk3288.h>
>   #include <asm/arch/edp_rk3288.h>
>   #include <asm/arch/vop_rk3288.h>
>   #include <dm/device-internal.h>
>   #include <dm/uclass-internal.h>
> -#include <dt-bindings/clock/rk3288-cru.h>
>   #include <power/regulator.h>
> +#include "rk_vop.h"
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> -struct rk_vop_priv {
> -	struct rk3288_vop *regs;
> -	struct rk3288_grf *grf;
> +enum vop_pol {
> +	HSYNC_POSITIVE = 0,
> +	VSYNC_POSITIVE = 1,
> +	DEN_NEGATIVE   = 2,
> +	DCLK_INVERT    = 3
>   };
>   
> -void rkvop_enable(struct rk3288_vop *regs, ulong fbbase,
> -		  int fb_bits_per_pixel, const struct display_timing *edid)
> +static void rkvop_enable(struct rk3288_vop *regs, ulong fbbase,
> +			 int fb_bits_per_pixel,
> +			 const struct display_timing *edid)
>   {
>   	u32 lb_mode;
>   	u32 rgb_mode;
> @@ -89,54 +90,83 @@ void rkvop_enable(struct rk3288_vop *regs, ulong fbbase,
>   	writel(0x01, &regs->reg_cfg_done); /* enable reg config */
>   }
>   
> -void rkvop_mode_set(struct rk3288_vop *regs,
> -		    const struct display_timing *edid, enum vop_modes mode)
> +static void rkvop_set_pin_polarity(struct udevice *dev,
> +				   enum vop_modes mode, u32 polarity)
>   {
> -	u32 hactive = edid->hactive.typ;
> -	u32 vactive = edid->vactive.typ;
> -	u32 hsync_len = edid->hsync_len.typ;
> -	u32 hback_porch = edid->hback_porch.typ;
> -	u32 vsync_len = edid->vsync_len.typ;
> -	u32 vback_porch = edid->vback_porch.typ;
> -	u32 hfront_porch = edid->hfront_porch.typ;
> -	u32 vfront_porch = edid->vfront_porch.typ;
> -	uint flags;
> -	int mode_flags;
> +	struct rkvop_driverdata *ops =
> +		(struct rkvop_driverdata *)dev_get_driver_data(dev);
> +
> +	if (ops->set_pin_polarity)
> +		ops->set_pin_polarity(dev, mode, polarity);
> +}
> +
> +static void rkvop_enable_output(struct udevice *dev, enum vop_modes mode)
> +{
> +	struct rk_vop_priv *priv = dev_get_priv(dev);
> +	struct rk3288_vop *regs = priv->regs;
>   
>   	switch (mode) {
>   	case VOP_MODE_HDMI:
>   		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
>   				V_HDMI_OUT_EN(1));
>   		break;
> +
>   	case VOP_MODE_EDP:
> -	default:
>   		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
>   				V_EDP_OUT_EN(1));
>   		break;
> +
>   	case VOP_MODE_LVDS:
>   		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
>   				V_RGB_OUT_EN(1));
>   		break;
> +
>   	case VOP_MODE_MIPI:
>   		clrsetbits_le32(&regs->sys_ctrl, M_ALL_OUT_EN,
>   				V_MIPI_OUT_EN(1));
> -		 break;
> +		break;
> +
> +	default:
> +		debug("%s: unsupported output mode %x\n", __func__, mode);
>   	}
> +}
>   
> -	if (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP)
> -		/* RGBaaa */
> -		mode_flags = 15;
> -	else
> -		/* RGB888 */
> -		mode_flags = 0;
> +static void rkvop_mode_set(struct udevice *dev,
> +			   const struct display_timing *edid,
> +			   enum vop_modes mode)
> +{
> +	struct rk_vop_priv *priv = dev_get_priv(dev);
> +	struct rk3288_vop *regs = priv->regs;
> +	struct rkvop_driverdata *data =
> +		(struct rkvop_driverdata *)dev_get_driver_data(dev);
>   
> -	flags = V_DSP_OUT_MODE(mode_flags) |
> -		V_DSP_HSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)) |
> -		V_DSP_VSYNC_POL(!!(edid->flags & DISPLAY_FLAGS_VSYNC_HIGH));
> +	u32 hactive = edid->hactive.typ;
> +	u32 vactive = edid->vactive.typ;
> +	u32 hsync_len = edid->hsync_len.typ;
> +	u32 hback_porch = edid->hback_porch.typ;
> +	u32 vsync_len = edid->vsync_len.typ;
> +	u32 vback_porch = edid->vback_porch.typ;
> +	u32 hfront_porch = edid->hfront_porch.typ;
> +	u32 vfront_porch = edid->vfront_porch.typ;
> +	int mode_flags;
> +	u32 pin_polarity;
> +
> +	pin_polarity = BIT(DCLK_INVERT);
> +	if (edid->flags & DISPLAY_FLAGS_HSYNC_HIGH)
> +		pin_polarity |= BIT(HSYNC_POSITIVE);
> +	if (edid->flags & DISPLAY_FLAGS_VSYNC_HIGH)
> +		pin_polarity |= BIT(VSYNC_POSITIVE);
> +
> +	rkvop_set_pin_polarity(dev, mode, pin_polarity);
> +	rkvop_enable_output(dev, mode);
>   
> -	clrsetbits_le32(&regs->dsp_ctrl0,
> -			M_DSP_OUT_MODE | M_DSP_VSYNC_POL | M_DSP_HSYNC_POL,
> -			flags);
> +	mode_flags = 0;  /* RGB888 */
> +	if ((data->features & VOP_FEATURE_OUTPUT_10BIT) &&
> +	    (mode == VOP_MODE_HDMI || mode == VOP_MODE_EDP))
> +		mode_flags = 15;  /* RGBaaa */
> +
> +	clrsetbits_le32(&regs->dsp_ctrl0, M_DSP_OUT_MODE,
> +			V_DSP_OUT_MODE(mode_flags));
>   
>   	writel(V_HSYNC(hsync_len) |
>   	       V_HORPRD(hsync_len + hback_porch + hactive + hfront_porch),
> @@ -185,7 +215,7 @@ void rkvop_mode_set(struct rk3288_vop *regs,
>    *		node within the VOP's 'port' list.
>    * @return 0 if OK, -ve if something went wrong
>    */
> -int rk_display_init(struct udevice *dev, ulong fbbase, int ep_node)
> +static int rk_display_init(struct udevice *dev, ulong fbbase, int ep_node)
>   {
>   	struct video_priv *uc_priv = dev_get_uclass_priv(dev);
>   	const void *blob = gd->fdt_blob;
> @@ -255,18 +285,18 @@ int rk_display_init(struct udevice *dev, ulong fbbase, int ep_node)
>   	/* Set bitwidth for vop display according to vop mode */
>   	switch (vop_id) {
>   	case VOP_MODE_EDP:
> -	case VOP_MODE_HDMI:
>   	case VOP_MODE_LVDS:
>   		l2bpp = VIDEO_BPP16;
>   		break;
> +	case VOP_MODE_HDMI:
>   	case VOP_MODE_MIPI:
>   		l2bpp = VIDEO_BPP32;
>   		break;
>   	default:
>   		l2bpp = VIDEO_BPP16;
>   	}
> -	rkvop_mode_set(regs, &timing, vop_id);
>   
> +	rkvop_mode_set(dev, &timing, vop_id);
>   	rkvop_enable(regs, fbbase, 1 << l2bpp, &timing);
>   
>   	ret = display_enable(disp, 1 << l2bpp, &timing);
> @@ -281,53 +311,37 @@ int rk_display_init(struct udevice *dev, ulong fbbase, int ep_node)
>   	return 0;
>   }
>   
> -static int rk_vop_probe(struct udevice *dev)
> +void rk_vop_probe_regulators(struct udevice *dev,
> +			     const char * const *names, int cnt)
> +{
> +	int i, ret;
> +	const char *name;
> +	struct udevice *reg;
> +
> +	for (i = 0; i < cnt; ++i) {
> +		name = names[i];
> +		debug("%s: probing regulator '%s'\n", dev->name, name);
> +
> +		ret = regulator_autoset_by_name(name, &reg);
> +		if (!ret)
> +			ret = regulator_set_enable(reg, true);
> +	}
> +}
> +
> +int rk_vop_probe(struct udevice *dev)
>   {
>   	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
>   	const void *blob = gd->fdt_blob;
>   	struct rk_vop_priv *priv = dev_get_priv(dev);
> -	struct udevice *reg;
> -	int ret, port, node;
> +	int ret = 0;
> +	int port, node;
>   
>   	/* Before relocation we don't need to do anything */
>   	if (!(gd->flags & GD_FLG_RELOC))
>   		return 0;
>   
> -	priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
>   	priv->regs = (struct rk3288_vop *)dev_get_addr(dev);
>   
> -	/* lcdc(vop) iodomain select 1.8V */
> -	rk_setreg(&priv->grf->io_vsel, 1 << 0);
> -
> -	/*
> -	 * Try some common regulators. We should really get these from the
> -	 * device tree somehow.
> -	 */
> -	ret = regulator_autoset_by_name("vcc18_lcd", &reg);
> -	if (ret)
> -		debug("%s: Cannot autoset regulator vcc18_lcd\n", __func__);
> -	ret = regulator_autoset_by_name("VCC18_LCD", &reg);
> -	if (ret)
> -		debug("%s: Cannot autoset regulator VCC18_LCD\n", __func__);
> -	ret = regulator_autoset_by_name("vdd10_lcd_pwren_h", &reg);
> -	if (ret) {
> -		debug("%s: Cannot autoset regulator vdd10_lcd_pwren_h\n",
> -		      __func__);
> -	}
> -	ret = regulator_autoset_by_name("vdd10_lcd", &reg);
> -	if (ret) {
> -		debug("%s: Cannot autoset regulator vdd10_lcd\n",
> -		      __func__);
> -	}
> -	ret = regulator_autoset_by_name("VDD10_LCD", &reg);
> -	if (ret) {
> -		debug("%s: Cannot autoset regulator VDD10_LCD\n",
> -		      __func__);
> -	}
> -	ret = regulator_autoset_by_name("vcc33_lcd", &reg);
> -	if (ret)
> -		debug("%s: Cannot autoset regulator vcc33_lcd\n", __func__);
> -
>   	/*
>   	 * Try all the ports until we find one that works. In practice this
>   	 * tries EDP first if available, then HDMI.
> @@ -353,7 +367,7 @@ static int rk_vop_probe(struct udevice *dev)
>   	return ret;
>   }
>   
> -static int rk_vop_bind(struct udevice *dev)
> +int rk_vop_bind(struct udevice *dev)
>   {
>   	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
>   
> @@ -362,23 +376,3 @@ static int rk_vop_bind(struct udevice *dev)
>   
>   	return 0;
>   }
> -
> -static const struct video_ops rk_vop_ops = {
> -};
> -
> -static const struct udevice_id rk_vop_ids[] = {
> -	{ .compatible = "rockchip,rk3399-vop-big" },
> -	{ .compatible = "rockchip,rk3399-vop-lit" },
> -	{ .compatible = "rockchip,rk3288-vop" },
> -	{ }
> -};
> -
> -U_BOOT_DRIVER(rk_vop) = {
> -	.name	= "rk_vop",
> -	.id	= UCLASS_VIDEO,
> -	.of_match = rk_vop_ids,
> -	.ops	= &rk_vop_ops,
> -	.bind	= rk_vop_bind,
> -	.probe	= rk_vop_probe,
> -	.priv_auto_alloc_size	= sizeof(struct rk_vop_priv),
> -};
> diff --git a/drivers/video/rockchip/rk_vop.h b/drivers/video/rockchip/rk_vop.h
> new file mode 100644
> index 0000000..f65ac17
> --- /dev/null
> +++ b/drivers/video/rockchip/rk_vop.h
> @@ -0,0 +1,32 @@
> +/*
> + * Copyright (c) 2017 Theobroma Systems Design und Consulting GmbH
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __RK_VOP_H__
> +#define __RK_VOP_H__
> +
> +#include <asm/arch/vop_rk3288.h>
> +
> +struct rk_vop_priv {
> +	void *regs;
> +};
> +
> +enum vop_features {
> +	VOP_FEATURE_OUTPUT_10BIT = (1 << 0),
> +};
> +
> +struct rkvop_driverdata {
> +	/* configuration */
> +	u32 features;
> +	/* block-specific setters/getters */
> +	void (*set_pin_polarity)(struct udevice *, enum vop_modes, u32);
> +};
> +
> +int rk_vop_probe(struct udevice *dev);
> +int rk_vop_bind(struct udevice *dev);
> +void rk_vop_probe_regulators(struct udevice *dev,
> +			     const char * const *names, int cnt);
> +
> +#endif



More information about the U-Boot mailing list