[PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Rob Clark]

If the bottom-most layer is not fullscreen, we need to use the BASE
mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
blend_setup() code pretty much handled this already, we just had to
figure this out in _atomic_check() and assign the stages appropriately.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
TODO mdp4 might need similar treatment?

 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 ++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index fa2be7c..e42f62d 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
 		plane_cnt++;
 	}
 
-	/*
-	* If there is no base layer, enable border color.
-	* Although it's not possbile in current blend logic,
-	* put it here as a reminder.
-	*/
 	if (!pstates[STAGE_BASE] && plane_cnt) {
 		ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
 		DBG("Border Color is enabled");
@@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
 	return pa->state->zpos - pb->state->zpos;
 }
 
+/* is there a helper for this? */
+static bool is_fullscreen(struct drm_crtc_state *cstate,
+		struct drm_plane_state *pstate)
+{
+	return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
+		(pstate->crtc_w == cstate->mode.hdisplay) &&
+		(pstate->crtc_h == cstate->mode.vdisplay);
+}
+
 static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
 		struct drm_crtc_state *state)
 {
@@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
 	struct plane_state pstates[STAGE_MAX + 1];
 	const struct mdp5_cfg_hw *hw_cfg;
 	const struct drm_plane_state *pstate;
-	int cnt = 0, i;
+	int cnt = 0, base = 0, i;
 
 	DBG("%s: check", mdp5_crtc->name);
 
-	/* verify that there are not too many planes attached to crtc
-	 * and that we don't have conflicting mixer stages:
-	 */
-	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
 	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
-		if (cnt >= (hw_cfg->lm.nb_stages)) {
-			dev_err(dev->dev, "too many planes!\n");
-			return -EINVAL;
-		}
-
-
 		pstates[cnt].plane = plane;
 		pstates[cnt].state = to_mdp5_plane_state(pstate);
 
@@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
 	/* assign a stage based on sorted zpos property */
 	sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
 
+	/* if the bottom-most layer is not fullscreen, we need to use
+	 * it for solid-color:
+	 */
+	if (!is_fullscreen(state, &pstates[0].state->base))
+		base++;
+
+	/* verify that there are not too many planes attached to crtc
+	 * and that we don't have conflicting mixer stages:
+	 */
+	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
+
+	if ((cnt + base) >= hw_cfg->lm.nb_stages) {
+		dev_err(dev->dev, "too many planes!\n");
+		return -EINVAL;
+	}
+
 	for (i = 0; i < cnt; i++) {
-		pstates[i].state->stage = STAGE_BASE + i;
+		pstates[i].state->stage = STAGE_BASE + i + base;
 		DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
 				pipe2name(mdp5_plane_pipe(pstates[i].plane)),
 				pstates[i].state->stage);
-- 
2.7.4

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

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Archit Taneja]

On 10/13/2016 10:18 PM, Rob Clark wrote:
> If the bottom-most layer is not fullscreen, we need to use the BASE
> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
> blend_setup() code pretty much handled this already, we just had to
> figure this out in _atomic_check() and assign the stages appropriately.

The patch looks good. I couldn't test the problematic scenario since I
don't have an easy way to reproduce it, but it doesn't break regular
usage (where base layer is full screen).

Reviewed-by: Archit Taneja <architt@codeaurora.org>

>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> TODO mdp4 might need similar treatment?
>
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 ++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index fa2be7c..e42f62d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>  		plane_cnt++;
>  	}
>
> -	/*
> -	* If there is no base layer, enable border color.
> -	* Although it's not possbile in current blend logic,
> -	* put it here as a reminder.
> -	*/
>  	if (!pstates[STAGE_BASE] && plane_cnt) {
>  		ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>  		DBG("Border Color is enabled");
> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>  	return pa->state->zpos - pb->state->zpos;
>  }
>
> +/* is there a helper for this? */
> +static bool is_fullscreen(struct drm_crtc_state *cstate,
> +		struct drm_plane_state *pstate)
> +{
> +	return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
> +		(pstate->crtc_w == cstate->mode.hdisplay) &&
> +		(pstate->crtc_h == cstate->mode.vdisplay);
> +}
> +
>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  		struct drm_crtc_state *state)
>  {
> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  	struct plane_state pstates[STAGE_MAX + 1];
>  	const struct mdp5_cfg_hw *hw_cfg;
>  	const struct drm_plane_state *pstate;
> -	int cnt = 0, i;
> +	int cnt = 0, base = 0, i;
>
>  	DBG("%s: check", mdp5_crtc->name);
>
> -	/* verify that there are not too many planes attached to crtc
> -	 * and that we don't have conflicting mixer stages:
> -	 */
> -	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>  	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> -		if (cnt >= (hw_cfg->lm.nb_stages)) {
> -			dev_err(dev->dev, "too many planes!\n");
> -			return -EINVAL;
> -		}
> -
> -
>  		pstates[cnt].plane = plane;
>  		pstates[cnt].state = to_mdp5_plane_state(pstate);
>
> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  	/* assign a stage based on sorted zpos property */
>  	sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>
> +	/* if the bottom-most layer is not fullscreen, we need to use
> +	 * it for solid-color:
> +	 */
> +	if (!is_fullscreen(state, &pstates[0].state->base))
> +		base++;
> +
> +	/* verify that there are not too many planes attached to crtc
> +	 * and that we don't have conflicting mixer stages:
> +	 */
> +	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
> +
> +	if ((cnt + base) >= hw_cfg->lm.nb_stages) {
> +		dev_err(dev->dev, "too many planes!\n");
> +		return -EINVAL;
> +	}
> +
>  	for (i = 0; i < cnt; i++) {
> -		pstates[i].state->stage = STAGE_BASE + i;
> +		pstates[i].state->stage = STAGE_BASE + i + base;
>  		DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>  				pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>  				pstates[i].state->stage);
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Ville Syrjälä]

On Thu, Oct 13, 2016 at 12:48:46PM -0400, Rob Clark wrote:
> If the bottom-most layer is not fullscreen, we need to use the BASE
> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
> blend_setup() code pretty much handled this already, we just had to
> figure this out in _atomic_check() and assign the stages appropriately.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> TODO mdp4 might need similar treatment?
> 
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 ++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index fa2be7c..e42f62d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>  		plane_cnt++;
>  	}
>  
> -	/*
> -	* If there is no base layer, enable border color.
> -	* Although it's not possbile in current blend logic,
> -	* put it here as a reminder.
> -	*/
>  	if (!pstates[STAGE_BASE] && plane_cnt) {
>  		ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>  		DBG("Border Color is enabled");
> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>  	return pa->state->zpos - pb->state->zpos;
>  }
>  
> +/* is there a helper for this? */
> +static bool is_fullscreen(struct drm_crtc_state *cstate,
> +		struct drm_plane_state *pstate)
> +{
> +	return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
> +		(pstate->crtc_w == cstate->mode.hdisplay) &&
> +		(pstate->crtc_h == cstate->mode.vdisplay);
> +}

And what if the plane is larger than the screen size while covering it
fully?

> +
>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  		struct drm_crtc_state *state)
>  {
> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  	struct plane_state pstates[STAGE_MAX + 1];
>  	const struct mdp5_cfg_hw *hw_cfg;
>  	const struct drm_plane_state *pstate;
> -	int cnt = 0, i;
> +	int cnt = 0, base = 0, i;
>  
>  	DBG("%s: check", mdp5_crtc->name);
>  
> -	/* verify that there are not too many planes attached to crtc
> -	 * and that we don't have conflicting mixer stages:
> -	 */
> -	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>  	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> -		if (cnt >= (hw_cfg->lm.nb_stages)) {
> -			dev_err(dev->dev, "too many planes!\n");
> -			return -EINVAL;
> -		}
> -
> -
>  		pstates[cnt].plane = plane;
>  		pstates[cnt].state = to_mdp5_plane_state(pstate);
>  
> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  	/* assign a stage based on sorted zpos property */
>  	sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>  
> +	/* if the bottom-most layer is not fullscreen, we need to use
> +	 * it for solid-color:
> +	 */
> +	if (!is_fullscreen(state, &pstates[0].state->base))
> +		base++;
> +
> +	/* verify that there are not too many planes attached to crtc
> +	 * and that we don't have conflicting mixer stages:
> +	 */
> +	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
> +
> +	if ((cnt + base) >= hw_cfg->lm.nb_stages) {
> +		dev_err(dev->dev, "too many planes!\n");
> +		return -EINVAL;
> +	}
> +
>  	for (i = 0; i < cnt; i++) {
> -		pstates[i].state->stage = STAGE_BASE + i;
> +		pstates[i].state->stage = STAGE_BASE + i + base;
>  		DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>  				pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>  				pstates[i].state->stage);
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Rob Clark]

On Fri, Oct 14, 2016 at 8:45 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Oct 13, 2016 at 12:48:46PM -0400, Rob Clark wrote:
>> If the bottom-most layer is not fullscreen, we need to use the BASE
>> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
>> blend_setup() code pretty much handled this already, we just had to
>> figure this out in _atomic_check() and assign the stages appropriately.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> TODO mdp4 might need similar treatment?
>>
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 ++++++++++++++++++++------------
>>  1 file changed, 27 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> index fa2be7c..e42f62d 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>>               plane_cnt++;
>>       }
>>
>> -     /*
>> -     * If there is no base layer, enable border color.
>> -     * Although it's not possbile in current blend logic,
>> -     * put it here as a reminder.
>> -     */
>>       if (!pstates[STAGE_BASE] && plane_cnt) {
>>               ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>>               DBG("Border Color is enabled");
>> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>>       return pa->state->zpos - pb->state->zpos;
>>  }
>>
>> +/* is there a helper for this? */
>> +static bool is_fullscreen(struct drm_crtc_state *cstate,
>> +             struct drm_plane_state *pstate)
>> +{
>> +     return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
>> +             (pstate->crtc_w == cstate->mode.hdisplay) &&
>> +             (pstate->crtc_h == cstate->mode.vdisplay);
>> +}
>
> And what if the plane is larger than the screen size while covering it
> fully?

good question.. if things aren't clipped elsewhere we probably end up
programming hw badly..

fwiw, it doesn't seem to hurt anything to turn on the solid-fill layer
but we'd be burning an extra mixer stage.  I guess I can make this
cover that case easily enough, but probably doesn't mean that case
works

BR,
-R

>> +
>>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>               struct drm_crtc_state *state)
>>  {
>> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>       struct plane_state pstates[STAGE_MAX + 1];
>>       const struct mdp5_cfg_hw *hw_cfg;
>>       const struct drm_plane_state *pstate;
>> -     int cnt = 0, i;
>> +     int cnt = 0, base = 0, i;
>>
>>       DBG("%s: check", mdp5_crtc->name);
>>
>> -     /* verify that there are not too many planes attached to crtc
>> -      * and that we don't have conflicting mixer stages:
>> -      */
>> -     hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>       drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
>> -             if (cnt >= (hw_cfg->lm.nb_stages)) {
>> -                     dev_err(dev->dev, "too many planes!\n");
>> -                     return -EINVAL;
>> -             }
>> -
>> -
>>               pstates[cnt].plane = plane;
>>               pstates[cnt].state = to_mdp5_plane_state(pstate);
>>
>> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>       /* assign a stage based on sorted zpos property */
>>       sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>>
>> +     /* if the bottom-most layer is not fullscreen, we need to use
>> +      * it for solid-color:
>> +      */
>> +     if (!is_fullscreen(state, &pstates[0].state->base))
>> +             base++;
>> +
>> +     /* verify that there are not too many planes attached to crtc
>> +      * and that we don't have conflicting mixer stages:
>> +      */
>> +     hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>> +
>> +     if ((cnt + base) >= hw_cfg->lm.nb_stages) {
>> +             dev_err(dev->dev, "too many planes!\n");
>> +             return -EINVAL;
>> +     }
>> +
>>       for (i = 0; i < cnt; i++) {
>> -             pstates[i].state->stage = STAGE_BASE + i;
>> +             pstates[i].state->stage = STAGE_BASE + i + base;
>>               DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>>                               pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>>                               pstates[i].state->stage);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Archit Taneja]

On 10/13/2016 10:18 PM, Rob Clark wrote:
> If the bottom-most layer is not fullscreen, we need to use the BASE
> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
> blend_setup() code pretty much handled this already, we just had to
> figure this out in _atomic_check() and assign the stages appropriately.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> TODO mdp4 might need similar treatment?
>
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44 ++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index fa2be7c..e42f62d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>  		plane_cnt++;
>  	}
>
> -	/*
> -	* If there is no base layer, enable border color.
> -	* Although it's not possbile in current blend logic,
> -	* put it here as a reminder.
> -	*/
>  	if (!pstates[STAGE_BASE] && plane_cnt) {
>  		ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>  		DBG("Border Color is enabled");
> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>  	return pa->state->zpos - pb->state->zpos;
>  }
>
> +/* is there a helper for this? */
> +static bool is_fullscreen(struct drm_crtc_state *cstate,
> +		struct drm_plane_state *pstate)
> +{
> +	return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
> +		(pstate->crtc_w == cstate->mode.hdisplay) &&
> +		(pstate->crtc_h == cstate->mode.vdisplay);
> +}
> +
>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  		struct drm_crtc_state *state)
>  {
> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  	struct plane_state pstates[STAGE_MAX + 1];
>  	const struct mdp5_cfg_hw *hw_cfg;
>  	const struct drm_plane_state *pstate;
> -	int cnt = 0, i;
> +	int cnt = 0, base = 0, i;
>
>  	DBG("%s: check", mdp5_crtc->name);
>
> -	/* verify that there are not too many planes attached to crtc
> -	 * and that we don't have conflicting mixer stages:
> -	 */
> -	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>  	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
> -		if (cnt >= (hw_cfg->lm.nb_stages)) {
> -			dev_err(dev->dev, "too many planes!\n");
> -			return -EINVAL;
> -		}
> -
> -
>  		pstates[cnt].plane = plane;
>  		pstates[cnt].state = to_mdp5_plane_state(pstate);
>
> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>  	/* assign a stage based on sorted zpos property */
>  	sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>
> +	/* if the bottom-most layer is not fullscreen, we need to use
> +	 * it for solid-color:
> +	 */
> +	if (!is_fullscreen(state, &pstates[0].state->base))
> +		base++;
> +

I get a crash here when fbcon is enabled and there are no connectors
connected. We're trying to refer pstates[0] when there is no plane
connected to the crtc. I guess we could bail out much earlier if cnt
is 0.

Archit

> +	/* verify that there are not too many planes attached to crtc
> +	 * and that we don't have conflicting mixer stages:
> +	 */
> +	hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
> +
> +	if ((cnt + base) >= hw_cfg->lm.nb_stages) {
> +		dev_err(dev->dev, "too many planes!\n");
> +		return -EINVAL;
> +	}
> +
>  	for (i = 0; i < cnt; i++) {
> -		pstates[i].state->stage = STAGE_BASE + i;
> +		pstates[i].state->stage = STAGE_BASE + i + base;
>  		DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>  				pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>  				pstates[i].state->stage);
>

-- 
Qualcomm Innovation Center, Inc. is a member of 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

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Rob Clark]

On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja <architt@codeaurora.org> wrote:
>
> On 10/13/2016 10:18 PM, Rob Clark wrote:
>>
>> If the bottom-most layer is not fullscreen, we need to use the BASE
>> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
>> blend_setup() code pretty much handled this already, we just had to
>> figure this out in _atomic_check() and assign the stages appropriately.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> TODO mdp4 might need similar treatment?
>>
>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44
>> ++++++++++++++++++++------------
>>  1 file changed, 27 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> index fa2be7c..e42f62d 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>>                 plane_cnt++;
>>         }
>>
>> -       /*
>> -       * If there is no base layer, enable border color.
>> -       * Although it's not possbile in current blend logic,
>> -       * put it here as a reminder.
>> -       */
>>         if (!pstates[STAGE_BASE] && plane_cnt) {
>>                 ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>>                 DBG("Border Color is enabled");
>> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>>         return pa->state->zpos - pb->state->zpos;
>>  }
>>
>> +/* is there a helper for this? */
>> +static bool is_fullscreen(struct drm_crtc_state *cstate,
>> +               struct drm_plane_state *pstate)
>> +{
>> +       return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
>> +               (pstate->crtc_w == cstate->mode.hdisplay) &&
>> +               (pstate->crtc_h == cstate->mode.vdisplay);
>> +}
>> +
>>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>                 struct drm_crtc_state *state)
>>  {
>> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>> *crtc,
>>         struct plane_state pstates[STAGE_MAX + 1];
>>         const struct mdp5_cfg_hw *hw_cfg;
>>         const struct drm_plane_state *pstate;
>> -       int cnt = 0, i;
>> +       int cnt = 0, base = 0, i;
>>
>>         DBG("%s: check", mdp5_crtc->name);
>>
>> -       /* verify that there are not too many planes attached to crtc
>> -        * and that we don't have conflicting mixer stages:
>> -        */
>> -       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>         drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
>> -               if (cnt >= (hw_cfg->lm.nb_stages)) {
>> -                       dev_err(dev->dev, "too many planes!\n");
>> -                       return -EINVAL;
>> -               }
>> -
>> -
>>                 pstates[cnt].plane = plane;
>>                 pstates[cnt].state = to_mdp5_plane_state(pstate);
>>
>> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>> *crtc,
>>         /* assign a stage based on sorted zpos property */
>>         sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>>
>> +       /* if the bottom-most layer is not fullscreen, we need to use
>> +        * it for solid-color:
>> +        */
>> +       if (!is_fullscreen(state, &pstates[0].state->base))
>> +               base++;
>> +
>
>
> I get a crash here when fbcon is enabled and there are no connectors
> connected. We're trying to refer pstates[0] when there is no plane
> connected to the crtc. I guess we could bail out much earlier if cnt
> is 0.

yeah, I hit that last night with no fbcon and killing the UI.. I've
already fixed it up locally with an 'if (pstates[0].state &&
!is_fullscreen())'

BR,
-R

> Archit
>
>> +       /* verify that there are not too many planes attached to crtc
>> +        * and that we don't have conflicting mixer stages:
>> +        */
>> +       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>> +
>> +       if ((cnt + base) >= hw_cfg->lm.nb_stages) {
>> +               dev_err(dev->dev, "too many planes!\n");
>> +               return -EINVAL;
>> +       }
>> +
>>         for (i = 0; i < cnt; i++) {
>> -               pstates[i].state->stage = STAGE_BASE + i;
>> +               pstates[i].state->stage = STAGE_BASE + i + base;
>>                 DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>>
>> pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>>                                 pstates[i].state->stage);
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Archit Taneja]

On 10/15/2016 5:02 PM, Rob Clark wrote:
> On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja <architt@codeaurora.org> wrote:
>>
>> On 10/13/2016 10:18 PM, Rob Clark wrote:
>>>
>>> If the bottom-most layer is not fullscreen, we need to use the BASE
>>> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
>>> blend_setup() code pretty much handled this already, we just had to
>>> figure this out in _atomic_check() and assign the stages appropriately.
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>> TODO mdp4 might need similar treatment?
>>>
>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44
>>> ++++++++++++++++++++------------
>>>  1 file changed, 27 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>> index fa2be7c..e42f62d 100644
>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>>>                 plane_cnt++;
>>>         }
>>>
>>> -       /*
>>> -       * If there is no base layer, enable border color.
>>> -       * Although it's not possbile in current blend logic,
>>> -       * put it here as a reminder.
>>> -       */
>>>         if (!pstates[STAGE_BASE] && plane_cnt) {
>>>                 ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>>>                 DBG("Border Color is enabled");
>>> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>>>         return pa->state->zpos - pb->state->zpos;
>>>  }
>>>
>>> +/* is there a helper for this? */
>>> +static bool is_fullscreen(struct drm_crtc_state *cstate,
>>> +               struct drm_plane_state *pstate)
>>> +{
>>> +       return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
>>> +               (pstate->crtc_w == cstate->mode.hdisplay) &&
>>> +               (pstate->crtc_h == cstate->mode.vdisplay);
>>> +}
>>> +
>>>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>>                 struct drm_crtc_state *state)
>>>  {
>>> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>>> *crtc,
>>>         struct plane_state pstates[STAGE_MAX + 1];
>>>         const struct mdp5_cfg_hw *hw_cfg;
>>>         const struct drm_plane_state *pstate;
>>> -       int cnt = 0, i;
>>> +       int cnt = 0, base = 0, i;
>>>
>>>         DBG("%s: check", mdp5_crtc->name);
>>>
>>> -       /* verify that there are not too many planes attached to crtc
>>> -        * and that we don't have conflicting mixer stages:
>>> -        */
>>> -       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>>         drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
>>> -               if (cnt >= (hw_cfg->lm.nb_stages)) {
>>> -                       dev_err(dev->dev, "too many planes!\n");
>>> -                       return -EINVAL;
>>> -               }
>>> -
>>> -
>>>                 pstates[cnt].plane = plane;
>>>                 pstates[cnt].state = to_mdp5_plane_state(pstate);
>>>
>>> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>>> *crtc,
>>>         /* assign a stage based on sorted zpos property */
>>>         sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>>>
>>> +       /* if the bottom-most layer is not fullscreen, we need to use
>>> +        * it for solid-color:
>>> +        */
>>> +       if (!is_fullscreen(state, &pstates[0].state->base))
>>> +               base++;
>>> +
>>
>>
>> I get a crash here when fbcon is enabled and there are no connectors
>> connected. We're trying to refer pstates[0] when there is no plane
>> connected to the crtc. I guess we could bail out much earlier if cnt
>> is 0.
>
> yeah, I hit that last night with no fbcon and killing the UI.. I've
> already fixed it up locally with an 'if (pstates[0].state &&
> !is_fullscreen())'

Okay, I guess we should probably memset pstates array to 0 in case the
stack memory has some older non NULL stuff in it.

Thanks,
Archit

>
> BR,
> -R
>
>> Archit
>>
>>> +       /* verify that there are not too many planes attached to crtc
>>> +        * and that we don't have conflicting mixer stages:
>>> +        */
>>> +       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>> +
>>> +       if ((cnt + base) >= hw_cfg->lm.nb_stages) {
>>> +               dev_err(dev->dev, "too many planes!\n");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>>         for (i = 0; i < cnt; i++) {
>>> -               pstates[i].state->stage = STAGE_BASE + i;
>>> +               pstates[i].state->stage = STAGE_BASE + i + base;
>>>                 DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>>>
>>> pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>>>                                 pstates[i].state->stage);
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project

-- 
Qualcomm Innovation Center, Inc. is a member of 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

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Rob Clark]

On Sat, Oct 15, 2016 at 8:57 AM, Archit Taneja <architt@codeaurora.org> wrote:
>
>
> On 10/15/2016 5:02 PM, Rob Clark wrote:
>>
>> On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja <architt@codeaurora.org>
>> wrote:
>>>
>>>
>>> On 10/13/2016 10:18 PM, Rob Clark wrote:
>>>>
>>>>
>>>> If the bottom-most layer is not fullscreen, we need to use the BASE
>>>> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
>>>> blend_setup() code pretty much handled this already, we just had to
>>>> figure this out in _atomic_check() and assign the stages appropriately.
>>>>
>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>> ---
>>>> TODO mdp4 might need similar treatment?
>>>>
>>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44
>>>> ++++++++++++++++++++------------
>>>>  1 file changed, 27 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>> index fa2be7c..e42f62d 100644
>>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>>>>                 plane_cnt++;
>>>>         }
>>>>
>>>> -       /*
>>>> -       * If there is no base layer, enable border color.
>>>> -       * Although it's not possbile in current blend logic,
>>>> -       * put it here as a reminder.
>>>> -       */
>>>>         if (!pstates[STAGE_BASE] && plane_cnt) {
>>>>                 ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>>>>                 DBG("Border Color is enabled");
>>>> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>>>>         return pa->state->zpos - pb->state->zpos;
>>>>  }
>>>>
>>>> +/* is there a helper for this? */
>>>> +static bool is_fullscreen(struct drm_crtc_state *cstate,
>>>> +               struct drm_plane_state *pstate)
>>>> +{
>>>> +       return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
>>>> +               (pstate->crtc_w == cstate->mode.hdisplay) &&
>>>> +               (pstate->crtc_h == cstate->mode.vdisplay);
>>>> +}
>>>> +
>>>>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>>>                 struct drm_crtc_state *state)
>>>>  {
>>>> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>>>> *crtc,
>>>>         struct plane_state pstates[STAGE_MAX + 1];
>>>>         const struct mdp5_cfg_hw *hw_cfg;
>>>>         const struct drm_plane_state *pstate;
>>>> -       int cnt = 0, i;
>>>> +       int cnt = 0, base = 0, i;
>>>>
>>>>         DBG("%s: check", mdp5_crtc->name);
>>>>
>>>> -       /* verify that there are not too many planes attached to crtc
>>>> -        * and that we don't have conflicting mixer stages:
>>>> -        */
>>>> -       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>>>         drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state)
>>>> {
>>>> -               if (cnt >= (hw_cfg->lm.nb_stages)) {
>>>> -                       dev_err(dev->dev, "too many planes!\n");
>>>> -                       return -EINVAL;
>>>> -               }
>>>> -
>>>> -
>>>>                 pstates[cnt].plane = plane;
>>>>                 pstates[cnt].state = to_mdp5_plane_state(pstate);
>>>>
>>>> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>>>> *crtc,
>>>>         /* assign a stage based on sorted zpos property */
>>>>         sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>>>>
>>>> +       /* if the bottom-most layer is not fullscreen, we need to use
>>>> +        * it for solid-color:
>>>> +        */
>>>> +       if (!is_fullscreen(state, &pstates[0].state->base))
>>>> +               base++;
>>>> +
>>>
>>>
>>>
>>> I get a crash here when fbcon is enabled and there are no connectors
>>> connected. We're trying to refer pstates[0] when there is no plane
>>> connected to the crtc. I guess we could bail out much earlier if cnt
>>> is 0.
>>
>>
>> yeah, I hit that last night with no fbcon and killing the UI.. I've
>> already fixed it up locally with an 'if (pstates[0].state &&
>> !is_fullscreen())'
>
>
> Okay, I guess we should probably memset pstates array to 0 in case the
> stack memory has some older non NULL stuff in it.

hmm, yeah, you are right.. I wonder how that wasn't already a
problem..  I guess we at least always have an outgoing plane?  I guess
changing the check to '(cnt > 0)' instead of 'pstates[0].state' would
do the trick..

fwiw, I pushed current version of this and the other patches to:

https://github.com/freedreno/kernel-msm/commits/integration-linux-qcomlt-fences-2

BR,
-R

> Thanks,
> Archit
>
>
>>
>> BR,
>> -R
>>
>>> Archit
>>>
>>>> +       /* verify that there are not too many planes attached to crtc
>>>> +        * and that we don't have conflicting mixer stages:
>>>> +        */
>>>> +       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>>> +
>>>> +       if ((cnt + base) >= hw_cfg->lm.nb_stages) {
>>>> +               dev_err(dev->dev, "too many planes!\n");
>>>> +               return -EINVAL;
>>>> +       }
>>>> +
>>>>         for (i = 0; i < cnt; i++) {
>>>> -               pstates[i].state->stage = STAGE_BASE + i;
>>>> +               pstates[i].state->stage = STAGE_BASE + i + base;
>>>>                 DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>>>>
>>>> pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>>>>                                 pstates[i].state->stage);
>>>>
>>>
>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>> a Linux Foundation Collaborative Project
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of 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

Re: [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

[Archit Taneja]

On 10/15/2016 6:38 PM, Rob Clark wrote:
> On Sat, Oct 15, 2016 at 8:57 AM, Archit Taneja <architt@codeaurora.org> wrote:
>>
>>
>> On 10/15/2016 5:02 PM, Rob Clark wrote:
>>>
>>> On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja <architt@codeaurora.org>
>>> wrote:
>>>>
>>>>
>>>> On 10/13/2016 10:18 PM, Rob Clark wrote:
>>>>>
>>>>>
>>>>> If the bottom-most layer is not fullscreen, we need to use the BASE
>>>>> mixer stage for solid fill (ie. MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT).  The
>>>>> blend_setup() code pretty much handled this already, we just had to
>>>>> figure this out in _atomic_check() and assign the stages appropriately.
>>>>>
>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>> ---
>>>>> TODO mdp4 might need similar treatment?
>>>>>
>>>>>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 44
>>>>> ++++++++++++++++++++------------
>>>>>  1 file changed, 27 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>>> index fa2be7c..e42f62d 100644
>>>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
>>>>> @@ -223,11 +223,6 @@ static void blend_setup(struct drm_crtc *crtc)
>>>>>                 plane_cnt++;
>>>>>         }
>>>>>
>>>>> -       /*
>>>>> -       * If there is no base layer, enable border color.
>>>>> -       * Although it's not possbile in current blend logic,
>>>>> -       * put it here as a reminder.
>>>>> -       */
>>>>>         if (!pstates[STAGE_BASE] && plane_cnt) {
>>>>>                 ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT;
>>>>>                 DBG("Border Color is enabled");
>>>>> @@ -365,6 +360,15 @@ static int pstate_cmp(const void *a, const void *b)
>>>>>         return pa->state->zpos - pb->state->zpos;
>>>>>  }
>>>>>
>>>>> +/* is there a helper for this? */
>>>>> +static bool is_fullscreen(struct drm_crtc_state *cstate,
>>>>> +               struct drm_plane_state *pstate)
>>>>> +{
>>>>> +       return (pstate->crtc_x == 0) && (pstate->crtc_y == 0) &&
>>>>> +               (pstate->crtc_w == cstate->mode.hdisplay) &&
>>>>> +               (pstate->crtc_h == cstate->mode.vdisplay);
>>>>> +}
>>>>> +
>>>>>  static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
>>>>>                 struct drm_crtc_state *state)
>>>>>  {
>>>>> @@ -375,21 +379,11 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>>>>> *crtc,
>>>>>         struct plane_state pstates[STAGE_MAX + 1];
>>>>>         const struct mdp5_cfg_hw *hw_cfg;
>>>>>         const struct drm_plane_state *pstate;
>>>>> -       int cnt = 0, i;
>>>>> +       int cnt = 0, base = 0, i;
>>>>>
>>>>>         DBG("%s: check", mdp5_crtc->name);
>>>>>
>>>>> -       /* verify that there are not too many planes attached to crtc
>>>>> -        * and that we don't have conflicting mixer stages:
>>>>> -        */
>>>>> -       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>>>>         drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state)
>>>>> {
>>>>> -               if (cnt >= (hw_cfg->lm.nb_stages)) {
>>>>> -                       dev_err(dev->dev, "too many planes!\n");
>>>>> -                       return -EINVAL;
>>>>> -               }
>>>>> -
>>>>> -
>>>>>                 pstates[cnt].plane = plane;
>>>>>                 pstates[cnt].state = to_mdp5_plane_state(pstate);
>>>>>
>>>>> @@ -399,8 +393,24 @@ static int mdp5_crtc_atomic_check(struct drm_crtc
>>>>> *crtc,
>>>>>         /* assign a stage based on sorted zpos property */
>>>>>         sort(pstates, cnt, sizeof(pstates[0]), pstate_cmp, NULL);
>>>>>
>>>>> +       /* if the bottom-most layer is not fullscreen, we need to use
>>>>> +        * it for solid-color:
>>>>> +        */
>>>>> +       if (!is_fullscreen(state, &pstates[0].state->base))
>>>>> +               base++;
>>>>> +
>>>>
>>>>
>>>>
>>>> I get a crash here when fbcon is enabled and there are no connectors
>>>> connected. We're trying to refer pstates[0] when there is no plane
>>>> connected to the crtc. I guess we could bail out much earlier if cnt
>>>> is 0.
>>>
>>>
>>> yeah, I hit that last night with no fbcon and killing the UI.. I've
>>> already fixed it up locally with an 'if (pstates[0].state &&
>>> !is_fullscreen())'
>>
>>
>> Okay, I guess we should probably memset pstates array to 0 in case the
>> stack memory has some older non NULL stuff in it.
>
> hmm, yeah, you are right.. I wonder how that wasn't already a
> problem..  I guess we at least always have an outgoing plane?  I guess
> changing the check to '(cnt > 0)' instead of 'pstates[0].state' would
> do the trick..
>
> fwiw, I pushed current version of this and the other patches to:
>
> https://github.com/freedreno/kernel-msm/commits/integration-linux-qcomlt-fences-2

thanks! I'll start using these patches too.

Archit

>
> BR,
> -R
>
>> Thanks,
>> Archit
>>
>>
>>>
>>> BR,
>>> -R
>>>
>>>> Archit
>>>>
>>>>> +       /* verify that there are not too many planes attached to crtc
>>>>> +        * and that we don't have conflicting mixer stages:
>>>>> +        */
>>>>> +       hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>>>> +
>>>>> +       if ((cnt + base) >= hw_cfg->lm.nb_stages) {
>>>>> +               dev_err(dev->dev, "too many planes!\n");
>>>>> +               return -EINVAL;
>>>>> +       }
>>>>> +
>>>>>         for (i = 0; i < cnt; i++) {
>>>>> -               pstates[i].state->stage = STAGE_BASE + i;
>>>>> +               pstates[i].state->stage = STAGE_BASE + i + base;
>>>>>                 DBG("%s: assign pipe %s on stage=%d", mdp5_crtc->name,
>>>>>
>>>>> pipe2name(mdp5_plane_pipe(pstates[i].plane)),
>>>>>                                 pstates[i].state->stage);
>>>>>
>>>>
>>>> --
>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>>> a Linux Foundation Collaborative Project
>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno