[PATCH] drm: hdlcd: Replace the mode config's atomic_commit with the proper implementation.

Liviu Dudau Liviu.Dudau at arm.com
Tue May 31 11:03:04 UTC 2016


The HDLCD atomic_commit code was mis-using the drm_atomic_helper_commit()
function to implement the non-blocking code. Replace that with the correct
implementation that does the async queue-ing of commits.

Signed-off-by: Liviu Dudau <Liviu.Dudau at arm.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 64 +++++++++++++++++++---------
 drivers/gpu/drm/arm/hdlcd_drv.c  | 90 ++++++++++++++++++++++++++++------------
 drivers/gpu/drm/arm/hdlcd_drv.h  |  3 +-
 3 files changed, 109 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index d1e8d31..523890d 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,
@@ -208,6 +202,21 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc,
 static void hdlcd_crtc_atomic_flush(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state)
 {
+	struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
+	struct drm_device *drm = crtc->dev;
+	unsigned long flags;
+	struct drm_pending_vblank_event	*e;
+
+	spin_lock_irqsave(&drm->event_lock, flags);
+	e = list_first_entry_or_null(&hdlcd->event_list, typeof(*e), base.link);
+
+	if (e) {
+		list_del(&e->base.link);
+		drm_crtc_send_vblank_event(&hdlcd->crtc, e);
+		drm_crtc_vblank_put(&hdlcd->crtc);
+	}
+
+	spin_unlock_irqrestore(&drm->event_lock, flags);
 }
 
 static bool hdlcd_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -219,13 +228,8 @@ static bool hdlcd_crtc_mode_fixup(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = {
 	.mode_fixup	= hdlcd_crtc_mode_fixup,
-	.mode_set	= drm_helper_crtc_mode_set,
-	.mode_set_base	= drm_helper_crtc_mode_set_base,
-	.mode_set_nofb	= hdlcd_crtc_mode_set_nofb,
 	.enable		= hdlcd_crtc_enable,
 	.disable	= hdlcd_crtc_disable,
-	.prepare	= hdlcd_crtc_disable,
-	.commit		= hdlcd_crtc_enable,
 	.atomic_check	= hdlcd_crtc_atomic_check,
 	.atomic_begin	= hdlcd_crtc_atomic_begin,
 	.atomic_flush	= hdlcd_crtc_atomic_flush,
@@ -234,6 +238,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;
 }
 
@@ -242,20 +255,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.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 21b1427..4585aa3 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -14,11 +14,14 @@
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
 #include <linux/of_graph.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/pm_runtime.h>
+#include <linux/wait.h>
 
 #include <drm/drmP.h>
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
@@ -108,10 +111,60 @@ static void hdlcd_fb_output_poll_changed(struct drm_device *drm)
 		drm_fbdev_cma_hotplug_event(hdlcd->fbdev);
 }
 
+static void hdlcd_atomic_complete(struct drm_device *dev,
+				  struct drm_atomic_state *state)
+{
+	/* ToDo: Wait for fences */
+	drm_atomic_helper_commit_modeset_disables(dev, state);
+	drm_atomic_helper_commit_planes(dev, state, false);
+	drm_atomic_helper_commit_modeset_enables(dev, state);
+
+	drm_atomic_helper_wait_for_vblanks(dev, state);
+
+	drm_atomic_helper_cleanup_planes(dev, state);
+	drm_atomic_state_free(state);
+}
+
+static void hdlcd_atomic_work(struct work_struct *work)
+{
+	struct hdlcd_drm_private *hdlcd;
+
+	hdlcd = container_of(work, struct hdlcd_drm_private, work);
+	hdlcd_atomic_complete(hdlcd->crtc.dev, hdlcd->state);
+}
+
 static int hdlcd_atomic_commit(struct drm_device *dev,
 			       struct drm_atomic_state *state, bool nonblock)
 {
-	return drm_atomic_helper_commit(dev, state, false);
+	struct hdlcd_drm_private *hdlcd = dev->dev_private;
+	int err = drm_atomic_helper_prepare_planes(dev, state);
+
+	if (err)
+		return err;
+
+	if (nonblock && !list_empty(&hdlcd->work.entry)) {
+		/* pending commit found, bail out */
+		return -EBUSY;
+	}
+
+	mutex_lock(&hdlcd->work_lock);
+	flush_work(&hdlcd->work);
+
+	/*
+	 * This is the point of non return, where software commits to
+	 * handle the new state
+	 */
+	drm_atomic_helper_swap_state(dev, state);
+	hdlcd->state = state;
+
+	if (nonblock)
+		schedule_work(&hdlcd->work);
+	else
+		hdlcd_atomic_complete(dev, state);
+
+	mutex_unlock(&hdlcd->work_lock);
+
+	return 0;
 }
 
 static const struct drm_mode_config_funcs hdlcd_mode_config_funcs = {
@@ -160,24 +213,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);
 
@@ -200,9 +238,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;
 }
 
@@ -225,20 +267,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
@@ -271,6 +304,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)
@@ -354,6 +388,8 @@ static int hdlcd_drm_bind(struct device *dev)
 
 	drm->dev_private = hdlcd;
 	dev_set_drvdata(dev, drm);
+	mutex_init(&hdlcd->work_lock);
+	INIT_WORK(&hdlcd->work, hdlcd_atomic_work);
 
 	hdlcd_setup_mode_config(drm);
 	ret = hdlcd_load(drm, 0);
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.h b/drivers/gpu/drm/arm/hdlcd_drv.h
index e7cea82..92822e9 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.h
+++ b/drivers/gpu/drm/arm/hdlcd_drv.h
@@ -9,11 +9,12 @@ struct hdlcd_drm_private {
 	void __iomem			*mmio;
 	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;
+	struct work_struct		work;
+	struct mutex			work_lock;
 #ifdef CONFIG_DEBUG_FS
 	atomic_t buffer_underrun_count;
 	atomic_t bus_error_count;
-- 
2.8.2



More information about the dri-devel mailing list