[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 at 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 at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC