[PATCH v2 0/5] HDLCD cleanups for v4.7

[PATCH v2 0/5] HDLCD cleanups for v4.7

[Liviu Dudau]

Hello,

Here are a series of patches that I would like to add to v4.7. It fixes issues
with suspend/resume on Juno (support for which has been added in v4.7-rc1).
When doing the work I've noticed some breakage on the vsync behaviour so I've
fixed that as well. In order to ease the introduction of Daniel Vetter's series
that adds support for non-blocking atomic operations I also picked up his patch
that cleans up the crtc->state->event handling.

The final patch adds support for dumping information from the CMA allocator on
the underlying framebuffers that I found useful while debugging the non-blocking
atomic operations.

A copy of the series has been pushed to git://linux-arm.org/linux-ld for-upstream/hdlcd

Best regards,
Liviu

Daniel Vetter (1):
  drm/hdlcd: Fix up crtc_state->event handling

Liviu Dudau (4):
  drm: hdlcd: Revamp runtime power management
  drm: hdlcd: Cleanup the atomic plane operations
  drm: hdlcd: Don't touch vsync interrupts during vblank enable/disable.
  drm: hdlcd: Add information about the underlying framebuffers in debugfs

 drivers/gpu/drm/arm/hdlcd_crtc.c | 85 +++++++++++++++++++++++-----------------
 drivers/gpu/drm/arm/hdlcd_drv.c  | 83 ++++++++++++++++-----------------------
 drivers/gpu/drm/arm/hdlcd_drv.h  |  5 +--
 3 files changed, 83 insertions(+), 90 deletions(-)

-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/5] drm: hdlcd: Revamp runtime power management

[Liviu Dudau]

Because the HDLCD driver acts as a component master it can end
up enabling the runtime PM functionality before the encoders
are initialised. This can cause crashes if the component slave
never probes (missing module) or if the PM operations kick in
before the probe finishes.

Move the enabling of the runtime PM after the component master
has finished collecting the slave components and use the DRM
atomic helpers to suspend and resume the device.

Tested-by: Robin Murphy <Robin.Murphy@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 23 +++++++++----------
 drivers/gpu/drm/arm/hdlcd_drv.c  | 48 ++++++++++++++++++++++------------------
 drivers/gpu/drm/arm/hdlcd_drv.h  |  3 +--
 3 files changed, 39 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index fef1b04..d1e8d31 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -33,8 +33,17 @@
  *
  */
 
+static void hdlcd_crtc_cleanup(struct drm_crtc *crtc)
+{
+	struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
+
+	/* stop the controller on cleanup */
+	hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
+	drm_crtc_cleanup(crtc);
+}
+
 static const struct drm_crtc_funcs hdlcd_crtc_funcs = {
-	.destroy = drm_crtc_cleanup,
+	.destroy = hdlcd_crtc_cleanup,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
 	.reset = drm_atomic_helper_crtc_reset,
@@ -155,8 +164,8 @@ static void hdlcd_crtc_disable(struct drm_crtc *crtc)
 	if (!crtc->primary->fb)
 		return;
 
-	clk_disable_unprepare(hdlcd->clk);
 	hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
+	clk_disable_unprepare(hdlcd->clk);
 	drm_crtc_vblank_off(crtc);
 }
 
@@ -294,16 +303,6 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device *drm)
 	return plane;
 }
 
-void hdlcd_crtc_suspend(struct drm_crtc *crtc)
-{
-	hdlcd_crtc_disable(crtc);
-}
-
-void hdlcd_crtc_resume(struct drm_crtc *crtc)
-{
-	hdlcd_crtc_enable(crtc);
-}
-
 int hdlcd_setup_crtc(struct drm_device *drm)
 {
 	struct hdlcd_drm_private *hdlcd = drm->dev_private;
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index b987c63..21b1427 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -84,11 +84,7 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
 		goto setup_fail;
 	}
 
-	pm_runtime_enable(drm->dev);
-
-	pm_runtime_get_sync(drm->dev);
 	ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
-	pm_runtime_put_sync(drm->dev);
 	if (ret < 0) {
 		DRM_ERROR("failed to install IRQ handler\n");
 		goto irq_fail;
@@ -357,6 +353,8 @@ static int hdlcd_drm_bind(struct device *dev)
 		return -ENOMEM;
 
 	drm->dev_private = hdlcd;
+	dev_set_drvdata(dev, drm);
+
 	hdlcd_setup_mode_config(drm);
 	ret = hdlcd_load(drm, 0);
 	if (ret)
@@ -366,14 +364,18 @@ static int hdlcd_drm_bind(struct device *dev)
 	if (ret)
 		goto err_unload;
 
-	dev_set_drvdata(dev, drm);
-
 	ret = component_bind_all(dev, drm);
 	if (ret) {
 		DRM_ERROR("Failed to bind all components\n");
 		goto err_unregister;
 	}
 
+	ret = pm_runtime_set_active(dev);
+	if (ret)
+		goto err_pm_active;
+
+	pm_runtime_enable(dev);
+
 	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
 	if (ret < 0) {
 		DRM_ERROR("failed to initialise vblank\n");
@@ -399,16 +401,16 @@ err_fbdev:
 	drm_mode_config_cleanup(drm);
 	drm_vblank_cleanup(drm);
 err_vblank:
+	pm_runtime_disable(drm->dev);
+err_pm_active:
 	component_unbind_all(dev, drm);
 err_unregister:
 	drm_dev_unregister(drm);
 err_unload:
-	pm_runtime_get_sync(drm->dev);
 	drm_irq_uninstall(drm);
-	pm_runtime_put_sync(drm->dev);
-	pm_runtime_disable(drm->dev);
 	of_reserved_mem_device_release(drm->dev);
 err_free:
+	dev_set_drvdata(dev, NULL);
 	drm_dev_unref(drm);
 
 	return ret;
@@ -495,30 +497,34 @@ MODULE_DEVICE_TABLE(of, hdlcd_of_match);
 static int __maybe_unused hdlcd_pm_suspend(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct drm_crtc *crtc;
+	struct hdlcd_drm_private *hdlcd = drm ? drm->dev_private : NULL;
 
-	if (pm_runtime_suspended(dev))
+	if (!hdlcd)
 		return 0;
 
-	drm_modeset_lock_all(drm);
-	list_for_each_entry(crtc, &drm->mode_config.crtc_list, head)
-		hdlcd_crtc_suspend(crtc);
-	drm_modeset_unlock_all(drm);
+	drm_kms_helper_poll_disable(drm);
+
+	hdlcd->state = drm_atomic_helper_suspend(drm);
+	if (IS_ERR(hdlcd->state)) {
+		drm_kms_helper_poll_enable(drm);
+		return PTR_ERR(hdlcd->state);
+	}
+
 	return 0;
 }
 
 static int __maybe_unused hdlcd_pm_resume(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct drm_crtc *crtc;
+	struct hdlcd_drm_private *hdlcd = drm ? drm->dev_private : NULL;
 
-	if (!pm_runtime_suspended(dev))
+	if (!hdlcd)
 		return 0;
 
-	drm_modeset_lock_all(drm);
-	list_for_each_entry(crtc, &drm->mode_config.crtc_list, head)
-		hdlcd_crtc_resume(crtc);
-	drm_modeset_unlock_all(drm);
+	drm_atomic_helper_resume(drm, hdlcd->state);
+	drm_kms_helper_poll_enable(drm);
+	pm_runtime_set_active(dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.h b/drivers/gpu/drm/arm/hdlcd_drv.h
index aa23478..e7cea82 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.h
+++ b/drivers/gpu/drm/arm/hdlcd_drv.h
@@ -13,6 +13,7 @@ struct hdlcd_drm_private {
 	struct list_head		event_list;
 	struct drm_crtc			crtc;
 	struct drm_plane		*plane;
+	struct drm_atomic_state		*state;
 #ifdef CONFIG_DEBUG_FS
 	atomic_t buffer_underrun_count;
 	atomic_t bus_error_count;
@@ -36,7 +37,5 @@ static inline u32 hdlcd_read(struct hdlcd_drm_private *hdlcd, unsigned int reg)
 
 int hdlcd_setup_crtc(struct drm_device *dev);
 void hdlcd_set_scanout(struct hdlcd_drm_private *hdlcd);
-void hdlcd_crtc_suspend(struct drm_crtc *crtc);
-void hdlcd_crtc_resume(struct drm_crtc *crtc);
 
 #endif /* __HDLCD_DRV_H__ */
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/5] drm/hdlcd: Fix up crtc_state->event handling

[Liviu Dudau]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

event_list just reimplemented what drm_crtc_arm_vblank_event does. And
we also need to send out drm events when shutting down a pipe.

With this it's possible to use the new nonblocking commit support in
the helpers.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 18 ++++++++----------
 drivers/gpu/drm/arm/hdlcd_drv.c  | 19 +------------------
 drivers/gpu/drm/arm/hdlcd_drv.h  |  1 -
 3 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index d1e8d31..97326c3 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -189,19 +189,17 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state)
 {
 	struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
-	unsigned long flags;
-
-	if (crtc->state->event) {
-		struct drm_pending_vblank_event *event = crtc->state->event;
+	struct drm_pending_vblank_event *event = crtc->state->event;
 
+	if (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, &hdlcd->event_list);
-		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
+		spin_lock_irq(&crtc->dev->event_lock);
+		if (drm_crtc_vblank_get(crtc) == 0)
+			drm_crtc_arm_vblank_event(crtc, event);
+		else
+			drm_crtc_send_vblank_event(crtc, event);
+		spin_unlock_irq(&crtc->dev->event_lock);
 	}
 }
 
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 21b1427..fb172d2 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -49,8 +49,6 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
 	atomic_set(&hdlcd->dma_end_count, 0);
 #endif
 
-	INIT_LIST_HEAD(&hdlcd->event_list);
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	hdlcd->mmio = devm_ioremap_resource(drm->dev, res);
 	if (IS_ERR(hdlcd->mmio)) {
@@ -160,24 +158,9 @@ static irqreturn_t hdlcd_irq(int irq, void *arg)
 		atomic_inc(&hdlcd->vsync_count);
 
 #endif
-	if (irq_status & HDLCD_INTERRUPT_VSYNC) {
-		bool events_sent = false;
-		unsigned long flags;
-		struct drm_pending_vblank_event	*e, *t;
-
+	if (irq_status & HDLCD_INTERRUPT_VSYNC)
 		drm_crtc_handle_vblank(&hdlcd->crtc);
 
-		spin_lock_irqsave(&drm->event_lock, flags);
-		list_for_each_entry_safe(e, t, &hdlcd->event_list, base.link) {
-			list_del(&e->base.link);
-			drm_crtc_send_vblank_event(&hdlcd->crtc, e);
-			events_sent = true;
-		}
-		if (events_sent)
-			drm_crtc_vblank_put(&hdlcd->crtc);
-		spin_unlock_irqrestore(&drm->event_lock, flags);
-	}
-
 	/* acknowledge interrupt(s) */
 	hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, irq_status);
 
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.h b/drivers/gpu/drm/arm/hdlcd_drv.h
index e7cea82..922a1dc 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.h
+++ b/drivers/gpu/drm/arm/hdlcd_drv.h
@@ -10,7 +10,6 @@ struct hdlcd_drm_private {
 	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;
 	struct drm_atomic_state		*state;
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/5] drm: hdlcd: Cleanup the atomic plane operations

[Liviu Dudau]

Harden the plane_check() code to drop attempts at scaling because
that is not supported. Make hdlcd_plane_atomic_update() set the pitch
and line length registers that correctly reflect the plane's values.
And make hdlcd_crtc_mode_set_nofb() a helper function for
hdlcd_crtc_enable() rather than an exposed hook.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 44 ++++++++++++++++++++++++++--------------
 drivers/gpu/drm/arm/hdlcd_drv.h  |  1 -
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 97326c3..31426ba 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -106,7 +106,7 @@ static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
 	struct drm_display_mode *m = &crtc->state->adjusted_mode;
 	struct videomode vm;
-	unsigned int polarities, line_length, err;
+	unsigned int polarities, err;
 
 	vm.vfront_porch = m->crtc_vsync_start - m->crtc_vdisplay;
 	vm.vback_porch = m->crtc_vtotal - m->crtc_vsync_end;
@@ -122,23 +122,18 @@ static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	if (m->flags & DRM_MODE_FLAG_PVSYNC)
 		polarities |= HDLCD_POLARITY_VSYNC;
 
-	line_length = crtc->primary->state->fb->pitches[0];
-
 	/* Allow max number of outstanding requests and largest burst size */
 	hdlcd_write(hdlcd, HDLCD_REG_BUS_OPTIONS,
 		    HDLCD_BUS_MAX_OUTSTAND | HDLCD_BUS_BURST_16);
 
-	hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, line_length);
-	hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_PITCH, line_length);
-	hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_COUNT, m->crtc_vdisplay - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_V_DATA, m->crtc_vdisplay - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_V_BACK_PORCH, vm.vback_porch - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_V_FRONT_PORCH, vm.vfront_porch - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_V_SYNC, vm.vsync_len - 1);
+	hdlcd_write(hdlcd, HDLCD_REG_H_DATA, m->crtc_hdisplay - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_H_BACK_PORCH, vm.hback_porch - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_H_FRONT_PORCH, vm.hfront_porch - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_H_SYNC, vm.hsync_len - 1);
-	hdlcd_write(hdlcd, HDLCD_REG_H_DATA, m->crtc_hdisplay - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_POLARITIES, polarities);
 
 	err = hdlcd_set_pxl_fmt(crtc);
