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

2016-10-15 Thread Archit Taneja 



On 10/15/2016 6:38 PM, Rob Clark wrote:

On Sat, Oct 15, 2016 at 8:57 AM, Archit Taneja  wrote:



On 10/15/2016 5:02 PM, Rob Clark wrote:


On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja 
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 
---
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 Foundat

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

2016-10-15 Thread Rob Clark 
On Sat, Oct 15, 2016 at 8:57 AM, Archit Taneja  wrote:
>
>
> On 10/15/2016 5:02 PM, Rob Clark wrote:
>>
>> On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja 
>> 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 
 ---
 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;
 +

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

2016-10-15 Thread Archit Taneja 



On 10/15/2016 5:02 PM, Rob Clark wrote:

On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja  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 
---
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
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

2016-10-15 Thread Rob Clark 
On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja  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 
>> ---
>> 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: [Freedreno] [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

2016-10-15 Thread 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 
---
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
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

2016-10-14 Thread Rob Clark 
On Fri, Oct 14, 2016 at 8:45 AM, Ville Syrjälä
 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 
>> ---
>> 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-de...@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: [Freedreno] [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

2016-10-14 Thread 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 
> ---
> 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-de...@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: [Freedreno] [PATCH] drm/msm/mdp5: handle non-fullscreen base plane case

2016-10-14 Thread 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 



Signed-off-by: Rob Clark 
---
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