[PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver

[sebastian.hesselbarth@gmail.com (Sebastian Hesselbarth)]

On 06/09/13 21:29, Russell King wrote:
> This patch adds support for the pair of LCD controllers on the Marvell
> Armada 510 SoCs.  This driver supports:
> - multiple contiguous scanout buffers for video and graphics
> - shm backed cacheable buffer objects for X pixmaps for Vivante GPU
>    acceleration
> - dual lcd0 and lcd1 crt operation
> - video overlay on each LCD crt
> - page flipping of the main scanout buffers
>
> Included in this commit is the core driver with no output support; output
> support is platform and encoder driver dependent.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
[...]
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> new file mode 100644
> index 0000000..e5ab4cb
> --- /dev/null
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -0,0 +1,766 @@
> +/*
> + * Copyright (C) 2012 Russell King
> + *  Rewritten from the dovefb driver, and Armada510 manuals.
> + *
> + * 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.
> + */
[...]
> +static void armada_drm_crtc_update(struct armada_crtc *dcrtc)
> +{
> +	uint32_t dumb_ctrl;
> +
> +	dumb_ctrl = dcrtc->cfg_dumb_ctrl;
> +
> +	if (!dpms_blanked(dcrtc->dpms))
> +		dumb_ctrl |= CFG_DUMB_ENA;
> +
> +	/*
> +	 * When a dumb interface isn't under 24bit, it might be
> +	 * under SPI or GPIO.  If set to 7, this will force
> +	 * LCD_D[23:0] to output blank color and damage GPIO
> +	 * and SPI behaviour.  So leave it as-is unless in
> +	 * DUMB24_RGB888_0 mode.
> +	 */
> +	if (dpms_blanked(dcrtc->dpms) &&
> +	    (dumb_ctrl & DUMB_MASK) == DUMB24_RGB888_0) {
> +		dumb_ctrl &= ~DUMB_MASK;
> +		dumb_ctrl |= DUMB_BLANK;
> +	}
> +
> +	/*
> +	 * The spec is unclear about the polarities of the syncs.
> +	 * We assume their non-inverted state is active high.
> +	 */

nit: "We confirmed their non-inverted state is active high."

> +	if (dcrtc->crtc.mode.flags & DRM_MODE_FLAG_NCSYNC)
> +		dumb_ctrl |= CFG_INV_CSYNC;
> +	if (dcrtc->crtc.mode.flags & DRM_MODE_FLAG_NHSYNC)
> +		dumb_ctrl |= CFG_INV_HSYNC;
> +	if (dcrtc->crtc.mode.flags & DRM_MODE_FLAG_NVSYNC)
> +		dumb_ctrl |= CFG_INV_VSYNC;
> +
> +	if (dcrtc->dumb_ctrl != dumb_ctrl) {
> +		dcrtc->dumb_ctrl = dumb_ctrl;
> +		writel_relaxed(dumb_ctrl, dcrtc->base + LCD_SPU_DUMB_CTRL);
> +	}
> +}
[...]
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> new file mode 100644
> index 0000000..bb70cf5
> --- /dev/null
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -0,0 +1,326 @@
> +/*
> + * Copyright (C) 2012 Russell King
> + *
> + * 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/module.h>
> +#include <drm/drmP.h>
> +#include <drm/drm_crtc_helper.h>
> +#include "armada_crtc.h"
> +#include "armada_drm.h"
> +#include "armada_gem.h"
> +#include "armada_hw.h"
> +#include "armada_ioctl.h"
> +#include "armada_ioctlP.h"
> +
> +static int armada_drm_load(struct drm_device *dev, unsigned long flags)
> +{
> +	struct armada_private *priv;
> +	struct resource *res[ARRAY_SIZE(priv->dcrtc)];
> +	struct resource *mem = NULL;
> +	int ret, n, i;
> +
> +	memset(res, 0, sizeof(res));
> +
> +	for (n = i = 0; ; n++) {
> +		struct resource *r = platform_get_resource(dev->platformdev,
> +							   IORESOURCE_MEM, n);
> +		if (!r)
> +			break;
> +
> +		if (resource_size(r) > SZ_4K)
> +			mem = r;

nit again: The register address window of Dove LCD is 64k although you 
are right an no more than 512B are used. Also a comment would be nice,
that everything above 4k (64k) is interpreted as gfx mem.

> +		else if (i < ARRAY_SIZE(priv->dcrtc))
> +			res[i++] = r;
> +		else
> +			return -EINVAL;
> +	}
> +
> +	if (!res[0] || !mem)
> +		return -ENXIO;
> +
> +	if (!devm_request_mem_region(dev->dev, mem->start,
> +			resource_size(mem), "armada-drm"))
> +		return -EBUSY;
> +
> +	priv = devm_kzalloc(dev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		DRM_ERROR("failed to allocate private\n");
> +		ret = -ENOMEM;
> +		goto err_buf;
> +	}
> +
> +	dev->dev_private = priv;
> +
> +	/* Mode setting support */
> +	drm_mode_config_init(dev);
> +	dev->mode_config.min_width = 0;
> +	dev->mode_config.min_height = 0;
> +	dev->mode_config.max_width = 2048;
> +	dev->mode_config.max_height = 2048;
> +	dev->mode_config.preferred_depth = 24;
> +	dev->mode_config.funcs = &armada_drm_mode_config_funcs;
> +	drm_mm_init(&priv->linear, mem->start, resource_size(mem));
> +
> +	/* Create all LCD controllers */
> +	for (n = 0; n < ARRAY_SIZE(priv->dcrtc); n++) {
> +		struct clk *clk;
> +
> +		if (!res[n])
> +			break;
> +
> +		clk = clk_get_sys("dovefb.0", "extclk");

To be precise: the above should have the index at extclk as there
is two extclk inputs that can be used for both lcdcs. So currently,
as armada_crtc is hard-coding extclk0 input it should be
"armadafb.%d", "extclk0".

But I know, clocking in general will work-out with parent select for
clk-mux and DT support.

[...]
> diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
> new file mode 100644
> index 0000000..68c4efd
> --- /dev/null
> +++ b/drivers/gpu/drm/armada/armada_overlay.c
> @@ -0,0 +1,514 @@
> +/*
> + * Copyright (C) 2012 Russell King
> + *  Rewritten from the dovefb driver, and Armada510 manuals.
> + *
> + * 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.
> + */
[...]
> +int armada_overlay_put_image_ioctl(struct drm_device *dev, void *data,
> +	struct drm_file *file)
> +{
> +	struct drm_armada_overlay_put_image *args = data;
> +	struct armada_private *priv = dev->dev_private;
> +	struct armada_overlay *ovl = priv->overlay;
> +	struct armada_gem_object *obj;
> +	struct armada_crtc *dcrtc;
> +	uint32_t stride_uv, stride_yc, src_hw, dst_hw, dst_yx;
> +	bool planar = false;
> +	int ret, idx;
> +
> +	if (!ovl) {
> +		DRM_DEBUG_DRIVER("no overlay");
> +		return -ENODEV;
> +	}
> +
> +	if (!(args->flags & ARMADA_OVERLAY_ENABLE)) {
> +		mutex_lock(&dev->mode_config.mutex);
> +		mutex_lock(&dev->struct_mutex);
> +
> +		armada_drm_overlay_off(dev, ovl);
> +
> +		mutex_unlock(&dev->struct_mutex);
> +		mutex_unlock(&dev->mode_config.mutex);
> +
> +		return 0;
> +	}
> +
> +//	DRM_DEBUG_DRIVER("flags %x handle %x src %dx%d dst %dx%d+%d+%d\n",
> +//		args->flags, args->bo_handle, args->src_scan_width, args->src_scan_height,
> +//		args->dst_width, args->dst_height, args->dst_x, args->dst_y);

nit: Reminder for a hopefully soon to surface non-RFC patch.

[...]
> diff --git a/drivers/gpu/drm/armada/armada_slave.c b/drivers/gpu/drm/armada/armada_slave.c
> new file mode 100644
> index 0000000..422a345
> --- /dev/null
> +++ b/drivers/gpu/drm/armada/armada_slave.c
> @@ -0,0 +1,138 @@
> +/*
> + * Copyright (C) 2012 Russell King
> + *  Rewritten from the dovefb driver, and Armada510 manuals.
> + *
> + * 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.
> + */
[...]
> +static int armada_drm_conn_slave_create(struct drm_connector *conn, const void *data)
> +{
> +	const struct armada_drm_slave_config *config = data;
> +	struct drm_encoder_slave *slave;
> +	struct i2c_adapter *adap;
> +	int ret;
> +
> +	conn->interlace_allowed = config->interlace_allowed;
> +	conn->doublescan_allowed = config->doublescan_allowed;
> +	conn->polled = config->polled;
> +
> +	drm_connector_helper_add(conn, &armada_drm_slave_helper_funcs);
> +
> +	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
> +	if (!slave)
> +		return -ENOMEM;
> +
> +	slave->base.possible_crtcs = config->crtcs;
> +
> +	adap = i2c_get_adapter(config->i2c_adapter_id);
> +	if (!adap) {
> +		kfree(slave);
> +		return -EPROBE_DEFER;
> +	}
> +
> +	ret = drm_encoder_init(conn->dev, &slave->base,
> +			       &armada_drm_slave_encoder_funcs,
> +			       DRM_MODE_ENCODER_TMDS);
> +	if (ret) {
> +		DRM_ERROR("unable to init encoder\n");
> +		i2c_put_adapter(adap);
> +		kfree(slave);
> +		return ret;
> +	}
> +
> +	ret = drm_i2c_encoder_init(conn->dev, slave, adap, &config->info);
> +	i2c_put_adapter(adap);
> +	if (ret) {
> +		DRM_ERROR("unable to init encoder slave\n");
> +		armada_drm_slave_destroy(&slave->base);
> +		return ret;
> +	}
> +
> +	drm_encoder_helper_add(&slave->base, &drm_slave_encoder_helpers);
> +
> +	ret = slave->slave_funcs->create_resources(&slave->base, conn);
> +	if (ret) {
> +		armada_drm_slave_destroy(&slave->base);
> +		return ret;
> +	}
> +
> +	ret = drm_mode_connector_attach_encoder(conn, &slave->base);
> +	if (ret) {
> +		armada_drm_slave_destroy(&slave->base);
> +		return ret;
> +	}
> +
> +	conn->encoder = &slave->base;
> +
> +	return ret;
> +}
> +
> +static const struct armada_output_type armada_drm_conn_slave = {
> +	.connector_type	= DRM_MODE_CONNECTOR_HDMIA,

For a rework of DRM slave encoder API, there should also be some way to
get .connector_type and .encoder_type above from that slave encoder.
IMHO it should be up to the slave encoder to determine connector and
encoder type.

[...]
> diff --git a/drivers/gpu/drm/armada/armada_slave.h b/drivers/gpu/drm/armada/armada_slave.h
> new file mode 100644
> index 0000000..1b86696
> --- /dev/null
> +++ b/drivers/gpu/drm/armada/armada_slave.h
> @@ -0,0 +1,26 @@
> +/*
> + * Copyright (C) 2012 Russell King
> + *
> + * 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 ARMADA_TDA19988_H
> +#define ARMADA_TDA19988_H

nit: ARMADA_SLAVE_H

> +
> +#include <linux/i2c.h>
> +#include <drm/drmP.h>
> +
> +struct armada_drm_slave_config {
> +	int i2c_adapter_id;
> +	uint32_t crtcs;
> +	uint8_t polled;
> +	bool interlace_allowed;
> +	bool doublescan_allowed;
> +	struct i2c_board_info info;
> +};
> +
> +int armada_drm_connector_slave_create(struct drm_device *dev,
> +	const struct armada_drm_slave_config *);
> +
> +#endif

Thanks again for sharing the driver as RFC! Will test soon and post
promised tda998x sync patches.

Sebastian