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. 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