[v2,4/4] drm: zte: add VGA driver support
diff mbox

Message ID 1491490870-6330-5-git-send-email-shawnguo@kernel.org
State New
Headers show

Commit Message

Shawn Guo April 6, 2017, 3:01 p.m. UTC
From: Shawn Guo <shawn.guo@linaro.org>

It adds VGA driver support, which needs to configure corresponding VOU
interface in RGB_888 format, and thus the following changes are needed
on zx_vou.

- Rename the CSC block of Graphic Layer a bit to make it more specific,
  and add CSC of Channel to support RGB output.
- Bypass Dither block for RGB output.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/zte/Makefile      |   1 +
 drivers/gpu/drm/zte/zx_drm_drv.c  |   1 +
 drivers/gpu/drm/zte/zx_drm_drv.h  |   1 +
 drivers/gpu/drm/zte/zx_vga.c      | 530 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/zte/zx_vga_regs.h |  36 +++
 drivers/gpu/drm/zte/zx_vou.c      |  33 ++-
 drivers/gpu/drm/zte/zx_vou_regs.h |  12 +-
 7 files changed, 610 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/zte/zx_vga.c
 create mode 100644 drivers/gpu/drm/zte/zx_vga_regs.h

Comments

Sean Paul April 6, 2017, 5:12 p.m. UTC | #1
On Thu, Apr 06, 2017 at 11:01:10PM +0800, Shawn Guo wrote:
> From: Shawn Guo <shawn.guo@linaro.org>
> 
> It adds VGA driver support, which needs to configure corresponding VOU
> interface in RGB_888 format, and thus the following changes are needed
> on zx_vou.
> 
> - Rename the CSC block of Graphic Layer a bit to make it more specific,
>   and add CSC of Channel to support RGB output.
> - Bypass Dither block for RGB output.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/zte/Makefile      |   1 +
>  drivers/gpu/drm/zte/zx_drm_drv.c  |   1 +
>  drivers/gpu/drm/zte/zx_drm_drv.h  |   1 +
>  drivers/gpu/drm/zte/zx_vga.c      | 530 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/zte/zx_vga_regs.h |  36 +++
>  drivers/gpu/drm/zte/zx_vou.c      |  33 ++-
>  drivers/gpu/drm/zte/zx_vou_regs.h |  12 +-
>  7 files changed, 610 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/zte/zx_vga.c
>  create mode 100644 drivers/gpu/drm/zte/zx_vga_regs.h
> 
> diff --git a/drivers/gpu/drm/zte/Makefile b/drivers/gpu/drm/zte/Makefile
> index 01352b56c418..9df7766a7f9d 100644
> --- a/drivers/gpu/drm/zte/Makefile
> +++ b/drivers/gpu/drm/zte/Makefile
> @@ -3,6 +3,7 @@ zxdrm-y := \
>  	zx_hdmi.o \
>  	zx_plane.o \
>  	zx_tvenc.o \
> +	zx_vga.o \
>  	zx_vou.o
>  
>  obj-$(CONFIG_DRM_ZTE) += zxdrm.o
> diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
> index 5c6944a1e72c..8a6892eeb44f 100644
> --- a/drivers/gpu/drm/zte/zx_drm_drv.c
> +++ b/drivers/gpu/drm/zte/zx_drm_drv.c
> @@ -248,6 +248,7 @@ static int zx_drm_remove(struct platform_device *pdev)
>  	&zx_crtc_driver,
>  	&zx_hdmi_driver,
>  	&zx_tvenc_driver,
> +	&zx_vga_driver,
>  	&zx_drm_platform_driver,
>  };
>  
> diff --git a/drivers/gpu/drm/zte/zx_drm_drv.h b/drivers/gpu/drm/zte/zx_drm_drv.h
> index 5ca035b079c7..2a8cdc5f8be4 100644
> --- a/drivers/gpu/drm/zte/zx_drm_drv.h
> +++ b/drivers/gpu/drm/zte/zx_drm_drv.h
> @@ -14,6 +14,7 @@
>  extern struct platform_driver zx_crtc_driver;
>  extern struct platform_driver zx_hdmi_driver;
>  extern struct platform_driver zx_tvenc_driver;
> +extern struct platform_driver zx_vga_driver;
>  
>  static inline u32 zx_readl(void __iomem *reg)
>  {
> diff --git a/drivers/gpu/drm/zte/zx_vga.c b/drivers/gpu/drm/zte/zx_vga.c
> new file mode 100644
> index 000000000000..0d850d9ea700
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_vga.c
> @@ -0,0 +1,530 @@
> +/*
> + * Copyright (C) 2017 Sanechips Technology Co., Ltd.
> + * Copyright 2017 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drmP.h>
> +
> +#include "zx_drm_drv.h"
> +#include "zx_vga_regs.h"
> +#include "zx_vou.h"
> +
> +struct zx_vga_pwrctrl {
> +	struct regmap *regmap;
> +	u32 reg;
> +	u32 mask;
> +};
> +
> +struct zx_vga_i2c {
> +	struct i2c_adapter adap;
> +	struct mutex lock;
> +};
> +
> +struct zx_vga {
> +	struct drm_connector connector;
> +	struct drm_encoder encoder;
> +	struct zx_vga_i2c *ddc;
> +	struct device *dev;
> +	void __iomem *mmio;
> +	struct clk *i2c_wclk;
> +	struct zx_vga_pwrctrl pwrctrl;
> +	spinlock_t lock;
> +	struct completion complete;
> +	bool connected;
> +};
> +
> +#define to_zx_vga(x) container_of(x, struct zx_vga, x)
> +
> +static void zx_vga_encoder_enable(struct drm_encoder *encoder)
> +{
> +	struct zx_vga *vga = to_zx_vga(encoder);
> +	struct zx_vga_pwrctrl *pwrctrl = &vga->pwrctrl;
> +
> +	/* Set bit to power up VGA DACs */
> +	regmap_update_bits(pwrctrl->regmap, pwrctrl->reg, pwrctrl->mask,
> +			   pwrctrl->mask);
> +
> +	vou_inf_enable(VOU_VGA, encoder->crtc);
> +}
> +
> +static void zx_vga_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct zx_vga *vga = to_zx_vga(encoder);
> +	struct zx_vga_pwrctrl *pwrctrl = &vga->pwrctrl;
> +
> +	vou_inf_disable(VOU_VGA, encoder->crtc);
> +
> +	/* Clear bit to power down VGA DACs */
> +	regmap_update_bits(pwrctrl->regmap, pwrctrl->reg, pwrctrl->mask, 0);
> +}
> +
> +static const struct drm_encoder_helper_funcs zx_vga_encoder_helper_funcs = {
> +	.enable	= zx_vga_encoder_enable,
> +	.disable = zx_vga_encoder_disable,
> +};
> +
> +static const struct drm_encoder_funcs zx_vga_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +static int zx_vga_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct zx_vga *vga = to_zx_vga(connector);
> +	unsigned long flags;
> +	struct edid *edid;
> +	int ret;
> +
> +	/*
> +	 * Clear both detection bits to switch I2C bus from device
> +	 * detecting to EDID reading.
> +	 */
> +	spin_lock_irqsave(&vga->lock, flags);
> +	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, 0);
> +	spin_unlock_irqrestore(&vga->lock, flags);
> +
> +	edid = drm_get_edid(connector, &vga->ddc->adap);
> +	if (!edid)
> +		return 0;
> +
> +	/*
> +	 * As edid reading succeeds, device must be connected, so we set
> +	 * up detection bit for unplug interrupt here.
> +	 */
> +	spin_lock_irqsave(&vga->lock, flags);
> +	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_HAS_DEVICE);
> +	spin_unlock_irqrestore(&vga->lock, flags);
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	ret = drm_add_edid_modes(connector, edid);
> +	kfree(edid);
> +
> +	return ret;
> +}
> +
> +static enum drm_mode_status
> +zx_vga_connector_mode_valid(struct drm_connector *connector,
> +			    struct drm_display_mode *mode)
> +{
> +	return MODE_OK;
> +}
> +
> +static struct drm_connector_helper_funcs zx_vga_connector_helper_funcs = {
> +	.get_modes = zx_vga_connector_get_modes,
> +	.mode_valid = zx_vga_connector_mode_valid,
> +};
> +
> +static enum drm_connector_status
> +zx_vga_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct zx_vga *vga = to_zx_vga(connector);
> +
> +	return vga->connected ? connector_status_connected :
> +				connector_status_disconnected;
> +}
> +
> +static const struct drm_connector_funcs zx_vga_connector_funcs = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.detect = zx_vga_connector_detect,
> +	.destroy = drm_connector_cleanup,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int zx_vga_register(struct drm_device *drm, struct zx_vga *vga)
> +{
> +	struct drm_encoder *encoder = &vga->encoder;
> +	struct drm_connector *connector = &vga->connector;
> +	struct device *dev = vga->dev;
> +	int ret;
> +
> +	encoder->possible_crtcs = VOU_CRTC_MASK;
> +
> +	ret = drm_encoder_init(drm, encoder, &zx_vga_encoder_funcs,
> +			       DRM_MODE_ENCODER_DAC, NULL);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to init encoder: %d\n", ret);
> +		return ret;
> +	};
> +
> +	drm_encoder_helper_add(encoder, &zx_vga_encoder_helper_funcs);
> +
> +	vga->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +
> +	ret = drm_connector_init(drm, connector, &zx_vga_connector_funcs,
> +				 DRM_MODE_CONNECTOR_VGA);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to init connector: %d\n", ret);
> +		goto clean_encoder;
> +	};
> +
> +	drm_connector_helper_add(connector, &zx_vga_connector_helper_funcs);
> +
> +	ret = drm_mode_connector_attach_encoder(connector, encoder);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to attach encoder: %d\n", ret);
> +		goto clean_connector;
> +	};
> +
> +	return 0;
> +
> +clean_connector:
> +	drm_connector_cleanup(connector);
> +clean_encoder:
> +	drm_encoder_cleanup(encoder);
> +	return ret;
> +}
> +
> +static int zx_vga_pwrctrl_init(struct zx_vga *vga)
> +{
> +	struct zx_vga_pwrctrl *pwrctrl = &vga->pwrctrl;
> +	struct device *dev = vga->dev;
> +	struct of_phandle_args out_args;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	ret = of_parse_phandle_with_fixed_args(dev->of_node,
> +				"zte,vga-power-control", 2, 0, &out_args);
> +	if (ret)
> +		return ret;
> +
> +	regmap = syscon_node_to_regmap(out_args.np);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		goto out;
> +	}
> +
> +	pwrctrl->regmap = regmap;
> +	pwrctrl->reg = out_args.args[0];
> +	pwrctrl->mask = out_args.args[1];
> +
> +out:
> +	of_node_put(out_args.np);
> +	return ret;
> +}
> +
> +static int zx_vga_i2c_read(struct zx_vga *vga, struct i2c_msg *msg)
> +{
> +	int len = msg->len;
> +	u8 *buf = msg->buf;
> +	u32 offset = 0;
> +	int i;
> +
> +	reinit_completion(&vga->complete);
> +
> +	/* Select combo write */
> +	zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_COMBO, VGA_CMD_COMBO);
> +	zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_RW, 0);
> +
> +	while (len > 0) {
> +		u32 cnt;
> +
> +		/* Clear RX FIFO */
> +		zx_writel_mask(vga->mmio + VGA_RXF_CTRL, VGA_RX_FIFO_CLEAR,
> +			       VGA_RX_FIFO_CLEAR);
> +
> +		/* Data offset to read from */
> +		zx_writel(vga->mmio + VGA_SUB_ADDR, offset);
> +
> +		/* Kick off the transfer */
> +		zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_TRANS,
> +			       VGA_CMD_TRANS);
> +
> +		if (!wait_for_completion_timeout(&vga->complete,
> +						 msecs_to_jiffies(1000))) {
> +			DRM_DEV_ERROR(vga->dev, "transfer timeout\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		cnt = zx_readl(vga->mmio + VGA_RXF_STATUS);
> +		cnt = (cnt & VGA_RXF_COUNT_MASK) >> VGA_RXF_COUNT_SHIFT;
> +		/* FIFO status may report more data than we need to read */
> +		cnt = min_t(u32, len, cnt);
> +
> +		for (i = 0; i < cnt; i++)
> +			*buf++ = zx_readl(vga->mmio + VGA_DATA);
> +
> +		len -= cnt;
> +		offset += cnt;
> +	}
> +
> +	return 0;
> +}
> +
> +static int zx_vga_i2c_write(struct zx_vga *vga, struct i2c_msg *msg)
> +{
> +	/*
> +	 * The DDC I2C adapter is only for reading EDID data, so we assume
> +	 * that the write to this adapter must be the EDID data offset.
> +	 */
> +	if ((msg->len != 1) || ((msg->addr != DDC_ADDR)))
> +		return -EINVAL;
> +
> +	/* Hardware will take care of the slave address shifting */
> +	zx_writel(vga->mmio + VGA_DEVICE_ADDR, msg->addr);
> +
> +	return 0;
> +}
> +
> +static int zx_vga_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +			   int num)
> +{
> +	struct zx_vga *vga = i2c_get_adapdata(adap);
> +	struct zx_vga_i2c *ddc = vga->ddc;
> +	int ret = 0;
> +	int i;
> +
> +	mutex_lock(&ddc->lock);
> +
> +	for (i = 0; i < num; i++) {
> +		if (msgs[i].flags & I2C_M_RD)
> +			ret = zx_vga_i2c_read(vga, &msgs[i]);
> +		else
> +			ret = zx_vga_i2c_write(vga, &msgs[i]);
> +
> +		if (ret < 0)
> +			break;
> +	}
> +
> +	if (!ret)
> +		ret = num;
> +
> +	mutex_unlock(&ddc->lock);
> +
> +	return ret;
> +}
> +
> +static u32 zx_vga_i2c_func(struct i2c_adapter *adapter)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static const struct i2c_algorithm zx_vga_algorithm = {
> +	.master_xfer	= zx_vga_i2c_xfer,
> +	.functionality	= zx_vga_i2c_func,
> +};
> +
> +static int zx_vga_ddc_register(struct zx_vga *vga)
> +{
> +	struct device *dev = vga->dev;
> +	struct i2c_adapter *adap;
> +	struct zx_vga_i2c *ddc;
> +	int ret;
> +
> +	ddc = devm_kzalloc(dev, sizeof(*ddc), GFP_KERNEL);
> +	if (!ddc)
> +		return -ENOMEM;
> +
> +	vga->ddc = ddc;
> +	mutex_init(&ddc->lock);
> +
> +	adap = &ddc->adap;
> +	adap->owner = THIS_MODULE;
> +	adap->class = I2C_CLASS_DDC;
> +	adap->dev.parent = dev;
> +	adap->algo = &zx_vga_algorithm;
> +	snprintf(adap->name, sizeof(adap->name), "zx vga i2c");
> +
> +	ret = i2c_add_adapter(adap);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to add I2C adapter: %d\n", ret);
> +		return ret;
> +	}
> +
> +	i2c_set_adapdata(adap, vga);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t zx_vga_irq_thread(int irq, void *dev_id)
> +{
> +	struct zx_vga *vga = dev_id;
> +
> +	drm_helper_hpd_irq_event(vga->connector.dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t zx_vga_irq_handler(int irq, void *dev_id)
> +{
> +	struct zx_vga *vga = dev_id;
> +	u32 status;
> +
> +	status = zx_readl(vga->mmio + VGA_I2C_STATUS);
> +
> +	/* Clear interrupt status */
> +	zx_writel_mask(vga->mmio + VGA_I2C_STATUS, VGA_CLEAR_IRQ,
> +		       VGA_CLEAR_IRQ);
> +
> +	if (status & VGA_DEVICE_CONNECTED) {
> +		/*
> +		 * Since VGA_DETECT_SEL bits need to be reset for switching DDC
> +		 * bus from device detection to EDID read, rather than setting
> +		 * up HAS_DEVICE bit here, we need to do that in .get_modes
> +		 * hook for unplug detecting after EDID read succeeds.
> +		 */
> +		vga->connected = true;
> +		return IRQ_WAKE_THREAD;
> +	}
> +
> +	if (status & VGA_DEVICE_DISCONNECTED) {
> +		spin_lock(&vga->lock);
> +		zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL,
> +			  VGA_DETECT_SEL_NO_DEVICE);
> +		spin_unlock(&vga->lock);

I think you still have the race here. If you get a disconnect between get_edid
successfully finishing, and resetting the DETECT_SEL register, you will end up
with the device being disconnected and DETECT_SEL == VGA_DETECT_SEL_HAS_DEVICE.

In order to close this, you'll need to hold the lock across the edid read.

Sean


> +		vga->connected = false;
> +		return IRQ_WAKE_THREAD;
> +	}
> +
> +	if (status & VGA_TRANS_DONE) {
> +		complete(&vga->complete);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static void zx_vga_hw_init(struct zx_vga *vga)
> +{
> +	unsigned long ref = clk_get_rate(vga->i2c_wclk);
> +	int div;
> +
> +	/*
> +	 * Set up I2C fast speed divider per formula below to get 400kHz.
> +	 *   scl = ref / ((div + 1) * 4)
> +	 */
> +	div = DIV_ROUND_UP(ref / 1000, 400 * 4) - 1;
> +	zx_writel(vga->mmio + VGA_CLK_DIV_FS, div);
> +
> +	/* Set up device detection */
> +	zx_writel(vga->mmio + VGA_AUTO_DETECT_PARA, 0x80);
> +	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_NO_DEVICE);
> +
> +	/*
> +	 * We need to poke monitor via DDC bus to get connection irq
> +	 * start working.
> +	 */
> +	zx_writel(vga->mmio + VGA_DEVICE_ADDR, DDC_ADDR);
> +	zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_TRANS, VGA_CMD_TRANS);
> +}
> +
> +static int zx_vga_bind(struct device *dev, struct device *master, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct drm_device *drm = data;
> +	struct resource *res;
> +	struct zx_vga *vga;
> +	int irq;
> +	int ret;
> +
> +	vga = devm_kzalloc(dev, sizeof(*vga), GFP_KERNEL);
> +	if (!vga)
> +		return -ENOMEM;
> +
> +	vga->dev = dev;
> +	dev_set_drvdata(dev, vga);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	vga->mmio = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(vga->mmio))
> +		return PTR_ERR(vga->mmio);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	vga->i2c_wclk = devm_clk_get(dev, "i2c_wclk");
> +	if (IS_ERR(vga->i2c_wclk)) {
> +		ret = PTR_ERR(vga->i2c_wclk);
> +		DRM_DEV_ERROR(dev, "failed to get i2c_wclk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = zx_vga_pwrctrl_init(vga);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to init power control: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = zx_vga_ddc_register(vga);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to register ddc: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = zx_vga_register(drm, vga);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to register vga: %d\n", ret);
> +		return ret;
> +	}
> +
> +	init_completion(&vga->complete);
> +	spin_lock_init(&vga->lock);
> +
> +	ret = devm_request_threaded_irq(dev, irq, zx_vga_irq_handler,
> +					zx_vga_irq_thread, IRQF_SHARED,
> +					dev_name(dev), vga);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to request threaded irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(vga->i2c_wclk);
> +	if (ret)
> +		return ret;
> +
> +	zx_vga_hw_init(vga);
> +
> +	return 0;
> +}
> +
> +static void zx_vga_unbind(struct device *dev, struct device *master,
> +			  void *data)
> +{
> +	struct zx_vga *vga = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(vga->i2c_wclk);
> +}
> +
> +static const struct component_ops zx_vga_component_ops = {
> +	.bind = zx_vga_bind,
> +	.unbind = zx_vga_unbind,
> +};
> +
> +static int zx_vga_probe(struct platform_device *pdev)
> +{
> +	return component_add(&pdev->dev, &zx_vga_component_ops);
> +}
> +
> +static int zx_vga_remove(struct platform_device *pdev)
> +{
> +	component_del(&pdev->dev, &zx_vga_component_ops);
> +	return 0;
> +}
> +
> +static const struct of_device_id zx_vga_of_match[] = {
> +	{ .compatible = "zte,zx296718-vga", },
> +	{ /* end */ },
> +};
> +MODULE_DEVICE_TABLE(of, zx_vga_of_match);
> +
> +struct platform_driver zx_vga_driver = {
> +	.probe = zx_vga_probe,
> +	.remove = zx_vga_remove,
> +	.driver	= {
> +		.name = "zx-vga",
> +		.of_match_table	= zx_vga_of_match,
> +	},
> +};
> diff --git a/drivers/gpu/drm/zte/zx_vga_regs.h b/drivers/gpu/drm/zte/zx_vga_regs.h
> new file mode 100644
> index 000000000000..feaa345fe6a6
> --- /dev/null
> +++ b/drivers/gpu/drm/zte/zx_vga_regs.h
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright (C) 2017 Sanechips Technology Co., Ltd.
> + * Copyright 2017 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ZX_VGA_REGS_H__
> +#define __ZX_VGA_REGS_H__
> +
> +#define VGA_CMD_CFG			0x04
> +#define VGA_CMD_TRANS			BIT(6)
> +#define VGA_CMD_COMBO			BIT(5)
> +#define VGA_CMD_RW			BIT(4)
> +#define VGA_SUB_ADDR			0x0c
> +#define VGA_DEVICE_ADDR			0x10
> +#define VGA_CLK_DIV_FS			0x14
> +#define VGA_RXF_CTRL			0x20
> +#define VGA_RX_FIFO_CLEAR		BIT(7)
> +#define VGA_DATA			0x24
> +#define VGA_I2C_STATUS			0x28
> +#define VGA_DEVICE_DISCONNECTED		BIT(7)
> +#define VGA_DEVICE_CONNECTED		BIT(6)
> +#define VGA_CLEAR_IRQ			BIT(4)
> +#define VGA_TRANS_DONE			BIT(0)
> +#define VGA_RXF_STATUS			0x30
> +#define VGA_RXF_COUNT_SHIFT		2
> +#define VGA_RXF_COUNT_MASK		GENMASK(7, 2)
> +#define VGA_AUTO_DETECT_PARA		0x34
> +#define VGA_AUTO_DETECT_SEL		0x38
> +#define VGA_DETECT_SEL_HAS_DEVICE	BIT(1)
> +#define VGA_DETECT_SEL_NO_DEVICE	BIT(0)
> +
> +#endif /* __ZX_VGA_REGS_H__ */
> diff --git a/drivers/gpu/drm/zte/zx_vou.c b/drivers/gpu/drm/zte/zx_vou.c
> index 2a2d90bd9425..cc5bdfc53e8e 100644
> --- a/drivers/gpu/drm/zte/zx_vou.c
> +++ b/drivers/gpu/drm/zte/zx_vou.c
> @@ -23,6 +23,7 @@
>  #include <drm/drm_plane_helper.h>
>  #include <drm/drmP.h>
>  
> +#include "zx_common_regs.h"
>  #include "zx_drm_drv.h"
>  #include "zx_plane.h"
>  #include "zx_vou.h"
> @@ -122,6 +123,8 @@ struct zx_crtc {
>  	struct drm_plane *primary;
>  	struct zx_vou_hw *vou;
>  	void __iomem *chnreg;
> +	void __iomem *chncsc;
> +	void __iomem *dither;
>  	const struct zx_crtc_regs *regs;
>  	const struct zx_crtc_bits *bits;
>  	enum vou_chn_type chn_type;
> @@ -204,6 +207,11 @@ struct vou_inf {
>  		.clocks_en_bits = BIT(15),
>  		.clocks_sel_bits = BIT(11) | BIT(0),
>  	},
> +	[VOU_VGA] = {
> +		.data_sel = VOU_RGB_888,
> +		.clocks_en_bits = BIT(1),
> +		.clocks_sel_bits = BIT(10),
> +	},
>  };
>  
>  static inline struct zx_vou_hw *crtc_to_vou(struct drm_crtc *crtc)
> @@ -227,9 +235,26 @@ void vou_inf_enable(enum vou_inf_id id, struct drm_crtc *crtc)
>  	struct zx_crtc *zcrtc = to_zx_crtc(crtc);
>  	struct zx_vou_hw *vou = zcrtc->vou;
>  	struct vou_inf *inf = &vou_infs[id];
> +	void __iomem *dither = zcrtc->dither;
> +	void __iomem *csc = zcrtc->chncsc;
>  	bool is_main = zcrtc->chn_type == VOU_CHN_MAIN;
>  	u32 data_sel_shift = id << 1;
>  
> +	if (inf->data_sel != VOU_YUV444) {
> +		/* Enable channel CSC for RGB output */
> +		zx_writel_mask(csc + CSC_CTRL0, CSC_COV_MODE_MASK,
> +			       CSC_BT709_IMAGE_YCBCR2RGB << CSC_COV_MODE_SHIFT);
> +		zx_writel_mask(csc + CSC_CTRL0, CSC_WORK_ENABLE,
> +			       CSC_WORK_ENABLE);
> +
> +		/* Bypass Dither block for RGB output */
> +		zx_writel_mask(dither + OSD_DITHER_CTRL0, DITHER_BYSPASS,
> +			       DITHER_BYSPASS);
> +	} else {
> +		zx_writel_mask(csc + CSC_CTRL0, CSC_WORK_ENABLE, 0);
> +		zx_writel_mask(dither + OSD_DITHER_CTRL0, DITHER_BYSPASS, 0);
> +	}
> +
>  	/* Select data format */
>  	zx_writel_mask(vou->vouctl + VOU_INF_DATA_SEL, 0x3 << data_sel_shift,
>  		       inf->data_sel << data_sel_shift);
> @@ -502,20 +527,24 @@ static int zx_crtc_init(struct drm_device *drm, struct zx_vou_hw *vou,
>  
>  	if (chn_type == VOU_CHN_MAIN) {
>  		zplane->layer = vou->osd + MAIN_GL_OFFSET;
> -		zplane->csc = vou->osd + MAIN_CSC_OFFSET;
> +		zplane->csc = vou->osd + MAIN_GL_CSC_OFFSET;
>  		zplane->hbsc = vou->osd + MAIN_HBSC_OFFSET;
>  		zplane->rsz = vou->otfppu + MAIN_RSZ_OFFSET;
>  		zplane->bits = &zx_gl_bits[0];
>  		zcrtc->chnreg = vou->osd + OSD_MAIN_CHN;
> +		zcrtc->chncsc = vou->osd + MAIN_CHN_CSC_OFFSET;
> +		zcrtc->dither = vou->osd + MAIN_DITHER_OFFSET;
>  		zcrtc->regs = &main_crtc_regs;
>  		zcrtc->bits = &main_crtc_bits;
>  	} else {
>  		zplane->layer = vou->osd + AUX_GL_OFFSET;
> -		zplane->csc = vou->osd + AUX_CSC_OFFSET;
> +		zplane->csc = vou->osd + AUX_GL_CSC_OFFSET;
>  		zplane->hbsc = vou->osd + AUX_HBSC_OFFSET;
>  		zplane->rsz = vou->otfppu + AUX_RSZ_OFFSET;
>  		zplane->bits = &zx_gl_bits[1];
>  		zcrtc->chnreg = vou->osd + OSD_AUX_CHN;
> +		zcrtc->chncsc = vou->osd + AUX_CHN_CSC_OFFSET;
> +		zcrtc->dither = vou->osd + AUX_DITHER_OFFSET;
>  		zcrtc->regs = &aux_crtc_regs;
>  		zcrtc->bits = &aux_crtc_bits;
>  	}
> diff --git a/drivers/gpu/drm/zte/zx_vou_regs.h b/drivers/gpu/drm/zte/zx_vou_regs.h
> index c066ef123434..5a218351b497 100644
> --- a/drivers/gpu/drm/zte/zx_vou_regs.h
> +++ b/drivers/gpu/drm/zte/zx_vou_regs.h
> @@ -13,13 +13,17 @@
>  
>  /* Sub-module offset */
>  #define MAIN_GL_OFFSET			0x130
> -#define MAIN_CSC_OFFSET			0x580
> +#define MAIN_GL_CSC_OFFSET		0x580
> +#define MAIN_CHN_CSC_OFFSET		0x6c0
>  #define MAIN_HBSC_OFFSET		0x820
> +#define MAIN_DITHER_OFFSET		0x960
>  #define MAIN_RSZ_OFFSET			0x600 /* OTFPPU sub-module */
>  
>  #define AUX_GL_OFFSET			0x200
> -#define AUX_CSC_OFFSET			0x5d0
> +#define AUX_GL_CSC_OFFSET		0x5d0
> +#define AUX_CHN_CSC_OFFSET		0x710
>  #define AUX_HBSC_OFFSET			0x860
> +#define AUX_DITHER_OFFSET		0x970
>  #define AUX_RSZ_OFFSET			0x800
>  
>  #define OSD_VL0_OFFSET			0x040
> @@ -78,6 +82,10 @@
>  #define CHN_INTERLACE_BUF_CTRL		0x24
>  #define CHN_INTERLACE_EN		BIT(2)
>  
> +/* Dither registers */
> +#define OSD_DITHER_CTRL0		0x00
> +#define DITHER_BYSPASS			BIT(31)
> +
>  /* TIMING_CTRL registers */
>  #define TIMING_TC_ENABLE		0x04
>  #define AUX_TC_EN			BIT(1)
> -- 
> 1.9.1
Shawn Guo April 7, 2017, 3:02 a.m. UTC | #2
On Thu, Apr 06, 2017 at 01:12:51PM -0400, Sean Paul wrote:
> On Thu, Apr 06, 2017 at 11:01:10PM +0800, Shawn Guo wrote:
> > +static int zx_vga_connector_get_modes(struct drm_connector *connector)
> > +{
> > +	struct zx_vga *vga = to_zx_vga(connector);
> > +	unsigned long flags;
> > +	struct edid *edid;
> > +	int ret;
> > +
> > +	/*
> > +	 * Clear both detection bits to switch I2C bus from device
> > +	 * detecting to EDID reading.
> > +	 */
> > +	spin_lock_irqsave(&vga->lock, flags);
> > +	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, 0);
> > +	spin_unlock_irqrestore(&vga->lock, flags);
> > +
> > +	edid = drm_get_edid(connector, &vga->ddc->adap);
> > +	if (!edid)
> > +		return 0;
> > +
> > +	/*
> > +	 * As edid reading succeeds, device must be connected, so we set
> > +	 * up detection bit for unplug interrupt here.
> > +	 */
> > +	spin_lock_irqsave(&vga->lock, flags);
> > +	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_HAS_DEVICE);
> > +	spin_unlock_irqrestore(&vga->lock, flags);
> > +
> > +	drm_mode_connector_update_edid_property(connector, edid);
> > +	ret = drm_add_edid_modes(connector, edid);
> > +	kfree(edid);
> > +
> > +	return ret;
> > +}

<snip>

> > +static irqreturn_t zx_vga_irq_handler(int irq, void *dev_id)
> > +{
> > +	struct zx_vga *vga = dev_id;
> > +	u32 status;
> > +
> > +	status = zx_readl(vga->mmio + VGA_I2C_STATUS);
> > +
> > +	/* Clear interrupt status */
> > +	zx_writel_mask(vga->mmio + VGA_I2C_STATUS, VGA_CLEAR_IRQ,
> > +		       VGA_CLEAR_IRQ);
> > +
> > +	if (status & VGA_DEVICE_CONNECTED) {
> > +		/*
> > +		 * Since VGA_DETECT_SEL bits need to be reset for switching DDC
> > +		 * bus from device detection to EDID read, rather than setting
> > +		 * up HAS_DEVICE bit here, we need to do that in .get_modes
> > +		 * hook for unplug detecting after EDID read succeeds.
> > +		 */
> > +		vga->connected = true;
> > +		return IRQ_WAKE_THREAD;
> > +	}
> > +
> > +	if (status & VGA_DEVICE_DISCONNECTED) {
> > +		spin_lock(&vga->lock);
> > +		zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL,
> > +			  VGA_DETECT_SEL_NO_DEVICE);
> > +		spin_unlock(&vga->lock);
> 
> I think you still have the race here. If you get a disconnect between get_edid
> successfully finishing, and resetting the DETECT_SEL register, you will end up
> with the device being disconnected and DETECT_SEL == VGA_DETECT_SEL_HAS_DEVICE.
> 
> In order to close this, you'll need to hold the lock across the edid read.

We cannot hold a spin lock across the EDID read procedure, which does
sleep.

When you suggested to have a lock for DETECT_SEL register access, I
thought we are accessing it in a read-modify-write way and thus agreed
to add a lock.  However, I just found that it's not the case actually.
All the accesses to the register are single direct write.

And here is my understanding about the condition you described above.
Once DETECT_SEL register gets reset (both bits cleared), the hardware
switches DDC bus for EDID read, and hotplug detection will stop working
right away (this is how ZTE hardware works).  That said, if we get a
disconnect before drm_get_edid() successfully finishes, two points:

- The irq handler will not be triggered, so it will not get a chance to
  update DETECT_SEL register.
- The drm_get_edid() fails, and .get_modes returns with both detect bits
  kept cleared.

I think what we can do better in this case is that we should set the
device state into disconnected, before returning from .get_modes,
something like the code below.  But still, I do not see we need a lock
for that.

Shawn

static int zx_vga_connector_get_modes(struct drm_connector *connector)
{
        struct zx_vga *vga = to_zx_vga(connector);
        unsigned long flags;
        struct edid *edid;
        int ret;

        /*
         * Clear both detection bits to switch I2C bus from device
         * detecting to EDID reading.
         */
        zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, 0);

        edid = drm_get_edid(connector, &vga->ddc->adap);
        if (!edid) {
                /*
                 * If EDID reading fails, we set the device state into
                 * disconnected.
                 */
                zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL,
                          VGA_DETECT_SEL_NO_DEVICE);
                vga->connected = false;
                return 0;
        }

        /*
         * As edid reading succeeds, device must be connected, so we set
         * up detection bit for unplug interrupt here.
         */
        zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_HAS_DEVICE);

        drm_mode_connector_update_edid_property(connector, edid);
        ret = drm_add_edid_modes(connector, edid);
        kfree(edid);

        return ret;
}
Sean Paul April 7, 2017, 2:43 p.m. UTC | #3
On Fri, Apr 07, 2017 at 11:02:33AM +0800, Shawn Guo wrote:
> On Thu, Apr 06, 2017 at 01:12:51PM -0400, Sean Paul wrote:
> > On Thu, Apr 06, 2017 at 11:01:10PM +0800, Shawn Guo wrote:
> > > +static int zx_vga_connector_get_modes(struct drm_connector *connector)
> > > +{
> > > +	struct zx_vga *vga = to_zx_vga(connector);
> > > +	unsigned long flags;
> > > +	struct edid *edid;
> > > +	int ret;
> > > +
> > > +	/*
> > > +	 * Clear both detection bits to switch I2C bus from device
> > > +	 * detecting to EDID reading.
> > > +	 */
> > > +	spin_lock_irqsave(&vga->lock, flags);
> > > +	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, 0);
> > > +	spin_unlock_irqrestore(&vga->lock, flags);
> > > +
> > > +	edid = drm_get_edid(connector, &vga->ddc->adap);
> > > +	if (!edid)
> > > +		return 0;
> > > +
> > > +	/*
> > > +	 * As edid reading succeeds, device must be connected, so we set
> > > +	 * up detection bit for unplug interrupt here.
> > > +	 */
> > > +	spin_lock_irqsave(&vga->lock, flags);
> > > +	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_HAS_DEVICE);
> > > +	spin_unlock_irqrestore(&vga->lock, flags);
> > > +
> > > +	drm_mode_connector_update_edid_property(connector, edid);
> > > +	ret = drm_add_edid_modes(connector, edid);
> > > +	kfree(edid);
> > > +
> > > +	return ret;
> > > +}
> 
> <snip>
> 
> > > +static irqreturn_t zx_vga_irq_handler(int irq, void *dev_id)
> > > +{
> > > +	struct zx_vga *vga = dev_id;
> > > +	u32 status;
> > > +
> > > +	status = zx_readl(vga->mmio + VGA_I2C_STATUS);
> > > +
> > > +	/* Clear interrupt status */
> > > +	zx_writel_mask(vga->mmio + VGA_I2C_STATUS, VGA_CLEAR_IRQ,
> > > +		       VGA_CLEAR_IRQ);
> > > +
> > > +	if (status & VGA_DEVICE_CONNECTED) {
> > > +		/*
> > > +		 * Since VGA_DETECT_SEL bits need to be reset for switching DDC
> > > +		 * bus from device detection to EDID read, rather than setting
> > > +		 * up HAS_DEVICE bit here, we need to do that in .get_modes
> > > +		 * hook for unplug detecting after EDID read succeeds.
> > > +		 */
> > > +		vga->connected = true;
> > > +		return IRQ_WAKE_THREAD;
> > > +	}
> > > +
> > > +	if (status & VGA_DEVICE_DISCONNECTED) {
> > > +		spin_lock(&vga->lock);
> > > +		zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL,
> > > +			  VGA_DETECT_SEL_NO_DEVICE);
> > > +		spin_unlock(&vga->lock);
> > 
> > I think you still have the race here. If you get a disconnect between get_edid
> > successfully finishing, and resetting the DETECT_SEL register, you will end up
> > with the device being disconnected and DETECT_SEL == VGA_DETECT_SEL_HAS_DEVICE.
> > 
> > In order to close this, you'll need to hold the lock across the edid read.
> 
> We cannot hold a spin lock across the EDID read procedure, which does
> sleep.

Yeah, this is a pretty common problem. We usually use a mutex in conjunction
with a work function to handle this type of thing.

> 
> When you suggested to have a lock for DETECT_SEL register access, I
> thought we are accessing it in a read-modify-write way and thus agreed
> to add a lock.  However, I just found that it's not the case actually.
> All the accesses to the register are single direct write.
> 
> And here is my understanding about the condition you described above.
> Once DETECT_SEL register gets reset (both bits cleared), the hardware
> switches DDC bus for EDID read, and hotplug detection will stop working
> right away (this is how ZTE hardware works).  That said, if we get a
> disconnect before drm_get_edid() successfully finishes, two points:
> 
> - The irq handler will not be triggered, so it will not get a chance to
>   update DETECT_SEL register.

Ah, this was the missing piece I needed. I hadn't realized that the first
VGA_AUTO_DETECT_SEL write in get_modes disabled the irq. 


> - The drm_get_edid() fails, and .get_modes returns with both detect bits
>   kept cleared.
> 
> I think what we can do better in this case is that we should set the
> device state into disconnected, before returning from .get_modes,
> something like the code below.  But still, I do not see we need a lock
> for that.

Yep, agreed. Can you also please add to your comment in the !edid case,
something like:

* Locking is not required here since the only other place to write this register
* (and connected var) is the irq handler. The irq handler is disabled because
* we've cleared AUTO_DETECT_SEL above.

Thanks for walking me through this.

Sean

> 
> Shawn
> 
> static int zx_vga_connector_get_modes(struct drm_connector *connector)
> {
>         struct zx_vga *vga = to_zx_vga(connector);
>         unsigned long flags;
>         struct edid *edid;
>         int ret;
> 
>         /*
>          * Clear both detection bits to switch I2C bus from device
>          * detecting to EDID reading.
>          */
>         zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, 0);
> 
>         edid = drm_get_edid(connector, &vga->ddc->adap);
>         if (!edid) {
>                 /*
>                  * If EDID reading fails, we set the device state into
>                  * disconnected.
>                  */
>                 zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL,
>                           VGA_DETECT_SEL_NO_DEVICE);
>                 vga->connected = false;
>                 return 0;
>         }
> 
>         /*
>          * As edid reading succeeds, device must be connected, so we set
>          * up detection bit for unplug interrupt here.
>          */
>         zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_HAS_DEVICE);
> 
>         drm_mode_connector_update_edid_property(connector, edid);
>         ret = drm_add_edid_modes(connector, edid);
>         kfree(edid);
> 
>         return ret;
> }
Shawn Guo April 11, 2017, 10:56 a.m. UTC | #4
On Fri, Apr 07, 2017 at 10:43:02AM -0400, Sean Paul wrote:
> On Fri, Apr 07, 2017 at 11:02:33AM +0800, Shawn Guo wrote:
> > When you suggested to have a lock for DETECT_SEL register access, I
> > thought we are accessing it in a read-modify-write way and thus agreed
> > to add a lock.  However, I just found that it's not the case actually.
> > All the accesses to the register are single direct write.
> > 
> > And here is my understanding about the condition you described above.
> > Once DETECT_SEL register gets reset (both bits cleared), the hardware
> > switches DDC bus for EDID read, and hotplug detection will stop working
> > right away (this is how ZTE hardware works).  That said, if we get a
> > disconnect before drm_get_edid() successfully finishes, two points:
> > 
> > - The irq handler will not be triggered, so it will not get a chance to
> >   update DETECT_SEL register.
> 
> Ah, this was the missing piece I needed. I hadn't realized that the first
> VGA_AUTO_DETECT_SEL write in get_modes disabled the irq. 

Sorry, something is not true with my point.  The irq handler can still
be triggered, but the cause has to be something else than hotplug
interrupt, like EDID data transfer done or various error conditions.

So my point should be that the VGA_AUTO_DETECT_SEL reset at the
beginning of .get_modes disables hotplug detection interrupt, and irq
handler will not get any chance to write to the register in this case.

> > - The drm_get_edid() fails, and .get_modes returns with both detect bits
> >   kept cleared.
> > 
> > I think what we can do better in this case is that we should set the
> > device state into disconnected, before returning from .get_modes,
> > something like the code below.  But still, I do not see we need a lock
> > for that.
> 
> Yep, agreed. Can you also please add to your comment in the !edid case,
> something like:
> 
> * Locking is not required here since the only other place to write this register
> * (and connected var) is the irq handler. The irq handler is disabled because
> * we've cleared AUTO_DETECT_SEL above.

Sure, thanks for the suggestion.  I will add proper comment in the new
version.

Shawn

Patch
diff mbox

diff --git a/drivers/gpu/drm/zte/Makefile b/drivers/gpu/drm/zte/Makefile
index 01352b56c418..9df7766a7f9d 100644
--- a/drivers/gpu/drm/zte/Makefile
+++ b/drivers/gpu/drm/zte/Makefile
@@ -3,6 +3,7 @@  zxdrm-y := \
 	zx_hdmi.o \
 	zx_plane.o \
 	zx_tvenc.o \
