On Sat, Oct 15, 2016 at 8:57 AM, Archit Taneja <architt@xxxxxxxxxxxxxx> wrote:
>
>
> On 10/15/2016 5:02 PM, Rob Clark wrote:
>>
>> On Sat, Oct 15, 2016 at 5:39 AM, Archit Taneja <architt@xxxxxxxxxxxxxx>
>> 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@xxxxxxxxx>
>>>> ---
>>>> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
