diff mbox

[2/2] drm: Add new driver for MXSFB controller

Message ID 20160826142742.7236-2-marex@denx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Marek Vasut Aug. 26, 2016, 2:27 p.m. UTC
Add new driver for the MXSFB controller found in i.MX23/28/6SX .
The MXSFB controller is a simple framebuffer controller with one
parallel LCD output. Unlike the MXSFB fbdev driver that is used
on these systems now, this driver uses the DRM/KMS framework.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Shawn Guo <shawnguo@kernel.org>
---
 MAINTAINERS                        |   6 +
 drivers/gpu/drm/Kconfig            |   2 +
 drivers/gpu/drm/Makefile           |   1 +
 drivers/gpu/drm/mxsfb/Kconfig      |  18 ++
 drivers/gpu/drm/mxsfb/Makefile     |   2 +
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 395 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 324 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/mxsfb/mxsfb_drv.h  |  58 ++++++
 drivers/gpu/drm/mxsfb/mxsfb_out.c  | 264 +++++++++++++++++++++++++
 drivers/gpu/drm/mxsfb/mxsfb_regs.h | 112 +++++++++++
 10 files changed, 1182 insertions(+)
 create mode 100644 drivers/gpu/drm/mxsfb/Kconfig
 create mode 100644 drivers/gpu/drm/mxsfb/Makefile
 create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_crtc.c
 create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_drv.c
 create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_drv.h
 create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_out.c
 create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_regs.h

Comments

Daniel Vetter Aug. 28, 2016, 4:44 p.m. UTC | #1
On Fri, Aug 26, 2016 at 04:27:42PM +0200, Marek Vasut wrote:
> Add new driver for the MXSFB controller found in i.MX23/28/6SX .
> The MXSFB controller is a simple framebuffer controller with one
> parallel LCD output. Unlike the MXSFB fbdev driver that is used
> on these systems now, this driver uses the DRM/KMS framework.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> ---
>  MAINTAINERS                        |   6 +
>  drivers/gpu/drm/Kconfig            |   2 +
>  drivers/gpu/drm/Makefile           |   1 +
>  drivers/gpu/drm/mxsfb/Kconfig      |  18 ++
>  drivers/gpu/drm/mxsfb/Makefile     |   2 +
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 395 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 324 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h  |  58 ++++++
>  drivers/gpu/drm/mxsfb/mxsfb_out.c  | 264 +++++++++++++++++++++++++
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h | 112 +++++++++++
>  10 files changed, 1182 insertions(+)
>  create mode 100644 drivers/gpu/drm/mxsfb/Kconfig
>  create mode 100644 drivers/gpu/drm/mxsfb/Makefile
>  create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_crtc.c
>  create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_drv.c
>  create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_drv.h
>  create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_out.c
>  create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_regs.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8c20323..39b16b4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7781,6 +7781,12 @@ T:	git git://linuxtv.org/mkrufky/tuners.git
>  S:	Maintained
>  F:	drivers/media/tuners/mxl5007t.*
>  
> +MXSFB DRM DRIVER
> +M:	Marek Vasut <marex@denx.de>
> +S:	Supported
> +F:	drivers/gpu/drm/mxsfb/
> +F:	Documentation/devicetree/bindings/display/mxsfb-drm.txt
> +
>  MYRICOM MYRI-10G 10GbE DRIVER (MYRI10GE)
>  M:	Hyong-Youb Kim <hykim@myri.com>
>  L:	netdev@vger.kernel.org
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index fc35731..5b1e7bc 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -290,3 +290,5 @@ source "drivers/gpu/drm/arc/Kconfig"
>  source "drivers/gpu/drm/hisilicon/Kconfig"
>  
>  source "drivers/gpu/drm/mediatek/Kconfig"
> +
> +source "drivers/gpu/drm/mxsfb/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index be43afb..cbb638d 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -82,3 +82,4 @@ obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
>  obj-$(CONFIG_DRM_ARCPGU)+= arc/
>  obj-y			+= hisilicon/
> +obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
> diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
> new file mode 100644
> index 0000000..0b6cb59
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/Kconfig
> @@ -0,0 +1,18 @@
> +config DRM_MXS
> +	bool
> +	help
> +	  Choose this option to select drivers for MXS FB devices
> +
> +config DRM_MXSFB
> +	tristate "i.MX23/i.MX28/i.MX6SX MXSFB LCD controller"
> +	depends on DRM && OF
> +	depends on COMMON_CLK
> +	select DRM_MXS
> +	select DRM_KMS_HELPER
> +	select DRM_KMS_FB_HELPER
> +	select DRM_KMS_CMA_HELPER
> +	help
> +	  Choose this option if you have an i.MX23/i.MX28/i.MX6SX MXSFB
> +	  LCD controller.
> +
> +	  If M is selected the module will be called mxsfb.
> diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile
> new file mode 100644
> index 0000000..857f3a4
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/Makefile
> @@ -0,0 +1,2 @@
> +mxsfb-y := mxsfb_drv.o mxsfb_crtc.o mxsfb_out.o
> +obj-$(CONFIG_DRM_MXSFB)	+= mxsfb.o
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> new file mode 100644
> index 0000000..5d2e2a1
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
> @@ -0,0 +1,395 @@
> +/*
> + * Copyright (C) 2016 Marek Vasut <marex@denx.de>
> + *
> + * This code is based on drivers/video/fbdev/mxsfb.c :
> + * Copyright (C) 2010 Juergen Beisert, Pengutronix
> + * Copyright (C) 2008-2009 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright (C) 2008 Embedded Alley Solutions, Inc All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_plane_helper.h>
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_data/simplefb.h>
> +#include <video/videomode.h>
> +
> +#include "mxsfb_drv.h"
> +#include "mxsfb_regs.h"
> +
> +static void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
> +{
> +	if (mxsfb->clk_axi)
> +		clk_prepare_enable(mxsfb->clk_axi);
> +}
> +
> +static void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
> +{
> +	if (mxsfb->clk_axi)
> +		clk_disable_unprepare(mxsfb->clk_axi);
> +}
> +
> +static u32 set_hsync_pulse_width(struct mxsfb_drm_private *mxsfb, u32 val)
> +{
> +	return (val & mxsfb->devdata->hs_wdth_mask) <<
> +		mxsfb->devdata->hs_wdth_shift;
> +}
> +
> +static struct simplefb_format supported_formats[] = SIMPLEFB_FORMATS;
> +
> +/*
> + * Setup the MXSFB registers for decoding the pixels out of the framebuffer
> + */
> +static int mxsfb_set_pixel_fmt(struct drm_crtc *crtc)
> +{
> +	struct drm_device *drm = crtc->dev;
> +	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
> +	struct simplefb_format *format = NULL;
> +	u32 pixel_format, ctrl;
> +	int i;
> +
> +	pixel_format = crtc->primary->state->fb->pixel_format;
> +
> +	for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
> +		if (supported_formats[i].fourcc == pixel_format)
> +			format = &supported_formats[i];
> +	}
> +
> +	if (WARN_ON(!format))
> +		return 0;
> +
> +	ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
> +
> +	/*
> +	 * WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
> +	 * match the selected mode here. This differs from the original
> +	 * MXSFB driver, which had the option to configure the bus width
> +	 * to arbitrary value. This limitation should not pose an issue.
> +	 */
> +
> +	switch (format->fourcc) {
> +	case DRM_FORMAT_RGB565:
> +		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
> +		ctrl |= CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT);
> +		ctrl |= CTRL_SET_WORD_LENGTH(0);
> +		writel(CTRL1_SET_BYTE_PACKAGING(0xf), mxsfb->base + LCDC_CTRL1);
> +		break;
> +	case DRM_FORMAT_XRGB8888:
> +		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
> +		ctrl |= CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT);
> +		ctrl |= CTRL_SET_WORD_LENGTH(3);
> +		/* Do not use packed pixels = one pixel per word instead */
> +		writel(CTRL1_SET_BYTE_PACKAGING(0x7), mxsfb->base + LCDC_CTRL1);
> +		break;
> +	default:
> +		dev_err(drm->dev, "Unhandled color format %s\n",
> +			format->name);
> +		return -EINVAL;

You need to check such failures in the atomic_check code, doing it only in
atomic_commit (or anything called from that) is way too late.

If you do check this already (simply restrict the list of support fourcc
codes to only the 2 above), then you can do a WARN_ON + return; and change
the return value from int to void here.

> +	}
> +
> +	writel(ctrl, mxsfb->base + LCDC_CTRL);
> +	return 0;
> +}
> +
> +static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
> +{
> +	u32 reg;
> +
> +	if (mxsfb->clk_disp_axi)
> +		clk_prepare_enable(mxsfb->clk_disp_axi);
> +	clk_prepare_enable(mxsfb->clk);
> +	mxsfb_enable_axi_clk(mxsfb);
> +
> +	/* If it was disabled, re-enable the mode again */
> +	writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
> +
> +	/* Enable the SYNC signals first, then the DMA engine */
> +	reg = readl(mxsfb->base + LCDC_VDCTRL4);
> +	reg |= VDCTRL4_SYNC_SIGNALS_ON;
> +	writel(reg, mxsfb->base + LCDC_VDCTRL4);
> +
> +	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
> +
> +	mxsfb->enabled = 1;
> +}
> +
> +static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb)
> +{
> +	u32 reg;
> +
> +	/*
> +	 * Even if we disable the controller here, it will still continue
> +	 * until its FIFOs are running out of data
> +	 */
> +	writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_CLR);
> +
> +	readl_poll_timeout(mxsfb->base + LCDC_CTRL, reg, !(reg & CTRL_RUN),
> +			   0, 1000);
> +
> +	reg = readl(mxsfb->base + LCDC_VDCTRL4);
> +	reg &= ~VDCTRL4_SYNC_SIGNALS_ON;
> +	writel(reg, mxsfb->base + LCDC_VDCTRL4);
> +
> +	mxsfb_disable_axi_clk(mxsfb);
> +
> +	clk_disable_unprepare(mxsfb->clk);
> +	if (mxsfb->clk_disp_axi)
> +		clk_disable_unprepare(mxsfb->clk_disp_axi);
> +
> +	mxsfb->enabled = 0;
> +}
> +
> +static void mxsfb_crtc_mode_set_nofb(struct drm_crtc *crtc)
> +{
> +	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
> +	struct drm_display_mode *m = &crtc->state->adjusted_mode;
> +	const u32 bus_flags = mxsfb->output.connector.display_info.bus_flags;
> +	u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
> +	bool reenable = false;
> +	int err;
> +
> +	/*
> +	 * It seems, you can't re-program the controller if it is still
> +	 * running. This may lead to shifted pictures (FIFO issue?), so
> +	 * first stop the controller and drain its FIFOs.
> +	 */
> +	if (mxsfb->enabled) {
> +		reenable = true;
> +		mxsfb_disable_controller(mxsfb);
> +	}

The atomic core should keep perfect track of the state of your controller
and never asky ou to re-enable it when it's already enabled. Please remove
this code (and the ->enabled tracking, it should be redundant).

If this doesn't work then we have a bug in the atomic core.

> +
> +	mxsfb_enable_axi_clk(mxsfb);
> +
> +	/* Clear the FIFOs */
> +	writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET);
> +
> +	err = mxsfb_set_pixel_fmt(crtc);
> +	if (err)
> +		return;
> +
> +	clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
> +
> +	writel(TRANSFER_COUNT_SET_VCOUNT(m->crtc_vdisplay) |
> +	       TRANSFER_COUNT_SET_HCOUNT(m->crtc_hdisplay),
> +	       mxsfb->base + mxsfb->devdata->transfer_count);
> +
> +	vsync_pulse_len = m->crtc_vsync_end - m->crtc_vsync_start;
> +
> +	vdctrl0 = VDCTRL0_ENABLE_PRESENT |	/* Always in DOTCLOCK mode */
> +		  VDCTRL0_VSYNC_PERIOD_UNIT |
> +		  VDCTRL0_VSYNC_PULSE_WIDTH_UNIT |
> +		  VDCTRL0_SET_VSYNC_PULSE_WIDTH(vsync_pulse_len);
> +	if (m->flags & DRM_MODE_FLAG_PHSYNC)
> +		vdctrl0 |= VDCTRL0_HSYNC_ACT_HIGH;
> +	if (m->flags & DRM_MODE_FLAG_PVSYNC)
> +		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
> +	if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> +		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
> +	if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
> +		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
> +
> +	writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0);
> +
> +	/* Frame length in lines. */
> +	writel(m->crtc_vtotal, mxsfb->base + LCDC_VDCTRL1);
> +
> +	/* Line length in units of clocks or pixels. */
> +	hsync_pulse_len = m->crtc_hsync_end - m->crtc_hsync_start;
> +	writel(set_hsync_pulse_width(mxsfb, hsync_pulse_len) |
> +	       VDCTRL2_SET_HSYNC_PERIOD(m->crtc_htotal),
> +	       mxsfb->base + LCDC_VDCTRL2);
> +
> +	writel(SET_HOR_WAIT_CNT(m->crtc_hblank_end - m->crtc_hsync_end) |
> +	       SET_VERT_WAIT_CNT(m->crtc_vblank_end - m->crtc_vsync_end),
> +	       mxsfb->base + LCDC_VDCTRL3);
> +
> +	writel(SET_DOTCLK_H_VALID_DATA_CNT(m->hdisplay),
> +	       mxsfb->base + LCDC_VDCTRL4);
> +
> +	mxsfb_disable_axi_clk(mxsfb);
> +
> +	if (reenable)
> +		mxsfb_enable_controller(mxsfb);
> +}
> +
> +static void mxsfb_crtc_enable(struct drm_crtc *crtc)
> +{
> +	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
> +
> +	mxsfb_crtc_mode_set_nofb(crtc);
> +	mxsfb_enable_controller(mxsfb);
> +}
> +
> +static void mxsfb_crtc_disable(struct drm_crtc *crtc)
> +{
> +	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
> +
> +	if (!crtc->state->active)
> +		return;
> +
> +	mxsfb_disable_controller(mxsfb);
> +}
> +
> +static int mxsfb_crtc_atomic_check(struct drm_crtc *crtc,
> +				   struct drm_crtc_state *state)
> +{
> +	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
> +	struct drm_display_mode *mode = &state->adjusted_mode;
> +	long rate, clk_rate = mode->clock * 1000;
> +
> +	if (!mode->clock)
> +		return 0;
> +
> +	rate = clk_round_rate(mxsfb->clk, clk_rate);
> +	/* clock required by mode not supported by hardware */
> +	if (rate != clk_rate)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static void mxsfb_crtc_atomic_begin(struct drm_crtc *crtc,
> +				    struct drm_crtc_state *state)
> +{
> +	struct drm_pending_vblank_event *event = crtc->state->event;
> +
> +	if (event) {
> +		crtc->state->event = NULL;
> +
> +		spin_lock_irq(&crtc->dev->event_lock);
> +		if (drm_crtc_vblank_get(crtc) == 0)
> +			drm_crtc_arm_vblank_event(crtc, event);
> +		else
> +			drm_crtc_send_vblank_event(crtc, event);
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +	}
> +}
> +
> +static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = {
> +	.mode_set	= drm_helper_crtc_mode_set,
> +	.mode_set_base	= drm_helper_crtc_mode_set_base,
> +	.mode_set_nofb	= mxsfb_crtc_mode_set_nofb,
> +	.enable		= mxsfb_crtc_enable,
> +	.disable	= mxsfb_crtc_disable,
> +	.prepare	= mxsfb_crtc_disable,
> +	.commit		= mxsfb_crtc_enable,
> +	.atomic_check	= mxsfb_crtc_atomic_check,
> +	.atomic_begin	= mxsfb_crtc_atomic_begin,
> +};

I think this driver is a perfect example for using the recently-merged
drm_simple_kms_helper framework - it will allow you to remove a lot of the
boiler-plate you have here. Please make sure you're using the latest
drm-misc code in linux-next, since that contains an important fix to these
helpers.

There's also some kernel-doc for these new helpers, which should help in
converting your driver:

https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms-helpers.html#simple-kms-helper-reference

> +
> +static void mxsfb_plane_atomic_update(struct drm_plane *plane,
> +				      struct drm_plane_state *state)
> +{
> +	struct mxsfb_drm_private *mxsfb = plane->dev->dev_private;
> +	struct drm_gem_cma_object *gem;
> +
> +	if (!plane->state->crtc || !plane->state->fb)
> +		return;
> +
> +	gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
> +
> +	mxsfb_enable_axi_clk(mxsfb);
> +	writel(gem->paddr, mxsfb->base + mxsfb->devdata->next_buf);
> +	mxsfb_disable_axi_clk(mxsfb);
> +}
> +
> +static const struct drm_plane_helper_funcs mxsfb_plane_helper_funcs = {
> +	.atomic_update	= mxsfb_plane_atomic_update,
> +};
> +
> +static void mxsfb_plane_destroy(struct drm_plane *plane)
> +{
> +	drm_plane_helper_disable(plane);
> +	drm_plane_cleanup(plane);
> +}
> +
> +static const struct drm_plane_funcs mxsfb_plane_funcs = {
> +	.update_plane		= drm_atomic_helper_update_plane,
> +	.disable_plane		= drm_atomic_helper_disable_plane,
> +	.destroy		= mxsfb_plane_destroy,
> +	.reset			= drm_atomic_helper_plane_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
> +};
> +
> +static struct drm_plane *mxsfb_plane_init(struct drm_device *drm)
> +{
> +	struct mxsfb_drm_private *mxsfb = drm->dev_private;
> +	struct drm_plane *plane = NULL;
> +	u32 formats[ARRAY_SIZE(supported_formats)], i;
> +	int ret;
> +
> +	plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
> +	if (!plane)
> +		return ERR_PTR(-ENOMEM);
> +
> +	for (i = 0; i < ARRAY_SIZE(supported_formats); i++)
> +		formats[i] = supported_formats[i].fourcc;
> +
> +	ret = drm_universal_plane_init(drm, plane, 0xff, &mxsfb_plane_funcs,
> +				       formats, ARRAY_SIZE(formats),
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret) {
> +		devm_kfree(drm->dev, plane);
> +		return ERR_PTR(ret);
> +	}
> +
> +	drm_plane_helper_add(plane, &mxsfb_plane_helper_funcs);
> +	mxsfb->plane = plane;
> +
> +	return plane;
> +}
> +
> +static void mxsfb_crtc_cleanup(struct drm_crtc *crtc)
> +{
> +	mxsfb_crtc_disable(crtc);
> +	drm_crtc_cleanup(crtc);
> +}
> +
> +static const struct drm_crtc_funcs mxsfb_crtc_funcs = {
> +	.destroy		= mxsfb_crtc_cleanup,
> +	.set_config		= drm_atomic_helper_set_config,
> +	.page_flip		= drm_atomic_helper_page_flip,
> +	.reset			= drm_atomic_helper_crtc_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_crtc_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +int mxsfb_setup_crtc(struct drm_device *drm)
> +{
> +	struct mxsfb_drm_private *mxsfb = drm->dev_private;
> +	struct drm_plane *primary;
> +	int ret;
> +
> +	primary = mxsfb_plane_init(drm);
> +	if (IS_ERR(primary))
> +		return PTR_ERR(primary);
> +
> +	ret = drm_crtc_init_with_planes(drm, &mxsfb->crtc, primary, NULL,
> +					&mxsfb_crtc_funcs, NULL);
> +	if (ret) {
> +		mxsfb_plane_destroy(primary);
> +		devm_kfree(drm->dev, primary);
> +		return ret;
> +	}
> +
> +	drm_crtc_helper_add(&mxsfb->crtc, &mxsfb_crtc_helper_funcs);
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> new file mode 100644
> index 0000000..bbc820f
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -0,0 +1,324 @@
> +/*
> + * Copyright (C) 2016 Marek Vasut <marex@denx.de>
> + *
> + * This code is based on drivers/video/fbdev/mxsfb.c :
> + * Copyright (C) 2010 Juergen Beisert, Pengutronix
> + * Copyright (C) 2008-2009 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright (C) 2008 Embedded Alley Solutions, Inc All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/list.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/pm_runtime.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> +
> +#include "mxsfb_drv.h"
> +#include "mxsfb_regs.h"
> +
> +enum mxsfb_devtype {
> +	MXSFB_V3,
> +	MXSFB_V4,
> +};
> +
> +static const struct mxsfb_devdata mxsfb_devdata[] = {
> +	[MXSFB_V3] = {
> +		.transfer_count	= LCDC_V3_TRANSFER_COUNT,
> +		.cur_buf	= LCDC_V3_CUR_BUF,
> +		.next_buf	= LCDC_V3_NEXT_BUF,
> +		.debug0		= LCDC_V3_DEBUG0,
> +		.hs_wdth_mask	= 0xff,
> +		.hs_wdth_shift	= 24,
> +		.ipversion	= 3,
> +	},
> +	[MXSFB_V4] = {
> +		.transfer_count	= LCDC_V4_TRANSFER_COUNT,
> +		.cur_buf	= LCDC_V4_CUR_BUF,
> +		.next_buf	= LCDC_V4_NEXT_BUF,
> +		.debug0		= LCDC_V4_DEBUG0,
> +		.hs_wdth_mask	= 0x3fff,
> +		.hs_wdth_shift	= 18,
> +		.ipversion	= 4,
> +	},
> +};
> +
> +static void mxsfb_fb_output_poll_changed(struct drm_device *drm)
> +{
> +	struct mxsfb_drm_private *mxsfb = drm->dev_private;
> +
> +	if (mxsfb->fbdev) {
> +		drm_fbdev_cma_hotplug_event(mxsfb->fbdev);
> +	} else {
> +		mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
> +					drm->mode_config.num_crtc,
> +					drm->mode_config.num_connector);
> +		if (IS_ERR(mxsfb->fbdev))
> +			mxsfb->fbdev = NULL;
> +	}
> +}

There's patches from Thierry Redding to make delayed fbdev init supported
in the fbdev helpers themselves (instead of reinventing it in every driver
like you do here). Please help get those patches landed&reviewed, I'll
ping Thierry to give you the relevant pointers.

> +
> +static int mxsfb_atomic_commit(struct drm_device *dev,
> +			       struct drm_atomic_state *state, bool nonblock)
> +{
> +	return drm_atomic_helper_commit(dev, state, false);

Atomic helpers support async commit nowadays, no need any more for this
hack.

> +}
> +
> +static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
> +	.fb_create		= drm_fb_cma_create,
> +	.output_poll_changed	= mxsfb_fb_output_poll_changed,
> +	.atomic_check		= drm_atomic_helper_check,
> +	.atomic_commit		= mxsfb_atomic_commit,
> +};
> +
> +static int mxsfb_load(struct drm_device *drm, unsigned long flags)
> +{
> +	struct platform_device *pdev = to_platform_device(drm->dev);
> +	struct mxsfb_drm_private *mxsfb;
> +	struct resource *res;
> +	int ret;
> +
> +	mxsfb = devm_kzalloc(&pdev->dev, sizeof(*mxsfb), GFP_KERNEL);
> +	if (!mxsfb)
> +		return -ENOMEM;
> +
> +	drm->dev_private = mxsfb;
> +	mxsfb->devdata = &mxsfb_devdata[pdev->id_entry->driver_data];
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mxsfb->base = devm_ioremap_resource(drm->dev, res);
> +	if (IS_ERR(mxsfb->base))
> +		return PTR_ERR(mxsfb->base);
> +
> +	mxsfb->clk = devm_clk_get(drm->dev, NULL);
> +	if (IS_ERR(mxsfb->clk))
> +		return PTR_ERR(mxsfb->clk);
> +
> +	mxsfb->clk_axi = devm_clk_get(drm->dev, "axi");
> +	if (IS_ERR(mxsfb->clk_axi))
> +		mxsfb->clk_axi = NULL;
> +
> +	mxsfb->clk_disp_axi = devm_clk_get(drm->dev, "disp_axi");
> +	if (IS_ERR(mxsfb->clk_disp_axi))
> +		mxsfb->clk_disp_axi = NULL;
> +
> +	ret = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32));
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_enable(drm->dev);
> +
> +	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> +	if (ret < 0) {
> +		dev_err(drm->dev, "Failed to initialise vblank\n");
> +		goto err_vblank;
> +	}
> +
> +	/* Modeset init */
> +	drm_mode_config_init(drm);
> +
> +	ret = mxsfb_create_outputs(drm);
> +	if (ret < 0) {
> +		dev_err(drm->dev, "Failed to create outputs\n");
> +		goto err_vblank;
> +	}
> +
> +	ret = mxsfb_setup_crtc(drm);
> +	if (ret < 0) {
> +		dev_err(drm->dev, "Failed to create crtc\n");
> +		goto err_vblank;
> +	}
> +
> +	drm->mode_config.min_width	= MXSFB_MIN_XRES;
> +	drm->mode_config.min_height	= MXSFB_MIN_YRES;
> +	drm->mode_config.max_width	= MXSFB_MAX_XRES;
> +	drm->mode_config.max_height	= MXSFB_MAX_YRES;
> +	drm->mode_config.funcs		= &mxsfb_mode_config_funcs;
> +
> +	drm_mode_config_reset(drm);
> +
> +	platform_set_drvdata(pdev, drm);
> +
> +	drm_kms_helper_poll_init(drm);
> +
> +	drm_helper_hpd_irq_event(drm);
> +
> +	return 0;
> +
> +err_vblank:
> +	pm_runtime_disable(drm->dev);
> +
> +	return ret;
> +}
> +
> +static void mxsfb_unload(struct drm_device *drm)
> +{
> +	struct mxsfb_drm_private *mxsfb = drm->dev_private;
> +
> +	if (mxsfb->fbdev)
> +		drm_fbdev_cma_fini(mxsfb->fbdev);
> +
> +	drm_kms_helper_poll_fini(drm);
> +	drm_mode_config_cleanup(drm);
> +	drm_vblank_cleanup(drm);
> +
> +	drm->dev_private = NULL;
> +
> +	pm_runtime_disable(drm->dev);
> +}
> +
> +static void mxsfb_lastclose(struct drm_device *drm)
> +{
> +	struct mxsfb_drm_private *mxsfb = drm->dev_private;
> +
> +	drm_fbdev_cma_restore_mode(mxsfb->fbdev);
> +}
> +
> +static const struct file_operations fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= drm_open,
> +	.release	= drm_release,
> +	.unlocked_ioctl	= drm_ioctl,
> +#ifdef CONFIG_COMPAT
> +	.compat_ioctl	= drm_compat_ioctl,
> +#endif
> +	.poll		= drm_poll,
> +	.read		= drm_read,
> +	.llseek		= noop_llseek,
> +	.mmap		= drm_gem_cma_mmap,
> +};
> +
> +static struct drm_driver mxsfb_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET |
> +				  DRIVER_PRIME | DRIVER_ATOMIC,
> +	.lastclose		= mxsfb_lastclose,
> +	.gem_free_object	= drm_gem_cma_free_object,
> +	.gem_vm_ops		= &drm_gem_cma_vm_ops,
> +	.dumb_create		= drm_gem_cma_dumb_create,
> +	.dumb_map_offset	= drm_gem_cma_dumb_map_offset,
> +	.dumb_destroy		= drm_gem_dumb_destroy,
> +	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
> +	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
> +	.gem_prime_export	= drm_gem_prime_export,
> +	.gem_prime_import	= drm_gem_prime_import,
> +	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
> +	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> +	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
> +	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
> +	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
> +	.fops	= &fops,
> +	.name	= "mxsfb-drm",
> +	.desc	= "MXSFB Controller DRM",
> +	.date	= "20160824",
> +	.major	= 1,
> +	.minor	= 0,
> +};
> +
> +static const struct platform_device_id mxsfb_devtype[] = {
> +	{ .name = "imx23-fb", .driver_data = MXSFB_V3, },
> +	{ .name = "imx28-fb", .driver_data = MXSFB_V4, },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
> +
> +static const struct of_device_id mxsfb_dt_ids[] = {
> +	{ .compatible = "fsl,imx23-lcdif", .data = &mxsfb_devtype[0], },
> +	{ .compatible = "fsl,imx28-lcdif", .data = &mxsfb_devtype[1], },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mxsfb_dt_ids);
> +
> +static int mxsfb_probe(struct platform_device *pdev)
> +{
> +	struct drm_device *drm;
> +	const struct of_device_id *of_id =
> +			of_match_device(mxsfb_dt_ids, &pdev->dev);
> +	int ret;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	if (of_id)
> +		pdev->id_entry = of_id->data;
> +
> +	drm = drm_dev_alloc(&mxsfb_driver, &pdev->dev);
> +	if (!drm)
> +		return -ENOMEM;
> +
> +	ret = mxsfb_load(drm, 0);
> +	if (ret)
> +		goto err_free;
> +
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		goto err_unload;
> +
> +	ret = drm_connector_register_all(drm);

No need anymore to call connector_register/unregister_all yourself. Please
remove here and in the unload code.

> +	if (ret) {
> +		DRM_ERROR("Failed to bind all components\n");
> +		goto err_unregister;
> +	}
> +
> +	return 0;
> +
> +err_unregister:
> +	drm_dev_unregister(drm);
> +err_unload:
> +	mxsfb_unload(drm);
> +err_free:
> +	drm_dev_unref(drm);
> +
> +	return ret;
> +}
> +
> +static int mxsfb_remove(struct platform_device *pdev)
> +{
> +	struct drm_device *drm = platform_get_drvdata(pdev);
> +
> +	mutex_lock(&drm->mode_config.mutex);
> +	drm_connector_unregister_all(drm);
> +	mutex_unlock(&drm->mode_config.mutex);
> +
> +	drm_dev_unregister(drm);
> +	mxsfb_unload(drm);
> +	drm_dev_unref(drm);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mxsfb_platform_driver = {
> +	.probe		= mxsfb_probe,
> +	.remove		= mxsfb_remove,
> +	.id_table	= mxsfb_devtype,
> +	.driver	= {
> +		.name		= "mxsfb",
> +		.of_match_table	= mxsfb_dt_ids,
> +	},
> +};
> +
> +module_platform_driver(mxsfb_platform_driver);
> +
> +MODULE_AUTHOR("Marek Vasut <marex@denx.de>");
> +MODULE_DESCRIPTION("Freescale MXS DRM/KMS driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> new file mode 100644
> index 0000000..afa1e34
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright (C) 2016 Marek Vasut <marex@denx.de>
> + *
> + * i.MX23/i.MX28/i.MX6SX MXSFB LCD controller driver.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __MXSFB_DRV_H__
> +#define __MXSFB_DRV_H__
> +
> +struct mxsfb_devdata {
> +	unsigned int	 transfer_count;
> +	unsigned int	 cur_buf;
> +	unsigned int	 next_buf;
> +	unsigned int	 debug0;
> +	unsigned int	 hs_wdth_mask;
> +	unsigned int	 hs_wdth_shift;
> +	unsigned int	 ipversion;
> +};
> +
> +struct mxsfb_rgb_output {
> +	struct drm_connector	connector;
> +	struct drm_encoder	encoder;
> +	struct drm_panel	*panel;
> +};
> +
> +struct mxsfb_drm_private {
> +	struct clk		*clk;
> +	struct clk		*clk_axi;
> +	struct clk		*clk_disp_axi;
> +	void __iomem		*base;	/* registers */
> +
> +	struct drm_fbdev_cma	*fbdev;
> +	struct drm_crtc		crtc;
> +	struct drm_plane	*plane;
> +	struct drm_atomic_state	*state;
> +
> +	struct mxsfb_rgb_output	output;
> +
> +	const struct mxsfb_devdata *devdata;
> +	bool enabled;
> +};
> +
> +#define crtc_to_mxsfb_priv(x)	container_of(x, struct mxsfb_drm_private, crtc)
> +
> +int mxsfb_setup_crtc(struct drm_device *dev);
> +int mxsfb_create_outputs(struct drm_device *dev);
> +void mxsfb_set_scanout(struct mxsfb_drm_private *mxsfb);
> +
> +#endif /* __MXSFB_DRV_H__ */
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_out.c b/drivers/gpu/drm/mxsfb/mxsfb_out.c
> new file mode 100644
> index 0000000..ffc4b29
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_out.c
> @@ -0,0 +1,264 @@
> +/*
> + * Copyright (C) 2016 Marek Vasut <marex@denx.de>
> + *
> + * This code is based on drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c :
> + * Copyright (C) 2014 Traphandler
> + * Copyright (C) 2014 Free Electrons
> + * Copyright (C) 2014 Atmel
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/of_graph.h>
> +
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_plane_helper.h>
> +#include <drm/drmP.h>
> +
> +#include "mxsfb_drv.h"
> +
> +static inline struct mxsfb_rgb_output *
> +drm_connector_to_mxsfb_rgb_output(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct mxsfb_rgb_output,
> +			    connector);
> +}
> +
> +static inline struct mxsfb_rgb_output *
> +drm_encoder_to_mxsfb_rgb_output(struct drm_encoder *encoder)
> +{
> +	return container_of(encoder, struct mxsfb_rgb_output, encoder);
> +}
> +
> +static void mxsfb_rgb_encoder_enable(struct drm_encoder *encoder)
> +{
> +	struct mxsfb_rgb_output *rgb =
> +			drm_encoder_to_mxsfb_rgb_output(encoder);
> +
> +	if (rgb->panel) {
> +		drm_panel_prepare(rgb->panel);
> +		drm_panel_enable(rgb->panel);
> +	}
> +}
> +
> +static void mxsfb_rgb_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct mxsfb_rgb_output *rgb =
> +			drm_encoder_to_mxsfb_rgb_output(encoder);
> +
> +	if (rgb->panel) {
> +		drm_panel_disable(rgb->panel);
> +		drm_panel_unprepare(rgb->panel);
> +	}
> +}
> +
> +static const struct
> +drm_encoder_helper_funcs mxsfb_panel_encoder_helper_funcs = {
> +	.disable	= mxsfb_rgb_encoder_disable,
> +	.enable		= mxsfb_rgb_encoder_enable,
> +};
> +
> +static void mxsfb_rgb_encoder_destroy(struct drm_encoder *encoder)
> +{
> +	drm_encoder_cleanup(encoder);
> +	memset(encoder, 0, sizeof(*encoder));
> +}
> +
> +static const struct drm_encoder_funcs mxsfb_panel_encoder_funcs = {
> +	.destroy = mxsfb_rgb_encoder_destroy,
> +};
> +
> +static int mxsfb_panel_get_modes(struct drm_connector *connector)
> +{
> +	struct mxsfb_rgb_output *rgb =
> +			drm_connector_to_mxsfb_rgb_output(connector);
> +
> +	if (rgb->panel)
> +		return rgb->panel->funcs->get_modes(rgb->panel);
> +
> +	return 0;
> +}
> +
> +static int mxsfb_rgb_mode_valid(struct drm_connector *connector,
> +				struct drm_display_mode *mode)
> +{
> +	return 0;
> +}
> +
> +static struct drm_encoder *
> +mxsfb_rgb_best_encoder(struct drm_connector *connector)
> +{
> +	struct mxsfb_rgb_output *rgb =
> +			drm_connector_to_mxsfb_rgb_output(connector);
> +
> +	return &rgb->encoder;
> +}
> +
> +static const struct
> +drm_connector_helper_funcs mxsfb_panel_connector_helper_funcs = {
> +	.get_modes = mxsfb_panel_get_modes,
> +	.mode_valid = mxsfb_rgb_mode_valid,
> +	.best_encoder = mxsfb_rgb_best_encoder,
> +};
> +
> +static enum drm_connector_status
> +mxsfb_panel_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct mxsfb_rgb_output *rgb =
> +			drm_connector_to_mxsfb_rgb_output(connector);
> +
> +	if (rgb->panel)
> +		return connector_status_connected;
> +
> +	return connector_status_disconnected;
> +}
> +
> +static void mxsfb_panel_connector_destroy(struct drm_connector *connector)
> +{
> +	struct mxsfb_rgb_output *rgb =
> +			drm_connector_to_mxsfb_rgb_output(connector);
> +
> +	if (rgb->panel)
> +		drm_panel_detach(rgb->panel);
> +
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs mxsfb_panel_connector_funcs = {
> +	.dpms			= drm_atomic_helper_connector_dpms,
> +	.detect			= mxsfb_panel_connector_detect,
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= mxsfb_panel_connector_destroy,
> +	.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 mxsfb_check_endpoint(struct drm_device *drm,
> +				const struct of_endpoint *ep)
> +{
> +	struct device_node *np;
> +	void *obj;
> +
> +	np = of_graph_get_remote_port_parent(ep->local_node);
> +
> +	obj = of_drm_find_panel(np);
> +	if (!obj)
> +		obj = of_drm_find_bridge(np);
> +
> +	of_node_put(np);
> +
> +	return obj ? 0 : -EPROBE_DEFER;
> +}
> +
> +static int mxsfb_attach_endpoint(struct drm_device *drm,
> +				 const struct of_endpoint *ep)
> +{
> +	struct mxsfb_drm_private *mxsfb = drm->dev_private;
> +	struct mxsfb_rgb_output *output = &mxsfb->output;
> +	struct device_node *np;
> +	struct drm_panel *panel;
> +	struct drm_bridge *bridge;
> +	int ret;
> +
> +	drm_encoder_helper_add(&output->encoder,
> +			       &mxsfb_panel_encoder_helper_funcs);
> +	ret = drm_encoder_init(drm, &output->encoder,
> +			       &mxsfb_panel_encoder_funcs,
> +			       DRM_MODE_ENCODER_NONE, NULL);
> +	if (ret)
> +		return ret;
> +
> +	output->encoder.possible_crtcs = 0x1;
> +
> +	np = of_graph_get_remote_port_parent(ep->local_node);
> +
> +	ret = -EPROBE_DEFER;
> +
> +	panel = of_drm_find_panel(np);
> +	if (panel) {
> +		of_node_put(np);
> +		output->connector.dpms = DRM_MODE_DPMS_OFF;
> +		output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
> +		drm_connector_helper_add(&output->connector,
> +				&mxsfb_panel_connector_helper_funcs);
> +		ret = drm_connector_init(drm, &output->connector,
> +					 &mxsfb_panel_connector_funcs,
> +					 DRM_MODE_CONNECTOR_Unknown);
> +		if (ret)
> +			goto err_encoder_cleanup;
> +
> +		drm_mode_connector_attach_encoder(&output->connector,
> +						  &output->encoder);
> +
> +		ret = drm_panel_attach(panel, &output->connector);
> +		if (ret) {
> +			drm_connector_cleanup(&output->connector);
> +			goto err_encoder_cleanup;
> +		}
> +
> +		output->panel = panel;
> +
> +		return 0;
> +	}
> +
> +	bridge = of_drm_find_bridge(np);
> +	of_node_put(np);
> +
> +	if (bridge) {
> +		output->encoder.bridge = bridge;
> +		bridge->encoder = &output->encoder;
> +		ret = drm_bridge_attach(drm, bridge);
> +		if (!ret)
> +			return 0;
> +	}
> +
> +err_encoder_cleanup:
> +	drm_encoder_cleanup(&output->encoder);
> +
> +	return ret;
> +}
> +
> +int mxsfb_create_outputs(struct drm_device *drm)
> +{
> +	struct device_node *ep_np = NULL;
> +	struct of_endpoint ep;
> +	int ret;
> +
> +	for_each_endpoint_of_node(drm->dev->of_node, ep_np) {
> +		ret = of_graph_parse_endpoint(ep_np, &ep);
> +		if (!ret)
> +			ret = mxsfb_check_endpoint(drm, &ep);
> +
> +		if (ret) {
> +			of_node_put(ep_np);
> +			return ret;
> +		}
> +	}
> +
> +	for_each_endpoint_of_node(drm->dev->of_node, ep_np) {
> +		ret = of_graph_parse_endpoint(ep_np, &ep);
> +		if (!ret)
> +			ret = mxsfb_attach_endpoint(drm, &ep);
> +
> +		if (ret) {
> +			of_node_put(ep_np);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> new file mode 100644
> index 0000000..e9a006a
> --- /dev/null
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
> @@ -0,0 +1,112 @@
> +/*
> + * Copyright (C) 2010 Juergen Beisert, Pengutronix
> + * Copyright (C) 2016 Marek Vasut <marex@denx.de>
> + *
> + * i.MX23/i.MX28/i.MX6SX MXSFB LCD controller driver.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __MXSFB_REGS_H__
> +#define __MXSFB_REGS_H__
> +
> +#define REG_SET	4
> +#define REG_CLR	8
> +
> +#define LCDC_CTRL			0x00
> +#define LCDC_CTRL1			0x10
> +#define LCDC_V3_TRANSFER_COUNT		0x20
> +#define LCDC_V4_TRANSFER_COUNT		0x30
> +#define LCDC_V4_CUR_BUF			0x40
> +#define LCDC_V4_NEXT_BUF		0x50
> +#define LCDC_V3_CUR_BUF			0x30
> +#define LCDC_V3_NEXT_BUF		0x40
> +#define LCDC_VDCTRL0			0x70
> +#define LCDC_VDCTRL1			0x80
> +#define LCDC_VDCTRL2			0x90
> +#define LCDC_VDCTRL3			0xa0
> +#define LCDC_VDCTRL4			0xb0
> +#define LCDC_V4_DEBUG0			0x1d0
> +#define LCDC_V3_DEBUG0			0x1f0
> +
> +#define CTRL_SFTRST			(1 << 31)
> +#define CTRL_CLKGATE			(1 << 30)
> +#define CTRL_BYPASS_COUNT		(1 << 19)
> +#define CTRL_VSYNC_MODE			(1 << 18)
> +#define CTRL_DOTCLK_MODE		(1 << 17)
> +#define CTRL_DATA_SELECT		(1 << 16)
> +#define CTRL_SET_BUS_WIDTH(x)		(((x) & 0x3) << 10)
> +#define CTRL_GET_BUS_WIDTH(x)		(((x) >> 10) & 0x3)
> +#define CTRL_SET_WORD_LENGTH(x)		(((x) & 0x3) << 8)
> +#define CTRL_GET_WORD_LENGTH(x)		(((x) >> 8) & 0x3)
> +#define CTRL_MASTER			(1 << 5)
> +#define CTRL_DF16			(1 << 3)
> +#define CTRL_DF18			(1 << 2)
> +#define CTRL_DF24			(1 << 1)
> +#define CTRL_RUN			(1 << 0)
> +
> +#define CTRL1_FIFO_CLEAR		(1 << 21)
> +#define CTRL1_SET_BYTE_PACKAGING(x)	(((x) & 0xf) << 16)
> +#define CTRL1_GET_BYTE_PACKAGING(x)	(((x) >> 16) & 0xf)
> +
> +#define TRANSFER_COUNT_SET_VCOUNT(x)	(((x) & 0xffff) << 16)
> +#define TRANSFER_COUNT_GET_VCOUNT(x)	(((x) >> 16) & 0xffff)
> +#define TRANSFER_COUNT_SET_HCOUNT(x)	((x) & 0xffff)
> +#define TRANSFER_COUNT_GET_HCOUNT(x)	((x) & 0xffff)
> +
> +#define VDCTRL0_ENABLE_PRESENT		(1 << 28)
> +#define VDCTRL0_VSYNC_ACT_HIGH		(1 << 27)
> +#define VDCTRL0_HSYNC_ACT_HIGH		(1 << 26)
> +#define VDCTRL0_DOTCLK_ACT_FALLING	(1 << 25)
> +#define VDCTRL0_ENABLE_ACT_HIGH		(1 << 24)
> +#define VDCTRL0_VSYNC_PERIOD_UNIT	(1 << 21)
> +#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	(1 << 20)
> +#define VDCTRL0_HALF_LINE		(1 << 19)
> +#define VDCTRL0_HALF_LINE_MODE		(1 << 18)
> +#define VDCTRL0_SET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
> +#define VDCTRL0_GET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
> +
> +#define VDCTRL2_SET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
> +#define VDCTRL2_GET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
> +
> +#define VDCTRL3_MUX_SYNC_SIGNALS	(1 << 29)
> +#define VDCTRL3_VSYNC_ONLY		(1 << 28)
> +#define SET_HOR_WAIT_CNT(x)		(((x) & 0xfff) << 16)
> +#define GET_HOR_WAIT_CNT(x)		(((x) >> 16) & 0xfff)
> +#define SET_VERT_WAIT_CNT(x)		((x) & 0xffff)
> +#define GET_VERT_WAIT_CNT(x)		((x) & 0xffff)
> +
> +#define VDCTRL4_SET_DOTCLK_DLY(x)	(((x) & 0x7) << 29) /* v4 only */
> +#define VDCTRL4_GET_DOTCLK_DLY(x)	(((x) >> 29) & 0x7) /* v4 only */
> +#define VDCTRL4_SYNC_SIGNALS_ON		(1 << 18)
> +#define SET_DOTCLK_H_VALID_DATA_CNT(x)	((x) & 0x3ffff)
> +
> +#define DEBUG0_HSYNC			(1 < 26)
> +#define DEBUG0_VSYNC			(1 < 25)
> +
> +#define MXSFB_MIN_XRES			120
> +#define MXSFB_MIN_YRES			120
> +#define MXSFB_MAX_XRES			0xffff
> +#define MXSFB_MAX_YRES			0xffff
> +
> +#define RED 0
> +#define GREEN 1
> +#define BLUE 2
> +#define TRANSP 3
> +
> +#define STMLCDIF_8BIT  1 /* pixel data bus to the display is of 8 bit width */
> +#define STMLCDIF_16BIT 0 /* pixel data bus to the display is of 16 bit width */
> +#define STMLCDIF_18BIT 2 /* pixel data bus to the display is of 18 bit width */
> +#define STMLCDIF_24BIT 3 /* pixel data bus to the display is of 24 bit width */
> +
> +#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
> +#define MXSFB_SYNC_DOTCLK_FALLING_ACT	(1 << 7) /* negative edge sampling */
> +
> +#endif /* __MXSFB_REGS_H__ */
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Marek Vasut Sept. 25, 2016, 7:26 p.m. UTC | #2
On 08/28/2016 06:44 PM, Daniel Vetter wrote:
> On Fri, Aug 26, 2016 at 04:27:42PM +0200, Marek Vasut wrote:
>> Add new driver for the MXSFB controller found in i.MX23/28/6SX .
>> The MXSFB controller is a simple framebuffer controller with one
>> parallel LCD output. Unlike the MXSFB fbdev driver that is used
>> on these systems now, this driver uses the DRM/KMS framework.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: Shawn Guo <shawnguo@kernel.org>

Hi, sorry for the late reply.

[...]
>> +	switch (format->fourcc) {
>> +	case DRM_FORMAT_RGB565:
>> +		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
>> +		ctrl |= CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT);
>> +		ctrl |= CTRL_SET_WORD_LENGTH(0);
>> +		writel(CTRL1_SET_BYTE_PACKAGING(0xf), mxsfb->base + LCDC_CTRL1);
>> +		break;
>> +	case DRM_FORMAT_XRGB8888:
>> +		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
>> +		ctrl |= CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT);
>> +		ctrl |= CTRL_SET_WORD_LENGTH(3);
>> +		/* Do not use packed pixels = one pixel per word instead */
>> +		writel(CTRL1_SET_BYTE_PACKAGING(0x7), mxsfb->base + LCDC_CTRL1);
>> +		break;
>> +	default:
>> +		dev_err(drm->dev, "Unhandled color format %s\n",
>> +			format->name);
>> +		return -EINVAL;
> 
> You need to check such failures in the atomic_check code, doing it only in
> atomic_commit (or anything called from that) is way too late.
> 
> If you do check this already (simply restrict the list of support fourcc
> codes to only the 2 above), then you can do a WARN_ON + return; and change
> the return value from int to void here.

I now switched to drm_simple_kms_.* , which does that in the check, so
fixed.

>> +	}
>> +
>> +	writel(ctrl, mxsfb->base + LCDC_CTRL);
>> +	return 0;
>> +}

[...]

>> +static void mxsfb_crtc_mode_set_nofb(struct drm_crtc *crtc)
>> +{
>> +	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
>> +	struct drm_display_mode *m = &crtc->state->adjusted_mode;
>> +	const u32 bus_flags = mxsfb->output.connector.display_info.bus_flags;
>> +	u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
>> +	bool reenable = false;
>> +	int err;
>> +
>> +	/*
>> +	 * It seems, you can't re-program the controller if it is still
>> +	 * running. This may lead to shifted pictures (FIFO issue?), so
>> +	 * first stop the controller and drain its FIFOs.
>> +	 */
>> +	if (mxsfb->enabled) {
>> +		reenable = true;
>> +		mxsfb_disable_controller(mxsfb);
>> +	}
> 
> The atomic core should keep perfect track of the state of your controller
> and never asky ou to re-enable it when it's already enabled. Please remove
> this code (and the ->enabled tracking, it should be redundant).
> 
> If this doesn't work then we have a bug in the atomic core.

What this code does is it temporarily disables the controller if it was
enabled when this function is invoked and re-enables it before exiting
this function. This is needed for this particular controller to
reconfigure it without odd misbehavior of the hardware itself.

Is there a better way ?

>> +
>> +	mxsfb_enable_axi_clk(mxsfb);
>> +
>> +	/* Clear the FIFOs */
>> +	writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET);