+	zx_vga.o \
 	zx_vou.o
 
 obj-$(CONFIG_DRM_ZTE) += zxdrm.o
diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
index 5c6944a1e72c..8a6892eeb44f 100644
--- a/drivers/gpu/drm/zte/zx_drm_drv.c
+++ b/drivers/gpu/drm/zte/zx_drm_drv.c
@@ -248,6 +248,7 @@  static int zx_drm_remove(struct platform_device *pdev)
 	&zx_crtc_driver,
 	&zx_hdmi_driver,
 	&zx_tvenc_driver,
+	&zx_vga_driver,
 	&zx_drm_platform_driver,
 };
 
diff --git a/drivers/gpu/drm/zte/zx_drm_drv.h b/drivers/gpu/drm/zte/zx_drm_drv.h
index 5ca035b079c7..2a8cdc5f8be4 100644
--- a/drivers/gpu/drm/zte/zx_drm_drv.h
+++ b/drivers/gpu/drm/zte/zx_drm_drv.h
@@ -14,6 +14,7 @@ 
 extern struct platform_driver zx_crtc_driver;
 extern struct platform_driver zx_hdmi_driver;
 extern struct platform_driver zx_tvenc_driver;
+extern struct platform_driver zx_vga_driver;
 
 static inline u32 zx_readl(void __iomem *reg)
 {
diff --git a/drivers/gpu/drm/zte/zx_vga.c b/drivers/gpu/drm/zte/zx_vga.c
new file mode 100644
index 000000000000..0d850d9ea700
--- /dev/null
+++ b/drivers/gpu/drm/zte/zx_vga.c
@@ -0,0 +1,530 @@ 
+/*
+ * Copyright (C) 2017 Sanechips Technology Co., Ltd.
+ * Copyright 2017 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drmP.h>
+
+#include "zx_drm_drv.h"
+#include "zx_vga_regs.h"
+#include "zx_vou.h"
+
+struct zx_vga_pwrctrl {
+	struct regmap *regmap;
+	u32 reg;
+	u32 mask;
+};
+
+struct zx_vga_i2c {
+	struct i2c_adapter adap;
+	struct mutex lock;
+};
+
+struct zx_vga {
+	struct drm_connector connector;
+	struct drm_encoder encoder;
+	struct zx_vga_i2c *ddc;
+	struct device *dev;
+	void __iomem *mmio;
+	struct clk *i2c_wclk;
+	struct zx_vga_pwrctrl pwrctrl;
+	spinlock_t lock;
+	struct completion complete;
+	bool connected;
+};
+
+#define to_zx_vga(x) container_of(x, struct zx_vga, x)
+
+static void zx_vga_encoder_enable(struct drm_encoder *encoder)
+{
+	struct zx_vga *vga = to_zx_vga(encoder);
+	struct zx_vga_pwrctrl *pwrctrl = &vga->pwrctrl;
+
+	/* Set bit to power up VGA DACs */
+	regmap_update_bits(pwrctrl->regmap, pwrctrl->reg, pwrctrl->mask,
+			   pwrctrl->mask);
+
+	vou_inf_enable(VOU_VGA, encoder->crtc);
+}
+
+static void zx_vga_encoder_disable(struct drm_encoder *encoder)
+{
+	struct zx_vga *vga = to_zx_vga(encoder);
+	struct zx_vga_pwrctrl *pwrctrl = &vga->pwrctrl;
+
+	vou_inf_disable(VOU_VGA, encoder->crtc);
+
+	/* Clear bit to power down VGA DACs */
+	regmap_update_bits(pwrctrl->regmap, pwrctrl->reg, pwrctrl->mask, 0);
+}
+
+static const struct drm_encoder_helper_funcs zx_vga_encoder_helper_funcs = {
+	.enable	= zx_vga_encoder_enable,
+	.disable = zx_vga_encoder_disable,
+};
+
+static const struct drm_encoder_funcs zx_vga_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static int zx_vga_connector_get_modes(struct drm_connector *connector)
+{
+	struct zx_vga *vga = to_zx_vga(connector);
+	unsigned long flags;
+	struct edid *edid;
+	int ret;
+
+	/*
+	 * Clear both detection bits to switch I2C bus from device
+	 * detecting to EDID reading.
+	 */
+	spin_lock_irqsave(&vga->lock, flags);
+	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, 0);
+	spin_unlock_irqrestore(&vga->lock, flags);
+
+	edid = drm_get_edid(connector, &vga->ddc->adap);
+	if (!edid)
+		return 0;
+
+	/*
+	 * As edid reading succeeds, device must be connected, so we set
+	 * up detection bit for unplug interrupt here.
+	 */
+	spin_lock_irqsave(&vga->lock, flags);
+	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_HAS_DEVICE);
+	spin_unlock_irqrestore(&vga->lock, flags);
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	ret = drm_add_edid_modes(connector, edid);
+	kfree(edid);
+
+	return ret;
+}
+
+static enum drm_mode_status
+zx_vga_connector_mode_valid(struct drm_connector *connector,
+			    struct drm_display_mode *mode)
+{
+	return MODE_OK;
+}
+
+static struct drm_connector_helper_funcs zx_vga_connector_helper_funcs = {
+	.get_modes = zx_vga_connector_get_modes,
+	.mode_valid = zx_vga_connector_mode_valid,
+};
+
+static enum drm_connector_status
+zx_vga_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct zx_vga *vga = to_zx_vga(connector);
+
+	return vga->connected ? connector_status_connected :
+				connector_status_disconnected;
+}
+
+static const struct drm_connector_funcs zx_vga_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.detect = zx_vga_connector_detect,
+	.destroy = drm_connector_cleanup,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int zx_vga_register(struct drm_device *drm, struct zx_vga *vga)
+{
+	struct drm_encoder *encoder = &vga->encoder;
+	struct drm_connector *connector = &vga->connector;
+	struct device *dev = vga->dev;
+	int ret;
+
+	encoder->possible_crtcs = VOU_CRTC_MASK;
+
+	ret = drm_encoder_init(drm, encoder, &zx_vga_encoder_funcs,
+			       DRM_MODE_ENCODER_DAC, NULL);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to init encoder: %d\n", ret);
+		return ret;
+	};
+
+	drm_encoder_helper_add(encoder, &zx_vga_encoder_helper_funcs);
+
+	vga->connector.polled = DRM_CONNECTOR_POLL_HPD;
+
+	ret = drm_connector_init(drm, connector, &zx_vga_connector_funcs,
+				 DRM_MODE_CONNECTOR_VGA);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to init connector: %d\n", ret);
+		goto clean_encoder;
+	};
+
+	drm_connector_helper_add(connector, &zx_vga_connector_helper_funcs);
+
+	ret = drm_mode_connector_attach_encoder(connector, encoder);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to attach encoder: %d\n", ret);
+		goto clean_connector;
+	};
+
+	return 0;
+
+clean_connector:
+	drm_connector_cleanup(connector);
+clean_encoder:
+	drm_encoder_cleanup(encoder);
+	return ret;
+}
+
+static int zx_vga_pwrctrl_init(struct zx_vga *vga)
+{
+	struct zx_vga_pwrctrl *pwrctrl = &vga->pwrctrl;
+	struct device *dev = vga->dev;
+	struct of_phandle_args out_args;
+	struct regmap *regmap;
+	int ret;
+
+	ret = of_parse_phandle_with_fixed_args(dev->of_node,
+				"zte,vga-power-control", 2, 0, &out_args);
+	if (ret)
+		return ret;
+
+	regmap = syscon_node_to_regmap(out_args.np);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		goto out;
+	}
+
+	pwrctrl->regmap = regmap;
+	pwrctrl->reg = out_args.args[0];
+	pwrctrl->mask = out_args.args[1];
+
+out:
+	of_node_put(out_args.np);
+	return ret;
+}
+
+static int zx_vga_i2c_read(struct zx_vga *vga, struct i2c_msg *msg)
+{
+	int len = msg->len;
+	u8 *buf = msg->buf;
+	u32 offset = 0;
+	int i;
+
+	reinit_completion(&vga->complete);
+
+	/* Select combo write */
+	zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_COMBO, VGA_CMD_COMBO);
+	zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_RW, 0);
+
+	while (len > 0) {
+		u32 cnt;
+
+		/* Clear RX FIFO */
+		zx_writel_mask(vga->mmio + VGA_RXF_CTRL, VGA_RX_FIFO_CLEAR,
+			       VGA_RX_FIFO_CLEAR);
+
+		/* Data offset to read from */
+		zx_writel(vga->mmio + VGA_SUB_ADDR, offset);
+
+		/* Kick off the transfer */
+		zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_TRANS,
+			       VGA_CMD_TRANS);
+
+		if (!wait_for_completion_timeout(&vga->complete,
+						 msecs_to_jiffies(1000))) {
+			DRM_DEV_ERROR(vga->dev, "transfer timeout\n");
+			return -ETIMEDOUT;
+		}
+
+		cnt = zx_readl(vga->mmio + VGA_RXF_STATUS);
+		cnt = (cnt & VGA_RXF_COUNT_MASK) >> VGA_RXF_COUNT_SHIFT;
+		/* FIFO status may report more data than we need to read */
+		cnt = min_t(u32, len, cnt);
+
+		for (i = 0; i < cnt; i++)
+			*buf++ = zx_readl(vga->mmio + VGA_DATA);
+
+		len -= cnt;
+		offset += cnt;
+	}
+
+	return 0;
+}
+
+static int zx_vga_i2c_write(struct zx_vga *vga, struct i2c_msg *msg)
+{
+	/*
+	 * The DDC I2C adapter is only for reading EDID data, so we assume
+	 * that the write to this adapter must be the EDID data offset.
+	 */
+	if ((msg->len != 1) || ((msg->addr != DDC_ADDR)))
+		return -EINVAL;
+
+	/* Hardware will take care of the slave address shifting */
+	zx_writel(vga->mmio + VGA_DEVICE_ADDR, msg->addr);
+
+	return 0;
+}
+
+static int zx_vga_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			   int num)
+{
+	struct zx_vga *vga = i2c_get_adapdata(adap);
+	struct zx_vga_i2c *ddc = vga->ddc;
+	int ret = 0;
+	int i;
+
+	mutex_lock(&ddc->lock);
+
+	for (i = 0; i < num; i++) {
+		if (msgs[i].flags & I2C_M_RD)
+			ret = zx_vga_i2c_read(vga, &msgs[i]);
+		else
+			ret = zx_vga_i2c_write(vga, &msgs[i]);
+
+		if (ret < 0)
+			break;
+	}
+
+	if (!ret)
+		ret = num;
+
+	mutex_unlock(&ddc->lock);
+
+	return ret;
+}
+
+static u32 zx_vga_i2c_func(struct i2c_adapter *adapter)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm zx_vga_algorithm = {
+	.master_xfer	= zx_vga_i2c_xfer,
+	.functionality	= zx_vga_i2c_func,
+};
+
+static int zx_vga_ddc_register(struct zx_vga *vga)
+{
+	struct device *dev = vga->dev;
+	struct i2c_adapter *adap;
+	struct zx_vga_i2c *ddc;
+	int ret;
+
+	ddc = devm_kzalloc(dev, sizeof(*ddc), GFP_KERNEL);
+	if (!ddc)
+		return -ENOMEM;
+
+	vga->ddc = ddc;
+	mutex_init(&ddc->lock);
+
+	adap = &ddc->adap;
+	adap->owner = THIS_MODULE;
+	adap->class = I2C_CLASS_DDC;
+	adap->dev.parent = dev;
+	adap->algo = &zx_vga_algorithm;
+	snprintf(adap->name, sizeof(adap->name), "zx vga i2c");
+
+	ret = i2c_add_adapter(adap);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to add I2C adapter: %d\n", ret);
+		return ret;
+	}
+
+	i2c_set_adapdata(adap, vga);
+
+	return 0;
+}
+
+static irqreturn_t zx_vga_irq_thread(int irq, void *dev_id)
+{
+	struct zx_vga *vga = dev_id;
+
+	drm_helper_hpd_irq_event(vga->connector.dev);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t zx_vga_irq_handler(int irq, void *dev_id)
+{
+	struct zx_vga *vga = dev_id;
+	u32 status;
+
+	status = zx_readl(vga->mmio + VGA_I2C_STATUS);
+
+	/* Clear interrupt status */
+	zx_writel_mask(vga->mmio + VGA_I2C_STATUS, VGA_CLEAR_IRQ,
+		       VGA_CLEAR_IRQ);
+
+	if (status & VGA_DEVICE_CONNECTED) {
+		/*
+		 * Since VGA_DETECT_SEL bits need to be reset for switching DDC
+		 * bus from device detection to EDID read, rather than setting
+		 * up HAS_DEVICE bit here, we need to do that in .get_modes
+		 * hook for unplug detecting after EDID read succeeds.
+		 */
+		vga->connected = true;
+		return IRQ_WAKE_THREAD;
+	}
+
+	if (status & VGA_DEVICE_DISCONNECTED) {
+		spin_lock(&vga->lock);
+		zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL,
+			  VGA_DETECT_SEL_NO_DEVICE);
+		spin_unlock(&vga->lock);
+		vga->connected = false;
+		return IRQ_WAKE_THREAD;
+	}
+
+	if (status & VGA_TRANS_DONE) {
+		complete(&vga->complete);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static void zx_vga_hw_init(struct zx_vga *vga)
+{
+	unsigned long ref = clk_get_rate(vga->i2c_wclk);
+	int div;
+
+	/*
+	 * Set up I2C fast speed divider per formula below to get 400kHz.
+	 *   scl = ref / ((div + 1) * 4)
+	 */
+	div = DIV_ROUND_UP(ref / 1000, 400 * 4) - 1;
+	zx_writel(vga->mmio + VGA_CLK_DIV_FS, div);
+
+	/* Set up device detection */
+	zx_writel(vga->mmio + VGA_AUTO_DETECT_PARA, 0x80);
+	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, VGA_DETECT_SEL_NO_DEVICE);
+
+	/*
+	 * We need to poke monitor via DDC bus to get connection irq
+	 * start working.
+	 */
+	zx_writel(vga->mmio + VGA_DEVICE_ADDR, DDC_ADDR);
+	zx_writel_mask(vga->mmio + VGA_CMD_CFG, VGA_CMD_TRANS, VGA_CMD_TRANS);
+}
+
+static int zx_vga_bind(struct device *dev, struct device *master, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct drm_device *drm = data;
+	struct resource *res;
+	struct zx_vga *vga;
+	int irq;
+	int ret;
+
+	vga = devm_kzalloc(dev, sizeof(*vga), GFP_KERNEL);
+	if (!vga)
+		return -ENOMEM;
+
+	vga->dev = dev;
+	dev_set_drvdata(dev, vga);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vga->mmio = devm_ioremap_resource(dev, res);
+	if (IS_ERR(vga->mmio))
+		return PTR_ERR(vga->mmio);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	vga->i2c_wclk = devm_clk_get(dev, "i2c_wclk");
+	if (IS_ERR(vga->i2c_wclk)) {
+		ret = PTR_ERR(vga->i2c_wclk);
+		DRM_DEV_ERROR(dev, "failed to get i2c_wclk: %d\n", ret);
+		return ret;
+	}
+
+	ret = zx_vga_pwrctrl_init(vga);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to init power control: %d\n", ret);
+		return ret;
+	}
+
+	ret = zx_vga_ddc_register(vga);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to register ddc: %d\n", ret);
+		return ret;
+	}
+
+	ret = zx_vga_register(drm, vga);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to register vga: %d\n", ret);
+		return ret;
+	}
+
+	init_completion(&vga->complete);
+	spin_lock_init(&vga->lock);
+
+	ret = devm_request_threaded_irq(dev, irq, zx_vga_irq_handler,
+					zx_vga_irq_thread, IRQF_SHARED,
+					dev_name(dev), vga);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to request threaded irq: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(vga->i2c_wclk);
+	if (ret)
+		return ret;
+
+	zx_vga_hw_init(vga);
+
+	return 0;
+}
+
+static void zx_vga_unbind(struct device *dev, struct device *master,
+			  void *data)
+{
+	struct zx_vga *vga = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(vga->i2c_wclk);
+}
+
+static const struct component_ops zx_vga_component_ops = {
+	.bind = zx_vga_bind,
+	.unbind = zx_vga_unbind,
+};
+
+static int zx_vga_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &zx_vga_component_ops);
+}
+
+static int zx_vga_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &zx_vga_component_ops);
+	return 0;
+}
+
+static const struct of_device_id zx_vga_of_match[] = {
+	{ .compatible = "zte,zx296718-vga", },
+	{ /* end */ },
+};
+MODULE_DEVICE_TABLE(of, zx_vga_of_match);
+
+struct platform_driver zx_vga_driver = {
+	.probe = zx_vga_probe,
+	.remove = zx_vga_remove,
+	.driver	= {
+		.name = "zx-vga",
+		.of_match_table	= zx_vga_of_match,
+	},
+};
diff --git a/drivers/gpu/drm/zte/zx_vga_regs.h b/drivers/gpu/drm/zte/zx_vga_regs.h
new file mode 100644
index 000000000000..feaa345fe6a6
--- /dev/null
+++ b/drivers/gpu/drm/zte/zx_vga_regs.h
@@ -0,0 +1,36 @@ 
+/*
+ * Copyright (C) 2017 Sanechips Technology Co., Ltd.
+ * Copyright 2017 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ZX_VGA_REGS_H__
+#define __ZX_VGA_REGS_H__
+
+#define VGA_CMD_CFG			0x04
+#define VGA_CMD_TRANS			BIT(6)
+#define VGA_CMD_COMBO			BIT(5)
+#define VGA_CMD_RW			BIT(4)
+#define VGA_SUB_ADDR			0x0c
+#define VGA_DEVICE_ADDR			0x10
+#define VGA_CLK_DIV_FS			0x14
+#define VGA_RXF_CTRL			0x20
+#define VGA_RX_FIFO_CLEAR		BIT(7)
+#define VGA_DATA			0x24
+#define VGA_I2C_STATUS			0x28
+#define VGA_DEVICE_DISCONNECTED		BIT(7)
+#define VGA_DEVICE_CONNECTED		BIT(6)
+#define VGA_CLEAR_IRQ			BIT(4)
+#define VGA_TRANS_DONE			BIT(0)
+#define VGA_RXF_STATUS			0x30
+#define VGA_RXF_COUNT_SHIFT		2
+#define VGA_RXF_COUNT_MASK		GENMASK(7, 2)
+#define VGA_AUTO_DETECT_PARA		0x34
+#define VGA_AUTO_DETECT_SEL		0x38
+#define VGA_DETECT_SEL_HAS_DEVICE	BIT(1)
+#define VGA_DETECT_SEL_NO_DEVICE	BIT(0)
+
+#endif /* __ZX_VGA_REGS_H__ */
diff --git a/drivers/gpu/drm/zte/zx_vou.c b/drivers/gpu/drm/zte/zx_vou.c
index 2a2d90bd9425..cc5bdfc53e8e 100644
--- a/drivers/gpu/drm/zte/zx_vou.c
+++ b/drivers/gpu/drm/zte/zx_vou.c
@@ -23,6 +23,7 @@ 
 #include <drm/drm_plane_helper.h>
 #include <drm/drmP.h>
 
+#include "zx_common_regs.h"
 #include "zx_drm_drv.h"
 #include "zx_plane.h"
 #include "zx_vou.h"
@@ -122,6 +123,8 @@  struct zx_crtc {
 	struct drm_plane *primary;
 	struct zx_vou_hw *vou;
 	void __iomem *chnreg;
+	void __iomem *chncsc;
+	void __iomem *dither;
 	const struct zx_crtc_regs *regs;
 	const struct zx_crtc_bits *bits;
 	enum vou_chn_type chn_type;
@@ -204,6 +207,11 @@  struct vou_inf {
 		.clocks_en_bits = BIT(15),
 		.clocks_sel_bits = BIT(11) | BIT(0),
 	},
+	[VOU_VGA] = {
+		.data_sel = VOU_RGB_888,
+		.clocks_en_bits = BIT(1),
+		.clocks_sel_bits = BIT(10),
+	},
 };
 
 static inline struct zx_vou_hw *crtc_to_vou(struct drm_crtc *crtc)
@@ -227,9 +235,26 @@  void vou_inf_enable(enum vou_inf_id id, struct drm_crtc *crtc)
 	struct zx_crtc *zcrtc = to_zx_crtc(crtc);
 	struct zx_vou_hw *vou = zcrtc->vou;
 	struct vou_inf *inf = &vou_infs[id];
