[PATCH 0/2] drm/msm/mdp5: hardware cursor support

[PATCH 0/2] drm/msm/mdp5: hardware cursor support

[Stephane Viau]

This patch set contains the hardware cursor changes. Split into two, as usual,
in order to separate header file generation from the rest of the code.

Beeresh Gopal (1):
  drm/msm/mdp5: Add hardware cursor support

Stephane Viau (1):
  drm/msm/mdp5: add register description for HW Cursor support

 drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h  |  79 +++++++++++++++
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 164 +++++++++++++++++++++++++++++++
 2 files changed, 243 insertions(+)

-- 
Qualcomm Innovation Center, Inc.

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH 1/2] drm/msm/mdp5: add register description for HW Cursor support

[Stephane Viau]

Update generated headers, in particular pick up the definitions
for Hardware Cursor support (MDP5).

Signed-off-by: Stephane Viau <sviau@codeaurora.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h | 79 +++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h
index 7833c22..09701a4 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h
@@ -129,6 +129,17 @@ enum mdp5_client_id {
 	CID_MAX = 23,
 };
 
+enum mdp5_cursor_format {
+	CURSOR_FMT_ARGB8888 = 0,
+	CURSOR_FMT_ARGB1555 = 2,
+	CURSOR_FMT_ARGB4444 = 4,
+};
+
+enum mdp5_cursor_alpha {
+	CURSOR_ALPHA_CONST = 0,
+	CURSOR_ALPHA_PER_PIXEL = 2,
+};
+
 enum mdp5_igc_type {
 	IGC_VIG = 0,
 	IGC_RGB = 1,
@@ -973,20 +984,88 @@ static inline uint32_t REG_MDP5_LM_BLEND_BG_TRANSP_HIGH0(uint32_t i0, uint32_t i
 static inline uint32_t REG_MDP5_LM_BLEND_BG_TRANSP_HIGH1(uint32_t i0, uint32_t i1) { return 0x00000048 + __offset_LM(i0) + 0x30*i1; }
 
 static inline uint32_t REG_MDP5_LM_CURSOR_IMG_SIZE(uint32_t i0) { return 0x000000e0 + __offset_LM(i0); }
+#define MDP5_LM_CURSOR_IMG_SIZE_SRC_W__MASK			0x0000ffff
+#define MDP5_LM_CURSOR_IMG_SIZE_SRC_W__SHIFT			0
+static inline uint32_t MDP5_LM_CURSOR_IMG_SIZE_SRC_W(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_IMG_SIZE_SRC_W__SHIFT) & MDP5_LM_CURSOR_IMG_SIZE_SRC_W__MASK;
+}
+#define MDP5_LM_CURSOR_IMG_SIZE_SRC_H__MASK			0xffff0000
+#define MDP5_LM_CURSOR_IMG_SIZE_SRC_H__SHIFT			16
+static inline uint32_t MDP5_LM_CURSOR_IMG_SIZE_SRC_H(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_IMG_SIZE_SRC_H__SHIFT) & MDP5_LM_CURSOR_IMG_SIZE_SRC_H__MASK;
+}
 
 static inline uint32_t REG_MDP5_LM_CURSOR_SIZE(uint32_t i0) { return 0x000000e4 + __offset_LM(i0); }
+#define MDP5_LM_CURSOR_SIZE_ROI_W__MASK				0x0000ffff
+#define MDP5_LM_CURSOR_SIZE_ROI_W__SHIFT			0
+static inline uint32_t MDP5_LM_CURSOR_SIZE_ROI_W(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_SIZE_ROI_W__SHIFT) & MDP5_LM_CURSOR_SIZE_ROI_W__MASK;
+}
+#define MDP5_LM_CURSOR_SIZE_ROI_H__MASK				0xffff0000
+#define MDP5_LM_CURSOR_SIZE_ROI_H__SHIFT			16
+static inline uint32_t MDP5_LM_CURSOR_SIZE_ROI_H(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_SIZE_ROI_H__SHIFT) & MDP5_LM_CURSOR_SIZE_ROI_H__MASK;
+}
 
 static inline uint32_t REG_MDP5_LM_CURSOR_XY(uint32_t i0) { return 0x000000e8 + __offset_LM(i0); }
+#define MDP5_LM_CURSOR_XY_SRC_X__MASK				0x0000ffff
+#define MDP5_LM_CURSOR_XY_SRC_X__SHIFT				0
+static inline uint32_t MDP5_LM_CURSOR_XY_SRC_X(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_XY_SRC_X__SHIFT) & MDP5_LM_CURSOR_XY_SRC_X__MASK;
+}
+#define MDP5_LM_CURSOR_XY_SRC_Y__MASK				0xffff0000
+#define MDP5_LM_CURSOR_XY_SRC_Y__SHIFT				16
+static inline uint32_t MDP5_LM_CURSOR_XY_SRC_Y(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_XY_SRC_Y__SHIFT) & MDP5_LM_CURSOR_XY_SRC_Y__MASK;
+}
 
 static inline uint32_t REG_MDP5_LM_CURSOR_STRIDE(uint32_t i0) { return 0x000000dc + __offset_LM(i0); }
+#define MDP5_LM_CURSOR_STRIDE_STRIDE__MASK			0x0000ffff
+#define MDP5_LM_CURSOR_STRIDE_STRIDE__SHIFT			0
+static inline uint32_t MDP5_LM_CURSOR_STRIDE_STRIDE(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_STRIDE_STRIDE__SHIFT) & MDP5_LM_CURSOR_STRIDE_STRIDE__MASK;
+}
 
 static inline uint32_t REG_MDP5_LM_CURSOR_FORMAT(uint32_t i0) { return 0x000000ec + __offset_LM(i0); }
+#define MDP5_LM_CURSOR_FORMAT_FORMAT__MASK			0x00000007
+#define MDP5_LM_CURSOR_FORMAT_FORMAT__SHIFT			0
+static inline uint32_t MDP5_LM_CURSOR_FORMAT_FORMAT(enum mdp5_cursor_format val)
+{
+	return ((val) << MDP5_LM_CURSOR_FORMAT_FORMAT__SHIFT) & MDP5_LM_CURSOR_FORMAT_FORMAT__MASK;
+}
 
 static inline uint32_t REG_MDP5_LM_CURSOR_BASE_ADDR(uint32_t i0) { return 0x000000f0 + __offset_LM(i0); }
 
 static inline uint32_t REG_MDP5_LM_CURSOR_START_XY(uint32_t i0) { return 0x000000f4 + __offset_LM(i0); }
+#define MDP5_LM_CURSOR_START_XY_X_START__MASK			0x0000ffff
+#define MDP5_LM_CURSOR_START_XY_X_START__SHIFT			0
+static inline uint32_t MDP5_LM_CURSOR_START_XY_X_START(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_START_XY_X_START__SHIFT) & MDP5_LM_CURSOR_START_XY_X_START__MASK;
+}
+#define MDP5_LM_CURSOR_START_XY_Y_START__MASK			0xffff0000
+#define MDP5_LM_CURSOR_START_XY_Y_START__SHIFT			16
+static inline uint32_t MDP5_LM_CURSOR_START_XY_Y_START(uint32_t val)
+{
+	return ((val) << MDP5_LM_CURSOR_START_XY_Y_START__SHIFT) & MDP5_LM_CURSOR_START_XY_Y_START__MASK;
+}
 
 static inline uint32_t REG_MDP5_LM_CURSOR_BLEND_CONFIG(uint32_t i0) { return 0x000000f8 + __offset_LM(i0); }
+#define MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_EN			0x00000001
+#define MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL__MASK	0x00000006
+#define MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL__SHIFT	1
+static inline uint32_t MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL(enum mdp5_cursor_alpha val)
+{
+	return ((val) << MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL__SHIFT) & MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL__MASK;
+}
+#define MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_TRANSP_EN		0x00000008
 
 static inline uint32_t REG_MDP5_LM_CURSOR_BLEND_PARAM(uint32_t i0) { return 0x000000fc + __offset_LM(i0); }
 
-- 
Qualcomm Innovation Center, Inc.

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH 2/2] drm/msm/mdp5: Add hardware cursor support

[Stephane Viau]

From: Beeresh Gopal <gbeeresh@codeaurora.org>

This patch implements the hardware accelarated cursor
support for MDP5 platforms.

Signed-off-by: Beeresh Gopal <gbeeresh@codeaurora.org>
Signed-off-by: Wentao Xu <wentaox@codeaurora.org>
Signed-off-by: Stephane Viau <sviau@codeaurora.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 164 +++++++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index f021f96..2021f09 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -24,6 +24,9 @@
 #include "drm_crtc_helper.h"
 #include "drm_flip_work.h"
 
+#define CURSOR_WIDTH	64
+#define CURSOR_HEIGHT	64
+
 #define SSPP_MAX	(SSPP_RGB3 + 1) /* TODO: Add SSPP_MAX in mdp5.xml.h */
 
 struct mdp5_crtc {
@@ -47,8 +50,21 @@ struct mdp5_crtc {
 #define PENDING_FLIP   0x2
 	atomic_t pending;
 
+	/* for unref'ing cursor bo's after scanout completes: */
+	struct drm_flip_work unref_cursor_work;
+
 	struct mdp_irq vblank;
 	struct mdp_irq err;
+
+	struct {
+		/* protect REG_MDP5_LM_CURSOR* registers and cursor scanout_bo*/
+		spinlock_t lock;
+
+		/* current cursor being scanned out: */
+		struct drm_gem_object *scanout_bo;
+		uint32_t width;
+		uint32_t height;
+	} cursor;
 };
 #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base)
 
@@ -129,11 +145,22 @@ static void complete_flip(struct drm_crtc *crtc, struct drm_file *file)
 	}
 }
 
+static void unref_cursor_worker(struct drm_flip_work *work, void *val)
+{
+	struct mdp5_crtc *mdp5_crtc =
+		container_of(work, struct mdp5_crtc, unref_cursor_work);
+	struct mdp5_kms *mdp5_kms = get_kms(&mdp5_crtc->base);
+
+	msm_gem_put_iova(val, mdp5_kms->id);
+	drm_gem_object_unreference_unlocked(val);
+}
+
 static void mdp5_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
 
 	drm_crtc_cleanup(crtc);
+	drm_flip_work_cleanup(&mdp5_crtc->unref_cursor_work);
 
 	kfree(mdp5_crtc);
 }
@@ -384,6 +411,132 @@ static int mdp5_crtc_set_property(struct drm_crtc *crtc,
 	return -EINVAL;
 }
 
+static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,
+		struct drm_file *file, uint32_t handle,
+		uint32_t width, uint32_t height)
+{
+	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct mdp5_kms *mdp5_kms = get_kms(crtc);
+	struct drm_gem_object *cursor_bo, *old_bo;
+	uint32_t blendcfg, cursor_addr, stride;
+	int ret, bpp, lm;
+	unsigned int depth;
+	enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;
+	uint32_t flush_mask = mdp_ctl_flush_mask_cursor(0);
+	unsigned long flags;
+
+	if ((width > CURSOR_WIDTH) || (height > CURSOR_HEIGHT)) {
+		dev_err(dev->dev, "bad cursor size: %dx%d\n", width, height);
+		return -EINVAL;
+	}
+
+	if (NULL == mdp5_crtc->ctl)
+		return -EINVAL;
+
+	if (!handle) {
+		DBG("Cursor off");
+		return mdp5_ctl_set_cursor(mdp5_crtc->ctl, false);
+	}
+
+	cursor_bo = drm_gem_object_lookup(dev, file, handle);
+	if (!cursor_bo)
+		return -ENOENT;
+
+	ret = msm_gem_get_iova(cursor_bo, mdp5_kms->id, &cursor_addr);
+	if (ret)
+		return -EINVAL;
+
+	lm = mdp5_crtc->lm;
+	drm_fb_get_bpp_depth(DRM_FORMAT_ARGB8888, &depth, &bpp);
+	stride = width * (bpp >> 3);
+
+	spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
+	old_bo = mdp5_crtc->cursor.scanout_bo;
+
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),
+			MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888));
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_IMG_SIZE(lm),
+			MDP5_LM_CURSOR_IMG_SIZE_SRC_H(height) |
+			MDP5_LM_CURSOR_IMG_SIZE_SRC_W(width));
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(lm),
+			MDP5_LM_CURSOR_SIZE_ROI_H(height) |
+			MDP5_LM_CURSOR_SIZE_ROI_W(width));
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), cursor_addr);
+
+
+	blendcfg = MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_EN;
+	blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_TRANSP_EN;
+	blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL(cur_alpha);
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BLEND_CONFIG(lm), blendcfg);
+
+	mdp5_crtc->cursor.scanout_bo = cursor_bo;
+	mdp5_crtc->cursor.width = width;
+	mdp5_crtc->cursor.height = height;
+	spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
+
+	ret = mdp5_ctl_set_cursor(mdp5_crtc->ctl, true);
+	if (ret)
+		goto end;
+
+	flush_mask |= mdp5_ctl_get_flush(mdp5_crtc->ctl);
+	crtc_flush(crtc, flush_mask);
+
+end:
+	if (old_bo) {
+		drm_flip_work_queue(&mdp5_crtc->unref_cursor_work, old_bo);
+		/* enable vblank to complete cursor work: */
+		request_pending(crtc, PENDING_CURSOR);
+	}
+	return ret;
+}
+
+static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
+{
+	struct mdp5_kms *mdp5_kms = get_kms(crtc);
+	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
+	uint32_t flush_mask = mdp_ctl_flush_mask_cursor(0);
+	uint32_t xres = crtc->mode.hdisplay;
+	uint32_t yres = crtc->mode.vdisplay;
+	uint32_t roi_w;
+	uint32_t roi_h;
+	unsigned long flags;
+
+	x = (x > 0) ? x : 0;
+	y = (y > 0) ? y : 0;
+
+	/*
+	 * Cursor Region Of Interest (ROI) is a plane read from cursor
+	 * buffer to render. The ROI region is determined by the visiblity of
+	 * the cursor point. In the default Cursor image the cursor point will
+	 * be at the top left of the cursor image, unless it is specified
+	 * otherwise using hotspot feature.
+	 *
+	 * If the cursor point reaches the right (xres - x < cursor.width) or
+	 * bottom (yres - y < cursor.height) boundary of the screen, then ROI
+	 * width and ROI height need to be evaluated to crop the cursor image
+	 * accordingly.
+	 * (xres-x) will be new cursor width when x > (xres - cursor.width)
+	 * (yres-y) will be new cursor height when y > (yres - cursor.height)
+	 */
+	roi_w = min(mdp5_crtc->cursor.width, xres - x);
+	roi_h = min(mdp5_crtc->cursor.height, yres - y);
+
+	spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(mdp5_crtc->lm),
+			MDP5_LM_CURSOR_SIZE_ROI_H(roi_h) |
+			MDP5_LM_CURSOR_SIZE_ROI_W(roi_w));
+	mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(mdp5_crtc->lm),
+			MDP5_LM_CURSOR_START_XY_Y_START(y) |
+			MDP5_LM_CURSOR_START_XY_X_START(x));
+	spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
+
+	crtc_flush(crtc, flush_mask);
+
+	return 0;
+}
+
 static const struct drm_crtc_funcs mdp5_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = mdp5_crtc_destroy,
@@ -392,6 +545,8 @@ static const struct drm_crtc_funcs mdp5_crtc_funcs = {
 	.reset = drm_atomic_helper_crtc_reset,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+	.cursor_set = mdp5_crtc_cursor_set,
+	.cursor_move = mdp5_crtc_cursor_move,
 };
 
 static const struct drm_crtc_helper_funcs mdp5_crtc_helper_funcs = {
@@ -412,6 +567,7 @@ static void mdp5_crtc_vblank_irq(struct mdp_irq *irq, uint32_t irqstatus)
 {
 	struct mdp5_crtc *mdp5_crtc = container_of(irq, struct mdp5_crtc, vblank);
 	struct drm_crtc *crtc = &mdp5_crtc->base;
+	struct msm_drm_private *priv = crtc->dev->dev_private;
 	unsigned pending;
 
 	mdp_irq_unregister(&get_kms(crtc)->base, &mdp5_crtc->vblank);
@@ -421,6 +577,9 @@ static void mdp5_crtc_vblank_irq(struct mdp_irq *irq, uint32_t irqstatus)
 	if (pending & PENDING_FLIP) {
 		complete_flip(crtc, NULL);
 	}
+
+	if (pending & PENDING_CURSOR)
+		drm_flip_work_commit(&mdp5_crtc->unref_cursor_work, priv->wq);
 }
 
 static void mdp5_crtc_err_irq(struct mdp_irq *irq, uint32_t irqstatus)
@@ -520,6 +679,7 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
 	mdp5_crtc->lm = GET_LM_ID(id);
 
 	spin_lock_init(&mdp5_crtc->lm_lock);
+	spin_lock_init(&mdp5_crtc->cursor.lock);
 
 	mdp5_crtc->vblank.irq = mdp5_crtc_vblank_irq;
 	mdp5_crtc->err.irq = mdp5_crtc_err_irq;
@@ -528,6 +688,10 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
 			pipe2name(mdp5_plane_pipe(plane)), id);
 
 	drm_crtc_init_with_planes(dev, crtc, plane, NULL, &mdp5_crtc_funcs);
+
+	drm_flip_work_init(&mdp5_crtc->unref_cursor_work,
+			"unref cursor", unref_cursor_worker);
+
 	drm_crtc_helper_add(crtc, &mdp5_crtc_helper_funcs);
 	plane->crtc = crtc;
 
-- 
Qualcomm Innovation Center, Inc.

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

Re: [PATCH 2/2] drm/msm/mdp5: Add hardware cursor support

[Daniel Vetter]

On Tue, Jan 13, 2015 at 05:18:04PM -0500, Stephane Viau wrote:
> From: Beeresh Gopal <gbeeresh@codeaurora.org>
>
> This patch implements the hardware accelarated cursor
> support for MDP5 platforms.
>
> Signed-off-by: Beeresh Gopal <gbeeresh@codeaurora.org>
> Signed-off-by: Wentao Xu <wentaox@codeaurora.org>
> Signed-off-by: Stephane Viau <sviau@codeaurora.org>

Imo implementing legacy cursor support instead of with universal planes
makes no sense. Especially since msm is converted to atomic already, and
you can't move the cursor with atomic when it's legacy only. See the
cursor argument for the drm_crtc_init_with_planes function and how it's
used in e.g. i915.

Cheers, Daniel

> ---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 164 +++++++++++++++++++++++++++++++
>  1 file changed, 164 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index f021f96..2021f09 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -24,6 +24,9 @@
>  #include "drm_crtc_helper.h"
>  #include "drm_flip_work.h"
>
> +#define CURSOR_WIDTH 64
> +#define CURSOR_HEIGHT 64
> +
>  #define SSPP_MAX (SSPP_RGB3 + 1) /* TODO: Add SSPP_MAX in mdp5.xml.h */
>
>  struct mdp5_crtc {
> @@ -47,8 +50,21 @@ struct mdp5_crtc {
>  #define PENDING_FLIP   0x2
>   atomic_t pending;
>
> + /* for unref'ing cursor bo's after scanout completes: */
> + struct drm_flip_work unref_cursor_work;
> +
>   struct mdp_irq vblank;
>   struct mdp_irq err;
> +
> + struct {
> + /* protect REG_MDP5_LM_CURSOR* registers and cursor scanout_bo*/
> + spinlock_t lock;
> +
> + /* current cursor being scanned out: */
> + struct drm_gem_object *scanout_bo;
> + uint32_t width;
> + uint32_t height;
> + } cursor;
>  };
>  #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base)
>
> @@ -129,11 +145,22 @@ static void complete_flip(struct drm_crtc *crtc, struct drm_file *file)
>   }
>  }
>
> +static void unref_cursor_worker(struct drm_flip_work *work, void *val)
> +{
> + struct mdp5_crtc *mdp5_crtc =
> + container_of(work, struct mdp5_crtc, unref_cursor_work);
> + struct mdp5_kms *mdp5_kms = get_kms(&mdp5_crtc->base);
> +
> + msm_gem_put_iova(val, mdp5_kms->id);
> + drm_gem_object_unreference_unlocked(val);
> +}
> +
>  static void mdp5_crtc_destroy(struct drm_crtc *crtc)
>  {
>   struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
>
>   drm_crtc_cleanup(crtc);
> + drm_flip_work_cleanup(&mdp5_crtc->unref_cursor_work);
>
>   kfree(mdp5_crtc);
>  }
> @@ -384,6 +411,132 @@ static int mdp5_crtc_set_property(struct drm_crtc *crtc,
>   return -EINVAL;
>  }
>
> +static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,
> + struct drm_file *file, uint32_t handle,
> + uint32_t width, uint32_t height)
> +{
> + struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
> + struct drm_device *dev = crtc->dev;
> + struct mdp5_kms *mdp5_kms = get_kms(crtc);
> + struct drm_gem_object *cursor_bo, *old_bo;
> + uint32_t blendcfg, cursor_addr, stride;
> + int ret, bpp, lm;
> + unsigned int depth;
> + enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;
> + uint32_t flush_mask = mdp_ctl_flush_mask_cursor(0);
> + unsigned long flags;
> +
> + if ((width > CURSOR_WIDTH) || (height > CURSOR_HEIGHT)) {
> + dev_err(dev->dev, "bad cursor size: %dx%d\n", width, height);
> + return -EINVAL;
> + }
> +
> + if (NULL == mdp5_crtc->ctl)
> + return -EINVAL;
> +
> + if (!handle) {
> + DBG("Cursor off");
> + return mdp5_ctl_set_cursor(mdp5_crtc->ctl, false);
> + }
> +
> + cursor_bo = drm_gem_object_lookup(dev, file, handle);
> + if (!cursor_bo)
> + return -ENOENT;
> +
> + ret = msm_gem_get_iova(cursor_bo, mdp5_kms->id, &cursor_addr);
> + if (ret)
> + return -EINVAL;
> +
> + lm = mdp5_crtc->lm;
> + drm_fb_get_bpp_depth(DRM_FORMAT_ARGB8888, &depth, &bpp);
> + stride = width * (bpp >> 3);
> +
> + spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
> + old_bo = mdp5_crtc->cursor.scanout_bo;
> +
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),
> + MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888));
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_IMG_SIZE(lm),
> + MDP5_LM_CURSOR_IMG_SIZE_SRC_H(height) |
> + MDP5_LM_CURSOR_IMG_SIZE_SRC_W(width));
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(lm),
> + MDP5_LM_CURSOR_SIZE_ROI_H(height) |
> + MDP5_LM_CURSOR_SIZE_ROI_W(width));
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), cursor_addr);
> +
> +
> + blendcfg = MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_EN;
> + blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_TRANSP_EN;
> + blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL(cur_alpha);
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BLEND_CONFIG(lm), blendcfg);
> +
> + mdp5_crtc->cursor.scanout_bo = cursor_bo;
> + mdp5_crtc->cursor.width = width;
> + mdp5_crtc->cursor.height = height;
> + spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
> +
> + ret = mdp5_ctl_set_cursor(mdp5_crtc->ctl, true);
> + if (ret)
> + goto end;
> +
> + flush_mask |= mdp5_ctl_get_flush(mdp5_crtc->ctl);
> + crtc_flush(crtc, flush_mask);
> +
> +end:
> + if (old_bo) {
> + drm_flip_work_queue(&mdp5_crtc->unref_cursor_work, old_bo);
> + /* enable vblank to complete cursor work: */
> + request_pending(crtc, PENDING_CURSOR);
> + }
> + return ret;
> +}
> +
> +static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
> +{
> + struct mdp5_kms *mdp5_kms = get_kms(crtc);
> + struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
> + uint32_t flush_mask = mdp_ctl_flush_mask_cursor(0);
> + uint32_t xres = crtc->mode.hdisplay;
> + uint32_t yres = crtc->mode.vdisplay;
> + uint32_t roi_w;
> + uint32_t roi_h;
> + unsigned long flags;
> +
> + x = (x > 0) ? x : 0;
> + y = (y > 0) ? y : 0;
> +
> + /*
> + * Cursor Region Of Interest (ROI) is a plane read from cursor
> + * buffer to render. The ROI region is determined by the visiblity of
> + * the cursor point. In the default Cursor image the cursor point will
> + * be at the top left of the cursor image, unless it is specified
> + * otherwise using hotspot feature.
> + *
> + * If the cursor point reaches the right (xres - x < cursor.width) or
> + * bottom (yres - y < cursor.height) boundary of the screen, then ROI
> + * width and ROI height need to be evaluated to crop the cursor image
> + * accordingly.
> + * (xres-x) will be new cursor width when x > (xres - cursor.width)
> + * (yres-y) will be new cursor height when y > (yres - cursor.height)
> + */
> + roi_w = min(mdp5_crtc->cursor.width, xres - x);
> + roi_h = min(mdp5_crtc->cursor.height, yres - y);
> +
> + spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(mdp5_crtc->lm),
> + MDP5_LM_CURSOR_SIZE_ROI_H(roi_h) |
> + MDP5_LM_CURSOR_SIZE_ROI_W(roi_w));
> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(mdp5_crtc->lm),
> + MDP5_LM_CURSOR_START_XY_Y_START(y) |
> + MDP5_LM_CURSOR_START_XY_X_START(x));
> + spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
> +
> + crtc_flush(crtc, flush_mask);
> +
> + return 0;
> +}
> +
>  static const struct drm_crtc_funcs mdp5_crtc_funcs = {
>   .set_config = drm_atomic_helper_set_config,
>   .destroy = mdp5_crtc_destroy,
> @@ -392,6 +545,8 @@ static const struct drm_crtc_funcs mdp5_crtc_funcs = {
>   .reset = drm_atomic_helper_crtc_reset,
>   .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>   .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> + .cursor_set = mdp5_crtc_cursor_set,
> + .cursor_move = mdp5_crtc_cursor_move,
>  };
>
>  static const struct drm_crtc_helper_funcs mdp5_crtc_helper_funcs = {
> @@ -412,6 +567,7 @@ static void mdp5_crtc_vblank_irq(struct mdp_irq *irq, uint32_t irqstatus)
>  {
>   struct mdp5_crtc *mdp5_crtc = container_of(irq, struct mdp5_crtc, vblank);
>   struct drm_crtc *crtc = &mdp5_crtc->base;
> + struct msm_drm_private *priv = crtc->dev->dev_private;
>   unsigned pending;
>
>   mdp_irq_unregister(&get_kms(crtc)->base, &mdp5_crtc->vblank);
> @@ -421,6 +577,9 @@ static void mdp5_crtc_vblank_irq(struct mdp_irq *irq, uint32_t irqstatus)
>   if (pending & PENDING_FLIP) {
>   complete_flip(crtc, NULL);
>   }
> +
> + if (pending & PENDING_CURSOR)
> + drm_flip_work_commit(&mdp5_crtc->unref_cursor_work, priv->wq);
>  }
>
>  static void mdp5_crtc_err_irq(struct mdp_irq *irq, uint32_t irqstatus)
> @@ -520,6 +679,7 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
>   mdp5_crtc->lm = GET_LM_ID(id);
>
>   spin_lock_init(&mdp5_crtc->lm_lock);
> + spin_lock_init(&mdp5_crtc->cursor.lock);
>
>   mdp5_crtc->vblank.irq = mdp5_crtc_vblank_irq;
>   mdp5_crtc->err.irq = mdp5_crtc_err_irq;
> @@ -528,6 +688,10 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
>   pipe2name(mdp5_plane_pipe(plane)), id);
>
>   drm_crtc_init_with_planes(dev, crtc, plane, NULL, &mdp5_crtc_funcs);
> +
> + drm_flip_work_init(&mdp5_crtc->unref_cursor_work,
> + "unref cursor", unref_cursor_worker);
> +
>   drm_crtc_helper_add(crtc, &mdp5_crtc_helper_funcs);
>   plane->crtc = crtc;
>
> --
> Qualcomm Innovation Center, Inc.
>
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/2] drm/msm/mdp5: Add hardware cursor support

[Rob Clark]

On Wed, Jan 14, 2015 at 7:55 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jan 13, 2015 at 05:18:04PM -0500, Stephane Viau wrote:
>> From: Beeresh Gopal <gbeeresh@codeaurora.org>
>>
>> This patch implements the hardware accelarated cursor
>> support for MDP5 platforms.
>>
>> Signed-off-by: Beeresh Gopal <gbeeresh@codeaurora.org>
>> Signed-off-by: Wentao Xu <wentaox@codeaurora.org>
>> Signed-off-by: Stephane Viau <sviau@codeaurora.org>
>
> Imo implementing legacy cursor support instead of with universal planes
> makes no sense. Especially since msm is converted to atomic already, and
> you can't move the cursor with atomic when it's legacy only. See the
> cursor argument for the drm_crtc_init_with_planes function and how it's
> used in e.g. i915.
>

well, I'm still not 100% convinced about going through the whole
atomic mechanism for cursors..  in particular stuff that tries to
enable/disable the cursor at 1000fps, goes *really* badly when things
start waiting for vsync.

I'll probably try some experiments with it at some point, but at this
point something that works with x11 is a lot more interesting for me
(since every time I switch from mdp4 device to mdp5 device I forget to
disable hw cursor the first time I start x)

BR,
-R

