* [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; 11+ 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: devicetree, Phil Edworthy, linux-kernel, dri-devel, Gareth Williams
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ 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; 11+ 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: Phil Edworthy, linux-kernel, dri-devel, Gareth Williams 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 11+ 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; 11+ messages in thread From: Sam Ravnborg @ 2020-04-28 18:18 UTC (permalink / raw) To: Gareth Williams Cc: Phil Edworthy, Maxime Ripard, linux-kernel, dri-devel, David Airlie, Sean Paul 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ 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; 11+ messages in thread From: Daniel Vetter @ 2020-04-28 18:23 UTC (permalink / raw) To: Sam Ravnborg Cc: Phil Edworthy, Maxime Ripard, Linux Kernel Mailing List, dri-devel, Gareth Williams, David Airlie, Sean Paul 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ 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; 11+ messages in thread From: Gareth Williams @ 2020-05-13 10:47 UTC (permalink / raw) To: Daniel Vetter, Sam Ravnborg Cc: Phil Edworthy, Maxime Ripard, Linux Kernel Mailing List, dri-devel, David Airlie, Sean Paul 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ 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; 11+ 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: devicetree, Phil Edworthy, linux-kernel, dri-devel, Gareth Williams 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 11+ 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; 11+ messages in thread From: Rob Herring @ 2020-04-27 21:33 UTC (permalink / raw) To: Gareth Williams Cc: Stephen Rothwell, Phil Edworthy, devicetree, David Airlie, linux-kernel, dri-devel, Gareth Williams, Vivek Unune 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. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ 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; 11+ messages in thread From: Gareth Williams @ 2020-04-27 8:21 UTC (permalink / raw) To: Thierry Reding, Sam Ravnborg, David Airlie, Daniel Vetter Cc: Phil Edworthy, linux-kernel, dri-devel, Gareth Williams 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 11+ 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; 11+ messages in thread From: Sam Ravnborg @ 2020-04-27 8:41 UTC (permalink / raw) To: Gareth Williams Cc: Phil Edworthy, David Airlie, linux-kernel, dri-devel, Thierry Reding 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ 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; 11+ messages in thread From: Gareth Williams @ 2020-04-27 11:03 UTC (permalink / raw) To: Sam Ravnborg Cc: Phil Edworthy, David Airlie, linux-kernel, dri-devel, Thierry Reding 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ 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; 11+ 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: devicetree, Phil Edworthy, Sam Ravnborg, linux-kernel, dri-devel 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-15 6:53 UTC | newest] Thread overview: 11+ 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
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).