[...]

>> +static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = {
>> +	.mode_set	= drm_helper_crtc_mode_set,
>> +	.mode_set_base	= drm_helper_crtc_mode_set_base,
>> +	.mode_set_nofb	= mxsfb_crtc_mode_set_nofb,
>> +	.enable		= mxsfb_crtc_enable,
>> +	.disable	= mxsfb_crtc_disable,
>> +	.prepare	= mxsfb_crtc_disable,
>> +	.commit		= mxsfb_crtc_enable,
>> +	.atomic_check	= mxsfb_crtc_atomic_check,
>> +	.atomic_begin	= mxsfb_crtc_atomic_begin,
>> +};
> 
> I think this driver is a perfect example for using the recently-merged
> drm_simple_kms_helper framework - it will allow you to remove a lot of the
> boiler-plate you have here. Please make sure you're using the latest
> drm-misc code in linux-next, since that contains an important fix to these
> helpers.
> 
> There's also some kernel-doc for these new helpers, which should help in
> converting your driver:
> 
> https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-kms-helpers.html#simple-kms-helper-reference

Done, thanks.

[...]

>> +static void mxsfb_fb_output_poll_changed(struct drm_device *drm)
>> +{
>> +	struct mxsfb_drm_private *mxsfb = drm->dev_private;
>> +
>> +	if (mxsfb->fbdev) {
>> +		drm_fbdev_cma_hotplug_event(mxsfb->fbdev);
>> +	} else {
>> +		mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
>> +					drm->mode_config.num_crtc,
>> +					drm->mode_config.num_connector);
>> +		if (IS_ERR(mxsfb->fbdev))
>> +			mxsfb->fbdev = NULL;
>> +	}
>> +}
> 
> There's patches from Thierry Redding to make delayed fbdev init supported
> in the fbdev helpers themselves (instead of reinventing it in every driver
> like you do here). Please help get those patches landed&reviewed, I'll
> ping Thierry to give you the relevant pointers.

It seems I won't even need this, since my output is not polled and
doesn't change. I moved the drm_fbdev_cma_init() into the mxsfb_load()
function.

>> +
>> +static int mxsfb_atomic_commit(struct drm_device *dev,
>> +			       struct drm_atomic_state *state, bool nonblock)
>> +{
>> +	return drm_atomic_helper_commit(dev, state, false);
> 
> Atomic helpers support async commit nowadays, no need any more for this
> hack.

I had to add the same fence check as imx drm driver, so this function
grew in V2.

>> +}
>> +
>> +static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
>> +	.fb_create		= drm_fb_cma_create,
>> +	.output_poll_changed	= mxsfb_fb_output_poll_changed,
>> +	.atomic_check		= drm_atomic_helper_check,
>> +	.atomic_commit		= mxsfb_atomic_commit,
>> +};

[...]

>> +static int mxsfb_probe(struct platform_device *pdev)
>> +{
>> +	struct drm_device *drm;
>> +	const struct of_device_id *of_id =
>> +			of_match_device(mxsfb_dt_ids, &pdev->dev);
>> +	int ret;
>> +
>> +	if (!pdev->dev.of_node)
>> +		return -ENODEV;
>> +
>> +	if (of_id)
>> +		pdev->id_entry = of_id->data;
>> +
>> +	drm = drm_dev_alloc(&mxsfb_driver, &pdev->dev);
>> +	if (!drm)
>> +		return -ENOMEM;
>> +
>> +	ret = mxsfb_load(drm, 0);
>> +	if (ret)
>> +		goto err_free;
>> +
>> +	ret = drm_dev_register(drm, 0);
>> +	if (ret)
>> +		goto err_unload;
>> +
>> +	ret = drm_connector_register_all(drm);
> 
> No need anymore to call connector_register/unregister_all yourself. Please
> remove here and in the unload code.

Done, dropped.

>> +	if (ret) {
>> +		DRM_ERROR("Failed to bind all components\n");
>> +		goto err_unregister;
>> +	}
>> +
>> +	return 0;

[...]

Thanks!
Daniel Vetter Sept. 25, 2016, 8:29 p.m. UTC | #3
On Sun, Sep 25, 2016 at 9:26 PM, Marek Vasut <marex@denx.de> wrote:
> On 08/28/2016 06:44 PM, Daniel Vetter wrote:
>> On Fri, Aug 26, 2016 at 04:27:42PM +0200, Marek Vasut wrote:
>>> +static void mxsfb_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>> +{
>>> +    struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
>>> +    struct drm_display_mode *m = &crtc->state->adjusted_mode;
>>> +    const u32 bus_flags = mxsfb->output.connector.display_info.bus_flags;
>>> +    u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
>>> +    bool reenable = false;
>>> +    int err;
>>> +
>>> +    /*
>>> +     * It seems, you can't re-program the controller if it is still
>>> +     * running. This may lead to shifted pictures (FIFO issue?), so
>>> +     * first stop the controller and drain its FIFOs.
>>> +     */
>>> +    if (mxsfb->enabled) {
>>> +            reenable = true;
>>> +            mxsfb_disable_controller(mxsfb);
>>> +    }
>>
>> The atomic core should keep perfect track of the state of your controller
>> and never asky ou to re-enable it when it's already enabled. Please remove
>> this code (and the ->enabled tracking, it should be redundant).
>>
>> If this doesn't work then we have a bug in the atomic core.
>
> What this code does is it temporarily disables the controller if it was
> enabled when this function is invoked and re-enables it before exiting
> this function. This is needed for this particular controller to
> reconfigure it without odd misbehavior of the hardware itself.
>
> Is there a better way ?

Atomic helpers shouldn't ever call your ->mode_set_nofb hook when the
crtc is on. Instead they will do exactly what you're doing here
already, and first shut down the entire pipeline (crtc and encoders),
then call ->mode_set_nofb, then re-enable again.

This code shouldn't be needed at all (or there's a bug somewhere in
the helpers ..).

>>> +static void mxsfb_fb_output_poll_changed(struct drm_device *drm)
>>> +{
>>> +    struct mxsfb_drm_private *mxsfb = drm->dev_private;
>>> +
>>> +    if (mxsfb->fbdev) {
>>> +            drm_fbdev_cma_hotplug_event(mxsfb->fbdev);
>>> +    } else {
>>> +            mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
>>> +                                    drm->mode_config.num_crtc,
>>> +                                    drm->mode_config.num_connector);
>>> +            if (IS_ERR(mxsfb->fbdev))
>>> +                    mxsfb->fbdev = NULL;
>>> +    }
>>> +}
>>
>> There's patches from Thierry Redding to make delayed fbdev init supported
>> in the fbdev helpers themselves (instead of reinventing it in every driver
>> like you do here). Please help get those patches landed&reviewed, I'll
>> ping Thierry to give you the relevant pointers.
>
> It seems I won't even need this, since my output is not polled and
> doesn't change. I moved the drm_fbdev_cma_init() into the mxsfb_load()
> function.
>
>>> +
>>> +static int mxsfb_atomic_commit(struct drm_device *dev,
>>> +                           struct drm_atomic_state *state, bool nonblock)
>>> +{
>>> +    return drm_atomic_helper_commit(dev, state, false);
>>
>> Atomic helpers support async commit nowadays, no need any more for this
>> hack.
>
> I had to add the same fence check as imx drm driver, so this function
> grew in V2.

Please check out my reply to Lucas patch. tldr: The fence check should
be done in prepare_fb plane hook, and we should have one
implementation in the cma fb helper library for all drivers. Would be
great if you could create that helper (and switch imx over to it).

Cheers, Daniel
Marek Vasut Sept. 26, 2016, 10:36 a.m. UTC | #4
On 09/25/2016 10:29 PM, Daniel Vetter wrote:
> On Sun, Sep 25, 2016 at 9:26 PM, Marek Vasut <marex@denx.de> wrote:
>> On 08/28/2016 06:44 PM, Daniel Vetter wrote:
>>> On Fri, Aug 26, 2016 at 04:27:42PM +0200, Marek Vasut wrote:
>>>> +static void mxsfb_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>>> +{
>>>> +    struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
>>>> +    struct drm_display_mode *m = &crtc->state->adjusted_mode;
>>>> +    const u32 bus_flags = mxsfb->output.connector.display_info.bus_flags;
>>>> +    u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
>>>> +    bool reenable = false;
>>>> +    int err;
>>>> +
>>>> +    /*
>>>> +     * It seems, you can't re-program the controller if it is still
>>>> +     * running. This may lead to shifted pictures (FIFO issue?), so
>>>> +     * first stop the controller and drain its FIFOs.
>>>> +     */
>>>> +    if (mxsfb->enabled) {
>>>> +            reenable = true;
>>>> +            mxsfb_disable_controller(mxsfb);
>>>> +    }
>>>
>>> The atomic core should keep perfect track of the state of your controller
>>> and never asky ou to re-enable it when it's already enabled. Please remove
>>> this code (and the ->enabled tracking, it should be redundant).
>>>
>>> If this doesn't work then we have a bug in the atomic core.
>>
>> What this code does is it temporarily disables the controller if it was
>> enabled when this function is invoked and re-enables it before exiting
>> this function. This is needed for this particular controller to
>> reconfigure it without odd misbehavior of the hardware itself.
>>
>> Is there a better way ?
> 
> Atomic helpers shouldn't ever call your ->mode_set_nofb hook when the
> crtc is on. Instead they will do exactly what you're doing here
> already, and first shut down the entire pipeline (crtc and encoders),
> then call ->mode_set_nofb, then re-enable again.

I see, thanks for clarifying, that's convenient. This code is dropped.

> This code shouldn't be needed at all (or there's a bug somewhere in
> the helpers ..).

It is not needed indeed, I double-checked now.

>>>> +static void mxsfb_fb_output_poll_changed(struct drm_device *drm)
>>>> +{
>>>> +    struct mxsfb_drm_private *mxsfb = drm->dev_private;
>>>> +
>>>> +    if (mxsfb->fbdev) {
>>>> +            drm_fbdev_cma_hotplug_event(mxsfb->fbdev);
>>>> +    } else {
>>>> +            mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
>>>> +                                    drm->mode_config.num_crtc,
>>>> +                                    drm->mode_config.num_connector);
>>>> +            if (IS_ERR(mxsfb->fbdev))
>>>> +                    mxsfb->fbdev = NULL;
>>>> +    }
>>>> +}
>>>
>>> There's patches from Thierry Redding to make delayed fbdev init supported
>>> in the fbdev helpers themselves (instead of reinventing it in every driver
>>> like you do here). Please help get those patches landed&reviewed, I'll
>>> ping Thierry to give you the relevant pointers.
>>
>> It seems I won't even need this, since my output is not polled and
>> doesn't change. I moved the drm_fbdev_cma_init() into the mxsfb_load()
>> function.
>>
>>>> +
>>>> +static int mxsfb_atomic_commit(struct drm_device *dev,
>>>> +                           struct drm_atomic_state *state, bool nonblock)
>>>> +{
>>>> +    return drm_atomic_helper_commit(dev, state, false);
>>>
>>> Atomic helpers support async commit nowadays, no need any more for this
>>> hack.
>>
>> I had to add the same fence check as imx drm driver, so this function
>> grew in V2.
> 
> Please check out my reply to Lucas patch. tldr: The fence check should
> be done in prepare_fb plane hook, and we should have one
> implementation in the cma fb helper library for all drivers. Would be
> great if you could create that helper (and switch imx over to it).

Done, although what I did might be wrong. I will submit the patches once
I am a bit more confident about that. I have a question -- should the
drm simple kms helper code be augmented with a new parameter to add
this prepare_fb helper into the plane helper functions or should the
drm simple kms helper always call the prepare_fb with the fence part ?

> Cheers, Daniel
>
Daniel Vetter Sept. 26, 2016, 12:11 p.m. UTC | #5
On Mon, Sep 26, 2016 at 12:36 PM, Marek Vasut <marex@denx.de> wrote:
>>>>> +
>>>>> +static int mxsfb_atomic_commit(struct drm_device *dev,
>>>>> +                           struct drm_atomic_state *state, bool nonblock)
>>>>> +{
>>>>> +    return drm_atomic_helper_commit(dev, state, false);
>>>>
>>>> Atomic helpers support async commit nowadays, no need any more for this
>>>> hack.
>>>
>>> I had to add the same fence check as imx drm driver, so this function
>>> grew in V2.
>>
>> Please check out my reply to Lucas patch. tldr: The fence check should
>> be done in prepare_fb plane hook, and we should have one
>> implementation in the cma fb helper library for all drivers. Would be
>> great if you could create that helper (and switch imx over to it).
>
> Done, although what I did might be wrong. I will submit the patches once
> I am a bit more confident about that. I have a question -- should the
> drm simple kms helper code be augmented with a new parameter to add
> this prepare_fb helper into the plane helper functions or should the
> drm simple kms helper always call the prepare_fb with the fence part ?

Great question, not sure what the answer should be tbh. Maybe we need
to wire up prepare_fb/cleanup_fb for simple drm? I think forcing a
default hook of using cma isn't a good idea, since e.g. udl would be a
perfect candidate for simple drm helpers, but that wants plain shmem
gem buffers, not cma gem buffers. Means a notch more boilerplate for
drivers (1 line in the vtable), but I think that's ok.
-Daniel
Marek Vasut Sept. 26, 2016, 12:39 p.m. UTC | #6
On 09/26/2016 02:11 PM, Daniel Vetter wrote:
> On Mon, Sep 26, 2016 at 12:36 PM, Marek Vasut <marex@denx.de> wrote:
>>>>>> +
>>>>>> +static int mxsfb_atomic_commit(struct drm_device *dev,
>>>>>> +                           struct drm_atomic_state *state, bool nonblock)
>>>>>> +{
>>>>>> +    return drm_atomic_helper_commit(dev, state, false);
>>>>>
>>>>> Atomic helpers support async commit nowadays, no need any more for this
>>>>> hack.
>>>>
>>>> I had to add the same fence check as imx drm driver, so this function
>>>> grew in V2.
>>>
>>> Please check out my reply to Lucas patch. tldr: The fence check should
>>> be done in prepare_fb plane hook, and we should have one
>>> implementation in the cma fb helper library for all drivers. Would be
>>> great if you could create that helper (and switch imx over to it).
>>
>> Done, although what I did might be wrong. I will submit the patches once
>> I am a bit more confident about that. I have a question -- should the
>> drm simple kms helper code be augmented with a new parameter to add
>> this prepare_fb helper into the plane helper functions or should the
>> drm simple kms helper always call the prepare_fb with the fence part ?
> 
> Great question, not sure what the answer should be tbh. Maybe we need
> to wire up prepare_fb/cleanup_fb for simple drm? I think forcing a
> default hook of using cma isn't a good idea, since e.g. udl would be a
> perfect candidate for simple drm helpers, but that wants plain shmem
> gem buffers, not cma gem buffers. Means a notch more boilerplate for
> drivers (1 line in the vtable), but I think that's ok.
> -Daniel

OK, makes sense, done.
diff mbox

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c20323..39b16b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7781,6 +7781,12 @@  T:	git git://linuxtv.org/mkrufky/tuners.git
 S:	Maintained
 F:	drivers/media/tuners/mxl5007t.*
 
+MXSFB DRM DRIVER
+M:	Marek Vasut <marex@denx.de>
+S:	Supported
+F:	drivers/gpu/drm/mxsfb/
+F:	Documentation/devicetree/bindings/display/mxsfb-drm.txt
+
 MYRICOM MYRI-10G 10GbE DRIVER (MYRI10GE)
 M:	Hyong-Youb Kim <hykim@myri.com>
 L:	netdev@vger.kernel.org
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index fc35731..5b1e7bc 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -290,3 +290,5 @@  source "drivers/gpu/drm/arc/Kconfig"
 source "drivers/gpu/drm/hisilicon/Kconfig"
 
 source "drivers/gpu/drm/mediatek/Kconfig"
+
+source "drivers/gpu/drm/mxsfb/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index be43afb..cbb638d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -82,3 +82,4 @@  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
 obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
 obj-$(CONFIG_DRM_ARCPGU)+= arc/
 obj-y			+= hisilicon/
+obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig
new file mode 100644
index 0000000..0b6cb59
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/Kconfig
@@ -0,0 +1,18 @@ 
+config DRM_MXS
+	bool
+	help
+	  Choose this option to select drivers for MXS FB devices
+
+config DRM_MXSFB
+	tristate "i.MX23/i.MX28/i.MX6SX MXSFB LCD controller"
+	depends on DRM && OF
+	depends on COMMON_CLK
+	select DRM_MXS
+	select DRM_KMS_HELPER
+	select DRM_KMS_FB_HELPER
+	select DRM_KMS_CMA_HELPER
+	help
+	  Choose this option if you have an i.MX23/i.MX28/i.MX6SX MXSFB
+	  LCD controller.
+
+	  If M is selected the module will be called mxsfb.
diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile
new file mode 100644
index 0000000..857f3a4
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/Makefile
@@ -0,0 +1,2 @@ 
+mxsfb-y := mxsfb_drv.o mxsfb_crtc.o mxsfb_out.o
+obj-$(CONFIG_DRM_MXSFB)	+= mxsfb.o
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
new file mode 100644
index 0000000..5d2e2a1
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c
@@ -0,0 +1,395 @@ 
+/*
+ * Copyright (C) 2016 Marek Vasut <marex@denx.de>
+ *
+ * This code is based on drivers/video/fbdev/mxsfb.c :
+ * Copyright (C) 2010 Juergen Beisert, Pengutronix
+ * Copyright (C) 2008-2009 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2008 Embedded Alley Solutions, Inc All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_plane_helper.h>
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <linux/of_graph.h>
+#include <linux/platform_data/simplefb.h>
+#include <video/videomode.h>
+
+#include "mxsfb_drv.h"
+#include "mxsfb_regs.h"
+
+static void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb)
+{
+	if (mxsfb->clk_axi)
+		clk_prepare_enable(mxsfb->clk_axi);
+}
+
+static void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
+{
+	if (mxsfb->clk_axi)
+		clk_disable_unprepare(mxsfb->clk_axi);
+}
+
+static u32 set_hsync_pulse_width(struct mxsfb_drm_private *mxsfb, u32 val)
+{
+	return (val & mxsfb->devdata->hs_wdth_mask) <<
+		mxsfb->devdata->hs_wdth_shift;
+}
+
+static struct simplefb_format supported_formats[] = SIMPLEFB_FORMATS;
+
+/*
+ * Setup the MXSFB registers for decoding the pixels out of the framebuffer
+ */
+static int mxsfb_set_pixel_fmt(struct drm_crtc *crtc)
+{
+	struct drm_device *drm = crtc->dev;
+	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
+	struct simplefb_format *format = NULL;
+	u32 pixel_format, ctrl;
+	int i;
+
+	pixel_format = crtc->primary->state->fb->pixel_format;
+
+	for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
+		if (supported_formats[i].fourcc == pixel_format)
+			format = &supported_formats[i];
+	}
+
+	if (WARN_ON(!format))
+		return 0;
+
+	ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER;
+
+	/*
+	 * WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to
+	 * match the selected mode here. This differs from the original
+	 * MXSFB driver, which had the option to configure the bus width
+	 * to arbitrary value. This limitation should not pose an issue.
+	 */
+
+	switch (format->fourcc) {
+	case DRM_FORMAT_RGB565:
+		dev_dbg(drm->dev, "Setting up RGB565 mode\n");
+		ctrl |= CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT);
+		ctrl |= CTRL_SET_WORD_LENGTH(0);
+		writel(CTRL1_SET_BYTE_PACKAGING(0xf), mxsfb->base + LCDC_CTRL1);
+		break;
+	case DRM_FORMAT_XRGB8888:
+		dev_dbg(drm->dev, "Setting up XRGB8888 mode\n");
+		ctrl |= CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT);
+		ctrl |= CTRL_SET_WORD_LENGTH(3);
+		/* Do not use packed pixels = one pixel per word instead */
+		writel(CTRL1_SET_BYTE_PACKAGING(0x7), mxsfb->base + LCDC_CTRL1);
+		break;
+	default:
+		dev_err(drm->dev, "Unhandled color format %s\n",
+			format->name);
+		return -EINVAL;
+	}
+
+	writel(ctrl, mxsfb->base + LCDC_CTRL);
+	return 0;
+}
+
+static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb)
+{
+	u32 reg;
+
+	if (mxsfb->clk_disp_axi)
+		clk_prepare_enable(mxsfb->clk_disp_axi);
+	clk_prepare_enable(mxsfb->clk);
+	mxsfb_enable_axi_clk(mxsfb);
+
+	/* If it was disabled, re-enable the mode again */
+	writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_SET);
+
+	/* Enable the SYNC signals first, then the DMA engine */
+	reg = readl(mxsfb->base + LCDC_VDCTRL4);
+	reg |= VDCTRL4_SYNC_SIGNALS_ON;
+	writel(reg, mxsfb->base + LCDC_VDCTRL4);
+
+	writel(CTRL_RUN, mxsfb->base + LCDC_CTRL + REG_SET);
+
+	mxsfb->enabled = 1;
+}
+
+static void mxsfb_disable_controller(struct mxsfb_drm_private *mxsfb)
+{
+	u32 reg;
+
+	/*
+	 * Even if we disable the controller here, it will still continue
+	 * until its FIFOs are running out of data
+	 */
+	writel(CTRL_DOTCLK_MODE, mxsfb->base + LCDC_CTRL + REG_CLR);
+
+	readl_poll_timeout(mxsfb->base + LCDC_CTRL, reg, !(reg & CTRL_RUN),
+			   0, 1000);
+
+	reg = readl(mxsfb->base + LCDC_VDCTRL4);
+	reg &= ~VDCTRL4_SYNC_SIGNALS_ON;
+	writel(reg, mxsfb->base + LCDC_VDCTRL4);
+
+	mxsfb_disable_axi_clk(mxsfb);
+
+	clk_disable_unprepare(mxsfb->clk);
+	if (mxsfb->clk_disp_axi)
+		clk_disable_unprepare(mxsfb->clk_disp_axi);
+
+	mxsfb->enabled = 0;
+}
+
+static void mxsfb_crtc_mode_set_nofb(struct drm_crtc *crtc)
+{
+	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
+	struct drm_display_mode *m = &crtc->state->adjusted_mode;
+	const u32 bus_flags = mxsfb->output.connector.display_info.bus_flags;
+	u32 vdctrl0, vsync_pulse_len, hsync_pulse_len;
+	bool reenable = false;
+	int err;
+
+	/*
+	 * It seems, you can't re-program the controller if it is still
+	 * running. This may lead to shifted pictures (FIFO issue?), so
+	 * first stop the controller and drain its FIFOs.
+	 */
+	if (mxsfb->enabled) {
+		reenable = true;
+		mxsfb_disable_controller(mxsfb);
+	}
+
+	mxsfb_enable_axi_clk(mxsfb);
+
+	/* Clear the FIFOs */
+	writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET);
+
+	err = mxsfb_set_pixel_fmt(crtc);
+	if (err)
+		return;
+
+	clk_set_rate(mxsfb->clk, m->crtc_clock * 1000);
+
+	writel(TRANSFER_COUNT_SET_VCOUNT(m->crtc_vdisplay) |
+	       TRANSFER_COUNT_SET_HCOUNT(m->crtc_hdisplay),
+	       mxsfb->base + mxsfb->devdata->transfer_count);
+
+	vsync_pulse_len = m->crtc_vsync_end - m->crtc_vsync_start;
+
+	vdctrl0 = VDCTRL0_ENABLE_PRESENT |	/* Always in DOTCLOCK mode */
+		  VDCTRL0_VSYNC_PERIOD_UNIT |
+		  VDCTRL0_VSYNC_PULSE_WIDTH_UNIT |
+		  VDCTRL0_SET_VSYNC_PULSE_WIDTH(vsync_pulse_len);
+	if (m->flags & DRM_MODE_FLAG_PHSYNC)
+		vdctrl0 |= VDCTRL0_HSYNC_ACT_HIGH;
+	if (m->flags & DRM_MODE_FLAG_PVSYNC)
+		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
+	if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
+		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
+	if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING;
+
+	writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0);
+
+	/* Frame length in lines. */
+	writel(m->crtc_vtotal, mxsfb->base + LCDC_VDCTRL1);
+
+	/* Line length in units of clocks or pixels. */
+	hsync_pulse_len = m->crtc_hsync_end - m->crtc_hsync_start;
+	writel(set_hsync_pulse_width(mxsfb, hsync_pulse_len) |
+	       VDCTRL2_SET_HSYNC_PERIOD(m->crtc_htotal),
+	       mxsfb->base + LCDC_VDCTRL2);
+
+	writel(SET_HOR_WAIT_CNT(m->crtc_hblank_end - m->crtc_hsync_end) |
+	       SET_VERT_WAIT_CNT(m->crtc_vblank_end - m->crtc_vsync_end),
+	       mxsfb->base + LCDC_VDCTRL3);
+
+	writel(SET_DOTCLK_H_VALID_DATA_CNT(m->hdisplay),
+	       mxsfb->base + LCDC_VDCTRL4);
+
+	mxsfb_disable_axi_clk(mxsfb);
+
+	if (reenable)
+		mxsfb_enable_controller(mxsfb);
+}
+
+static void mxsfb_crtc_enable(struct drm_crtc *crtc)
+{
+	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
+
+	mxsfb_crtc_mode_set_nofb(crtc);
+	mxsfb_enable_controller(mxsfb);
+}
+
+static void mxsfb_crtc_disable(struct drm_crtc *crtc)
+{
+	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
+
+	if (!crtc->state->active)
+		return;
+
+	mxsfb_disable_controller(mxsfb);
+}
+
+static int mxsfb_crtc_atomic_check(struct drm_crtc *crtc,
+				   struct drm_crtc_state *state)
+{
+	struct mxsfb_drm_private *mxsfb = crtc_to_mxsfb_priv(crtc);
+	struct drm_display_mode *mode = &state->adjusted_mode;
+	long rate, clk_rate = mode->clock * 1000;
+
+	if (!mode->clock)
+		return 0;
+
+	rate = clk_round_rate(mxsfb->clk, clk_rate);
+	/* clock required by mode not supported by hardware */
+	if (rate != clk_rate)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void mxsfb_crtc_atomic_begin(struct drm_crtc *crtc,
+				    struct drm_crtc_state *state)
+{
+	struct drm_pending_vblank_event *event = crtc->state->event;
+
+	if (event) {
+		crtc->state->event = NULL;
+
+		spin_lock_irq(&crtc->dev->event_lock);
+		if (drm_crtc_vblank_get(crtc) == 0)
+			drm_crtc_arm_vblank_event(crtc, event);
+		else
+			drm_crtc_send_vblank_event(crtc, event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+	}
+}
+
+static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = {
+	.mode_set	= drm_helper_crtc_mode_set,
+	.mode_set_base	= drm_helper_crtc_mode_set_base,
+	.mode_set_nofb	= mxsfb_crtc_mode_set_nofb,
+	.enable		= mxsfb_crtc_enable,
+	.disable	= mxsfb_crtc_disable,
+	.prepare	= mxsfb_crtc_disable,
+	.commit		= mxsfb_crtc_enable,
+	.atomic_check	= mxsfb_crtc_atomic_check,
+	.atomic_begin	= mxsfb_crtc_atomic_begin,
+};
+
+static void mxsfb_plane_atomic_update(struct drm_plane *plane,
+				      struct drm_plane_state *state)
+{
+	struct mxsfb_drm_private *mxsfb = plane->dev->dev_private;
+	struct drm_gem_cma_object *gem;
+
+	if (!plane->state->crtc || !plane->state->fb)
+		return;
+
+	gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
+
+	mxsfb_enable_axi_clk(mxsfb);
+	writel(gem->paddr, mxsfb->base + mxsfb->devdata->next_buf);
+	mxsfb_disable_axi_clk(mxsfb);
+}
+
+static const struct drm_plane_helper_funcs mxsfb_plane_helper_funcs = {
+	.atomic_update	= mxsfb_plane_atomic_update,
+};
+
+static void mxsfb_plane_destroy(struct drm_plane *plane)
+{
+	drm_plane_helper_disable(plane);
+	drm_plane_cleanup(plane);
+}
+
+static const struct drm_plane_funcs mxsfb_plane_funcs = {
+	.update_plane		= drm_atomic_helper_update_plane,
+	.disable_plane		= drm_atomic_helper_disable_plane,
+	.destroy		= mxsfb_plane_destroy,
+	.reset			= drm_atomic_helper_plane_reset,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
+};
+
+static struct drm_plane *mxsfb_plane_init(struct drm_device *drm)
+{
+	struct mxsfb_drm_private *mxsfb = drm->dev_private;
+	struct drm_plane *plane = NULL;
+	u32 formats[ARRAY_SIZE(supported_formats)], i;
+	int ret;
+
+	plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);
+	if (!plane)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < ARRAY_SIZE(supported_formats); i++)
+		formats[i] = supported_formats[i].fourcc;
+
+	ret = drm_universal_plane_init(drm, plane, 0xff, &mxsfb_plane_funcs,
+				       formats, ARRAY_SIZE(formats),
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret) {
+		devm_kfree(drm->dev, plane);
+		return ERR_PTR(ret);
+	}
+
+	drm_plane_helper_add(plane, &mxsfb_plane_helper_funcs);
+	mxsfb->plane = plane;
+
+	return plane;
+}
+
+static void mxsfb_crtc_cleanup(struct drm_crtc *crtc)
+{
+	mxsfb_crtc_disable(crtc);
+	drm_crtc_cleanup(crtc);
+}
+
+static const struct drm_crtc_funcs mxsfb_crtc_funcs = {
+	.destroy		= mxsfb_crtc_cleanup,
+	.set_config		= drm_atomic_helper_set_config,
+	.page_flip		= drm_atomic_helper_page_flip,
+	.reset			= drm_atomic_helper_crtc_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_crtc_destroy_state,
+};
+
+int mxsfb_setup_crtc(struct drm_device *drm)
+{
+	struct mxsfb_drm_private *mxsfb = drm->dev_private;
+	struct drm_plane *primary;
+	int ret;
+
+	primary = mxsfb_plane_init(drm);
+	if (IS_ERR(primary))
+		return PTR_ERR(primary);
+
+	ret = drm_crtc_init_with_planes(drm, &mxsfb->crtc, primary, NULL,
+					&mxsfb_crtc_funcs, NULL);
+	if (ret) {
+		mxsfb_plane_destroy(primary);
+		devm_kfree(drm->dev, primary);
+		return ret;
+	}
+
+	drm_crtc_helper_add(&mxsfb->crtc, &mxsfb_crtc_helper_funcs);
+	return 0;
+}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
new file mode 100644
index 0000000..bbc820f
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -0,0 +1,324 @@ 
+/*
+ * Copyright (C) 2016 Marek Vasut <marex@denx.de>
+ *
+ * This code is based on drivers/video/fbdev/mxsfb.c :
+ * Copyright (C) 2010 Juergen Beisert, Pengutronix
+ * Copyright (C) 2008-2009 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2008 Embedded Alley Solutions, Inc All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/list.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/pm_runtime.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_of.h>
+
+#include "mxsfb_drv.h"
+#include "mxsfb_regs.h"
+
+enum mxsfb_devtype {
+	MXSFB_V3,
+	MXSFB_V4,
+};
+
+static const struct mxsfb_devdata mxsfb_devdata[] = {
+	[MXSFB_V3] = {
+		.transfer_count	= LCDC_V3_TRANSFER_COUNT,
+		.cur_buf	= LCDC_V3_CUR_BUF,
+		.next_buf	= LCDC_V3_NEXT_BUF,
+		.debug0		= LCDC_V3_DEBUG0,
+		.hs_wdth_mask	= 0xff,
+		.hs_wdth_shift	= 24,
+		.ipversion	= 3,
+	},
+	[MXSFB_V4] = {
+		.transfer_count	= LCDC_V4_TRANSFER_COUNT,
+		.cur_buf	= LCDC_V4_CUR_BUF,
+		.next_buf	= LCDC_V4_NEXT_BUF,
+		.debug0		= LCDC_V4_DEBUG0,
+		.hs_wdth_mask	= 0x3fff,
+		.hs_wdth_shift	= 18,
+		.ipversion	= 4,
+	},
+};
+
+static void mxsfb_fb_output_poll_changed(struct drm_device *drm)
+{
+	struct mxsfb_drm_private *mxsfb = drm->dev_private;
+
+	if (mxsfb->fbdev) {
+		drm_fbdev_cma_hotplug_event(mxsfb->fbdev);
+	} else {
+		mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
+					drm->mode_config.num_crtc,
+					drm->mode_config.num_connector);
+		if (IS_ERR(mxsfb->fbdev))
+			mxsfb->fbdev = NULL;
+	}
+}
+
+static int mxsfb_atomic_commit(struct drm_device *dev,
+			       struct drm_atomic_state *state, bool nonblock)
+{
+	return drm_atomic_helper_commit(dev, state, false);
+}
+
+static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
+	.fb_create		= drm_fb_cma_create,
+	.output_poll_changed	= mxsfb_fb_output_poll_changed,
+	.atomic_check		= drm_atomic_helper_check,
+	.atomic_commit		= mxsfb_atomic_commit,
+};
+
+static int mxsfb_load(struct drm_device *drm, unsigned long flags)
+{
+	struct platform_device *pdev = to_platform_device(drm->dev);
+	struct mxsfb_drm_private *mxsfb;
+	struct resource *res;
+	int ret;
+
+	mxsfb = devm_kzalloc(&pdev->dev, sizeof(*mxsfb), GFP_KERNEL);
+	if (!mxsfb)
+		return -ENOMEM;
+
+	drm->dev_private = mxsfb;
+	mxsfb->devdata = &mxsfb_devdata[pdev->id_entry->driver_data];
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mxsfb->base = devm_ioremap_resource(drm->dev, res);
+	if (IS_ERR(mxsfb->base))
+		return PTR_ERR(mxsfb->base);
+
+	mxsfb->clk = devm_clk_get(drm->dev, NULL);
+	if (IS_ERR(mxsfb->clk))
+		return PTR_ERR(mxsfb->clk);
+
+	mxsfb->clk_axi = devm_clk_get(drm->dev, "axi");
+	if (IS_ERR(mxsfb->clk_axi))
+		mxsfb->clk_axi = NULL;
+
+	mxsfb->clk_disp_axi = devm_clk_get(drm->dev, "disp_axi");
+	if (IS_ERR(mxsfb->clk_disp_axi))
+		mxsfb->clk_disp_axi = NULL;
+
+	ret = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32));
+	if (ret)
+		return ret;
+
+	pm_runtime_enable(drm->dev);
+
+	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to initialise vblank\n");
+		goto err_vblank;
+	}
+
+	/* Modeset init */
+	drm_mode_config_init(drm);
+
+	ret = mxsfb_create_outputs(drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to create outputs\n");
+		goto err_vblank;
+	}
+
+	ret = mxsfb_setup_crtc(drm);
+	if (ret < 0) {
+		dev_err(drm->dev, "Failed to create crtc\n");
+		goto err_vblank;
+	}
+
+	drm->mode_config.min_width	= MXSFB_MIN_XRES;
+	drm->mode_config.min_height	= MXSFB_MIN_YRES;
+	drm->mode_config.max_width	= MXSFB_MAX_XRES;
+	drm->mode_config.max_height	= MXSFB_MAX_YRES;
+	drm->mode_config.funcs		= &mxsfb_mode_config_funcs;
+
+	drm_mode_config_reset(drm);
+
+	platform_set_drvdata(pdev, drm);
+
+	drm_kms_helper_poll_init(drm);
+
+	drm_helper_hpd_irq_event(drm);
+
+	return 0;
+
+err_vblank:
+	pm_runtime_disable(drm->dev);
+
+	return ret;
+}
+
+static void mxsfb_unload(struct drm_device *drm)
+{
+	struct mxsfb_drm_private *mxsfb = drm->dev_private;
+
+	if (mxsfb->fbdev)
+		drm_fbdev_cma_fini(mxsfb->fbdev);
+
+	drm_kms_helper_poll_fini(drm);
+	drm_mode_config_cleanup(drm);
+	drm_vblank_cleanup(drm);
+
+	drm->dev_private = NULL;
+
+	pm_runtime_disable(drm->dev);
+}
+
+static void mxsfb_lastclose(struct drm_device *drm)
+{
+	struct mxsfb_drm_private *mxsfb = drm->dev_private;
+
+	drm_fbdev_cma_restore_mode(mxsfb->fbdev);
+}
+
+static const struct file_operations fops = {
+	.owner		= THIS_MODULE,
+	.open		= drm_open,
+	.release	= drm_release,
+	.unlocked_ioctl	= drm_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= drm_compat_ioctl,
+#endif
+	.poll		= drm_poll,
+	.read		= drm_read,
+	.llseek		= noop_llseek,
+	.mmap		= drm_gem_cma_mmap,
+};
+
+static struct drm_driver mxsfb_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET |
+				  DRIVER_PRIME | DRIVER_ATOMIC,
+	.lastclose		= mxsfb_lastclose,
+	.gem_free_object	= drm_gem_cma_free_object,
+	.gem_vm_ops		= &drm_gem_cma_vm_ops,
+	.dumb_create		= drm_gem_cma_dumb_create,
+	.dumb_map_offset	= drm_gem_cma_dumb_map_offset,
+	.dumb_destroy		= drm_gem_dumb_destroy,
+	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
+	.gem_prime_export	= drm_gem_prime_export,
+	.gem_prime_import	= drm_gem_prime_import,
+	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
+	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
+	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
+	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
+	.fops	= &fops,
+	.name	= "mxsfb-drm",
+	.desc	= "MXSFB Controller DRM",
+	.date	= "20160824",
+	.major	= 1,
+	.minor	= 0,
+};
+
+static const struct platform_device_id mxsfb_devtype[] = {
+	{ .name = "imx23-fb", .driver_data = MXSFB_V3, },
+	{ .name = "imx28-fb", .driver_data = MXSFB_V4, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
+
+static const struct of_device_id mxsfb_dt_ids[] = {
+	{ .compatible = "fsl,imx23-lcdif", .data = &mxsfb_devtype[0], },
+	{ .compatible = "fsl,imx28-lcdif", .data = &mxsfb_devtype[1], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxsfb_dt_ids);
+
+static int mxsfb_probe(struct platform_device *pdev)
+{
+	struct drm_device *drm;
+	const struct of_device_id *of_id =
+			of_match_device(mxsfb_dt_ids, &pdev->dev);
+	int ret;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	if (of_id)
+		pdev->id_entry = of_id->data;
+
+	drm = drm_dev_alloc(&mxsfb_driver, &pdev->dev);
+	if (!drm)
+		return -ENOMEM;
+
+	ret = mxsfb_load(drm, 0);
+	if (ret)
+		goto err_free;
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		goto err_unload;
+
+	ret = drm_connector_register_all(drm);
+	if (ret) {
+		DRM_ERROR("Failed to bind all components\n");
+		goto err_unregister;
+	}
+
+	return 0;
+
+err_unregister:
+	drm_dev_unregister(drm);
+err_unload:
+	mxsfb_unload(drm);
+err_free:
+	drm_dev_unref(drm);
+
+	return ret;
+}
+
+static int mxsfb_remove(struct platform_device *pdev)
+{
+	struct drm_device *drm = platform_get_drvdata(pdev);
+
+	mutex_lock(&drm->mode_config.mutex);
+	drm_connector_unregister_all(drm);
+	mutex_unlock(&drm->mode_config.mutex);
+
+	drm_dev_unregister(drm);
+	mxsfb_unload(drm);
+	drm_dev_unref(drm);
+
+	return 0;
+}
+
+static struct platform_driver mxsfb_platform_driver = {
+	.probe		= mxsfb_probe,
+	.remove		= mxsfb_remove,
+	.id_table	= mxsfb_devtype,
+	.driver	= {
+		.name		= "mxsfb",
+		.of_match_table	= mxsfb_dt_ids,
+	},
+};
+
+module_platform_driver(mxsfb_platform_driver);
+
+MODULE_AUTHOR("Marek Vasut <marex@denx.de>");
+MODULE_DESCRIPTION("Freescale MXS DRM/KMS driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
new file mode 100644
index 0000000..afa1e34
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -0,0 +1,58 @@ 
+/*
+ * Copyright (C) 2016 Marek Vasut <marex@denx.de>
+ *
+ * i.MX23/i.MX28/i.MX6SX MXSFB LCD controller driver.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __MXSFB_DRV_H__
+#define __MXSFB_DRV_H__
+
+struct mxsfb_devdata {
+	unsigned int	 transfer_count;
+	unsigned int	 cur_buf;
+	unsigned int	 next_buf;
+	unsigned int	 debug0;
+	unsigned int	 hs_wdth_mask;
+	unsigned int	 hs_wdth_shift;
+	unsigned int	 ipversion;
+};
+
+struct mxsfb_rgb_output {
+	struct drm_connector	connector;
+	struct drm_encoder	encoder;
+	struct drm_panel	*panel;
+};
+
+struct mxsfb_drm_private {
+	struct clk		*clk;
+	struct clk		*clk_axi;
+	struct clk		*clk_disp_axi;
+	void __iomem		*base;	/* registers */
+
+	struct drm_fbdev_cma	*fbdev;
+	struct drm_crtc		crtc;
+	struct drm_plane	*plane;
+	struct drm_atomic_state	*state;
+
+	struct mxsfb_rgb_output	output;
+
+	const struct mxsfb_devdata *devdata;
+	bool enabled;
+};
+
+#define crtc_to_mxsfb_priv(x)	container_of(x, struct mxsfb_drm_private, crtc)
+
+int mxsfb_setup_crtc(struct drm_device *dev);
+int mxsfb_create_outputs(struct drm_device *dev);
+void mxsfb_set_scanout(struct mxsfb_drm_private *mxsfb);
+
+#endif /* __MXSFB_DRV_H__ */
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_out.c b/drivers/gpu/drm/mxsfb/mxsfb_out.c
new file mode 100644
index 0000000..ffc4b29
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/mxsfb_out.c
@@ -0,0 +1,264 @@ 
+/*
+ * Copyright (C) 2016 Marek Vasut <marex@denx.de>
+ *
+ * This code is based on drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c :
+ * Copyright (C) 2014 Traphandler
+ * Copyright (C) 2014 Free Electrons
+ * Copyright (C) 2014 Atmel
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/of_graph.h>
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drmP.h>
+
+#include "mxsfb_drv.h"
+
+static inline struct mxsfb_rgb_output *
+drm_connector_to_mxsfb_rgb_output(struct drm_connector *connector)
+{
+	return container_of(connector, struct mxsfb_rgb_output,
+			    connector);
+}
+
+static inline struct mxsfb_rgb_output *
+drm_encoder_to_mxsfb_rgb_output(struct drm_encoder *encoder)
+{
+	return container_of(encoder, struct mxsfb_rgb_output, encoder);
+}
+
+static void mxsfb_rgb_encoder_enable(struct drm_encoder *encoder)
+{
+	struct mxsfb_rgb_output *rgb =
+			drm_encoder_to_mxsfb_rgb_output(encoder);
+
+	if (rgb->panel) {
+		drm_panel_prepare(rgb->panel);
+		drm_panel_enable(rgb->panel);
+	}
+}
+
+static void mxsfb_rgb_encoder_disable(struct drm_encoder *encoder)
+{
+	struct mxsfb_rgb_output *rgb =
+			drm_encoder_to_mxsfb_rgb_output(encoder);
+
+	if (rgb->panel) {
+		drm_panel_disable(rgb->panel);
+		drm_panel_unprepare(rgb->panel);
+	}
+}
+
+static const struct
+drm_encoder_helper_funcs mxsfb_panel_encoder_helper_funcs = {
+	.disable	= mxsfb_rgb_encoder_disable,
+	.enable		= mxsfb_rgb_encoder_enable,
+};
+
+static void mxsfb_rgb_encoder_destroy(struct drm_encoder *encoder)
+{
+	drm_encoder_cleanup(encoder);
+	memset(encoder, 0, sizeof(*encoder));
+}
+
+static const struct drm_encoder_funcs mxsfb_panel_encoder_funcs = {
+	.destroy = mxsfb_rgb_encoder_destroy,
+};
+
+static int mxsfb_panel_get_modes(struct drm_connector *connector)
+{
+	struct mxsfb_rgb_output *rgb =
+			drm_connector_to_mxsfb_rgb_output(connector);
+
+	if (rgb->panel)
+		return rgb->panel->funcs->get_modes(rgb->panel);
+
+	return 0;
+}
+
+static int mxsfb_rgb_mode_valid(struct drm_connector *connector,
+				struct drm_display_mode *mode)
+{
+	return 0;
+}
+
+static struct drm_encoder *
+mxsfb_rgb_best_encoder(struct drm_connector *connector)
+{
+	struct mxsfb_rgb_output *rgb =
+			drm_connector_to_mxsfb_rgb_output(connector);
+
+	return &rgb->encoder;
+}
+
+static const struct
+drm_connector_helper_funcs mxsfb_panel_connector_helper_funcs = {
+	.get_modes = mxsfb_panel_get_modes,
+	.mode_valid = mxsfb_rgb_mode_valid,
+	.best_encoder = mxsfb_rgb_best_encoder,
+};
+
+static enum drm_connector_status
+mxsfb_panel_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct mxsfb_rgb_output *rgb =
+			drm_connector_to_mxsfb_rgb_output(connector);
+
+	if (rgb->panel)
+		return connector_status_connected;
+
+	return connector_status_disconnected;
+}
+
+static void mxsfb_panel_connector_destroy(struct drm_connector *connector)
+{
+	struct mxsfb_rgb_output *rgb =
+			drm_connector_to_mxsfb_rgb_output(connector);
+
+	if (rgb->panel)
+		drm_panel_detach(rgb->panel);
+
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs mxsfb_panel_connector_funcs = {
+	.dpms			= drm_atomic_helper_connector_dpms,
+	.detect			= mxsfb_panel_connector_detect,
+	.fill_modes		= drm_helper_probe_single_connector_modes,
+	.destroy		= mxsfb_panel_connector_destroy,
+	.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 mxsfb_check_endpoint(struct drm_device *drm,
+				const struct of_endpoint *ep)
+{
+	struct device_node *np;
+	void *obj;
+
+	np = of_graph_get_remote_port_parent(ep->local_node);
+
+	obj = of_drm_find_panel(np);
+	if (!obj)
+		obj = of_drm_find_bridge(np);
+
+	of_node_put(np);
+
+	return obj ? 0 : -EPROBE_DEFER;
+}
+
+static int mxsfb_attach_endpoint(struct drm_device *drm,
+				 const struct of_endpoint *ep)
+{
+	struct mxsfb_drm_private *mxsfb = drm->dev_private;
+	struct mxsfb_rgb_output *output = &mxsfb->output;
+	struct device_node *np;
+	struct drm_panel *panel;
+	struct drm_bridge *bridge;
+	int ret;
+
+	drm_encoder_helper_add(&output->encoder,
+			       &mxsfb_panel_encoder_helper_funcs);
+	ret = drm_encoder_init(drm, &output->encoder,
+			       &mxsfb_panel_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret)
+		return ret;
+
+	output->encoder.possible_crtcs = 0x1;
+
+	np = of_graph_get_remote_port_parent(ep->local_node);
+
+	ret = -EPROBE_DEFER;
+
+	panel = of_drm_find_panel(np);
+	if (panel) {
+		of_node_put(np);
+		output->connector.dpms = DRM_MODE_DPMS_OFF;
+		output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
+		drm_connector_helper_add(&output->connector,
+				&mxsfb_panel_connector_helper_funcs);
+		ret = drm_connector_init(drm, &output->connector,
+					 &mxsfb_panel_connector_funcs,
+					 DRM_MODE_CONNECTOR_Unknown);
+		if (ret)
+			goto err_encoder_cleanup;
+
+		drm_mode_connector_attach_encoder(&output->connector,
+						  &output->encoder);
+
+		ret = drm_panel_attach(panel, &output->connector);
+		if (ret) {
+			drm_connector_cleanup(&output->connector);
+			goto err_encoder_cleanup;
+		}
+
+		output->panel = panel;
+
+		return 0;
+	}
+
+	bridge = of_drm_find_bridge(np);
+	of_node_put(np);
+
+	if (bridge) {
+		output->encoder.bridge = bridge;
+		bridge->encoder = &output->encoder;
+		ret = drm_bridge_attach(drm, bridge);
+		if (!ret)
+			return 0;
+	}
+
+err_encoder_cleanup:
+	drm_encoder_cleanup(&output->encoder);
+
+	return ret;
+}
+
+int mxsfb_create_outputs(struct drm_device *drm)
+{
+	struct device_node *ep_np = NULL;
+	struct of_endpoint ep;
+	int ret;
+
+	for_each_endpoint_of_node(drm->dev->of_node, ep_np) {
+		ret = of_graph_parse_endpoint(ep_np, &ep);
+		if (!ret)
+			ret = mxsfb_check_endpoint(drm, &ep);
+
+		if (ret) {
+			of_node_put(ep_np);
+			return ret;
+		}
+	}
+
+	for_each_endpoint_of_node(drm->dev->of_node, ep_np) {
+		ret = of_graph_parse_endpoint(ep_np, &ep);
+		if (!ret)
+			ret = mxsfb_attach_endpoint(drm, &ep);
+
+		if (ret) {
+			of_node_put(ep_np);
+			return ret;
+		}
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
new file mode 100644
index 0000000..e9a006a
--- /dev/null
+++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -0,0 +1,112 @@ 
+/*
+ * Copyright (C) 2010 Juergen Beisert, Pengutronix
+ * Copyright (C) 2016 Marek Vasut <marex@denx.de>
+ *
+ * i.MX23/i.MX28/i.MX6SX MXSFB LCD controller driver.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __MXSFB_REGS_H__
+#define __MXSFB_REGS_H__
+
+#define REG_SET	4
+#define REG_CLR	8
+
+#define LCDC_CTRL			0x00
+#define LCDC_CTRL1			0x10
+#define LCDC_V3_TRANSFER_COUNT		0x20
+#define LCDC_V4_TRANSFER_COUNT		0x30
+#define LCDC_V4_CUR_BUF			0x40
+#define LCDC_V4_NEXT_BUF		0x50
+#define LCDC_V3_CUR_BUF			0x30
+#define LCDC_V3_NEXT_BUF		0x40
+#define LCDC_VDCTRL0			0x70
+#define LCDC_VDCTRL1			0x80
+#define LCDC_VDCTRL2			0x90
+#define LCDC_VDCTRL3			0xa0
+#define LCDC_VDCTRL4			0xb0
+#define LCDC_V4_DEBUG0			0x1d0
+#define LCDC_V3_DEBUG0			0x1f0
+
+#define CTRL_SFTRST			(1 << 31)
+#define CTRL_CLKGATE			(1 << 30)
+#define CTRL_BYPASS_COUNT		(1 << 19)
+#define CTRL_VSYNC_MODE			(1 << 18)
+#define CTRL_DOTCLK_MODE		(1 << 17)
+#define CTRL_DATA_SELECT		(1 << 16)
+#define CTRL_SET_BUS_WIDTH(x)		(((x) & 0x3) << 10)
+#define CTRL_GET_BUS_WIDTH(x)		(((x) >> 10) & 0x3)
+#define CTRL_SET_WORD_LENGTH(x)		(((x) & 0x3) << 8)
+#define CTRL_GET_WORD_LENGTH(x)		(((x) >> 8) & 0x3)
+#define CTRL_MASTER			(1 << 5)
+#define CTRL_DF16			(1 << 3)
+#define CTRL_DF18			(1 << 2)
+#define CTRL_DF24			(1 << 1)
+#define CTRL_RUN			(1 << 0)
+
+#define CTRL1_FIFO_CLEAR		(1 << 21)
+#define CTRL1_SET_BYTE_PACKAGING(x)	(((x) & 0xf) << 16)
+#define CTRL1_GET_BYTE_PACKAGING(x)	(((x) >> 16) & 0xf)
+
+#define TRANSFER_COUNT_SET_VCOUNT(x)	(((x) & 0xffff) << 16)
+#define TRANSFER_COUNT_GET_VCOUNT(x)	(((x) >> 16) & 0xffff)
+#define TRANSFER_COUNT_SET_HCOUNT(x)	((x) & 0xffff)
+#define TRANSFER_COUNT_GET_HCOUNT(x)	((x) & 0xffff)
+
+#define VDCTRL0_ENABLE_PRESENT		(1 << 28)
+#define VDCTRL0_VSYNC_ACT_HIGH		(1 << 27)
+#define VDCTRL0_HSYNC_ACT_HIGH		(1 << 26)
+#define VDCTRL0_DOTCLK_ACT_FALLING	(1 << 25)
+#define VDCTRL0_ENABLE_ACT_HIGH		(1 << 24)
+#define VDCTRL0_VSYNC_PERIOD_UNIT	(1 << 21)
+#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	(1 << 20)
+#define VDCTRL0_HALF_LINE		(1 << 19)
+#define VDCTRL0_HALF_LINE_MODE		(1 << 18)
+#define VDCTRL0_SET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
+#define VDCTRL0_GET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
+
+#define VDCTRL2_SET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
+#define VDCTRL2_GET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
+
+#define VDCTRL3_MUX_SYNC_SIGNALS	(1 << 29)
+#define VDCTRL3_VSYNC_ONLY		(1 << 28)
+#define SET_HOR_WAIT_CNT(x)		(((x) & 0xfff) << 16)
+#define GET_HOR_WAIT_CNT(x)		(((x) >> 16) & 0xfff)
+#define SET_VERT_WAIT_CNT(x)		((x) & 0xffff)
+#define GET_VERT_WAIT_CNT(x)		((x) & 0xffff)
+
+#define VDCTRL4_SET_DOTCLK_DLY(x)	(((x) & 0x7) << 29) /* v4 only */
+#define VDCTRL4_GET_DOTCLK_DLY(x)	(((x) >> 29) & 0x7) /* v4 only */
+#define VDCTRL4_SYNC_SIGNALS_ON		(1 << 18)
+#define SET_DOTCLK_H_VALID_DATA_CNT(x)	((x) & 0x3ffff)
+
+#define DEBUG0_HSYNC			(1 < 26)
+#define DEBUG0_VSYNC			(1 < 25)
+
+#define MXSFB_MIN_XRES			120
+#define MXSFB_MIN_YRES			120
+#define MXSFB_MAX_XRES			0xffff
+#define MXSFB_MAX_YRES			0xffff
+
+#define RED 0
+#define GREEN 1
+#define BLUE 2
+#define TRANSP 3
+
+#define STMLCDIF_8BIT  1 /* pixel data bus to the display is of 8 bit width */
+#define STMLCDIF_16BIT 0 /* pixel data bus to the display is of 16 bit width */
+#define STMLCDIF_18BIT 2 /* pixel data bus to the display is of 18 bit width */
+#define STMLCDIF_24BIT 3 /* pixel data bus to the display is of 24 bit width */
+
+#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
+#define MXSFB_SYNC_DOTCLK_FALLING_ACT	(1 << 7) /* negative edge sampling */
+
+#endif /* __MXSFB_REGS_H__ */