On 10/15/2016 6:38 PM, Rob Clark wrote: > On Sat, Oct 15, 2016 at 8:57 AM, Archit Taneja <architt at 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 at 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 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); >>>>> +} >>>>> + >>>>> 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