* [PATCH 0/4 v3] drm: Add support of ARC PGU display controller
@ 2016-03-11 15:42 Alexey Brodkin
2016-03-11 15:42 ` [PATCH 1/4 " Alexey Brodkin
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Alexey Brodkin @ 2016-03-11 15:42 UTC (permalink / raw)
To: linux-kernel
Cc: linux-snps-arc, Alexey Brodkin, David Airlie, devicetree,
dri-devel, Ian Campbell, Kumar Gala, Mark Rutland, Pawel Moll,
Rob Herring, Vineet Gupta
This series add support of ARC PGU display controller.
ARC PGU is a quite simple byte streamer that gets data from the framebuffer
and pushes it to hte connected encoder (DP or HDMI).
It was tested on ARC SDP boards (axs101 in particular).
Changes v2 -> v3:
* Improved failure path if arcpgu_connector wasn't allocated.
* Fixed driver building as module.
* Implemented uncached mapping of user-space FB pages.
* Again updated DT bindings docs.
Changes v1 -> v2:
* Clean-up of DT bindings documentation.
* Added missing "pxlclk" clock in axs10x_mb.dtsi.
Cc: David Airlie <airlied@linux.ie>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: linux-snps-arc@lists.infradead.org
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Vineet Gupta <vgupta@synopsys.com>
Alexey Brodkin (4):
drm: Add support of ARC PGU display controller
drm: Add DT bindings documentation for ARC PGU display controller
arc: axs10x - add support of ARC PGU
MAINTAINERS: Add maintainer for ARC PGU display controller
.../devicetree/bindings/display/snps,arcpgu.txt | 72 ++++++
MAINTAINERS | 6 +
arch/arc/boot/dts/axs10x_mb.dtsi | 61 +++++
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/arc/Kconfig | 10 +
drivers/gpu/drm/arc/Makefile | 2 +
drivers/gpu/drm/arc/arcpgu.h | 50 ++++
drivers/gpu/drm/arc/arcpgu_crtc.c | 274 +++++++++++++++++++++
drivers/gpu/drm/arc/arcpgu_drv.c | 252 +++++++++++++++++++
drivers/gpu/drm/arc/arcpgu_fbdev.c | 245 ++++++++++++++++++
drivers/gpu/drm/arc/arcpgu_hdmi.c | 207 ++++++++++++++++
drivers/gpu/drm/arc/arcpgu_regs.h | 36 +++
13 files changed, 1218 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/snps,arcpgu.txt
create mode 100644 drivers/gpu/drm/arc/Kconfig
create mode 100644 drivers/gpu/drm/arc/Makefile
create mode 100644 drivers/gpu/drm/arc/arcpgu.h
create mode 100644 drivers/gpu/drm/arc/arcpgu_crtc.c
create mode 100644 drivers/gpu/drm/arc/arcpgu_drv.c
create mode 100644 drivers/gpu/drm/arc/arcpgu_fbdev.c
create mode 100644 drivers/gpu/drm/arc/arcpgu_hdmi.c
create mode 100644 drivers/gpu/drm/arc/arcpgu_regs.h
--
2.5.0
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-11 15:42 [PATCH 0/4 v3] drm: Add support of ARC PGU display controller Alexey Brodkin @ 2016-03-11 15:42 ` Alexey Brodkin 2016-03-11 16:45 ` kbuild test robot ` (2 more replies) 2016-03-11 15:42 ` [PATCH 2/4 v3] drm: Add DT bindings documentation for " Alexey Brodkin ` (2 subsequent siblings) 3 siblings, 3 replies; 18+ messages in thread From: Alexey Brodkin @ 2016-03-11 15:42 UTC (permalink / raw) To: linux-kernel Cc: linux-snps-arc, Alexey Brodkin, David Airlie, dri-devel, Jose Abreu ARC PGU could be found on some development boards from Synopsys. This is a simple byte streamer that reads data from a framebuffer and sends data to the single encoder. Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org Cc: linux-snps-arc@lists.infradead.org Cc: Jose Abreu <joabreu@synopsys.com> --- Changes v2 -> v3: * Improved failure path if arcpgu_connector wasn't allocated (thanks Jose). * Fixed driver building as module (reported by 0-DAY kernel test infrastruct.) * Implemented uncached mapping of user-space FB pages. No changes v1 -> v2. drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/arc/Kconfig | 10 ++ drivers/gpu/drm/arc/Makefile | 2 + drivers/gpu/drm/arc/arcpgu.h | 50 +++++++ drivers/gpu/drm/arc/arcpgu_crtc.c | 274 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/arc/arcpgu_drv.c | 252 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/arc/arcpgu_fbdev.c | 245 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/arc/arcpgu_hdmi.c | 207 ++++++++++++++++++++++++++++ drivers/gpu/drm/arc/arcpgu_regs.h | 36 +++++ 10 files changed, 1079 insertions(+) create mode 100644 drivers/gpu/drm/arc/Kconfig create mode 100644 drivers/gpu/drm/arc/Makefile create mode 100644 drivers/gpu/drm/arc/arcpgu.h create mode 100644 drivers/gpu/drm/arc/arcpgu_crtc.c create mode 100644 drivers/gpu/drm/arc/arcpgu_drv.c create mode 100644 drivers/gpu/drm/arc/arcpgu_fbdev.c create mode 100644 drivers/gpu/drm/arc/arcpgu_hdmi.c create mode 100644 drivers/gpu/drm/arc/arcpgu_regs.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f2a74d0..9e4f2f1 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -281,3 +281,5 @@ source "drivers/gpu/drm/imx/Kconfig" source "drivers/gpu/drm/vc4/Kconfig" source "drivers/gpu/drm/etnaviv/Kconfig" + +source "drivers/gpu/drm/arc/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 6eb94fc..c338d04 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -78,3 +78,4 @@ obj-y += panel/ obj-y += bridge/ obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/ obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/ +obj-$(CONFIG_DRM_ARCPGU)+= arc/ diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig new file mode 100644 index 0000000..f9a13b6 --- /dev/null +++ b/drivers/gpu/drm/arc/Kconfig @@ -0,0 +1,10 @@ +config DRM_ARCPGU + tristate "ARC PGU" + depends on DRM && OF + select DRM_KMS_CMA_HELPER + select DRM_KMS_FB_HELPER + select DRM_KMS_HELPER + help + Choose this option if you have an ARC PGU controller. + + If M is selected the module will be called arcpgu. diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile new file mode 100644 index 0000000..736ee6f --- /dev/null +++ b/drivers/gpu/drm/arc/Makefile @@ -0,0 +1,2 @@ +arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_fbdev.o arcpgu_drv.o +obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h new file mode 100644 index 0000000..86574b6 --- /dev/null +++ b/drivers/gpu/drm/arc/arcpgu.h @@ -0,0 +1,50 @@ +/* + * ARC PGU DRM driver. + * + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#ifndef _ARCPGU_H_ +#define _ARCPGU_H_ + +struct arcpgu_drm_private { + void __iomem *regs; + struct clk *clk; + struct drm_fbdev_cma *fbdev; + struct drm_framebuffer *fb; + struct list_head event_list; + struct drm_crtc crtc; + struct drm_plane *plane; +}; + +#define crtc_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, crtc) + +static inline void arc_pgu_write(struct arcpgu_drm_private *arcpgu, + unsigned int reg, u32 value) +{ + iowrite32(value, arcpgu->regs + reg); +} + +static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu, + unsigned int reg) +{ + return ioread32(arcpgu->regs + reg); +} + +int arc_pgu_setup_crtc(struct drm_device *dev); +int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np); +struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev, + unsigned int preferred_bpp, unsigned int num_crtc, + unsigned int max_conn_count); + +#endif diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c new file mode 100644 index 0000000..0fe5c8a --- /dev/null +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -0,0 +1,274 @@ +/* + * ARC PGU DRM driver. + * + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_plane_helper.h> +#include <linux/clk.h> +#include <linux/platform_data/simplefb.h> + +#include "arcpgu.h" +#include "arcpgu_regs.h" + +#define ENCODE_PGU_XY(x, y) ((((x) - 1) << 16) | ((y) - 1)) + +static struct simplefb_format supported_formats[] = { + { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }, + { "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, +}; + +static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) +{ + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); + uint32_t pixel_format = crtc->primary->state->fb->pixel_format; + struct simplefb_format *format = NULL; + int i; + + for (i = 0; i < ARRAY_SIZE(supported_formats); i++) { + if (supported_formats[i].fourcc == pixel_format) + format = &supported_formats[i]; + } + + if (WARN_ON(!format)) + return; + + if (format->fourcc == DRM_FORMAT_RGB888) + arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, + arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) | + ARCPGU_MODE_RGB888_MASK); + +} + +static const struct drm_crtc_funcs arc_pgu_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, +}; + +static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc) +{ + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); + struct drm_display_mode *m = &crtc->state->adjusted_mode; + + arc_pgu_write(arcpgu, ARCPGU_REG_FMT, + ENCODE_PGU_XY(m->crtc_htotal, m->crtc_vtotal)); + + arc_pgu_write(arcpgu, ARCPGU_REG_HSYNC, + ENCODE_PGU_XY(m->crtc_hsync_start - m->crtc_hdisplay, + m->crtc_hsync_end - m->crtc_hdisplay)); + + arc_pgu_write(arcpgu, ARCPGU_REG_VSYNC, + ENCODE_PGU_XY(m->crtc_vsync_start - m->crtc_vdisplay, + m->crtc_vsync_end - m->crtc_vdisplay)); + + arc_pgu_write(arcpgu, ARCPGU_REG_ACTIVE, + ENCODE_PGU_XY(m->crtc_hblank_end - m->crtc_hblank_start, + m->crtc_vblank_end - m->crtc_vblank_start)); + + arc_pgu_write(arcpgu, ARCPGU_REG_STRIDE, 0); + arc_pgu_write(arcpgu, ARCPGU_REG_START_SET, 1); + + arc_pgu_set_pxl_fmt(crtc); + + clk_set_rate(arcpgu->clk, m->crtc_clock * 1000); +} + +static void arc_pgu_crtc_enable(struct drm_crtc *crtc) +{ + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); + + clk_prepare_enable(arcpgu->clk); + arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, + arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) | + ARCPGU_CTRL_ENABLE_MASK); +} + +static void arc_pgu_crtc_disable(struct drm_crtc *crtc) +{ + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); + + if (!crtc->primary->fb) + return; + + clk_disable_unprepare(arcpgu->clk); + arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, + arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) & + ~ARCPGU_CTRL_ENABLE_MASK); +} + +static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc, + struct drm_crtc_state *state) +{ + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); + struct drm_display_mode *mode = &state->adjusted_mode; + long rate, clk_rate = mode->clock * 1000; + + rate = clk_round_rate(arcpgu->clk, clk_rate); + if (rate != clk_rate) + return -EINVAL; + + return 0; +} + +static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc, + struct drm_crtc_state *state) +{ + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); + unsigned long flags; + + if (crtc->state->event) { + struct drm_pending_vblank_event *event = crtc->state->event; + + crtc->state->event = NULL; + event->pipe = drm_crtc_index(crtc); + + WARN_ON(drm_crtc_vblank_get(crtc) != 0); + + spin_lock_irqsave(&crtc->dev->event_lock, flags); + list_add_tail(&event->base.link, &arcpgu->event_list); + spin_unlock_irqrestore(&crtc->dev->event_lock, flags); + } +} + +static void arc_pgu_crtc_atomic_flush(struct drm_crtc *crtc, + struct drm_crtc_state *state) +{ +} + +static bool arc_pgu_crtc_mode_fixup(struct drm_crtc *crtc, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + return true; +} + +static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = { + .mode_fixup = arc_pgu_crtc_mode_fixup, + .mode_set = drm_helper_crtc_mode_set, + .mode_set_base = drm_helper_crtc_mode_set_base, + .mode_set_nofb = arc_pgu_crtc_mode_set_nofb, + .enable = arc_pgu_crtc_enable, + .disable = arc_pgu_crtc_disable, + .prepare = arc_pgu_crtc_disable, + .commit = arc_pgu_crtc_enable, + .atomic_check = arc_pgu_crtc_atomic_check, + .atomic_begin = arc_pgu_crtc_atomic_begin, + .atomic_flush = arc_pgu_crtc_atomic_flush, +}; + +static int arc_pgu_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state) +{ + return 0; +} + +static void arc_pgu_plane_atomic_update(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct arcpgu_drm_private *arcpgu; + struct drm_gem_cma_object *gem; + + if (!plane->state->crtc || !plane->state->fb) + return; + + arcpgu = crtc_to_arcpgu_priv(plane->state->crtc); + gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0); + arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr); +} + +static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = { + .prepare_fb = NULL, + .cleanup_fb = NULL, + .atomic_check = arc_pgu_plane_atomic_check, + .atomic_update = arc_pgu_plane_atomic_update, +}; + +static void arc_pgu_plane_destroy(struct drm_plane *plane) +{ + drm_plane_helper_disable(plane); + drm_plane_cleanup(plane); +} + +static const struct drm_plane_funcs arc_pgu_plane_funcs = { + .update_plane = drm_atomic_helper_update_plane, + .disable_plane = drm_atomic_helper_disable_plane, + .destroy = arc_pgu_plane_destroy, + .reset = drm_atomic_helper_plane_reset, + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, +}; + +static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm) +{ + struct arcpgu_drm_private *arcpgu = drm->dev_private; + struct drm_plane *plane = NULL; + u32 formats[ARRAY_SIZE(supported_formats)], i; + int ret; + + plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL); + if (!plane) + return ERR_PTR(-ENOMEM); + + for (i = 0; i < ARRAY_SIZE(supported_formats); i++) + formats[i] = supported_formats[i].fourcc; + + ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs, + formats, ARRAY_SIZE(formats), + DRM_PLANE_TYPE_PRIMARY, NULL); + if (ret) + return ERR_PTR(ret); + + drm_plane_helper_add(plane, &arc_pgu_plane_helper_funcs); + arcpgu->plane = plane; + + return plane; +} + +void arc_pgu_crtc_suspend(struct drm_crtc *crtc) +{ + arc_pgu_crtc_disable(crtc); +} + +void arc_pgu_crtc_resume(struct drm_crtc *crtc) +{ + arc_pgu_crtc_enable(crtc); +} + +int arc_pgu_setup_crtc(struct drm_device *drm) +{ + struct arcpgu_drm_private *arcpgu = drm->dev_private; + struct drm_plane *primary; + int ret; + + primary = arc_pgu_plane_init(drm); + if (IS_ERR(primary)) + return PTR_ERR(primary); + + ret = drm_crtc_init_with_planes(drm, &arcpgu->crtc, primary, NULL, + &arc_pgu_crtc_funcs, NULL); + if (ret) { + arc_pgu_plane_destroy(primary); + return ret; + } + + drm_crtc_helper_add(&arcpgu->crtc, &arc_pgu_crtc_helper_funcs); + return 0; +} diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c new file mode 100644 index 0000000..d47481d --- /dev/null +++ b/drivers/gpu/drm/arc/arcpgu_drv.c @@ -0,0 +1,252 @@ +/* + * ARC PGU DRM driver. + * + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/clk.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_gem_cma_helper.h> +#include <drm/drm_atomic_helper.h> + +#include "arcpgu.h" +#include "arcpgu_regs.h" + +static void arcpgu_fb_output_poll_changed(struct drm_device *dev) +{ + struct arcpgu_drm_private *arcpgu = dev->dev_private; + + if (arcpgu->fbdev) + drm_fbdev_cma_hotplug_event(arcpgu->fbdev); +} + +static int arcpgu_atomic_commit(struct drm_device *dev, + struct drm_atomic_state *state, bool async) +{ + return drm_atomic_helper_commit(dev, state, false); +} + +static struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = { + .fb_create = drm_fb_cma_create, + .output_poll_changed = arcpgu_fb_output_poll_changed, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = arcpgu_atomic_commit, +}; + +static void arcpgu_setup_mode_config(struct drm_device *drm) +{ + drm_mode_config_init(drm); + drm->mode_config.min_width = 0; + drm->mode_config.min_height = 0; + drm->mode_config.max_width = 1920; + drm->mode_config.max_height = 1080; + drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs; +} + +int arcpgu_gem_mmap(struct file *filp, struct vm_area_struct *vma) +{ + int ret; + + ret = drm_gem_mmap(filp, vma); + if (ret) + return ret; + + vma->vm_page_prot = pgprot_noncached(vm_get_page_prot(vma->vm_flags)); + return 0; +} + +static const struct file_operations arcpgu_drm_ops = { + .owner = THIS_MODULE, + .open = drm_open, + .release = drm_release, + .unlocked_ioctl = drm_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = drm_compat_ioctl, +#endif + .poll = drm_poll, + .read = drm_read, + .llseek = no_llseek, + .mmap = arcpgu_gem_mmap, +}; + +static void arcpgu_preclose(struct drm_device *drm, struct drm_file *file) +{ + struct arcpgu_drm_private *arcpgu = drm->dev_private; + struct drm_pending_vblank_event *e, *t; + unsigned long flags; + + spin_lock_irqsave(&drm->event_lock, flags); + list_for_each_entry_safe(e, t, &arcpgu->event_list, base.link) { + if (e->base.file_priv != file) + continue; + list_del(&e->base.link); + e->base.destroy(&e->base); + } + spin_unlock_irqrestore(&drm->event_lock, flags); +} + +static void arcpgu_lastclose(struct drm_device *drm) +{ + struct arcpgu_drm_private *arcpgu = drm->dev_private; + + drm_fbdev_cma_restore_mode(arcpgu->fbdev); +} + +static int arcpgu_load(struct drm_device *drm, unsigned long flags) +{ + struct platform_device *pdev = drm->platformdev; + struct arcpgu_drm_private *arcpgu; + struct device_node *encoder_node; + struct resource *res; + int ret; + + arcpgu = devm_kzalloc(&pdev->dev, sizeof(*arcpgu), GFP_KERNEL); + if (arcpgu == NULL) + return -ENOMEM; + + drm->dev_private = arcpgu; + + arcpgu->clk = devm_clk_get(drm->dev, "pxlclk"); + if (IS_ERR(arcpgu->clk)) + return PTR_ERR(arcpgu->clk); + + INIT_LIST_HEAD(&arcpgu->event_list); + + arcpgu_setup_mode_config(drm); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + arcpgu->regs = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(arcpgu->regs)) { + dev_err(drm->dev, "Could not remap IO mem\n"); + return PTR_ERR(arcpgu->regs); + } + + dev_info(drm->dev, "arc_pgu ID: 0x%x\n", + arc_pgu_read(arcpgu, ARCPGU_REG_ID)); + + if (dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32))) + return -ENODEV; + + if (arc_pgu_setup_crtc(drm) < 0) + return -ENODEV; + + /* find the encoder node and initialize it */ + encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0); + if (!encoder_node) { + dev_err(drm->dev, "failed to get an encoder slave node\n"); + return -ENODEV; + } + + ret = arcpgu_drm_hdmi_init(drm, encoder_node); + if (ret < 0) + return ret; + + drm_mode_config_reset(drm); + drm_kms_helper_poll_init(drm); + + arcpgu->fbdev = arcpgu_fbdev_cma_init(drm, 16, + drm->mode_config.num_crtc, + drm->mode_config.num_connector); + if (IS_ERR(arcpgu->fbdev)) { + ret = PTR_ERR(arcpgu->fbdev); + arcpgu->fbdev = NULL; + return -ENODEV; + } + + platform_set_drvdata(pdev, arcpgu); + return 0; +} + +int arcpgu_unload(struct drm_device *drm) +{ + struct arcpgu_drm_private *arcpgu = drm->dev_private; + + if (arcpgu->fbdev) { + drm_fbdev_cma_fini(arcpgu->fbdev); + arcpgu->fbdev = NULL; + } + drm_kms_helper_poll_fini(drm); + drm_vblank_cleanup(drm); + drm_mode_config_cleanup(drm); + + return 0; +} + +static struct drm_driver arcpgu_drm_driver = { + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | + DRIVER_ATOMIC, + .preclose = arcpgu_preclose, + .lastclose = arcpgu_lastclose, + .name = "drm-arcpgu", + .desc = "ARC PGU Controller", + .date = "20160219", + .major = 1, + .minor = 0, + .patchlevel = 0, + .fops = &arcpgu_drm_ops, + .load = arcpgu_load, + .unload = arcpgu_unload, + .dumb_create = drm_gem_cma_dumb_create, + .dumb_map_offset = drm_gem_cma_dumb_map_offset, + .dumb_destroy = drm_gem_dumb_destroy, + .get_vblank_counter = drm_vblank_no_hw_counter, + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .gem_free_object = 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 arcpgu_probe(struct platform_device *pdev) +{ + return drm_platform_init(&arcpgu_drm_driver, pdev); +} + +static int arcpgu_remove(struct platform_device *pdev) +{ + struct drm_device *drm = dev_get_drvdata(&pdev->dev); + + drm_put_dev(drm); + + return 0; +} + +static const struct of_device_id arcpgu_of_table[] = { + {.compatible = "snps,arcpgu"}, + {} +}; + +MODULE_DEVICE_TABLE(of, arcpgu_of_table); + +static struct platform_driver arcpgu_platform_driver = { + .probe = arcpgu_probe, + .remove = arcpgu_remove, + .driver = { + .name = "arcpgu", + .owner = THIS_MODULE, + .of_match_table = arcpgu_of_table, + }, +}; + +module_platform_driver(arcpgu_platform_driver); + +MODULE_AUTHOR("Carlos Palminha <palminha@synopsys.com"); +MODULE_DESCRIPTION("ARC PGU DRM driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/arc/arcpgu_fbdev.c b/drivers/gpu/drm/arc/arcpgu_fbdev.c new file mode 100644 index 0000000..6f67706 --- /dev/null +++ b/drivers/gpu/drm/arc/arcpgu_fbdev.c @@ -0,0 +1,245 @@ +/* + * ARC PGU DRM driver. + * + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_cma_helper.h> +#include <drm/drm_fb_helper.h> +#include <drm/drm_gem_cma_helper.h> + +struct drm_fb_cma { + struct drm_framebuffer fb; + struct drm_gem_cma_object *obj[4]; +}; + +struct drm_fbdev_cma { + struct drm_fb_helper fb_helper; + struct drm_fb_cma *fb; +}; + +static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper) +{ + return container_of(helper, struct drm_fbdev_cma, fb_helper); +} + +static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb) +{ + return container_of(fb, struct drm_fb_cma, fb); +} + +static void drm_fb_cma_destroy(struct drm_framebuffer *fb) +{ + struct drm_fb_cma *fb_cma = to_fb_cma(fb); + int i; + + for (i = 0; i < 4; i++) { + if (fb_cma->obj[i]) + drm_gem_object_unreference_unlocked(&fb_cma->obj[i]->base); + } + + drm_framebuffer_cleanup(fb); + kfree(fb_cma); +} + +static int drm_fb_cma_create_handle(struct drm_framebuffer *fb, + struct drm_file *file_priv, unsigned int *handle) +{ + struct drm_fb_cma *fb_cma = to_fb_cma(fb); + + return drm_gem_handle_create(file_priv, + &fb_cma->obj[0]->base, handle); +} + +static struct drm_framebuffer_funcs drm_fb_cma_funcs = { + .destroy = drm_fb_cma_destroy, + .create_handle = drm_fb_cma_create_handle, +}; + +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, + const struct drm_mode_fb_cmd2 *mode_cmd, + struct drm_gem_cma_object **obj, + unsigned int num_planes) +{ + struct drm_fb_cma *fb_cma; + int ret; + int i; + + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); + if (!fb_cma) + return ERR_PTR(-ENOMEM); + + drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); + + for (i = 0; i < num_planes; i++) + fb_cma->obj[i] = obj[i]; + + ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); + if (ret) { + dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret); + kfree(fb_cma); + return ERR_PTR(ret); + } + + return fb_cma; +} + +/* + * This function is the only reason to have a copy of drm_fbdev_cma_init() + * here in this driver. + * + * In its turn this mmap() is required to mark user-space page as non-cached + * because it is just a mirror or real hardware frame-buffer. + */ +static int arcpgu_mmap(struct fb_info *info, struct vm_area_struct *vma) +{ + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); + return vm_iomap_memory(vma, info->fix.smem_start, info->fix.smem_len); +} + +static struct fb_ops drm_fbdev_cma_ops = { + .owner = THIS_MODULE, + .fb_mmap = arcpgu_mmap, + .fb_fillrect = drm_fb_helper_sys_fillrect, + .fb_copyarea = drm_fb_helper_sys_copyarea, + .fb_imageblit = drm_fb_helper_sys_imageblit, + .fb_check_var = drm_fb_helper_check_var, + .fb_set_par = drm_fb_helper_set_par, + .fb_blank = drm_fb_helper_blank, + .fb_pan_display = drm_fb_helper_pan_display, + .fb_setcmap = drm_fb_helper_setcmap, +}; + +static int drm_fbdev_cma_create(struct drm_fb_helper *helper, + struct drm_fb_helper_surface_size *sizes) +{ + struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper); + struct drm_mode_fb_cmd2 mode_cmd = { 0 }; + struct drm_device *dev = helper->dev; + struct drm_gem_cma_object *obj; + struct drm_framebuffer *fb; + unsigned int bytes_per_pixel; + unsigned long offset; + struct fb_info *fbi; + size_t size; + int ret; + + DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n", + sizes->surface_width, sizes->surface_height, + sizes->surface_bpp); + + bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8); + + mode_cmd.width = sizes->surface_width; + mode_cmd.height = sizes->surface_height; + mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel; + mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, + sizes->surface_depth); + + size = mode_cmd.pitches[0] * mode_cmd.height; + obj = drm_gem_cma_create(dev, size); + if (IS_ERR(obj)) + return -ENOMEM; + + fbi = drm_fb_helper_alloc_fbi(helper); + if (IS_ERR(fbi)) { + ret = PTR_ERR(fbi); + goto err_gem_free_object; + } + + fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1); + if (IS_ERR(fbdev_cma->fb)) { + dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n"); + ret = PTR_ERR(fbdev_cma->fb); + goto err_fb_info_destroy; + } + + fb = &fbdev_cma->fb->fb; + helper->fb = fb; + + fbi->par = helper; + fbi->flags = FBINFO_FLAG_DEFAULT; + fbi->fbops = &drm_fbdev_cma_ops; + + drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->depth); + drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height); + + offset = fbi->var.xoffset * bytes_per_pixel; + offset += fbi->var.yoffset * fb->pitches[0]; + + dev->mode_config.fb_base = (resource_size_t)obj->paddr; + fbi->screen_base = obj->vaddr + offset; + fbi->fix.smem_start = (unsigned long)(obj->paddr + offset); + fbi->screen_size = size; + fbi->fix.smem_len = size; + + return 0; + +err_fb_info_destroy: + drm_fb_helper_release_fbi(helper); +err_gem_free_object: + dev->driver->gem_free_object(&obj->base); + return ret; +} + +static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { + .fb_probe = drm_fbdev_cma_create, +}; + +struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev, + unsigned int preferred_bpp, unsigned int num_crtc, + unsigned int max_conn_count) +{ + struct drm_fbdev_cma *fbdev_cma; + struct drm_fb_helper *helper; + int ret; + + fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL); + if (!fbdev_cma) { + dev_err(dev->dev, "Failed to allocate drm fbdev.\n"); + return ERR_PTR(-ENOMEM); + } + + helper = &fbdev_cma->fb_helper; + + drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); + + ret = drm_fb_helper_init(dev, helper, num_crtc, max_conn_count); + if (ret < 0) { + dev_err(dev->dev, "Failed to initialize drm fb helper.\n"); + goto err_free; + } + + ret = drm_fb_helper_single_add_all_connectors(helper); + if (ret < 0) { + dev_err(dev->dev, "Failed to add connectors.\n"); + goto err_drm_fb_helper_fini; + + } + + ret = drm_fb_helper_initial_config(helper, preferred_bpp); + if (ret < 0) { + dev_err(dev->dev, "Failed to set initial hw configuration.\n"); + goto err_drm_fb_helper_fini; + } + + return fbdev_cma; + +err_drm_fb_helper_fini: + drm_fb_helper_fini(helper); +err_free: + kfree(fbdev_cma); + + return ERR_PTR(ret); +} diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c new file mode 100644 index 0000000..7adafa3 --- /dev/null +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c @@ -0,0 +1,207 @@ +/* + * ARC PGU DRM driver. + * + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder_slave.h> +#include <drm/drm_atomic_helper.h> + +#include "arcpgu.h" + +struct arcpgu_drm_connector { + struct drm_connector connector; + struct drm_encoder_slave *encoder_slave; +}; + +static int arcpgu_drm_connector_get_modes(struct drm_connector *connector) +{ + const struct drm_encoder_slave_funcs *sfuncs; + struct drm_encoder_slave *slave; + struct arcpgu_drm_connector *con = + container_of(connector, struct arcpgu_drm_connector, connector); + + slave = con->encoder_slave; + if (slave == NULL) { + dev_err(connector->dev->dev, + "connector_get_modes: cannot find slave encoder for connector\n"); + return 0; + } + + sfuncs = slave->slave_funcs; + if (sfuncs->get_modes == NULL) + return 0; + + return sfuncs->get_modes(&slave->base, connector); +} + +struct drm_encoder * +arcpgu_drm_connector_best_encoder(struct drm_connector *connector) +{ + struct drm_encoder_slave *slave; + struct arcpgu_drm_connector *con = + container_of(connector, struct arcpgu_drm_connector, connector); + + slave = con->encoder_slave; + if (slave == NULL) { + dev_err(connector->dev->dev, + "connector_best_encoder: cannot find slave encoder for connector\n"); + return NULL; + } + + return &slave->base; +} + +static enum drm_connector_status +arcpgu_drm_connector_detect(struct drm_connector *connector, bool force) +{ + enum drm_connector_status status = connector_status_unknown; + const struct drm_encoder_slave_funcs *sfuncs; + struct drm_encoder_slave *slave; + + struct arcpgu_drm_connector *con = + container_of(connector, struct arcpgu_drm_connector, connector); + + slave = con->encoder_slave; + if (slave == NULL) { + dev_err(connector->dev->dev, + "connector_detect: cannot find slave encoder for connector\n"); + return status; + } + + sfuncs = slave->slave_funcs; + if (sfuncs && sfuncs->detect) + return sfuncs->detect(&slave->base, connector); + + dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); + return status; +} + +static void arcpgu_drm_connector_destroy(struct drm_connector *connector) +{ + drm_connector_unregister(connector); + drm_connector_cleanup(connector); +} + +static const struct drm_connector_helper_funcs +arcpgu_drm_connector_helper_funcs = { + .get_modes = arcpgu_drm_connector_get_modes, + .best_encoder = arcpgu_drm_connector_best_encoder, +}; + +static const struct drm_connector_funcs arcpgu_drm_connector_funcs = { + .dpms = drm_helper_connector_dpms, + .reset = drm_atomic_helper_connector_reset, + .detect = arcpgu_drm_connector_detect, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = arcpgu_drm_connector_destroy, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static struct drm_encoder_helper_funcs arcpgu_drm_encoder_helper_funcs = { + .dpms = drm_i2c_encoder_dpms, + .mode_fixup = drm_i2c_encoder_mode_fixup, + .mode_set = drm_i2c_encoder_mode_set, + .prepare = drm_i2c_encoder_prepare, + .commit = drm_i2c_encoder_commit, + .detect = drm_i2c_encoder_detect, +}; + +static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = { + .destroy = drm_encoder_cleanup, +}; + +int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np) +{ + struct arcpgu_drm_connector *arcpgu_connector; + struct drm_i2c_encoder_driver *driver; + struct drm_encoder_slave *encoder; + struct drm_connector *connector; + struct i2c_client *i2c_slave; + int ret; + + encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL); + if (encoder == NULL) + return -ENOMEM; + + i2c_slave = of_find_i2c_device_by_node(np); + if (!i2c_slave || !i2c_get_clientdata(i2c_slave)) { + dev_err(drm->dev, "failed to find i2c slave encoder\n"); + return -EPROBE_DEFER; + } + + if (i2c_slave->dev.driver == NULL) { + dev_err(drm->dev, "failed to find i2c slave driver\n"); + return -EPROBE_DEFER; + } + + driver = + to_drm_i2c_encoder_driver(to_i2c_driver(i2c_slave->dev.driver)); + ret = driver->encoder_init(i2c_slave, drm, encoder); + if (ret) { + dev_err(drm->dev, "failed to initialize i2c encoder slave\n"); + return ret; + } + + encoder->base.possible_crtcs = 1; + encoder->base.possible_clones = 0; + ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs, + DRM_MODE_ENCODER_TMDS, NULL); + if (ret) + return ret; + + drm_encoder_helper_add(&encoder->base, + &arcpgu_drm_encoder_helper_funcs); + + arcpgu_connector = devm_kzalloc(drm->dev, sizeof(*arcpgu_connector), + GFP_KERNEL); + if (!arcpgu_connector) { + ret = -ENOMEM; + goto error_encoder_cleanup; + } + + connector = &arcpgu_connector->connector; + drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs); + ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA); + if (ret < 0) { + dev_err(drm->dev, "failed to initialize drm connector\n"); + goto error_encoder_cleanup; + } + + ret = drm_connector_register(connector); + if (ret < 0) { + dev_err(drm->dev, "failed to regiter DRM connector and helper funcs\n"); + goto error_connector_cleanup; + } + + ret = drm_mode_connector_attach_encoder(connector, &encoder->base); + if (ret < 0) { + dev_err(drm->dev, "could not attach connector to encoder\n"); + drm_connector_unregister(connector); + goto error_connector_cleanup; + } + + arcpgu_connector->encoder_slave = encoder; + + return 0; + +error_connector_cleanup: + drm_connector_cleanup(connector); + +error_encoder_cleanup: + drm_encoder_cleanup(&encoder->base); + return ret; +} diff --git a/drivers/gpu/drm/arc/arcpgu_regs.h b/drivers/gpu/drm/arc/arcpgu_regs.h new file mode 100644 index 0000000..f04165f --- /dev/null +++ b/drivers/gpu/drm/arc/arcpgu_regs.h @@ -0,0 +1,36 @@ +/* + * ARC PGU DRM driver. + * + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#ifndef _ARC_PGU_REGS_H_ +#define _ARC_PGU_REGS_H_ + +#define ARCPGU_REG_CTRL 0x00 +#define ARCPGU_REG_STAT 0x04 +#define ARCPGU_REG_FMT 0x10 +#define ARCPGU_REG_HSYNC 0x14 +#define ARCPGU_REG_VSYNC 0x18 +#define ARCPGU_REG_ACTIVE 0x1c +#define ARCPGU_REG_BUF0_ADDR 0x40 +#define ARCPGU_REG_STRIDE 0x50 +#define ARCPGU_REG_START_SET 0x84 + +#define ARCPGU_REG_ID 0x3FC + +#define ARCPGU_CTRL_ENABLE_MASK 0x02 +#define ARCPGU_MODE_RGB888_MASK 0x04 +#define ARCPGU_STAT_BUSY_MASK 0x02 + +#endif -- 2.5.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-11 15:42 ` [PATCH 1/4 " Alexey Brodkin @ 2016-03-11 16:45 ` kbuild test robot 2016-03-11 16:45 ` [PATCH] drm: fix platform_no_drv_owner.cocci warnings kbuild test robot 2016-03-14 7:00 ` [PATCH 1/4 v3] drm: Add support of ARC PGU display controller Daniel Vetter 2 siblings, 0 replies; 18+ messages in thread From: kbuild test robot @ 2016-03-11 16:45 UTC (permalink / raw) To: Alexey Brodkin Cc: kbuild-all, linux-kernel, linux-snps-arc, Alexey Brodkin, David Airlie, dri-devel, Jose Abreu Hi Alexey, [auto build test WARNING on drm/drm-next] [also build test WARNING on v4.5-rc7 next-20160311] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Alexey-Brodkin/drm-Add-support-of-ARC-PGU-display-controller/20160311-234824 base: git://people.freedesktop.org/~airlied/linux.git drm-next coccinelle warnings: (new ones prefixed by >>) >> drivers/gpu/drm/arc/arcpgu_drv.c:243:6-11: No need to set .owner here. The core will do it. Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] drm: fix platform_no_drv_owner.cocci warnings 2016-03-11 15:42 ` [PATCH 1/4 " Alexey Brodkin 2016-03-11 16:45 ` kbuild test robot @ 2016-03-11 16:45 ` kbuild test robot 2016-03-14 7:00 ` [PATCH 1/4 v3] drm: Add support of ARC PGU display controller Daniel Vetter 2 siblings, 0 replies; 18+ messages in thread From: kbuild test robot @ 2016-03-11 16:45 UTC (permalink / raw) To: Alexey Brodkin Cc: kbuild-all, linux-kernel, linux-snps-arc, Alexey Brodkin, David Airlie, dri-devel, Jose Abreu drivers/gpu/drm/arc/arcpgu_drv.c:243:6-11: No need to set .owner here. The core will do it. Remove .owner field if calls are used which set it automatically Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci CC: Alexey Brodkin <Alexey.Brodkin@synopsys.com> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- arcpgu_drv.c | 1 - 1 file changed, 1 deletion(-) --- a/drivers/gpu/drm/arc/arcpgu_drv.c +++ b/drivers/gpu/drm/arc/arcpgu_drv.c @@ -240,7 +240,6 @@ static struct platform_driver arcpgu_pla .remove = arcpgu_remove, .driver = { .name = "arcpgu", - .owner = THIS_MODULE, .of_match_table = arcpgu_of_table, }, }; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-11 15:42 ` [PATCH 1/4 " Alexey Brodkin 2016-03-11 16:45 ` kbuild test robot 2016-03-11 16:45 ` [PATCH] drm: fix platform_no_drv_owner.cocci warnings kbuild test robot @ 2016-03-14 7:00 ` Daniel Vetter 2016-03-14 11:15 ` Alexey Brodkin 2 siblings, 1 reply; 18+ messages in thread From: Daniel Vetter @ 2016-03-14 7:00 UTC (permalink / raw) To: Alexey Brodkin; +Cc: linux-kernel, Jose Abreu, linux-snps-arc, dri-devel On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > ARC PGU could be found on some development boards from Synopsys. > This is a simple byte streamer that reads data from a framebuffer > and sends data to the single encoder. > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > Cc: David Airlie <airlied@linux.ie> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-snps-arc@lists.infradead.org > Cc: Jose Abreu <joabreu@synopsys.com> > --- > > Changes v2 -> v3: > * Improved failure path if arcpgu_connector wasn't allocated (thanks Jose). > * Fixed driver building as module (reported by 0-DAY kernel test infrastruct.) > * Implemented uncached mapping of user-space FB pages. > > No changes v1 -> v2. > Bunch of comments below to update your driver to latest styles and best practices. Cheers, Daniel > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/arc/Kconfig | 10 ++ > drivers/gpu/drm/arc/Makefile | 2 + > drivers/gpu/drm/arc/arcpgu.h | 50 +++++++ > drivers/gpu/drm/arc/arcpgu_crtc.c | 274 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/arc/arcpgu_drv.c | 252 ++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/arc/arcpgu_fbdev.c | 245 +++++++++++++++++++++++++++++++++ > drivers/gpu/drm/arc/arcpgu_hdmi.c | 207 ++++++++++++++++++++++++++++ > drivers/gpu/drm/arc/arcpgu_regs.h | 36 +++++ > 10 files changed, 1079 insertions(+) > create mode 100644 drivers/gpu/drm/arc/Kconfig > create mode 100644 drivers/gpu/drm/arc/Makefile > create mode 100644 drivers/gpu/drm/arc/arcpgu.h > create mode 100644 drivers/gpu/drm/arc/arcpgu_crtc.c > create mode 100644 drivers/gpu/drm/arc/arcpgu_drv.c > create mode 100644 drivers/gpu/drm/arc/arcpgu_fbdev.c > create mode 100644 drivers/gpu/drm/arc/arcpgu_hdmi.c > create mode 100644 drivers/gpu/drm/arc/arcpgu_regs.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index f2a74d0..9e4f2f1 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -281,3 +281,5 @@ source "drivers/gpu/drm/imx/Kconfig" > source "drivers/gpu/drm/vc4/Kconfig" > > source "drivers/gpu/drm/etnaviv/Kconfig" > + > +source "drivers/gpu/drm/arc/Kconfig" > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 6eb94fc..c338d04 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -78,3 +78,4 @@ obj-y += panel/ > obj-y += bridge/ > obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/ > obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/ > +obj-$(CONFIG_DRM_ARCPGU)+= arc/ > diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig > new file mode 100644 > index 0000000..f9a13b6 > --- /dev/null > +++ b/drivers/gpu/drm/arc/Kconfig > @@ -0,0 +1,10 @@ > +config DRM_ARCPGU > + tristate "ARC PGU" > + depends on DRM && OF > + select DRM_KMS_CMA_HELPER > + select DRM_KMS_FB_HELPER > + select DRM_KMS_HELPER > + help > + Choose this option if you have an ARC PGU controller. > + > + If M is selected the module will be called arcpgu. > diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile > new file mode 100644 > index 0000000..736ee6f > --- /dev/null > +++ b/drivers/gpu/drm/arc/Makefile > @@ -0,0 +1,2 @@ > +arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_fbdev.o arcpgu_drv.o > +obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o > diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h > new file mode 100644 > index 0000000..86574b6 > --- /dev/null > +++ b/drivers/gpu/drm/arc/arcpgu.h > @@ -0,0 +1,50 @@ > +/* > + * ARC PGU DRM driver. > + * > + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#ifndef _ARCPGU_H_ > +#define _ARCPGU_H_ > + > +struct arcpgu_drm_private { > + void __iomem *regs; > + struct clk *clk; > + struct drm_fbdev_cma *fbdev; > + struct drm_framebuffer *fb; > + struct list_head event_list; > + struct drm_crtc crtc; > + struct drm_plane *plane; > +}; > + > +#define crtc_to_arcpgu_priv(x) container_of(x, struct arcpgu_drm_private, crtc) > + > +static inline void arc_pgu_write(struct arcpgu_drm_private *arcpgu, > + unsigned int reg, u32 value) > +{ > + iowrite32(value, arcpgu->regs + reg); > +} > + > +static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu, > + unsigned int reg) > +{ > + return ioread32(arcpgu->regs + reg); > +} > + > +int arc_pgu_setup_crtc(struct drm_device *dev); > +int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np); > +struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev, > + unsigned int preferred_bpp, unsigned int num_crtc, > + unsigned int max_conn_count); > + > +#endif > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c > new file mode 100644 > index 0000000..0fe5c8a > --- /dev/null > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c > @@ -0,0 +1,274 @@ > +/* > + * ARC PGU DRM driver. > + * > + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_cma_helper.h> > +#include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_plane_helper.h> > +#include <linux/clk.h> > +#include <linux/platform_data/simplefb.h> > + > +#include "arcpgu.h" > +#include "arcpgu_regs.h" > + > +#define ENCODE_PGU_XY(x, y) ((((x) - 1) << 16) | ((y) - 1)) > + > +static struct simplefb_format supported_formats[] = { > + { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }, > + { "r8g8b8", 24, {16, 8}, {8, 8}, {0, 8}, {0, 0}, DRM_FORMAT_RGB888 }, > +}; > + > +static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc) > +{ > + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > + uint32_t pixel_format = crtc->primary->state->fb->pixel_format; > + struct simplefb_format *format = NULL; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(supported_formats); i++) { > + if (supported_formats[i].fourcc == pixel_format) > + format = &supported_formats[i]; > + } > + > + if (WARN_ON(!format)) > + return; > + > + if (format->fourcc == DRM_FORMAT_RGB888) > + arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, > + arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) | > + ARCPGU_MODE_RGB888_MASK); > + > +} > + > +static const struct drm_crtc_funcs arc_pgu_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, > +}; > + > +static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc) > +{ > + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > + struct drm_display_mode *m = &crtc->state->adjusted_mode; > + > + arc_pgu_write(arcpgu, ARCPGU_REG_FMT, > + ENCODE_PGU_XY(m->crtc_htotal, m->crtc_vtotal)); > + > + arc_pgu_write(arcpgu, ARCPGU_REG_HSYNC, > + ENCODE_PGU_XY(m->crtc_hsync_start - m->crtc_hdisplay, > + m->crtc_hsync_end - m->crtc_hdisplay)); > + > + arc_pgu_write(arcpgu, ARCPGU_REG_VSYNC, > + ENCODE_PGU_XY(m->crtc_vsync_start - m->crtc_vdisplay, > + m->crtc_vsync_end - m->crtc_vdisplay)); > + > + arc_pgu_write(arcpgu, ARCPGU_REG_ACTIVE, > + ENCODE_PGU_XY(m->crtc_hblank_end - m->crtc_hblank_start, > + m->crtc_vblank_end - m->crtc_vblank_start)); > + > + arc_pgu_write(arcpgu, ARCPGU_REG_STRIDE, 0); > + arc_pgu_write(arcpgu, ARCPGU_REG_START_SET, 1); > + > + arc_pgu_set_pxl_fmt(crtc); > + > + clk_set_rate(arcpgu->clk, m->crtc_clock * 1000); > +} > + > +static void arc_pgu_crtc_enable(struct drm_crtc *crtc) > +{ > + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > + > + clk_prepare_enable(arcpgu->clk); > + arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, > + arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) | > + ARCPGU_CTRL_ENABLE_MASK); > +} > + > +static void arc_pgu_crtc_disable(struct drm_crtc *crtc) > +{ > + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > + > + if (!crtc->primary->fb) > + return; > + > + clk_disable_unprepare(arcpgu->clk); > + arc_pgu_write(arcpgu, ARCPGU_REG_CTRL, > + arc_pgu_read(arcpgu, ARCPGU_REG_CTRL) & > + ~ARCPGU_CTRL_ENABLE_MASK); > +} > + > +static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc, > + struct drm_crtc_state *state) > +{ > + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > + struct drm_display_mode *mode = &state->adjusted_mode; > + long rate, clk_rate = mode->clock * 1000; > + > + rate = clk_round_rate(arcpgu->clk, clk_rate); > + if (rate != clk_rate) > + return -EINVAL; > + > + return 0; > +} > + > +static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc, > + struct drm_crtc_state *state) > +{ > + struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > + unsigned long flags; > + > + if (crtc->state->event) { > + struct drm_pending_vblank_event *event = crtc->state->event; > + > + crtc->state->event = NULL; > + event->pipe = drm_crtc_index(crtc); > + > + WARN_ON(drm_crtc_vblank_get(crtc) != 0); > + > + spin_lock_irqsave(&crtc->dev->event_lock, flags); > + list_add_tail(&event->base.link, &arcpgu->event_list); > + spin_unlock_irqrestore(&crtc->dev->event_lock, flags); > + } > +} > + > +static void arc_pgu_crtc_atomic_flush(struct drm_crtc *crtc, > + struct drm_crtc_state *state) > +{ > +} > + > +static bool arc_pgu_crtc_mode_fixup(struct drm_crtc *crtc, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > + return true; > +} You can drop the above 2 dummy functions. > + > +static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = { > + .mode_fixup = arc_pgu_crtc_mode_fixup, > + .mode_set = drm_helper_crtc_mode_set, > + .mode_set_base = drm_helper_crtc_mode_set_base, > + .mode_set_nofb = arc_pgu_crtc_mode_set_nofb, > + .enable = arc_pgu_crtc_enable, > + .disable = arc_pgu_crtc_disable, > + .prepare = arc_pgu_crtc_disable, > + .commit = arc_pgu_crtc_enable, > + .atomic_check = arc_pgu_crtc_atomic_check, > + .atomic_begin = arc_pgu_crtc_atomic_begin, > + .atomic_flush = arc_pgu_crtc_atomic_flush, > +}; > + > +static int arc_pgu_plane_atomic_check(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + return 0; > +} You don't need dummy functions for this. > + > +static void arc_pgu_plane_atomic_update(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + struct arcpgu_drm_private *arcpgu; > + struct drm_gem_cma_object *gem; > + > + if (!plane->state->crtc || !plane->state->fb) > + return; > + > + arcpgu = crtc_to_arcpgu_priv(plane->state->crtc); > + gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0); > + arc_pgu_write(arcpgu, ARCPGU_REG_BUF0_ADDR, gem->paddr); > +} > + > +static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = { > + .prepare_fb = NULL, > + .cleanup_fb = NULL, > + .atomic_check = arc_pgu_plane_atomic_check, > + .atomic_update = arc_pgu_plane_atomic_update, > +}; > + > +static void arc_pgu_plane_destroy(struct drm_plane *plane) > +{ > + drm_plane_helper_disable(plane); > + drm_plane_cleanup(plane); > +} > + > +static const struct drm_plane_funcs arc_pgu_plane_funcs = { > + .update_plane = drm_atomic_helper_update_plane, > + .disable_plane = drm_atomic_helper_disable_plane, > + .destroy = arc_pgu_plane_destroy, > + .reset = drm_atomic_helper_plane_reset, > + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, > +}; > + > +static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm) > +{ > + struct arcpgu_drm_private *arcpgu = drm->dev_private; > + struct drm_plane *plane = NULL; > + u32 formats[ARRAY_SIZE(supported_formats)], i; > + int ret; > + > + plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL); > + if (!plane) > + return ERR_PTR(-ENOMEM); > + > + for (i = 0; i < ARRAY_SIZE(supported_formats); i++) > + formats[i] = supported_formats[i].fourcc; > + > + ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs, > + formats, ARRAY_SIZE(formats), > + DRM_PLANE_TYPE_PRIMARY, NULL); > + if (ret) > + return ERR_PTR(ret); > + > + drm_plane_helper_add(plane, &arc_pgu_plane_helper_funcs); > + arcpgu->plane = plane; > + > + return plane; > +} > + > +void arc_pgu_crtc_suspend(struct drm_crtc *crtc) > +{ > + arc_pgu_crtc_disable(crtc); > +} > + > +void arc_pgu_crtc_resume(struct drm_crtc *crtc) > +{ > + arc_pgu_crtc_enable(crtc); > +} Please use the atomic suspend/resume helper that Thierry recently merged. See the kerneldoc of drm_atomic_helper_suspend as a starting point for how it works and how it's supposed to be used. > + > +int arc_pgu_setup_crtc(struct drm_device *drm) > +{ > + struct arcpgu_drm_private *arcpgu = drm->dev_private; > + struct drm_plane *primary; > + int ret; > + > + primary = arc_pgu_plane_init(drm); > + if (IS_ERR(primary)) > + return PTR_ERR(primary); > + > + ret = drm_crtc_init_with_planes(drm, &arcpgu->crtc, primary, NULL, > + &arc_pgu_crtc_funcs, NULL); > + if (ret) { > + arc_pgu_plane_destroy(primary); > + return ret; > + } > + > + drm_crtc_helper_add(&arcpgu->crtc, &arc_pgu_crtc_helper_funcs); > + return 0; > +} > diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c > new file mode 100644 > index 0000000..d47481d > --- /dev/null > +++ b/drivers/gpu/drm/arc/arcpgu_drv.c > @@ -0,0 +1,252 @@ > +/* > + * ARC PGU DRM driver. > + * > + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <linux/clk.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_cma_helper.h> > +#include <drm/drm_gem_cma_helper.h> > +#include <drm/drm_atomic_helper.h> > + > +#include "arcpgu.h" > +#include "arcpgu_regs.h" > + > +static void arcpgu_fb_output_poll_changed(struct drm_device *dev) > +{ > + struct arcpgu_drm_private *arcpgu = dev->dev_private; > + > + if (arcpgu->fbdev) > + drm_fbdev_cma_hotplug_event(arcpgu->fbdev); > +} > + > +static int arcpgu_atomic_commit(struct drm_device *dev, > + struct drm_atomic_state *state, bool async) > +{ > + return drm_atomic_helper_commit(dev, state, false); Note that this isn't really async if you ever get around to implement fence support or vblank support. Just fyi. > +} > + > +static struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = { > + .fb_create = drm_fb_cma_create, > + .output_poll_changed = arcpgu_fb_output_poll_changed, > + .atomic_check = drm_atomic_helper_check, > + .atomic_commit = arcpgu_atomic_commit, > +}; > + > +static void arcpgu_setup_mode_config(struct drm_device *drm) > +{ > + drm_mode_config_init(drm); > + drm->mode_config.min_width = 0; > + drm->mode_config.min_height = 0; > + drm->mode_config.max_width = 1920; > + drm->mode_config.max_height = 1080; > + drm->mode_config.funcs = &arcpgu_drm_modecfg_funcs; > +} > + > +int arcpgu_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +{ > + int ret; > + > + ret = drm_gem_mmap(filp, vma); > + if (ret) > + return ret; > + > + vma->vm_page_prot = pgprot_noncached(vm_get_page_prot(vma->vm_flags)); > + return 0; > +} > + > +static const struct file_operations arcpgu_drm_ops = { > + .owner = THIS_MODULE, > + .open = drm_open, > + .release = drm_release, > + .unlocked_ioctl = drm_ioctl, > +#ifdef CONFIG_COMPAT > + .compat_ioctl = drm_compat_ioctl, > +#endif > + .poll = drm_poll, > + .read = drm_read, > + .llseek = no_llseek, > + .mmap = arcpgu_gem_mmap, > +}; > + > +static void arcpgu_preclose(struct drm_device *drm, struct drm_file *file) > +{ > + struct arcpgu_drm_private *arcpgu = drm->dev_private; > + struct drm_pending_vblank_event *e, *t; > + unsigned long flags; > + > + spin_lock_irqsave(&drm->event_lock, flags); > + list_for_each_entry_safe(e, t, &arcpgu->event_list, base.link) { > + if (e->base.file_priv != file) > + continue; > + list_del(&e->base.link); > + e->base.destroy(&e->base); > + } > + spin_unlock_irqrestore(&drm->event_lock, flags); > +} > + > +static void arcpgu_lastclose(struct drm_device *drm) > +{ > + struct arcpgu_drm_private *arcpgu = drm->dev_private; > + > + drm_fbdev_cma_restore_mode(arcpgu->fbdev); > +} > + > +static int arcpgu_load(struct drm_device *drm, unsigned long flags) > +{ > + struct platform_device *pdev = drm->platformdev; > + struct arcpgu_drm_private *arcpgu; > + struct device_node *encoder_node; > + struct resource *res; > + int ret; > + > + arcpgu = devm_kzalloc(&pdev->dev, sizeof(*arcpgu), GFP_KERNEL); > + if (arcpgu == NULL) > + return -ENOMEM; > + > + drm->dev_private = arcpgu; > + > + arcpgu->clk = devm_clk_get(drm->dev, "pxlclk"); > + if (IS_ERR(arcpgu->clk)) > + return PTR_ERR(arcpgu->clk); > + > + INIT_LIST_HEAD(&arcpgu->event_list); > + > + arcpgu_setup_mode_config(drm); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + arcpgu->regs = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(arcpgu->regs)) { > + dev_err(drm->dev, "Could not remap IO mem\n"); > + return PTR_ERR(arcpgu->regs); > + } > + > + dev_info(drm->dev, "arc_pgu ID: 0x%x\n", > + arc_pgu_read(arcpgu, ARCPGU_REG_ID)); > + > + if (dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32))) > + return -ENODEV; > + > + if (arc_pgu_setup_crtc(drm) < 0) > + return -ENODEV; > + > + /* find the encoder node and initialize it */ > + encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0); > + if (!encoder_node) { > + dev_err(drm->dev, "failed to get an encoder slave node\n"); > + return -ENODEV; > + } > + > + ret = arcpgu_drm_hdmi_init(drm, encoder_node); > + if (ret < 0) > + return ret; > + > + drm_mode_config_reset(drm); > + drm_kms_helper_poll_init(drm); > + > + arcpgu->fbdev = arcpgu_fbdev_cma_init(drm, 16, > + drm->mode_config.num_crtc, > + drm->mode_config.num_connector); > + if (IS_ERR(arcpgu->fbdev)) { > + ret = PTR_ERR(arcpgu->fbdev); > + arcpgu->fbdev = NULL; > + return -ENODEV; > + } > + > + platform_set_drvdata(pdev, arcpgu); > + return 0; > +} > + > +int arcpgu_unload(struct drm_device *drm) > +{ > + struct arcpgu_drm_private *arcpgu = drm->dev_private; > + > + if (arcpgu->fbdev) { > + drm_fbdev_cma_fini(arcpgu->fbdev); > + arcpgu->fbdev = NULL; > + } > + drm_kms_helper_poll_fini(drm); > + drm_vblank_cleanup(drm); > + drm_mode_config_cleanup(drm); > + > + return 0; > +} > + > +static struct drm_driver arcpgu_drm_driver = { > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > + DRIVER_ATOMIC, > + .preclose = arcpgu_preclose, > + .lastclose = arcpgu_lastclose, > + .name = "drm-arcpgu", > + .desc = "ARC PGU Controller", > + .date = "20160219", > + .major = 1, > + .minor = 0, > + .patchlevel = 0, > + .fops = &arcpgu_drm_ops, > + .load = arcpgu_load, > + .unload = arcpgu_unload, Load and unload hooks are deprecated (it's a classic midlayer mistake). Please use drm_dev_alloc/register pairs directly instead, and put your device setup code in-between. Similar for unloading. There's a bunch of example drivers converted already. > + .dumb_create = drm_gem_cma_dumb_create, > + .dumb_map_offset = drm_gem_cma_dumb_map_offset, > + .dumb_destroy = drm_gem_dumb_destroy, > + .get_vblank_counter = drm_vblank_no_hw_counter, > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > + .gem_free_object = 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 arcpgu_probe(struct platform_device *pdev) > +{ > + return drm_platform_init(&arcpgu_drm_driver, pdev); ... or read the kerneldoc of this function, which also explains what you should do ;-) > +} > + > +static int arcpgu_remove(struct platform_device *pdev) > +{ > + struct drm_device *drm = dev_get_drvdata(&pdev->dev); > + > + drm_put_dev(drm); > + > + return 0; > +} > + > +static const struct of_device_id arcpgu_of_table[] = { > + {.compatible = "snps,arcpgu"}, > + {} > +}; > + > +MODULE_DEVICE_TABLE(of, arcpgu_of_table); > + > +static struct platform_driver arcpgu_platform_driver = { > + .probe = arcpgu_probe, > + .remove = arcpgu_remove, > + .driver = { > + .name = "arcpgu", > + .owner = THIS_MODULE, > + .of_match_table = arcpgu_of_table, > + }, > +}; > + > +module_platform_driver(arcpgu_platform_driver); > + > +MODULE_AUTHOR("Carlos Palminha <palminha@synopsys.com"); > +MODULE_DESCRIPTION("ARC PGU DRM driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/gpu/drm/arc/arcpgu_fbdev.c b/drivers/gpu/drm/arc/arcpgu_fbdev.c > new file mode 100644 > index 0000000..6f67706 > --- /dev/null > +++ b/drivers/gpu/drm/arc/arcpgu_fbdev.c > @@ -0,0 +1,245 @@ > +/* > + * ARC PGU DRM driver. > + * > + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_fb_cma_helper.h> > +#include <drm/drm_fb_helper.h> > +#include <drm/drm_gem_cma_helper.h> > + > +struct drm_fb_cma { > + struct drm_framebuffer fb; > + struct drm_gem_cma_object *obj[4]; > +}; > + > +struct drm_fbdev_cma { > + struct drm_fb_helper fb_helper; > + struct drm_fb_cma *fb; > +}; > + > +static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper) > +{ > + return container_of(helper, struct drm_fbdev_cma, fb_helper); > +} > + > +static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb) > +{ > + return container_of(fb, struct drm_fb_cma, fb); > +} > + > +static void drm_fb_cma_destroy(struct drm_framebuffer *fb) > +{ > + struct drm_fb_cma *fb_cma = to_fb_cma(fb); > + int i; > + > + for (i = 0; i < 4; i++) { > + if (fb_cma->obj[i]) > + drm_gem_object_unreference_unlocked(&fb_cma->obj[i]->base); > + } > + > + drm_framebuffer_cleanup(fb); > + kfree(fb_cma); > +} > + > +static int drm_fb_cma_create_handle(struct drm_framebuffer *fb, > + struct drm_file *file_priv, unsigned int *handle) > +{ > + struct drm_fb_cma *fb_cma = to_fb_cma(fb); > + > + return drm_gem_handle_create(file_priv, > + &fb_cma->obj[0]->base, handle); > +} > + > +static struct drm_framebuffer_funcs drm_fb_cma_funcs = { > + .destroy = drm_fb_cma_destroy, > + .create_handle = drm_fb_cma_create_handle, > +}; > + > +static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, > + const struct drm_mode_fb_cmd2 *mode_cmd, > + struct drm_gem_cma_object **obj, > + unsigned int num_planes) > +{ > + struct drm_fb_cma *fb_cma; > + int ret; > + int i; > + > + fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL); > + if (!fb_cma) > + return ERR_PTR(-ENOMEM); > + > + drm_helper_mode_fill_fb_struct(&fb_cma->fb, mode_cmd); > + > + for (i = 0; i < num_planes; i++) > + fb_cma->obj[i] = obj[i]; > + > + ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); > + if (ret) { > + dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret); > + kfree(fb_cma); > + return ERR_PTR(ret); > + } > + > + return fb_cma; > +} > + > +/* > + * This function is the only reason to have a copy of drm_fbdev_cma_init() > + * here in this driver. > + * > + * In its turn this mmap() is required to mark user-space page as non-cached > + * because it is just a mirror or real hardware frame-buffer. > + */ > +static int arcpgu_mmap(struct fb_info *info, struct vm_area_struct *vma) > +{ > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > + return vm_iomap_memory(vma, info->fix.smem_start, info->fix.smem_len); > +} This looks very fishy, no other drm driver even bothers with providing an fb_mmap hook. What exactly do you need this for? Assuming you've mmapped your fbcon drm_framebuffer correctly for kernel access things should just work ... > + > +static struct fb_ops drm_fbdev_cma_ops = { > + .owner = THIS_MODULE, > + .fb_mmap = arcpgu_mmap, > + .fb_fillrect = drm_fb_helper_sys_fillrect, > + .fb_copyarea = drm_fb_helper_sys_copyarea, > + .fb_imageblit = drm_fb_helper_sys_imageblit, > + .fb_check_var = drm_fb_helper_check_var, > + .fb_set_par = drm_fb_helper_set_par, > + .fb_blank = drm_fb_helper_blank, > + .fb_pan_display = drm_fb_helper_pan_display, > + .fb_setcmap = drm_fb_helper_setcmap, > +}; > + > +static int drm_fbdev_cma_create(struct drm_fb_helper *helper, > + struct drm_fb_helper_surface_size *sizes) > +{ > + struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper); > + struct drm_mode_fb_cmd2 mode_cmd = { 0 }; > + struct drm_device *dev = helper->dev; > + struct drm_gem_cma_object *obj; > + struct drm_framebuffer *fb; > + unsigned int bytes_per_pixel; > + unsigned long offset; > + struct fb_info *fbi; > + size_t size; > + int ret; > + > + DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n", > + sizes->surface_width, sizes->surface_height, > + sizes->surface_bpp); > + > + bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8); > + > + mode_cmd.width = sizes->surface_width; > + mode_cmd.height = sizes->surface_height; > + mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel; > + mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, > + sizes->surface_depth); > + > + size = mode_cmd.pitches[0] * mode_cmd.height; > + obj = drm_gem_cma_create(dev, size); > + if (IS_ERR(obj)) > + return -ENOMEM; > + > + fbi = drm_fb_helper_alloc_fbi(helper); > + if (IS_ERR(fbi)) { > + ret = PTR_ERR(fbi); > + goto err_gem_free_object; > + } > + > + fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1); > + if (IS_ERR(fbdev_cma->fb)) { > + dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n"); > + ret = PTR_ERR(fbdev_cma->fb); > + goto err_fb_info_destroy; > + } > + > + fb = &fbdev_cma->fb->fb; > + helper->fb = fb; > + > + fbi->par = helper; > + fbi->flags = FBINFO_FLAG_DEFAULT; > + fbi->fbops = &drm_fbdev_cma_ops; > + > + drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->depth); > + drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height); > + > + offset = fbi->var.xoffset * bytes_per_pixel; > + offset += fbi->var.yoffset * fb->pitches[0]; > + > + dev->mode_config.fb_base = (resource_size_t)obj->paddr; > + fbi->screen_base = obj->vaddr + offset; > + fbi->fix.smem_start = (unsigned long)(obj->paddr + offset); > + fbi->screen_size = size; > + fbi->fix.smem_len = size; > + > + return 0; > + > +err_fb_info_destroy: > + drm_fb_helper_release_fbi(helper); > +err_gem_free_object: > + dev->driver->gem_free_object(&obj->base); > + return ret; > +} > + > +static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { > + .fb_probe = drm_fbdev_cma_create, > +}; > + > +struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev, > + unsigned int preferred_bpp, unsigned int num_crtc, > + unsigned int max_conn_count) > +{ > + struct drm_fbdev_cma *fbdev_cma; > + struct drm_fb_helper *helper; > + int ret; > + > + fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL); > + if (!fbdev_cma) { > + dev_err(dev->dev, "Failed to allocate drm fbdev.\n"); > + return ERR_PTR(-ENOMEM); > + } > + > + helper = &fbdev_cma->fb_helper; > + > + drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); > + > + ret = drm_fb_helper_init(dev, helper, num_crtc, max_conn_count); > + if (ret < 0) { > + dev_err(dev->dev, "Failed to initialize drm fb helper.\n"); > + goto err_free; > + } > + > + ret = drm_fb_helper_single_add_all_connectors(helper); > + if (ret < 0) { > + dev_err(dev->dev, "Failed to add connectors.\n"); > + goto err_drm_fb_helper_fini; > + > + } > + > + ret = drm_fb_helper_initial_config(helper, preferred_bpp); > + if (ret < 0) { > + dev_err(dev->dev, "Failed to set initial hw configuration.\n"); > + goto err_drm_fb_helper_fini; > + } > + > + return fbdev_cma; > + > +err_drm_fb_helper_fini: > + drm_fb_helper_fini(helper); > +err_free: > + kfree(fbdev_cma); > + > + return ERR_PTR(ret); > +} > diff --git a/drivers/gpu/drm/arc/arcpgu_hdmi.c b/drivers/gpu/drm/arc/arcpgu_hdmi.c > new file mode 100644 > index 0000000..7adafa3 > --- /dev/null > +++ b/drivers/gpu/drm/arc/arcpgu_hdmi.c > @@ -0,0 +1,207 @@ > +/* > + * ARC PGU DRM driver. > + * > + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_encoder_slave.h> > +#include <drm/drm_atomic_helper.h> > + > +#include "arcpgu.h" > + > +struct arcpgu_drm_connector { > + struct drm_connector connector; > + struct drm_encoder_slave *encoder_slave; > +}; > + > +static int arcpgu_drm_connector_get_modes(struct drm_connector *connector) > +{ > + const struct drm_encoder_slave_funcs *sfuncs; > + struct drm_encoder_slave *slave; > + struct arcpgu_drm_connector *con = > + container_of(connector, struct arcpgu_drm_connector, connector); > + > + slave = con->encoder_slave; > + if (slave == NULL) { > + dev_err(connector->dev->dev, > + "connector_get_modes: cannot find slave encoder for connector\n"); > + return 0; > + } > + > + sfuncs = slave->slave_funcs; > + if (sfuncs->get_modes == NULL) > + return 0; > + > + return sfuncs->get_modes(&slave->base, connector); > +} > + > +struct drm_encoder * > +arcpgu_drm_connector_best_encoder(struct drm_connector *connector) > +{ > + struct drm_encoder_slave *slave; > + struct arcpgu_drm_connector *con = > + container_of(connector, struct arcpgu_drm_connector, connector); > + > + slave = con->encoder_slave; > + if (slave == NULL) { > + dev_err(connector->dev->dev, > + "connector_best_encoder: cannot find slave encoder for connector\n"); > + return NULL; > + } > + > + return &slave->base; > +} > + > +static enum drm_connector_status > +arcpgu_drm_connector_detect(struct drm_connector *connector, bool force) > +{ > + enum drm_connector_status status = connector_status_unknown; > + const struct drm_encoder_slave_funcs *sfuncs; > + struct drm_encoder_slave *slave; > + > + struct arcpgu_drm_connector *con = > + container_of(connector, struct arcpgu_drm_connector, connector); > + > + slave = con->encoder_slave; > + if (slave == NULL) { > + dev_err(connector->dev->dev, > + "connector_detect: cannot find slave encoder for connector\n"); > + return status; > + } > + > + sfuncs = slave->slave_funcs; > + if (sfuncs && sfuncs->detect) > + return sfuncs->detect(&slave->base, connector); > + > + dev_err(connector->dev->dev, "connector_detect: could not detect slave funcs\n"); > + return status; > +} > + > +static void arcpgu_drm_connector_destroy(struct drm_connector *connector) > +{ > + drm_connector_unregister(connector); > + drm_connector_cleanup(connector); > +} > + > +static const struct drm_connector_helper_funcs > +arcpgu_drm_connector_helper_funcs = { > + .get_modes = arcpgu_drm_connector_get_modes, > + .best_encoder = arcpgu_drm_connector_best_encoder, > +}; > + > +static const struct drm_connector_funcs arcpgu_drm_connector_funcs = { > + .dpms = drm_helper_connector_dpms, > + .reset = drm_atomic_helper_connector_reset, > + .detect = arcpgu_drm_connector_detect, > + .fill_modes = drm_helper_probe_single_connector_modes, > + .destroy = arcpgu_drm_connector_destroy, > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > +}; > + > +static struct drm_encoder_helper_funcs arcpgu_drm_encoder_helper_funcs = { > + .dpms = drm_i2c_encoder_dpms, > + .mode_fixup = drm_i2c_encoder_mode_fixup, > + .mode_set = drm_i2c_encoder_mode_set, > + .prepare = drm_i2c_encoder_prepare, > + .commit = drm_i2c_encoder_commit, > + .detect = drm_i2c_encoder_detect, > +}; > + > +static struct drm_encoder_funcs arcpgu_drm_encoder_funcs = { > + .destroy = drm_encoder_cleanup, > +}; > + > +int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np) > +{ > + struct arcpgu_drm_connector *arcpgu_connector; > + struct drm_i2c_encoder_driver *driver; > + struct drm_encoder_slave *encoder; > + struct drm_connector *connector; > + struct i2c_client *i2c_slave; > + int ret; > + > + encoder = devm_kzalloc(drm->dev, sizeof(*encoder), GFP_KERNEL); > + if (encoder == NULL) > + return -ENOMEM; > + > + i2c_slave = of_find_i2c_device_by_node(np); > + if (!i2c_slave || !i2c_get_clientdata(i2c_slave)) { > + dev_err(drm->dev, "failed to find i2c slave encoder\n"); > + return -EPROBE_DEFER; > + } > + > + if (i2c_slave->dev.driver == NULL) { > + dev_err(drm->dev, "failed to find i2c slave driver\n"); > + return -EPROBE_DEFER; > + } > + > + driver = > + to_drm_i2c_encoder_driver(to_i2c_driver(i2c_slave->dev.driver)); > + ret = driver->encoder_init(i2c_slave, drm, encoder); > + if (ret) { > + dev_err(drm->dev, "failed to initialize i2c encoder slave\n"); > + return ret; > + } > + > + encoder->base.possible_crtcs = 1; > + encoder->base.possible_clones = 0; > + ret = drm_encoder_init(drm, &encoder->base, &arcpgu_drm_encoder_funcs, > + DRM_MODE_ENCODER_TMDS, NULL); > + if (ret) > + return ret; > + > + drm_encoder_helper_add(&encoder->base, > + &arcpgu_drm_encoder_helper_funcs); > + > + arcpgu_connector = devm_kzalloc(drm->dev, sizeof(*arcpgu_connector), > + GFP_KERNEL); > + if (!arcpgu_connector) { > + ret = -ENOMEM; > + goto error_encoder_cleanup; > + } > + > + connector = &arcpgu_connector->connector; > + drm_connector_helper_add(connector, &arcpgu_drm_connector_helper_funcs); > + ret = drm_connector_init(drm, connector, &arcpgu_drm_connector_funcs, > + DRM_MODE_CONNECTOR_HDMIA); > + if (ret < 0) { > + dev_err(drm->dev, "failed to initialize drm connector\n"); > + goto error_encoder_cleanup; > + } > + > + ret = drm_connector_register(connector); > + if (ret < 0) { > + dev_err(drm->dev, "failed to regiter DRM connector and helper funcs\n"); > + goto error_connector_cleanup; > + } > + > + ret = drm_mode_connector_attach_encoder(connector, &encoder->base); > + if (ret < 0) { > + dev_err(drm->dev, "could not attach connector to encoder\n"); > + drm_connector_unregister(connector); > + goto error_connector_cleanup; > + } > + > + arcpgu_connector->encoder_slave = encoder; > + > + return 0; > + > +error_connector_cleanup: > + drm_connector_cleanup(connector); > + > +error_encoder_cleanup: > + drm_encoder_cleanup(&encoder->base); > + return ret; > +} > diff --git a/drivers/gpu/drm/arc/arcpgu_regs.h b/drivers/gpu/drm/arc/arcpgu_regs.h > new file mode 100644 > index 0000000..f04165f > --- /dev/null > +++ b/drivers/gpu/drm/arc/arcpgu_regs.h > @@ -0,0 +1,36 @@ > +/* > + * ARC PGU DRM driver. > + * > + * Copyright (C) 2016 Synopsys, Inc. (www.synopsys.com) > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#ifndef _ARC_PGU_REGS_H_ > +#define _ARC_PGU_REGS_H_ > + > +#define ARCPGU_REG_CTRL 0x00 > +#define ARCPGU_REG_STAT 0x04 > +#define ARCPGU_REG_FMT 0x10 > +#define ARCPGU_REG_HSYNC 0x14 > +#define ARCPGU_REG_VSYNC 0x18 > +#define ARCPGU_REG_ACTIVE 0x1c > +#define ARCPGU_REG_BUF0_ADDR 0x40 > +#define ARCPGU_REG_STRIDE 0x50 > +#define ARCPGU_REG_START_SET 0x84 > + > +#define ARCPGU_REG_ID 0x3FC > + > +#define ARCPGU_CTRL_ENABLE_MASK 0x02 > +#define ARCPGU_MODE_RGB888_MASK 0x04 > +#define ARCPGU_STAT_BUSY_MASK 0x02 > + > +#endif > -- > 2.5.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-14 7:00 ` [PATCH 1/4 v3] drm: Add support of ARC PGU display controller Daniel Vetter @ 2016-03-14 11:15 ` Alexey Brodkin 2016-03-15 8:10 ` Daniel Vetter 0 siblings, 1 reply; 18+ messages in thread From: Alexey Brodkin @ 2016-03-14 11:15 UTC (permalink / raw) To: daniel; +Cc: dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc Hi Daniel, On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > ARC PGU could be found on some development boards from Synopsys. > > This is a simple byte streamer that reads data from a framebuffer > > and sends data to the single encoder. > > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > Cc: David Airlie <airlied@linux.ie> > > Cc: dri-devel@lists.freedesktop.org > > Cc: linux-snps-arc@lists.infradead.org > > Cc: Jose Abreu <joabreu@synopsys.com> > > --- > > > > Changes v2 -> v3: > > * Improved failure path if arcpgu_connector wasn't allocated (thanks Jose). > > * Fixed driver building as module (reported by 0-DAY kernel test infrastruct.) > > * Implemented uncached mapping of user-space FB pages. > > > > No changes v1 -> v2. > > > Bunch of comments below to update your driver to latest styles and best > practices. > > Cheers, Daniel Thanks for doing that review! > > + > > +static void arc_pgu_crtc_atomic_flush(struct drm_crtc *crtc, > > + struct drm_crtc_state *state) > > +{ > > +} > > + > > +static bool arc_pgu_crtc_mode_fixup(struct drm_crtc *crtc, > > + const struct drm_display_mode *mode, > > + struct drm_display_mode *adjusted_mode) > > +{ > > + return true; > > +} > You can drop the above 2 dummy functions. Ok will do. > > > > + > > +static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = { > > + .mode_fixup = arc_pgu_crtc_mode_fixup, > > + .mode_set = drm_helper_crtc_mode_set, > > + .mode_set_base = drm_helper_crtc_mode_set_base, > > + .mode_set_nofb = arc_pgu_crtc_mode_set_nofb, > > + .enable = arc_pgu_crtc_enable, > > + .disable = arc_pgu_crtc_disable, > > + .prepare = arc_pgu_crtc_disable, > > + .commit = arc_pgu_crtc_enable, > > + .atomic_check = arc_pgu_crtc_atomic_check, > > + .atomic_begin = arc_pgu_crtc_atomic_begin, > > + .atomic_flush = arc_pgu_crtc_atomic_flush, > > +}; > > + > > +static int arc_pgu_plane_atomic_check(struct drm_plane *plane, > > + struct drm_plane_state *state) > > +{ > > + return 0; > > +} > You don't need dummy functions for this. Ditto. > > + > > +void arc_pgu_crtc_suspend(struct drm_crtc *crtc) > > +{ > > + arc_pgu_crtc_disable(crtc); > > +} > > + > > +void arc_pgu_crtc_resume(struct drm_crtc *crtc) > > +{ > > + arc_pgu_crtc_enable(crtc); > > +} > Please use the atomic suspend/resume helper that Thierry recently merged. > See the kerneldoc of drm_atomic_helper_suspend as a starting point for how > it works and how it's supposed to be used. Well looks like this is a reminder if dummy copy-paste. We don't support PM in that driver yet, so I'll remove both functions for now. > > +static int arcpgu_atomic_commit(struct drm_device *dev, > > + struct drm_atomic_state *state, bool async) > > +{ > > + return drm_atomic_helper_commit(dev, state, false); > Note that this isn't really async if you ever get around to implement > fence support or vblank support. Just fyi. Ok but for now should I leave it as it is? > > +static struct drm_driver arcpgu_drm_driver = { > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > + DRIVER_ATOMIC, > > + .preclose = arcpgu_preclose, > > + .lastclose = arcpgu_lastclose, > > + .name = "drm-arcpgu", > > + .desc = "ARC PGU Controller", > > + .date = "20160219", > > + .major = 1, > > + .minor = 0, > > + .patchlevel = 0, > > + .fops = &arcpgu_drm_ops, > > + .load = arcpgu_load, > > + .unload = arcpgu_unload, > Load and unload hooks are deprecated (it's a classic midlayer mistake). > Please use drm_dev_alloc/register pairs directly instead, and put your > device setup code in-between. Similar for unloading. There's a bunch of > example drivers converted already. Ok I took "atmel-hlcdc" as example. And that's interesting. If I put my arcpgu_load() in between drm_dev_alloc() and drm_dev_register() then I'm getting this on the driver probe: ---------------------------------->8------------------------------- [drm] Initialized drm 1.1.0 20060810 arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 Stack Trace: arc_unwind_core.constprop.1+0xa4/0x110 warn_slowpath_fmt+0x6e/0xfc kobject_add_internal+0x17c/0x498 kobject_add+0x98/0xe4 device_add+0xc6/0x734 device_create_with_groups+0x12a/0x144 drm_sysfs_connector_add+0x54/0xe8 arcpgu_drm_hdmi_init+0xd4/0x17c arcpgu_probe+0x138/0x24c platform_drv_probe+0x2e/0x6c really_probe+0x212/0x35c __driver_attach+0x90/0x94 bus_for_each_dev+0x46/0x80 bus_add_driver+0x14e/0x1b4 driver_register+0x64/0x108 do_one_initcall+0x86/0x194 kernel_init_freeable+0xf0/0x188 ---[ end trace c67166ad43ddcce2 ]--- [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs arcpgu: probe of e0017000.pgu failed with error -2 ---------------------------------->8------------------------------- But if I move arcpgu_load() after drm_dev_register() then everything starts properly and I may see HDMI screen works perfectly fine. Any thoughts? > > > > + .dumb_create = drm_gem_cma_dumb_create, > > + .dumb_map_offset = drm_gem_cma_dumb_map_offset, > > + .dumb_destroy = drm_gem_dumb_destroy, > > + .get_vblank_counter = drm_vblank_no_hw_counter, > > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > > + .gem_free_object = 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 arcpgu_probe(struct platform_device *pdev) > > +{ > > + return drm_platform_init(&arcpgu_drm_driver, pdev); > ... or read the kerneldoc of this function, which also explains what you > should do ;-) Could you please point me to the relevant document? I wasn't able to find anything related from the first glance :( > > + > > +/* > > + * This function is the only reason to have a copy of drm_fbdev_cma_init() > > + * here in this driver. > > + * > > + * In its turn this mmap() is required to mark user-space page as non-cached > > + * because it is just a mirror or real hardware frame-buffer. > > + */ > > +static int arcpgu_mmap(struct fb_info *info, struct vm_area_struct *vma) > > +{ > > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > + return vm_iomap_memory(vma, info->fix.smem_start, info->fix.smem_len); > > +} > This looks very fishy, no other drm driver even bothers with providing an > fb_mmap hook. What exactly do you need this for? Assuming you've mmapped > your fbcon drm_framebuffer correctly for kernel access things should just > work ... Indeed for kernel there's non need to that hack. Kernel deals directly with HW frame-buffer area (that address we get from gem->paddr). And so every byte written gets picked up by PGU and is then rendered on the display. But when user-space opens /dev/fb0 and mmaps() it deals with memory pages which are by default (at least on ARC) marked as "cached". I.e. user-space application (I use that nice demo app https://github.com/qtproject/qt/blob/4.8/examples/qws/framebuffer/main.c) deals with frame-buffer via data cache. And that has 2 problems: [1] Since no explicit cache flush gets executed some data is left in data cache, i.e. some parts of the picture never reaches real PGU. See what happens on display - http://imgur.com/iAbnnx3 Those missing lines are exactly those 32-byte missing cache lines. [2] Even if we manage to flush data somehow massive amount of data that goes through data cache (let's sat 1080p@30Hz) will thrash it and as a result there will be no benefit for other cache users to use cache. So we fix it simply marking pages mapped to user-space apps as uncached that effectively routes all FB data directly to memry instead of polluting cache. Hopefully that explanation makes sense. -Alexey ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-14 11:15 ` Alexey Brodkin @ 2016-03-15 8:10 ` Daniel Vetter 2016-03-15 15:24 ` Alexey Brodkin 0 siblings, 1 reply; 18+ messages in thread From: Daniel Vetter @ 2016-03-15 8:10 UTC (permalink / raw) To: Alexey Brodkin Cc: daniel, dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > +static int arcpgu_atomic_commit(struct drm_device *dev, > > > + struct drm_atomic_state *state, bool async) > > > +{ > > > + return drm_atomic_helper_commit(dev, state, false); > > Note that this isn't really async if you ever get around to implement > > fence support or vblank support. Just fyi. > > Ok but for now should I leave it as it is? Yeah ok as-is, hence just fyi. > > > +static struct drm_driver arcpgu_drm_driver = { > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > > + DRIVER_ATOMIC, > > > + .preclose = arcpgu_preclose, > > > + .lastclose = arcpgu_lastclose, > > > + .name = "drm-arcpgu", > > > + .desc = "ARC PGU Controller", > > > + .date = "20160219", > > > + .major = 1, > > > + .minor = 0, > > > + .patchlevel = 0, > > > + .fops = &arcpgu_drm_ops, > > > + .load = arcpgu_load, > > > + .unload = arcpgu_unload, > > Load and unload hooks are deprecated (it's a classic midlayer mistake). > > Please use drm_dev_alloc/register pairs directly instead, and put your > > device setup code in-between. Similar for unloading. There's a bunch of > > example drivers converted already. > > Ok I took "atmel-hlcdc" as example. > And that's interesting. > > If I put my arcpgu_load() in between drm_dev_alloc() and > drm_dev_register() then I'm getting this on the driver probe: > ---------------------------------->8------------------------------- > [drm] Initialized drm 1.1.0 20060810 > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > Modules linked in: > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > Stack Trace: > arc_unwind_core.constprop.1+0xa4/0x110 > warn_slowpath_fmt+0x6e/0xfc > kobject_add_internal+0x17c/0x498 > kobject_add+0x98/0xe4 > device_add+0xc6/0x734 > device_create_with_groups+0x12a/0x144 > drm_sysfs_connector_add+0x54/0xe8 > arcpgu_drm_hdmi_init+0xd4/0x17c > arcpgu_probe+0x138/0x24c > platform_drv_probe+0x2e/0x6c > really_probe+0x212/0x35c > __driver_attach+0x90/0x94 > bus_for_each_dev+0x46/0x80 > bus_add_driver+0x14e/0x1b4 > driver_register+0x64/0x108 > do_one_initcall+0x86/0x194 > kernel_init_freeable+0xf0/0x188 > ---[ end trace c67166ad43ddcce2 ]--- > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > arcpgu: probe of e0017000.pgu failed with error -2 > ---------------------------------->8------------------------------- > > But if I move arcpgu_load() after drm_dev_register() then everything > starts properly and I may see HDMI screen works perfectly fine. > > Any thoughts? Oops, yeah missed that detail. If you look at atmel it has a loop to register all the drm connectors _after_ calling drm_dev_register(). Totally forgot about that. Can you pls - Extract a new drm_connector_register_all() function (atmel_hlcdc_dc_connector_plug_all seems to be the best template), including kerneldoc. - Adjust kerneldoc of drm_dev_register() to mention drm_connector_register_all() and that ordering constraint. - Roll that helper out to all the drivers that currently hand-roll it (one patch per driver). I know a bit of work but imo not too much, and by doing some small refactoring every time someone stumbles over a drm pitfall we keep the subsystem clean&easy to understand. You're up for this? Would be a prep series, I'll happily review it to get it merged fast. Just a few weeks ago I merged 20+ patches to make ->mode_fixup hooks optional and remove dummy ones all over the subsystem, in other words: You'll have my full attention ;-) > > > + .dumb_create = drm_gem_cma_dumb_create, > > > + .dumb_map_offset = drm_gem_cma_dumb_map_offset, > > > + .dumb_destroy = drm_gem_dumb_destroy, > > > + .get_vblank_counter = drm_vblank_no_hw_counter, > > > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > > > + .gem_free_object = 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 arcpgu_probe(struct platform_device *pdev) > > > +{ > > > + return drm_platform_init(&arcpgu_drm_driver, pdev); > > ... or read the kerneldoc of this function, which also explains what you > > should do ;-) > > Could you please point me to the relevant document? > I wasn't able to find anything related from the first glance :( The kerneldoc comment for drm_platform_init says it's deprecated, please use drm_dev_alloc/register instead. > > > > + > > > +/* > > > + * This function is the only reason to have a copy of drm_fbdev_cma_init() > > > + * here in this driver. > > > + * > > > + * In its turn this mmap() is required to mark user-space page as non-cached > > > + * because it is just a mirror or real hardware frame-buffer. > > > + */ > > > +static int arcpgu_mmap(struct fb_info *info, struct vm_area_struct *vma) > > > +{ > > > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > > + return vm_iomap_memory(vma, info->fix.smem_start, info->fix.smem_len); > > > +} > > This looks very fishy, no other drm driver even bothers with providing an > > fb_mmap hook. What exactly do you need this for? Assuming you've mmapped > > your fbcon drm_framebuffer correctly for kernel access things should just > > work ... > > Indeed for kernel there's non need to that hack. Kernel deals directly with HW > frame-buffer area (that address we get from gem->paddr). And so every byte written gets > picked up by PGU and is then rendered on the display. > > But when user-space opens /dev/fb0 and mmaps() it deals with memory pages which > are by default (at least on ARC) marked as "cached". I.e. user-space application > (I use that nice demo app https://github.com/qtproject/qt/blob/4.8/examples/qws/framebuffer/main.c) > deals with frame-buffer via data cache. And that has 2 problems: > [1] Since no explicit cache flush gets executed some data is left in data cache, > i.e. some parts of the picture never reaches real PGU. > See what happens on display - http://imgur.com/iAbnnx3 > Those missing lines are exactly those 32-byte missing cache lines. > [2] Even if we manage to flush data somehow massive amount of data that goes > through data cache (let's sat 1080p@30Hz) will thrash it and as a result > there will be no benefit for other cache users to use cache. > > So we fix it simply marking pages mapped to user-space apps as uncached > that effectively routes all FB data directly to memry instead of polluting cache. > > Hopefully that explanation makes sense. Still strange that you're the only one who needs this. I think best case would be to provide some mmap helpers for fbdev which remap to dumb buffer helpers or something similar. That way other drivers don't need to Another piece is that right now fbdev emulation hasn't wired up the manual update mode to the drm ->dirtyfb call. Drivers that need to manual flush stuff to the screen currently all have to hand-roll those calls one way or the other (udl, qxl, i915). But just an observation really, nothing to fix up for this driver imo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-15 8:10 ` Daniel Vetter @ 2016-03-15 15:24 ` Alexey Brodkin 2016-03-15 15:59 ` Daniel Vetter 0 siblings, 1 reply; 18+ messages in thread From: Alexey Brodkin @ 2016-03-15 15:24 UTC (permalink / raw) To: daniel; +Cc: dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc Hi Daniel, On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > > > > > +static int arcpgu_atomic_commit(struct drm_device *dev, > > > > + struct drm_atomic_state *state, bool async) > > > > +{ > > > > + return drm_atomic_helper_commit(dev, state, false); > > > Note that this isn't really async if you ever get around to implement > > > fence support or vblank support. Just fyi. > > Ok but for now should I leave it as it is? > Yeah ok as-is, hence just fyi. Thanks. Just wanted to make sure we're on the same page :) > > > > > > > > > > > > > +static struct drm_driver arcpgu_drm_driver = { > > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > > > + DRIVER_ATOMIC, > > > > + .preclose = arcpgu_preclose, > > > > + .lastclose = arcpgu_lastclose, > > > > + .name = "drm-arcpgu", > > > > + .desc = "ARC PGU Controller", > > > > + .date = "20160219", > > > > + .major = 1, > > > > + .minor = 0, > > > > + .patchlevel = 0, > > > > + .fops = &arcpgu_drm_ops, > > > > + .load = arcpgu_load, > > > > + .unload = arcpgu_unload, > > > Load and unload hooks are deprecated (it's a classic midlayer mistake). > > > Please use drm_dev_alloc/register pairs directly instead, and put your > > > device setup code in-between. Similar for unloading. There's a bunch of > > > example drivers converted already. > > Ok I took "atmel-hlcdc" as example. > > And that's interesting. > > > > If I put my arcpgu_load() in between drm_dev_alloc() and > > drm_dev_register() then I'm getting this on the driver probe: > > ---------------------------------->8------------------------------- > > [drm] Initialized drm 1.1.0 20060810 > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > ------------[ cut here ]------------ > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > > Modules linked in: > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > > > Stack Trace: > > arc_unwind_core.constprop.1+0xa4/0x110 > > warn_slowpath_fmt+0x6e/0xfc > > kobject_add_internal+0x17c/0x498 > > kobject_add+0x98/0xe4 > > device_add+0xc6/0x734 > > device_create_with_groups+0x12a/0x144 > > drm_sysfs_connector_add+0x54/0xe8 > > arcpgu_drm_hdmi_init+0xd4/0x17c > > arcpgu_probe+0x138/0x24c > > platform_drv_probe+0x2e/0x6c > > really_probe+0x212/0x35c > > __driver_attach+0x90/0x94 > > bus_for_each_dev+0x46/0x80 > > bus_add_driver+0x14e/0x1b4 > > driver_register+0x64/0x108 > > do_one_initcall+0x86/0x194 > > kernel_init_freeable+0xf0/0x188 > > ---[ end trace c67166ad43ddcce2 ]--- > > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > > arcpgu: probe of e0017000.pgu failed with error -2 > > ---------------------------------->8------------------------------- > > > > But if I move arcpgu_load() after drm_dev_register() then everything > > starts properly and I may see HDMI screen works perfectly fine. > > > > Any thoughts? > Oops, yeah missed that detail. If you look at atmel it has a loop to > register all the drm connectors _after_ calling drm_dev_register(). > Totally forgot about that. Can you pls > - Extract a new drm_connector_register_all() function > (atmel_hlcdc_dc_connector_plug_all seems to be the best template), > including kerneldoc. > - Adjust kerneldoc of drm_dev_register() to mention > drm_connector_register_all() and that ordering constraint. > - Roll that helper out to all the drivers that currently hand-roll it (one > patch per driver). > > I know a bit of work but imo not too much, and by doing some small > refactoring every time someone stumbles over a drm pitfall we keep the > subsystem clean&easy to understand. You're up for this? Would be a prep > series, I'll happily review it to get it merged fast. Just a few weeks ago > I merged 20+ patches to make ->mode_fixup hooks optional and remove dummy > ones all over the subsystem, in other words: You'll have my full attention > ;-) Sure, I'm ready to pay that price :) Stay tuned and patches will follow. > > > > > > > > > > > > > + .dumb_create = drm_gem_cma_dumb_create, > > > > + .dumb_map_offset = drm_gem_cma_dumb_map_offset, > > > > + .dumb_destroy = drm_gem_dumb_destroy, > > > > + .get_vblank_counter = drm_vblank_no_hw_counter, > > > > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > > > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > > > > + .gem_free_object = 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 arcpgu_probe(struct platform_device *pdev) > > > > +{ > > > > + return drm_platform_init(&arcpgu_drm_driver, pdev); > > > ... or read the kerneldoc of this function, which also explains what you > > > should do ;-) > > Could you please point me to the relevant document? > > I wasn't able to find anything related from the first glance :( > The kerneldoc comment for drm_platform_init says it's deprecated, please > use drm_dev_alloc/register instead. Yeah I saw this. I was a bit confused with word "kerneldoc" thinking about .txt files in Documentation folder. > > > > > > > > > > > > > + > > > > +/* > > > > + * This function is the only reason to have a copy of drm_fbdev_cma_init() > > > > + * here in this driver. > > > > + * > > > > + * In its turn this mmap() is required to mark user-space page as non-cached > > > > + * because it is just a mirror or real hardware frame-buffer. > > > > + */ > > > > +static int arcpgu_mmap(struct fb_info *info, struct vm_area_struct *vma) > > > > +{ > > > > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > > > + return vm_iomap_memory(vma, info->fix.smem_start, info->fix.smem_len); > > > > +} > > > This looks very fishy, no other drm driver even bothers with providing an > > > fb_mmap hook. What exactly do you need this for? Assuming you've mmapped > > > your fbcon drm_framebuffer correctly for kernel access things should just > > > work ... > > Indeed for kernel there's non need to that hack. Kernel deals directly with HW > > frame-buffer area (that address we get from gem->paddr). And so every byte written gets > > picked up by PGU and is then rendered on the display. > > > > But when user-space opens /dev/fb0 and mmaps() it deals with memory pages which > > are by default (at least on ARC) marked as "cached". I.e. user-space application > > (I use that nice demo app https://github.com/qtproject/qt/blob/4.8/examples/qws/framebuffer/main.c) > > deals with frame-buffer via data cache. And that has 2 problems: > > [1] Since no explicit cache flush gets executed some data is left in data cache, > > i.e. some parts of the picture never reaches real PGU. > > See what happens on display - http://imgur.com/iAbnnx3 > > Those missing lines are exactly those 32-byte missing cache lines. > > [2] Even if we manage to flush data somehow massive amount of data that goes > > through data cache (let's sat 1080p@30Hz) will thrash it and as a result > > there will be no benefit for other cache users to use cache. > > > > So we fix it simply marking pages mapped to user-space apps as uncached > > that effectively routes all FB data directly to memry instead of polluting cache. > > > > Hopefully that explanation makes sense. > Still strange that you're the only one who needs this. I think best case > would be to provide some mmap helpers for fbdev which remap to dumb buffer > helpers or something similar. That way other drivers don't need to. Ok I took another look and now see what's missing. All other arches implement their own fb_pgprotect() in arch/X/include/asm/fb.h which does exactly what I need. So entire arcpgu_fbdev.c will be dumped in v4 :) -Alexey ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-15 15:24 ` Alexey Brodkin @ 2016-03-15 15:59 ` Daniel Vetter 2016-03-17 20:27 ` Alexey Brodkin 0 siblings, 1 reply; 18+ messages in thread From: Daniel Vetter @ 2016-03-15 15:59 UTC (permalink / raw) To: Alexey Brodkin Cc: daniel, dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc On Tue, Mar 15, 2016 at 03:24:46PM +0000, Alexey Brodkin wrote: > Hi Daniel, > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > > > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > > > > > > > +static int arcpgu_atomic_commit(struct drm_device *dev, > > > > > + struct drm_atomic_state *state, bool async) > > > > > +{ > > > > > + return drm_atomic_helper_commit(dev, state, false); > > > > Note that this isn't really async if you ever get around to implement > > > > fence support or vblank support. Just fyi. > > > Ok but for now should I leave it as it is? > > Yeah ok as-is, hence just fyi. > > Thanks. Just wanted to make sure we're on the same page :) > > > > > > > > > > > > > > > > > > +static struct drm_driver arcpgu_drm_driver = { > > > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > > > > + DRIVER_ATOMIC, > > > > > + .preclose = arcpgu_preclose, > > > > > + .lastclose = arcpgu_lastclose, > > > > > + .name = "drm-arcpgu", > > > > > + .desc = "ARC PGU Controller", > > > > > + .date = "20160219", > > > > > + .major = 1, > > > > > + .minor = 0, > > > > > + .patchlevel = 0, > > > > > + .fops = &arcpgu_drm_ops, > > > > > + .load = arcpgu_load, > > > > > + .unload = arcpgu_unload, > > > > Load and unload hooks are deprecated (it's a classic midlayer mistake). > > > > Please use drm_dev_alloc/register pairs directly instead, and put your > > > > device setup code in-between. Similar for unloading. There's a bunch of > > > > example drivers converted already. > > > Ok I took "atmel-hlcdc" as example. > > > And that's interesting. > > > > > > If I put my arcpgu_load() in between drm_dev_alloc() and > > > drm_dev_register() then I'm getting this on the driver probe: > > > ---------------------------------->8------------------------------- > > > [drm] Initialized drm 1.1.0 20060810 > > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > > ------------[ cut here ]------------ > > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() > > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > > > Modules linked in: > > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > > > > > Stack Trace: > > > arc_unwind_core.constprop.1+0xa4/0x110 > > > warn_slowpath_fmt+0x6e/0xfc > > > kobject_add_internal+0x17c/0x498 > > > kobject_add+0x98/0xe4 > > > device_add+0xc6/0x734 > > > device_create_with_groups+0x12a/0x144 > > > drm_sysfs_connector_add+0x54/0xe8 > > > arcpgu_drm_hdmi_init+0xd4/0x17c > > > arcpgu_probe+0x138/0x24c > > > platform_drv_probe+0x2e/0x6c > > > really_probe+0x212/0x35c > > > __driver_attach+0x90/0x94 > > > bus_for_each_dev+0x46/0x80 > > > bus_add_driver+0x14e/0x1b4 > > > driver_register+0x64/0x108 > > > do_one_initcall+0x86/0x194 > > > kernel_init_freeable+0xf0/0x188 > > > ---[ end trace c67166ad43ddcce2 ]--- > > > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > > > arcpgu: probe of e0017000.pgu failed with error -2 > > > ---------------------------------->8------------------------------- > > > > > > But if I move arcpgu_load() after drm_dev_register() then everything > > > starts properly and I may see HDMI screen works perfectly fine. > > > > > > Any thoughts? > > Oops, yeah missed that detail. If you look at atmel it has a loop to > > register all the drm connectors _after_ calling drm_dev_register(). > > Totally forgot about that. Can you pls > > - Extract a new drm_connector_register_all() function > > (atmel_hlcdc_dc_connector_plug_all seems to be the best template), > > including kerneldoc. > > - Adjust kerneldoc of drm_dev_register() to mention > > drm_connector_register_all() and that ordering constraint. > > - Roll that helper out to all the drivers that currently hand-roll it (one > > patch per driver). > > > > I know a bit of work but imo not too much, and by doing some small > > refactoring every time someone stumbles over a drm pitfall we keep the > > subsystem clean&easy to understand. You're up for this? Would be a prep > > series, I'll happily review it to get it merged fast. Just a few weeks ago > > I merged 20+ patches to make ->mode_fixup hooks optional and remove dummy > > ones all over the subsystem, in other words: You'll have my full attention > > ;-) > > Sure, I'm ready to pay that price :) > Stay tuned and patches will follow. Awesome, looking forward to your patches. > > > > > > > > > > > > > > > > > + .dumb_create = drm_gem_cma_dumb_create, > > > > > + .dumb_map_offset = drm_gem_cma_dumb_map_offset, > > > > > + .dumb_destroy = drm_gem_dumb_destroy, > > > > > + .get_vblank_counter = drm_vblank_no_hw_counter, > > > > > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > > > > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > > > > > + .gem_free_object = 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 arcpgu_probe(struct platform_device *pdev) > > > > > +{ > > > > > + return drm_platform_init(&arcpgu_drm_driver, pdev); > > > > ... or read the kerneldoc of this function, which also explains what you > > > > should do ;-) > > > Could you please point me to the relevant document? > > > I wasn't able to find anything related from the first glance :( > > The kerneldoc comment for drm_platform_init says it's deprecated, please > > use drm_dev_alloc/register instead. > > Yeah I saw this. I was a bit confused with word "kerneldoc" thinking about > .txt files in Documentation folder. kerneldoc is the kernel-specific tool to parse the specially formatted in-source comments starting with /**. Run $ make htmldocs and you should have the generated gpu docs in Documentation/DocBook/gpu/ which pulls all those together into something somewhat coherent. > > > > > > > > > > > > > > > > > > + > > > > > +/* > > > > > + * This function is the only reason to have a copy of drm_fbdev_cma_init() > > > > > + * here in this driver. > > > > > + * > > > > > + * In its turn this mmap() is required to mark user-space page as non-cached > > > > > + * because it is just a mirror or real hardware frame-buffer. > > > > > + */ > > > > > +static int arcpgu_mmap(struct fb_info *info, struct vm_area_struct *vma) > > > > > +{ > > > > > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > > > > + return vm_iomap_memory(vma, info->fix.smem_start, info->fix.smem_len); > > > > > +} > > > > This looks very fishy, no other drm driver even bothers with providing an > > > > fb_mmap hook. What exactly do you need this for? Assuming you've mmapped > > > > your fbcon drm_framebuffer correctly for kernel access things should just > > > > work ... > > > Indeed for kernel there's non need to that hack. Kernel deals directly with HW > > > frame-buffer area (that address we get from gem->paddr). And so every byte written gets > > > picked up by PGU and is then rendered on the display. > > > > > > But when user-space opens /dev/fb0 and mmaps() it deals with memory pages which > > > are by default (at least on ARC) marked as "cached". I.e. user-space application > > > (I use that nice demo app https://github.com/qtproject/qt/blob/4.8/examples/qws/framebuffer/main.c) > > > deals with frame-buffer via data cache. And that has 2 problems: > > > [1] Since no explicit cache flush gets executed some data is left in data cache, > > > i.e. some parts of the picture never reaches real PGU. > > > See what happens on display - http://imgur.com/iAbnnx3 > > > Those missing lines are exactly those 32-byte missing cache lines. > > > [2] Even if we manage to flush data somehow massive amount of data that goes > > > through data cache (let's sat 1080p@30Hz) will thrash it and as a result > > > there will be no benefit for other cache users to use cache. > > > > > > So we fix it simply marking pages mapped to user-space apps as uncached > > > that effectively routes all FB data directly to memry instead of polluting cache. > > > > > > Hopefully that explanation makes sense. > > Still strange that you're the only one who needs this. I think best case > > would be to provide some mmap helpers for fbdev which remap to dumb buffer > > helpers or something similar. That way other drivers don't need to. > > Ok I took another look and now see what's missing. > All other arches implement their own fb_pgprotect() in arch/X/include/asm/fb.h > which does exactly what I need. So entire arcpgu_fbdev.c will be dumped in v4 :) Ah, excellent. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-15 15:59 ` Daniel Vetter @ 2016-03-17 20:27 ` Alexey Brodkin 2016-03-18 8:02 ` Daniel Vetter 0 siblings, 1 reply; 18+ messages in thread From: Alexey Brodkin @ 2016-03-17 20:27 UTC (permalink / raw) To: daniel; +Cc: dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc Hi Daniel, On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote: > On Tue, Mar 15, 2016 at 03:24:46PM +0000, Alexey Brodkin wrote: > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > > > > > > > > > +static struct drm_driver arcpgu_drm_driver = { > > > > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > > > > > + DRIVER_ATOMIC, > > > > > > + .preclose = arcpgu_preclose, > > > > > > + .lastclose = arcpgu_lastclose, > > > > > > + .name = "drm-arcpgu", > > > > > > + .desc = "ARC PGU Controller", > > > > > > + .date = "20160219", > > > > > > + .major = 1, > > > > > > + .minor = 0, > > > > > > + .patchlevel = 0, > > > > > > + .fops = &arcpgu_drm_ops, > > > > > > + .load = arcpgu_load, > > > > > > + .unload = arcpgu_unload, > > > > > Load and unload hooks are deprecated (it's a classic midlayer mistake). > > > > > Please use drm_dev_alloc/register pairs directly instead, and put your > > > > > device setup code in-between. Similar for unloading. There's a bunch of > > > > > example drivers converted already. > > > > Ok I took "atmel-hlcdc" as example. > > > > And that's interesting. > > > > > > > > If I put my arcpgu_load() in between drm_dev_alloc() and > > > > drm_dev_register() then I'm getting this on the driver probe: > > > > ---------------------------------->8------------------------------- > > > > [drm] Initialized drm 1.1.0 20060810 > > > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > > > ------------[ cut here ]------------ > > > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() > > > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > > > > Modules linked in: > > > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > > > > > > > Stack Trace: > > > > arc_unwind_core.constprop.1+0xa4/0x110 > > > > warn_slowpath_fmt+0x6e/0xfc > > > > kobject_add_internal+0x17c/0x498 > > > > kobject_add+0x98/0xe4 > > > > device_add+0xc6/0x734 > > > > device_create_with_groups+0x12a/0x144 > > > > drm_sysfs_connector_add+0x54/0xe8 > > > > arcpgu_drm_hdmi_init+0xd4/0x17c > > > > arcpgu_probe+0x138/0x24c > > > > platform_drv_probe+0x2e/0x6c > > > > really_probe+0x212/0x35c > > > > __driver_attach+0x90/0x94 > > > > bus_for_each_dev+0x46/0x80 > > > > bus_add_driver+0x14e/0x1b4 > > > > driver_register+0x64/0x108 > > > > do_one_initcall+0x86/0x194 > > > > kernel_init_freeable+0xf0/0x188 > > > > ---[ end trace c67166ad43ddcce2 ]--- > > > > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > > > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > > > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > > > > arcpgu: probe of e0017000.pgu failed with error -2 > > > > ---------------------------------->8------------------------------- > > > > > > > > But if I move arcpgu_load() after drm_dev_register() then everything > > > > starts properly and I may see HDMI screen works perfectly fine. > > > > > > > > Any thoughts? > > > Oops, yeah missed that detail. If you look at atmel it has a loop to > > > register all the drm connectors _after_ calling drm_dev_register(). > > > Totally forgot about that. Can you pls > > > - Extract a new drm_connector_register_all() function > > > (atmel_hlcdc_dc_connector_plug_all seems to be the best template), > > > including kerneldoc. > > > - Adjust kerneldoc of drm_dev_register() to mention > > > drm_connector_register_all() and that ordering constraint. > > > - Roll that helper out to all the drivers that currently hand-roll it (one > > > patch per driver). > > > > > > I know a bit of work but imo not too much, and by doing some small > > > refactoring every time someone stumbles over a drm pitfall we keep the > > > subsystem clean&easy to understand. You're up for this? Would be a prep > > > series, I'll happily review it to get it merged fast. Just a few weeks ago > > > I merged 20+ patches to make ->mode_fixup hooks optional and remove dummy > > > ones all over the subsystem, in other words: You'll have my full attention > > > ;-) > > Sure, I'm ready to pay that price :) > > Stay tuned and patches will follow. > Awesome, looking forward to your patches. Sorry it took longer for me to finally put my hands on that work but anyways. I'm looking now at how drivers use existing drm_connector_unplug_all() and their implementation of what would be drm_connector_plug_all() and see in some implementations people wraps both helpers with mutex_{lock|unlock}(&dev->mode_config.mutex). But not everybody does this. So essentially my questions are: [1] If it's necessary to get hold of that mutex before execution of either helper? [2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock in helpers itself, right? -Alexey ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-17 20:27 ` Alexey Brodkin @ 2016-03-18 8:02 ` Daniel Vetter 2016-03-18 8:11 ` Alexey Brodkin 0 siblings, 1 reply; 18+ messages in thread From: Daniel Vetter @ 2016-03-18 8:02 UTC (permalink / raw) To: Alexey Brodkin Cc: daniel, dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc On Thu, Mar 17, 2016 at 08:27:10PM +0000, Alexey Brodkin wrote: > Hi Daniel, > > On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote: > > On Tue, Mar 15, 2016 at 03:24:46PM +0000, Alexey Brodkin wrote: > > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > > > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > > > > > > > > > > > +static struct drm_driver arcpgu_drm_driver = { > > > > > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > > > > > > + DRIVER_ATOMIC, > > > > > > > + .preclose = arcpgu_preclose, > > > > > > > + .lastclose = arcpgu_lastclose, > > > > > > > + .name = "drm-arcpgu", > > > > > > > + .desc = "ARC PGU Controller", > > > > > > > + .date = "20160219", > > > > > > > + .major = 1, > > > > > > > + .minor = 0, > > > > > > > + .patchlevel = 0, > > > > > > > + .fops = &arcpgu_drm_ops, > > > > > > > + .load = arcpgu_load, > > > > > > > + .unload = arcpgu_unload, > > > > > > Load and unload hooks are deprecated (it's a classic midlayer mistake). > > > > > > Please use drm_dev_alloc/register pairs directly instead, and put your > > > > > > device setup code in-between. Similar for unloading. There's a bunch of > > > > > > example drivers converted already. > > > > > Ok I took "atmel-hlcdc" as example. > > > > > And that's interesting. > > > > > > > > > > If I put my arcpgu_load() in between drm_dev_alloc() and > > > > > drm_dev_register() then I'm getting this on the driver probe: > > > > > ---------------------------------->8------------------------------- > > > > > [drm] Initialized drm 1.1.0 20060810 > > > > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > > > > ------------[ cut here ]------------ > > > > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() > > > > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > > > > > Modules linked in: > > > > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > > > > > > > > > Stack Trace: > > > > > arc_unwind_core.constprop.1+0xa4/0x110 > > > > > warn_slowpath_fmt+0x6e/0xfc > > > > > kobject_add_internal+0x17c/0x498 > > > > > kobject_add+0x98/0xe4 > > > > > device_add+0xc6/0x734 > > > > > device_create_with_groups+0x12a/0x144 > > > > > drm_sysfs_connector_add+0x54/0xe8 > > > > > arcpgu_drm_hdmi_init+0xd4/0x17c > > > > > arcpgu_probe+0x138/0x24c > > > > > platform_drv_probe+0x2e/0x6c > > > > > really_probe+0x212/0x35c > > > > > __driver_attach+0x90/0x94 > > > > > bus_for_each_dev+0x46/0x80 > > > > > bus_add_driver+0x14e/0x1b4 > > > > > driver_register+0x64/0x108 > > > > > do_one_initcall+0x86/0x194 > > > > > kernel_init_freeable+0xf0/0x188 > > > > > ---[ end trace c67166ad43ddcce2 ]--- > > > > > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > > > > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > > > > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > > > > > arcpgu: probe of e0017000.pgu failed with error -2 > > > > > ---------------------------------->8------------------------------- > > > > > > > > > > But if I move arcpgu_load() after drm_dev_register() then everything > > > > > starts properly and I may see HDMI screen works perfectly fine. > > > > > > > > > > Any thoughts? > > > > Oops, yeah missed that detail. If you look at atmel it has a loop to > > > > register all the drm connectors _after_ calling drm_dev_register(). > > > > Totally forgot about that. Can you pls > > > > - Extract a new drm_connector_register_all() function > > > > (atmel_hlcdc_dc_connector_plug_all seems to be the best template), > > > > including kerneldoc. > > > > - Adjust kerneldoc of drm_dev_register() to mention > > > > drm_connector_register_all() and that ordering constraint. > > > > - Roll that helper out to all the drivers that currently hand-roll it (one > > > > patch per driver). > > > > > > > > I know a bit of work but imo not too much, and by doing some small > > > > refactoring every time someone stumbles over a drm pitfall we keep the > > > > subsystem clean&easy to understand. You're up for this? Would be a prep > > > > series, I'll happily review it to get it merged fast. Just a few weeks ago > > > > I merged 20+ patches to make ->mode_fixup hooks optional and remove dummy > > > > ones all over the subsystem, in other words: You'll have my full attention > > > > ;-) > > > Sure, I'm ready to pay that price :) > > > Stay tuned and patches will follow. > > Awesome, looking forward to your patches. > > Sorry it took longer for me to finally put my hands on that work but anyways. > > I'm looking now at how drivers use existing drm_connector_unplug_all() and > their implementation of what would be drm_connector_plug_all() and see > in some implementations people wraps both helpers with > mutex_{lock|unlock}(&dev->mode_config.mutex). But not everybody does this. > > So essentially my questions are: > [1] If it's necessary to get hold of that mutex before execution of either helper? In plug_all I think so, unplug_all has a FIXME comment about how locking against sysfs is horrible and it's all going to blow up. But we did recently change the connector sysfs files, so maybe that's fixed now. Not sure. > [2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock > in helpers itself, right? Yeah, locking in the helper makes imo sense. Aside: I'd vote for register_all/unregister_all for consistency with drm_connector_register. register/unregister has a very clear meaning of "publish the object to userspace/other kernel subsystems". plug/unplug is confusing in DRM because of hotplug. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-18 8:02 ` Daniel Vetter @ 2016-03-18 8:11 ` Alexey Brodkin 2016-03-18 17:23 ` Daniel Vetter 0 siblings, 1 reply; 18+ messages in thread From: Alexey Brodkin @ 2016-03-18 8:11 UTC (permalink / raw) To: daniel; +Cc: dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc Hi Daniel, On Fri, 2016-03-18 at 09:02 +0100, Daniel Vetter wrote: > On Thu, Mar 17, 2016 at 08:27:10PM +0000, Alexey Brodkin wrote: > > > > Hi Daniel, > > > > On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote: > > > > > > On Tue, Mar 15, 2016 at 03:24:46PM +0000, Alexey Brodkin wrote: > > > > > > > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > > > > > > > > > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > > > > > > > > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > > > > > > > > > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > > > > > > > > > > > > > > > > > > > > > +static struct drm_driver arcpgu_drm_driver = { > > > > > > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > > > > > > > + DRIVER_ATOMIC, > > > > > > > > + .preclose = arcpgu_preclose, > > > > > > > > + .lastclose = arcpgu_lastclose, > > > > > > > > + .name = "drm-arcpgu", > > > > > > > > + .desc = "ARC PGU Controller", > > > > > > > > + .date = "20160219", > > > > > > > > + .major = 1, > > > > > > > > + .minor = 0, > > > > > > > > + .patchlevel = 0, > > > > > > > > + .fops = &arcpgu_drm_ops, > > > > > > > > + .load = arcpgu_load, > > > > > > > > + .unload = arcpgu_unload, > > > > > > > Load and unload hooks are deprecated (it's a classic midlayer mistake). > > > > > > > Please use drm_dev_alloc/register pairs directly instead, and put your > > > > > > > device setup code in-between. Similar for unloading. There's a bunch of > > > > > > > example drivers converted already. > > > > > > Ok I took "atmel-hlcdc" as example. > > > > > > And that's interesting. > > > > > > > > > > > > If I put my arcpgu_load() in between drm_dev_alloc() and > > > > > > drm_dev_register() then I'm getting this on the driver probe: > > > > > > ---------------------------------->8------------------------------- > > > > > > [drm] Initialized drm 1.1.0 20060810 > > > > > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > > > > > ------------[ cut here ]------------ > > > > > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() > > > > > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > > > > > > Modules linked in: > > > > > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > > > > > > > > > > > Stack Trace: > > > > > > arc_unwind_core.constprop.1+0xa4/0x110 > > > > > > warn_slowpath_fmt+0x6e/0xfc > > > > > > kobject_add_internal+0x17c/0x498 > > > > > > kobject_add+0x98/0xe4 > > > > > > device_add+0xc6/0x734 > > > > > > device_create_with_groups+0x12a/0x144 > > > > > > drm_sysfs_connector_add+0x54/0xe8 > > > > > > arcpgu_drm_hdmi_init+0xd4/0x17c > > > > > > arcpgu_probe+0x138/0x24c > > > > > > platform_drv_probe+0x2e/0x6c > > > > > > really_probe+0x212/0x35c > > > > > > __driver_attach+0x90/0x94 > > > > > > bus_for_each_dev+0x46/0x80 > > > > > > bus_add_driver+0x14e/0x1b4 > > > > > > driver_register+0x64/0x108 > > > > > > do_one_initcall+0x86/0x194 > > > > > > kernel_init_freeable+0xf0/0x188 > > > > > > ---[ end trace c67166ad43ddcce2 ]--- > > > > > > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > > > > > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > > > > > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > > > > > > arcpgu: probe of e0017000.pgu failed with error -2 > > > > > > ---------------------------------->8------------------------------- > > > > > > > > > > > > But if I move arcpgu_load() after drm_dev_register() then everything > > > > > > starts properly and I may see HDMI screen works perfectly fine. > > > > > > > > > > > > Any thoughts? > > > > > Oops, yeah missed that detail. If you look at atmel it has a loop to > > > > > register all the drm connectors _after_ calling drm_dev_register(). > > > > > Totally forgot about that. Can you pls > > > > > - Extract a new drm_connector_register_all() function > > > > > (atmel_hlcdc_dc_connector_plug_all seems to be the best template), > > > > > including kerneldoc. > > > > > - Adjust kerneldoc of drm_dev_register() to mention > > > > > drm_connector_register_all() and that ordering constraint. > > > > > - Roll that helper out to all the drivers that currently hand-roll it (one > > > > > patch per driver). > > > > > > > > > > I know a bit of work but imo not too much, and by doing some small > > > > > refactoring every time someone stumbles over a drm pitfall we keep the > > > > > subsystem clean&easy to understand. You're up for this? Would be a prep > > > > > series, I'll happily review it to get it merged fast. Just a few weeks ago > > > > > I merged 20+ patches to make ->mode_fixup hooks optional and remove dummy > > > > > ones all over the subsystem, in other words: You'll have my full attention > > > > > ;-) > > > > Sure, I'm ready to pay that price :) > > > > Stay tuned and patches will follow. > > > Awesome, looking forward to your patches. > > Sorry it took longer for me to finally put my hands on that work but anyways. > > > > I'm looking now at how drivers use existing drm_connector_unplug_all() and > > their implementation of what would be drm_connector_plug_all() and see > > in some implementations people wraps both helpers with > > mutex_{lock|unlock}(&dev->mode_config.mutex). But not everybody does this. > > > > So essentially my questions are: > > [1] If it's necessary to get hold of that mutex before execution of either helper? > In plug_all I think so, unplug_all has a FIXME comment about how locking > against sysfs is horrible and it's all going to blow up. But we did > recently change the connector sysfs files, so maybe that's fixed now. Not > sure. > > > [2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock > > in helpers itself, right? > Yeah, locking in the helper makes imo sense. Hm, now I'm even more confused :) Above you're not sure if this locking is safe (at least on unplug) but here you say it makes sense (not mentioning the case - both helpers or only on "plugging"). I may indeed try to embed mutex locks in both helpers but what if it works for me but others will get broken stuff? > Aside: I'd vote for > register_all/unregister_all for consistency with drm_connector_register. > register/unregister has a very clear meaning of "publish the object to > userspace/other kernel subsystems". plug/unplug is confusing in DRM > because of hotplug. Well but what should happen to existing unplug() then? Should I rename it to unregister_all() in the same change? -Alexey ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller 2016-03-18 8:11 ` Alexey Brodkin @ 2016-03-18 17:23 ` Daniel Vetter 0 siblings, 0 replies; 18+ messages in thread From: Daniel Vetter @ 2016-03-18 17:23 UTC (permalink / raw) To: Alexey Brodkin Cc: daniel, dri-devel, linux-kernel, Jose.Abreu, linux-snps-arc On Fri, Mar 18, 2016 at 08:11:49AM +0000, Alexey Brodkin wrote: > Hi Daniel, > > On Fri, 2016-03-18 at 09:02 +0100, Daniel Vetter wrote: > > On Thu, Mar 17, 2016 at 08:27:10PM +0000, Alexey Brodkin wrote: > > > > > > Hi Daniel, > > > > > > On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote: > > > > > > > > On Tue, Mar 15, 2016 at 03:24:46PM +0000, Alexey Brodkin wrote: > > > > > > > > > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > > > > > > > > > > > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > > > > > > > > > > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > > > > > > > > > > > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > +static struct drm_driver arcpgu_drm_driver = { > > > > > > > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | > > > > > > > > > + DRIVER_ATOMIC, > > > > > > > > > + .preclose = arcpgu_preclose, > > > > > > > > > + .lastclose = arcpgu_lastclose, > > > > > > > > > + .name = "drm-arcpgu", > > > > > > > > > + .desc = "ARC PGU Controller", > > > > > > > > > + .date = "20160219", > > > > > > > > > + .major = 1, > > > > > > > > > + .minor = 0, > > > > > > > > > + .patchlevel = 0, > > > > > > > > > + .fops = &arcpgu_drm_ops, > > > > > > > > > + .load = arcpgu_load, > > > > > > > > > + .unload = arcpgu_unload, > > > > > > > > Load and unload hooks are deprecated (it's a classic midlayer mistake). > > > > > > > > Please use drm_dev_alloc/register pairs directly instead, and put your > > > > > > > > device setup code in-between. Similar for unloading. There's a bunch of > > > > > > > > example drivers converted already. > > > > > > > Ok I took "atmel-hlcdc" as example. > > > > > > > And that's interesting. > > > > > > > > > > > > > > If I put my arcpgu_load() in between drm_dev_alloc() and > > > > > > > drm_dev_register() then I'm getting this on the driver probe: > > > > > > > ---------------------------------->8------------------------------- > > > > > > > [drm] Initialized drm 1.1.0 20060810 > > > > > > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > > > > > > ------------[ cut here ]------------ > > > > > > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x498() > > > > > > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > > > > > > > Modules linked in: > > > > > > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > > > > > > > > > > > > > Stack Trace: > > > > > > > arc_unwind_core.constprop.1+0xa4/0x110 > > > > > > > warn_slowpath_fmt+0x6e/0xfc > > > > > > > kobject_add_internal+0x17c/0x498 > > > > > > > kobject_add+0x98/0xe4 > > > > > > > device_add+0xc6/0x734 > > > > > > > device_create_with_groups+0x12a/0x144 > > > > > > > drm_sysfs_connector_add+0x54/0xe8 > > > > > > > arcpgu_drm_hdmi_init+0xd4/0x17c > > > > > > > arcpgu_probe+0x138/0x24c > > > > > > > platform_drv_probe+0x2e/0x6c > > > > > > > really_probe+0x212/0x35c > > > > > > > __driver_attach+0x90/0x94 > > > > > > > bus_for_each_dev+0x46/0x80 > > > > > > > bus_add_driver+0x14e/0x1b4 > > > > > > > driver_register+0x64/0x108 > > > > > > > do_one_initcall+0x86/0x194 > > > > > > > kernel_init_freeable+0xf0/0x188 > > > > > > > ---[ end trace c67166ad43ddcce2 ]--- > > > > > > > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > > > > > > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > > > > > > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > > > > > > > arcpgu: probe of e0017000.pgu failed with error -2 > > > > > > > ---------------------------------->8------------------------------- > > > > > > > > > > > > > > But if I move arcpgu_load() after drm_dev_register() then everything > > > > > > > starts properly and I may see HDMI screen works perfectly fine. > > > > > > > > > > > > > > Any thoughts? > > > > > > Oops, yeah missed that detail. If you look at atmel it has a loop to > > > > > > register all the drm connectors _after_ calling drm_dev_register(). > > > > > > Totally forgot about that. Can you pls > > > > > > - Extract a new drm_connector_register_all() function > > > > > > (atmel_hlcdc_dc_connector_plug_all seems to be the best template), > > > > > > including kerneldoc. > > > > > > - Adjust kerneldoc of drm_dev_register() to mention > > > > > > drm_connector_register_all() and that ordering constraint. > > > > > > - Roll that helper out to all the drivers that currently hand-roll it (one > > > > > > patch per driver). > > > > > > > > > > > > I know a bit of work but imo not too much, and by doing some small > > > > > > refactoring every time someone stumbles over a drm pitfall we keep the > > > > > > subsystem clean&easy to understand. You're up for this? Would be a prep > > > > > > series, I'll happily review it to get it merged fast. Just a few weeks ago > > > > > > I merged 20+ patches to make ->mode_fixup hooks optional and remove dummy > > > > > > ones all over the subsystem, in other words: You'll have my full attention > > > > > > ;-) > > > > > Sure, I'm ready to pay that price :) > > > > > Stay tuned and patches will follow. > > > > Awesome, looking forward to your patches. > > > Sorry it took longer for me to finally put my hands on that work but anyways. > > > > > > I'm looking now at how drivers use existing drm_connector_unplug_all() and > > > their implementation of what would be drm_connector_plug_all() and see > > > in some implementations people wraps both helpers with > > > mutex_{lock|unlock}(&dev->mode_config.mutex). But not everybody does this. > > > > > > So essentially my questions are: > > > [1] If it's necessary to get hold of that mutex before execution of either helper? > > In plug_all I think so, unplug_all has a FIXME comment about how locking > > against sysfs is horrible and it's all going to blow up. But we did > > recently change the connector sysfs files, so maybe that's fixed now. Not > > sure. > > > > > [2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock > > > in helpers itself, right? > > Yeah, locking in the helper makes imo sense. > > Hm, now I'm even more confused :) > Above you're not sure if this locking is safe (at least on unplug) but here you say > it makes sense (not mentioning the case - both helpers or only on "plugging"). > > I may indeed try to embed mutex locks in both helpers but what if it works for me but > others will get broken stuff? Sorry, I meant only embed it it for register_all. And keep the FIXME comment for unregister_all (well unplug_all as it's called still). Fixing up sysfs vs. mode_config.mutex is a lot more work than just this. > > Aside: I'd vote for > > register_all/unregister_all for consistency with drm_connector_register. > > register/unregister has a very clear meaning of "publish the object to > > userspace/other kernel subsystems". plug/unplug is confusing in DRM > > because of hotplug. > > Well but what should happen to existing unplug() then? > Should I rename it to unregister_all() in the same change? Yes, that's what I'd do. I think that's clearer, if we go with a strict distinction between register/unregister and hotplug handling. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4 v3] drm: Add DT bindings documentation for ARC PGU display controller 2016-03-11 15:42 [PATCH 0/4 v3] drm: Add support of ARC PGU display controller Alexey Brodkin 2016-03-11 15:42 ` [PATCH 1/4 " Alexey Brodkin @ 2016-03-11 15:42 ` Alexey Brodkin 2016-03-21 13:04 ` Rob Herring 2016-03-11 15:42 ` [PATCH 3/4 v3] arc: axs10x - add support of ARC PGU Alexey Brodkin 2016-03-11 15:42 ` [PATCH 4/4 v3] MAINTAINERS: Add maintainer for ARC PGU display controller Alexey Brodkin 3 siblings, 1 reply; 18+ messages in thread From: Alexey Brodkin @ 2016-03-11 15:42 UTC (permalink / raw) To: linux-kernel Cc: linux-snps-arc, Alexey Brodkin, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, David Airlie, dri-devel, devicetree This add DT bindings documentation for ARC PGU display controller. Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: Kumar Gala <galak@codeaurora.org> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org Cc: devicetree@vger.kernel.org Cc: linux-snps-arc@lists.infradead.org --- Changes v2 -> v3: * Reverted back to initial larger version of example with minor fixes (thanks Rob for spotting those). Changes v1 -> v2: * Removed everything except PGU node itself. .../devicetree/bindings/display/snps,arcpgu.txt | 72 ++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/snps,arcpgu.txt diff --git a/Documentation/devicetree/bindings/display/snps,arcpgu.txt b/Documentation/devicetree/bindings/display/snps,arcpgu.txt new file mode 100644 index 0000000..b130924 --- /dev/null +++ b/Documentation/devicetree/bindings/display/snps,arcpgu.txt @@ -0,0 +1,72 @@ +ARC PGU + +This is a display controller found on several development boards produced +by Synopsys. The ARC PGU is an RGB streamer that reads the data from a +framebuffer and sends it to a single digital encoder (usually HDMI). + +Required properties: + - compatible: "snps,arcpgu" + - reg: Physical base address and length of the controller's registers. + - clocks: A list of phandle + clock-specifier pairs, one for each + entry in 'clock-names'. + - clock-names: A list of clock names. For ARC PGU it should contain: + - "pxlclk" for the clock feeding the output PLL of the controller. + - encoder-slave: Phandle of encoder chip. + +Required sub-nodes: + - port: The PGU connection to an encoder chip. + +Example: + +/ { + ... + + pgu@XXXXXXXX { + compatible = "snps,arcpgu"; + reg = <0xXXXXXXXX 0x400>; + clocks = <&clock_node>; + clock-names = "pxlclk"; + encoder-slave = <&encoder_node>; + + port { + pgu_output: endpoint { + remote-endpoint = <&hdmi_enc_input>; + }; + }; + }; + + /* HDMI encoder on I2C bus */ + i2c@XXXXXXXX { + compatible = "..."; + + encoder_node:encoder_node@0xXX{ + compatible="..."; + + ports { + port@0 { + reg = <0>; + hdmi_enc_input:endpoint { + remote-endpoint = <&pgu_output>; + }; + }; + + port@1 { + reg = <1>; + hdmi_enc_output:endpoint { + remote-endpoint = <&hdmi_connector_in>; + }; + }; + }; + }; + } + + hdmi0: connector { + compatible = "hdmi-connector"; + + port { + hdmi_connector_in: endpoint { + remote-endpoint = <&hdmi_enc_output>; + }; + }; + }; +}; -- 2.5.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4 v3] drm: Add DT bindings documentation for ARC PGU display controller 2016-03-11 15:42 ` [PATCH 2/4 v3] drm: Add DT bindings documentation for " Alexey Brodkin @ 2016-03-21 13:04 ` Rob Herring 2016-03-24 14:57 ` Alexey Brodkin 0 siblings, 1 reply; 18+ messages in thread From: Rob Herring @ 2016-03-21 13:04 UTC (permalink / raw) To: Alexey Brodkin Cc: linux-kernel, linux-snps-arc, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, David Airlie, dri-devel, devicetree On Fri, Mar 11, 2016 at 06:42:37PM +0300, Alexey Brodkin wrote: > This add DT bindings documentation for ARC PGU display controller. > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Pawel Moll <pawel.moll@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> > Cc: Kumar Gala <galak@codeaurora.org> > Cc: David Airlie <airlied@linux.ie> > Cc: dri-devel@lists.freedesktop.org > Cc: devicetree@vger.kernel.org > Cc: linux-snps-arc@lists.infradead.org > --- > > Changes v2 -> v3: > * Reverted back to initial larger version of example > with minor fixes (thanks Rob for spotting those). > > Changes v1 -> v2: > * Removed everything except PGU node itself. > > .../devicetree/bindings/display/snps,arcpgu.txt | 72 ++++++++++++++++++++++ > 1 file changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/snps,arcpgu.txt > > diff --git a/Documentation/devicetree/bindings/display/snps,arcpgu.txt b/Documentation/devicetree/bindings/display/snps,arcpgu.txt > new file mode 100644 > index 0000000..b130924 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/snps,arcpgu.txt > @@ -0,0 +1,72 @@ > +ARC PGU > + > +This is a display controller found on several development boards produced > +by Synopsys. The ARC PGU is an RGB streamer that reads the data from a > +framebuffer and sends it to a single digital encoder (usually HDMI). > + > +Required properties: > + - compatible: "snps,arcpgu" > + - reg: Physical base address and length of the controller's registers. > + - clocks: A list of phandle + clock-specifier pairs, one for each > + entry in 'clock-names'. > + - clock-names: A list of clock names. For ARC PGU it should contain: > + - "pxlclk" for the clock feeding the output PLL of the controller. > + - encoder-slave: Phandle of encoder chip. Drop this as discussed. > + > +Required sub-nodes: > + - port: The PGU connection to an encoder chip. > + > +Example: > + > +/ { > + ... > + > + pgu@XXXXXXXX { > + compatible = "snps,arcpgu"; > + reg = <0xXXXXXXXX 0x400>; > + clocks = <&clock_node>; > + clock-names = "pxlclk"; > + encoder-slave = <&encoder_node>; > + > + port { > + pgu_output: endpoint { > + remote-endpoint = <&hdmi_enc_input>; > + }; > + }; > + }; > + > + /* HDMI encoder on I2C bus */ > + i2c@XXXXXXXX { > + compatible = "..."; > + > + encoder_node:encoder_node@0xXX{ Don't use underscores in node names. Just "hdmi@xx" > + compatible="..."; > + > + ports { > + port@0 { > + reg = <0>; > + hdmi_enc_input:endpoint { > + remote-endpoint = <&pgu_output>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + hdmi_enc_output:endpoint { > + remote-endpoint = <&hdmi_connector_in>; > + }; > + }; > + }; > + }; > + } > + > + hdmi0: connector { > + compatible = "hdmi-connector"; > + > + port { > + hdmi_connector_in: endpoint { > + remote-endpoint = <&hdmi_enc_output>; > + }; > + }; > + }; > +}; > -- > 2.5.0 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4 v3] drm: Add DT bindings documentation for ARC PGU display controller 2016-03-21 13:04 ` Rob Herring @ 2016-03-24 14:57 ` Alexey Brodkin 0 siblings, 0 replies; 18+ messages in thread From: Alexey Brodkin @ 2016-03-24 14:57 UTC (permalink / raw) To: robh Cc: linux-kernel, pawel.moll, devicetree, linux-snps-arc, mark.rutland, galak, ijc+devicetree, dri-devel, airlied Hi Rob, On Mon, 2016-03-21 at 08:04 -0500, Rob Herring wrote: > On Fri, Mar 11, 2016 at 06:42:37PM +0300, Alexey Brodkin wrote: > > > > This add DT bindings documentation for ARC PGU display controller. > > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Pawel Moll <pawel.moll@arm.com> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> > > Cc: Kumar Gala <galak@codeaurora.org> > > Cc: David Airlie <airlied@linux.ie> > > Cc: dri-devel@lists.freedesktop.org > > Cc: devicetree@vger.kernel.org > > Cc: linux-snps-arc@lists.infradead.org > > --- > > > > Changes v2 -> v3: > > * Reverted back to initial larger version of example > > with minor fixes (thanks Rob for spotting those). > > > > Changes v1 -> v2: > > * Removed everything except PGU node itself. > > > > .../devicetree/bindings/display/snps,arcpgu.txt | 72 ++++++++++++++++++++++ > > 1 file changed, 72 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/display/snps,arcpgu.txt > > > > diff --git a/Documentation/devicetree/bindings/display/snps,arcpgu.txt > > b/Documentation/devicetree/bindings/display/snps,arcpgu.txt > > new file mode 100644 > > index 0000000..b130924 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/snps,arcpgu.txt > > @@ -0,0 +1,72 @@ > > +ARC PGU > > + > > +This is a display controller found on several development boards produced > > +by Synopsys. The ARC PGU is an RGB streamer that reads the data from a > > +framebuffer and sends it to a single digital encoder (usually HDMI). > > + > > +Required properties: > > + - compatible: "snps,arcpgu" > > + - reg: Physical base address and length of the controller's registers. > > + - clocks: A list of phandle + clock-specifier pairs, one for each > > + entry in 'clock-names'. > > + - clock-names: A list of clock names. For ARC PGU it should contain: > > + - "pxlclk" for the clock feeding the output PLL of the controller. > > + - encoder-slave: Phandle of encoder chip. > Drop this as discussed. Ok I'll do that. But what about my example below? Do I need to remove encoder node from it as well? > > > > + > > +Required sub-nodes: > > + - port: The PGU connection to an encoder chip. > > + > > +Example: > > + > > +/ { > > + ... > > + > > + pgu@XXXXXXXX { > > + compatible = "snps,arcpgu"; > > + reg = <0xXXXXXXXX 0x400>; > > + clocks = <&clock_node>; > > + clock-names = "pxlclk"; > > + encoder-slave = <&encoder_node>; > > + > > + port { > > + pgu_output: endpoint { > > + remote-endpoint = <&hdmi_enc_input>; > > + }; > > + }; > > + }; > > + > > + /* HDMI encoder on I2C bus */ > > + i2c@XXXXXXXX { > > + compatible = "..."; > > + > > + encoder_node:encoder_node@0xXX{ > Don't use underscores in node names. Just "hdmi@xx" Ok still I may keep label "encoder_node" here, right? -Alexey ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/4 v3] arc: axs10x - add support of ARC PGU 2016-03-11 15:42 [PATCH 0/4 v3] drm: Add support of ARC PGU display controller Alexey Brodkin 2016-03-11 15:42 ` [PATCH 1/4 " Alexey Brodkin 2016-03-11 15:42 ` [PATCH 2/4 v3] drm: Add DT bindings documentation for " Alexey Brodkin @ 2016-03-11 15:42 ` Alexey Brodkin 2016-03-11 15:42 ` [PATCH 4/4 v3] MAINTAINERS: Add maintainer for ARC PGU display controller Alexey Brodkin 3 siblings, 0 replies; 18+ messages in thread From: Alexey Brodkin @ 2016-03-11 15:42 UTC (permalink / raw) To: linux-kernel Cc: linux-snps-arc, Alexey Brodkin, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Vineet Gupta, David Airlie, dri-devel, devicetree Synopsys DesignWare ARC SDP boards sport ARC SDP display controller attached to ADV7511 HDMI encoder. That change adds desctiption of both ARC PGU and ADV7511 in ARC SDP'd base-board Device Tree. Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: Kumar Gala <galak@codeaurora.org> Cc: Vineet Gupta <vgupta@synopsys.com> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org Cc: devicetree@vger.kernel.org Cc: linux-snps-arc@lists.infradead.org --- No changes v2 -> v3. Changes v1 -> v2: * Added missing "pxlclk" clock in axs10x_mb.dtsi arch/arc/boot/dts/axs10x_mb.dtsi | 61 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi index 44a578c..6e639c9 100644 --- a/arch/arc/boot/dts/axs10x_mb.dtsi +++ b/arch/arc/boot/dts/axs10x_mb.dtsi @@ -34,6 +34,12 @@ clock-frequency = <50000000>; #clock-cells = <0>; }; + + pguclk: pguclk { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <74440000>; + }; }; ethernet@0x18000 { @@ -147,6 +153,37 @@ clocks = <&i2cclk>; interrupts = <16>; + adv7511:adv7511@0x39{ + compatible="adi,adv7511"; + reg = <0x39>; + interrupts = <23>; + adi,input-depth = <8>; + adi,input-colorspace = "rgb"; + adi,input-clock = "1x"; + adi,clock-delay = <0x03>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + /* RGB/YUV input */ + port@0 { + reg = <0>; + adv7511_input:endpoint { + remote-endpoint = <&pgu_output>; + }; + }; + + /* HDMI output */ + port@1 { + reg = <1>; + adv7511_output: endpoint { + remote-endpoint = <&hdmi_connector_in>; + }; + }; + }; + }; + eeprom@0x54{ compatible = "24c01"; reg = <0x54>; @@ -160,6 +197,16 @@ }; }; + hdmi0: connector { + compatible = "hdmi-connector"; + type = "a"; + port { + hdmi_connector_in: endpoint { + remote-endpoint = <&adv7511_output>; + }; + }; + }; + gpio0:gpio@13000 { compatible = "snps,dw-apb-gpio"; reg = <0x13000 0x1000>; @@ -221,5 +268,19 @@ reg = <2>; }; }; + + pgu@0x17000 { + compatible = "snps,arcpgu"; + reg = <0x17000 0x400>; + encoder-slave = <&adv7511>; + clocks = <&pguclk>; + clock-names = "pxlclk"; + + port { + pgu_output: endpoint { + remote-endpoint = <&adv7511_input>; + }; + }; + }; }; }; -- 2.5.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4 v3] MAINTAINERS: Add maintainer for ARC PGU display controller 2016-03-11 15:42 [PATCH 0/4 v3] drm: Add support of ARC PGU display controller Alexey Brodkin ` (2 preceding siblings ...) 2016-03-11 15:42 ` [PATCH 3/4 v3] arc: axs10x - add support of ARC PGU Alexey Brodkin @ 2016-03-11 15:42 ` Alexey Brodkin 3 siblings, 0 replies; 18+ messages in thread From: Alexey Brodkin @ 2016-03-11 15:42 UTC (permalink / raw) To: linux-kernel; +Cc: linux-snps-arc, Alexey Brodkin, dri-devel This updates MAINTEINERS file with information about maintainer of ARC PGU display controller driver. Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: linux-snps-arc@lists.infradead.org Cc: dri-devel@lists.freedesktop.org --- No changes v2 -> v3. No changes v1 -> v2. MAINTAINERS | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index ea1d1de..0dcba67 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -827,6 +827,12 @@ S: Maintained F: drivers/net/arcnet/ F: include/uapi/linux/if_arcnet.h +ARC PGU DRM DRIVER +M: Alexey Brodkin <abrodkin@synopsys.com> +S: Supported +F: drivers/gpu/drm/arc/ +F: Documentation/devicetree/bindings/display/snps,arcpgu.txt + ARM HDLCD DRM DRIVER M: Liviu Dudau <liviu.dudau@arm.com> S: Supported -- 2.5.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-03-24 14:58 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-11 15:42 [PATCH 0/4 v3] drm: Add support of ARC PGU display controller Alexey Brodkin 2016-03-11 15:42 ` [PATCH 1/4 " Alexey Brodkin 2016-03-11 16:45 ` kbuild test robot 2016-03-11 16:45 ` [PATCH] drm: fix platform_no_drv_owner.cocci warnings kbuild test robot 2016-03-14 7:00 ` [PATCH 1/4 v3] drm: Add support of ARC PGU display controller Daniel Vetter 2016-03-14 11:15 ` Alexey Brodkin 2016-03-15 8:10 ` Daniel Vetter 2016-03-15 15:24 ` Alexey Brodkin 2016-03-15 15:59 ` Daniel Vetter 2016-03-17 20:27 ` Alexey Brodkin 2016-03-18 8:02 ` Daniel Vetter 2016-03-18 8:11 ` Alexey Brodkin 2016-03-18 17:23 ` Daniel Vetter 2016-03-11 15:42 ` [PATCH 2/4 v3] drm: Add DT bindings documentation for " Alexey Brodkin 2016-03-21 13:04 ` Rob Herring 2016-03-24 14:57 ` Alexey Brodkin 2016-03-11 15:42 ` [PATCH 3/4 v3] arc: axs10x - add support of ARC PGU Alexey Brodkin 2016-03-11 15:42 ` [PATCH 4/4 v3] MAINTAINERS: Add maintainer for ARC PGU display controller Alexey Brodkin
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).