+	void __iomem *dither = zcrtc->dither;
+	void __iomem *csc = zcrtc->chncsc;
 	bool is_main = zcrtc->chn_type == VOU_CHN_MAIN;
 	u32 data_sel_shift = id << 1;
 
+	if (inf->data_sel != VOU_YUV444) {
+		/* Enable channel CSC for RGB output */
+		zx_writel_mask(csc + CSC_CTRL0, CSC_COV_MODE_MASK,
+			       CSC_BT709_IMAGE_YCBCR2RGB << CSC_COV_MODE_SHIFT);
+		zx_writel_mask(csc + CSC_CTRL0, CSC_WORK_ENABLE,
+			       CSC_WORK_ENABLE);
+
+		/* Bypass Dither block for RGB output */
+		zx_writel_mask(dither + OSD_DITHER_CTRL0, DITHER_BYSPASS,
+			       DITHER_BYSPASS);
+	} else {
+		zx_writel_mask(csc + CSC_CTRL0, CSC_WORK_ENABLE, 0);
+		zx_writel_mask(dither + OSD_DITHER_CTRL0, DITHER_BYSPASS, 0);
+	}
+
 	/* Select data format */
 	zx_writel_mask(vou->vouctl + VOU_INF_DATA_SEL, 0x3 << data_sel_shift,
 		       inf->data_sel << data_sel_shift);
@@ -502,20 +527,24 @@  static int zx_crtc_init(struct drm_device *drm, struct zx_vou_hw *vou,
 
 	if (chn_type == VOU_CHN_MAIN) {
 		zplane->layer = vou->osd + MAIN_GL_OFFSET;
-		zplane->csc = vou->osd + MAIN_CSC_OFFSET;
+		zplane->csc = vou->osd + MAIN_GL_CSC_OFFSET;
 		zplane->hbsc = vou->osd + MAIN_HBSC_OFFSET;
 		zplane->rsz = vou->otfppu + MAIN_RSZ_OFFSET;
 		zplane->bits = &zx_gl_bits[0];
 		zcrtc->chnreg = vou->osd + OSD_MAIN_CHN;
+		zcrtc->chncsc = vou->osd + MAIN_CHN_CSC_OFFSET;
+		zcrtc->dither = vou->osd + MAIN_DITHER_OFFSET;
 		zcrtc->regs = &main_crtc_regs;
 		zcrtc->bits = &main_crtc_bits;
 	} else {
 		zplane->layer = vou->osd + AUX_GL_OFFSET;
-		zplane->csc = vou->osd + AUX_CSC_OFFSET;
+		zplane->csc = vou->osd + AUX_GL_CSC_OFFSET;
 		zplane->hbsc = vou->osd + AUX_HBSC_OFFSET;
 		zplane->rsz = vou->otfppu + AUX_RSZ_OFFSET;
 		zplane->bits = &zx_gl_bits[1];
 		zcrtc->chnreg = vou->osd + OSD_AUX_CHN;
+		zcrtc->chncsc = vou->osd + AUX_CHN_CSC_OFFSET;
+		zcrtc->dither = vou->osd + AUX_DITHER_OFFSET;
 		zcrtc->regs = &aux_crtc_regs;
 		zcrtc->bits = &aux_crtc_bits;
 	}
diff --git a/drivers/gpu/drm/zte/zx_vou_regs.h b/drivers/gpu/drm/zte/zx_vou_regs.h
index c066ef123434..5a218351b497 100644
--- a/drivers/gpu/drm/zte/zx_vou_regs.h
+++ b/drivers/gpu/drm/zte/zx_vou_regs.h
@@ -13,13 +13,17 @@ 
 
 /* Sub-module offset */
 #define MAIN_GL_OFFSET			0x130
-#define MAIN_CSC_OFFSET			0x580
+#define MAIN_GL_CSC_OFFSET		0x580
+#define MAIN_CHN_CSC_OFFSET		0x6c0
 #define MAIN_HBSC_OFFSET		0x820
+#define MAIN_DITHER_OFFSET		0x960
 #define MAIN_RSZ_OFFSET			0x600 /* OTFPPU sub-module */
 
 #define AUX_GL_OFFSET			0x200
-#define AUX_CSC_OFFSET			0x5d0
+#define AUX_GL_CSC_OFFSET		0x5d0
+#define AUX_CHN_CSC_OFFSET		0x710
 #define AUX_HBSC_OFFSET			0x860
+#define AUX_DITHER_OFFSET		0x970
 #define AUX_RSZ_OFFSET			0x800
 
 #define OSD_VL0_OFFSET			0x040
@@ -78,6 +82,10 @@ 
 #define CHN_INTERLACE_BUF_CTRL		0x24
 #define CHN_INTERLACE_EN		BIT(2)
 
+/* Dither registers */
+#define OSD_DITHER_CTRL0		0x00
+#define DITHER_BYSPASS			BIT(31)
+
 /* TIMING_CTRL registers */
 #define TIMING_TC_ENABLE		0x04
 #define AUX_TC_EN			BIT(1)