[PATCH v2 0/2] Enable cursor plane on DPU

[PATCH v2 0/2] Enable cursor plane on DPU

[Sravanthi Kollukuduru]

s patchset adds the cursor plane support on DPU driver.
It also includes the fix for the black box artifacts seen
around the cursor when enabled.

Changes in V2:
	- Minor comments in logging in blend config function (Sean)

Sravanthi Kollukuduru (2):
  drm/msm/dpu: enable cursor plane on dpu
  drm/msm/dpu: fix for cursor blend issue

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c       | 31 +++++++++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h       |  4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 53 +++++++++++---------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c        | 35 +++++++++++------
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c      |  9 +----
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h      |  4 +-
 6 files changed, 76 insertions(+), 60 deletions(-)

-- 
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
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/2] drm/msm/dpu: enable cursor plane on dpu

[Sravanthi Kollukuduru]

Reserve DMA pipe for cursor plane and attach it to the
crtc during the initialization.

Changes in V2:
	None

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c       |  5 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h       |  4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 53 +++++++++++---------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c        | 35 +++++++++++------
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c      |  9 +----
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h      |  4 +-
 6 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 80cbf75bc2ff..0cd9456a6c4c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -2082,7 +2082,8 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = {
 };
 
 /* initialize crtc */
-struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane)
+struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane,
+				struct drm_plane *cursor)
 {
 	struct drm_crtc *crtc = NULL;
 	struct dpu_crtc *dpu_crtc = NULL;
@@ -2119,7 +2120,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane)
 				dpu_crtc_frame_event_work);
 	}
 
-	drm_crtc_init_with_planes(dev, crtc, plane, NULL, &dpu_crtc_funcs,
+	drm_crtc_init_with_planes(dev, crtc, plane, cursor, &dpu_crtc_funcs,
 				NULL);
 
 	drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index e87109e608e9..a89679160237 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -375,9 +375,11 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc,
  * dpu_crtc_init - create a new crtc object
  * @dev: dpu device
  * @plane: base plane
+ * @cursor: cursor plane
  * @Return: new crtc object or error
  */
-struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane);
+struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane,
+			       struct drm_plane *cursor);
 
 /**
  * dpu_crtc_register_custom_event - api for enabling/disabling crtc event
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 44ee06398b1d..c04f3f3acae4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -29,6 +29,9 @@
 	BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\
 	BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT))
 
+#define DMA_CURSOR_SDM845_MASK \
+	(DMA_SDM845_MASK | BIT(DPU_SSPP_CURSOR))
+
 #define MIXER_SDM845_MASK \
 	(BIT(DPU_MIXER_SOURCESPLIT) | BIT(DPU_DIM_LAYER))
 
@@ -174,45 +177,35 @@ static const struct dpu_sspp_sub_blks sdm845_dma_sblk_1 = _DMA_SBLK("9", 2);
 static const struct dpu_sspp_sub_blks sdm845_dma_sblk_2 = _DMA_SBLK("10", 3);
 static const struct dpu_sspp_sub_blks sdm845_dma_sblk_3 = _DMA_SBLK("11", 4);
 
-#define SSPP_VIG_BLK(_name, _id, _base, _sblk, _xinid, _clkctrl) \
-	{ \
-	.name = _name, .id = _id, \
-	.base = _base, .len = 0x1c8, \
-	.features = VIG_SDM845_MASK, \
-	.sblk = &_sblk, \
-	.xin_id = _xinid, \
-	.type = SSPP_TYPE_VIG, \
-	.clk_ctrl = _clkctrl \
-	}
-
-#define SSPP_DMA_BLK(_name, _id, _base, _sblk, _xinid, _clkctrl) \
+#define SSPP_BLK(_name, _id, _base, _features, \
+		_sblk, _xinid, _type, _clkctrl) \
 	{ \
 	.name = _name, .id = _id, \
 	.base = _base, .len = 0x1c8, \
-	.features = DMA_SDM845_MASK, \
+	.features = _features, \
 	.sblk = &_sblk, \
 	.xin_id = _xinid, \
-	.type = SSPP_TYPE_DMA, \
+	.type = _type, \
 	.clk_ctrl = _clkctrl \
 	}
 
 static struct dpu_sspp_cfg sdm845_sspp[] = {
-	SSPP_VIG_BLK("sspp_0", SSPP_VIG0, 0x4000,
-		sdm845_vig_sblk_0, 0, DPU_CLK_CTRL_VIG0),
-	SSPP_VIG_BLK("sspp_1", SSPP_VIG1, 0x6000,
-		sdm845_vig_sblk_1, 4, DPU_CLK_CTRL_VIG1),
-	SSPP_VIG_BLK("sspp_2", SSPP_VIG2, 0x8000,
-		sdm845_vig_sblk_2, 8, DPU_CLK_CTRL_VIG2),
-	SSPP_VIG_BLK("sspp_3", SSPP_VIG3, 0xa000,
-		sdm845_vig_sblk_3, 12, DPU_CLK_CTRL_VIG3),
-	SSPP_DMA_BLK("sspp_8", SSPP_DMA0, 0x24000,
-		sdm845_dma_sblk_0, 1, DPU_CLK_CTRL_DMA0),
-	SSPP_DMA_BLK("sspp_9", SSPP_DMA1, 0x26000,
-		sdm845_dma_sblk_1, 5, DPU_CLK_CTRL_DMA1),
-	SSPP_DMA_BLK("sspp_10", SSPP_DMA2, 0x28000,
-		sdm845_dma_sblk_2, 9, DPU_CLK_CTRL_CURSOR0),
-	SSPP_DMA_BLK("sspp_11", SSPP_DMA3, 0x2a000,
-		sdm845_dma_sblk_3, 13, DPU_CLK_CTRL_CURSOR1),
+	SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK,
+		sdm845_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
+	SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK,
+		sdm845_vig_sblk_1, 4,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
+	SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK,
+		sdm845_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
+	SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK,
+		sdm845_vig_sblk_3, 12,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
+	SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK,
+		sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
+	SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000,  DMA_SDM845_MASK,
+		sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
+	SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000,  DMA_CURSOR_SDM845_MASK,
+		sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+	SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000,  DMA_CURSOR_SDM845_MASK,
+		sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
 };
 
 /*************************************************************
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 7dd6bd2d6d37..2dca6b14d602 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -531,12 +531,13 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 {
 	struct drm_device *dev;
 	struct drm_plane *primary_planes[MAX_PLANES], *plane;
+	struct drm_plane *cursor_planes[MAX_PLANES] = { NULL };
 	struct drm_crtc *crtc;
 
 	struct msm_drm_private *priv;
 	struct dpu_mdss_cfg *catalog;
 
-	int primary_planes_idx = 0, i, ret;
+	int primary_planes_idx = 0, cursor_planes_idx = 0, i, ret;
 	int max_crtc_count;
 
 	if (!dpu_kms || !dpu_kms->dev || !dpu_kms->dev->dev) {
@@ -556,16 +557,24 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 
 	max_crtc_count = min(catalog->mixer_count, priv->num_encoders);
 
-	/* Create the planes */
+	/* Create the planes, keeping track of one primary/cursor per crtc */
 	for (i = 0; i < catalog->sspp_count; i++) {
-		bool primary = true;
-
-		if (catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR)
-			|| primary_planes_idx >= max_crtc_count)
-			primary = false;
-
-		plane = dpu_plane_init(dev, catalog->sspp[i].id, primary,
-				(1UL << max_crtc_count) - 1, 0);
+		enum drm_plane_type type;
+
+		if ((catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR))
+			&& cursor_planes_idx < max_crtc_count)
+			type = DRM_PLANE_TYPE_CURSOR;
+		else if (primary_planes_idx < max_crtc_count)
+			type = DRM_PLANE_TYPE_PRIMARY;
+		else
+			type = DRM_PLANE_TYPE_OVERLAY;
+
+		DPU_DEBUG("Create plane type %d with features %lx (cur %lx)\n",
+			  type, catalog->sspp[i].features,
+			  catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR));
+
+		plane = dpu_plane_init(dev, catalog->sspp[i].id, type,
+				       (1UL << max_crtc_count) - 1, 0);
 		if (IS_ERR(plane)) {
 			DPU_ERROR("dpu_plane_init failed\n");
 			ret = PTR_ERR(plane);
@@ -573,7 +582,9 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 		}
 		priv->planes[priv->num_planes++] = plane;
 
-		if (primary)
+		if (type == DRM_PLANE_TYPE_CURSOR)
+			cursor_planes[cursor_planes_idx++] = plane;
+		else if (type == DRM_PLANE_TYPE_PRIMARY)
 			primary_planes[primary_planes_idx++] = plane;
 	}
 
@@ -581,7 +592,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 
 	/* Create one CRTC per encoder */
 	for (i = 0; i < max_crtc_count; i++) {
-		crtc = dpu_crtc_init(dev, primary_planes[i]);
+		crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]);
 		if (IS_ERR(crtc)) {
 			ret = PTR_ERR(crtc);
 			goto fail;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index b640e39ebaca..efdf9b200dd9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1827,7 +1827,7 @@ bool is_dpu_plane_virtual(struct drm_plane *plane)
 
 /* initialize plane */
 struct drm_plane *dpu_plane_init(struct drm_device *dev,
-		uint32_t pipe, bool primary_plane,
+		uint32_t pipe, enum drm_plane_type type,
 		unsigned long possible_crtcs, u32 master_plane_id)
 {
 	struct drm_plane *plane = NULL, *master_plane = NULL;
@@ -1835,7 +1835,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 	struct dpu_plane *pdpu;
 	struct msm_drm_private *priv;
 	struct dpu_kms *kms;
-	enum drm_plane_type type;
 	int zpos_max = DPU_ZPOS_MAX;
 	int ret = -EINVAL;
 
@@ -1916,12 +1915,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 		goto clean_sspp;
 	}
 
-	if (pdpu->features & BIT(DPU_SSPP_CURSOR))
-		type = DRM_PLANE_TYPE_CURSOR;
-	else if (primary_plane)
-		type = DRM_PLANE_TYPE_PRIMARY;
-	else
-		type = DRM_PLANE_TYPE_OVERLAY;
 	ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,
 				pdpu->formats, pdpu->nformats,
 				NULL, type, NULL);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index f6fe6ddc7a3a..7fed0b627708 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -122,7 +122,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error);
  * dpu_plane_init - create new dpu plane for the given pipe
  * @dev:   Pointer to DRM device
  * @pipe:  dpu hardware pipe identifier
- * @primary_plane: true if this pipe is primary plane for crtc
+ * @type:  Plane type - PRIMARY/OVERLAY/CURSOR
  * @possible_crtcs: bitmask of crtc that can be attached to the given pipe
  * @master_plane_id: primary plane id of a multirect pipe. 0 value passed for
  *                   a regular plane initialization. A non-zero primary plane
@@ -130,7 +130,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error);
  *
  */
 struct drm_plane *dpu_plane_init(struct drm_device *dev,
-		uint32_t pipe, bool primary_plane,
+		uint32_t pipe, enum drm_plane_type type,
 		unsigned long possible_crtcs, u32 master_plane_id);
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 2/2] drm/msm/dpu: fix for cursor blend issue

[Sravanthi Kollukuduru]

The current driver has the opaque blend mode set as the
default causing the black box effect around the cursor.
The fix enables choosing a different blend mode for alpha
enabled formats.

Changes in V2:
	- Use drm_get_format_name() in the logs (Sean)

Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0cd9456a6c4c..07c2d15b45f2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -297,14 +297,29 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
 }
 
 static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
-		struct dpu_plane_state *pstate)
+		struct dpu_plane_state *pstate, struct dpu_format *format)
 {
 	struct dpu_hw_mixer *lm = mixer->hw_lm;
+	uint32_t blend_op;
+	struct drm_format_name_buf format_name;
 
 	/* default to opaque blending */
-	lm->ops.setup_blend_config(lm, pstate->stage, 0XFF, 0,
-				DPU_BLEND_FG_ALPHA_FG_CONST |
-				DPU_BLEND_BG_ALPHA_BG_CONST);
+	blend_op = DPU_BLEND_FG_ALPHA_FG_CONST |
+		DPU_BLEND_BG_ALPHA_BG_CONST;
+
+	if (format->alpha_enable) {
+		/* coverage blending */
+		blend_op = DPU_BLEND_FG_ALPHA_FG_PIXEL |
+			DPU_BLEND_BG_ALPHA_FG_PIXEL |
+			DPU_BLEND_BG_INV_ALPHA;
+	}
+
+	lm->ops.setup_blend_config(lm, pstate->stage,
+				0xFF, 0, blend_op);
+
+	DPU_DEBUG("format:%s, alpha_en:%u blend_op:0x%x\n",
+		drm_get_format_name(format->base.pixel_format, &format_name),
+		format->alpha_enable, blend_op);
 }
 
 static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
@@ -401,7 +416,8 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
 
 		/* blend config update */
 		for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) {
-			_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate);
+			_dpu_crtc_setup_blend_cfg(mixer + lm_idx,
+						pstate, format);
 
 			mixer[lm_idx].flush_mask |= flush_mask;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/2] drm/msm/dpu: enable cursor plane on dpu

[Sean Paul]

On Wed, Aug 22, 2018 at 11:19:56PM +0530, Sravanthi Kollukuduru wrote:
> Reserve DMA pipe for cursor plane and attach it to the
> crtc during the initialization.
> 
> Changes in V2:
> 	None
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>

In the future, if someone gives you a Reviewed-by: on a previous version, you
should add it to subsequent versions so people know it's already bee acked.

Reviewed-by: Sean Paul <seanpaul@chromium.org>

I've applied this to dpu-staging/for-next.

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c       |  5 ++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h       |  4 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 53 +++++++++++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c        | 35 +++++++++++------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c      |  9 +----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h      |  4 +-
>  6 files changed, 55 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 80cbf75bc2ff..0cd9456a6c4c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -2082,7 +2082,8 @@ static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = {
>  };
>  
>  /* initialize crtc */
> -struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane)
> +struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane,
> +				struct drm_plane *cursor)
>  {
>  	struct drm_crtc *crtc = NULL;
>  	struct dpu_crtc *dpu_crtc = NULL;
> @@ -2119,7 +2120,7 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane)
>  				dpu_crtc_frame_event_work);
>  	}
>  
> -	drm_crtc_init_with_planes(dev, crtc, plane, NULL, &dpu_crtc_funcs,
> +	drm_crtc_init_with_planes(dev, crtc, plane, cursor, &dpu_crtc_funcs,
>  				NULL);
>  
>  	drm_crtc_helper_add(crtc, &dpu_crtc_helper_funcs);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index e87109e608e9..a89679160237 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -375,9 +375,11 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc,
>   * dpu_crtc_init - create a new crtc object
>   * @dev: dpu device
>   * @plane: base plane
> + * @cursor: cursor plane
>   * @Return: new crtc object or error
>   */
> -struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane);
> +struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane,
> +			       struct drm_plane *cursor);
>  
>  /**
>   * dpu_crtc_register_custom_event - api for enabling/disabling crtc event
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 44ee06398b1d..c04f3f3acae4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -29,6 +29,9 @@
>  	BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\
>  	BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT))
>  
> +#define DMA_CURSOR_SDM845_MASK \
> +	(DMA_SDM845_MASK | BIT(DPU_SSPP_CURSOR))
> +
>  #define MIXER_SDM845_MASK \
>  	(BIT(DPU_MIXER_SOURCESPLIT) | BIT(DPU_DIM_LAYER))
>  
> @@ -174,45 +177,35 @@ static const struct dpu_sspp_sub_blks sdm845_dma_sblk_1 = _DMA_SBLK("9", 2);
>  static const struct dpu_sspp_sub_blks sdm845_dma_sblk_2 = _DMA_SBLK("10", 3);
>  static const struct dpu_sspp_sub_blks sdm845_dma_sblk_3 = _DMA_SBLK("11", 4);
>  
> -#define SSPP_VIG_BLK(_name, _id, _base, _sblk, _xinid, _clkctrl) \
> -	{ \
> -	.name = _name, .id = _id, \
> -	.base = _base, .len = 0x1c8, \
> -	.features = VIG_SDM845_MASK, \
> -	.sblk = &_sblk, \
> -	.xin_id = _xinid, \
> -	.type = SSPP_TYPE_VIG, \
> -	.clk_ctrl = _clkctrl \
> -	}
> -
> -#define SSPP_DMA_BLK(_name, _id, _base, _sblk, _xinid, _clkctrl) \
> +#define SSPP_BLK(_name, _id, _base, _features, \
> +		_sblk, _xinid, _type, _clkctrl) \
>  	{ \
>  	.name = _name, .id = _id, \
>  	.base = _base, .len = 0x1c8, \
> -	.features = DMA_SDM845_MASK, \
> +	.features = _features, \
>  	.sblk = &_sblk, \
>  	.xin_id = _xinid, \
> -	.type = SSPP_TYPE_DMA, \
> +	.type = _type, \
>  	.clk_ctrl = _clkctrl \
>  	}
>  
>  static struct dpu_sspp_cfg sdm845_sspp[] = {
> -	SSPP_VIG_BLK("sspp_0", SSPP_VIG0, 0x4000,
> -		sdm845_vig_sblk_0, 0, DPU_CLK_CTRL_VIG0),
> -	SSPP_VIG_BLK("sspp_1", SSPP_VIG1, 0x6000,
> -		sdm845_vig_sblk_1, 4, DPU_CLK_CTRL_VIG1),
> -	SSPP_VIG_BLK("sspp_2", SSPP_VIG2, 0x8000,
> -		sdm845_vig_sblk_2, 8, DPU_CLK_CTRL_VIG2),
> -	SSPP_VIG_BLK("sspp_3", SSPP_VIG3, 0xa000,
> -		sdm845_vig_sblk_3, 12, DPU_CLK_CTRL_VIG3),
> -	SSPP_DMA_BLK("sspp_8", SSPP_DMA0, 0x24000,
> -		sdm845_dma_sblk_0, 1, DPU_CLK_CTRL_DMA0),
> -	SSPP_DMA_BLK("sspp_9", SSPP_DMA1, 0x26000,
> -		sdm845_dma_sblk_1, 5, DPU_CLK_CTRL_DMA1),
> -	SSPP_DMA_BLK("sspp_10", SSPP_DMA2, 0x28000,
> -		sdm845_dma_sblk_2, 9, DPU_CLK_CTRL_CURSOR0),
> -	SSPP_DMA_BLK("sspp_11", SSPP_DMA3, 0x2a000,
> -		sdm845_dma_sblk_3, 13, DPU_CLK_CTRL_CURSOR1),
> +	SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK,
> +		sdm845_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
> +	SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK,
> +		sdm845_vig_sblk_1, 4,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
> +	SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK,
> +		sdm845_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
> +	SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK,
> +		sdm845_vig_sblk_3, 12,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
> +	SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000,  DMA_SDM845_MASK,
> +		sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> +	SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000,  DMA_SDM845_MASK,
> +		sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> +	SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000,  DMA_CURSOR_SDM845_MASK,
> +		sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> +	SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000,  DMA_CURSOR_SDM845_MASK,
> +		sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
>  };
>  
>  /*************************************************************
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 7dd6bd2d6d37..2dca6b14d602 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -531,12 +531,13 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
>  {
>  	struct drm_device *dev;
>  	struct drm_plane *primary_planes[MAX_PLANES], *plane;
> +	struct drm_plane *cursor_planes[MAX_PLANES] = { NULL };
>  	struct drm_crtc *crtc;
>  
>  	struct msm_drm_private *priv;
>  	struct dpu_mdss_cfg *catalog;
>  
> -	int primary_planes_idx = 0, i, ret;
> +	int primary_planes_idx = 0, cursor_planes_idx = 0, i, ret;
>  	int max_crtc_count;
>  
>  	if (!dpu_kms || !dpu_kms->dev || !dpu_kms->dev->dev) {
> @@ -556,16 +557,24 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
>  
>  	max_crtc_count = min(catalog->mixer_count, priv->num_encoders);
>  
> -	/* Create the planes */
> +	/* Create the planes, keeping track of one primary/cursor per crtc */
>  	for (i = 0; i < catalog->sspp_count; i++) {
> -		bool primary = true;
> -
> -		if (catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR)
> -			|| primary_planes_idx >= max_crtc_count)
> -			primary = false;
> -
> -		plane = dpu_plane_init(dev, catalog->sspp[i].id, primary,
> -				(1UL << max_crtc_count) - 1, 0);
> +		enum drm_plane_type type;
> +
> +		if ((catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR))
> +			&& cursor_planes_idx < max_crtc_count)
> +			type = DRM_PLANE_TYPE_CURSOR;
> +		else if (primary_planes_idx < max_crtc_count)
> +			type = DRM_PLANE_TYPE_PRIMARY;
> +		else
> +			type = DRM_PLANE_TYPE_OVERLAY;
> +
> +		DPU_DEBUG("Create plane type %d with features %lx (cur %lx)\n",
> +			  type, catalog->sspp[i].features,
> +			  catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR));
> +
> +		plane = dpu_plane_init(dev, catalog->sspp[i].id, type,
> +				       (1UL << max_crtc_count) - 1, 0);
>  		if (IS_ERR(plane)) {
>  			DPU_ERROR("dpu_plane_init failed\n");
>  			ret = PTR_ERR(plane);
> @@ -573,7 +582,9 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
>  		}
>  		priv->planes[priv->num_planes++] = plane;
>  
> -		if (primary)
> +		if (type == DRM_PLANE_TYPE_CURSOR)
> +			cursor_planes[cursor_planes_idx++] = plane;
> +		else if (type == DRM_PLANE_TYPE_PRIMARY)
>  			primary_planes[primary_planes_idx++] = plane;
>  	}
>  
> @@ -581,7 +592,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
>  
>  	/* Create one CRTC per encoder */
>  	for (i = 0; i < max_crtc_count; i++) {
> -		crtc = dpu_crtc_init(dev, primary_planes[i]);
> +		crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]);
>  		if (IS_ERR(crtc)) {
>  			ret = PTR_ERR(crtc);
>  			goto fail;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index b640e39ebaca..efdf9b200dd9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1827,7 +1827,7 @@ bool is_dpu_plane_virtual(struct drm_plane *plane)
>  
>  /* initialize plane */
>  struct drm_plane *dpu_plane_init(struct drm_device *dev,
> -		uint32_t pipe, bool primary_plane,
> +		uint32_t pipe, enum drm_plane_type type,
>  		unsigned long possible_crtcs, u32 master_plane_id)
>  {
>  	struct drm_plane *plane = NULL, *master_plane = NULL;
> @@ -1835,7 +1835,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>  	struct dpu_plane *pdpu;
>  	struct msm_drm_private *priv;
>  	struct dpu_kms *kms;
> -	enum drm_plane_type type;
>  	int zpos_max = DPU_ZPOS_MAX;
>  	int ret = -EINVAL;
>  
> @@ -1916,12 +1915,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>  		goto clean_sspp;
>  	}
>  
> -	if (pdpu->features & BIT(DPU_SSPP_CURSOR))
> -		type = DRM_PLANE_TYPE_CURSOR;
> -	else if (primary_plane)
> -		type = DRM_PLANE_TYPE_PRIMARY;
> -	else
> -		type = DRM_PLANE_TYPE_OVERLAY;
>  	ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,
>  				pdpu->formats, pdpu->nformats,
>  				NULL, type, NULL);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index f6fe6ddc7a3a..7fed0b627708 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -122,7 +122,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error);
>   * dpu_plane_init - create new dpu plane for the given pipe
>   * @dev:   Pointer to DRM device
>   * @pipe:  dpu hardware pipe identifier
> - * @primary_plane: true if this pipe is primary plane for crtc
> + * @type:  Plane type - PRIMARY/OVERLAY/CURSOR
>   * @possible_crtcs: bitmask of crtc that can be attached to the given pipe
>   * @master_plane_id: primary plane id of a multirect pipe. 0 value passed for
>   *                   a regular plane initialization. A non-zero primary plane
> @@ -130,7 +130,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error);
>   *
>   */
>  struct drm_plane *dpu_plane_init(struct drm_device *dev,
> -		uint32_t pipe, bool primary_plane,
> +		uint32_t pipe, enum drm_plane_type type,
>  		unsigned long possible_crtcs, u32 master_plane_id);
>  
>  /**
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 2/2] drm/msm/dpu: fix for cursor blend issue

[Sean Paul]

On Wed, Aug 22, 2018 at 11:19:57PM +0530, Sravanthi Kollukuduru wrote:
> The current driver has the opaque blend mode set as the
> default causing the black box effect around the cursor.
> The fix enables choosing a different blend mode for alpha
> enabled formats.
> 
> Changes in V2:
> 	- Use drm_get_format_name() in the logs (Sean)
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

I've applied this to dpu-staging/for-next.

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 0cd9456a6c4c..07c2d15b45f2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -297,14 +297,29 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
>  }
>  
>  static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
> -		struct dpu_plane_state *pstate)
> +		struct dpu_plane_state *pstate, struct dpu_format *format)
>  {
>  	struct dpu_hw_mixer *lm = mixer->hw_lm;
> +	uint32_t blend_op;
> +	struct drm_format_name_buf format_name;
>  
>  	/* default to opaque blending */
> -	lm->ops.setup_blend_config(lm, pstate->stage, 0XFF, 0,
> -				DPU_BLEND_FG_ALPHA_FG_CONST |
> -				DPU_BLEND_BG_ALPHA_BG_CONST);
> +	blend_op = DPU_BLEND_FG_ALPHA_FG_CONST |
> +		DPU_BLEND_BG_ALPHA_BG_CONST;
> +
> +	if (format->alpha_enable) {
> +		/* coverage blending */
> +		blend_op = DPU_BLEND_FG_ALPHA_FG_PIXEL |
> +			DPU_BLEND_BG_ALPHA_FG_PIXEL |
> +			DPU_BLEND_BG_INV_ALPHA;
> +	}
> +
> +	lm->ops.setup_blend_config(lm, pstate->stage,
> +				0xFF, 0, blend_op);
> +
> +	DPU_DEBUG("format:%s, alpha_en:%u blend_op:0x%x\n",
> +		drm_get_format_name(format->base.pixel_format, &format_name),
> +		format->alpha_enable, blend_op);
>  }
>  
>  static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
> @@ -401,7 +416,8 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>  
>  		/* blend config update */
>  		for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) {
> -			_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate);
> +			_dpu_crtc_setup_blend_cfg(mixer + lm_idx,
> +						pstate, format);
>  
>  			mixer[lm_idx].flush_mask |= flush_mask;
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno