linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3]
@ 2020-04-27  8:21 Gareth Williams
  2020-04-27  8:21 ` [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Gareth Williams
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Gareth Williams @ 2020-04-27  8:21 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland, Hans Verkuil,
	Icenowy Zheng, Mauro Carvalho Chehab, Vivek Unune,
	Stephen Rothwell, Thierry Reding, Sam Ravnborg
  Cc: Gareth Williams, Phil Edworthy, dri-devel, devicetree, linux-kernel

This series adds DRM support for the Digital Blocks db9000
LCD controller with RZ/N1 specific changes and updates simple-panel to
include the associated panel. As this has not previously been
documented, also include a yaml file to provide this.

Gareth Williams (3):
  drm/db9000: Add Digital Blocks DB9000 LCD Controller
  drm/db9000: Add bindings documentation for LCD controller
  drm/panel: simple: Add Newhaven ATXL#-CTP panel

 .../devicetree/bindings/display/db9000,du.yaml     |  87 ++
 .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
 drivers/gpu/drm/Kconfig                            |   2 +
 drivers/gpu/drm/Makefile                           |   1 +
 drivers/gpu/drm/digital-blocks/Kconfig             |  13 +
 drivers/gpu/drm/digital-blocks/Makefile            |   3 +
 drivers/gpu/drm/digital-blocks/db9000-du.c         | 953 +++++++++++++++++++++
 drivers/gpu/drm/digital-blocks/db9000-du.h         | 192 +++++
 drivers/gpu/drm/panel/panel-simple.c               |  27 +
 9 files changed, 1280 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/db9000,du.yaml
 create mode 100644 drivers/gpu/drm/digital-blocks/Kconfig
 create mode 100644 drivers/gpu/drm/digital-blocks/Makefile
 create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.c
 create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.h

-- 
2.7.4


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller
  2020-04-27  8:21 [PATCH 0/3] Gareth Williams
@ 2020-04-27  8:21 ` Gareth Williams
  2020-04-28 18:18   ` Sam Ravnborg
  2020-04-27  8:21 ` [PATCH 2/3] drm/db9000: Add bindings documentation for LCD controller Gareth Williams
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Gareth Williams @ 2020-04-27  8:21 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie, Daniel Vetter
  Cc: Gareth Williams, Phil Edworthy, linux-kernel, dri-devel

Add DRM support for the Digital Blocks DB9000 LCD Controller
with the Renesas RZ/N1 specific changes.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
 drivers/gpu/drm/Kconfig                    |   2 +
 drivers/gpu/drm/Makefile                   |   1 +
 drivers/gpu/drm/digital-blocks/Kconfig     |  13 +
 drivers/gpu/drm/digital-blocks/Makefile    |   3 +
 drivers/gpu/drm/digital-blocks/db9000-du.c | 953 +++++++++++++++++++++++++++++
 drivers/gpu/drm/digital-blocks/db9000-du.h | 192 ++++++
 6 files changed, 1164 insertions(+)
 create mode 100644 drivers/gpu/drm/digital-blocks/Kconfig
 create mode 100644 drivers/gpu/drm/digital-blocks/Makefile
 create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.c
 create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 3c88420..159832d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -280,6 +280,8 @@ source "drivers/gpu/drm/mgag200/Kconfig"
 
 source "drivers/gpu/drm/cirrus/Kconfig"
 
+source "drivers/gpu/drm/digital-blocks/Kconfig"
+
 source "drivers/gpu/drm/armada/Kconfig"
 
 source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 9f0d2ee..f525807 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_DRM_MGAG200) += mgag200/
 obj-$(CONFIG_DRM_V3D)  += v3d/
 obj-$(CONFIG_DRM_VC4)  += vc4/
 obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
+obj-$(CONFIG_DRM_DB9000) += digital-blocks/
 obj-$(CONFIG_DRM_SIS)   += sis/
 obj-$(CONFIG_DRM_SAVAGE)+= savage/
 obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
diff --git a/drivers/gpu/drm/digital-blocks/Kconfig b/drivers/gpu/drm/digital-blocks/Kconfig
new file mode 100644
index 0000000..436a7c0
--- /dev/null
+++ b/drivers/gpu/drm/digital-blocks/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+config DRM_DB9000
+	bool "DRM Support for DB9000 LCD Controller"
+	depends on DRM && (ARCH_MULTIPLATFORM || COMPILE_TEST)
+	select DRM_KMS_HELPER
+	select DRM_GEM_CMA_HELPER
+	select DRM_KMS_CMA_HELPER
+	select DRM_PANEL_BRIDGE
+	select VIDEOMODE_HELPERS
+	select FB_PROVIDE_GET_FB_UNMAPPED_AREA if FB
+
+	help
+	  Enable DRM support for the DB9000 LCD controller.
diff --git a/drivers/gpu/drm/digital-blocks/Makefile b/drivers/gpu/drm/digital-blocks/Makefile
new file mode 100644
index 0000000..9f78492
--- /dev/null
+++ b/drivers/gpu/drm/digital-blocks/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_DRM_DB9000) += db9000-du.o
diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.c b/drivers/gpu/drm/digital-blocks/db9000-du.c
new file mode 100644
index 0000000..d84d446
--- /dev/null
+++ b/drivers/gpu/drm/digital-blocks/db9000-du.c
@@ -0,0 +1,953 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
+ *
+ * Author: Gareth Williams <gareth.williams.jx@renesas.com>
+ *
+ * Based on ltdc.c
+ * Copyright (C) STMicroelectronics SA 2017
+ *
+ * Authors: Philippe Cornu <philippe.cornu@st.com>
+ *          Yannick Fertre <yannick.fertre@st.com>
+ *          Fabien Dessenne <fabien.dessenne@st.com>
+ *          Mickael Reulier <mickael.reulier@st.com>
+ */
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_device.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_drv.h>
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+
+#include <video/display_timing.h>
+#include <video/videomode.h>
+
+#include "db9000-du.h"
+
+#define NR_CRTC		1
+#define DB9000_FB_MAX_WIDTH	4095
+#define DB9000_FB_MAX_HEIGHT	4095
+#define RZN1_REGS		((void *) 1)
+
+static const u32 db9000_fmts[] = {
+	DRM_FORMAT_RGB888,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_BGR888,
+	DRM_FORMAT_BGR565,
+	DRM_FORMAT_XBGR1555,
+	DRM_FORMAT_XBGR8888
+};
+
+static u32 reg_read(void __iomem *base, u32 reg)
+{
+	return readl(base + reg);
+}
+
+static void reg_write(void __iomem *base, u32 reg, u32 val)
+{
+	writel(val, base + reg);
+}
+
+static struct db9000_device *crtc_to_db9000(struct drm_crtc *crtc)
+{
+	return container_of(crtc->dev, struct db9000_device, drm);
+}
+
+static struct db9000_device *plane_to_db9000(struct drm_plane *plane)
+{
+	return container_of(plane->dev, struct db9000_device, drm);
+}
+
+static struct db9000_device *pwm_chip_to_db9000(struct pwm_chip *chip)
+{
+	return container_of(chip, struct db9000_device, pwm);
+}
+
+void db9000_bpp_setup(struct db9000_device *db9000_dev, int bpp, int bus_width,
+		      bool pixelSelect)
+{
+	u32 format;
+	u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
+
+	/* reset the BPP bits */
+	reg_cr1 &= ~DB9000_CR1_BPP(7);
+	reg_cr1 &= ~DB9000_CR1_OPS(5);
+	reg_cr1 &= ~DB9000_CR1_OPS(1);
+	db9000_dev->bpp = bpp;
+
+	switch (bpp) {
+	case 16:
+		if (pixelSelect) {
+			reg_cr1 |= DB9000_CR1_OPS(5);
+			reg_cr1 |= DB9000_CR1_OPS(1);
+		}
+
+		format = DB9000_CR1_BPP(DB9000_CR1_BPP_16bpp);
+		break;
+	case 24:
+	case 32:
+	default:
+		format = DB9000_CR1_BPP(DB9000_CR1_BPP_24bpp);
+	}
+
+	if (bpp <= 16 && bus_width == 24)
+		reg_cr1 |= DB9000_CR1_OPS(2);
+	else
+		reg_cr1 &= ~DB9000_CR1_OPS(2);
+
+	if (bpp == 24)
+		reg_cr1 |= DB9000_CR1_FBP;
+	else
+		reg_cr1 &= ~DB9000_CR1_FBP;
+
+	reg_cr1 |= format;
+	reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
+}
+
+void db9000_toggle_controller(struct db9000_device *db9000_dev, bool on)
+{
+	u32 isr;
+	u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
+	unsigned long flags;
+
+	if (on) {
+		/* Enable controller */
+		reg_cr1 |= DB9000_CR1_LCE;
+		reg_cr1 |= DB9000_CR1_LPE;
+
+		/* DMA Burst Size */
+		reg_cr1 |= DB9000_CR1_FDW(2);
+
+		/* Release pixel clock domain reset */
+		reg_write(db9000_dev->regs, DB9000_PCTR, DB9000_PCTR_PCR);
+
+		/* Enable BAU event for IRQ */
+		spin_lock_irqsave(&db9000_dev->lock, flags);
+		isr = reg_read(db9000_dev->regs, DB9000_ISR);
+		reg_write(db9000_dev->regs, DB9000_ISR, isr | DB9000_ISR_BAU);
+		reg_write(db9000_dev->regs, DB9000_IMR, DB9000_IMR_BAUM);
+		spin_unlock_irqrestore(&db9000_dev->lock, flags);
+
+	} else {
+		/* Disable controller */
+		reg_cr1 &= ~DB9000_CR1_LCE;
+		reg_cr1 &= ~DB9000_CR1_LPE;
+	}
+
+	reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
+}
+
+/* CRTC Functions */
+static void db9000_crtc_atomic_enable(struct drm_crtc *crtc,
+				      struct drm_crtc_state *old_state)
+{
+	struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
+	u32 imr;
+
+	/* Enable IRQ */
+	imr = reg_read(db9000_dev->regs, DB9000_IMR);
+	reg_write(db9000_dev->regs, DB9000_IMR, imr | DB9000_IMR_BAUM);
+}
+
+static void db9000_crtc_atomic_disable(struct drm_crtc *crtc,
+				       struct drm_crtc_state *old_state)
+{
+	struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
+	u32 imr;
+
+	/* disable IRQ */
+	imr = reg_read(db9000_dev->regs, DB9000_IMR);
+	reg_write(db9000_dev->regs, DB9000_IMR, imr & ~DB9000_IMR_BAUM);
+}
+
+static void db9000_crtc_mode_set_nofb(struct drm_crtc *crtc)
+{
+	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+	struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
+	struct videomode vm;
+	int bus_flags = db9000_dev->connector->display_info.bus_flags;
+	u32 vtr1, vtr2, hvter, htr, hpplor, dear_offset;
+	u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
+
+	drm_display_mode_to_videomode(mode, &vm);
+
+	if (mode->flags & DISPLAY_FLAGS_HSYNC_HIGH)
+		reg_cr1 |= DB9000_CR1_HSP;
+	else
+		reg_cr1 &= ~DB9000_CR1_HSP;
+
+	if (mode->flags & DISPLAY_FLAGS_VSYNC_HIGH)
+		reg_cr1 |= DB9000_CR1_VSP;
+	else
+		reg_cr1 &= ~DB9000_CR1_VSP;
+
+	if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
+		reg_cr1 |= DB9000_CR1_DEP;
+	else
+		reg_cr1 &= ~DB9000_CR1_DEP;
+
+	/* Horizontal Timing Register */
+	htr =	DB9000_HTR_HSW(vm.hsync_len) |
+		DB9000_HTR_HBP(vm.hback_porch) |
+		/* Pixels per line */
+		DB9000_HTR_HFP(vm.hfront_porch);
+
+	/* Horizontal Pixels-Per-Line Override */
+	hpplor = vm.hactive | DB9000_HPOE;
+
+	/* Vertical timing registers setup */
+	vtr1 =	DB9000_VTR1_VBP(vm.vback_porch) |
+		DB9000_VTR1_VFP(vm.vfront_porch) |
+		DB9000_VTR1_VSW(vm.vsync_len);
+
+	vtr2 = DB9000_VTR2_LPP(vm.vactive);
+
+	/* Vertical and Horizontal Timing Extension write */
+	hvter =	DB9000_HVTER_HFPE(vm.hfront_porch) |
+		DB9000_HVTER_HBPE(vm.hback_porch) |
+		DB9000_HVTER_VFPE(vm.vback_porch) |
+		DB9000_HVTER_VBPE(vm.vfront_porch);
+
+	db9000_dev->frame_size = vm.hactive * vm.vactive;
+
+	/* DEAR register must be configured to the block end + 8 */
+	dear_offset =
+		(db9000_dev->frame_size * db9000_dev->bpp) / 8 + 8;
+
+	reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
+	reg_write(db9000_dev->regs, DB9000_HTR, htr);
+	reg_write(db9000_dev->regs, DB9000_VTR1, vtr1);
+	reg_write(db9000_dev->regs, DB9000_VTR2, vtr2);
+	reg_write(db9000_dev->regs, DB9000_HPPLOR, hpplor);
+	reg_write(db9000_dev->regs, DB9000_HVTER, hvter);
+
+	DRM_DEBUG_DRIVER("CRTC:%d mode:%s\n", crtc->base.id, mode->name);
+	DRM_DEBUG_DRIVER("Video mode: %dx%d", vm.hactive, vm.vactive);
+	DRM_DEBUG_DRIVER(" hfp %d hbp %d hsl %d vfp %d vbp %d vsl %d\n",
+			 vm.hfront_porch, vm.hback_porch, vm.hsync_len,
+			 vm.vfront_porch, vm.vback_porch, vm.vsync_len);
+}
+
+static void db9000_crtc_atomic_flush(struct drm_crtc *crtc,
+				     struct drm_crtc_state *old_crtc_state)
+{
+	struct drm_pending_vblank_event *event = crtc->state->event;
+
+	if (event) {
+		crtc->state->event = NULL;
+
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+	}
+}
+
+static const struct drm_crtc_helper_funcs db9000_crtc_helper_funcs = {
+	.mode_set_nofb = db9000_crtc_mode_set_nofb,
+	.atomic_flush = db9000_crtc_atomic_flush,
+	.atomic_enable = db9000_crtc_atomic_enable,
+	.atomic_disable = db9000_crtc_atomic_disable,
+};
+
+static const struct drm_crtc_funcs db9000_crtc_funcs = {
+	.destroy = drm_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,
+	.gamma_set = drm_atomic_helper_legacy_gamma_set,
+};
+
+/*
+ * DRM_PLANE
+ */
+static void db9000_plane_atomic_update(struct drm_plane *plane,
+				       struct drm_plane_state *oldstate)
+{
+	struct db9000_device *db9000_dev = plane_to_db9000(plane);
+	struct drm_plane_state *state = plane->state;
+	struct drm_framebuffer *fb = state->fb;
+	unsigned long flags;
+	u32 isr, paddr, dear_offset;
+	u32 format;
+
+	if (!state->crtc || !fb) {
+		DRM_DEBUG_DRIVER("fb or crtc NULL\n");
+		return;
+	}
+
+	format = fb->format->format;
+
+	/* The single plane is turning visible, so turn on the display */
+	if (!oldstate->visible && state->visible)
+		db9000_toggle_controller(db9000_dev, false);
+
+	/* The plane is no longer visible */
+	if (oldstate->visible && !state->visible)
+		db9000_toggle_controller(db9000_dev, true);
+
+	/* Check for format changes */
+	if (format == DRM_FORMAT_RGB565 || format == DRM_FORMAT_BGR565)
+		db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width, false);
+	else if (format == DRM_FORMAT_XRGB1555 || format == DRM_FORMAT_XBGR1555)
+		db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width, true);
+	else if (format == DRM_FORMAT_RGB888 || format == DRM_FORMAT_BGR888)
+		db9000_bpp_setup(db9000_dev, 24, db9000_dev->bus_width, false);
+	else if (format == DRM_FORMAT_XRGB8888 || format == DRM_FORMAT_XBGR8888)
+		db9000_bpp_setup(db9000_dev, 32, db9000_dev->bus_width, false);
+
+	dear_offset = (db9000_dev->frame_size * db9000_dev->bpp) / 8 + 8;
+
+	/* The frame buffer has changed, so get the new FB address */
+	if (oldstate->fb != state->fb || state->crtc->state->mode_changed) {
+		paddr = (u32)drm_fb_cma_get_gem_addr(fb, state, 0);
+
+		DRM_DEBUG_DRIVER("fb: phys 0x%08x\n", paddr);
+		reg_write(db9000_dev->regs, DB9000_DBAR, paddr);
+		reg_write(db9000_dev->regs, DB9000_DEAR,
+			  paddr + dear_offset);
+	}
+
+	/* Enable BAU event */
+	spin_lock_irqsave(&db9000_dev->lock, flags);
+	isr = reg_read(db9000_dev->regs, DB9000_ISR);
+	reg_write(db9000_dev->regs, DB9000_ISR, isr | DB9000_ISR_BAU);
+	reg_write(db9000_dev->regs, DB9000_IMR, DB9000_IMR_BAUM);
+	spin_unlock_irqrestore(&db9000_dev->lock, flags);
+
+	db9000_dev->plane_fpsi->counter++;
+
+	if (isr & DB9000_ISR_MBE) {
+		if (isr & DB9000_ISR_OFU)
+			DRM_ERROR("Output FIFO Underrun\n");
+
+		if (isr & DB9000_ISR_OFO)
+			DRM_ERROR("Output FIFO Overrun\n");
+
+		if (isr & DB9000_ISR_IFU)
+			DRM_ERROR("Input FIFO Underrun\n");
+
+		if (isr & DB9000_ISR_IFO)
+			DRM_ERROR("Input FIFO Overrun\n");
+	}
+}
+
+static void db9000_plane_atomic_print_state(struct drm_printer *p,
+					    const struct drm_plane_state *state)
+{
+	struct drm_plane *plane = state->plane;
+	struct db9000_device *db9000_dev = plane_to_db9000(plane);
+	struct fps_info *fpsi = db9000_dev->plane_fpsi;
+	int ms_since_last;
+	ktime_t now;
+
+	now = ktime_get();
+	ms_since_last = ktime_to_ms(ktime_sub(now, fpsi->last_timestamp));
+
+	drm_printf(p, "\tuser_updates=%dfps\n",
+		   DIV_ROUND_CLOSEST(fpsi->counter * 1000, ms_since_last));
+
+	fpsi->last_timestamp = now;
+	fpsi->counter = 0;
+}
+
+static const struct drm_plane_funcs db9000_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	.reset = drm_atomic_helper_plane_reset,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	.atomic_print_state = db9000_plane_atomic_print_state,
+};
+
+static const struct drm_plane_helper_funcs db9000_plane_helper_funcs = {
+	.atomic_update = db9000_plane_atomic_update,
+};
+
+static struct drm_plane *db9000_plane_create(struct drm_device *ddev,
+					     enum drm_plane_type type)
+{
+	struct device *dev = ddev->dev;
+	struct drm_plane *plane;
+	int ret;
+
+	plane = devm_kzalloc(dev, sizeof(*plane), GFP_KERNEL);
+	if (!plane)
+		return NULL;
+
+	ret = drm_universal_plane_init(ddev, plane, NR_CRTC,
+				       &db9000_plane_funcs,
+				       db9000_fmts,
+				       ARRAY_SIZE(db9000_fmts), NULL,
+				       type, "rzn1_primary_rgb");
+	if (ret < 0)
+		return NULL;
+
+	drm_plane_helper_add(plane, &db9000_plane_helper_funcs);
+
+	DRM_DEBUG_DRIVER("plane:%d created\n", plane->base.id);
+
+	return plane;
+}
+
+static void db9000_plane_destroy_all(struct drm_device *ddev)
+{
+	struct drm_plane *plane, *plane_temp;
+
+	list_for_each_entry_safe(plane, plane_temp,
+				 &ddev->mode_config.plane_list, head)
+				 drm_plane_cleanup(plane);
+}
+
+static int db9000_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
+{
+	struct drm_plane *primary;
+	int ret;
+
+	primary = db9000_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY);
+	if (!primary) {
+		DRM_ERROR("Can not create primary plane\n");
+		return -EINVAL;
+	}
+
+	ret = drm_crtc_init_with_planes(ddev, crtc, primary, NULL,
+					&db9000_crtc_funcs, NULL);
+	if (ret) {
+		DRM_ERROR("Can not initialize CRTC\n");
+		goto cleanup;
+	}
+
+	drm_crtc_helper_add(crtc, &db9000_crtc_helper_funcs);
+	DRM_DEBUG_DRIVER("CRTC:%d created\n", crtc->base.id);
+	return 0;
+
+cleanup:
+	db9000_plane_destroy_all(ddev);
+	return ret;
+}
+
+/*
+ * DRM_ENCODER
+ */
+static const struct drm_encoder_funcs db9000_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static int db9000_encoder_init(struct drm_device *ddev,
+			       struct drm_bridge *bridge)
+{
+	struct drm_encoder *encoder;
+	int ret;
+
+	encoder = devm_kzalloc(ddev->dev, sizeof(*encoder), GFP_KERNEL);
+	if (!encoder)
+		return -ENOMEM;
+
+	encoder->possible_crtcs = NR_CRTC;
+	encoder->possible_clones = 0; /* No cloning support */
+
+	drm_encoder_init(ddev, encoder, &db9000_encoder_funcs,
+			 DRM_MODE_ENCODER_NONE, NULL);
+
+	ret = drm_bridge_attach(encoder, bridge, NULL);
+	if (ret) {
+		drm_encoder_cleanup(encoder);
+		return -EINVAL;
+	}
+
+	DRM_DEBUG_DRIVER("Bridge encoder:%d created\n", encoder->base.id);
+
+	return 0;
+}
+
+void __maybe_unused db9000_suspend(struct drm_device *ddev)
+{
+	struct db9000_device *db9000_dev = container_of(ddev,
+						    struct db9000_device, drm);
+
+	clk_disable_unprepare(db9000_dev->lcd_eclk);
+}
+
+int __maybe_unused db9000_resume(struct drm_device *ddev)
+{
+	struct db9000_device *db9000_dev = container_of(ddev,
+						    struct db9000_device, drm);
+	int ret;
+
+	ret = clk_prepare_enable(db9000_dev->lcd_eclk);
+	if (ret) {
+		DRM_ERROR("failed to enable pixel clock (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int db9000_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	return pm_runtime_get_sync(chip->dev);
+}
+
+static void db9000_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	pm_runtime_put(chip->dev);
+}
+
+static int db9000_pwm_enable(struct db9000_device *db9000_dev)
+{
+	u32 reg_pwmfr = reg_read(db9000_dev->regs, db9000_dev->addr_pwmfr);
+
+	reg_pwmfr |= DB9000_PWMFR_PWMFCE;
+	reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, reg_pwmfr);
+
+	return 0;
+}
+
+static void db9000_pwm_disable(struct db9000_device *db9000_dev)
+{
+	u32 reg_pwmfr = reg_read(db9000_dev->regs, db9000_dev->addr_pwmfr);
+
+	reg_pwmfr &= ~DB9000_PWMFR_PWMFCE;
+	reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, reg_pwmfr);
+}
+
+static int db9000_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			    struct pwm_state *state)
+{
+	struct pwm_state cur_state;
+	struct db9000_device *db9000_dev = pwm_chip_to_db9000(chip);
+	int ret;
+
+	pwm_get_state(pwm, &cur_state);
+
+	if (state->enabled)
+		ret = db9000_pwm_enable(db9000_dev);
+	else
+		db9000_pwm_disable(db9000_dev);
+
+	if (state->period != cur_state.period) {
+		u32 pwmfcd;
+
+		pwmfcd = clk_get_rate(db9000_dev->lcd_eclk) / 256;
+		pwmfcd *= state->period;
+		pwmfcd = DB9000_PWMFR_PWMFCD(pwmfcd - 1);
+		reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, pwmfcd);
+	}
+
+	if (state->duty_cycle == 0) {
+		db9000_pwm_disable(db9000_dev);
+	} else if (state->period != cur_state.period ||
+	    state->duty_cycle != cur_state.duty_cycle) {
+		u32 dcr = div_u64((state->duty_cycle * ULL(256)),
+				   state->period) - 1;
+
+		reg_write(db9000_dev->regs, db9000_dev->addr_pwmdcr, dcr);
+	}
+
+	return ret;
+}
+
+static const struct pwm_ops db9000_pwm_ops = {
+	.request = db9000_pwm_request,
+	.free = db9000_pwm_free,
+	.apply = db9000_pwm_apply,
+	.owner = THIS_MODULE,
+};
+
+static int db9000_pwm_setup(struct device *dev,
+			    struct db9000_device *db9000_dev)
+{
+	struct pwm_chip *db9000_pwm;
+	int ret;
+
+	db9000_pwm = devm_kzalloc(dev, sizeof(*db9000_pwm), GFP_KERNEL);
+	if (db9000_pwm == NULL)
+		return -ENOMEM;
+
+	db9000_pwm = &db9000_dev->pwm;
+
+	db9000_pwm->dev = dev;
+	db9000_pwm->ops = &db9000_pwm_ops;
+	db9000_pwm->base = -1;
+	db9000_pwm->npwm = 1;
+
+	ret = pwmchip_add(db9000_pwm);
+	if (ret < 0) {
+		dev_err(dev, "failed to register PWM chip: %d\n", ret);
+		return ret;
+	}
+
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+int db9000_load(struct drm_device *ddev, int rzn1_pwm)
+{
+	struct platform_device *pdev = to_platform_device(ddev->dev);
+	struct device *dev = ddev->dev;
+	struct db9000_device *db9000_dev = container_of(ddev,
+						    struct db9000_device, drm);
+	struct device_node *np = dev->of_node;
+	struct drm_bridge *bridge;
+	struct drm_panel *panel;
+	struct drm_crtc *crtc;
+	struct reset_control *rstc;
+	struct resource *res;
+	int ret;
+
+	spin_lock_init(&db9000_dev->lock);
+
+	if (rzn1_pwm) {
+		db9000_dev->addr_pwmfr = DB9000_PWMFR_RZN1;
+		db9000_dev->addr_pwmdcr = DB9000_PWMDCR_RZN1;
+	} else {
+		db9000_dev->addr_pwmfr = DB9000_PWMFR_0;
+		db9000_dev->addr_pwmdcr = DB9000_PWMDCR_0;
+	}
+
+	/* Panel Setup */
+	ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge);
+	if (ret != 0) {
+		DRM_ERROR("Could not get Panel or bridge");
+		return ret;
+	}
+
+	rstc = devm_reset_control_get_exclusive(dev, NULL);
+
+	/* Clock setup */
+	db9000_dev->lcd_eclk = devm_clk_get(dev, "lcd_eclk");
+
+	if (IS_ERR(db9000_dev->lcd_eclk)) {
+		DRM_ERROR("Unable to get pixel clock\n");
+		return -ENODEV;
+	}
+
+	if (clk_prepare_enable(db9000_dev->lcd_eclk)) {
+		DRM_ERROR("Unable to prepare pixel clock\n");
+		return -ENODEV;
+	}
+
+	/* Memory setup */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		DRM_ERROR("Could not retrieve platform resources\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	db9000_dev->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(db9000_dev->regs)) {
+		DRM_ERROR("Unable to get memory resource\n");
+		ret = PTR_ERR(db9000_dev->regs);
+		goto err;
+	}
+
+	db9000_bpp_setup(db9000_dev, db9000_dev->bpp, db9000_dev->bus_width,
+			 false);
+
+	db9000_toggle_controller(db9000_dev, true);
+
+	/* Add endpoints panels or bridges if any */
+	if (panel) {
+		bridge = drm_panel_bridge_add(panel,
+					      DRM_MODE_CONNECTOR_Unknown);
+		if (IS_ERR(bridge)) {
+			DRM_ERROR("panel-bridge endpoint\n");
+			ret = PTR_ERR(bridge);
+			goto err;
+		}
+	}
+
+	if (bridge) {
+		ret = db9000_encoder_init(ddev, bridge);
+		if (ret) {
+			DRM_ERROR("init encoder endpoint\n");
+			goto err;
+		}
+	}
+
+	db9000_dev->connector = panel->connector;
+
+	/* CRTC setup  */
+	crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
+	if (!crtc) {
+		DRM_ERROR("Failed to allocate crtc\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ret = db9000_crtc_init(&db9000_dev->drm, crtc);
+	if (ret) {
+		DRM_ERROR("Failed to init crtc\n");
+		goto err;
+	}
+
+	if (!IS_ERR(rstc)) {
+		reset_control_assert(rstc);
+		usleep_range(10, 20);
+		reset_control_deassert(rstc);
+	}
+
+	return db9000_pwm_setup(dev, db9000_dev);
+
+err:
+	drm_panel_bridge_remove(bridge);
+
+	clk_disable_unprepare(db9000_dev->lcd_eclk);
+
+	return ret;
+}
+
+void db9000_unload(struct drm_device *ddev)
+{
+	struct db9000_device *db9000_dev;
+
+	db9000_dev = container_of(ddev, struct db9000_device, drm);
+
+	drm_of_panel_bridge_remove(ddev->dev->of_node, 0, 0);
+	clk_disable_unprepare(db9000_dev->lcd_eclk);
+}
+
+static const struct drm_mode_config_funcs drv_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static int db9000_gem_cma_dumb_create(struct drm_file *file,
+				      struct drm_device *dev,
+				      struct drm_mode_create_dumb *args)
+{
+	return drm_gem_cma_dumb_create_internal(file, dev, args);
+}
+
+DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
+
+static struct drm_driver drv_driver = {
+	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
+			   DRIVER_ATOMIC,
+	.name = "drm-db9000",
+	.desc = "Digital Blocks DB9000 DRM Driver",
+	.date = "20190825",
+	.major = 1,
+	.minor = 0,
+	.patchlevel = 0,
+	.fops = &drv_driver_fops,
+	.dumb_create = db9000_gem_cma_dumb_create,
+	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+	.gem_free_object_unlocked = drm_gem_cma_free_object,
+	.gem_vm_ops = &drm_gem_cma_vm_ops,
+	.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,
+};
+
+static int drv_load(struct drm_device *ddev, u32 bpp,
+		    u32 bus_width, int rzn1_pwm)
+{
+	struct platform_device *pdev = to_platform_device(ddev->dev);
+	struct db9000_device *db9000_dev = container_of(ddev,
+						    struct db9000_device, drm);
+	int ret;
+
+	drm_mode_config_init(ddev);
+
+	/*
+	 * set max width and height as default value.
+	 * this value would be used to check framebuffer size limitation
+	 * at drm_mode_addfb().
+	 */
+	ddev->mode_config.min_width = 0;
+	ddev->mode_config.min_height = 0;
+	ddev->mode_config.max_width = DB9000_FB_MAX_WIDTH;
+	ddev->mode_config.max_height = DB9000_FB_MAX_WIDTH;
+	ddev->mode_config.funcs = &drv_mode_config_funcs;
+
+	db9000_dev->bus_width = bus_width;
+
+	ret = db9000_load(ddev, rzn1_pwm);
+
+	if (ret)
+		goto err;
+
+	drm_mode_config_reset(ddev);
+
+	drm_kms_helper_poll_init(ddev);
+
+	platform_set_drvdata(pdev, ddev);
+
+	return 0;
+
+err:
+	drm_mode_config_cleanup(ddev);
+
+	return ret;
+}
+
+static void drv_unload(struct drm_device *ddev)
+{
+	drm_kms_helper_poll_fini(ddev);
+	db9000_unload(ddev);
+	drm_mode_config_cleanup(ddev);
+}
+
+static __maybe_unused int db9000_drv_suspend(struct device *dev)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct db9000_device *db9000_dev = container_of(ddev,
+							struct db9000_device,
+							drm);
+	struct drm_atomic_state *state;
+
+	db9000_toggle_controller(db9000_dev, false);
+
+	drm_kms_helper_poll_disable(ddev);
+	state = drm_atomic_helper_suspend(ddev);
+	if (IS_ERR(state)) {
+		drm_kms_helper_poll_enable(ddev);
+		return PTR_ERR(state);
+	}
+	db9000_dev->suspend_state = state;
+	db9000_suspend(ddev);
+
+	return 0;
+}
+
+static __maybe_unused int db9000_drv_resume(struct device *dev)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct db9000_device *db9000_dev = container_of(ddev,
+							struct db9000_device,
+							drm);
+
+	db9000_resume(ddev);
+	drm_atomic_helper_resume(ddev, db9000_dev->suspend_state);
+	drm_kms_helper_poll_enable(ddev);
+	db9000_toggle_controller(db9000_dev, true);
+
+	return 0;
+}
+
+static const struct dev_pm_ops drv_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(db9000_drv_suspend, db9000_drv_resume)
+};
+
+static int db9000_drm_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct drm_device *ddev;
+	struct db9000_device *db9000_dev;
+	struct device_node *np = dev->of_node;
+	u32 bpp;
+	u32 bus_width;
+	int rzn1_pwm = 0;
+	int ret;
+
+	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+
+	db9000_dev = kzalloc(sizeof(*db9000_dev), GFP_KERNEL);
+	if (!db9000_dev)
+		return -ENOMEM;
+
+	drm_dev_init(&db9000_dev->drm, &drv_driver, dev);
+	ddev = &db9000_dev->drm;
+
+	/* Parse the DTB */
+	ret = of_property_read_u32(np, "bits-per-pixel", &bpp);
+	if (ret)
+		bpp = 16;
+
+	if (bpp != 16 && bpp != 24 && bpp != 32) {
+		DRM_WARN("bits-per-pixel value invalid, default is 16 bpp");
+		bpp = 16;
+	}
+
+	ret = of_property_read_u32(np, "bus-width", &bus_width);
+	if (ret)
+		bus_width = 24;
+
+	rzn1_pwm = (int) of_device_get_match_data(dev);
+
+	ret = drv_load(ddev, bpp, bus_width, rzn1_pwm);
+	if (ret)
+		goto err_put;
+
+	ret = drm_dev_register(ddev, 0);
+	if (ret)
+		goto err_put;
+
+	ret = drm_fbdev_generic_setup(ddev, bpp);
+	if (ret)
+		goto err_put;
+
+	dev_info(dev, "DB9000 LCD Controller Ready");
+
+	return 0;
+
+err_put:
+	drm_dev_put(ddev);
+
+	return ret;
+}
+
+static int db9000_drm_platform_remove(struct platform_device *pdev)
+{
+	struct drm_device *ddev = platform_get_drvdata(pdev);
+
+	drv_unload(ddev);
+	drm_dev_put(ddev);
+
+	return 0;
+}
+
+static const struct of_device_id drv_dt_ids[] = {
+	{ .compatible = "digital-blocks,drm-db9000"},
+	{ .compatible = "digital-blocks,drm-rzn1", .data = RZN1_REGS },
+	{ /* end node */ },
+};
+MODULE_DEVICE_TABLE(of, drv_dt_ids);
+
+static struct platform_driver db9000_drm_platform_driver = {
+	.probe = db9000_drm_platform_probe,
+	.remove = db9000_drm_platform_remove,
+	.driver = {
+		.name = "drm-db9000",
+		.of_match_table = drv_dt_ids,
+		.pm = &drv_pm_ops,
+	},
+};
+
+module_platform_driver(db9000_drm_platform_driver);
+
+MODULE_AUTHOR("Gareth Williams <gareth.williams.jx@renesas.com>");
+MODULE_DESCRIPTION("Digital Blocks DB9000 LCD Controller Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.h b/drivers/gpu/drm/digital-blocks/db9000-du.h
new file mode 100644
index 0000000..325c9f0
--- /dev/null
+++ b/drivers/gpu/drm/digital-blocks/db9000-du.h
@@ -0,0 +1,192 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
+ *
+ * Author: Gareth Williams <gareth.williams.jx@renesas.com>
+ *
+ * Based on ltdc.h
+ * Copyright (C) STMicroelectronics SA 2017
+ *
+ * Authors: Philippe Cornu <philippe.cornu@st.com>
+ *          Yannick Fertre <yannick.fertre@st.com>
+ *          Fabien Dessenne <fabien.dessenne@st.com>
+ *          Mickael Reulier <mickael.reulier@st.com>
+ */
+#ifndef __DB9000_DU_H__
+#define __DB9000_DU_H__
+
+#include <linux/backlight.h>
+#include <linux/interrupt.h>
+#include <linux/pwm.h>
+
+#define DB9000_MAX_LAYER		1
+
+/* LCD Controller Control Register 1 */
+#define DB9000_CR1			0x000
+/* Horizontal Timing Register */
+#define DB9000_HTR			0x008
+/* Vertical Timing Register 1 */
+#define DB9000_VTR1			0x00C
+/* Vertical Timing Register 2 */
+#define DB9000_VTR2			0x010
+/* Pixel Clock Timing Register */
+#define DB9000_PCTR			0x014
+/* Interrupt Status Register */
+#define DB9000_ISR			0x018
+/* Interrupt Mask Register */
+#define DB9000_IMR			0x01C
+/* Interrupt Vector Register */
+#define DB9000_IVR			0x020
+/* Interrupt Scan Compare Register */
+#define DB9000_ISCR			0x024
+/* DMA Base Address Register */
+#define DB9000_DBAR			0x028
+/* DMA Current Address Register */
+#define DB9000_DCAR			0x02C
+/* DMA End Address Register */
+#define DB9000_DEAR			0x030
+/* DMA Horizontal and Vertical Timing Extension Register */
+#define DB9000_HVTER			0x044
+/* Horizontal Pixels-Per-Line Override Control */
+#define DB9000_HPPLOR			0x048
+/* Horizontal Pixels-Per-Line Override Enable */
+#define DB9000_HPOE			BIT(31)
+/* GPIO Register */
+#define DB9000_GPIOR			0x1F8
+/* Core Identification Register */
+#define DB9000_CIR			0x1FC
+/* Palette Data Words */
+#define DB9000_PALT			0x200
+
+/* Control Register 1, Offset 0x000 */
+/* LCD Controller Enable */
+#define DB9000_CR1_LCE			BIT(0)
+/* LCD Power Enable */
+#define DB9000_CR1_LPE			BIT(1)
+/* LCD Bits per Pixel */
+#define DB9000_CR1_BPP(x)		(((x) & 0x7) << 2)
+/* RGB or BGR Format */
+#define DB9000_CR1_RGB			BIT(5)
+/* Data Enable Polarity */
+#define DB9000_CR1_DEP			BIT(8)
+/* Pixel Clock Polarity */
+#define DB9000_CR1_PCP			BIT(9)
+/* Horizontal Sync Polarity */
+#define DB9000_CR1_HSP			BIT(10)
+/* Vertical Sync Polarity */
+#define DB9000_CR1_VSP			BIT(11)
+/* Output Pixel Select */
+#define DB9000_CR1_OPS(x)		(((x) & 0x7) << 12)
+/* FIFO DMA Request Words */
+#define DB9000_CR1_FDW(x)		(((x) & 0x3) << 16)
+/* LCD 1 or Port Select */
+#define DB9000_CR1_LPS			BIT(18)
+/* Frame Buffer 24bpp Packed Word */
+#define DB9000_CR1_FBP			BIT(19)
+
+enum DB9000_CR1_BPP {
+	/* 1 bit per pixel */
+	DB9000_CR1_BPP_1bpp,
+	/* 2 bits per pixel */
+	DB9000_CR1_BPP_2bpp,
+	/* 4 bits per pixel */
+	DB9000_CR1_BPP_4bpp,
+	/* 8 bits per pixel */
+	DB9000_CR1_BPP_8bpp,
+	/* 16 bits per pixel */
+	DB9000_CR1_BPP_16bpp,
+	/* 18 bits per pixel */
+	DB9000_CR1_BPP_18bpp,
+	/* 24 bits per pixel */
+	DB9000_CR1_BPP_24bpp
+} DB9000_CR1_BPP_VAL;
+
+/* Horizontal Timing Register, Offset 0x008 */
+/* Horizontal Front Porch */
+#define DB9000_HTR_HFP(x)		(((x) & 0xff) << 0)
+/* Pixels per Line */
+#define DB9000_HTR_PPL(x)		(((x) & 0xff) << 8)
+/* Horizontal Back Porch */
+#define DB9000_HTR_HBP(x)		(((x) & 0xff) << 16)
+/* Horizontal Sync Width */
+#define DB9000_HTR_HSW(x)		(((x) & 0xff) << 24)
+
+/* Vertical Timing Register 1, Offset 0x00C */
+/* Vertical Sync Width */
+#define DB9000_VTR1_VSW(x)		(((x) & 0xff) << 0)
+/* Vertical Front Porch */
+#define DB9000_VTR1_VFP(x)		(((x) & 0xff) << 8)
+/* Vertical Back Porch */
+#define DB9000_VTR1_VBP(x)		(((x) & 0xff) << 16)
+
+/* Vertical Timing Register 2, Offset 0x010 */
+/* Lines Per Panel */
+#define DB9000_VTR2_LPP(x)		(((x) & 0xfff) << 0)
+
+/* Vertical and Horizontal Timing Extension Register, Offset 0x044 */
+/* Horizontal Front Porch Extension */
+#define DB9000_HVTER_HFPE(x)		((((x) >> 8) & 0x3) << 0)
+/* Horizontal Back Porch Extension */
+#define DB9000_HVTER_HBPE(x)		((((x) >> 8) & 0x3) << 4)
+/* Vertical Front Porch Extension */
+#define DB9000_HVTER_VFPE(x)		((((x) >> 8) & 0x3) << 8)
+/* Vertical Back Porch Extension */
+#define DB9000_HVTER_VBPE(x)		((((x) >> 8) & 0x3) << 12)
+
+/* clock reset select */
+#define DB9000_PCTR_PCR			BIT(10)
+
+/* Interrupt Status Register, Offset 0x018 */
+#define DB9000_ISR_OFU			BIT(0) /* Output FIFO Underrun */
+#define DB9000_ISR_OFO			BIT(1) /* Output FIFO Overrun */
+#define DB9000_ISR_IFU			BIT(2) /* Input FIFO Underrun */
+#define DB9000_ISR_IFO			BIT(3) /* Input FIFO Overrun */
+#define DB9000_ISR_FER			BIT(4) /* OR of OFU, OFO, IFU, IFO */
+#define DB9000_ISR_MBE			BIT(5) /* Master Bus Error */
+#define DB9000_ISR_VCT			BIT(6) /* Vertical Compare Triggered */
+#define DB9000_ISR_BAU			BIT(7) /* DMA Base Address Register Update to CAR */
+#define DB9000_ISR_LDD			BIT(8) /* LCD Controller Disable Done */
+
+/* Interrupt Mask Register, Offset 0x01C */
+#define DB9000_IMR_OFUM			BIT(0) /* Output FIFO Underrun - Mask */
+#define DB9000_IMR_OFOM			BIT(1) /* Output FIFO Overrun - Mask */
+#define DB9000_IMR_IFUM			BIT(2) /* Input FIFO Underrun - Mask */
+#define DB9000_IMR_IFOM			BIT(3) /* Input FIFO Overrun - Mask */
+#define DB9000_IMR_FERM			BIT(4) /* OR of OFU, OFO, IFU, IFO - Mask */
+#define DB9000_IMR_MBEM			BIT(5) /* Master Bus Error - Mask */
+#define DB9000_IMR_VCTM			BIT(6) /* Vertical Compare Triggered - Mask */
+/* DMA Base Address Register Update to CAR - Mask */
+#define DB9000_IMR_BAUM			BIT(7)
+#define DB9000_IMR_LDDM			BIT(8) /* LCD Controller Disable Done - Mask */
+
+/* PWM Frequency Register */
+#define DB9000_PWMFR_0			0x034
+#define DB9000_PWMFR_RZN1		0x04C
+/* PWM Duty Cycle Register */
+#define DB9000_PWMDCR_0			0x038
+#define DB9000_PWMDCR_RZN1		0x050
+/* PWM Frequency Registers, Offset 0x034 and 0x04c */
+#define DB9000_PWMFR_PWMFCD(x)		(((x) & 0x3fffff) << 0)
+#define DB9000_PWMFR_PWMFCE		BIT(22)
+
+struct fps_info {
+	unsigned int counter;
+	ktime_t last_timestamp;
+};
+
+struct db9000_device {
+	struct drm_device drm;
+	struct pwm_chip pwm;
+	struct drm_connector *connector;
+	void __iomem *regs;
+	spinlock_t lock;
+	struct clk *lcd_eclk;
+	struct fps_info plane_fpsi[DB9000_MAX_LAYER];
+	struct drm_atomic_state *suspend_state;
+	u8 bpp;
+	int bus_width;
+	size_t frame_size;
+	u32 addr_pwmfr;
+	u32 addr_pwmdcr;
+};
+
+#endif /* __DB9000_DU_H__ */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] drm/db9000: Add bindings documentation for LCD controller
  2020-04-27  8:21 [PATCH 0/3] Gareth Williams
  2020-04-27  8:21 ` [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Gareth Williams
@ 2020-04-27  8:21 ` Gareth Williams
  2020-04-27 21:33   ` Rob Herring
  2020-04-27  8:21 ` [PATCH 3/3] drm/panel: simple: Add Newhaven ATXL#-CTP panel Gareth Williams
  2020-04-27 13:21 ` [PATCH 0/3] Gareth Williams
  3 siblings, 1 reply; 20+ messages in thread
From: Gareth Williams @ 2020-04-27  8:21 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Maxime Ripard, Hans Verkuil, Icenowy Zheng,
	Mauro Carvalho Chehab, Vivek Unune, Stephen Rothwell
  Cc: Gareth Williams, Phil Edworthy, dri-devel, devicetree, linux-kernel

Add the DT bindings information for the Digital Blocks DB9000 LCD
controller. Also include documentation for the Renesas RZN1 specific
compatible string.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
 .../devicetree/bindings/display/db9000,du.yaml     | 87 ++++++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml       |  2 +
 2 files changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/db9000,du.yaml

diff --git a/Documentation/devicetree/bindings/display/db9000,du.yaml b/Documentation/devicetree/bindings/display/db9000,du.yaml
new file mode 100644
index 0000000..73a9311
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/db9000,du.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/db9000,du.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: DB9000 LCD Controller
+
+maintainers:
+    - Gareth Williams <gareth.williams.jx@renesas.com>
+
+description: |
+  This is an LCD controller by Digital Blocks available for SoCs. The DB9000
+  controller reads from the framebuffer to display on a single RGB interface.
+  Output may be formatted in RGB or BGR. The driver also supports the PWM
+  logic that is included with the controller.
+
+properties:
+
+  compatible:
+    oneOf:
+      - const: digital-blocks,drm-db9000
+      - const: digital-blocks,drm-rzn1
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: |
+          A phandle and clock-specifier pair to be used as a pixel clock.
+
+  clock-names:
+   items:
+      - const: lcd_eclk
+
+  port:
+    type: object
+    description: The panel endpoint connection.
+
+  bits-per-pixel:
+    description: |
+            Default is 24. This selects the number of bits used to represent
+            a single pixel within the controller.
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    enum: [8, 16, 24, 32]
+
+  bus-width:
+    description: |
+       The width of the interface to the LCD panel. This is needed
+       if the bits-per-pixel property is set to 16 or less, but the board
+       connects to a 24-bit panel. In which case, the controller will shift the
+       16-bit data to the most significant bits of the device. Default is 24.
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible
+  - "#pwm-cells"
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - port
+
+examples:
+  - |+
+    drm@53004000 {
+      compatible = "digital-blocks,drm-db9000";
+      reg = <0x53004000 0x1000>;
+      interrupts = <10 97 120>;
+      clocks = <&sysctrl 26>;
+      clock-names = "clk_slcd";
+      bus-width = <24>;
+      pinctrl-0 = <&pins_lcd>;
+      #pwm-cells = <2>;
+
+      port {
+        drm_point: endpoint@0 {
+          remote-endpoint = <&display_in>;
+        };
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6992bbb..138f76e 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -235,6 +235,8 @@ patternProperties:
     description: Shenzhen Yagu Electronic Technology Co., Ltd.
   "^digi,.*":
     description: Digi International Inc.
+  "^digital-blocks,.*":
+    description: Digital Blocks, Inc.
   "^digilent,.*":
     description: Diglent, Inc.
   "^dioo,.*":
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] drm/panel: simple: Add Newhaven ATXL#-CTP panel
  2020-04-27  8:21 [PATCH 0/3] Gareth Williams
  2020-04-27  8:21 ` [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Gareth Williams
  2020-04-27  8:21 ` [PATCH 2/3] drm/db9000: Add bindings documentation for LCD controller Gareth Williams
@ 2020-04-27  8:21 ` Gareth Williams
  2020-04-27  8:41   ` Sam Ravnborg
  2020-04-27 13:21 ` [PATCH 0/3] Gareth Williams
  3 siblings, 1 reply; 20+ messages in thread
From: Gareth Williams @ 2020-04-27  8:21 UTC (permalink / raw)
  To: Thierry Reding, Sam Ravnborg, David Airlie, Daniel Vetter
  Cc: Gareth Williams, Phil Edworthy, dri-devel, linux-kernel

This commit adds support for the Newhaven ATXL#-CTP panel used by the
Renesas RZ/N1 Demo Board.

Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 5a93c4e..0b57b0e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2021,6 +2021,30 @@ static const struct panel_desc newhaven_nhd_43_480272ef_atxl = {
 		     DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
 };
 
+static const struct display_timing newhaven_nhd_50_800480tf_atxl_timing = {
+	.pixelclock = { 33300000, 33400000, 33500000 },
+	.hactive = { 800, 800, 800 },
+	.hfront_porch = { 40, 40, 40 },
+	.hback_porch = { 88, 88, 88 },
+	.hsync_len = { 1, 1, 1 },
+	.vactive = { 480, 480, 480 },
+	.vfront_porch = { 13, 13, 13 },
+	.vback_porch = { 32, 32, 32 },
+	.vsync_len = { 3, 3, 3 },
+	.flags = DISPLAY_FLAGS_HSYNC_HIGH | DISPLAY_FLAGS_VSYNC_HIGH,
+};
+
+static const struct panel_desc newhaven_nhd_50_800480tf_atxl = {
+		.timings = &newhaven_nhd_50_800480tf_atxl_timing,
+		.num_timings = 1,
+		.bpc = 8,
+		.size = {
+			.width = 400,
+			.height = 300,
+		},
+		.bus_flags = DRM_BUS_FLAG_DE_HIGH,
+};
+
 static const struct display_timing nlt_nl192108ac18_02d_timing = {
 	.pixelclock = { 130000000, 148350000, 163000000 },
 	.hactive = { 1920, 1920, 1920 },
@@ -2964,6 +2988,9 @@ static const struct of_device_id platform_of_match[] = {
 		.compatible = "newhaven,nhd-4.3-480272ef-atxl",
 		.data = &newhaven_nhd_43_480272ef_atxl,
 	}, {
+		.compatible = "newhaven,nhd-5.0-800480tf-atxl",
+		.data = &newhaven_nhd_50_800480tf_atxl,
+	}, {
 		.compatible = "nlt,nl192108ac18-02d",
 		.data = &nlt_nl192108ac18_02d,
 	}, {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] drm/panel: simple: Add Newhaven ATXL#-CTP panel
  2020-04-27  8:21 ` [PATCH 3/3] drm/panel: simple: Add Newhaven ATXL#-CTP panel Gareth Williams
@ 2020-04-27  8:41   ` Sam Ravnborg
  2020-04-27 11:03     ` Gareth Williams
  0 siblings, 1 reply; 20+ messages in thread
From: Sam Ravnborg @ 2020-04-27  8:41 UTC (permalink / raw)
  To: Gareth Williams
  Cc: Thierry Reding, David Airlie, Daniel Vetter, Phil Edworthy,
	dri-devel, linux-kernel

Hi Gareth.

Looking forward to see the other patches - they seem to be stuck
awaiting approval.

A few comments in the following.

	Sam

On Mon, Apr 27, 2020 at 09:21:49AM +0100, Gareth Williams wrote:
> This commit adds support for the Newhaven ATXL#-CTP panel used by the
> Renesas RZ/N1 Demo Board.
> 
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 5a93c4e..0b57b0e 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2021,6 +2021,30 @@ static const struct panel_desc newhaven_nhd_43_480272ef_atxl = {
>  		     DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
>  };
>  
> +static const struct display_timing newhaven_nhd_50_800480tf_atxl_timing = {
> +	.pixelclock = { 33300000, 33400000, 33500000 },
> +	.hactive = { 800, 800, 800 },
> +	.hfront_porch = { 40, 40, 40 },
> +	.hback_porch = { 88, 88, 88 },
> +	.hsync_len = { 1, 1, 1 },
> +	.vactive = { 480, 480, 480 },
> +	.vfront_porch = { 13, 13, 13 },
> +	.vback_porch = { 32, 32, 32 },
> +	.vsync_len = { 3, 3, 3 },
> +	.flags = DISPLAY_FLAGS_HSYNC_HIGH | DISPLAY_FLAGS_VSYNC_HIGH,
> +};
> +
> +static const struct panel_desc newhaven_nhd_50_800480tf_atxl = {
> +		.timings = &newhaven_nhd_50_800480tf_atxl_timing,
> +		.num_timings = 1,
> +		.bpc = 8,
> +		.size = {
> +			.width = 400,
> +			.height = 300,
> +		},
> +		.bus_flags = DRM_BUS_FLAG_DE_HIGH,
Please add DRM_BUS_FLAG_PIXDATA too - as I assume this is an LVDS panel.

> +};
For new panels .connector_type is mandatory.


> +
>  static const struct display_timing nlt_nl192108ac18_02d_timing = {
>  	.pixelclock = { 130000000, 148350000, 163000000 },
>  	.hactive = { 1920, 1920, 1920 },
> @@ -2964,6 +2988,9 @@ static const struct of_device_id platform_of_match[] = {
>  		.compatible = "newhaven,nhd-4.3-480272ef-atxl",
>  		.data = &newhaven_nhd_43_480272ef_atxl,
>  	}, {
> +		.compatible = "newhaven,nhd-5.0-800480tf-atxl",
> +		.data = &newhaven_nhd_50_800480tf_atxl,
> +	}, {

I did not see the other patches - so this is maybe covered.
But newhaven,nhd-5.0-800480tf-atxl must be documented in
panel-simple.yaml before we can apply this.

	Sam

>  		.compatible = "nlt,nl192108ac18-02d",
>  		.data = &nlt_nl192108ac18_02d,
>  	}, {
> -- 
> 2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 3/3] drm/panel: simple: Add Newhaven ATXL#-CTP panel
  2020-04-27  8:41   ` Sam Ravnborg
@ 2020-04-27 11:03     ` Gareth Williams
  0 siblings, 0 replies; 20+ messages in thread
From: Gareth Williams @ 2020-04-27 11:03 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Thierry Reding, David Airlie, Daniel Vetter, Phil Edworthy,
	dri-devel, linux-kernel

Hi Sam,

Thanks for giving feedback. When I send out v2 I'll CC you on the rest of the series
to be sure you have visibility. My responses are below.

> On Mon, Apr 27, 2020 at 09:41:49AM +0100, Sam Ravnborg wrote:
> Hi Gareth.
> 
> Looking forward to see the other patches - they seem to be stuck awaiting
> approval.
> 
> A few comments in the following.
> 
> 	Sam
> 
> On Mon, Apr 27, 2020 at 09:21:49AM +0100, Gareth Williams wrote:
> > This commit adds support for the Newhaven ATXL#-CTP panel used by the
> > Renesas RZ/N1 Demo Board.
> >
> > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 27
> +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c
> > b/drivers/gpu/drm/panel/panel-simple.c
> > index 5a93c4e..0b57b0e 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -2021,6 +2021,30 @@ static const struct panel_desc
> newhaven_nhd_43_480272ef_atxl = {
> >  		     DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,  };
> >
> > +static const struct display_timing
> newhaven_nhd_50_800480tf_atxl_timing = {
> > +	.pixelclock = { 33300000, 33400000, 33500000 },
> > +	.hactive = { 800, 800, 800 },
> > +	.hfront_porch = { 40, 40, 40 },
> > +	.hback_porch = { 88, 88, 88 },
> > +	.hsync_len = { 1, 1, 1 },
> > +	.vactive = { 480, 480, 480 },
> > +	.vfront_porch = { 13, 13, 13 },
> > +	.vback_porch = { 32, 32, 32 },
> > +	.vsync_len = { 3, 3, 3 },
> > +	.flags = DISPLAY_FLAGS_HSYNC_HIGH |
> DISPLAY_FLAGS_VSYNC_HIGH, };
> > +
> > +static const struct panel_desc newhaven_nhd_50_800480tf_atxl = {
> > +		.timings = &newhaven_nhd_50_800480tf_atxl_timing,
> > +		.num_timings = 1,
> > +		.bpc = 8,
> > +		.size = {
> > +			.width = 400,
> > +			.height = 300,
> > +		},
> > +		.bus_flags = DRM_BUS_FLAG_DE_HIGH,
> Please add DRM_BUS_FLAG_PIXDATA too - as I assume this is an LVDS panel.

This is an RGB interface, however the manual lists that the rising edge is
being used so I will include the DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
flag.

> 
> > +};
> For new panels .connector_type is mandatory.
Okay, I will include this in v2 of the series.

> 
> 
> > +
> >  static const struct display_timing nlt_nl192108ac18_02d_timing = {
> >  	.pixelclock = { 130000000, 148350000, 163000000 },
> >  	.hactive = { 1920, 1920, 1920 },
> > @@ -2964,6 +2988,9 @@ static const struct of_device_id
> platform_of_match[] = {
> >  		.compatible = "newhaven,nhd-4.3-480272ef-atxl",
> >  		.data = &newhaven_nhd_43_480272ef_atxl,
> >  	}, {
> > +		.compatible = "newhaven,nhd-5.0-800480tf-atxl",
> > +		.data = &newhaven_nhd_50_800480tf_atxl,
> > +	}, {
> 
> I did not see the other patches - so this is maybe covered.
> But newhaven,nhd-5.0-800480tf-atxl must be documented in panel-
> simple.yaml before we can apply this.

I will include a separate documentation patch in v2 with you
on CC for this.


> 
> 	Sam
> 
> >  		.compatible = "nlt,nl192108ac18-02d",
> >  		.data = &nlt_nl192108ac18_02d,
> >  	}, {
> > --
> > 2.7.4

	Gareth

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 0/3]
  2020-04-27  8:21 [PATCH 0/3] Gareth Williams
                   ` (2 preceding siblings ...)
  2020-04-27  8:21 ` [PATCH 3/3] drm/panel: simple: Add Newhaven ATXL#-CTP panel Gareth Williams
@ 2020-04-27 13:21 ` Gareth Williams
  3 siblings, 0 replies; 20+ messages in thread
From: Gareth Williams @ 2020-04-27 13:21 UTC (permalink / raw)
  To: Gareth Williams, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Hans Verkuil, Icenowy Zheng, Mauro Carvalho Chehab, Vivek Unune,
	Stephen Rothwell, Thierry Reding, Sam Ravnborg
  Cc: Phil Edworthy, Sam Ravnborg, dri-devel, devicetree, linux-kernel

Hi All,

I noticed some API changes that were not present when I first wrote this driver. 
This will need correcting so I will send out a second version and respond 
to Sam Ravnborg's feedback at the same time. I recommend waiting for that
version before reviewing as this will not function on Linux-next otherwise.

Gareth

On Mon, Apr 27, 2020 at 09:21:49AM +0100, Gareth Williams wrote:
> 
> This series adds DRM support for the Digital Blocks db9000 LCD controller with
> RZ/N1 specific changes and updates simple-panel to include the associated
> panel. As this has not previously been documented, also include a yaml file to
> provide this.
> 
> Gareth Williams (3):
>   drm/db9000: Add Digital Blocks DB9000 LCD Controller
>   drm/db9000: Add bindings documentation for LCD controller
>   drm/panel: simple: Add Newhaven ATXL#-CTP panel
> 
>  .../devicetree/bindings/display/db9000,du.yaml     |  87 ++
>  .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
>  drivers/gpu/drm/Kconfig                            |   2 +
>  drivers/gpu/drm/Makefile                           |   1 +
>  drivers/gpu/drm/digital-blocks/Kconfig             |  13 +
>  drivers/gpu/drm/digital-blocks/Makefile            |   3 +
>  drivers/gpu/drm/digital-blocks/db9000-du.c         | 953
> +++++++++++++++++++++
>  drivers/gpu/drm/digital-blocks/db9000-du.h         | 192 +++++
>  drivers/gpu/drm/panel/panel-simple.c               |  27 +
>  9 files changed, 1280 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/db9000,du.yaml
>  create mode 100644 drivers/gpu/drm/digital-blocks/Kconfig
>  create mode 100644 drivers/gpu/drm/digital-blocks/Makefile
>  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.c
>  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.h
> 
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] drm/db9000: Add bindings documentation for LCD controller
  2020-04-27  8:21 ` [PATCH 2/3] drm/db9000: Add bindings documentation for LCD controller Gareth Williams
@ 2020-04-27 21:33   ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2020-04-27 21:33 UTC (permalink / raw)
  To: Gareth Williams
  Cc: David Airlie, Daniel Vetter, Vivek Unune, Stephen Rothwell,
	Gareth Williams, Phil Edworthy, dri-devel, devicetree,
	linux-kernel

On Mon, 27 Apr 2020 09:21:48 +0100, Gareth Williams wrote:
> Add the DT bindings information for the Digital Blocks DB9000 LCD
> controller. Also include documentation for the Renesas RZN1 specific
> compatible string.
> 
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> ---
>  .../devicetree/bindings/display/db9000,du.yaml     | 87 ++++++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml       |  2 +
>  2 files changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/db9000,du.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/display/db9000,du.example.dts:28.35-30.15: Warning (unit_address_vs_reg): /example-0/drm@53004000/port/endpoint@0: node has a unit name, but no reg or ranges property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/db9000,du.example.dt.yaml: drm@53004000: clock-names:0: 'lcd_eclk' was expected

See https://patchwork.ozlabs.org/patch/1277401

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller
  2020-04-27  8:21 ` [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Gareth Williams
@ 2020-04-28 18:18   ` Sam Ravnborg
  2020-04-28 18:23     ` Daniel Vetter
  0 siblings, 1 reply; 20+ messages in thread
From: Sam Ravnborg @ 2020-04-28 18:18 UTC (permalink / raw)
  To: Gareth Williams
  Cc: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Phil Edworthy, linux-kernel, dri-devel

Hi Gareth.

On Mon, Apr 27, 2020 at 09:21:47AM +0100, Gareth Williams wrote:
> Add DRM support for the Digital Blocks DB9000 LCD Controller
> with the Renesas RZ/N1 specific changes.
> 
> Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> ---
>  drivers/gpu/drm/Kconfig                    |   2 +
>  drivers/gpu/drm/Makefile                   |   1 +
>  drivers/gpu/drm/digital-blocks/Kconfig     |  13 +
>  drivers/gpu/drm/digital-blocks/Makefile    |   3 +
>  drivers/gpu/drm/digital-blocks/db9000-du.c | 953 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/digital-blocks/db9000-du.h | 192 ++++++
>  6 files changed, 1164 insertions(+)
>  create mode 100644 drivers/gpu/drm/digital-blocks/Kconfig
>  create mode 100644 drivers/gpu/drm/digital-blocks/Makefile
>  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.c
>  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.h

The general impression is a well written driver.
It looks like it was wrtten some tiem ago and thus fail to take full
benefit from the improvements impemented the last year or so.

The driver has a size so it is a candidate to belong in the tiny/
directory.
If you do not see any other DRM drivers for digital-blocks or that this
driver should be extended, then please consider a move to tiny/

If you do so embed the header file in the .c file so it is a single file
driver.

Other general comments:
The driver looks like a one plane - one crtc - one encoder driver.
Please use drm_simple - or explain why you cannot use drm_simple.

For the encoder use drm_simple_encoder_init

A small intro to the driver would be good.
For example that is includes a pwm etc.

I provided a mix of diverse comments in the following.

Looks forward for the next revision!

	Sam
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 3c88420..159832d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -280,6 +280,8 @@ source "drivers/gpu/drm/mgag200/Kconfig"
>  
>  source "drivers/gpu/drm/cirrus/Kconfig"
>  
> +source "drivers/gpu/drm/digital-blocks/Kconfig"
> +
>  source "drivers/gpu/drm/armada/Kconfig"
>  
>  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 9f0d2ee..f525807 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_DRM_MGAG200) += mgag200/
>  obj-$(CONFIG_DRM_V3D)  += v3d/
>  obj-$(CONFIG_DRM_VC4)  += vc4/
>  obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
> +obj-$(CONFIG_DRM_DB9000) += digital-blocks/
>  obj-$(CONFIG_DRM_SIS)   += sis/
>  obj-$(CONFIG_DRM_SAVAGE)+= savage/
>  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
> diff --git a/drivers/gpu/drm/digital-blocks/Kconfig b/drivers/gpu/drm/digital-blocks/Kconfig
> new file mode 100644
> index 0000000..436a7c0
> --- /dev/null
> +++ b/drivers/gpu/drm/digital-blocks/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config DRM_DB9000
> +	bool "DRM Support for DB9000 LCD Controller"
> +	depends on DRM && (ARCH_MULTIPLATFORM || COMPILE_TEST)
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_CMA_HELPER
> +	select DRM_KMS_CMA_HELPER
> +	select DRM_PANEL_BRIDGE
> +	select VIDEOMODE_HELPERS
> +	select FB_PROVIDE_GET_FB_UNMAPPED_AREA if FB
> +
> +	help
> +	  Enable DRM support for the DB9000 LCD controller.
> diff --git a/drivers/gpu/drm/digital-blocks/Makefile b/drivers/gpu/drm/digital-blocks/Makefile
> new file mode 100644
> index 0000000..9f78492
> --- /dev/null
> +++ b/drivers/gpu/drm/digital-blocks/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_DRM_DB9000) += db9000-du.o
> diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.c b/drivers/gpu/drm/digital-blocks/db9000-du.c
> new file mode 100644
> index 0000000..d84d446
> --- /dev/null
> +++ b/drivers/gpu/drm/digital-blocks/db9000-du.c
> @@ -0,0 +1,953 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
2020?

> + *
> + * Author: Gareth Williams <gareth.williams.jx@renesas.com>
> + *
> + * Based on ltdc.c
> + * Copyright (C) STMicroelectronics SA 2017
> + *
> + * Authors: Philippe Cornu <philippe.cornu@st.com>
> + *          Yannick Fertre <yannick.fertre@st.com>
> + *          Fabien Dessenne <fabien.dessenne@st.com>
> + *          Mickael Reulier <mickael.reulier@st.com>
> + */
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_plane_helper.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_drv.h>
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pwm.h>
> +#include <linux/reset.h>
> +
> +#include <video/display_timing.h>
> +#include <video/videomode.h>
> +
> +#include "db9000-du.h"
> +
> +#define NR_CRTC		1
> +#define DB9000_FB_MAX_WIDTH	4095
> +#define DB9000_FB_MAX_HEIGHT	4095
> +#define RZN1_REGS		((void *) 1)
> +
> +static const u32 db9000_fmts[] = {
> +	DRM_FORMAT_RGB888,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB1555,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_BGR888,
> +	DRM_FORMAT_BGR565,
> +	DRM_FORMAT_XBGR1555,
> +	DRM_FORMAT_XBGR8888
> +};
> +
> +static u32 reg_read(void __iomem *base, u32 reg)
> +{
> +	return readl(base + reg);
> +}
> +
> +static void reg_write(void __iomem *base, u32 reg, u32 val)
> +{
> +	writel(val, base + reg);
> +}
Thiese helpers do not really add any value.
Consider using db9000_device* as first argument, so you do not need to
do a "->regs" for every use.
Or drop them.

db9000_device is so much more than a device.
Why not just name it struct db9000, and then use db9000 as the preferred
variable name?
Just a personal preference, feel free to ignore.


> +
> +static struct db9000_device *crtc_to_db9000(struct drm_crtc *crtc)
> +{
> +	return container_of(crtc->dev, struct db9000_device, drm);
> +}
> +
> +static struct db9000_device *plane_to_db9000(struct drm_plane *plane)
> +{
> +	return container_of(plane->dev, struct db9000_device, drm);
> +}
> +
> +static struct db9000_device *pwm_chip_to_db9000(struct pwm_chip *chip)
> +{
> +	return container_of(chip, struct db9000_device, pwm);
> +}
> +
> +void db9000_bpp_setup(struct db9000_device *db9000_dev, int bpp, int bus_width,
> +		      bool pixelSelect)

The linux kernel style is small caps and underscore like this:
pixel_select

> +{
> +	u32 format;
> +	u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> +
> +	/* reset the BPP bits */
> +	reg_cr1 &= ~DB9000_CR1_BPP(7);
> +	reg_cr1 &= ~DB9000_CR1_OPS(5);
> +	reg_cr1 &= ~DB9000_CR1_OPS(1);
> +	db9000_dev->bpp = bpp;
> +
> +	switch (bpp) {
> +	case 16:
> +		if (pixelSelect) {
> +			reg_cr1 |= DB9000_CR1_OPS(5);
> +			reg_cr1 |= DB9000_CR1_OPS(1);
> +		}
> +
> +		format = DB9000_CR1_BPP(DB9000_CR1_BPP_16bpp);
> +		break;
> +	case 24:
> +	case 32:
> +	default:
> +		format = DB9000_CR1_BPP(DB9000_CR1_BPP_24bpp);
> +	}
> +
> +	if (bpp <= 16 && bus_width == 24)
> +		reg_cr1 |= DB9000_CR1_OPS(2);
> +	else
> +		reg_cr1 &= ~DB9000_CR1_OPS(2);
> +
> +	if (bpp == 24)
> +		reg_cr1 |= DB9000_CR1_FBP;
> +	else
> +		reg_cr1 &= ~DB9000_CR1_FBP;
> +
> +	reg_cr1 |= format;
> +	reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
> +}
> +
> +void db9000_toggle_controller(struct db9000_device *db9000_dev, bool on)

USe two helpers here:
db9000_controller_on()
db9000_controller_off()

More descriptive than the bool flag.

> +{
> +	u32 isr;
> +	u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> +	unsigned long flags;
> +
> +	if (on) {
> +		/* Enable controller */
> +		reg_cr1 |= DB9000_CR1_LCE;
> +		reg_cr1 |= DB9000_CR1_LPE;
> +
> +		/* DMA Burst Size */
> +		reg_cr1 |= DB9000_CR1_FDW(2);
> +
> +		/* Release pixel clock domain reset */
> +		reg_write(db9000_dev->regs, DB9000_PCTR, DB9000_PCTR_PCR);
> +
> +		/* Enable BAU event for IRQ */
> +		spin_lock_irqsave(&db9000_dev->lock, flags);
> +		isr = reg_read(db9000_dev->regs, DB9000_ISR);
> +		reg_write(db9000_dev->regs, DB9000_ISR, isr | DB9000_ISR_BAU);
> +		reg_write(db9000_dev->regs, DB9000_IMR, DB9000_IMR_BAUM);
> +		spin_unlock_irqrestore(&db9000_dev->lock, flags);
> +
> +	} else {
> +		/* Disable controller */
> +		reg_cr1 &= ~DB9000_CR1_LCE;
> +		reg_cr1 &= ~DB9000_CR1_LPE;
> +	}
> +
> +	reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
> +}
> +
> +/* CRTC Functions */
> +static void db9000_crtc_atomic_enable(struct drm_crtc *crtc,
> +				      struct drm_crtc_state *old_state)
> +{
> +	struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> +	u32 imr;
> +
> +	/* Enable IRQ */
> +	imr = reg_read(db9000_dev->regs, DB9000_IMR);
> +	reg_write(db9000_dev->regs, DB9000_IMR, imr | DB9000_IMR_BAUM);
> +}
> +
> +static void db9000_crtc_atomic_disable(struct drm_crtc *crtc,
> +				       struct drm_crtc_state *old_state)
> +{
> +	struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> +	u32 imr;
> +
> +	/* disable IRQ */
> +	imr = reg_read(db9000_dev->regs, DB9000_IMR);
> +	reg_write(db9000_dev->regs, DB9000_IMR, imr & ~DB9000_IMR_BAUM);
> +}
> +
> +static void db9000_crtc_mode_set_nofb(struct drm_crtc *crtc)
> +{
> +	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> +	struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> +	struct videomode vm;
> +	int bus_flags = db9000_dev->connector->display_info.bus_flags;
> +	u32 vtr1, vtr2, hvter, htr, hpplor, dear_offset;
> +	u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> +
> +	drm_display_mode_to_videomode(mode, &vm);
There is no need for this conversion, the timing can be fetched from
display_mode.
And this is the only use of videomode, so you can drop the select in
Kconfig too, and the include.

> +
> +	if (mode->flags & DISPLAY_FLAGS_HSYNC_HIGH)
> +		reg_cr1 |= DB9000_CR1_HSP;
> +	else
> +		reg_cr1 &= ~DB9000_CR1_HSP;
> +
> +	if (mode->flags & DISPLAY_FLAGS_VSYNC_HIGH)
> +		reg_cr1 |= DB9000_CR1_VSP;
> +	else
> +		reg_cr1 &= ~DB9000_CR1_VSP;
> +
> +	if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> +		reg_cr1 |= DB9000_CR1_DEP;
> +	else
> +		reg_cr1 &= ~DB9000_CR1_DEP;
> +
> +	/* Horizontal Timing Register */
> +	htr =	DB9000_HTR_HSW(vm.hsync_len) |
> +		DB9000_HTR_HBP(vm.hback_porch) |
> +		/* Pixels per line */
> +		DB9000_HTR_HFP(vm.hfront_porch);
> +
> +	/* Horizontal Pixels-Per-Line Override */
> +	hpplor = vm.hactive | DB9000_HPOE;
> +
> +	/* Vertical timing registers setup */
> +	vtr1 =	DB9000_VTR1_VBP(vm.vback_porch) |
> +		DB9000_VTR1_VFP(vm.vfront_porch) |
> +		DB9000_VTR1_VSW(vm.vsync_len);
> +
> +	vtr2 = DB9000_VTR2_LPP(vm.vactive);
> +
> +	/* Vertical and Horizontal Timing Extension write */
> +	hvter =	DB9000_HVTER_HFPE(vm.hfront_porch) |
> +		DB9000_HVTER_HBPE(vm.hback_porch) |
> +		DB9000_HVTER_VFPE(vm.vback_porch) |
> +		DB9000_HVTER_VBPE(vm.vfront_porch);
> +
> +	db9000_dev->frame_size = vm.hactive * vm.vactive;
> +
> +	/* DEAR register must be configured to the block end + 8 */
> +	dear_offset =
> +		(db9000_dev->frame_size * db9000_dev->bpp) / 8 + 8;
> +
> +	reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
> +	reg_write(db9000_dev->regs, DB9000_HTR, htr);
> +	reg_write(db9000_dev->regs, DB9000_VTR1, vtr1);
> +	reg_write(db9000_dev->regs, DB9000_VTR2, vtr2);
> +	reg_write(db9000_dev->regs, DB9000_HPPLOR, hpplor);
> +	reg_write(db9000_dev->regs, DB9000_HVTER, hvter);
> +
> +	DRM_DEBUG_DRIVER("CRTC:%d mode:%s\n", crtc->base.id, mode->name);
> +	DRM_DEBUG_DRIVER("Video mode: %dx%d", vm.hactive, vm.vactive);
> +	DRM_DEBUG_DRIVER(" hfp %d hbp %d hsl %d vfp %d vbp %d vsl %d\n",
> +			 vm.hfront_porch, vm.hback_porch, vm.hsync_len,
> +			 vm.vfront_porch, vm.vback_porch, vm.vsync_len);
As a general comment:
use drm_dbg(db9000->drm, "foo") and not the deprecated DRM_DEBUG_DRIVER.

> +}
> +
> +static void db9000_crtc_atomic_flush(struct drm_crtc *crtc,
> +				     struct drm_crtc_state *old_crtc_state)
> +{
> +	struct drm_pending_vblank_event *event = crtc->state->event;
> +
> +	if (event) {
> +		crtc->state->event = NULL;
> +
> +		spin_lock_irq(&crtc->dev->event_lock);
> +		drm_crtc_send_vblank_event(crtc, event);
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +	}
> +}
> +
> +static const struct drm_crtc_helper_funcs db9000_crtc_helper_funcs = {
> +	.mode_set_nofb = db9000_crtc_mode_set_nofb,
> +	.atomic_flush = db9000_crtc_atomic_flush,
> +	.atomic_enable = db9000_crtc_atomic_enable,
> +	.atomic_disable = db9000_crtc_atomic_disable,
> +};
> +
> +static const struct drm_crtc_funcs db9000_crtc_funcs = {
> +	.destroy = drm_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,
> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> +};
> +
> +/*
> + * DRM_PLANE
> + */
> +static void db9000_plane_atomic_update(struct drm_plane *plane,
> +				       struct drm_plane_state *oldstate)
> +{
> +	struct db9000_device *db9000_dev = plane_to_db9000(plane);
> +	struct drm_plane_state *state = plane->state;
> +	struct drm_framebuffer *fb = state->fb;
> +	unsigned long flags;
> +	u32 isr, paddr, dear_offset;
> +	u32 format;
> +
> +	if (!state->crtc || !fb) {
> +		DRM_DEBUG_DRIVER("fb or crtc NULL\n");
> +		return;
> +	}
> +
> +	format = fb->format->format;
> +
> +	/* The single plane is turning visible, so turn on the display */
> +	if (!oldstate->visible && state->visible)
> +		db9000_toggle_controller(db9000_dev, false);
> +
> +	/* The plane is no longer visible */
> +	if (oldstate->visible && !state->visible)
> +		db9000_toggle_controller(db9000_dev, true);
> +
> +	/* Check for format changes */
> +	if (format == DRM_FORMAT_RGB565 || format == DRM_FORMAT_BGR565)
> +		db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width, false);
> +	else if (format == DRM_FORMAT_XRGB1555 || format == DRM_FORMAT_XBGR1555)
> +		db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width, true);
> +	else if (format == DRM_FORMAT_RGB888 || format == DRM_FORMAT_BGR888)
> +		db9000_bpp_setup(db9000_dev, 24, db9000_dev->bus_width, false);
> +	else if (format == DRM_FORMAT_XRGB8888 || format == DRM_FORMAT_XBGR8888)
> +		db9000_bpp_setup(db9000_dev, 32, db9000_dev->bus_width, false);
> +
> +	dear_offset = (db9000_dev->frame_size * db9000_dev->bpp) / 8 + 8;
> +
> +	/* The frame buffer has changed, so get the new FB address */
> +	if (oldstate->fb != state->fb || state->crtc->state->mode_changed) {
> +		paddr = (u32)drm_fb_cma_get_gem_addr(fb, state, 0);
> +
> +		DRM_DEBUG_DRIVER("fb: phys 0x%08x\n", paddr);
> +		reg_write(db9000_dev->regs, DB9000_DBAR, paddr);
> +		reg_write(db9000_dev->regs, DB9000_DEAR,
> +			  paddr + dear_offset);
> +	}
> +
> +	/* Enable BAU event */
> +	spin_lock_irqsave(&db9000_dev->lock, flags);
> +	isr = reg_read(db9000_dev->regs, DB9000_ISR);
> +	reg_write(db9000_dev->regs, DB9000_ISR, isr | DB9000_ISR_BAU);
> +	reg_write(db9000_dev->regs, DB9000_IMR, DB9000_IMR_BAUM);
> +	spin_unlock_irqrestore(&db9000_dev->lock, flags);
> +
> +	db9000_dev->plane_fpsi->counter++;
> +
> +	if (isr & DB9000_ISR_MBE) {
> +		if (isr & DB9000_ISR_OFU)
> +			DRM_ERROR("Output FIFO Underrun\n");
> +
> +		if (isr & DB9000_ISR_OFO)
> +			DRM_ERROR("Output FIFO Overrun\n");
> +
> +		if (isr & DB9000_ISR_IFU)
> +			DRM_ERROR("Input FIFO Underrun\n");
> +
> +		if (isr & DB9000_ISR_IFO)
> +			DRM_ERROR("Input FIFO Overrun\n");
> +	}
> +}
> +
> +static void db9000_plane_atomic_print_state(struct drm_printer *p,
> +					    const struct drm_plane_state *state)
> +{
> +	struct drm_plane *plane = state->plane;
> +	struct db9000_device *db9000_dev = plane_to_db9000(plane);
> +	struct fps_info *fpsi = db9000_dev->plane_fpsi;
> +	int ms_since_last;
> +	ktime_t now;
> +
> +	now = ktime_get();
> +	ms_since_last = ktime_to_ms(ktime_sub(now, fpsi->last_timestamp));
> +
> +	drm_printf(p, "\tuser_updates=%dfps\n",
> +		   DIV_ROUND_CLOSEST(fpsi->counter * 1000, ms_since_last));
> +
> +	fpsi->last_timestamp = now;
> +	fpsi->counter = 0;
> +}
> +
> +static const struct drm_plane_funcs db9000_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = drm_plane_cleanup,
> +	.reset = drm_atomic_helper_plane_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +	.atomic_print_state = db9000_plane_atomic_print_state,
> +};
> +
> +static const struct drm_plane_helper_funcs db9000_plane_helper_funcs = {
> +	.atomic_update = db9000_plane_atomic_update,
> +};
> +
> +static struct drm_plane *db9000_plane_create(struct drm_device *ddev,
> +					     enum drm_plane_type type)
> +{
> +	struct device *dev = ddev->dev;
> +	struct drm_plane *plane;
> +	int ret;
> +
> +	plane = devm_kzalloc(dev, sizeof(*plane), GFP_KERNEL);
> +	if (!plane)
> +		return NULL;
> +
> +	ret = drm_universal_plane_init(ddev, plane, NR_CRTC,
> +				       &db9000_plane_funcs,
> +				       db9000_fmts,
> +				       ARRAY_SIZE(db9000_fmts), NULL,
> +				       type, "rzn1_primary_rgb");
> +	if (ret < 0)
> +		return NULL;
> +
> +	drm_plane_helper_add(plane, &db9000_plane_helper_funcs);
> +
> +	DRM_DEBUG_DRIVER("plane:%d created\n", plane->base.id);
> +
> +	return plane;
> +}
> +
> +static void db9000_plane_destroy_all(struct drm_device *ddev)
> +{
> +	struct drm_plane *plane, *plane_temp;
> +
> +	list_for_each_entry_safe(plane, plane_temp,
> +				 &ddev->mode_config.plane_list, head)
> +				 drm_plane_cleanup(plane);
> +}
> +
> +static int db9000_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
> +{
> +	struct drm_plane *primary;
> +	int ret;
> +
> +	primary = db9000_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY);
> +	if (!primary) {
> +		DRM_ERROR("Can not create primary plane\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = drm_crtc_init_with_planes(ddev, crtc, primary, NULL,
> +					&db9000_crtc_funcs, NULL);
> +	if (ret) {
> +		DRM_ERROR("Can not initialize CRTC\n");
> +		goto cleanup;
> +	}
> +
> +	drm_crtc_helper_add(crtc, &db9000_crtc_helper_funcs);
> +	DRM_DEBUG_DRIVER("CRTC:%d created\n", crtc->base.id);
> +	return 0;
> +
> +cleanup:
> +	db9000_plane_destroy_all(ddev);
> +	return ret;
> +}
> +
> +/*
> + * DRM_ENCODER
> + */
> +static const struct drm_encoder_funcs db9000_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +static int db9000_encoder_init(struct drm_device *ddev,
> +			       struct drm_bridge *bridge)
> +{
> +	struct drm_encoder *encoder;
> +	int ret;
> +
> +	encoder = devm_kzalloc(ddev->dev, sizeof(*encoder), GFP_KERNEL);
> +	if (!encoder)
> +		return -ENOMEM;
> +
> +	encoder->possible_crtcs = NR_CRTC;
> +	encoder->possible_clones = 0; /* No cloning support */
> +
> +	drm_encoder_init(ddev, encoder, &db9000_encoder_funcs,
> +			 DRM_MODE_ENCODER_NONE, NULL);
> +
> +	ret = drm_bridge_attach(encoder, bridge, NULL);
> +	if (ret) {
> +		drm_encoder_cleanup(encoder);
> +		return -EINVAL;
> +	}
> +
> +	DRM_DEBUG_DRIVER("Bridge encoder:%d created\n", encoder->base.id);
> +
> +	return 0;
> +}
> +
> +void __maybe_unused db9000_suspend(struct drm_device *ddev)
> +{
> +	struct db9000_device *db9000_dev = container_of(ddev,
> +						    struct db9000_device, drm);
> +
> +	clk_disable_unprepare(db9000_dev->lcd_eclk);
> +}
> +
> +int __maybe_unused db9000_resume(struct drm_device *ddev)
> +{
> +	struct db9000_device *db9000_dev = container_of(ddev,
> +						    struct db9000_device, drm);
> +	int ret;
> +
> +	ret = clk_prepare_enable(db9000_dev->lcd_eclk);
> +	if (ret) {
> +		DRM_ERROR("failed to enable pixel clock (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int db9000_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	return pm_runtime_get_sync(chip->dev);
> +}
> +
> +static void db9000_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	pm_runtime_put(chip->dev);
> +}
> +
> +static int db9000_pwm_enable(struct db9000_device *db9000_dev)
> +{
> +	u32 reg_pwmfr = reg_read(db9000_dev->regs, db9000_dev->addr_pwmfr);
> +
> +	reg_pwmfr |= DB9000_PWMFR_PWMFCE;
> +	reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, reg_pwmfr);
> +
> +	return 0;
> +}
> +
> +static void db9000_pwm_disable(struct db9000_device *db9000_dev)
> +{
> +	u32 reg_pwmfr = reg_read(db9000_dev->regs, db9000_dev->addr_pwmfr);
> +
> +	reg_pwmfr &= ~DB9000_PWMFR_PWMFCE;
> +	reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, reg_pwmfr);
> +}
> +
> +static int db9000_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    struct pwm_state *state)
> +{
> +	struct pwm_state cur_state;
> +	struct db9000_device *db9000_dev = pwm_chip_to_db9000(chip);
> +	int ret;
> +
> +	pwm_get_state(pwm, &cur_state);
> +
> +	if (state->enabled)
> +		ret = db9000_pwm_enable(db9000_dev);
> +	else
> +		db9000_pwm_disable(db9000_dev);
> +
> +	if (state->period != cur_state.period) {
> +		u32 pwmfcd;
> +
> +		pwmfcd = clk_get_rate(db9000_dev->lcd_eclk) / 256;
> +		pwmfcd *= state->period;
> +		pwmfcd = DB9000_PWMFR_PWMFCD(pwmfcd - 1);
> +		reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, pwmfcd);
> +	}
> +
> +	if (state->duty_cycle == 0) {
> +		db9000_pwm_disable(db9000_dev);
> +	} else if (state->period != cur_state.period ||
> +	    state->duty_cycle != cur_state.duty_cycle) {
> +		u32 dcr = div_u64((state->duty_cycle * ULL(256)),
> +				   state->period) - 1;
> +
> +		reg_write(db9000_dev->regs, db9000_dev->addr_pwmdcr, dcr);
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct pwm_ops db9000_pwm_ops = {
> +	.request = db9000_pwm_request,
> +	.free = db9000_pwm_free,
> +	.apply = db9000_pwm_apply,
> +	.owner = THIS_MODULE,
> +};
> +
> +static int db9000_pwm_setup(struct device *dev,
> +			    struct db9000_device *db9000_dev)
> +{
> +	struct pwm_chip *db9000_pwm;
> +	int ret;
> +
> +	db9000_pwm = devm_kzalloc(dev, sizeof(*db9000_pwm), GFP_KERNEL);
> +	if (db9000_pwm == NULL)
> +		return -ENOMEM;
> +
> +	db9000_pwm = &db9000_dev->pwm;
> +
> +	db9000_pwm->dev = dev;
> +	db9000_pwm->ops = &db9000_pwm_ops;
> +	db9000_pwm->base = -1;
> +	db9000_pwm->npwm = 1;
> +
> +	ret = pwmchip_add(db9000_pwm);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to register PWM chip: %d\n", ret);
> +		return ret;
> +	}
> +
> +	pm_runtime_enable(dev);
> +
> +	return 0;
> +}
> +
> +int db9000_load(struct drm_device *ddev, int rzn1_pwm)
> +{
> +	struct platform_device *pdev = to_platform_device(ddev->dev);
> +	struct device *dev = ddev->dev;
> +	struct db9000_device *db9000_dev = container_of(ddev,
> +						    struct db9000_device, drm);
> +	struct device_node *np = dev->of_node;
> +	struct drm_bridge *bridge;
> +	struct drm_panel *panel;
> +	struct drm_crtc *crtc;
> +	struct reset_control *rstc;
> +	struct resource *res;
> +	int ret;
> +
> +	spin_lock_init(&db9000_dev->lock);
> +
> +	if (rzn1_pwm) {
> +		db9000_dev->addr_pwmfr = DB9000_PWMFR_RZN1;
> +		db9000_dev->addr_pwmdcr = DB9000_PWMDCR_RZN1;
> +	} else {
> +		db9000_dev->addr_pwmfr = DB9000_PWMFR_0;
> +		db9000_dev->addr_pwmdcr = DB9000_PWMDCR_0;
> +	}
> +
> +	/* Panel Setup */
> +	ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge);
> +	if (ret != 0) {
> +		DRM_ERROR("Could not get Panel or bridge");
> +		return ret;
> +	}
> +
> +	rstc = devm_reset_control_get_exclusive(dev, NULL);
> +
> +	/* Clock setup */
> +	db9000_dev->lcd_eclk = devm_clk_get(dev, "lcd_eclk");
> +
> +	if (IS_ERR(db9000_dev->lcd_eclk)) {
> +		DRM_ERROR("Unable to get pixel clock\n");
> +		return -ENODEV;
> +	}
> +
> +	if (clk_prepare_enable(db9000_dev->lcd_eclk)) {
> +		DRM_ERROR("Unable to prepare pixel clock\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Memory setup */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		DRM_ERROR("Could not retrieve platform resources\n");
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	db9000_dev->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(db9000_dev->regs)) {
> +		DRM_ERROR("Unable to get memory resource\n");
> +		ret = PTR_ERR(db9000_dev->regs);
> +		goto err;
> +	}
> +
> +	db9000_bpp_setup(db9000_dev, db9000_dev->bpp, db9000_dev->bus_width,
> +			 false);
> +
> +	db9000_toggle_controller(db9000_dev, true);
> +
> +	/* Add endpoints panels or bridges if any */
> +	if (panel) {
> +		bridge = drm_panel_bridge_add(panel,
> +					      DRM_MODE_CONNECTOR_Unknown);
> +		if (IS_ERR(bridge)) {
> +			DRM_ERROR("panel-bridge endpoint\n");
> +			ret = PTR_ERR(bridge);
> +			goto err;
> +		}
> +	}
> +
> +	if (bridge) {
> +		ret = db9000_encoder_init(ddev, bridge);
> +		if (ret) {
> +			DRM_ERROR("init encoder endpoint\n");
> +			goto err;
> +		}
> +	}
> +
> +	db9000_dev->connector = panel->connector;
> +
> +	/* CRTC setup  */
> +	crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
> +	if (!crtc) {
> +		DRM_ERROR("Failed to allocate crtc\n");
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	ret = db9000_crtc_init(&db9000_dev->drm, crtc);
> +	if (ret) {
> +		DRM_ERROR("Failed to init crtc\n");
> +		goto err;
> +	}
> +
> +	if (!IS_ERR(rstc)) {
> +		reset_control_assert(rstc);
> +		usleep_range(10, 20);
> +		reset_control_deassert(rstc);
> +	}
> +
> +	return db9000_pwm_setup(dev, db9000_dev);
> +
> +err:
> +	drm_panel_bridge_remove(bridge);
> +
> +	clk_disable_unprepare(db9000_dev->lcd_eclk);
> +
> +	return ret;
> +}
> +
> +void db9000_unload(struct drm_device *ddev)
> +{
> +	struct db9000_device *db9000_dev;
> +
> +	db9000_dev = container_of(ddev, struct db9000_device, drm);
> +
> +	drm_of_panel_bridge_remove(ddev->dev->of_node, 0, 0);
> +	clk_disable_unprepare(db9000_dev->lcd_eclk);
> +}
> +
> +static const struct drm_mode_config_funcs drv_mode_config_funcs = {
> +	.fb_create = drm_gem_fb_create,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static int db9000_gem_cma_dumb_create(struct drm_file *file,
> +				      struct drm_device *dev,
> +				      struct drm_mode_create_dumb *args)
> +{
> +	return drm_gem_cma_dumb_create_internal(file, dev, args);
> +}
This wrapper seems like it can be dropped.

> +
> +DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
> +
> +static struct drm_driver drv_driver = {
> +	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
> +			   DRIVER_ATOMIC,
> +	.name = "drm-db9000",
> +	.desc = "Digital Blocks DB9000 DRM Driver",
> +	.date = "20190825",
> +	.major = 1,
> +	.minor = 0,
> +	.patchlevel = 0,
> +	.fops = &drv_driver_fops,
> +	.dumb_create = db9000_gem_cma_dumb_create,
> +	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> +	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> +	.gem_free_object_unlocked = drm_gem_cma_free_object,
> +	.gem_vm_ops = &drm_gem_cma_vm_ops,
> +	.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,
> +};
I think if you look up these operations, then you have assinged the
default operations in mayny cases.
Please see if you can trim the list.

Also double check that no legacy operatiosn are used.

> +
> +static int drv_load(struct drm_device *ddev, u32 bpp,
> +		    u32 bus_width, int rzn1_pwm)
> +{
> +	struct platform_device *pdev = to_platform_device(ddev->dev);
> +	struct db9000_device *db9000_dev = container_of(ddev,
> +						    struct db9000_device, drm);
> +	int ret;
> +
> +	drm_mode_config_init(ddev);
> +
> +	/*
> +	 * set max width and height as default value.
> +	 * this value would be used to check framebuffer size limitation
> +	 * at drm_mode_addfb().
> +	 */
> +	ddev->mode_config.min_width = 0;
> +	ddev->mode_config.min_height = 0;
> +	ddev->mode_config.max_width = DB9000_FB_MAX_WIDTH;
> +	ddev->mode_config.max_height = DB9000_FB_MAX_WIDTH;
> +	ddev->mode_config.funcs = &drv_mode_config_funcs;
> +
> +	db9000_dev->bus_width = bus_width;
> +
> +	ret = db9000_load(ddev, rzn1_pwm);
> +
> +	if (ret)
> +		goto err;
> +
> +	drm_mode_config_reset(ddev);
> +
> +	drm_kms_helper_poll_init(ddev);
> +
> +	platform_set_drvdata(pdev, ddev);
> +
> +	return 0;
> +
> +err:
> +	drm_mode_config_cleanup(ddev);
> +
> +	return ret;
> +}
> +
> +static void drv_unload(struct drm_device *ddev)
> +{
> +	drm_kms_helper_poll_fini(ddev);
> +	db9000_unload(ddev);
> +	drm_mode_config_cleanup(ddev);
> +}
> +
> +static __maybe_unused int db9000_drv_suspend(struct device *dev)
> +{
> +	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct db9000_device *db9000_dev = container_of(ddev,
> +							struct db9000_device,
> +							drm);
> +	struct drm_atomic_state *state;
> +
> +	db9000_toggle_controller(db9000_dev, false);
> +
> +	drm_kms_helper_poll_disable(ddev);
> +	state = drm_atomic_helper_suspend(ddev);
> +	if (IS_ERR(state)) {
> +		drm_kms_helper_poll_enable(ddev);
> +		return PTR_ERR(state);
> +	}
> +	db9000_dev->suspend_state = state;
> +	db9000_suspend(ddev);
> +
> +	return 0;
> +}
> +
> +static __maybe_unused int db9000_drv_resume(struct device *dev)
> +{
> +	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct db9000_device *db9000_dev = container_of(ddev,
> +							struct db9000_device,
> +							drm);
> +
> +	db9000_resume(ddev);
> +	drm_atomic_helper_resume(ddev, db9000_dev->suspend_state);
> +	drm_kms_helper_poll_enable(ddev);
> +	db9000_toggle_controller(db9000_dev, true);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops drv_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(db9000_drv_suspend, db9000_drv_resume)
> +};
> +
> +static int db9000_drm_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct drm_device *ddev;
> +	struct db9000_device *db9000_dev;
> +	struct device_node *np = dev->of_node;
> +	u32 bpp;
> +	u32 bus_width;
> +	int rzn1_pwm = 0;
> +	int ret;
> +
> +	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> +
> +	db9000_dev = kzalloc(sizeof(*db9000_dev), GFP_KERNEL);
> +	if (!db9000_dev)
> +		return -ENOMEM;
> +
> +	drm_dev_init(&db9000_dev->drm, &drv_driver, dev);
> +	ddev = &db9000_dev->drm;
Use the newly merged drmm stuff here.

> +
> +	/* Parse the DTB */
> +	ret = of_property_read_u32(np, "bits-per-pixel", &bpp);
> +	if (ret)
> +		bpp = 16;
> +
> +	if (bpp != 16 && bpp != 24 && bpp != 32) {
> +		DRM_WARN("bits-per-pixel value invalid, default is 16 bpp");
> +		bpp = 16;
> +	}
> +
> +	ret = of_property_read_u32(np, "bus-width", &bus_width);
> +	if (ret)
> +		bus_width = 24;
> +
> +	rzn1_pwm = (int) of_device_get_match_data(dev);
> +
> +	ret = drv_load(ddev, bpp, bus_width, rzn1_pwm);
> +	if (ret)
> +		goto err_put;
> +
> +	ret = drm_dev_register(ddev, 0);
> +	if (ret)
> +		goto err_put;
> +
> +	ret = drm_fbdev_generic_setup(ddev, bpp);
> +	if (ret)
> +		goto err_put;
> +
> +	dev_info(dev, "DB9000 LCD Controller Ready");
> +
> +	return 0;
> +
> +err_put:
> +	drm_dev_put(ddev);
With drmm_ support this put goes aways.
> +
> +	return ret;
> +}
> +
> +static int db9000_drm_platform_remove(struct platform_device *pdev)
> +{
> +	struct drm_device *ddev = platform_get_drvdata(pdev);
> +
> +	drv_unload(ddev);
> +	drm_dev_put(ddev);
With drmm_ support this put goes aways.
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id drv_dt_ids[] = {
> +	{ .compatible = "digital-blocks,drm-db9000"},
> +	{ .compatible = "digital-blocks,drm-rzn1", .data = RZN1_REGS },
> +	{ /* end node */ },
> +};
> +MODULE_DEVICE_TABLE(of, drv_dt_ids);
> +
> +static struct platform_driver db9000_drm_platform_driver = {
> +	.probe = db9000_drm_platform_probe,
> +	.remove = db9000_drm_platform_remove,
> +	.driver = {
> +		.name = "drm-db9000",
> +		.of_match_table = drv_dt_ids,
> +		.pm = &drv_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(db9000_drm_platform_driver);
> +
> +MODULE_AUTHOR("Gareth Williams <gareth.williams.jx@renesas.com>");
> +MODULE_DESCRIPTION("Digital Blocks DB9000 LCD Controller Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.h b/drivers/gpu/drm/digital-blocks/db9000-du.h
> new file mode 100644
> index 0000000..325c9f0
> --- /dev/null
> +++ b/drivers/gpu/drm/digital-blocks/db9000-du.h
> @@ -0,0 +1,192 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
> + *
> + * Author: Gareth Williams <gareth.williams.jx@renesas.com>
> + *
> + * Based on ltdc.h
> + * Copyright (C) STMicroelectronics SA 2017
> + *
> + * Authors: Philippe Cornu <philippe.cornu@st.com>
> + *          Yannick Fertre <yannick.fertre@st.com>
> + *          Fabien Dessenne <fabien.dessenne@st.com>
> + *          Mickael Reulier <mickael.reulier@st.com>
> + */
> +#ifndef __DB9000_DU_H__
> +#define __DB9000_DU_H__
> +
> +#include <linux/backlight.h>
> +#include <linux/interrupt.h>
> +#include <linux/pwm.h>
> +
> +#define DB9000_MAX_LAYER		1
> +
> +/* LCD Controller Control Register 1 */
> +#define DB9000_CR1			0x000
> +/* Horizontal Timing Register */
> +#define DB9000_HTR			0x008
> +/* Vertical Timing Register 1 */
> +#define DB9000_VTR1			0x00C
> +/* Vertical Timing Register 2 */
> +#define DB9000_VTR2			0x010
> +/* Pixel Clock Timing Register */
> +#define DB9000_PCTR			0x014
> +/* Interrupt Status Register */
> +#define DB9000_ISR			0x018
> +/* Interrupt Mask Register */
> +#define DB9000_IMR			0x01C
> +/* Interrupt Vector Register */
> +#define DB9000_IVR			0x020
> +/* Interrupt Scan Compare Register */
> +#define DB9000_ISCR			0x024
> +/* DMA Base Address Register */
> +#define DB9000_DBAR			0x028
> +/* DMA Current Address Register */
> +#define DB9000_DCAR			0x02C
> +/* DMA End Address Register */
> +#define DB9000_DEAR			0x030
> +/* DMA Horizontal and Vertical Timing Extension Register */
> +#define DB9000_HVTER			0x044
> +/* Horizontal Pixels-Per-Line Override Control */
> +#define DB9000_HPPLOR			0x048
> +/* Horizontal Pixels-Per-Line Override Enable */
> +#define DB9000_HPOE			BIT(31)
> +/* GPIO Register */
> +#define DB9000_GPIOR			0x1F8
> +/* Core Identification Register */
> +#define DB9000_CIR			0x1FC
> +/* Palette Data Words */
> +#define DB9000_PALT			0x200
> +
> +/* Control Register 1, Offset 0x000 */
> +/* LCD Controller Enable */
> +#define DB9000_CR1_LCE			BIT(0)
> +/* LCD Power Enable */
> +#define DB9000_CR1_LPE			BIT(1)
> +/* LCD Bits per Pixel */
> +#define DB9000_CR1_BPP(x)		(((x) & 0x7) << 2)
> +/* RGB or BGR Format */
> +#define DB9000_CR1_RGB			BIT(5)
> +/* Data Enable Polarity */
> +#define DB9000_CR1_DEP			BIT(8)
> +/* Pixel Clock Polarity */
> +#define DB9000_CR1_PCP			BIT(9)
> +/* Horizontal Sync Polarity */
> +#define DB9000_CR1_HSP			BIT(10)
> +/* Vertical Sync Polarity */
> +#define DB9000_CR1_VSP			BIT(11)
> +/* Output Pixel Select */
> +#define DB9000_CR1_OPS(x)		(((x) & 0x7) << 12)
> +/* FIFO DMA Request Words */
> +#define DB9000_CR1_FDW(x)		(((x) & 0x3) << 16)
> +/* LCD 1 or Port Select */
> +#define DB9000_CR1_LPS			BIT(18)
> +/* Frame Buffer 24bpp Packed Word */
> +#define DB9000_CR1_FBP			BIT(19)
> +
> +enum DB9000_CR1_BPP {
> +	/* 1 bit per pixel */
> +	DB9000_CR1_BPP_1bpp,
> +	/* 2 bits per pixel */
> +	DB9000_CR1_BPP_2bpp,
> +	/* 4 bits per pixel */
> +	DB9000_CR1_BPP_4bpp,
> +	/* 8 bits per pixel */
> +	DB9000_CR1_BPP_8bpp,
> +	/* 16 bits per pixel */
> +	DB9000_CR1_BPP_16bpp,
> +	/* 18 bits per pixel */
> +	DB9000_CR1_BPP_18bpp,
> +	/* 24 bits per pixel */
> +	DB9000_CR1_BPP_24bpp
> +} DB9000_CR1_BPP_VAL;
> +
> +/* Horizontal Timing Register, Offset 0x008 */
> +/* Horizontal Front Porch */
> +#define DB9000_HTR_HFP(x)		(((x) & 0xff) << 0)
> +/* Pixels per Line */
> +#define DB9000_HTR_PPL(x)		(((x) & 0xff) << 8)
> +/* Horizontal Back Porch */
> +#define DB9000_HTR_HBP(x)		(((x) & 0xff) << 16)
> +/* Horizontal Sync Width */
> +#define DB9000_HTR_HSW(x)		(((x) & 0xff) << 24)
> +
> +/* Vertical Timing Register 1, Offset 0x00C */
> +/* Vertical Sync Width */
> +#define DB9000_VTR1_VSW(x)		(((x) & 0xff) << 0)
> +/* Vertical Front Porch */
> +#define DB9000_VTR1_VFP(x)		(((x) & 0xff) << 8)
> +/* Vertical Back Porch */
> +#define DB9000_VTR1_VBP(x)		(((x) & 0xff) << 16)
> +
> +/* Vertical Timing Register 2, Offset 0x010 */
> +/* Lines Per Panel */
> +#define DB9000_VTR2_LPP(x)		(((x) & 0xfff) << 0)
> +
> +/* Vertical and Horizontal Timing Extension Register, Offset 0x044 */
> +/* Horizontal Front Porch Extension */
> +#define DB9000_HVTER_HFPE(x)		((((x) >> 8) & 0x3) << 0)
> +/* Horizontal Back Porch Extension */
> +#define DB9000_HVTER_HBPE(x)		((((x) >> 8) & 0x3) << 4)
> +/* Vertical Front Porch Extension */
> +#define DB9000_HVTER_VFPE(x)		((((x) >> 8) & 0x3) << 8)
> +/* Vertical Back Porch Extension */
> +#define DB9000_HVTER_VBPE(x)		((((x) >> 8) & 0x3) << 12)
> +
> +/* clock reset select */
> +#define DB9000_PCTR_PCR			BIT(10)
> +
> +/* Interrupt Status Register, Offset 0x018 */
> +#define DB9000_ISR_OFU			BIT(0) /* Output FIFO Underrun */
> +#define DB9000_ISR_OFO			BIT(1) /* Output FIFO Overrun */
> +#define DB9000_ISR_IFU			BIT(2) /* Input FIFO Underrun */
> +#define DB9000_ISR_IFO			BIT(3) /* Input FIFO Overrun */
> +#define DB9000_ISR_FER			BIT(4) /* OR of OFU, OFO, IFU, IFO */
> +#define DB9000_ISR_MBE			BIT(5) /* Master Bus Error */
> +#define DB9000_ISR_VCT			BIT(6) /* Vertical Compare Triggered */
> +#define DB9000_ISR_BAU			BIT(7) /* DMA Base Address Register Update to CAR */
> +#define DB9000_ISR_LDD			BIT(8) /* LCD Controller Disable Done */
> +
> +/* Interrupt Mask Register, Offset 0x01C */
> +#define DB9000_IMR_OFUM			BIT(0) /* Output FIFO Underrun - Mask */
> +#define DB9000_IMR_OFOM			BIT(1) /* Output FIFO Overrun - Mask */
> +#define DB9000_IMR_IFUM			BIT(2) /* Input FIFO Underrun - Mask */
> +#define DB9000_IMR_IFOM			BIT(3) /* Input FIFO Overrun - Mask */
> +#define DB9000_IMR_FERM			BIT(4) /* OR of OFU, OFO, IFU, IFO - Mask */
> +#define DB9000_IMR_MBEM			BIT(5) /* Master Bus Error - Mask */
> +#define DB9000_IMR_VCTM			BIT(6) /* Vertical Compare Triggered - Mask */
> +/* DMA Base Address Register Update to CAR - Mask */
> +#define DB9000_IMR_BAUM			BIT(7)
> +#define DB9000_IMR_LDDM			BIT(8) /* LCD Controller Disable Done - Mask */
> +
> +/* PWM Frequency Register */
> +#define DB9000_PWMFR_0			0x034
> +#define DB9000_PWMFR_RZN1		0x04C
> +/* PWM Duty Cycle Register */
> +#define DB9000_PWMDCR_0			0x038
> +#define DB9000_PWMDCR_RZN1		0x050
> +/* PWM Frequency Registers, Offset 0x034 and 0x04c */
> +#define DB9000_PWMFR_PWMFCD(x)		(((x) & 0x3fffff) << 0)
> +#define DB9000_PWMFR_PWMFCE		BIT(22)
> +
> +struct fps_info {
> +	unsigned int counter;
> +	ktime_t last_timestamp;
> +};
> +
> +struct db9000_device {
> +	struct drm_device drm;
> +	struct pwm_chip pwm;
> +	struct drm_connector *connector;
> +	void __iomem *regs;
> +	spinlock_t lock;
> +	struct clk *lcd_eclk;
> +	struct fps_info plane_fpsi[DB9000_MAX_LAYER];
> +	struct drm_atomic_state *suspend_state;
> +	u8 bpp;
> +	int bus_width;
> +	size_t frame_size;
> +	u32 addr_pwmfr;
> +	u32 addr_pwmdcr;
> +};
> +
> +#endif /* __DB9000_DU_H__ */
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller
  2020-04-28 18:18   ` Sam Ravnborg
@ 2020-04-28 18:23     ` Daniel Vetter
  2020-05-13 10:47       ` Gareth Williams
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2020-04-28 18:23 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Gareth Williams, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Phil Edworthy, Linux Kernel Mailing List,
	dri-devel

On Tue, Apr 28, 2020 at 8:18 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Gareth.
>
> On Mon, Apr 27, 2020 at 09:21:47AM +0100, Gareth Williams wrote:
> > Add DRM support for the Digital Blocks DB9000 LCD Controller
> > with the Renesas RZ/N1 specific changes.
> >
> > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> > ---
> >  drivers/gpu/drm/Kconfig                    |   2 +
> >  drivers/gpu/drm/Makefile                   |   1 +
> >  drivers/gpu/drm/digital-blocks/Kconfig     |  13 +
> >  drivers/gpu/drm/digital-blocks/Makefile    |   3 +
> >  drivers/gpu/drm/digital-blocks/db9000-du.c | 953 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/digital-blocks/db9000-du.h | 192 ++++++
> >  6 files changed, 1164 insertions(+)
> >  create mode 100644 drivers/gpu/drm/digital-blocks/Kconfig
> >  create mode 100644 drivers/gpu/drm/digital-blocks/Makefile
> >  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.c
> >  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.h
>
> The general impression is a well written driver.
> It looks like it was wrtten some tiem ago and thus fail to take full
> benefit from the improvements impemented the last year or so.
>
> The driver has a size so it is a candidate to belong in the tiny/
> directory.
> If you do not see any other DRM drivers for digital-blocks or that this
> driver should be extended, then please consider a move to tiny/
>
> If you do so embed the header file in the .c file so it is a single file
> driver.
>
> Other general comments:
> The driver looks like a one plane - one crtc - one encoder driver.
> Please use drm_simple - or explain why you cannot use drm_simple.
>
> For the encoder use drm_simple_encoder_init
>
> A small intro to the driver would be good.
> For example that is includes a pwm etc.

One more since I just landed my series: Please use devm_drm_dev_alloc.
There's a ton of examples now in-tree.
-Daniel

>
> I provided a mix of diverse comments in the following.
>
> Looks forward for the next revision!
>
>         Sam
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 3c88420..159832d 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -280,6 +280,8 @@ source "drivers/gpu/drm/mgag200/Kconfig"
> >
> >  source "drivers/gpu/drm/cirrus/Kconfig"
> >
> > +source "drivers/gpu/drm/digital-blocks/Kconfig"
> > +
> >  source "drivers/gpu/drm/armada/Kconfig"
> >
> >  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 9f0d2ee..f525807 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -72,6 +72,7 @@ obj-$(CONFIG_DRM_MGAG200) += mgag200/
> >  obj-$(CONFIG_DRM_V3D)  += v3d/
> >  obj-$(CONFIG_DRM_VC4)  += vc4/
> >  obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
> > +obj-$(CONFIG_DRM_DB9000) += digital-blocks/
> >  obj-$(CONFIG_DRM_SIS)   += sis/
> >  obj-$(CONFIG_DRM_SAVAGE)+= savage/
> >  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
> > diff --git a/drivers/gpu/drm/digital-blocks/Kconfig b/drivers/gpu/drm/digital-blocks/Kconfig
> > new file mode 100644
> > index 0000000..436a7c0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/digital-blocks/Kconfig
> > @@ -0,0 +1,13 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +config DRM_DB9000
> > +     bool "DRM Support for DB9000 LCD Controller"
> > +     depends on DRM && (ARCH_MULTIPLATFORM || COMPILE_TEST)
> > +     select DRM_KMS_HELPER
> > +     select DRM_GEM_CMA_HELPER
> > +     select DRM_KMS_CMA_HELPER
> > +     select DRM_PANEL_BRIDGE
> > +     select VIDEOMODE_HELPERS
> > +     select FB_PROVIDE_GET_FB_UNMAPPED_AREA if FB
> > +
> > +     help
> > +       Enable DRM support for the DB9000 LCD controller.
> > diff --git a/drivers/gpu/drm/digital-blocks/Makefile b/drivers/gpu/drm/digital-blocks/Makefile
> > new file mode 100644
> > index 0000000..9f78492
> > --- /dev/null
> > +++ b/drivers/gpu/drm/digital-blocks/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_DRM_DB9000) += db9000-du.o
> > diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.c b/drivers/gpu/drm/digital-blocks/db9000-du.c
> > new file mode 100644
> > index 0000000..d84d446
> > --- /dev/null
> > +++ b/drivers/gpu/drm/digital-blocks/db9000-du.c
> > @@ -0,0 +1,953 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
> 2020?
>
> > + *
> > + * Author: Gareth Williams <gareth.williams.jx@renesas.com>
> > + *
> > + * Based on ltdc.c
> > + * Copyright (C) STMicroelectronics SA 2017
> > + *
> > + * Authors: Philippe Cornu <philippe.cornu@st.com>
> > + *          Yannick Fertre <yannick.fertre@st.com>
> > + *          Fabien Dessenne <fabien.dessenne@st.com>
> > + *          Mickael Reulier <mickael.reulier@st.com>
> > + */
> > +#include <drm/drm_atomic.h>
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_bridge.h>
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_fb_cma_helper.h>
> > +#include <drm/drm_fb_helper.h>
> > +#include <drm/drm_fourcc.h>
> > +#include <drm/drm_gem_framebuffer_helper.h>
> > +#include <drm/drm_gem_cma_helper.h>
> > +#include <drm/drm_of.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_plane_helper.h>
> > +#include <drm/drm_probe_helper.h>
> > +#include <drm/drm_vblank.h>
> > +#include <drm/drm_drv.h>
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/pwm.h>
> > +#include <linux/reset.h>
> > +
> > +#include <video/display_timing.h>
> > +#include <video/videomode.h>
> > +
> > +#include "db9000-du.h"
> > +
> > +#define NR_CRTC              1
> > +#define DB9000_FB_MAX_WIDTH  4095
> > +#define DB9000_FB_MAX_HEIGHT 4095
> > +#define RZN1_REGS            ((void *) 1)
> > +
> > +static const u32 db9000_fmts[] = {
> > +     DRM_FORMAT_RGB888,
> > +     DRM_FORMAT_RGB565,
> > +     DRM_FORMAT_XRGB1555,
> > +     DRM_FORMAT_XRGB8888,
> > +     DRM_FORMAT_BGR888,
> > +     DRM_FORMAT_BGR565,
> > +     DRM_FORMAT_XBGR1555,
> > +     DRM_FORMAT_XBGR8888
> > +};
> > +
> > +static u32 reg_read(void __iomem *base, u32 reg)
> > +{
> > +     return readl(base + reg);
> > +}
> > +
> > +static void reg_write(void __iomem *base, u32 reg, u32 val)
> > +{
> > +     writel(val, base + reg);
> > +}
> Thiese helpers do not really add any value.
> Consider using db9000_device* as first argument, so you do not need to
> do a "->regs" for every use.
> Or drop them.
>
> db9000_device is so much more than a device.
> Why not just name it struct db9000, and then use db9000 as the preferred
> variable name?
> Just a personal preference, feel free to ignore.
>
>
> > +
> > +static struct db9000_device *crtc_to_db9000(struct drm_crtc *crtc)
> > +{
> > +     return container_of(crtc->dev, struct db9000_device, drm);
> > +}
> > +
> > +static struct db9000_device *plane_to_db9000(struct drm_plane *plane)
> > +{
> > +     return container_of(plane->dev, struct db9000_device, drm);
> > +}
> > +
> > +static struct db9000_device *pwm_chip_to_db9000(struct pwm_chip *chip)
> > +{
> > +     return container_of(chip, struct db9000_device, pwm);
> > +}
> > +
> > +void db9000_bpp_setup(struct db9000_device *db9000_dev, int bpp, int bus_width,
> > +                   bool pixelSelect)
>
> The linux kernel style is small caps and underscore like this:
> pixel_select
>
> > +{
> > +     u32 format;
> > +     u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> > +
> > +     /* reset the BPP bits */
> > +     reg_cr1 &= ~DB9000_CR1_BPP(7);
> > +     reg_cr1 &= ~DB9000_CR1_OPS(5);
> > +     reg_cr1 &= ~DB9000_CR1_OPS(1);
> > +     db9000_dev->bpp = bpp;
> > +
> > +     switch (bpp) {
> > +     case 16:
> > +             if (pixelSelect) {
> > +                     reg_cr1 |= DB9000_CR1_OPS(5);
> > +                     reg_cr1 |= DB9000_CR1_OPS(1);
> > +             }
> > +
> > +             format = DB9000_CR1_BPP(DB9000_CR1_BPP_16bpp);
> > +             break;
> > +     case 24:
> > +     case 32:
> > +     default:
> > +             format = DB9000_CR1_BPP(DB9000_CR1_BPP_24bpp);
> > +     }
> > +
> > +     if (bpp <= 16 && bus_width == 24)
> > +             reg_cr1 |= DB9000_CR1_OPS(2);
> > +     else
> > +             reg_cr1 &= ~DB9000_CR1_OPS(2);
> > +
> > +     if (bpp == 24)
> > +             reg_cr1 |= DB9000_CR1_FBP;
> > +     else
> > +             reg_cr1 &= ~DB9000_CR1_FBP;
> > +
> > +     reg_cr1 |= format;
> > +     reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
> > +}
> > +
> > +void db9000_toggle_controller(struct db9000_device *db9000_dev, bool on)
>
> USe two helpers here:
> db9000_controller_on()
> db9000_controller_off()
>
> More descriptive than the bool flag.
>
> > +{
> > +     u32 isr;
> > +     u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> > +     unsigned long flags;
> > +
> > +     if (on) {
> > +             /* Enable controller */
> > +             reg_cr1 |= DB9000_CR1_LCE;
> > +             reg_cr1 |= DB9000_CR1_LPE;
> > +
> > +             /* DMA Burst Size */
> > +             reg_cr1 |= DB9000_CR1_FDW(2);
> > +
> > +             /* Release pixel clock domain reset */
> > +             reg_write(db9000_dev->regs, DB9000_PCTR, DB9000_PCTR_PCR);
> > +
> > +             /* Enable BAU event for IRQ */
> > +             spin_lock_irqsave(&db9000_dev->lock, flags);
> > +             isr = reg_read(db9000_dev->regs, DB9000_ISR);
> > +             reg_write(db9000_dev->regs, DB9000_ISR, isr | DB9000_ISR_BAU);
> > +             reg_write(db9000_dev->regs, DB9000_IMR, DB9000_IMR_BAUM);
> > +             spin_unlock_irqrestore(&db9000_dev->lock, flags);
> > +
> > +     } else {
> > +             /* Disable controller */
> > +             reg_cr1 &= ~DB9000_CR1_LCE;
> > +             reg_cr1 &= ~DB9000_CR1_LPE;
> > +     }
> > +
> > +     reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
> > +}
> > +
> > +/* CRTC Functions */
> > +static void db9000_crtc_atomic_enable(struct drm_crtc *crtc,
> > +                                   struct drm_crtc_state *old_state)
> > +{
> > +     struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> > +     u32 imr;
> > +
> > +     /* Enable IRQ */
> > +     imr = reg_read(db9000_dev->regs, DB9000_IMR);
> > +     reg_write(db9000_dev->regs, DB9000_IMR, imr | DB9000_IMR_BAUM);
> > +}
> > +
> > +static void db9000_crtc_atomic_disable(struct drm_crtc *crtc,
> > +                                    struct drm_crtc_state *old_state)
> > +{
> > +     struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> > +     u32 imr;
> > +
> > +     /* disable IRQ */
> > +     imr = reg_read(db9000_dev->regs, DB9000_IMR);
> > +     reg_write(db9000_dev->regs, DB9000_IMR, imr & ~DB9000_IMR_BAUM);
> > +}
> > +
> > +static void db9000_crtc_mode_set_nofb(struct drm_crtc *crtc)
> > +{
> > +     struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> > +     struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> > +     struct videomode vm;
> > +     int bus_flags = db9000_dev->connector->display_info.bus_flags;
> > +     u32 vtr1, vtr2, hvter, htr, hpplor, dear_offset;
> > +     u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> > +
> > +     drm_display_mode_to_videomode(mode, &vm);
> There is no need for this conversion, the timing can be fetched from
> display_mode.
> And this is the only use of videomode, so you can drop the select in
> Kconfig too, and the include.
>
> > +
> > +     if (mode->flags & DISPLAY_FLAGS_HSYNC_HIGH)
> > +             reg_cr1 |= DB9000_CR1_HSP;
> > +     else
> > +             reg_cr1 &= ~DB9000_CR1_HSP;
> > +
> > +     if (mode->flags & DISPLAY_FLAGS_VSYNC_HIGH)
> > +             reg_cr1 |= DB9000_CR1_VSP;
> > +     else
> > +             reg_cr1 &= ~DB9000_CR1_VSP;
> > +
> > +     if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> > +             reg_cr1 |= DB9000_CR1_DEP;
> > +     else
> > +             reg_cr1 &= ~DB9000_CR1_DEP;
> > +
> > +     /* Horizontal Timing Register */
> > +     htr =   DB9000_HTR_HSW(vm.hsync_len) |
> > +             DB9000_HTR_HBP(vm.hback_porch) |
> > +             /* Pixels per line */
> > +             DB9000_HTR_HFP(vm.hfront_porch);
> > +
> > +     /* Horizontal Pixels-Per-Line Override */
> > +     hpplor = vm.hactive | DB9000_HPOE;
> > +
> > +     /* Vertical timing registers setup */
> > +     vtr1 =  DB9000_VTR1_VBP(vm.vback_porch) |
> > +             DB9000_VTR1_VFP(vm.vfront_porch) |
> > +             DB9000_VTR1_VSW(vm.vsync_len);
> > +
> > +     vtr2 = DB9000_VTR2_LPP(vm.vactive);
> > +
> > +     /* Vertical and Horizontal Timing Extension write */
> > +     hvter = DB9000_HVTER_HFPE(vm.hfront_porch) |
> > +             DB9000_HVTER_HBPE(vm.hback_porch) |
> > +             DB9000_HVTER_VFPE(vm.vback_porch) |
> > +             DB9000_HVTER_VBPE(vm.vfront_porch);
> > +
> > +     db9000_dev->frame_size = vm.hactive * vm.vactive;
> > +
> > +     /* DEAR register must be configured to the block end + 8 */
> > +     dear_offset =
> > +             (db9000_dev->frame_size * db9000_dev->bpp) / 8 + 8;
> > +
> > +     reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
> > +     reg_write(db9000_dev->regs, DB9000_HTR, htr);
> > +     reg_write(db9000_dev->regs, DB9000_VTR1, vtr1);
> > +     reg_write(db9000_dev->regs, DB9000_VTR2, vtr2);
> > +     reg_write(db9000_dev->regs, DB9000_HPPLOR, hpplor);
> > +     reg_write(db9000_dev->regs, DB9000_HVTER, hvter);
> > +
> > +     DRM_DEBUG_DRIVER("CRTC:%d mode:%s\n", crtc->base.id, mode->name);
> > +     DRM_DEBUG_DRIVER("Video mode: %dx%d", vm.hactive, vm.vactive);
> > +     DRM_DEBUG_DRIVER(" hfp %d hbp %d hsl %d vfp %d vbp %d vsl %d\n",
> > +                      vm.hfront_porch, vm.hback_porch, vm.hsync_len,
> > +                      vm.vfront_porch, vm.vback_porch, vm.vsync_len);
> As a general comment:
> use drm_dbg(db9000->drm, "foo") and not the deprecated DRM_DEBUG_DRIVER.
>
> > +}
> > +
> > +static void db9000_crtc_atomic_flush(struct drm_crtc *crtc,
> > +                                  struct drm_crtc_state *old_crtc_state)
> > +{
> > +     struct drm_pending_vblank_event *event = crtc->state->event;
> > +
> > +     if (event) {
> > +             crtc->state->event = NULL;
> > +
> > +             spin_lock_irq(&crtc->dev->event_lock);
> > +             drm_crtc_send_vblank_event(crtc, event);
> > +             spin_unlock_irq(&crtc->dev->event_lock);
> > +     }
> > +}
> > +
> > +static const struct drm_crtc_helper_funcs db9000_crtc_helper_funcs = {
> > +     .mode_set_nofb = db9000_crtc_mode_set_nofb,
> > +     .atomic_flush = db9000_crtc_atomic_flush,
> > +     .atomic_enable = db9000_crtc_atomic_enable,
> > +     .atomic_disable = db9000_crtc_atomic_disable,
> > +};
> > +
> > +static const struct drm_crtc_funcs db9000_crtc_funcs = {
> > +     .destroy = drm_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,
> > +     .gamma_set = drm_atomic_helper_legacy_gamma_set,
> > +};
> > +
> > +/*
> > + * DRM_PLANE
> > + */
> > +static void db9000_plane_atomic_update(struct drm_plane *plane,
> > +                                    struct drm_plane_state *oldstate)
> > +{
> > +     struct db9000_device *db9000_dev = plane_to_db9000(plane);
> > +     struct drm_plane_state *state = plane->state;
> > +     struct drm_framebuffer *fb = state->fb;
> > +     unsigned long flags;
> > +     u32 isr, paddr, dear_offset;
> > +     u32 format;
> > +
> > +     if (!state->crtc || !fb) {
> > +             DRM_DEBUG_DRIVER("fb or crtc NULL\n");
> > +             return;
> > +     }
> > +
> > +     format = fb->format->format;
> > +
> > +     /* The single plane is turning visible, so turn on the display */
> > +     if (!oldstate->visible && state->visible)
> > +             db9000_toggle_controller(db9000_dev, false);
> > +
> > +     /* The plane is no longer visible */
> > +     if (oldstate->visible && !state->visible)
> > +             db9000_toggle_controller(db9000_dev, true);
> > +
> > +     /* Check for format changes */
> > +     if (format == DRM_FORMAT_RGB565 || format == DRM_FORMAT_BGR565)
> > +             db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width, false);
> > +     else if (format == DRM_FORMAT_XRGB1555 || format == DRM_FORMAT_XBGR1555)
> > +             db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width, true);
> > +     else if (format == DRM_FORMAT_RGB888 || format == DRM_FORMAT_BGR888)
> > +             db9000_bpp_setup(db9000_dev, 24, db9000_dev->bus_width, false);
> > +     else if (format == DRM_FORMAT_XRGB8888 || format == DRM_FORMAT_XBGR8888)
> > +             db9000_bpp_setup(db9000_dev, 32, db9000_dev->bus_width, false);
> > +
> > +     dear_offset = (db9000_dev->frame_size * db9000_dev->bpp) / 8 + 8;
> > +
> > +     /* The frame buffer has changed, so get the new FB address */
> > +     if (oldstate->fb != state->fb || state->crtc->state->mode_changed) {
> > +             paddr = (u32)drm_fb_cma_get_gem_addr(fb, state, 0);
> > +
> > +             DRM_DEBUG_DRIVER("fb: phys 0x%08x\n", paddr);
> > +             reg_write(db9000_dev->regs, DB9000_DBAR, paddr);
> > +             reg_write(db9000_dev->regs, DB9000_DEAR,
> > +                       paddr + dear_offset);
> > +     }
> > +
> > +     /* Enable BAU event */
> > +     spin_lock_irqsave(&db9000_dev->lock, flags);
> > +     isr = reg_read(db9000_dev->regs, DB9000_ISR);
> > +     reg_write(db9000_dev->regs, DB9000_ISR, isr | DB9000_ISR_BAU);
> > +     reg_write(db9000_dev->regs, DB9000_IMR, DB9000_IMR_BAUM);
> > +     spin_unlock_irqrestore(&db9000_dev->lock, flags);
> > +
> > +     db9000_dev->plane_fpsi->counter++;
> > +
> > +     if (isr & DB9000_ISR_MBE) {
> > +             if (isr & DB9000_ISR_OFU)
> > +                     DRM_ERROR("Output FIFO Underrun\n");
> > +
> > +             if (isr & DB9000_ISR_OFO)
> > +                     DRM_ERROR("Output FIFO Overrun\n");
> > +
> > +             if (isr & DB9000_ISR_IFU)
> > +                     DRM_ERROR("Input FIFO Underrun\n");
> > +
> > +             if (isr & DB9000_ISR_IFO)
> > +                     DRM_ERROR("Input FIFO Overrun\n");
> > +     }
> > +}
> > +
> > +static void db9000_plane_atomic_print_state(struct drm_printer *p,
> > +                                         const struct drm_plane_state *state)
> > +{
> > +     struct drm_plane *plane = state->plane;
> > +     struct db9000_device *db9000_dev = plane_to_db9000(plane);
> > +     struct fps_info *fpsi = db9000_dev->plane_fpsi;
> > +     int ms_since_last;
> > +     ktime_t now;
> > +
> > +     now = ktime_get();
> > +     ms_since_last = ktime_to_ms(ktime_sub(now, fpsi->last_timestamp));
> > +
> > +     drm_printf(p, "\tuser_updates=%dfps\n",
> > +                DIV_ROUND_CLOSEST(fpsi->counter * 1000, ms_since_last));
> > +
> > +     fpsi->last_timestamp = now;
> > +     fpsi->counter = 0;
> > +}
> > +
> > +static const struct drm_plane_funcs db9000_plane_funcs = {
> > +     .update_plane = drm_atomic_helper_update_plane,
> > +     .disable_plane = drm_atomic_helper_disable_plane,
> > +     .destroy = drm_plane_cleanup,
> > +     .reset = drm_atomic_helper_plane_reset,
> > +     .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> > +     .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> > +     .atomic_print_state = db9000_plane_atomic_print_state,
> > +};
> > +
> > +static const struct drm_plane_helper_funcs db9000_plane_helper_funcs = {
> > +     .atomic_update = db9000_plane_atomic_update,
> > +};
> > +
> > +static struct drm_plane *db9000_plane_create(struct drm_device *ddev,
> > +                                          enum drm_plane_type type)
> > +{
> > +     struct device *dev = ddev->dev;
> > +     struct drm_plane *plane;
> > +     int ret;
> > +
> > +     plane = devm_kzalloc(dev, sizeof(*plane), GFP_KERNEL);
> > +     if (!plane)
> > +             return NULL;
> > +
> > +     ret = drm_universal_plane_init(ddev, plane, NR_CRTC,
> > +                                    &db9000_plane_funcs,
> > +                                    db9000_fmts,
> > +                                    ARRAY_SIZE(db9000_fmts), NULL,
> > +                                    type, "rzn1_primary_rgb");
> > +     if (ret < 0)
> > +             return NULL;
> > +
> > +     drm_plane_helper_add(plane, &db9000_plane_helper_funcs);
> > +
> > +     DRM_DEBUG_DRIVER("plane:%d created\n", plane->base.id);
> > +
> > +     return plane;
> > +}
> > +
> > +static void db9000_plane_destroy_all(struct drm_device *ddev)
> > +{
> > +     struct drm_plane *plane, *plane_temp;
> > +
> > +     list_for_each_entry_safe(plane, plane_temp,
> > +                              &ddev->mode_config.plane_list, head)
> > +                              drm_plane_cleanup(plane);
> > +}
> > +
> > +static int db9000_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
> > +{
> > +     struct drm_plane *primary;
> > +     int ret;
> > +
> > +     primary = db9000_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY);
> > +     if (!primary) {
> > +             DRM_ERROR("Can not create primary plane\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = drm_crtc_init_with_planes(ddev, crtc, primary, NULL,
> > +                                     &db9000_crtc_funcs, NULL);
> > +     if (ret) {
> > +             DRM_ERROR("Can not initialize CRTC\n");
> > +             goto cleanup;
> > +     }
> > +
> > +     drm_crtc_helper_add(crtc, &db9000_crtc_helper_funcs);
> > +     DRM_DEBUG_DRIVER("CRTC:%d created\n", crtc->base.id);
> > +     return 0;
> > +
> > +cleanup:
> > +     db9000_plane_destroy_all(ddev);
> > +     return ret;
> > +}
> > +
> > +/*
> > + * DRM_ENCODER
> > + */
> > +static const struct drm_encoder_funcs db9000_encoder_funcs = {
> > +     .destroy = drm_encoder_cleanup,
> > +};
> > +
> > +static int db9000_encoder_init(struct drm_device *ddev,
> > +                            struct drm_bridge *bridge)
> > +{
> > +     struct drm_encoder *encoder;
> > +     int ret;
> > +
> > +     encoder = devm_kzalloc(ddev->dev, sizeof(*encoder), GFP_KERNEL);
> > +     if (!encoder)
> > +             return -ENOMEM;
> > +
> > +     encoder->possible_crtcs = NR_CRTC;
> > +     encoder->possible_clones = 0; /* No cloning support */
> > +
> > +     drm_encoder_init(ddev, encoder, &db9000_encoder_funcs,
> > +                      DRM_MODE_ENCODER_NONE, NULL);
> > +
> > +     ret = drm_bridge_attach(encoder, bridge, NULL);
> > +     if (ret) {
> > +             drm_encoder_cleanup(encoder);
> > +             return -EINVAL;
> > +     }
> > +
> > +     DRM_DEBUG_DRIVER("Bridge encoder:%d created\n", encoder->base.id);
> > +
> > +     return 0;
> > +}
> > +
> > +void __maybe_unused db9000_suspend(struct drm_device *ddev)
> > +{
> > +     struct db9000_device *db9000_dev = container_of(ddev,
> > +                                                 struct db9000_device, drm);
> > +
> > +     clk_disable_unprepare(db9000_dev->lcd_eclk);
> > +}
> > +
> > +int __maybe_unused db9000_resume(struct drm_device *ddev)
> > +{
> > +     struct db9000_device *db9000_dev = container_of(ddev,
> > +                                                 struct db9000_device, drm);
> > +     int ret;
> > +
> > +     ret = clk_prepare_enable(db9000_dev->lcd_eclk);
> > +     if (ret) {
> > +             DRM_ERROR("failed to enable pixel clock (%d)\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int db9000_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> > +{
> > +     return pm_runtime_get_sync(chip->dev);
> > +}
> > +
> > +static void db9000_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> > +{
> > +     pm_runtime_put(chip->dev);
> > +}
> > +
> > +static int db9000_pwm_enable(struct db9000_device *db9000_dev)
> > +{
> > +     u32 reg_pwmfr = reg_read(db9000_dev->regs, db9000_dev->addr_pwmfr);
> > +
> > +     reg_pwmfr |= DB9000_PWMFR_PWMFCE;
> > +     reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, reg_pwmfr);
> > +
> > +     return 0;
> > +}
> > +
> > +static void db9000_pwm_disable(struct db9000_device *db9000_dev)
> > +{
> > +     u32 reg_pwmfr = reg_read(db9000_dev->regs, db9000_dev->addr_pwmfr);
> > +
> > +     reg_pwmfr &= ~DB9000_PWMFR_PWMFCE;
> > +     reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, reg_pwmfr);
> > +}
> > +
> > +static int db9000_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > +                         struct pwm_state *state)
> > +{
> > +     struct pwm_state cur_state;
> > +     struct db9000_device *db9000_dev = pwm_chip_to_db9000(chip);
> > +     int ret;
> > +
> > +     pwm_get_state(pwm, &cur_state);
> > +
> > +     if (state->enabled)
> > +             ret = db9000_pwm_enable(db9000_dev);
> > +     else
> > +             db9000_pwm_disable(db9000_dev);
> > +
> > +     if (state->period != cur_state.period) {
> > +             u32 pwmfcd;
> > +
> > +             pwmfcd = clk_get_rate(db9000_dev->lcd_eclk) / 256;
> > +             pwmfcd *= state->period;
> > +             pwmfcd = DB9000_PWMFR_PWMFCD(pwmfcd - 1);
> > +             reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr, pwmfcd);
> > +     }
> > +
> > +     if (state->duty_cycle == 0) {
> > +             db9000_pwm_disable(db9000_dev);
> > +     } else if (state->period != cur_state.period ||
> > +         state->duty_cycle != cur_state.duty_cycle) {
> > +             u32 dcr = div_u64((state->duty_cycle * ULL(256)),
> > +                                state->period) - 1;
> > +
> > +             reg_write(db9000_dev->regs, db9000_dev->addr_pwmdcr, dcr);
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct pwm_ops db9000_pwm_ops = {
> > +     .request = db9000_pwm_request,
> > +     .free = db9000_pwm_free,
> > +     .apply = db9000_pwm_apply,
> > +     .owner = THIS_MODULE,
> > +};
> > +
> > +static int db9000_pwm_setup(struct device *dev,
> > +                         struct db9000_device *db9000_dev)
> > +{
> > +     struct pwm_chip *db9000_pwm;
> > +     int ret;
> > +
> > +     db9000_pwm = devm_kzalloc(dev, sizeof(*db9000_pwm), GFP_KERNEL);
> > +     if (db9000_pwm == NULL)
> > +             return -ENOMEM;
> > +
> > +     db9000_pwm = &db9000_dev->pwm;
> > +
> > +     db9000_pwm->dev = dev;
> > +     db9000_pwm->ops = &db9000_pwm_ops;
> > +     db9000_pwm->base = -1;
> > +     db9000_pwm->npwm = 1;
> > +
> > +     ret = pwmchip_add(db9000_pwm);
> > +     if (ret < 0) {
> > +             dev_err(dev, "failed to register PWM chip: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     pm_runtime_enable(dev);
> > +
> > +     return 0;
> > +}
> > +
> > +int db9000_load(struct drm_device *ddev, int rzn1_pwm)
> > +{
> > +     struct platform_device *pdev = to_platform_device(ddev->dev);
> > +     struct device *dev = ddev->dev;
> > +     struct db9000_device *db9000_dev = container_of(ddev,
> > +                                                 struct db9000_device, drm);
> > +     struct device_node *np = dev->of_node;
> > +     struct drm_bridge *bridge;
> > +     struct drm_panel *panel;
> > +     struct drm_crtc *crtc;
> > +     struct reset_control *rstc;
> > +     struct resource *res;
> > +     int ret;
> > +
> > +     spin_lock_init(&db9000_dev->lock);
> > +
> > +     if (rzn1_pwm) {
> > +             db9000_dev->addr_pwmfr = DB9000_PWMFR_RZN1;
> > +             db9000_dev->addr_pwmdcr = DB9000_PWMDCR_RZN1;
> > +     } else {
> > +             db9000_dev->addr_pwmfr = DB9000_PWMFR_0;
> > +             db9000_dev->addr_pwmdcr = DB9000_PWMDCR_0;
> > +     }
> > +
> > +     /* Panel Setup */
> > +     ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge);
> > +     if (ret != 0) {
> > +             DRM_ERROR("Could not get Panel or bridge");
> > +             return ret;
> > +     }
> > +
> > +     rstc = devm_reset_control_get_exclusive(dev, NULL);
> > +
> > +     /* Clock setup */
> > +     db9000_dev->lcd_eclk = devm_clk_get(dev, "lcd_eclk");
> > +
> > +     if (IS_ERR(db9000_dev->lcd_eclk)) {
> > +             DRM_ERROR("Unable to get pixel clock\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     if (clk_prepare_enable(db9000_dev->lcd_eclk)) {
> > +             DRM_ERROR("Unable to prepare pixel clock\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* Memory setup */
> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +     if (!res) {
> > +             DRM_ERROR("Could not retrieve platform resources\n");
> > +             ret = -EINVAL;
> > +             goto err;
> > +     }
> > +
> > +     db9000_dev->regs = devm_ioremap_resource(dev, res);
> > +     if (IS_ERR(db9000_dev->regs)) {
> > +             DRM_ERROR("Unable to get memory resource\n");
> > +             ret = PTR_ERR(db9000_dev->regs);
> > +             goto err;
> > +     }
> > +
> > +     db9000_bpp_setup(db9000_dev, db9000_dev->bpp, db9000_dev->bus_width,
> > +                      false);
> > +
> > +     db9000_toggle_controller(db9000_dev, true);
> > +
> > +     /* Add endpoints panels or bridges if any */
> > +     if (panel) {
> > +             bridge = drm_panel_bridge_add(panel,
> > +                                           DRM_MODE_CONNECTOR_Unknown);
> > +             if (IS_ERR(bridge)) {
> > +                     DRM_ERROR("panel-bridge endpoint\n");
> > +                     ret = PTR_ERR(bridge);
> > +                     goto err;
> > +             }
> > +     }
> > +
> > +     if (bridge) {
> > +             ret = db9000_encoder_init(ddev, bridge);
> > +             if (ret) {
> > +                     DRM_ERROR("init encoder endpoint\n");
> > +                     goto err;
> > +             }
> > +     }
> > +
> > +     db9000_dev->connector = panel->connector;
> > +
> > +     /* CRTC setup  */
> > +     crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
> > +     if (!crtc) {
> > +             DRM_ERROR("Failed to allocate crtc\n");
> > +             ret = -ENOMEM;
> > +             goto err;
> > +     }
> > +
> > +     ret = db9000_crtc_init(&db9000_dev->drm, crtc);
> > +     if (ret) {
> > +             DRM_ERROR("Failed to init crtc\n");
> > +             goto err;
> > +     }
> > +
> > +     if (!IS_ERR(rstc)) {
> > +             reset_control_assert(rstc);
> > +             usleep_range(10, 20);
> > +             reset_control_deassert(rstc);
> > +     }
> > +
> > +     return db9000_pwm_setup(dev, db9000_dev);
> > +
> > +err:
> > +     drm_panel_bridge_remove(bridge);
> > +
> > +     clk_disable_unprepare(db9000_dev->lcd_eclk);
> > +
> > +     return ret;
> > +}
> > +
> > +void db9000_unload(struct drm_device *ddev)
> > +{
> > +     struct db9000_device *db9000_dev;
> > +
> > +     db9000_dev = container_of(ddev, struct db9000_device, drm);
> > +
> > +     drm_of_panel_bridge_remove(ddev->dev->of_node, 0, 0);
> > +     clk_disable_unprepare(db9000_dev->lcd_eclk);
> > +}
> > +
> > +static const struct drm_mode_config_funcs drv_mode_config_funcs = {
> > +     .fb_create = drm_gem_fb_create,
> > +     .atomic_check = drm_atomic_helper_check,
> > +     .atomic_commit = drm_atomic_helper_commit,
> > +};
> > +
> > +static int db9000_gem_cma_dumb_create(struct drm_file *file,
> > +                                   struct drm_device *dev,
> > +                                   struct drm_mode_create_dumb *args)
> > +{
> > +     return drm_gem_cma_dumb_create_internal(file, dev, args);
> > +}
> This wrapper seems like it can be dropped.
>
> > +
> > +DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
> > +
> > +static struct drm_driver drv_driver = {
> > +     .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
> > +                        DRIVER_ATOMIC,
> > +     .name = "drm-db9000",
> > +     .desc = "Digital Blocks DB9000 DRM Driver",
> > +     .date = "20190825",
> > +     .major = 1,
> > +     .minor = 0,
> > +     .patchlevel = 0,
> > +     .fops = &drv_driver_fops,
> > +     .dumb_create = db9000_gem_cma_dumb_create,
> > +     .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > +     .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > +     .gem_free_object_unlocked = drm_gem_cma_free_object,
> > +     .gem_vm_ops = &drm_gem_cma_vm_ops,
> > +     .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,
> > +};
> I think if you look up these operations, then you have assinged the
> default operations in mayny cases.
> Please see if you can trim the list.
>
> Also double check that no legacy operatiosn are used.
>
> > +
> > +static int drv_load(struct drm_device *ddev, u32 bpp,
> > +                 u32 bus_width, int rzn1_pwm)
> > +{
> > +     struct platform_device *pdev = to_platform_device(ddev->dev);
> > +     struct db9000_device *db9000_dev = container_of(ddev,
> > +                                                 struct db9000_device, drm);
> > +     int ret;
> > +
> > +     drm_mode_config_init(ddev);
> > +
> > +     /*
> > +      * set max width and height as default value.
> > +      * this value would be used to check framebuffer size limitation
> > +      * at drm_mode_addfb().
> > +      */
> > +     ddev->mode_config.min_width = 0;
> > +     ddev->mode_config.min_height = 0;
> > +     ddev->mode_config.max_width = DB9000_FB_MAX_WIDTH;
> > +     ddev->mode_config.max_height = DB9000_FB_MAX_WIDTH;
> > +     ddev->mode_config.funcs = &drv_mode_config_funcs;
> > +
> > +     db9000_dev->bus_width = bus_width;
> > +
> > +     ret = db9000_load(ddev, rzn1_pwm);
> > +
> > +     if (ret)
> > +             goto err;
> > +
> > +     drm_mode_config_reset(ddev);
> > +
> > +     drm_kms_helper_poll_init(ddev);
> > +
> > +     platform_set_drvdata(pdev, ddev);
> > +
> > +     return 0;
> > +
> > +err:
> > +     drm_mode_config_cleanup(ddev);
> > +
> > +     return ret;
> > +}
> > +
> > +static void drv_unload(struct drm_device *ddev)
> > +{
> > +     drm_kms_helper_poll_fini(ddev);
> > +     db9000_unload(ddev);
> > +     drm_mode_config_cleanup(ddev);
> > +}
> > +
> > +static __maybe_unused int db9000_drv_suspend(struct device *dev)
> > +{
> > +     struct drm_device *ddev = dev_get_drvdata(dev);
> > +     struct db9000_device *db9000_dev = container_of(ddev,
> > +                                                     struct db9000_device,
> > +                                                     drm);
> > +     struct drm_atomic_state *state;
> > +
> > +     db9000_toggle_controller(db9000_dev, false);
> > +
> > +     drm_kms_helper_poll_disable(ddev);
> > +     state = drm_atomic_helper_suspend(ddev);
> > +     if (IS_ERR(state)) {
> > +             drm_kms_helper_poll_enable(ddev);
> > +             return PTR_ERR(state);
> > +     }
> > +     db9000_dev->suspend_state = state;
> > +     db9000_suspend(ddev);
> > +
> > +     return 0;
> > +}
> > +
> > +static __maybe_unused int db9000_drv_resume(struct device *dev)
> > +{
> > +     struct drm_device *ddev = dev_get_drvdata(dev);
> > +     struct db9000_device *db9000_dev = container_of(ddev,
> > +                                                     struct db9000_device,
> > +                                                     drm);
> > +
> > +     db9000_resume(ddev);
> > +     drm_atomic_helper_resume(ddev, db9000_dev->suspend_state);
> > +     drm_kms_helper_poll_enable(ddev);
> > +     db9000_toggle_controller(db9000_dev, true);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct dev_pm_ops drv_pm_ops = {
> > +     SET_SYSTEM_SLEEP_PM_OPS(db9000_drv_suspend, db9000_drv_resume)
> > +};
> > +
> > +static int db9000_drm_platform_probe(struct platform_device *pdev)
> > +{
> > +     struct device *dev = &pdev->dev;
> > +     struct drm_device *ddev;
> > +     struct db9000_device *db9000_dev;
> > +     struct device_node *np = dev->of_node;
> > +     u32 bpp;
> > +     u32 bus_width;
> > +     int rzn1_pwm = 0;
> > +     int ret;
> > +
> > +     dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> > +
> > +     db9000_dev = kzalloc(sizeof(*db9000_dev), GFP_KERNEL);
> > +     if (!db9000_dev)
> > +             return -ENOMEM;
> > +
> > +     drm_dev_init(&db9000_dev->drm, &drv_driver, dev);
> > +     ddev = &db9000_dev->drm;
> Use the newly merged drmm stuff here.
>
> > +
> > +     /* Parse the DTB */
> > +     ret = of_property_read_u32(np, "bits-per-pixel", &bpp);
> > +     if (ret)
> > +             bpp = 16;
> > +
> > +     if (bpp != 16 && bpp != 24 && bpp != 32) {
> > +             DRM_WARN("bits-per-pixel value invalid, default is 16 bpp");
> > +             bpp = 16;
> > +     }
> > +
> > +     ret = of_property_read_u32(np, "bus-width", &bus_width);
> > +     if (ret)
> > +             bus_width = 24;
> > +
> > +     rzn1_pwm = (int) of_device_get_match_data(dev);
> > +
> > +     ret = drv_load(ddev, bpp, bus_width, rzn1_pwm);
> > +     if (ret)
> > +             goto err_put;
> > +
> > +     ret = drm_dev_register(ddev, 0);
> > +     if (ret)
> > +             goto err_put;
> > +
> > +     ret = drm_fbdev_generic_setup(ddev, bpp);
> > +     if (ret)
> > +             goto err_put;
> > +
> > +     dev_info(dev, "DB9000 LCD Controller Ready");
> > +
> > +     return 0;
> > +
> > +err_put:
> > +     drm_dev_put(ddev);
> With drmm_ support this put goes aways.
> > +
> > +     return ret;
> > +}
> > +
> > +static int db9000_drm_platform_remove(struct platform_device *pdev)
> > +{
> > +     struct drm_device *ddev = platform_get_drvdata(pdev);
> > +
> > +     drv_unload(ddev);
> > +     drm_dev_put(ddev);
> With drmm_ support this put goes aways.
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id drv_dt_ids[] = {
> > +     { .compatible = "digital-blocks,drm-db9000"},
> > +     { .compatible = "digital-blocks,drm-rzn1", .data = RZN1_REGS },
> > +     { /* end node */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, drv_dt_ids);
> > +
> > +static struct platform_driver db9000_drm_platform_driver = {
> > +     .probe = db9000_drm_platform_probe,
> > +     .remove = db9000_drm_platform_remove,
> > +     .driver = {
> > +             .name = "drm-db9000",
> > +             .of_match_table = drv_dt_ids,
> > +             .pm = &drv_pm_ops,
> > +     },
> > +};
> > +
> > +module_platform_driver(db9000_drm_platform_driver);
> > +
> > +MODULE_AUTHOR("Gareth Williams <gareth.williams.jx@renesas.com>");
> > +MODULE_DESCRIPTION("Digital Blocks DB9000 LCD Controller Driver");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.h b/drivers/gpu/drm/digital-blocks/db9000-du.h
> > new file mode 100644
> > index 0000000..325c9f0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/digital-blocks/db9000-du.h
> > @@ -0,0 +1,192 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
> > + *
> > + * Author: Gareth Williams <gareth.williams.jx@renesas.com>
> > + *
> > + * Based on ltdc.h
> > + * Copyright (C) STMicroelectronics SA 2017
> > + *
> > + * Authors: Philippe Cornu <philippe.cornu@st.com>
> > + *          Yannick Fertre <yannick.fertre@st.com>
> > + *          Fabien Dessenne <fabien.dessenne@st.com>
> > + *          Mickael Reulier <mickael.reulier@st.com>
> > + */
> > +#ifndef __DB9000_DU_H__
> > +#define __DB9000_DU_H__
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/pwm.h>
> > +
> > +#define DB9000_MAX_LAYER             1
> > +
> > +/* LCD Controller Control Register 1 */
> > +#define DB9000_CR1                   0x000
> > +/* Horizontal Timing Register */
> > +#define DB9000_HTR                   0x008
> > +/* Vertical Timing Register 1 */
> > +#define DB9000_VTR1                  0x00C
> > +/* Vertical Timing Register 2 */
> > +#define DB9000_VTR2                  0x010
> > +/* Pixel Clock Timing Register */
> > +#define DB9000_PCTR                  0x014
> > +/* Interrupt Status Register */
> > +#define DB9000_ISR                   0x018
> > +/* Interrupt Mask Register */
> > +#define DB9000_IMR                   0x01C
> > +/* Interrupt Vector Register */
> > +#define DB9000_IVR                   0x020
> > +/* Interrupt Scan Compare Register */
> > +#define DB9000_ISCR                  0x024
> > +/* DMA Base Address Register */
> > +#define DB9000_DBAR                  0x028
> > +/* DMA Current Address Register */
> > +#define DB9000_DCAR                  0x02C
> > +/* DMA End Address Register */
> > +#define DB9000_DEAR                  0x030
> > +/* DMA Horizontal and Vertical Timing Extension Register */
> > +#define DB9000_HVTER                 0x044
> > +/* Horizontal Pixels-Per-Line Override Control */
> > +#define DB9000_HPPLOR                        0x048
> > +/* Horizontal Pixels-Per-Line Override Enable */
> > +#define DB9000_HPOE                  BIT(31)
> > +/* GPIO Register */
> > +#define DB9000_GPIOR                 0x1F8
> > +/* Core Identification Register */
> > +#define DB9000_CIR                   0x1FC
> > +/* Palette Data Words */
> > +#define DB9000_PALT                  0x200
> > +
> > +/* Control Register 1, Offset 0x000 */
> > +/* LCD Controller Enable */
> > +#define DB9000_CR1_LCE                       BIT(0)
> > +/* LCD Power Enable */
> > +#define DB9000_CR1_LPE                       BIT(1)
> > +/* LCD Bits per Pixel */
> > +#define DB9000_CR1_BPP(x)            (((x) & 0x7) << 2)
> > +/* RGB or BGR Format */
> > +#define DB9000_CR1_RGB                       BIT(5)
> > +/* Data Enable Polarity */
> > +#define DB9000_CR1_DEP                       BIT(8)
> > +/* Pixel Clock Polarity */
> > +#define DB9000_CR1_PCP                       BIT(9)
> > +/* Horizontal Sync Polarity */
> > +#define DB9000_CR1_HSP                       BIT(10)
> > +/* Vertical Sync Polarity */
> > +#define DB9000_CR1_VSP                       BIT(11)
> > +/* Output Pixel Select */
> > +#define DB9000_CR1_OPS(x)            (((x) & 0x7) << 12)
> > +/* FIFO DMA Request Words */
> > +#define DB9000_CR1_FDW(x)            (((x) & 0x3) << 16)
> > +/* LCD 1 or Port Select */
> > +#define DB9000_CR1_LPS                       BIT(18)
> > +/* Frame Buffer 24bpp Packed Word */
> > +#define DB9000_CR1_FBP                       BIT(19)
> > +
> > +enum DB9000_CR1_BPP {
> > +     /* 1 bit per pixel */
> > +     DB9000_CR1_BPP_1bpp,
> > +     /* 2 bits per pixel */
> > +     DB9000_CR1_BPP_2bpp,
> > +     /* 4 bits per pixel */
> > +     DB9000_CR1_BPP_4bpp,
> > +     /* 8 bits per pixel */
> > +     DB9000_CR1_BPP_8bpp,
> > +     /* 16 bits per pixel */
> > +     DB9000_CR1_BPP_16bpp,
> > +     /* 18 bits per pixel */
> > +     DB9000_CR1_BPP_18bpp,
> > +     /* 24 bits per pixel */
> > +     DB9000_CR1_BPP_24bpp
> > +} DB9000_CR1_BPP_VAL;
> > +
> > +/* Horizontal Timing Register, Offset 0x008 */
> > +/* Horizontal Front Porch */
> > +#define DB9000_HTR_HFP(x)            (((x) & 0xff) << 0)
> > +/* Pixels per Line */
> > +#define DB9000_HTR_PPL(x)            (((x) & 0xff) << 8)
> > +/* Horizontal Back Porch */
> > +#define DB9000_HTR_HBP(x)            (((x) & 0xff) << 16)
> > +/* Horizontal Sync Width */
> > +#define DB9000_HTR_HSW(x)            (((x) & 0xff) << 24)
> > +
> > +/* Vertical Timing Register 1, Offset 0x00C */
> > +/* Vertical Sync Width */
> > +#define DB9000_VTR1_VSW(x)           (((x) & 0xff) << 0)
> > +/* Vertical Front Porch */
> > +#define DB9000_VTR1_VFP(x)           (((x) & 0xff) << 8)
> > +/* Vertical Back Porch */
> > +#define DB9000_VTR1_VBP(x)           (((x) & 0xff) << 16)
> > +
> > +/* Vertical Timing Register 2, Offset 0x010 */
> > +/* Lines Per Panel */
> > +#define DB9000_VTR2_LPP(x)           (((x) & 0xfff) << 0)
> > +
> > +/* Vertical and Horizontal Timing Extension Register, Offset 0x044 */
> > +/* Horizontal Front Porch Extension */
> > +#define DB9000_HVTER_HFPE(x)         ((((x) >> 8) & 0x3) << 0)
> > +/* Horizontal Back Porch Extension */
> > +#define DB9000_HVTER_HBPE(x)         ((((x) >> 8) & 0x3) << 4)
> > +/* Vertical Front Porch Extension */
> > +#define DB9000_HVTER_VFPE(x)         ((((x) >> 8) & 0x3) << 8)
> > +/* Vertical Back Porch Extension */
> > +#define DB9000_HVTER_VBPE(x)         ((((x) >> 8) & 0x3) << 12)
> > +
> > +/* clock reset select */
> > +#define DB9000_PCTR_PCR                      BIT(10)
> > +
> > +/* Interrupt Status Register, Offset 0x018 */
> > +#define DB9000_ISR_OFU                       BIT(0) /* Output FIFO Underrun */
> > +#define DB9000_ISR_OFO                       BIT(1) /* Output FIFO Overrun */
> > +#define DB9000_ISR_IFU                       BIT(2) /* Input FIFO Underrun */
> > +#define DB9000_ISR_IFO                       BIT(3) /* Input FIFO Overrun */
> > +#define DB9000_ISR_FER                       BIT(4) /* OR of OFU, OFO, IFU, IFO */
> > +#define DB9000_ISR_MBE                       BIT(5) /* Master Bus Error */
> > +#define DB9000_ISR_VCT                       BIT(6) /* Vertical Compare Triggered */
> > +#define DB9000_ISR_BAU                       BIT(7) /* DMA Base Address Register Update to CAR */
> > +#define DB9000_ISR_LDD                       BIT(8) /* LCD Controller Disable Done */
> > +
> > +/* Interrupt Mask Register, Offset 0x01C */
> > +#define DB9000_IMR_OFUM                      BIT(0) /* Output FIFO Underrun - Mask */
> > +#define DB9000_IMR_OFOM                      BIT(1) /* Output FIFO Overrun - Mask */
> > +#define DB9000_IMR_IFUM                      BIT(2) /* Input FIFO Underrun - Mask */
> > +#define DB9000_IMR_IFOM                      BIT(3) /* Input FIFO Overrun - Mask */
> > +#define DB9000_IMR_FERM                      BIT(4) /* OR of OFU, OFO, IFU, IFO - Mask */
> > +#define DB9000_IMR_MBEM                      BIT(5) /* Master Bus Error - Mask */
> > +#define DB9000_IMR_VCTM                      BIT(6) /* Vertical Compare Triggered - Mask */
> > +/* DMA Base Address Register Update to CAR - Mask */
> > +#define DB9000_IMR_BAUM                      BIT(7)
> > +#define DB9000_IMR_LDDM                      BIT(8) /* LCD Controller Disable Done - Mask */
> > +
> > +/* PWM Frequency Register */
> > +#define DB9000_PWMFR_0                       0x034
> > +#define DB9000_PWMFR_RZN1            0x04C
> > +/* PWM Duty Cycle Register */
> > +#define DB9000_PWMDCR_0                      0x038
> > +#define DB9000_PWMDCR_RZN1           0x050
> > +/* PWM Frequency Registers, Offset 0x034 and 0x04c */
> > +#define DB9000_PWMFR_PWMFCD(x)               (((x) & 0x3fffff) << 0)
> > +#define DB9000_PWMFR_PWMFCE          BIT(22)
> > +
> > +struct fps_info {
> > +     unsigned int counter;
> > +     ktime_t last_timestamp;
> > +};
> > +
> > +struct db9000_device {
> > +     struct drm_device drm;
> > +     struct pwm_chip pwm;
> > +     struct drm_connector *connector;
> > +     void __iomem *regs;
> > +     spinlock_t lock;
> > +     struct clk *lcd_eclk;
> > +     struct fps_info plane_fpsi[DB9000_MAX_LAYER];
> > +     struct drm_atomic_state *suspend_state;
> > +     u8 bpp;
> > +     int bus_width;
> > +     size_t frame_size;
> > +     u32 addr_pwmfr;
> > +     u32 addr_pwmdcr;
> > +};
> > +
> > +#endif /* __DB9000_DU_H__ */
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller
  2020-04-28 18:23     ` Daniel Vetter
@ 2020-05-13 10:47       ` Gareth Williams
  0 siblings, 0 replies; 20+ messages in thread
From: Gareth Williams @ 2020-05-13 10:47 UTC (permalink / raw)
  To: Daniel Vetter, Sam Ravnborg
  Cc: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Phil Edworthy, Linux Kernel Mailing List, dri-devel

Hi Sam/Daniel,

This is all very useful feedback, thank you. 

On Tue, Apr 28, 2020 at 19:24 PM Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> On Tue, Apr 28, 2020 at 8:18 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Gareth.
> >
> > On Mon, Apr 27, 2020 at 09:21:47AM +0100, Gareth Williams wrote:
> > > Add DRM support for the Digital Blocks DB9000 LCD Controller with
> > > the Renesas RZ/N1 specific changes.
> > >
> > > Signed-off-by: Gareth Williams <gareth.williams.jx@renesas.com>
> > > ---
> > >  drivers/gpu/drm/Kconfig                    |   2 +
> > >  drivers/gpu/drm/Makefile                   |   1 +
> > >  drivers/gpu/drm/digital-blocks/Kconfig     |  13 +
> > >  drivers/gpu/drm/digital-blocks/Makefile    |   3 +
> > >  drivers/gpu/drm/digital-blocks/db9000-du.c | 953
> > > +++++++++++++++++++++++++++++
> > > drivers/gpu/drm/digital-blocks/db9000-du.h | 192 ++++++
> > >  6 files changed, 1164 insertions(+)  create mode 100644
> > > drivers/gpu/drm/digital-blocks/Kconfig
> > >  create mode 100644 drivers/gpu/drm/digital-blocks/Makefile
> > >  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.c
> > >  create mode 100644 drivers/gpu/drm/digital-blocks/db9000-du.h
> >
> > The general impression is a well written driver.
> > It looks like it was wrtten some tiem ago and thus fail to take full
> > benefit from the improvements impemented the last year or so.
> >
> > The driver has a size so it is a candidate to belong in the tiny/
> > directory.
> > If you do not see any other DRM drivers for digital-blocks or that
> > this driver should be extended, then please consider a move to tiny/
> >
> > If you do so embed the header file in the .c file so it is a single
> > file driver.
That's okay with me, I'll merge the two files and move it to tinydrm.

> >
> > Other general comments:
> > The driver looks like a one plane - one crtc - one encoder driver.
> > Please use drm_simple - or explain why you cannot use drm_simple.
I'll take a look at this in more detail. I've tested with drm_simple and
it is feasible. Thanks for the direction.

> >
> > For the encoder use drm_simple_encoder_init
> >
> > A small intro to the driver would be good.
> > For example that is includes a pwm etc.
I'll add an introduction to the commit message and cover letter.

> 
> One more since I just landed my series: Please use devm_drm_dev_alloc.
> There's a ton of examples now in-tree.
> -Daniel
Thanks Daniel, I might have missed that otherwise.

> 
> >
> > I provided a mix of diverse comments in the following.
> >
> > Looks forward for the next revision!
> >
> >         Sam
> > >
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index
> > > 3c88420..159832d 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -280,6 +280,8 @@ source "drivers/gpu/drm/mgag200/Kconfig"
> > >
> > >  source "drivers/gpu/drm/cirrus/Kconfig"
> > >
> > > +source "drivers/gpu/drm/digital-blocks/Kconfig"
> > > +
> > >  source "drivers/gpu/drm/armada/Kconfig"
> > >
> > >  source "drivers/gpu/drm/atmel-hlcdc/Kconfig"
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index 9f0d2ee..f525807 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -72,6 +72,7 @@ obj-$(CONFIG_DRM_MGAG200) += mgag200/
> > >  obj-$(CONFIG_DRM_V3D)  += v3d/
> > >  obj-$(CONFIG_DRM_VC4)  += vc4/
> > >  obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
> > > +obj-$(CONFIG_DRM_DB9000) += digital-blocks/
> > >  obj-$(CONFIG_DRM_SIS)   += sis/
> > >  obj-$(CONFIG_DRM_SAVAGE)+= savage/
> > >  obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
> > > diff --git a/drivers/gpu/drm/digital-blocks/Kconfig
> > > b/drivers/gpu/drm/digital-blocks/Kconfig
> > > new file mode 100644
> > > index 0000000..436a7c0
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/digital-blocks/Kconfig
> > > @@ -0,0 +1,13 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +config DRM_DB9000
> > > +     bool "DRM Support for DB9000 LCD Controller"
> > > +     depends on DRM && (ARCH_MULTIPLATFORM || COMPILE_TEST)
> > > +     select DRM_KMS_HELPER
> > > +     select DRM_GEM_CMA_HELPER
> > > +     select DRM_KMS_CMA_HELPER
> > > +     select DRM_PANEL_BRIDGE
> > > +     select VIDEOMODE_HELPERS
> > > +     select FB_PROVIDE_GET_FB_UNMAPPED_AREA if FB
> > > +
> > > +     help
> > > +       Enable DRM support for the DB9000 LCD controller.
> > > diff --git a/drivers/gpu/drm/digital-blocks/Makefile
> > > b/drivers/gpu/drm/digital-blocks/Makefile
> > > new file mode 100644
> > > index 0000000..9f78492
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/digital-blocks/Makefile
> > > @@ -0,0 +1,3 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +obj-$(CONFIG_DRM_DB9000) += db9000-du.o
> > > diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.c
> > > b/drivers/gpu/drm/digital-blocks/db9000-du.c
> > > new file mode 100644
> > > index 0000000..d84d446
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/digital-blocks/db9000-du.c
> > > @@ -0,0 +1,953 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
> > 2020?
I'll correct this in the next version, thanks.

> >
> > > + *
> > > + * Author: Gareth Williams <gareth.williams.jx@renesas.com>
> > > + *
> > > + * Based on ltdc.c
> > > + * Copyright (C) STMicroelectronics SA 2017
> > > + *
> > > + * Authors: Philippe Cornu <philippe.cornu@st.com>
> > > + *          Yannick Fertre <yannick.fertre@st.com>
> > > + *          Fabien Dessenne <fabien.dessenne@st.com>
> > > + *          Mickael Reulier <mickael.reulier@st.com>
> > > + */
> > > +#include <drm/drm_atomic.h>
> > > +#include <drm/drm_atomic_helper.h>
> > > +#include <drm/drm_bridge.h>
> > > +#include <drm/drm_device.h>
> > > +#include <drm/drm_fb_cma_helper.h>
> > > +#include <drm/drm_fb_helper.h>
> > > +#include <drm/drm_fourcc.h>
> > > +#include <drm/drm_gem_framebuffer_helper.h>
> > > +#include <drm/drm_gem_cma_helper.h> #include <drm/drm_of.h>
> > > +#include <drm/drm_panel.h> #include <drm/drm_plane_helper.h>
> > > +#include <drm/drm_probe_helper.h> #include <drm/drm_vblank.h>
> > > +#include <drm/drm_drv.h>
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/dma-mapping.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/pwm.h>
> > > +#include <linux/reset.h>
> > > +
> > > +#include <video/display_timing.h>
> > > +#include <video/videomode.h>
> > > +
> > > +#include "db9000-du.h"
> > > +
> > > +#define NR_CRTC              1
> > > +#define DB9000_FB_MAX_WIDTH  4095
> > > +#define DB9000_FB_MAX_HEIGHT 4095
> > > +#define RZN1_REGS            ((void *) 1)
> > > +
> > > +static const u32 db9000_fmts[] = {
> > > +     DRM_FORMAT_RGB888,
> > > +     DRM_FORMAT_RGB565,
> > > +     DRM_FORMAT_XRGB1555,
> > > +     DRM_FORMAT_XRGB8888,
> > > +     DRM_FORMAT_BGR888,
> > > +     DRM_FORMAT_BGR565,
> > > +     DRM_FORMAT_XBGR1555,
> > > +     DRM_FORMAT_XBGR8888
> > > +};
> > > +
> > > +static u32 reg_read(void __iomem *base, u32 reg) {
> > > +     return readl(base + reg);
> > > +}
> > > +
> > > +static void reg_write(void __iomem *base, u32 reg, u32 val) {
> > > +     writel(val, base + reg);
> > > +}
> > Thiese helpers do not really add any value.
> > Consider using db9000_device* as first argument, so you do not need to
> > do a "->regs" for every use.
> > Or drop them.
I'll change the first argument as that might help readability as well.

> >
> > db9000_device is so much more than a device.
> > Why not just name it struct db9000, and then use db9000 as the
> > preferred variable name?
> > Just a personal preference, feel free to ignore.
I think this is a good observation. The struct expanded in purpose
over time, I will adjust the name. 

> >
> >
> > > +
> > > +static struct db9000_device *crtc_to_db9000(struct drm_crtc *crtc)
> > > +{
> > > +     return container_of(crtc->dev, struct db9000_device, drm); }
> > > +
> > > +static struct db9000_device *plane_to_db9000(struct drm_plane
> > > +*plane) {
> > > +     return container_of(plane->dev, struct db9000_device, drm); }
> > > +
> > > +static struct db9000_device *pwm_chip_to_db9000(struct pwm_chip
> > > +*chip) {
> > > +     return container_of(chip, struct db9000_device, pwm); }
> > > +
> > > +void db9000_bpp_setup(struct db9000_device *db9000_dev, int bpp,
> int bus_width,
> > > +                   bool pixelSelect)
> >
> > The linux kernel style is small caps and underscore like this:
> > pixel_select
I'll correct this in the next version, thanks.

> >
> > > +{
> > > +     u32 format;
> > > +     u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> > > +
> > > +     /* reset the BPP bits */
> > > +     reg_cr1 &= ~DB9000_CR1_BPP(7);
> > > +     reg_cr1 &= ~DB9000_CR1_OPS(5);
> > > +     reg_cr1 &= ~DB9000_CR1_OPS(1);
> > > +     db9000_dev->bpp = bpp;
> > > +
> > > +     switch (bpp) {
> > > +     case 16:
> > > +             if (pixelSelect) {
> > > +                     reg_cr1 |= DB9000_CR1_OPS(5);
> > > +                     reg_cr1 |= DB9000_CR1_OPS(1);
> > > +             }
> > > +
> > > +             format = DB9000_CR1_BPP(DB9000_CR1_BPP_16bpp);
> > > +             break;
> > > +     case 24:
> > > +     case 32:
> > > +     default:
> > > +             format = DB9000_CR1_BPP(DB9000_CR1_BPP_24bpp);
> > > +     }
> > > +
> > > +     if (bpp <= 16 && bus_width == 24)
> > > +             reg_cr1 |= DB9000_CR1_OPS(2);
> > > +     else
> > > +             reg_cr1 &= ~DB9000_CR1_OPS(2);
> > > +
> > > +     if (bpp == 24)
> > > +             reg_cr1 |= DB9000_CR1_FBP;
> > > +     else
> > > +             reg_cr1 &= ~DB9000_CR1_FBP;
> > > +
> > > +     reg_cr1 |= format;
> > > +     reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1); }
> > > +
> > > +void db9000_toggle_controller(struct db9000_device *db9000_dev,
> > > +bool on)
> >
> > USe two helpers here:
> > db9000_controller_on()
> > db9000_controller_off()
> >
> > More descriptive than the bool flag.
I'll implement this as two helpers.

> >
> > > +{
> > > +     u32 isr;
> > > +     u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> > > +     unsigned long flags;
> > > +
> > > +     if (on) {
> > > +             /* Enable controller */
> > > +             reg_cr1 |= DB9000_CR1_LCE;
> > > +             reg_cr1 |= DB9000_CR1_LPE;
> > > +
> > > +             /* DMA Burst Size */
> > > +             reg_cr1 |= DB9000_CR1_FDW(2);
> > > +
> > > +             /* Release pixel clock domain reset */
> > > +             reg_write(db9000_dev->regs, DB9000_PCTR,
> > > + DB9000_PCTR_PCR);
> > > +
> > > +             /* Enable BAU event for IRQ */
> > > +             spin_lock_irqsave(&db9000_dev->lock, flags);
> > > +             isr = reg_read(db9000_dev->regs, DB9000_ISR);
> > > +             reg_write(db9000_dev->regs, DB9000_ISR, isr |
> DB9000_ISR_BAU);
> > > +             reg_write(db9000_dev->regs, DB9000_IMR,
> DB9000_IMR_BAUM);
> > > +             spin_unlock_irqrestore(&db9000_dev->lock, flags);
> > > +
> > > +     } else {
> > > +             /* Disable controller */
> > > +             reg_cr1 &= ~DB9000_CR1_LCE;
> > > +             reg_cr1 &= ~DB9000_CR1_LPE;
> > > +     }
> > > +
> > > +     reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1); }
> > > +
> > > +/* CRTC Functions */
> > > +static void db9000_crtc_atomic_enable(struct drm_crtc *crtc,
> > > +                                   struct drm_crtc_state
> > > +*old_state) {
> > > +     struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> > > +     u32 imr;
> > > +
> > > +     /* Enable IRQ */
> > > +     imr = reg_read(db9000_dev->regs, DB9000_IMR);
> > > +     reg_write(db9000_dev->regs, DB9000_IMR, imr |
> > > +DB9000_IMR_BAUM); }
> > > +
> > > +static void db9000_crtc_atomic_disable(struct drm_crtc *crtc,
> > > +                                    struct drm_crtc_state
> > > +*old_state) {
> > > +     struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> > > +     u32 imr;
> > > +
> > > +     /* disable IRQ */
> > > +     imr = reg_read(db9000_dev->regs, DB9000_IMR);
> > > +     reg_write(db9000_dev->regs, DB9000_IMR, imr &
> > > +~DB9000_IMR_BAUM); }
> > > +
> > > +static void db9000_crtc_mode_set_nofb(struct drm_crtc *crtc) {
> > > +     struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> > > +     struct db9000_device *db9000_dev = crtc_to_db9000(crtc);
> > > +     struct videomode vm;
> > > +     int bus_flags = db9000_dev->connector->display_info.bus_flags;
> > > +     u32 vtr1, vtr2, hvter, htr, hpplor, dear_offset;
> > > +     u32 reg_cr1 = reg_read(db9000_dev->regs, DB9000_CR1);
> > > +
> > > +     drm_display_mode_to_videomode(mode, &vm);
> > There is no need for this conversion, the timing can be fetched from
> > display_mode.
> > And this is the only use of videomode, so you can drop the select in
> > Kconfig too, and the include.
I have it working from the display mode, so I can include it in the next
patch version.

> >
> > > +
> > > +     if (mode->flags & DISPLAY_FLAGS_HSYNC_HIGH)
> > > +             reg_cr1 |= DB9000_CR1_HSP;
> > > +     else
> > > +             reg_cr1 &= ~DB9000_CR1_HSP;
> > > +
> > > +     if (mode->flags & DISPLAY_FLAGS_VSYNC_HIGH)
> > > +             reg_cr1 |= DB9000_CR1_VSP;
> > > +     else
> > > +             reg_cr1 &= ~DB9000_CR1_VSP;
> > > +
> > > +     if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> > > +             reg_cr1 |= DB9000_CR1_DEP;
> > > +     else
> > > +             reg_cr1 &= ~DB9000_CR1_DEP;
> > > +
> > > +     /* Horizontal Timing Register */
> > > +     htr =   DB9000_HTR_HSW(vm.hsync_len) |
> > > +             DB9000_HTR_HBP(vm.hback_porch) |
> > > +             /* Pixels per line */
> > > +             DB9000_HTR_HFP(vm.hfront_porch);
> > > +
> > > +     /* Horizontal Pixels-Per-Line Override */
> > > +     hpplor = vm.hactive | DB9000_HPOE;
> > > +
> > > +     /* Vertical timing registers setup */
> > > +     vtr1 =  DB9000_VTR1_VBP(vm.vback_porch) |
> > > +             DB9000_VTR1_VFP(vm.vfront_porch) |
> > > +             DB9000_VTR1_VSW(vm.vsync_len);
> > > +
> > > +     vtr2 = DB9000_VTR2_LPP(vm.vactive);
> > > +
> > > +     /* Vertical and Horizontal Timing Extension write */
> > > +     hvter = DB9000_HVTER_HFPE(vm.hfront_porch) |
> > > +             DB9000_HVTER_HBPE(vm.hback_porch) |
> > > +             DB9000_HVTER_VFPE(vm.vback_porch) |
> > > +             DB9000_HVTER_VBPE(vm.vfront_porch);
> > > +
> > > +     db9000_dev->frame_size = vm.hactive * vm.vactive;
> > > +
> > > +     /* DEAR register must be configured to the block end + 8 */
> > > +     dear_offset =
> > > +             (db9000_dev->frame_size * db9000_dev->bpp) / 8 + 8;
> > > +
> > > +     reg_write(db9000_dev->regs, DB9000_CR1, reg_cr1);
> > > +     reg_write(db9000_dev->regs, DB9000_HTR, htr);
> > > +     reg_write(db9000_dev->regs, DB9000_VTR1, vtr1);
> > > +     reg_write(db9000_dev->regs, DB9000_VTR2, vtr2);
> > > +     reg_write(db9000_dev->regs, DB9000_HPPLOR, hpplor);
> > > +     reg_write(db9000_dev->regs, DB9000_HVTER, hvter);
> > > +
> > > +     DRM_DEBUG_DRIVER("CRTC:%d mode:%s\n", crtc->base.id, mode-
> >name);
> > > +     DRM_DEBUG_DRIVER("Video mode: %dx%d", vm.hactive,
> vm.vactive);
> > > +     DRM_DEBUG_DRIVER(" hfp %d hbp %d hsl %d vfp %d vbp %d vsl
> %d\n",
> > > +                      vm.hfront_porch, vm.hback_porch, vm.hsync_len,
> > > +                      vm.vfront_porch, vm.vback_porch,
> > > + vm.vsync_len);
> > As a general comment:
> > use drm_dbg(db9000->drm, "foo") and not the deprecated
> DRM_DEBUG_DRIVER.
Okay, I will correct this.

> >
> > > +}
> > > +
> > > +static void db9000_crtc_atomic_flush(struct drm_crtc *crtc,
> > > +                                  struct drm_crtc_state
> > > +*old_crtc_state) {
> > > +     struct drm_pending_vblank_event *event = crtc->state->event;
> > > +
> > > +     if (event) {
> > > +             crtc->state->event = NULL;
> > > +
> > > +             spin_lock_irq(&crtc->dev->event_lock);
> > > +             drm_crtc_send_vblank_event(crtc, event);
> > > +             spin_unlock_irq(&crtc->dev->event_lock);
> > > +     }
> > > +}
> > > +
> > > +static const struct drm_crtc_helper_funcs db9000_crtc_helper_funcs = {
> > > +     .mode_set_nofb = db9000_crtc_mode_set_nofb,
> > > +     .atomic_flush = db9000_crtc_atomic_flush,
> > > +     .atomic_enable = db9000_crtc_atomic_enable,
> > > +     .atomic_disable = db9000_crtc_atomic_disable, };
> > > +
> > > +static const struct drm_crtc_funcs db9000_crtc_funcs = {
> > > +     .destroy = drm_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,
> > > +     .gamma_set = drm_atomic_helper_legacy_gamma_set,
> > > +};
> > > + 
> > > +/*
> > > + * DRM_PLANE
> > > + */
> > > +static void db9000_plane_atomic_update(struct drm_plane *plane,
> > > +                                    struct drm_plane_state
> > > +*oldstate) {
> > > +     struct db9000_device *db9000_dev = plane_to_db9000(plane);
> > > +     struct drm_plane_state *state = plane->state;
> > > +     struct drm_framebuffer *fb = state->fb;
> > > +     unsigned long flags;
> > > +     u32 isr, paddr, dear_offset;
> > > +     u32 format;
> > > +
> > > +     if (!state->crtc || !fb) {
> > > +             DRM_DEBUG_DRIVER("fb or crtc NULL\n");
> > > +             return;
> > > +     }
> > > +
> > > +     format = fb->format->format;
> > > +
> > > +     /* The single plane is turning visible, so turn on the display */
> > > +     if (!oldstate->visible && state->visible)
> > > +             db9000_toggle_controller(db9000_dev, false);
> > > +
> > > +     /* The plane is no longer visible */
> > > +     if (oldstate->visible && !state->visible)
> > > +             db9000_toggle_controller(db9000_dev, true);
> > > +
> > > +     /* Check for format changes */
> > > +     if (format == DRM_FORMAT_RGB565 || format ==
> DRM_FORMAT_BGR565)
> > > +             db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width,
> false);
> > > +     else if (format == DRM_FORMAT_XRGB1555 || format ==
> DRM_FORMAT_XBGR1555)
> > > +             db9000_bpp_setup(db9000_dev, 16, db9000_dev->bus_width,
> true);
> > > +     else if (format == DRM_FORMAT_RGB888 || format ==
> DRM_FORMAT_BGR888)
> > > +             db9000_bpp_setup(db9000_dev, 24, db9000_dev->bus_width,
> false);
> > > +     else if (format == DRM_FORMAT_XRGB8888 || format ==
> DRM_FORMAT_XBGR8888)
> > > +             db9000_bpp_setup(db9000_dev, 32,
> > > + db9000_dev->bus_width, false);
> > > +
> > > +     dear_offset = (db9000_dev->frame_size * db9000_dev->bpp) / 8 +
> > > + 8;
> > > +
> > > +     /* The frame buffer has changed, so get the new FB address */
> > > +     if (oldstate->fb != state->fb || state->crtc->state->mode_changed) {
> > > +             paddr = (u32)drm_fb_cma_get_gem_addr(fb, state, 0);
> > > +
> > > +             DRM_DEBUG_DRIVER("fb: phys 0x%08x\n", paddr);
> > > +             reg_write(db9000_dev->regs, DB9000_DBAR, paddr);
> > > +             reg_write(db9000_dev->regs, DB9000_DEAR,
> > > +                       paddr + dear_offset);
> > > +     }
> > > +
> > > +     /* Enable BAU event */
> > > +     spin_lock_irqsave(&db9000_dev->lock, flags);
> > > +     isr = reg_read(db9000_dev->regs, DB9000_ISR);
> > > +     reg_write(db9000_dev->regs, DB9000_ISR, isr | DB9000_ISR_BAU);
> > > +     reg_write(db9000_dev->regs, DB9000_IMR, DB9000_IMR_BAUM);
> > > +     spin_unlock_irqrestore(&db9000_dev->lock, flags);
> > > +
> > > +     db9000_dev->plane_fpsi->counter++;
> > > +
> > > +     if (isr & DB9000_ISR_MBE) {
> > > +             if (isr & DB9000_ISR_OFU)
> > > +                     DRM_ERROR("Output FIFO Underrun\n");
> > > +
> > > +             if (isr & DB9000_ISR_OFO)
> > > +                     DRM_ERROR("Output FIFO Overrun\n");
> > > +
> > > +             if (isr & DB9000_ISR_IFU)
> > > +                     DRM_ERROR("Input FIFO Underrun\n");
> > > +
> > > +             if (isr & DB9000_ISR_IFO)
> > > +                     DRM_ERROR("Input FIFO Overrun\n");
> > > +     }
> > > +}
> > > +
> > > +static void db9000_plane_atomic_print_state(struct drm_printer *p,
> > > +                                         const struct
> > > +drm_plane_state *state) {
> > > +     struct drm_plane *plane = state->plane;
> > > +     struct db9000_device *db9000_dev = plane_to_db9000(plane);
> > > +     struct fps_info *fpsi = db9000_dev->plane_fpsi;
> > > +     int ms_since_last;
> > > +     ktime_t now;
> > > +
> > > +     now = ktime_get();
> > > +     ms_since_last = ktime_to_ms(ktime_sub(now,
> > > + fpsi->last_timestamp));
> > > +
> > > +     drm_printf(p, "\tuser_updates=%dfps\n",
> > > +                DIV_ROUND_CLOSEST(fpsi->counter * 1000,
> > > + ms_since_last));
> > > +
> > > +     fpsi->last_timestamp = now;
> > > +     fpsi->counter = 0;
> > > +}
> > > +
> > > +static const struct drm_plane_funcs db9000_plane_funcs = {
> > > +     .update_plane = drm_atomic_helper_update_plane,
> > > +     .disable_plane = drm_atomic_helper_disable_plane,
> > > +     .destroy = drm_plane_cleanup,
> > > +     .reset = drm_atomic_helper_plane_reset,
> > > +     .atomic_duplicate_state =
> drm_atomic_helper_plane_duplicate_state,
> > > +     .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> > > +     .atomic_print_state = db9000_plane_atomic_print_state, };
> > > +
> > > +static const struct drm_plane_helper_funcs
> db9000_plane_helper_funcs = {
> > > +     .atomic_update = db9000_plane_atomic_update, };
> > > +
> > > +static struct drm_plane *db9000_plane_create(struct drm_device
> *ddev,
> > > +                                          enum drm_plane_type type)
> > > +{
> > > +     struct device *dev = ddev->dev;
> > > +     struct drm_plane *plane;
> > > +     int ret;
> > > +
> > > +     plane = devm_kzalloc(dev, sizeof(*plane), GFP_KERNEL);
> > > +     if (!plane)
> > > +             return NULL;
> > > +
> > > +     ret = drm_universal_plane_init(ddev, plane, NR_CRTC,
> > > +                                    &db9000_plane_funcs,
> > > +                                    db9000_fmts,
> > > +                                    ARRAY_SIZE(db9000_fmts), NULL,
> > > +                                    type, "rzn1_primary_rgb");
> > > +     if (ret < 0)
> > > +             return NULL;
> > > +
> > > +     drm_plane_helper_add(plane, &db9000_plane_helper_funcs);
> > > +
> > > +     DRM_DEBUG_DRIVER("plane:%d created\n", plane->base.id);
> > > +
> > > +     return plane;
> > > +}
> > > +
> > > +static void db9000_plane_destroy_all(struct drm_device *ddev) {
> > > +     struct drm_plane *plane, *plane_temp;
> > > +
> > > +     list_for_each_entry_safe(plane, plane_temp,
> > > +                              &ddev->mode_config.plane_list, head)
> > > +                              drm_plane_cleanup(plane); }
> > > +
> > > +static int db9000_crtc_init(struct drm_device *ddev, struct
> > > +drm_crtc *crtc) {
> > > +     struct drm_plane *primary;
> > > +     int ret;
> > > +
> > > +     primary = db9000_plane_create(ddev,
> DRM_PLANE_TYPE_PRIMARY);
> > > +     if (!primary) {
> > > +             DRM_ERROR("Can not create primary plane\n");
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     ret = drm_crtc_init_with_planes(ddev, crtc, primary, NULL,
> > > +                                     &db9000_crtc_funcs, NULL);
> > > +     if (ret) {
> > > +             DRM_ERROR("Can not initialize CRTC\n");
> > > +             goto cleanup;
> > > +     }
> > > +
> > > +     drm_crtc_helper_add(crtc, &db9000_crtc_helper_funcs);
> > > +     DRM_DEBUG_DRIVER("CRTC:%d created\n", crtc->base.id);
> > > +     return 0;
> > > +
> > > +cleanup:
> > > +     db9000_plane_destroy_all(ddev);
> > > +     return ret;
> > > +}
> > > +
> > > +/*
> > > + * DRM_ENCODER
> > > + */
> > > +static const struct drm_encoder_funcs db9000_encoder_funcs = {
> > > +     .destroy = drm_encoder_cleanup, };
> > > +
> > > +static int db9000_encoder_init(struct drm_device *ddev,
> > > +                            struct drm_bridge *bridge) {
> > > +     struct drm_encoder *encoder;
> > > +     int ret;
> > > +
> > > +     encoder = devm_kzalloc(ddev->dev, sizeof(*encoder),
> GFP_KERNEL);
> > > +     if (!encoder)
> > > +             return -ENOMEM;
> > > +
> > > +     encoder->possible_crtcs = NR_CRTC;
> > > +     encoder->possible_clones = 0; /* No cloning support */
> > > +
> > > +     drm_encoder_init(ddev, encoder, &db9000_encoder_funcs,
> > > +                      DRM_MODE_ENCODER_NONE, NULL);
> > > +
> > > +     ret = drm_bridge_attach(encoder, bridge, NULL);
> > > +     if (ret) {
> > > +             drm_encoder_cleanup(encoder);
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     DRM_DEBUG_DRIVER("Bridge encoder:%d created\n",
> > > + encoder->base.id);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +void __maybe_unused db9000_suspend(struct drm_device *ddev) {
> > > +     struct db9000_device *db9000_dev = container_of(ddev,
> > > +                                                 struct
> > > +db9000_device, drm);
> > > +
> > > +     clk_disable_unprepare(db9000_dev->lcd_eclk);
> > > +}
> > > +
> > > +int __maybe_unused db9000_resume(struct drm_device *ddev) {
> > > +     struct db9000_device *db9000_dev = container_of(ddev,
> > > +                                                 struct db9000_device, drm);
> > > +     int ret;
> > > +
> > > +     ret = clk_prepare_enable(db9000_dev->lcd_eclk);
> > > +     if (ret) {
> > > +             DRM_ERROR("failed to enable pixel clock (%d)\n", ret);
> > > +             return ret;
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int db9000_pwm_request(struct pwm_chip *chip, struct
> > > +pwm_device *pwm) {
> > > +     return pm_runtime_get_sync(chip->dev); }
> > > +
> > > +static void db9000_pwm_free(struct pwm_chip *chip, struct
> > > +pwm_device *pwm) {
> > > +     pm_runtime_put(chip->dev);
> > > +}
> > > +
> > > +static int db9000_pwm_enable(struct db9000_device *db9000_dev) {
> > > +     u32 reg_pwmfr = reg_read(db9000_dev->regs,
> > > +db9000_dev->addr_pwmfr);
> > > +
> > > +     reg_pwmfr |= DB9000_PWMFR_PWMFCE;
> > > +     reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr,
> > > + reg_pwmfr);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static void db9000_pwm_disable(struct db9000_device *db9000_dev) {
> > > +     u32 reg_pwmfr = reg_read(db9000_dev->regs,
> > > +db9000_dev->addr_pwmfr);
> > > +
> > > +     reg_pwmfr &= ~DB9000_PWMFR_PWMFCE;
> > > +     reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr,
> > > +reg_pwmfr); }
> > > +
> > > +static int db9000_pwm_apply(struct pwm_chip *chip, struct
> pwm_device *pwm,
> > > +                         struct pwm_state *state) {
> > > +     struct pwm_state cur_state;
> > > +     struct db9000_device *db9000_dev = pwm_chip_to_db9000(chip);
> > > +     int ret;
> > > +
> > > +     pwm_get_state(pwm, &cur_state);
> > > +
> > > +     if (state->enabled)
> > > +             ret = db9000_pwm_enable(db9000_dev);
> > > +     else
> > > +             db9000_pwm_disable(db9000_dev);
> > > +
> > > +     if (state->period != cur_state.period) {
> > > +             u32 pwmfcd;
> > > +
> > > +             pwmfcd = clk_get_rate(db9000_dev->lcd_eclk) / 256;
> > > +             pwmfcd *= state->period;
> > > +             pwmfcd = DB9000_PWMFR_PWMFCD(pwmfcd - 1);
> > > +             reg_write(db9000_dev->regs, db9000_dev->addr_pwmfr,
> pwmfcd);
> > > +     }
> > > +
> > > +     if (state->duty_cycle == 0) {
> > > +             db9000_pwm_disable(db9000_dev);
> > > +     } else if (state->period != cur_state.period ||
> > > +         state->duty_cycle != cur_state.duty_cycle) {
> > > +             u32 dcr = div_u64((state->duty_cycle * ULL(256)),
> > > +                                state->period) - 1;
> > > +
> > > +             reg_write(db9000_dev->regs, db9000_dev->addr_pwmdcr, dcr);
> > > +     }
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static const struct pwm_ops db9000_pwm_ops = {
> > > +     .request = db9000_pwm_request,
> > > +     .free = db9000_pwm_free,
> > > +     .apply = db9000_pwm_apply,
> > > +     .owner = THIS_MODULE,
> > > +};
> > > +
> > > +static int db9000_pwm_setup(struct device *dev,
> > > +                         struct db9000_device *db9000_dev) {
> > > +     struct pwm_chip *db9000_pwm;
> > > +     int ret;
> > > +
> > > +     db9000_pwm = devm_kzalloc(dev, sizeof(*db9000_pwm),
> GFP_KERNEL);
> > > +     if (db9000_pwm == NULL)
> > > +             return -ENOMEM;
> > > +
> > > +     db9000_pwm = &db9000_dev->pwm;
> > > +
> > > +     db9000_pwm->dev = dev;
> > > +     db9000_pwm->ops = &db9000_pwm_ops;
> > > +     db9000_pwm->base = -1;
> > > +     db9000_pwm->npwm = 1;
> > > +
> > > +     ret = pwmchip_add(db9000_pwm);
> > > +     if (ret < 0) {
> > > +             dev_err(dev, "failed to register PWM chip: %d\n", ret);
> > > +             return ret;
> > > +     }
> > > +
> > > +     pm_runtime_enable(dev);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +int db9000_load(struct drm_device *ddev, int rzn1_pwm) {
> > > +     struct platform_device *pdev = to_platform_device(ddev->dev);
> > > +     struct device *dev = ddev->dev;
> > > +     struct db9000_device *db9000_dev = container_of(ddev,
> > > +                                                 struct db9000_device, drm);
> > > +     struct device_node *np = dev->of_node;
> > > +     struct drm_bridge *bridge;
> > > +     struct drm_panel *panel;
> > > +     struct drm_crtc *crtc;
> > > +     struct reset_control *rstc;
> > > +     struct resource *res;
> > > +     int ret;
> > > +
> > > +     spin_lock_init(&db9000_dev->lock);
> > > +
> > > +     if (rzn1_pwm) {
> > > +             db9000_dev->addr_pwmfr = DB9000_PWMFR_RZN1;
> > > +             db9000_dev->addr_pwmdcr = DB9000_PWMDCR_RZN1;
> > > +     } else {
> > > +             db9000_dev->addr_pwmfr = DB9000_PWMFR_0;
> > > +             db9000_dev->addr_pwmdcr = DB9000_PWMDCR_0;
> > > +     }
> > > +
> > > +     /* Panel Setup */
> > > +     ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge);
> > > +     if (ret != 0) {
> > > +             DRM_ERROR("Could not get Panel or bridge");
> > > +             return ret;
> > > +     }
> > > +
> > > +     rstc = devm_reset_control_get_exclusive(dev, NULL);
> > > +
> > > +     /* Clock setup */
> > > +     db9000_dev->lcd_eclk = devm_clk_get(dev, "lcd_eclk");
> > > +
> > > +     if (IS_ERR(db9000_dev->lcd_eclk)) {
> > > +             DRM_ERROR("Unable to get pixel clock\n");
> > > +             return -ENODEV;
> > > +     }
> > > +
> > > +     if (clk_prepare_enable(db9000_dev->lcd_eclk)) {
> > > +             DRM_ERROR("Unable to prepare pixel clock\n");
> > > +             return -ENODEV;
> > > +     }
> > > +
> > > +     /* Memory setup */
> > > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +     if (!res) {
> > > +             DRM_ERROR("Could not retrieve platform resources\n");
> > > +             ret = -EINVAL;
> > > +             goto err;
> > > +     }
> > > +
> > > +     db9000_dev->regs = devm_ioremap_resource(dev, res);
> > > +     if (IS_ERR(db9000_dev->regs)) {
> > > +             DRM_ERROR("Unable to get memory resource\n");
> > > +             ret = PTR_ERR(db9000_dev->regs);
> > > +             goto err;
> > > +     }
> > > +
> > > +     db9000_bpp_setup(db9000_dev, db9000_dev->bpp, db9000_dev-
> >bus_width,
> > > +                      false);
> > > +
> > > +     db9000_toggle_controller(db9000_dev, true);
> > > +
> > > +     /* Add endpoints panels or bridges if any */
> > > +     if (panel) {
> > > +             bridge = drm_panel_bridge_add(panel,
> > > +                                           DRM_MODE_CONNECTOR_Unknown);
> > > +             if (IS_ERR(bridge)) {
> > > +                     DRM_ERROR("panel-bridge endpoint\n");
> > > +                     ret = PTR_ERR(bridge);
> > > +                     goto err;
> > > +             }
> > > +     }
> > > +
> > > +     if (bridge) {
> > > +             ret = db9000_encoder_init(ddev, bridge);
> > > +             if (ret) {
> > > +                     DRM_ERROR("init encoder endpoint\n");
> > > +                     goto err;
> > > +             }
> > > +     }
> > > +
> > > +     db9000_dev->connector = panel->connector;
> > > +
> > > +     /* CRTC setup  */
> > > +     crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
> > > +     if (!crtc) {
> > > +             DRM_ERROR("Failed to allocate crtc\n");
> > > +             ret = -ENOMEM;
> > > +             goto err;
> > > +     }
> > > +
> > > +     ret = db9000_crtc_init(&db9000_dev->drm, crtc);
> > > +     if (ret) {
> > > +             DRM_ERROR("Failed to init crtc\n");
> > > +             goto err;
> > > +     }
> > > +
> > > +     if (!IS_ERR(rstc)) {
> > > +             reset_control_assert(rstc);
> > > +             usleep_range(10, 20);
> > > +             reset_control_deassert(rstc);
> > > +     }
> > > +
> > > +     return db9000_pwm_setup(dev, db9000_dev);
> > > +
> > > +err:
> > > +     drm_panel_bridge_remove(bridge);
> > > +
> > > +     clk_disable_unprepare(db9000_dev->lcd_eclk);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +void db9000_unload(struct drm_device *ddev) {
> > > +     struct db9000_device *db9000_dev;
> > > +
> > > +     db9000_dev = container_of(ddev, struct db9000_device, drm);
> > > +
> > > +     drm_of_panel_bridge_remove(ddev->dev->of_node, 0, 0);
> > > +     clk_disable_unprepare(db9000_dev->lcd_eclk);
> > > +}
> > > +
> > > +static const struct drm_mode_config_funcs drv_mode_config_funcs = {
> > > +     .fb_create = drm_gem_fb_create,
> > > +     .atomic_check = drm_atomic_helper_check,
> > > +     .atomic_commit = drm_atomic_helper_commit, };
> > > +
> > > +static int db9000_gem_cma_dumb_create(struct drm_file *file,
> > > +                                   struct drm_device *dev,
> > > +                                   struct drm_mode_create_dumb
> > > +*args) {
> > > +     return drm_gem_cma_dumb_create_internal(file, dev, args); }
> > This wrapper seems like it can be dropped.
I agree, I will remove this.

> >
> > > +
> > > +DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
> > > +
> > > +static struct drm_driver drv_driver = {
> > > +     .driver_features = DRIVER_MODESET | DRIVER_GEM |
> DRIVER_PRIME |
> > > +                        DRIVER_ATOMIC,
> > > +     .name = "drm-db9000",
> > > +     .desc = "Digital Blocks DB9000 DRM Driver",
> > > +     .date = "20190825",
> > > +     .major = 1,
> > > +     .minor = 0,
> > > +     .patchlevel = 0,
> > > +     .fops = &drv_driver_fops,
> > > +     .dumb_create = db9000_gem_cma_dumb_create,
> > > +     .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > > +     .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > > +     .gem_free_object_unlocked = drm_gem_cma_free_object,
> > > +     .gem_vm_ops = &drm_gem_cma_vm_ops,
> > > +     .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, };
> > I think if you look up these operations, then you have assinged the
> > default operations in mayny cases.
> > Please see if you can trim the list.
> >
> > Also double check that no legacy operatiosn are used.
I'll simplify this as much as possible.

> >
> > > +
> > > +static int drv_load(struct drm_device *ddev, u32 bpp,
> > > +                 u32 bus_width, int rzn1_pwm) {
> > > +     struct platform_device *pdev = to_platform_device(ddev->dev);
> > > +     struct db9000_device *db9000_dev = container_of(ddev,
> > > +                                                 struct db9000_device, drm);
> > > +     int ret;
> > > +
> > > +     drm_mode_config_init(ddev);
> > > +
> > > +     /*
> > > +      * set max width and height as default value.
> > > +      * this value would be used to check framebuffer size limitation
> > > +      * at drm_mode_addfb().
> > > +      */
> > > +     ddev->mode_config.min_width = 0;
> > > +     ddev->mode_config.min_height = 0;
> > > +     ddev->mode_config.max_width = DB9000_FB_MAX_WIDTH;
> > > +     ddev->mode_config.max_height = DB9000_FB_MAX_WIDTH;
> > > +     ddev->mode_config.funcs = &drv_mode_config_funcs;
> > > +
> > > +     db9000_dev->bus_width = bus_width;
> > > +
> > > +     ret = db9000_load(ddev, rzn1_pwm);
> > > +
> > > +     if (ret)
> > > +             goto err;
> > > +
> > > +     drm_mode_config_reset(ddev);
> > > +
> > > +     drm_kms_helper_poll_init(ddev);
> > > +
> > > +     platform_set_drvdata(pdev, ddev);
> > > +
> > > +     return 0;
> > > +
> > > +err:
> > > +     drm_mode_config_cleanup(ddev);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static void drv_unload(struct drm_device *ddev) {
> > > +     drm_kms_helper_poll_fini(ddev);
> > > +     db9000_unload(ddev);
> > > +     drm_mode_config_cleanup(ddev); }
> > > +
> > > +static __maybe_unused int db9000_drv_suspend(struct device *dev) {
> > > +     struct drm_device *ddev = dev_get_drvdata(dev);
> > > +     struct db9000_device *db9000_dev = container_of(ddev,
> > > +                                                     struct db9000_device,
> > > +                                                     drm);
> > > +     struct drm_atomic_state *state;
> > > +
> > > +     db9000_toggle_controller(db9000_dev, false);
> > > +
> > > +     drm_kms_helper_poll_disable(ddev);
> > > +     state = drm_atomic_helper_suspend(ddev);
> > > +     if (IS_ERR(state)) {
> > > +             drm_kms_helper_poll_enable(ddev);
> > > +             return PTR_ERR(state);
> > > +     }
> > > +     db9000_dev->suspend_state = state;
> > > +     db9000_suspend(ddev);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static __maybe_unused int db9000_drv_resume(struct device *dev) {
> > > +     struct drm_device *ddev = dev_get_drvdata(dev);
> > > +     struct db9000_device *db9000_dev = container_of(ddev,
> > > +                                                     struct db9000_device,
> > > +                                                     drm);
> > > +
> > > +     db9000_resume(ddev);
> > > +     drm_atomic_helper_resume(ddev, db9000_dev->suspend_state);
> > > +     drm_kms_helper_poll_enable(ddev);
> > > +     db9000_toggle_controller(db9000_dev, true);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static const struct dev_pm_ops drv_pm_ops = {
> > > +     SET_SYSTEM_SLEEP_PM_OPS(db9000_drv_suspend,
> db9000_drv_resume)
> > > +};
> > > +
> > > +static int db9000_drm_platform_probe(struct platform_device *pdev)
> > > +{
> > > +     struct device *dev = &pdev->dev;
> > > +     struct drm_device *ddev;
> > > +     struct db9000_device *db9000_dev;
> > > +     struct device_node *np = dev->of_node;
> > > +     u32 bpp;
> > > +     u32 bus_width;
> > > +     int rzn1_pwm = 0;
> > > +     int ret;
> > > +
> > > +     dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> > > +
> > > +     db9000_dev = kzalloc(sizeof(*db9000_dev), GFP_KERNEL);
> > > +     if (!db9000_dev)
> > > +             return -ENOMEM;
> > > +
> > > +     drm_dev_init(&db9000_dev->drm, &drv_driver, dev);
> > > +     ddev = &db9000_dev->drm;
> > Use the newly merged drmm stuff here.
> >
> > > +
> > > +     /* Parse the DTB */
> > > +     ret = of_property_read_u32(np, "bits-per-pixel", &bpp);
> > > +     if (ret)
> > > +             bpp = 16;
> > > +
> > > +     if (bpp != 16 && bpp != 24 && bpp != 32) {
> > > +             DRM_WARN("bits-per-pixel value invalid, default is 16 bpp");
> > > +             bpp = 16;
> > > +     }
> > > +
> > > +     ret = of_property_read_u32(np, "bus-width", &bus_width);
> > > +     if (ret)
> > > +             bus_width = 24;
> > > +
> > > +     rzn1_pwm = (int) of_device_get_match_data(dev);
> > > +
> > > +     ret = drv_load(ddev, bpp, bus_width, rzn1_pwm);
> > > +     if (ret)
> > > +             goto err_put;
> > > +
> > > +     ret = drm_dev_register(ddev, 0);
> > > +     if (ret)
> > > +             goto err_put;
> > > +
> > > +     ret = drm_fbdev_generic_setup(ddev, bpp);
> > > +     if (ret)
> > > +             goto err_put;
> > > +
> > > +     dev_info(dev, "DB9000 LCD Controller Ready");
> > > +
> > > +     return 0;
> > > +
> > > +err_put:
> > > +     drm_dev_put(ddev);
> > With drmm_ support this put goes aways.
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static int db9000_drm_platform_remove(struct platform_device *pdev)
> > > +{
> > > +     struct drm_device *ddev = platform_get_drvdata(pdev);
> > > +
> > > +     drv_unload(ddev);
> > > +     drm_dev_put(ddev);
> > With drmm_ support this put goes aways.
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static const struct of_device_id drv_dt_ids[] = {
> > > +     { .compatible = "digital-blocks,drm-db9000"},
> > > +     { .compatible = "digital-blocks,drm-rzn1", .data = RZN1_REGS },
> > > +     { /* end node */ },
> > > +};
> > > +MODULE_DEVICE_TABLE(of, drv_dt_ids);
> > > +
> > > +static struct platform_driver db9000_drm_platform_driver = {
> > > +     .probe = db9000_drm_platform_probe,
> > > +     .remove = db9000_drm_platform_remove,
> > > +     .driver = {
> > > +             .name = "drm-db9000",
> > > +             .of_match_table = drv_dt_ids,
> > > +             .pm = &drv_pm_ops,
> > > +     },
> > > +};
> > > +
> > > +module_platform_driver(db9000_drm_platform_driver);
> > > +
> > > +MODULE_AUTHOR("Gareth Williams
> <gareth.williams.jx@renesas.com>");
> > > +MODULE_DESCRIPTION("Digital Blocks DB9000 LCD Controller Driver");
> > > +MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/gpu/drm/digital-blocks/db9000-du.h
> > > b/drivers/gpu/drm/digital-blocks/db9000-du.h
> > > new file mode 100644
> > > index 0000000..325c9f0
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/digital-blocks/db9000-du.h
> > > @@ -0,0 +1,192 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/* Copyright (C) 2019 Renesas Electronics Europe Ltd.
> > > + *
> > > + * Author: Gareth Williams <gareth.williams.jx@renesas.com>
> > > + *
> > > + * Based on ltdc.h
> > > + * Copyright (C) STMicroelectronics SA 2017
> > > + *
> > > + * Authors: Philippe Cornu <philippe.cornu@st.com>
> > > + *          Yannick Fertre <yannick.fertre@st.com>
> > > + *          Fabien Dessenne <fabien.dessenne@st.com>
> > > + *          Mickael Reulier <mickael.reulier@st.com>
> > > + */
> > > +#ifndef __DB9000_DU_H__
> > > +#define __DB9000_DU_H__
> > > +
> > > +#include <linux/backlight.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/pwm.h>
> > > +
> > > +#define DB9000_MAX_LAYER             1
> > > +
> > > +/* LCD Controller Control Register 1 */
> > > +#define DB9000_CR1                   0x000
> > > +/* Horizontal Timing Register */
> > > +#define DB9000_HTR                   0x008
> > > +/* Vertical Timing Register 1 */
> > > +#define DB9000_VTR1                  0x00C
> > > +/* Vertical Timing Register 2 */
> > > +#define DB9000_VTR2                  0x010
> > > +/* Pixel Clock Timing Register */
> > > +#define DB9000_PCTR                  0x014
> > > +/* Interrupt Status Register */
> > > +#define DB9000_ISR                   0x018
> > > +/* Interrupt Mask Register */
> > > +#define DB9000_IMR                   0x01C
> > > +/* Interrupt Vector Register */
> > > +#define DB9000_IVR                   0x020
> > > +/* Interrupt Scan Compare Register */
> > > +#define DB9000_ISCR                  0x024
> > > +/* DMA Base Address Register */
> > > +#define DB9000_DBAR                  0x028
> > > +/* DMA Current Address Register */
> > > +#define DB9000_DCAR                  0x02C
> > > +/* DMA End Address Register */
> > > +#define DB9000_DEAR                  0x030
> > > +/* DMA Horizontal and Vertical Timing Extension Register */
> > > +#define DB9000_HVTER                 0x044
> > > +/* Horizontal Pixels-Per-Line Override Control */
> > > +#define DB9000_HPPLOR                        0x048
> > > +/* Horizontal Pixels-Per-Line Override Enable */
> > > +#define DB9000_HPOE                  BIT(31)
> > > +/* GPIO Register */
> > > +#define DB9000_GPIOR                 0x1F8
> > > +/* Core Identification Register */
> > > +#define DB9000_CIR                   0x1FC
> > > +/* Palette Data Words */
> > > +#define DB9000_PALT                  0x200
> > > +
> > > +/* Control Register 1, Offset 0x000 */
> > > +/* LCD Controller Enable */
> > > +#define DB9000_CR1_LCE                       BIT(0)
> > > +/* LCD Power Enable */
> > > +#define DB9000_CR1_LPE                       BIT(1)
> > > +/* LCD Bits per Pixel */
> > > +#define DB9000_CR1_BPP(x)            (((x) & 0x7) << 2)
> > > +/* RGB or BGR Format */
> > > +#define DB9000_CR1_RGB                       BIT(5)
> > > +/* Data Enable Polarity */
> > > +#define DB9000_CR1_DEP                       BIT(8)
> > > +/* Pixel Clock Polarity */
> > > +#define DB9000_CR1_PCP                       BIT(9)
> > > +/* Horizontal Sync Polarity */
> > > +#define DB9000_CR1_HSP                       BIT(10)
> > > +/* Vertical Sync Polarity */
> > > +#define DB9000_CR1_VSP                       BIT(11)
> > > +/* Output Pixel Select */
> > > +#define DB9000_CR1_OPS(x)            (((x) & 0x7) << 12)
> > > +/* FIFO DMA Request Words */
> > > +#define DB9000_CR1_FDW(x)            (((x) & 0x3) << 16)
> > > +/* LCD 1 or Port Select */
> > > +#define DB9000_CR1_LPS                       BIT(18)
> > > +/* Frame Buffer 24bpp Packed Word */
> > > +#define DB9000_CR1_FBP                       BIT(19)
> > > +
> > > +enum DB9000_CR1_BPP {
> > > +     /* 1 bit per pixel */
> > > +     DB9000_CR1_BPP_1bpp,
> > > +     /* 2 bits per pixel */
> > > +     DB9000_CR1_BPP_2bpp,
> > > +     /* 4 bits per pixel */
> > > +     DB9000_CR1_BPP_4bpp,
> > > +     /* 8 bits per pixel */
> > > +     DB9000_CR1_BPP_8bpp,
> > > +     /* 16 bits per pixel */
> > > +     DB9000_CR1_BPP_16bpp,
> > > +     /* 18 bits per pixel */
> > > +     DB9000_CR1_BPP_18bpp,
> > > +     /* 24 bits per pixel */
> > > +     DB9000_CR1_BPP_24bpp
> > > +} DB9000_CR1_BPP_VAL;
> > > +
> > > +/* Horizontal Timing Register, Offset 0x008 */
> > > +/* Horizontal Front Porch */
> > > +#define DB9000_HTR_HFP(x)            (((x) & 0xff) << 0)
> > > +/* Pixels per Line */
> > > +#define DB9000_HTR_PPL(x)            (((x) & 0xff) << 8)
> > > +/* Horizontal Back Porch */
> > > +#define DB9000_HTR_HBP(x)            (((x) & 0xff) << 16)
> > > +/* Horizontal Sync Width */
> > > +#define DB9000_HTR_HSW(x)            (((x) & 0xff) << 24)
> > > +
> > > +/* Vertical Timing Register 1, Offset 0x00C */
> > > +/* Vertical Sync Width */
> > > +#define DB9000_VTR1_VSW(x)           (((x) & 0xff) << 0)
> > > +/* Vertical Front Porch */
> > > +#define DB9000_VTR1_VFP(x)           (((x) & 0xff) << 8)
> > > +/* Vertical Back Porch */
> > > +#define DB9000_VTR1_VBP(x)           (((x) & 0xff) << 16)
> > > +
> > > +/* Vertical Timing Register 2, Offset 0x010 */
> > > +/* Lines Per Panel */
> > > +#define DB9000_VTR2_LPP(x)           (((x) & 0xfff) << 0)
> > > +
> > > +/* Vertical and Horizontal Timing Extension Register, Offset 0x044
> > > +*/
> > > +/* Horizontal Front Porch Extension */
> > > +#define DB9000_HVTER_HFPE(x)         ((((x) >> 8) & 0x3) << 0)
> > > +/* Horizontal Back Porch Extension */
> > > +#define DB9000_HVTER_HBPE(x)         ((((x) >> 8) & 0x3) << 4)
> > > +/* Vertical Front Porch Extension */
> > > +#define DB9000_HVTER_VFPE(x)         ((((x) >> 8) & 0x3) << 8)
> > > +/* Vertical Back Porch Extension */
> > > +#define DB9000_HVTER_VBPE(x)         ((((x) >> 8) & 0x3) << 12)
> > > +
> > > +/* clock reset select */
> > > +#define DB9000_PCTR_PCR                      BIT(10)
> > > +
> > > +/* Interrupt Status Register, Offset 0x018 */
> > > +#define DB9000_ISR_OFU                       BIT(0) /* Output FIFO Underrun */
> > > +#define DB9000_ISR_OFO                       BIT(1) /* Output FIFO Overrun */
> > > +#define DB9000_ISR_IFU                       BIT(2) /* Input FIFO Underrun */
> > > +#define DB9000_ISR_IFO                       BIT(3) /* Input FIFO Overrun */
> > > +#define DB9000_ISR_FER                       BIT(4) /* OR of OFU, OFO, IFU, IFO
> */
> > > +#define DB9000_ISR_MBE                       BIT(5) /* Master Bus Error */
> > > +#define DB9000_ISR_VCT                       BIT(6) /* Vertical Compare
> Triggered */
> > > +#define DB9000_ISR_BAU                       BIT(7) /* DMA Base Address
> Register Update to CAR */
> > > +#define DB9000_ISR_LDD                       BIT(8) /* LCD Controller Disable
> Done */
> > > +
> > > +/* Interrupt Mask Register, Offset 0x01C */
> > > +#define DB9000_IMR_OFUM                      BIT(0) /* Output FIFO Underrun
> - Mask */
> > > +#define DB9000_IMR_OFOM                      BIT(1) /* Output FIFO Overrun -
> Mask */
> > > +#define DB9000_IMR_IFUM                      BIT(2) /* Input FIFO Underrun -
> Mask */
> > > +#define DB9000_IMR_IFOM                      BIT(3) /* Input FIFO Overrun -
> Mask */
> > > +#define DB9000_IMR_FERM                      BIT(4) /* OR of OFU, OFO, IFU,
> IFO - Mask */
> > > +#define DB9000_IMR_MBEM                      BIT(5) /* Master Bus Error -
> Mask */
> > > +#define DB9000_IMR_VCTM                      BIT(6) /* Vertical Compare
> Triggered - Mask */
> > > +/* DMA Base Address Register Update to CAR - Mask */
> > > +#define DB9000_IMR_BAUM                      BIT(7)
> > > +#define DB9000_IMR_LDDM                      BIT(8) /* LCD Controller Disable
> Done - Mask */
> > > +
> > > +/* PWM Frequency Register */
> > > +#define DB9000_PWMFR_0                       0x034
> > > +#define DB9000_PWMFR_RZN1            0x04C
> > > +/* PWM Duty Cycle Register */
> > > +#define DB9000_PWMDCR_0                      0x038
> > > +#define DB9000_PWMDCR_RZN1           0x050
> > > +/* PWM Frequency Registers, Offset 0x034 and 0x04c */
> > > +#define DB9000_PWMFR_PWMFCD(x)               (((x) & 0x3fffff) << 0)
> > > +#define DB9000_PWMFR_PWMFCE          BIT(22)
> > > +
> > > +struct fps_info {
> > > +     unsigned int counter;
> > > +     ktime_t last_timestamp;
> > > +};
> > > +
> > > +struct db9000_device {
> > > +     struct drm_device drm;
> > > +     struct pwm_chip pwm;
> > > +     struct drm_connector *connector;
> > > +     void __iomem *regs;
> > > +     spinlock_t lock;
> > > +     struct clk *lcd_eclk;
> > > +     struct fps_info plane_fpsi[DB9000_MAX_LAYER];
> > > +     struct drm_atomic_state *suspend_state;
> > > +     u8 bpp;
> > > +     int bus_width;
> > > +     size_t frame_size;
> > > +     u32 addr_pwmfr;
> > > +     u32 addr_pwmdcr;
> > > +};
> > > +
> > > +#endif /* __DB9000_DU_H__ */
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Gareth Williams

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3]
@ 2022-01-03  9:21 Antoniu Miclaus
  0 siblings, 0 replies; 20+ messages in thread
From: Antoniu Miclaus @ 2022-01-03  9:21 UTC (permalink / raw)
  To: jic23, robh+dt, linux-iio, devicetree, linux-kernel; +Cc: Antoniu Miclaus

The ADMV1014 is a silicon germanium (SiGe), wideband,
microwave downconverter optimized for point to point microwave
radio designs operating in the 24 GHz to 44 GHz frequency range.

Datasheet:
https://www.analog.com/media/en/technical-documentation/data-sheets/ADMV1014.pdf

NOTE:
Currently depends on 64-bit architecture since the input
clock that server as Local Oscillator should support values
in the range 24 GHz to 44 GHz.

We might need some scaling implementation in the clock
framework so that u64 types are supported when using 32-bit
architectures.

Antoniu Miclaus (3):
  iio:frequency:admv1014: add support for ADMV1014
  dt-bindings:iio:frequency: add admv1014 doc
  Documentation:ABI:testing:admv1014: add ABI docs

 .../testing/sysfs-bus-iio-frequency-admv1014  |  23 +
 .../bindings/iio/frequency/adi,admv1014.yaml  |  97 +++
 drivers/iio/frequency/Kconfig                 |  10 +
 drivers/iio/frequency/Makefile                |   1 +
 drivers/iio/frequency/admv1014.c              | 784 ++++++++++++++++++
 5 files changed, 915 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-frequency-admv1014
 create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admv1014.yaml
 create mode 100644 drivers/iio/frequency/admv1014.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3]
@ 2020-06-05 18:46 Matthias Kaehlcke
  0 siblings, 0 replies; 20+ messages in thread
From: Matthias Kaehlcke @ 2020-06-05 18:46 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, Rocky Liao, Zijun Hu, linux-kernel,
	Balakrishna Godavarthi, Abhishek Pandit-Subedi, Claire Chang,
	Matthias Kaehlcke

This series includes a fix for a possible race in qca_suspend() and
some minor refactoring of the same function.


Matthias Kaehlcke (3):
  Bluetooth: hci_qca: Only remove TX clock vote after TX is completed
  Bluetooth: hci_qca: Skip serdev wait when no transfer is pending
  Bluetooth: hci_qca: Refactor error handling in qca_suspend()

 drivers/bluetooth/hci_qca.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

-- 
2.27.0.278.ge193c7cf3a9-goog


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3]
@ 2013-09-21 13:05 Fan Rong
  0 siblings, 0 replies; 20+ messages in thread
From: Fan Rong @ 2013-09-21 13:05 UTC (permalink / raw)
  To: coosty, maxime.ripard, daniel.lezcano, linux, tglx,
	linux-arm-kernel, linux-kernel, mark.rutland, pawel.moll,
	rob.herring, linux-sunxi
  Cc: Fan Rong


Fan Rong (3):
  Add smp support for Allwinner A20(sunxi 7i).
  Add cpuconfig nodes in dts for smp configure.
  Add arch count timer node in dts for Allwinner A20(sunxi 7i).

 arch/arm/boot/dts/sun7i-a20.dtsi |  19 ++-
 arch/arm/mach-sunxi/Makefile     |   2 +
 arch/arm/mach-sunxi/headsmp.S    |  12 ++
 arch/arm/mach-sunxi/platform.h   | 347 +++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-sunxi/platsmp.c    | 100 +++++++++++
 arch/arm/mach-sunxi/sunxi.c      |  34 +++-
 6 files changed, 511 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/mach-sunxi/headsmp.S
 create mode 100644 arch/arm/mach-sunxi/platform.h
 create mode 100644 arch/arm/mach-sunxi/platsmp.c

-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3]
  2013-06-13  8:22   ` Daniel Vetter
@ 2013-06-14 16:23     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2013-06-14 16:23 UTC (permalink / raw)
  To: Jani Nikula, linux-kernel, intel-gfx, Andrew Morton, chris

On Thu, Jun 13, 2013 at 10:22:05AM +0200, Daniel Vetter wrote:
> On Thu, Jun 06, 2013 at 04:59:26PM +0300, Jani Nikula wrote:
> > 
> > With Greg's address fixed. Please drop the old one from any
> > replies. Sorry for the noise.
> 
> Oops, replied with the old one still there.
> 
> Greg, Andrew: Imo it's best to merge all three patches through the same
> tree, so:
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> on the i915 parts of it. If you want I can also slurp them in through the
> intel tree, including the new dmi match code.

Please feel free to take them through your tree, I don't need to take
them.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3]
  2013-06-06 13:59 ` Jani Nikula
@ 2013-06-13  8:22   ` Daniel Vetter
  2013-06-14 16:23     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Vetter @ 2013-06-13  8:22 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, intel-gfx, Andrew Morton, Greg Kroah-Hartman,
	chris, daniel

On Thu, Jun 06, 2013 at 04:59:26PM +0300, Jani Nikula wrote:
> 
> With Greg's address fixed. Please drop the old one from any
> replies. Sorry for the noise.

Oops, replied with the old one still there.

Greg, Andrew: Imo it's best to merge all three patches through the same
tree, so:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

on the i915 parts of it. If you want I can also slurp them in through the
intel tree, including the new dmi match code.

Thanks, Daniel

> 
> On Thu, 06 Jun 2013, Jani Nikula <jani.nikula@intel.com> wrote:
> > Hi Greg, Andrew -
> >
> > Patch 1 is for DMI, bugfixes in patches 2-3 for i915 and included for
> > completeness. After a tested-by they should be good for stable. I'll
> > leave it to Daniel to sort out how the last two get in.
> >
> > BR,
> > Jani.
> >
> > Chris Wilson (1):
> >   drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard
> >
> > Jani Nikula (2):
> >   dmi: add support for exact DMI matches in addition to substring
> >     matching
> >   drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard
> >
> >  drivers/firmware/dmi_scan.c       |   12 +++++++++---
> >  drivers/gpu/drm/i915/intel_lvds.c |   16 ++++++++++++++++
> >  include/linux/mod_devicetable.h   |    6 ++++--
> >  3 files changed, 29 insertions(+), 5 deletions(-)
> >
> > -- 
> > 1.7.9.5
> >
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3]
  2013-06-06 13:53 Jani Nikula
  2013-06-06 13:59 ` Jani Nikula
@ 2013-06-06 14:33 ` Daniel Vetter
  1 sibling, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2013-06-06 14:33 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, intel-gfx, Andrew Morton, Greg Kroah-Hartman,
	chris, daniel

On Thu, Jun 06, 2013 at 04:53:01PM +0300, Jani Nikula wrote:
> Hi Greg, Andrew -
> 
> Patch 1 is for DMI, bugfixes in patches 2-3 for i915 and included for
> completeness. After a tested-by they should be good for stable. I'll
> leave it to Daniel to sort out how the last two get in.

I'd prefer all to go through the same tree (to avoid tracking them), and
conflicts around lvds quirks will be trivial at most. So no problem for me
if this doesn't go in through drm-next. So

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

on the i915 patches for merging through whatever tree the drm stuff goes
through.
-Daniel

> 
> BR,
> Jani.
> 
> Chris Wilson (1):
>   drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard
> 
> Jani Nikula (2):
>   dmi: add support for exact DMI matches in addition to substring
>     matching
>   drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard
> 
>  drivers/firmware/dmi_scan.c       |   12 +++++++++---
>  drivers/gpu/drm/i915/intel_lvds.c |   16 ++++++++++++++++
>  include/linux/mod_devicetable.h   |    6 ++++--
>  3 files changed, 29 insertions(+), 5 deletions(-)
> 
> -- 
> 1.7.9.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3]
  2013-06-06 13:53 Jani Nikula
@ 2013-06-06 13:59 ` Jani Nikula
  2013-06-13  8:22   ` Daniel Vetter
  2013-06-06 14:33 ` Daniel Vetter
  1 sibling, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2013-06-06 13:59 UTC (permalink / raw)
  To: linux-kernel, intel-gfx, Andrew Morton, Greg Kroah-Hartman; +Cc: chris, daniel


With Greg's address fixed. Please drop the old one from any
replies. Sorry for the noise.

On Thu, 06 Jun 2013, Jani Nikula <jani.nikula@intel.com> wrote:
> Hi Greg, Andrew -
>
> Patch 1 is for DMI, bugfixes in patches 2-3 for i915 and included for
> completeness. After a tested-by they should be good for stable. I'll
> leave it to Daniel to sort out how the last two get in.
>
> BR,
> Jani.
>
> Chris Wilson (1):
>   drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard
>
> Jani Nikula (2):
>   dmi: add support for exact DMI matches in addition to substring
>     matching
>   drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard
>
>  drivers/firmware/dmi_scan.c       |   12 +++++++++---
>  drivers/gpu/drm/i915/intel_lvds.c |   16 ++++++++++++++++
>  include/linux/mod_devicetable.h   |    6 ++++--
>  3 files changed, 29 insertions(+), 5 deletions(-)
>
> -- 
> 1.7.9.5
>

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3]
@ 2013-06-06 13:53 Jani Nikula
  2013-06-06 13:59 ` Jani Nikula
  2013-06-06 14:33 ` Daniel Vetter
  0 siblings, 2 replies; 20+ messages in thread
From: Jani Nikula @ 2013-06-06 13:53 UTC (permalink / raw)
  To: linux-kernel, intel-gfx, Andrew Morton, Greg Kroah-Hartman
  Cc: chris, daniel, jani.nikula

Hi Greg, Andrew -

Patch 1 is for DMI, bugfixes in patches 2-3 for i915 and included for
completeness. After a tested-by they should be good for stable. I'll
leave it to Daniel to sort out how the last two get in.

BR,
Jani.

Chris Wilson (1):
  drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard

Jani Nikula (2):
  dmi: add support for exact DMI matches in addition to substring
    matching
  drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard

 drivers/firmware/dmi_scan.c       |   12 +++++++++---
 drivers/gpu/drm/i915/intel_lvds.c |   16 ++++++++++++++++
 include/linux/mod_devicetable.h   |    6 ++++--
 3 files changed, 29 insertions(+), 5 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/3]
@ 2005-12-12  0:41 Petr Baudis
  0 siblings, 0 replies; 20+ messages in thread
From: Petr Baudis @ 2005-12-12  0:41 UTC (permalink / raw)
  To: zippel; +Cc: linux-kernel, sam, kbuild-devel

The following series implements...

-- 
And on the eigth day, God started debugging.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-01-03  9:22 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-27  8:21 [PATCH 0/3] Gareth Williams
2020-04-27  8:21 ` [PATCH 1/3] drm/db9000: Add Digital Blocks DB9000 LCD Controller Gareth Williams
2020-04-28 18:18   ` Sam Ravnborg
2020-04-28 18:23     ` Daniel Vetter
2020-05-13 10:47       ` Gareth Williams
2020-04-27  8:21 ` [PATCH 2/3] drm/db9000: Add bindings documentation for LCD controller Gareth Williams
2020-04-27 21:33   ` Rob Herring
2020-04-27  8:21 ` [PATCH 3/3] drm/panel: simple: Add Newhaven ATXL#-CTP panel Gareth Williams
2020-04-27  8:41   ` Sam Ravnborg
2020-04-27 11:03     ` Gareth Williams
2020-04-27 13:21 ` [PATCH 0/3] Gareth Williams
  -- strict thread matches above, loose matches on Subject: below --
2022-01-03  9:21 Antoniu Miclaus
2020-06-05 18:46 Matthias Kaehlcke
2013-09-21 13:05 Fan Rong
2013-06-06 13:53 Jani Nikula
2013-06-06 13:59 ` Jani Nikula
2013-06-13  8:22   ` Daniel Vetter
2013-06-14 16:23     ` Greg Kroah-Hartman
2013-06-06 14:33 ` Daniel Vetter
2005-12-12  0:41 Petr Baudis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).