@@ -153,20 +148,19 @@ static void hdlcd_crtc_enable(struct drm_crtc *crtc)
 	struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
 
 	clk_prepare_enable(hdlcd->clk);
+	hdlcd_crtc_mode_set_nofb(crtc);
 	hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 1);
-	drm_crtc_vblank_on(crtc);
 }
 
 static void hdlcd_crtc_disable(struct drm_crtc *crtc)
 {
 	struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
 
-	if (!crtc->primary->fb)
+	if (!crtc->state->active)
 		return;
 
 	hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
 	clk_disable_unprepare(hdlcd->clk);
-	drm_crtc_vblank_off(crtc);
 }
 
 static int hdlcd_crtc_atomic_check(struct drm_crtc *crtc,
@@ -232,6 +226,15 @@ static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = {
 static int hdlcd_plane_atomic_check(struct drm_plane *plane,
 				    struct drm_plane_state *state)
 {
+	u32 src_w, src_h;
+
+	src_w = state->src_w >> 16;
+	src_h = state->src_h >> 16;
+
+	/* we can't do any scaling of the plane source */
+	if ((src_w != state->crtc_w) || (src_h != state->crtc_h))
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -240,20 +243,31 @@ static void hdlcd_plane_atomic_update(struct drm_plane *plane,
 {
 	struct hdlcd_drm_private *hdlcd;
 	struct drm_gem_cma_object *gem;
+	unsigned int depth, bpp;
+	u32 src_w, src_h, dest_w, dest_h;
 	dma_addr_t scanout_start;
 
-	if (!plane->state->crtc || !plane->state->fb)
+	if (!plane->state->fb)
 		return;
 
-	hdlcd = crtc_to_hdlcd_priv(plane->state->crtc);
+	drm_fb_get_bpp_depth(plane->state->fb->pixel_format, &depth, &bpp);
+	src_w = plane->state->src_w >> 16;
+	src_h = plane->state->src_h >> 16;
+	dest_w = plane->state->crtc_w;
+	dest_h = plane->state->crtc_h;
 	gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
-	scanout_start = gem->paddr;
+	scanout_start = gem->paddr + plane->state->fb->offsets[0] +
+		plane->state->crtc_y * plane->state->fb->pitches[0] +
+		plane->state->crtc_x * bpp / 8;
+
+	hdlcd = plane->dev->dev_private;
+	hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, plane->state->fb->pitches[0]);
+	hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_PITCH, plane->state->fb->pitches[0]);
+	hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_COUNT, dest_h - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_FB_BASE, scanout_start);
 }
 
 static const struct drm_plane_helper_funcs hdlcd_plane_helper_funcs = {
-	.prepare_fb = NULL,
-	.cleanup_fb = NULL,
 	.atomic_check = hdlcd_plane_atomic_check,
 	.atomic_update = hdlcd_plane_atomic_update,
 };
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.h b/drivers/gpu/drm/arm/hdlcd_drv.h
index 922a1dc..e3950a0 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.h
+++ b/drivers/gpu/drm/arm/hdlcd_drv.h
@@ -9,7 +9,6 @@ struct hdlcd_drm_private {
 	void __iomem			*mmio;
 	struct clk			*clk;
 	struct drm_fbdev_cma		*fbdev;
-	struct drm_framebuffer		*fb;
 	struct drm_crtc			crtc;
 	struct drm_plane		*plane;
 	struct drm_atomic_state		*state;
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/5] drm: hdlcd: Don't touch vsync interrupts during vblank enable/disable.

[Liviu Dudau]

Because the HDLCD lacks a hardware counter for vsync signal, the DRM
framework expects that the vsync interrupts are left running to feed
the internal software counter. Currently the HDLCD is masking/unmasking
the vsync interrupt on vblank enable/disable calls, which break that
expectation. Fix that.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index fb172d2..3f92dfa 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -183,9 +183,13 @@ static int hdlcd_irq_postinstall(struct drm_device *drm)
 
 	/* enable debug interrupts */
 	irq_mask |= HDLCD_DEBUG_INT_MASK;
+#endif
+
+	/* enable vsync interrupts */
+	irq_mask |= HDLCD_INTERRUPT_VSYNC;
 
 	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask);
-#endif
+
 	return 0;
 }
 
@@ -208,20 +212,11 @@ static void hdlcd_irq_uninstall(struct drm_device *drm)
 
 static int hdlcd_enable_vblank(struct drm_device *drm, unsigned int crtc)
 {
-	struct hdlcd_drm_private *hdlcd = drm->dev_private;
-	unsigned int mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
-
-	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, mask | HDLCD_INTERRUPT_VSYNC);
-
 	return 0;
 }
 
 static void hdlcd_disable_vblank(struct drm_device *drm, unsigned int crtc)
 {
-	struct hdlcd_drm_private *hdlcd = drm->dev_private;
-	unsigned int mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
-
-	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, mask & ~HDLCD_INTERRUPT_VSYNC);
 }
 
 #ifdef CONFIG_DEBUG_FS
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/5] drm: hdlcd: Don't touch vsync interrupts during vblank enable/disable.

[Daniel Vetter]

On Wed, Jun 01, 2016 at 03:48:38PM +0100, Liviu Dudau wrote:
> Because the HDLCD lacks a hardware counter for vsync signal, the DRM
> framework expects that the vsync interrupts are left running to feed
> the internal software counter. Currently the HDLCD is masking/unmasking
> the vsync interrupt on vblank enable/disable calls, which break that
> expectation. Fix that.
> 
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

As discussed, this is actually not needed. The only thing you need to do
is set max_vblank_count = 0. If vblank enable/disable is causing problems,
then that would indicate an issue in the core drm_irq.c code, and should
be fixed there.

And if the docs are confusion, then please update those insted. This patch
here shouldn't be needed at all.
-Daniel

> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index fb172d2..3f92dfa 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -183,9 +183,13 @@ static int hdlcd_irq_postinstall(struct drm_device *drm)
>  
>  	/* enable debug interrupts */
>  	irq_mask |= HDLCD_DEBUG_INT_MASK;
> +#endif
> +
> +	/* enable vsync interrupts */
> +	irq_mask |= HDLCD_INTERRUPT_VSYNC;
>  
>  	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask);
> -#endif
> +
>  	return 0;
>  }
>  
> @@ -208,20 +212,11 @@ static void hdlcd_irq_uninstall(struct drm_device *drm)
>  
>  static int hdlcd_enable_vblank(struct drm_device *drm, unsigned int crtc)
>  {
> -	struct hdlcd_drm_private *hdlcd = drm->dev_private;
> -	unsigned int mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
> -
> -	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, mask | HDLCD_INTERRUPT_VSYNC);
> -
>  	return 0;
>  }
>  
>  static void hdlcd_disable_vblank(struct drm_device *drm, unsigned int crtc)
>  {
> -	struct hdlcd_drm_private *hdlcd = drm->dev_private;
> -	unsigned int mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
> -
> -	hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, mask & ~HDLCD_INTERRUPT_VSYNC);
>  }
>  
>  #ifdef CONFIG_DEBUG_FS
> -- 
> 2.8.2
> 
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 5/5] drm: hdlcd: Add information about the underlying framebuffers in debugfs

[Liviu Dudau]

drm_fb_cma code has a nice helper function to display in the debugfs
information about the underlying framebuffers used by HDLCD:

$ cat /sys/kernel/debug/dri/0/fb
fb: 1920x1200@XR24
   0: offset=0 pitch=7680, obj:  0 ( 2) 001011ba 0x00000000fc300000 ffffff800a27c000 9338880
fb: 1920x1200@XR24
   0: offset=0 pitch=7680, obj:  0 ( 2) 001008ca 0x00000000fba00000 ffffff8009987000 9338880
fb: 1920x1200@XR24
   0: offset=0 pitch=7680, obj:  0 ( 1) 00100000 0x00000000fb100000 ffffff8008fdc000 9216000

Add the entry in HDLCD's debugfs node.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 3f92dfa..6f389e6 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -249,6 +249,7 @@ static int hdlcd_show_pxlclock(struct seq_file *m, void *arg)
 static struct drm_info_list hdlcd_debugfs_list[] = {
 	{ "interrupt_count", hdlcd_show_underrun_count, 0 },
 	{ "clocks", hdlcd_show_pxlclock, 0 },
+	{ "fb", drm_fb_cma_debugfs_show, 0 },
 };
 
 static int hdlcd_debugfs_init(struct drm_minor *minor)
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel