diff mbox series

drm/msm/dpu: enable cursor plane on dpu

Message ID 1533207284-4326-1-git-send-email-skolluku@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series drm/msm/dpu: enable cursor plane on dpu | expand

Commit Message

Sravanthi Kollukuduru Aug. 2, 2018, 10:54 a.m. UTC
Reserve DMA pipe for cursor plane and attach it to the
crtc during the initialization.

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(-)

Comments

Sean Paul Aug. 3, 2018, 3:37 p.m. UTC | #1
On Thu, Aug 02, 2018 at 04:24:44PM +0530, Sravanthi Kollukuduru wrote:
> Reserve DMA pipe for cursor plane and attach it to the
> crtc during the initialization.
> 
> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>

Hi Sravanthi,
Thanks for sending this. I have a few comments, mostly based off my own cursor
patch [1] I posted last week. It's mostly the same as what you have here, but
takes a couple different things into consideration.

[1]-
https://gitlab.freedesktop.org/seanpaul/dpu-staging/commit/fde9f11f8f3be3ceaacfd0751e250a3c03476a8c

> ---
>  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(-)
> 

/snip

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 8d4678d..dc647ee 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)

You don't need to check that the index is less than max_crtc_count, it'll fit in
the array regardless.

> +			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]);

Here you assume that there is at least one cursor per crtc. That might not be
the case. Check out how I handle this in my patch with max_cursor_planes, it's
a bit more safe than this.



>  		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 b640e39..efdf9b2 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 f6fe6dd..7fed0b6 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_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
>   * 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_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
>   *
>   */
>  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
>
Sravanthi Kollukuduru Aug. 6, 2018, 12:26 p.m. UTC | #2
On 2018-08-03 21:07, Sean Paul wrote:
> On Thu, Aug 02, 2018 at 04:24:44PM +0530, Sravanthi Kollukuduru wrote:
>> Reserve DMA pipe for cursor plane and attach it to the
>> crtc during the initialization.
>> 
>> Signed-off-by: Sravanthi Kollukuduru <skolluku@codeaurora.org>
> 
> Hi Sravanthi,
> Thanks for sending this. I have a few comments, mostly based off my own 
> cursor
> patch [1] I posted last week. It's mostly the same as what you have 
> here, but
> takes a couple different things into consideration.
> 
> [1]-
> https://gitlab.freedesktop.org/seanpaul/dpu-staging/commit/fde9f11f8f3be3ceaacfd0751e250a3c03476a8c
> 
>> ---
>>  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(-)
>> 
> 
> /snip
> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> index 8d4678d..dc647ee 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)
> 
> You don't need to check that the index is less than max_crtc_count, 
> it'll fit in
> the array regardless.
> 
The idea of adding this is not to address array bounds but to avoid 
marking
the plane as cursor when it actually won't be used as one.
That is, based on catalog info, two DMA pipes are marked for cursor 
planes but since currently,
we have just one crtc, there is no need to expose two cursor planes and 
instead
use the other one as overlay.
Basically, the crtc count should take precedence over pipe capabilities 
while deciding
the number of cursor planes required.

>> +			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]);
> 
> Here you assume that there is at least one cursor per crtc. That might 
> not be
> the case. Check out how I handle this in my patch with 
> max_cursor_planes, it's
> a bit more safe than this.
> 
As the cursor planes array is inititalized to NULL, there shouldn't be 
any issue if
there is no cursor assignment for a given crtc.

Thanks,
Sravanthi
> 
> 
>>  		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 b640e39..efdf9b2 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 f6fe6dd..7fed0b6 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_get_ctl_flush(struct drm_plane 
>> *plane, struct dpu_hw_ctl *ctl,
>>   * 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_get_ctl_flush(struct drm_plane 
>> *plane, struct dpu_hw_ctl *ctl,
>>   *
>>   */
>>  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
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 7ac0e0d..25eba80 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -2456,7 +2456,8 @@  static int _dpu_crtc_init_events(struct dpu_crtc *dpu_crtc)
 }
 
 /* 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;
@@ -2493,7 +2494,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 4a97ba1..a7e3173 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -396,9 +396,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 1793cfd..f53e42d 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_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 8d4678d..dc647ee 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 b640e39..efdf9b2 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 f6fe6dd..7fed0b6 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_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
  * 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_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
  *
  */
 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);
 
 /**