Hi Sam/Daniel, This is all very useful feedback, thank you.
On Tue, Apr 28, 2020 at 19:24 PM Daniel Vetter <dan...@ffwll.ch> wrote: > > On Tue, Apr 28, 2020 at 8:18 PM Sam Ravnborg <s...@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...@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...@renesas.com> > > > + * > > > + * Based on ltdc.c > > > + * Copyright (C) STMicroelectronics SA 2017 > > > + * > > > + * Authors: Philippe Cornu <philippe.co...@st.com> > > > + * Yannick Fertre <yannick.fer...@st.com> > > > + * Fabien Dessenne <fabien.desse...@st.com> > > > + * Mickael Reulier <mickael.reul...@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...@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...@renesas.com> > > > + * > > > + * Based on ltdc.h > > > + * Copyright (C) STMicroelectronics SA 2017 > > > + * > > > + * Authors: Philippe Cornu <philippe.co...@st.com> > > > + * Yannick Fertre <yannick.fer...@st.com> > > > + * Fabien Dessenne <fabien.desse...@st.com> > > > + * Mickael Reulier <mickael.reul...@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-de...@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