| Message ID | 20241009-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-21-v2-12-76d4f5d413bf@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dpu: Support quad pipe with dual-DSI | expand |
On Wed, Oct 09, 2024 at 04:50:25PM GMT, Jun Nie wrote: > Clip plane into pipes per left and right half screen ROI if topology > is quad pipe. Why? Please provide an explanation for the reviewers not knowing the details. > Then split the clipped rectangle by half if the rectangle > width still exceeds width limit. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 7 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 6 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 99 ++++++++++++++++++++++--------- > 3 files changed, 84 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 66f745399a602..d2aca0a9493d5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -1310,6 +1310,13 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en) > return 0; > } > > +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state) I think the DPU driver uses num_foo rather than foo_num > +{ > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); > + > + return cstate->num_mixers; > +} > + > #ifdef CONFIG_DEBUG_FS > static int _dpu_debugfs_status_show(struct seq_file *s, void *data) > { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > index 5260e2440f059..ee7cf71f89fc7 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > @@ -304,4 +304,10 @@ static inline enum dpu_crtc_client_type dpu_crtc_get_client_type( > > void dpu_crtc_frame_event_cb(struct drm_crtc *crtc, u32 event); > > +/** > + * dpu_crtc_get_lm_num - Get mixer number in this CRTC pipeline > + * state: Pointer to drm crtc state object > + */ > +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state); > + > #endif /* _DPU_CRTC_H_ */ > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > index 898fc2937954e..480a1b46aba72 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -837,10 +837,12 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base); > u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate; > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); > - struct dpu_sw_pipe_cfg *pipe_cfg; > - struct dpu_sw_pipe_cfg *r_pipe_cfg; > + struct dpu_sw_pipe_cfg pipe_cfg; > struct drm_rect fb_rect = { 0 }; > + const struct drm_display_mode *mode = &crtc_state->adjusted_mode; > uint32_t max_linewidth; > + u32 lm_num; > + int lmcfg_id, lmcfg_num; > > min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO); > max_scale = MAX_DOWNSCALE_RATIO << 16; > @@ -863,13 +865,10 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > return -EINVAL; > } > > - /* move the assignment here, to ease handling to another pairs later */ > - pipe_cfg = &pstate->pipe_cfg[0]; > - r_pipe_cfg = &pstate->pipe_cfg[1]; > - /* state->src is 16.16, src_rect is not */ > - drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); > + lm_num = dpu_crtc_get_lm_num(crtc_state); > > - pipe_cfg->dst_rect = new_plane_state->dst; > + /* state->src is 16.16, src_rect is not */ > + drm_rect_fp_to_int(&pipe_cfg.src_rect, &new_plane_state->src); > > fb_rect.x2 = new_plane_state->fb->width; > fb_rect.y2 = new_plane_state->fb->height; > @@ -884,34 +883,78 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > > max_linewidth = pdpu->catalog->caps->max_linewidth; > > - drm_rect_rotate(&pipe_cfg->src_rect, > + drm_rect_rotate(&pipe_cfg.src_rect, > new_plane_state->fb->width, new_plane_state->fb->height, > new_plane_state->rotation); > > - if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) || > - _dpu_plane_calc_clk(&crtc_state->adjusted_mode, pipe_cfg) > max_mdp_clk_rate) { > - if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) { > - DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", > - DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth); > - return -E2BIG; > + /* > + * We have 1 mixer pair cfg for 1:1:1 and 2:2:1 topology, 2 mixer pair > + * configs for left and right half screen in case of 4:4:2 topology. > + * But we may have 2 rect to split plane with 1 config for 2:2:1. > + * So need to handle super wide plane splitting, and plane on right half > + * for quad-pipe case. Check dest rectangle left/right clipping > + * first, then check super wide rectangle splitting in every half next. > + */ > + lmcfg_num = (lm_num + 1) / 2; num_stages? > + /* iterate mixer configs for this plane, to separate left/right with the id */ > + for (lmcfg_id = 0; lmcfg_id < lmcfg_num; lmcfg_id++) { > + struct drm_rect mixer_rect = {lmcfg_id * mode->hdisplay / lmcfg_num, 0, > + (lmcfg_id + 1) * mode->hdisplay / lmcfg_num, mode->vdisplay}; > + int cfg_idx = lmcfg_id * PIPES_PER_LM_PAIR; > + struct dpu_sw_pipe_cfg *cur_pipecfg = &pstate->pipe_cfg[cfg_idx]; > + > + drm_rect_fp_to_int(&cur_pipecfg->src_rect, &new_plane_state->src); > + cur_pipecfg->dst_rect = new_plane_state->dst; > + > + DPU_DEBUG_PLANE(pdpu, "checking src " DRM_RECT_FMT > + " vs clip window " DRM_RECT_FMT "\n", > + DRM_RECT_ARG(&cur_pipecfg->src_rect), > + DRM_RECT_ARG(&mixer_rect)); > + > + /* If this plane does not fall into mixer rect, check next mixer rect */ > + if (!drm_rect_clip_scaled(&cur_pipecfg->src_rect, &cur_pipecfg->dst_rect, &mixer_rect)) { > + memset(&pstate->pipe_cfg[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe_cfg)); > + memset(&pstate->pipe[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe)); > + continue; > } > > - *r_pipe_cfg = *pipe_cfg; > - pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1; > - pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1; > - r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2; > - r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; > - } else { > - memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg)); > - } > + cur_pipecfg->valid = true; ... and checks have been broken up to now. This isn't good. > + cur_pipecfg->dst_rect.x1 -= mixer_rect.x1; > + cur_pipecfg->dst_rect.x2 -= mixer_rect.x1; > + > + DPU_DEBUG_PLANE(pdpu, "Got clip src:" DRM_RECT_FMT " dst: " DRM_RECT_FMT "\n", > + DRM_RECT_ARG(&cur_pipecfg->src_rect), DRM_RECT_ARG(&cur_pipecfg->dst_rect)); > + > + /* Split super wide rect into 2 rect */ > + if ((drm_rect_width(&cur_pipecfg->src_rect) > max_linewidth) || > + _dpu_plane_calc_clk(mode, cur_pipecfg) > max_mdp_clk_rate) { > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[cfg_idx + 1]; > + > + if (drm_rect_width(&cur_pipecfg->src_rect) > 2 * max_linewidth) { > + DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", > + DRM_RECT_ARG(&cur_pipecfg->src_rect), max_linewidth); > + return -E2BIG; > + } > + > + memcpy(r_pipe_cfg, cur_pipecfg, sizeof(struct dpu_sw_pipe_cfg)); > + cur_pipecfg->src_rect.x2 = (cur_pipecfg->src_rect.x1 + cur_pipecfg->src_rect.x2) >> 1; > + cur_pipecfg->dst_rect.x2 = (cur_pipecfg->dst_rect.x1 + cur_pipecfg->dst_rect.x2) >> 1; pipe_cfg. If you need, rename the topmost var name. > + r_pipe_cfg->src_rect.x1 = cur_pipecfg->src_rect.x2; > + r_pipe_cfg->dst_rect.x1 = cur_pipecfg->dst_rect.x2; > + r_pipe_cfg->valid = true; > + DPU_DEBUG_PLANE(pdpu, "Split super wide plane into:" > + DRM_RECT_FMT " and " DRM_RECT_FMT "\n", > + DRM_RECT_ARG(&cur_pipecfg->src_rect), > + DRM_RECT_ARG(&r_pipe_cfg->src_rect)); > + } else { > + memset(&pstate->pipe_cfg[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe_cfg)); > + memset(&pstate->pipe[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe)); Please keep using r_pipe_cfg here. > + } > > - drm_rect_rotate_inv(&pipe_cfg->src_rect, > - new_plane_state->fb->width, new_plane_state->fb->height, > - new_plane_state->rotation); > - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) > - drm_rect_rotate_inv(&r_pipe_cfg->src_rect, > + drm_rect_rotate_inv(&cur_pipecfg->src_rect, > new_plane_state->fb->width, new_plane_state->fb->height, > new_plane_state->rotation); > + } > > pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state); > > > -- > 2.34.1 >
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:29写道： > > On Wed, Oct 09, 2024 at 04:50:25PM GMT, Jun Nie wrote: > > Clip plane into pipes per left and right half screen ROI if topology > > is quad pipe. > > Why? Please provide an explanation for the reviewers not knowing the > details. The content of every half of screen is sent out via one interface in dual-DSI case. The content for every interface is blended by a LM pair, thus no content of any pipe shall cross the LM pair. We need to clip plane into pipes per left and right half screen ROI if topology is quad pipe. > > > Then split the clipped rectangle by half if the rectangle > > width still exceeds width limit. > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 7 +++ > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 6 ++ > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 99 ++++++++++++++++++++++--------- > > 3 files changed, 84 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > index 66f745399a602..d2aca0a9493d5 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > @@ -1310,6 +1310,13 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en) > > return 0; > > } > > > > +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state) > > I think the DPU driver uses num_foo rather than foo_num dpu_crtc_get_num_lm() > > > +{ > > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); > > + > > + return cstate->num_mixers; > > +} > > + > > #ifdef CONFIG_DEBUG_FS > > static int _dpu_debugfs_status_show(struct seq_file *s, void *data) > > { > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > > index 5260e2440f059..ee7cf71f89fc7 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > > @@ -304,4 +304,10 @@ static inline enum dpu_crtc_client_type dpu_crtc_get_client_type( > > > > void dpu_crtc_frame_event_cb(struct drm_crtc *crtc, u32 event); > > > > +/** > > + * dpu_crtc_get_lm_num - Get mixer number in this CRTC pipeline > > + * state: Pointer to drm crtc state object > > + */ > > +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state); > > + > > #endif /* _DPU_CRTC_H_ */ > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > index 898fc2937954e..480a1b46aba72 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > @@ -837,10 +837,12 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > > struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base); > > u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate; > > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); > > - struct dpu_sw_pipe_cfg *pipe_cfg; > > - struct dpu_sw_pipe_cfg *r_pipe_cfg; > > + struct dpu_sw_pipe_cfg pipe_cfg; > > struct drm_rect fb_rect = { 0 }; > > + const struct drm_display_mode *mode = &crtc_state->adjusted_mode; > > uint32_t max_linewidth; > > + u32 lm_num; > > + int lmcfg_id, lmcfg_num; > > > > min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO); > > max_scale = MAX_DOWNSCALE_RATIO << 16; > > @@ -863,13 +865,10 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > > return -EINVAL; > > } > > > > - /* move the assignment here, to ease handling to another pairs later */ > > - pipe_cfg = &pstate->pipe_cfg[0]; > > - r_pipe_cfg = &pstate->pipe_cfg[1]; > > - /* state->src is 16.16, src_rect is not */ > > - drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); > > + lm_num = dpu_crtc_get_lm_num(crtc_state); > > > > - pipe_cfg->dst_rect = new_plane_state->dst; > > + /* state->src is 16.16, src_rect is not */ > > + drm_rect_fp_to_int(&pipe_cfg.src_rect, &new_plane_state->src); > > > > fb_rect.x2 = new_plane_state->fb->width; > > fb_rect.y2 = new_plane_state->fb->height; > > @@ -884,34 +883,78 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > > > > max_linewidth = pdpu->catalog->caps->max_linewidth; > > > > - drm_rect_rotate(&pipe_cfg->src_rect, > > + drm_rect_rotate(&pipe_cfg.src_rect, > > new_plane_state->fb->width, new_plane_state->fb->height, > > new_plane_state->rotation); > > > > - if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) || > > - _dpu_plane_calc_clk(&crtc_state->adjusted_mode, pipe_cfg) > max_mdp_clk_rate) { > > - if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) { > > - DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", > > - DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth); > > - return -E2BIG; > > + /* > > + * We have 1 mixer pair cfg for 1:1:1 and 2:2:1 topology, 2 mixer pair > > + * configs for left and right half screen in case of 4:4:2 topology. > > + * But we may have 2 rect to split plane with 1 config for 2:2:1. > > + * So need to handle super wide plane splitting, and plane on right half > > + * for quad-pipe case. Check dest rectangle left/right clipping > > + * first, then check super wide rectangle splitting in every half next. > > + */ > > + lmcfg_num = (lm_num + 1) / 2; > > num_stages? OK. Then lmcfg_id -> stage_id > > > + /* iterate mixer configs for this plane, to separate left/right with the id */ > > + for (lmcfg_id = 0; lmcfg_id < lmcfg_num; lmcfg_id++) { > > + struct drm_rect mixer_rect = {lmcfg_id * mode->hdisplay / lmcfg_num, 0, > > + (lmcfg_id + 1) * mode->hdisplay / lmcfg_num, mode->vdisplay}; > > + int cfg_idx = lmcfg_id * PIPES_PER_LM_PAIR; > > + struct dpu_sw_pipe_cfg *cur_pipecfg = &pstate->pipe_cfg[cfg_idx]; > > + > > + drm_rect_fp_to_int(&cur_pipecfg->src_rect, &new_plane_state->src); > > + cur_pipecfg->dst_rect = new_plane_state->dst; > > + > > + DPU_DEBUG_PLANE(pdpu, "checking src " DRM_RECT_FMT > > + " vs clip window " DRM_RECT_FMT "\n", > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), > > + DRM_RECT_ARG(&mixer_rect)); > > + > > + /* If this plane does not fall into mixer rect, check next mixer rect */ > > + if (!drm_rect_clip_scaled(&cur_pipecfg->src_rect, &cur_pipecfg->dst_rect, &mixer_rect)) { > > + memset(&pstate->pipe_cfg[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe_cfg)); > > + memset(&pstate->pipe[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe)); > > + continue; > > } > > > > - *r_pipe_cfg = *pipe_cfg; > > - pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1; > > - pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1; > > - r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2; > > - r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; > > - } else { > > - memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg)); > > - } > > + cur_pipecfg->valid = true; > > ... and checks have been broken up to now. This isn't good. Will move this patch before the plane checking patch. > > > + cur_pipecfg->dst_rect.x1 -= mixer_rect.x1; > > + cur_pipecfg->dst_rect.x2 -= mixer_rect.x1; > > + > > + DPU_DEBUG_PLANE(pdpu, "Got clip src:" DRM_RECT_FMT " dst: " DRM_RECT_FMT "\n", > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), DRM_RECT_ARG(&cur_pipecfg->dst_rect)); > > + > > + /* Split super wide rect into 2 rect */ > > + if ((drm_rect_width(&cur_pipecfg->src_rect) > max_linewidth) || > > + _dpu_plane_calc_clk(mode, cur_pipecfg) > max_mdp_clk_rate) { > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[cfg_idx + 1]; > > + > > + if (drm_rect_width(&cur_pipecfg->src_rect) > 2 * max_linewidth) { > > + DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), max_linewidth); > > + return -E2BIG; > > + } > > + > > + memcpy(r_pipe_cfg, cur_pipecfg, sizeof(struct dpu_sw_pipe_cfg)); > > + cur_pipecfg->src_rect.x2 = (cur_pipecfg->src_rect.x1 + cur_pipecfg->src_rect.x2) >> 1; > > + cur_pipecfg->dst_rect.x2 = (cur_pipecfg->dst_rect.x1 + cur_pipecfg->dst_rect.x2) >> 1; > > pipe_cfg. If you need, rename the topmost var name. OK. pipe_cfg_cur ? > > > + r_pipe_cfg->src_rect.x1 = cur_pipecfg->src_rect.x2; > > + r_pipe_cfg->dst_rect.x1 = cur_pipecfg->dst_rect.x2; > > + r_pipe_cfg->valid = true; > > + DPU_DEBUG_PLANE(pdpu, "Split super wide plane into:" > > + DRM_RECT_FMT " and " DRM_RECT_FMT "\n", > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), > > + DRM_RECT_ARG(&r_pipe_cfg->src_rect)); > > + } else { > > + memset(&pstate->pipe_cfg[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe_cfg)); > > + memset(&pstate->pipe[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe)); > > Please keep using r_pipe_cfg here. OK, will make r_pipe_cfg a variable in function scope, not bracket scope. > > > + } > > > > - drm_rect_rotate_inv(&pipe_cfg->src_rect, > > - new_plane_state->fb->width, new_plane_state->fb->height, > > - new_plane_state->rotation); > > - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) > > - drm_rect_rotate_inv(&r_pipe_cfg->src_rect, > > + drm_rect_rotate_inv(&cur_pipecfg->src_rect, > > new_plane_state->fb->width, new_plane_state->fb->height, > > new_plane_state->rotation); > > + } > > > > pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state); > > > > > > -- > > 2.34.1 > > > > -- > With best wishes > Dmitry
On Fri, 11 Oct 2024 at 11:13, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:29写道： > > > > On Wed, Oct 09, 2024 at 04:50:25PM GMT, Jun Nie wrote: > > > Clip plane into pipes per left and right half screen ROI if topology > > > is quad pipe. > > > > Why? Please provide an explanation for the reviewers not knowing the > > details. > > The content of every half of screen is sent out via one interface in > dual-DSI case. The content for every interface is blended by a LM > pair, thus no content of any pipe shall cross the LM pair. We need > to clip plane into pipes per left and right half screen ROI if topology > is quad pipe. => commit message. > > > > > Then split the clipped rectangle by half if the rectangle > > > width still exceeds width limit. > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 7 +++ > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 6 ++ > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 99 ++++++++++++++++++++++--------- > > > 3 files changed, 84 insertions(+), 28 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > index 66f745399a602..d2aca0a9493d5 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > @@ -1310,6 +1310,13 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en) > > > return 0; > > > } > > > > > > +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state) > > > > I think the DPU driver uses num_foo rather than foo_num > > dpu_crtc_get_num_lm() > > > > > +{ > > > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); > > > + > > > + return cstate->num_mixers; > > > +} > > > + > > > #ifdef CONFIG_DEBUG_FS > > > static int _dpu_debugfs_status_show(struct seq_file *s, void *data) > > > { > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > > > index 5260e2440f059..ee7cf71f89fc7 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h > > > @@ -304,4 +304,10 @@ static inline enum dpu_crtc_client_type dpu_crtc_get_client_type( > > > > > > void dpu_crtc_frame_event_cb(struct drm_crtc *crtc, u32 event); > > > > > > +/** > > > + * dpu_crtc_get_lm_num - Get mixer number in this CRTC pipeline > > > + * state: Pointer to drm crtc state object > > > + */ > > > +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state); > > > + > > > #endif /* _DPU_CRTC_H_ */ > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > > index 898fc2937954e..480a1b46aba72 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > > @@ -837,10 +837,12 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > > > struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base); > > > u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate; > > > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); > > > - struct dpu_sw_pipe_cfg *pipe_cfg; > > > - struct dpu_sw_pipe_cfg *r_pipe_cfg; > > > + struct dpu_sw_pipe_cfg pipe_cfg; > > > struct drm_rect fb_rect = { 0 }; > > > + const struct drm_display_mode *mode = &crtc_state->adjusted_mode; > > > uint32_t max_linewidth; > > > + u32 lm_num; > > > + int lmcfg_id, lmcfg_num; > > > > > > min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO); > > > max_scale = MAX_DOWNSCALE_RATIO << 16; > > > @@ -863,13 +865,10 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > > > return -EINVAL; > > > } > > > > > > - /* move the assignment here, to ease handling to another pairs later */ > > > - pipe_cfg = &pstate->pipe_cfg[0]; > > > - r_pipe_cfg = &pstate->pipe_cfg[1]; > > > - /* state->src is 16.16, src_rect is not */ > > > - drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); > > > + lm_num = dpu_crtc_get_lm_num(crtc_state); > > > > > > - pipe_cfg->dst_rect = new_plane_state->dst; > > > + /* state->src is 16.16, src_rect is not */ > > > + drm_rect_fp_to_int(&pipe_cfg.src_rect, &new_plane_state->src); > > > > > > fb_rect.x2 = new_plane_state->fb->width; > > > fb_rect.y2 = new_plane_state->fb->height; > > > @@ -884,34 +883,78 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, > > > > > > max_linewidth = pdpu->catalog->caps->max_linewidth; > > > > > > - drm_rect_rotate(&pipe_cfg->src_rect, > > > + drm_rect_rotate(&pipe_cfg.src_rect, > > > new_plane_state->fb->width, new_plane_state->fb->height, > > > new_plane_state->rotation); > > > > > > - if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) || > > > - _dpu_plane_calc_clk(&crtc_state->adjusted_mode, pipe_cfg) > max_mdp_clk_rate) { > > > - if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) { > > > - DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", > > > - DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth); > > > - return -E2BIG; > > > + /* > > > + * We have 1 mixer pair cfg for 1:1:1 and 2:2:1 topology, 2 mixer pair > > > + * configs for left and right half screen in case of 4:4:2 topology. > > > + * But we may have 2 rect to split plane with 1 config for 2:2:1. > > > + * So need to handle super wide plane splitting, and plane on right half > > > + * for quad-pipe case. Check dest rectangle left/right clipping > > > + * first, then check super wide rectangle splitting in every half next. > > > + */ > > > + lmcfg_num = (lm_num + 1) / 2; > > > > num_stages? > > OK. Then lmcfg_id -> stage_id > > > > > + /* iterate mixer configs for this plane, to separate left/right with the id */ > > > + for (lmcfg_id = 0; lmcfg_id < lmcfg_num; lmcfg_id++) { > > > + struct drm_rect mixer_rect = {lmcfg_id * mode->hdisplay / lmcfg_num, 0, > > > + (lmcfg_id + 1) * mode->hdisplay / lmcfg_num, mode->vdisplay}; > > > + int cfg_idx = lmcfg_id * PIPES_PER_LM_PAIR; > > > + struct dpu_sw_pipe_cfg *cur_pipecfg = &pstate->pipe_cfg[cfg_idx]; > > > + > > > + drm_rect_fp_to_int(&cur_pipecfg->src_rect, &new_plane_state->src); > > > + cur_pipecfg->dst_rect = new_plane_state->dst; > > > + > > > + DPU_DEBUG_PLANE(pdpu, "checking src " DRM_RECT_FMT > > > + " vs clip window " DRM_RECT_FMT "\n", > > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), > > > + DRM_RECT_ARG(&mixer_rect)); > > > + > > > + /* If this plane does not fall into mixer rect, check next mixer rect */ > > > + if (!drm_rect_clip_scaled(&cur_pipecfg->src_rect, &cur_pipecfg->dst_rect, &mixer_rect)) { > > > + memset(&pstate->pipe_cfg[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe_cfg)); > > > + memset(&pstate->pipe[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe)); > > > + continue; > > > } > > > > > > - *r_pipe_cfg = *pipe_cfg; > > > - pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1; > > > - pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1; > > > - r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2; > > > - r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; > > > - } else { > > > - memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg)); > > > - } > > > + cur_pipecfg->valid = true; > > > > ... and checks have been broken up to now. This isn't good. > > Will move this patch before the plane checking patch. You might need to move related chunks as well. > > > > > + cur_pipecfg->dst_rect.x1 -= mixer_rect.x1; > > > + cur_pipecfg->dst_rect.x2 -= mixer_rect.x1; > > > + > > > + DPU_DEBUG_PLANE(pdpu, "Got clip src:" DRM_RECT_FMT " dst: " DRM_RECT_FMT "\n", > > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), DRM_RECT_ARG(&cur_pipecfg->dst_rect)); > > > + > > > + /* Split super wide rect into 2 rect */ > > > + if ((drm_rect_width(&cur_pipecfg->src_rect) > max_linewidth) || > > > + _dpu_plane_calc_clk(mode, cur_pipecfg) > max_mdp_clk_rate) { > > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[cfg_idx + 1]; > > > + > > > + if (drm_rect_width(&cur_pipecfg->src_rect) > 2 * max_linewidth) { > > > + DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", > > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), max_linewidth); > > > + return -E2BIG; > > > + } > > > + > > > + memcpy(r_pipe_cfg, cur_pipecfg, sizeof(struct dpu_sw_pipe_cfg)); > > > + cur_pipecfg->src_rect.x2 = (cur_pipecfg->src_rect.x1 + cur_pipecfg->src_rect.x2) >> 1; > > > + cur_pipecfg->dst_rect.x2 = (cur_pipecfg->dst_rect.x1 + cur_pipecfg->dst_rect.x2) >> 1; > > > > pipe_cfg. If you need, rename the topmost var name. > > OK. pipe_cfg_cur ? screen_pipe_cfg > > > > > > + r_pipe_cfg->src_rect.x1 = cur_pipecfg->src_rect.x2; > > > + r_pipe_cfg->dst_rect.x1 = cur_pipecfg->dst_rect.x2; > > > + r_pipe_cfg->valid = true; > > > + DPU_DEBUG_PLANE(pdpu, "Split super wide plane into:" > > > + DRM_RECT_FMT " and " DRM_RECT_FMT "\n", > > > + DRM_RECT_ARG(&cur_pipecfg->src_rect), > > > + DRM_RECT_ARG(&r_pipe_cfg->src_rect)); > > > + } else { > > > + memset(&pstate->pipe_cfg[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe_cfg)); > > > + memset(&pstate->pipe[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe)); > > > > Please keep using r_pipe_cfg here. > > OK, will make r_pipe_cfg a variable in function scope, not bracket scope. > > > > > + } > > > > > > - drm_rect_rotate_inv(&pipe_cfg->src_rect, > > > - new_plane_state->fb->width, new_plane_state->fb->height, > > > - new_plane_state->rotation); > > > - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) > > > - drm_rect_rotate_inv(&r_pipe_cfg->src_rect, > > > + drm_rect_rotate_inv(&cur_pipecfg->src_rect, > > > new_plane_state->fb->width, new_plane_state->fb->height, > > > new_plane_state->rotation); > > > + } > > > > > > pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state); > > > > > > > > > -- > > > 2.34.1 > > > > > > > -- > > With best wishes > > Dmitry
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 66f745399a602..d2aca0a9493d5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1310,6 +1310,13 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en) return 0; } +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state) +{ + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); + + return cstate->num_mixers; +} + #ifdef CONFIG_DEBUG_FS static int _dpu_debugfs_status_show(struct seq_file *s, void *data) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 5260e2440f059..ee7cf71f89fc7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -304,4 +304,10 @@ static inline enum dpu_crtc_client_type dpu_crtc_get_client_type( void dpu_crtc_frame_event_cb(struct drm_crtc *crtc, u32 event); +/** + * dpu_crtc_get_lm_num - Get mixer number in this CRTC pipeline + * state: Pointer to drm crtc state object + */ +unsigned int dpu_crtc_get_lm_num(const struct drm_crtc_state *state); + #endif /* _DPU_CRTC_H_ */ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 898fc2937954e..480a1b46aba72 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -837,10 +837,12 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base); u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate; struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); - struct dpu_sw_pipe_cfg *pipe_cfg; - struct dpu_sw_pipe_cfg *r_pipe_cfg; + struct dpu_sw_pipe_cfg pipe_cfg; struct drm_rect fb_rect = { 0 }; + const struct drm_display_mode *mode = &crtc_state->adjusted_mode; uint32_t max_linewidth; + u32 lm_num; + int lmcfg_id, lmcfg_num; min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO); max_scale = MAX_DOWNSCALE_RATIO << 16; @@ -863,13 +865,10 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, return -EINVAL; } - /* move the assignment here, to ease handling to another pairs later */ - pipe_cfg = &pstate->pipe_cfg[0]; - r_pipe_cfg = &pstate->pipe_cfg[1]; - /* state->src is 16.16, src_rect is not */ - drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src); + lm_num = dpu_crtc_get_lm_num(crtc_state); - pipe_cfg->dst_rect = new_plane_state->dst; + /* state->src is 16.16, src_rect is not */ + drm_rect_fp_to_int(&pipe_cfg.src_rect, &new_plane_state->src); fb_rect.x2 = new_plane_state->fb->width; fb_rect.y2 = new_plane_state->fb->height; @@ -884,34 +883,78 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane, max_linewidth = pdpu->catalog->caps->max_linewidth; - drm_rect_rotate(&pipe_cfg->src_rect, + drm_rect_rotate(&pipe_cfg.src_rect, new_plane_state->fb->width, new_plane_state->fb->height, new_plane_state->rotation); - if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) || - _dpu_plane_calc_clk(&crtc_state->adjusted_mode, pipe_cfg) > max_mdp_clk_rate) { - if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) { - DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", - DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth); - return -E2BIG; + /* + * We have 1 mixer pair cfg for 1:1:1 and 2:2:1 topology, 2 mixer pair + * configs for left and right half screen in case of 4:4:2 topology. + * But we may have 2 rect to split plane with 1 config for 2:2:1. + * So need to handle super wide plane splitting, and plane on right half + * for quad-pipe case. Check dest rectangle left/right clipping + * first, then check super wide rectangle splitting in every half next. + */ + lmcfg_num = (lm_num + 1) / 2; + /* iterate mixer configs for this plane, to separate left/right with the id */ + for (lmcfg_id = 0; lmcfg_id < lmcfg_num; lmcfg_id++) { + struct drm_rect mixer_rect = {lmcfg_id * mode->hdisplay / lmcfg_num, 0, + (lmcfg_id + 1) * mode->hdisplay / lmcfg_num, mode->vdisplay}; + int cfg_idx = lmcfg_id * PIPES_PER_LM_PAIR; + struct dpu_sw_pipe_cfg *cur_pipecfg = &pstate->pipe_cfg[cfg_idx]; + + drm_rect_fp_to_int(&cur_pipecfg->src_rect, &new_plane_state->src); + cur_pipecfg->dst_rect = new_plane_state->dst; + + DPU_DEBUG_PLANE(pdpu, "checking src " DRM_RECT_FMT + " vs clip window " DRM_RECT_FMT "\n", + DRM_RECT_ARG(&cur_pipecfg->src_rect), + DRM_RECT_ARG(&mixer_rect)); + + /* If this plane does not fall into mixer rect, check next mixer rect */ + if (!drm_rect_clip_scaled(&cur_pipecfg->src_rect, &cur_pipecfg->dst_rect, &mixer_rect)) { + memset(&pstate->pipe_cfg[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe_cfg)); + memset(&pstate->pipe[cfg_idx], 0, 2 * sizeof(struct dpu_sw_pipe)); + continue; } - *r_pipe_cfg = *pipe_cfg; - pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1; - pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1; - r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2; - r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2; - } else { - memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg)); - } + cur_pipecfg->valid = true; + cur_pipecfg->dst_rect.x1 -= mixer_rect.x1; + cur_pipecfg->dst_rect.x2 -= mixer_rect.x1; + + DPU_DEBUG_PLANE(pdpu, "Got clip src:" DRM_RECT_FMT " dst: " DRM_RECT_FMT "\n", + DRM_RECT_ARG(&cur_pipecfg->src_rect), DRM_RECT_ARG(&cur_pipecfg->dst_rect)); + + /* Split super wide rect into 2 rect */ + if ((drm_rect_width(&cur_pipecfg->src_rect) > max_linewidth) || + _dpu_plane_calc_clk(mode, cur_pipecfg) > max_mdp_clk_rate) { + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[cfg_idx + 1]; + + if (drm_rect_width(&cur_pipecfg->src_rect) > 2 * max_linewidth) { + DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n", + DRM_RECT_ARG(&cur_pipecfg->src_rect), max_linewidth); + return -E2BIG; + } + + memcpy(r_pipe_cfg, cur_pipecfg, sizeof(struct dpu_sw_pipe_cfg)); + cur_pipecfg->src_rect.x2 = (cur_pipecfg->src_rect.x1 + cur_pipecfg->src_rect.x2) >> 1; + cur_pipecfg->dst_rect.x2 = (cur_pipecfg->dst_rect.x1 + cur_pipecfg->dst_rect.x2) >> 1; + r_pipe_cfg->src_rect.x1 = cur_pipecfg->src_rect.x2; + r_pipe_cfg->dst_rect.x1 = cur_pipecfg->dst_rect.x2; + r_pipe_cfg->valid = true; + DPU_DEBUG_PLANE(pdpu, "Split super wide plane into:" + DRM_RECT_FMT " and " DRM_RECT_FMT "\n", + DRM_RECT_ARG(&cur_pipecfg->src_rect), + DRM_RECT_ARG(&r_pipe_cfg->src_rect)); + } else { + memset(&pstate->pipe_cfg[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe_cfg)); + memset(&pstate->pipe[cfg_idx + 1], 0, sizeof(struct dpu_sw_pipe)); + } - drm_rect_rotate_inv(&pipe_cfg->src_rect, - new_plane_state->fb->width, new_plane_state->fb->height, - new_plane_state->rotation); - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) - drm_rect_rotate_inv(&r_pipe_cfg->src_rect, + drm_rect_rotate_inv(&cur_pipecfg->src_rect, new_plane_state->fb->width, new_plane_state->fb->height, new_plane_state->rotation); + } pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state);
Clip plane into pipes per left and right half screen ROI if topology is quad pipe. Then split the clipped rectangle by half if the rectangle width still exceeds width limit. Signed-off-by: Jun Nie <jun.nie@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 7 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 6 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 99 ++++++++++++++++++++++--------- 3 files changed, 84 insertions(+), 28 deletions(-)