> Cheers, Daniel
>
>> ---
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 164 +++++++++++++++++++++++++++++++
>>  1 file changed, 164 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> index f021f96..2021f09 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> @@ -24,6 +24,9 @@
>>  #include "drm_crtc_helper.h"
>>  #include "drm_flip_work.h"
>>
>> +#define CURSOR_WIDTH 64
>> +#define CURSOR_HEIGHT 64
>> +
>>  #define SSPP_MAX (SSPP_RGB3 + 1) /* TODO: Add SSPP_MAX in mdp5.xml.h */
>>
>>  struct mdp5_crtc {
>> @@ -47,8 +50,21 @@ struct mdp5_crtc {
>>  #define PENDING_FLIP   0x2
>>   atomic_t pending;
>>
>> + /* for unref'ing cursor bo's after scanout completes: */
>> + struct drm_flip_work unref_cursor_work;
>> +
>>   struct mdp_irq vblank;
>>   struct mdp_irq err;
>> +
>> + struct {
>> + /* protect REG_MDP5_LM_CURSOR* registers and cursor scanout_bo*/
>> + spinlock_t lock;
>> +
>> + /* current cursor being scanned out: */
>> + struct drm_gem_object *scanout_bo;
>> + uint32_t width;
>> + uint32_t height;
>> + } cursor;
>>  };
>>  #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base)
>>
>> @@ -129,11 +145,22 @@ static void complete_flip(struct drm_crtc *crtc, struct drm_file *file)
>>   }
>>  }
>>
>> +static void unref_cursor_worker(struct drm_flip_work *work, void *val)
>> +{
>> + struct mdp5_crtc *mdp5_crtc =
>> + container_of(work, struct mdp5_crtc, unref_cursor_work);
>> + struct mdp5_kms *mdp5_kms = get_kms(&mdp5_crtc->base);
>> +
>> + msm_gem_put_iova(val, mdp5_kms->id);
>> + drm_gem_object_unreference_unlocked(val);
>> +}
>> +
>>  static void mdp5_crtc_destroy(struct drm_crtc *crtc)
>>  {
>>   struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
>>
>>   drm_crtc_cleanup(crtc);
>> + drm_flip_work_cleanup(&mdp5_crtc->unref_cursor_work);
>>
>>   kfree(mdp5_crtc);
>>  }
>> @@ -384,6 +411,132 @@ static int mdp5_crtc_set_property(struct drm_crtc *crtc,
>>   return -EINVAL;
>>  }
>>
>> +static int mdp5_crtc_cursor_set(struct drm_crtc *crtc,
>> + struct drm_file *file, uint32_t handle,
>> + uint32_t width, uint32_t height)
>> +{
>> + struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
>> + struct drm_device *dev = crtc->dev;
>> + struct mdp5_kms *mdp5_kms = get_kms(crtc);
>> + struct drm_gem_object *cursor_bo, *old_bo;
>> + uint32_t blendcfg, cursor_addr, stride;
>> + int ret, bpp, lm;
>> + unsigned int depth;
>> + enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;
>> + uint32_t flush_mask = mdp_ctl_flush_mask_cursor(0);
>> + unsigned long flags;
>> +
>> + if ((width > CURSOR_WIDTH) || (height > CURSOR_HEIGHT)) {
>> + dev_err(dev->dev, "bad cursor size: %dx%d\n", width, height);
>> + return -EINVAL;
>> + }
>> +
>> + if (NULL == mdp5_crtc->ctl)
>> + return -EINVAL;
>> +
>> + if (!handle) {
>> + DBG("Cursor off");
>> + return mdp5_ctl_set_cursor(mdp5_crtc->ctl, false);
>> + }
>> +
>> + cursor_bo = drm_gem_object_lookup(dev, file, handle);
>> + if (!cursor_bo)
>> + return -ENOENT;
>> +
>> + ret = msm_gem_get_iova(cursor_bo, mdp5_kms->id, &cursor_addr);
>> + if (ret)
>> + return -EINVAL;
>> +
>> + lm = mdp5_crtc->lm;
>> + drm_fb_get_bpp_depth(DRM_FORMAT_ARGB8888, &depth, &bpp);
>> + stride = width * (bpp >> 3);
>> +
>> + spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
>> + old_bo = mdp5_crtc->cursor.scanout_bo;
>> +
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),
>> + MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888));
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_IMG_SIZE(lm),
>> + MDP5_LM_CURSOR_IMG_SIZE_SRC_H(height) |
>> + MDP5_LM_CURSOR_IMG_SIZE_SRC_W(width));
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(lm),
>> + MDP5_LM_CURSOR_SIZE_ROI_H(height) |
>> + MDP5_LM_CURSOR_SIZE_ROI_W(width));
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), cursor_addr);
>> +
>> +
>> + blendcfg = MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_EN;
>> + blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_TRANSP_EN;
>> + blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL(cur_alpha);
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BLEND_CONFIG(lm), blendcfg);
>> +
>> + mdp5_crtc->cursor.scanout_bo = cursor_bo;
>> + mdp5_crtc->cursor.width = width;
>> + mdp5_crtc->cursor.height = height;
>> + spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
>> +
>> + ret = mdp5_ctl_set_cursor(mdp5_crtc->ctl, true);
>> + if (ret)
>> + goto end;
>> +
>> + flush_mask |= mdp5_ctl_get_flush(mdp5_crtc->ctl);
>> + crtc_flush(crtc, flush_mask);
>> +
>> +end:
>> + if (old_bo) {
>> + drm_flip_work_queue(&mdp5_crtc->unref_cursor_work, old_bo);
>> + /* enable vblank to complete cursor work: */
>> + request_pending(crtc, PENDING_CURSOR);
>> + }
>> + return ret;
>> +}
>> +
>> +static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
>> +{
>> + struct mdp5_kms *mdp5_kms = get_kms(crtc);
>> + struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
>> + uint32_t flush_mask = mdp_ctl_flush_mask_cursor(0);
>> + uint32_t xres = crtc->mode.hdisplay;
>> + uint32_t yres = crtc->mode.vdisplay;
>> + uint32_t roi_w;
>> + uint32_t roi_h;
>> + unsigned long flags;
>> +
>> + x = (x > 0) ? x : 0;
>> + y = (y > 0) ? y : 0;
>> +
>> + /*
>> + * Cursor Region Of Interest (ROI) is a plane read from cursor
>> + * buffer to render. The ROI region is determined by the visiblity of
>> + * the cursor point. In the default Cursor image the cursor point will
>> + * be at the top left of the cursor image, unless it is specified
>> + * otherwise using hotspot feature.
>> + *
>> + * If the cursor point reaches the right (xres - x < cursor.width) or
>> + * bottom (yres - y < cursor.height) boundary of the screen, then ROI
>> + * width and ROI height need to be evaluated to crop the cursor image
>> + * accordingly.
>> + * (xres-x) will be new cursor width when x > (xres - cursor.width)
>> + * (yres-y) will be new cursor height when y > (yres - cursor.height)
>> + */
>> + roi_w = min(mdp5_crtc->cursor.width, xres - x);
>> + roi_h = min(mdp5_crtc->cursor.height, yres - y);
>> +
>> + spin_lock_irqsave(&mdp5_crtc->cursor.lock, flags);
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(mdp5_crtc->lm),
>> + MDP5_LM_CURSOR_SIZE_ROI_H(roi_h) |
>> + MDP5_LM_CURSOR_SIZE_ROI_W(roi_w));
>> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(mdp5_crtc->lm),
>> + MDP5_LM_CURSOR_START_XY_Y_START(y) |
>> + MDP5_LM_CURSOR_START_XY_X_START(x));
>> + spin_unlock_irqrestore(&mdp5_crtc->cursor.lock, flags);
>> +
>> + crtc_flush(crtc, flush_mask);
>> +
>> + return 0;
>> +}
>> +
>>  static const struct drm_crtc_funcs mdp5_crtc_funcs = {
>>   .set_config = drm_atomic_helper_set_config,
>>   .destroy = mdp5_crtc_destroy,
>> @@ -392,6 +545,8 @@ static const struct drm_crtc_funcs mdp5_crtc_funcs = {
>>   .reset = drm_atomic_helper_crtc_reset,
>>   .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>>   .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>> + .cursor_set = mdp5_crtc_cursor_set,
>> + .cursor_move = mdp5_crtc_cursor_move,
>>  };
>>
>>  static const struct drm_crtc_helper_funcs mdp5_crtc_helper_funcs = {
>> @@ -412,6 +567,7 @@ static void mdp5_crtc_vblank_irq(struct mdp_irq *irq, uint32_t irqstatus)
>>  {
>>   struct mdp5_crtc *mdp5_crtc = container_of(irq, struct mdp5_crtc, vblank);
>>   struct drm_crtc *crtc = &mdp5_crtc->base;
>> + struct msm_drm_private *priv = crtc->dev->dev_private;
>>   unsigned pending;
>>
>>   mdp_irq_unregister(&get_kms(crtc)->base, &mdp5_crtc->vblank);
>> @@ -421,6 +577,9 @@ static void mdp5_crtc_vblank_irq(struct mdp_irq *irq, uint32_t irqstatus)
>>   if (pending & PENDING_FLIP) {
>>   complete_flip(crtc, NULL);
>>   }
>> +
>> + if (pending & PENDING_CURSOR)
>> + drm_flip_work_commit(&mdp5_crtc->unref_cursor_work, priv->wq);
>>  }
>>
>>  static void mdp5_crtc_err_irq(struct mdp_irq *irq, uint32_t irqstatus)
>> @@ -520,6 +679,7 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
>>   mdp5_crtc->lm = GET_LM_ID(id);
>>
>>   spin_lock_init(&mdp5_crtc->lm_lock);
>> + spin_lock_init(&mdp5_crtc->cursor.lock);
>>
>>   mdp5_crtc->vblank.irq = mdp5_crtc_vblank_irq;
>>   mdp5_crtc->err.irq = mdp5_crtc_err_irq;
>> @@ -528,6 +688,10 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev,
>>   pipe2name(mdp5_plane_pipe(plane)), id);
>>
>>   drm_crtc_init_with_planes(dev, crtc, plane, NULL, &mdp5_crtc_funcs);
>> +
>> + drm_flip_work_init(&mdp5_crtc->unref_cursor_work,
>> + "unref cursor", unref_cursor_worker);
>> +
>>   drm_crtc_helper_add(crtc, &mdp5_crtc_helper_funcs);
>>   plane->crtc = crtc;
>>
>> --
>> Qualcomm Innovation Center, Inc.
>>
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/msm/mdp5: Add hardware cursor support

[Daniel Vetter]

On Thu, Jan 15, 2015 at 08:46:46AM -0500, Rob Clark wrote:
> On Wed, Jan 14, 2015 at 7:55 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Jan 13, 2015 at 05:18:04PM -0500, Stephane Viau wrote:
> >> From: Beeresh Gopal <gbeeresh@codeaurora.org>
> >>
> >> This patch implements the hardware accelarated cursor
> >> support for MDP5 platforms.
> >>
> >> Signed-off-by: Beeresh Gopal <gbeeresh@codeaurora.org>
> >> Signed-off-by: Wentao Xu <wentaox@codeaurora.org>
> >> Signed-off-by: Stephane Viau <sviau@codeaurora.org>
> >
> > Imo implementing legacy cursor support instead of with universal planes
> > makes no sense. Especially since msm is converted to atomic already, and
> > you can't move the cursor with atomic when it's legacy only. See the
> > cursor argument for the drm_crtc_init_with_planes function and how it's
> > used in e.g. i915.
> >
>
> well, I'm still not 100% convinced about going through the whole
> atomic mechanism for cursors..  in particular stuff that tries to
> enable/disable the cursor at 1000fps, goes *really* badly when things
> start waiting for vsync.
>
> I'll probably try some experiments with it at some point, but at this
> point something that works with x11 is a lot more interesting for me
> (since every time I switch from mdp4 device to mdp5 device I forget to
> disable hw cursor the first time I start x)

Well for one this uses the legacy cursor callbacks directly, at least a
cursor plane is imo in order.

Otoh we just need to fix up the cursor atomic implementation to allow
drivers to do fully async updates which get merged down to one update.
Which Ville's original atomic stuff already had. So all recoverable by
adding a flag somewhere and setting that in the plane_update-on-atomic
function.

Merging legacy code just because the new stuff isn't 100% perfect yet imo
just doesn't make that much sense.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/2] drm/msm/mdp5: Add hardware cursor support

[Rob Clark]

On Fri, Jan 16, 2015 at 11:06 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Jan 15, 2015 at 08:46:46AM -0500, Rob Clark wrote:
>> On Wed, Jan 14, 2015 at 7:55 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Tue, Jan 13, 2015 at 05:18:04PM -0500, Stephane Viau wrote:
>> >> From: Beeresh Gopal <gbeeresh@codeaurora.org>
>> >>
>> >> This patch implements the hardware accelarated cursor
>> >> support for MDP5 platforms.
>> >>
>> >> Signed-off-by: Beeresh Gopal <gbeeresh@codeaurora.org>
>> >> Signed-off-by: Wentao Xu <wentaox@codeaurora.org>
>> >> Signed-off-by: Stephane Viau <sviau@codeaurora.org>
>> >
>> > Imo implementing legacy cursor support instead of with universal planes
>> > makes no sense. Especially since msm is converted to atomic already, and
>> > you can't move the cursor with atomic when it's legacy only. See the
>> > cursor argument for the drm_crtc_init_with_planes function and how it's
>> > used in e.g. i915.
>> >
>>
>> well, I'm still not 100% convinced about going through the whole
>> atomic mechanism for cursors..  in particular stuff that tries to
>> enable/disable the cursor at 1000fps, goes *really* badly when things
>> start waiting for vsync.
>>
>> I'll probably try some experiments with it at some point, but at this
>> point something that works with x11 is a lot more interesting for me
>> (since every time I switch from mdp4 device to mdp5 device I forget to
>> disable hw cursor the first time I start x)
>
> Well for one this uses the legacy cursor callbacks directly, at least a
> cursor plane is imo in order.
>
> Otoh we just need to fix up the cursor atomic implementation to allow
> drivers to do fully async updates which get merged down to one update.
> Which Ville's original atomic stuff already had. So all recoverable by
> adding a flag somewhere and setting that in the plane_update-on-atomic
> function.

something that could merge multiple intra-vsync updates would, I
think, make cursor planes usable

> Merging legacy code just because the new stuff isn't 100% perfect yet imo
> just doesn't make that much sense.

but merging legacy because new stuff isn't usable yet does ;-)

BR,
-R


> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/2] drm/msm/mdp5: Add hardware cursor support

[Daniel Vetter]

On Sat, Jan 17, 2015 at 08:46:05AM -0500, Rob Clark wrote:
> On Fri, Jan 16, 2015 at 11:06 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Jan 15, 2015 at 08:46:46AM -0500, Rob Clark wrote:
> >> On Wed, Jan 14, 2015 at 7:55 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> > On Tue, Jan 13, 2015 at 05:18:04PM -0500, Stephane Viau wrote:
> >> >> From: Beeresh Gopal <gbeeresh@codeaurora.org>
> >> >>
> >> >> This patch implements the hardware accelarated cursor
> >> >> support for MDP5 platforms.
> >> >>
> >> >> Signed-off-by: Beeresh Gopal <gbeeresh@codeaurora.org>
> >> >> Signed-off-by: Wentao Xu <wentaox@codeaurora.org>
> >> >> Signed-off-by: Stephane Viau <sviau@codeaurora.org>
> >> >
> >> > Imo implementing legacy cursor support instead of with universal planes
> >> > makes no sense. Especially since msm is converted to atomic already, and
> >> > you can't move the cursor with atomic when it's legacy only. See the
> >> > cursor argument for the drm_crtc_init_with_planes function and how it's
> >> > used in e.g. i915.
> >> >
> >>
> >> well, I'm still not 100% convinced about going through the whole
> >> atomic mechanism for cursors..  in particular stuff that tries to
> >> enable/disable the cursor at 1000fps, goes *really* badly when things
> >> start waiting for vsync.
> >>
> >> I'll probably try some experiments with it at some point, but at this
> >> point something that works with x11 is a lot more interesting for me
> >> (since every time I switch from mdp4 device to mdp5 device I forget to
> >> disable hw cursor the first time I start x)
> >
> > Well for one this uses the legacy cursor callbacks directly, at least a
> > cursor plane is imo in order.
> >
> > Otoh we just need to fix up the cursor atomic implementation to allow
> > drivers to do fully async updates which get merged down to one update.
> > Which Ville's original atomic stuff already had. So all recoverable by
> > adding a flag somewhere and setting that in the plane_update-on-atomic
> > function.
> 
> something that could merge multiple intra-vsync updates would, I
> think, make cursor planes usable
> 
> > Merging legacy code just because the new stuff isn't 100% perfect yet imo
> > just doesn't make that much sense.
> 
> but merging legacy because new stuff isn't usable yet does ;-)

I've thought up a quick&tiny hack which should give us perfect (as in
"matches old i915 semantics") legacy cursor ioctls on top of universal
planes + atomic helpers. I need it for i915 too ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch