[PATCH v7 27/32] drm/msm/dpu: add support for wide planes
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Thu Mar 30 03:30:48 UTC 2023
On 30/03/2023 05:53, Abhinav Kumar wrote:
>
>
> On 3/16/2023 9:16 AM, Dmitry Baryshkov wrote:
>> It is possible to use multirect feature and split source to use the SSPP
>> to output two consecutive rectangles. This commit brings in this
>> capability to support wider screen resolutions.
>>
>> Tested-by: Abhinav Kumar <quic_abhinavk at quicinc.com> # sc7280
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 19 ++-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 136 +++++++++++++++++++---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 +
>> 3 files changed, 142 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index 217a8112f1a2..90b406e409d3 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -481,6 +481,15 @@ static void _dpu_crtc_blend_setup_mixer(struct
>> drm_crtc *crtc,
>> format, fb ? fb->modifier : 0,
>> &pstate->pipe, 0, stage_cfg);
>> + if (pstate->r_pipe.sspp) {
>> + set_bit(pstate->r_pipe.sspp->idx, fetch_active);
>> + _dpu_crtc_blend_setup_pipe(crtc, plane,
>> + mixer, cstate->num_mixers,
>> + pstate->stage,
>> + format, fb ? fb->modifier : 0,
>> + &pstate->r_pipe, 1, stage_cfg);
>> + }
>> +
>> /* blend config update */
>> for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
>> _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format);
>> @@ -1341,10 +1350,16 @@ static int _dpu_debugfs_status_show(struct
>> seq_file *s, void *data)
>> seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
>> state->crtc_x, state->crtc_y, state->crtc_w,
>> state->crtc_h);
>> - seq_printf(s, "\tsspp:%s\n",
>> + seq_printf(s, "\tsspp[0]:%s\n",
>> pstate->pipe.sspp->cap->name);
>> - seq_printf(s, "\tmultirect: mode: %d index: %d\n",
>> + seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
>> pstate->pipe.multirect_mode, pstate->pipe.multirect_index);
>> + if (pstate->r_pipe.sspp) {
>> + seq_printf(s, "\tsspp[1]:%s\n",
>> + pstate->r_pipe.sspp->cap->name);
>> + seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
>> + pstate->r_pipe.multirect_mode,
>> pstate->r_pipe.multirect_index);
>> + }
>> seq_puts(s, "\n");
>> }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index f52120b05b6e..73db15d76059 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -701,6 +701,10 @@ static void _dpu_plane_color_fill(struct
>> dpu_plane *pdpu,
>> /* update sspp */
>> _dpu_plane_color_fill_pipe(pstate, &pstate->pipe,
>> &pstate->pipe_cfg.dst_rect,
>> fill_color, fmt);
>> +
>> + if (pstate->r_pipe.sspp)
>> + _dpu_plane_color_fill_pipe(pstate, &pstate->r_pipe,
>> &pstate->r_pipe_cfg.dst_rect,
>> + fill_color, fmt);
>> }
>> int dpu_plane_validate_multirect_v2(struct
>> dpu_multirect_plane_states *plane)
>> @@ -959,9 +963,12 @@ static int dpu_plane_atomic_check(struct
>> drm_plane *plane,
>> int ret = 0, min_scale;
>> struct dpu_plane *pdpu = to_dpu_plane(plane);
>> struct dpu_plane_state *pstate =
>> to_dpu_plane_state(new_plane_state);
>> + struct dpu_sw_pipe *pipe = &pstate->pipe;
>> + struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>> const struct drm_crtc_state *crtc_state = NULL;
>> const struct dpu_format *fmt;
>> struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>> + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>> struct drm_rect fb_rect = { 0 };
>> uint32_t max_linewidth;
>> unsigned int rotation;
>> @@ -985,8 +992,11 @@ static int dpu_plane_atomic_check(struct
>> drm_plane *plane,
>> if (!new_plane_state->visible)
>> return 0;
>> - pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>> - pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> + pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>> + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>> + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> + r_pipe->sspp = NULL;
>> pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
>> if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
>> @@ -1016,21 +1026,67 @@ static int dpu_plane_atomic_check(struct
>> drm_plane *plane,
>> return -E2BIG;
>> }
>> + fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
>> +
>> max_linewidth = pdpu->catalog->caps->max_linewidth;
>> - /* check decimated source width */
>> if (drm_rect_width(&pipe_cfg->src_rect) > 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;
>> - }
>> + /*
>> + * In parallel multirect case only the half of the usual width
>> + * is supported for tiled formats. If we are here, we know that
>> + * full width is more than max_linewidth, thus each rect is
>> + * wider than allowed.
>> + */
>> + if (DPU_FORMAT_IS_UBWC(fmt)) {
>> + DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
>> line:%u, tiled format\n",
>> + DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>> + return -E2BIG;
>> + }
>
> I am seeing a strange issue due to this check. Looks like Chrome
> compositor tries to do the first frame with UBWC. When I bootup with my
> DP monitor connected, it tries to use 2560x1440 with UBWC enabled but
> then fails and then uses linear format and works.
>
> I confirmed by commenting out this check that its always UBWC.
>
> I feel this is a compositor issue. If someone can confirm this that
> would be great because there is nothing wrong with this check.
Yes, I think it is a compositor issue (or misfeature).
>
> I only want to report this because when this series is cherry-picked and
> someone tries to bootup with DP connected and it if it does hit this
> check based on which monitor they try, UBWC no longer works.
>
> If we can move on and leave that out as compositor issue, I will add my
> R-b once the other comment is responded to.
>
> Rest of the testing looks fine.
>
>> - fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
>> + 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;
>> + }
>> - ret = dpu_plane_atomic_check_pipe(pdpu, &pstate->pipe, pipe_cfg,
>> fmt);
>> + if (drm_rect_width(&pipe_cfg->src_rect) !=
>> drm_rect_width(&pipe_cfg->dst_rect) ||
>> + drm_rect_height(&pipe_cfg->src_rect) !=
>> drm_rect_height(&pipe_cfg->dst_rect) ||
>> + (!test_bit(DPU_SSPP_SMART_DMA_V1,
>> &pipe->sspp->cap->features) &&
>> + !test_bit(DPU_SSPP_SMART_DMA_V2,
>> &pipe->sspp->cap->features)) ||
>> + DPU_FORMAT_IS_YUV(fmt)) {
>> + DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
>> line:%u, can't use split source\n",
>> + DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
>> + return -E2BIG;
>> + }
>> +
>> + /*
>> + * Use multirect for wide plane. We do not support dynamic
>> + * assignment of SSPPs, so we know the configuration.
>> + */
>> + pipe->multirect_index = DPU_SSPP_RECT_0;
>> + pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>> +
>> + r_pipe->sspp = pipe->sspp;
>> + r_pipe->multirect_index = DPU_SSPP_RECT_1;
>> + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>> +
>> + *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;
>> + }
>> +
>> + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
>> if (ret)
>> return ret;
>> + if (r_pipe->sspp) {
>> + ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg,
>> fmt);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
>> if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
>> @@ -1097,8 +1153,10 @@ void dpu_plane_flush(struct drm_plane *plane)
>> else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
>> /* force 100% alpha */
>> _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
>> - else
>> + else {
>> dpu_plane_flush_csc(pdpu, &pstate->pipe);
>> + dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
>> + }
>> /* flag h/w flush complete */
>> if (plane->state)
>> @@ -1210,13 +1268,14 @@ static void
>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>> struct drm_plane_state *state = plane->state;
>> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>> struct dpu_sw_pipe *pipe = &pstate->pipe;
>> + struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>> struct drm_crtc *crtc = state->crtc;
>> struct drm_framebuffer *fb = state->fb;
>> bool is_rt_pipe;
>> const struct dpu_format *fmt =
>> to_dpu_format(msm_framebuffer_format(fb));
>> struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>> -
>> + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>> struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>> struct msm_gem_address_space *aspace = kms->base.aspace;
>> struct dpu_hw_fmt_layout layout;
>> @@ -1244,6 +1303,12 @@ static void dpu_plane_sspp_atomic_update(struct
>> drm_plane *plane)
>> drm_mode_vrefresh(&crtc->mode),
>> layout_valid ? &layout : NULL);
>> + if (r_pipe->sspp) {
>> + dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
>> + drm_mode_vrefresh(&crtc->mode),
>> + layout_valid ? &layout : NULL);
>> + }
>> +
>> if (pstate->needs_qos_remap)
>> pstate->needs_qos_remap = false;
>> @@ -1251,16 +1316,31 @@ static void
>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>> &crtc->mode, pipe_cfg);
>> pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
>> +
>> + if (r_pipe->sspp) {
>> + pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog,
>> fmt, &crtc->mode, r_pipe_cfg);
>> +
>> + pstate->plane_clk = max(pstate->plane_clk,
>> _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg));
>> + }
>> }
>> static void _dpu_plane_atomic_disable(struct drm_plane *plane)
>> {
>> struct drm_plane_state *state = plane->state;
>> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>> + struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>> trace_dpu_plane_disable(DRMID(plane), false,
>> pstate->pipe.multirect_mode);
>> + if (r_pipe->sspp) {
>> + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>> + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +
>> + if (r_pipe->sspp->ops.setup_multirect)
>> + r_pipe->sspp->ops.setup_multirect(r_pipe);
>> + }
>
> Dont we need to do this even for pstate->pipe?
No. pstate->pipe will be set up when we call
dpu_plane_sspp_atomic_update() next time. It doesn't seem to matter if
the disabled SSPP is setup for REC_SOLO or REC_0. We have to ensure that
REC_1 is disabled so that it doesn't conflict with REC_SOLO setup.
>> +
>> pstate->pending = true;
>> }
>> @@ -1293,6 +1373,9 @@ static void dpu_plane_destroy(struct drm_plane
>> *plane)
>> pstate = to_dpu_plane_state(plane->state);
>> _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, false,
>> DPU_PLANE_QOS_PANIC_CTRL);
>> + if (pstate->r_pipe.sspp)
>> + _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, false,
>> DPU_PLANE_QOS_PANIC_CTRL);
>> +
>> mutex_destroy(&pdpu->lock);
>> /* this will destroy the states as well */
>> @@ -1373,12 +1456,29 @@ static void
>> dpu_plane_atomic_print_state(struct drm_printer *p,
>> const struct drm_plane_state *state)
>> {
>> const struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>> + const struct dpu_sw_pipe *pipe = &pstate->pipe;
>> + const struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
>> + const struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>> + const struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
>> drm_printf(p, "\tstage=%d\n", pstate->stage);
>> - drm_printf(p, "\tsspp=%s\n", pstate->pipe.sspp->cap->name);
>> - drm_printf(p, "\tmultirect_mode=%s\n",
>> dpu_get_multirect_mode(pstate->pipe.multirect_mode));
>> - drm_printf(p, "\tmultirect_index=%s\n",
>> - dpu_get_multirect_index(pstate->pipe.multirect_index));
>> +
>> + drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
>> + drm_printf(p, "\tmultirect_mode[0]=%s\n",
>> dpu_get_multirect_mode(pipe->multirect_mode));
>> + drm_printf(p, "\tmultirect_index[0]=%s\n",
>> + dpu_get_multirect_index(pipe->multirect_index));
>> + drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n",
>> DRM_RECT_ARG(&pipe_cfg->src_rect));
>> + drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n",
>> DRM_RECT_ARG(&pipe_cfg->dst_rect));
>> +
>> + if (r_pipe->sspp) {
>> + drm_printf(p, "\tsspp[1]=%s\n", r_pipe->sspp->cap->name);
>> + drm_printf(p, "\tmultirect_mode[1]=%s\n",
>> + dpu_get_multirect_mode(r_pipe->multirect_mode));
>> + drm_printf(p, "\tmultirect_index[1]=%s\n",
>> + dpu_get_multirect_index(r_pipe->multirect_index));
>> + drm_printf(p, "\tsrc[1]=" DRM_RECT_FMT "\n",
>> DRM_RECT_ARG(&r_pipe_cfg->src_rect));
>> + drm_printf(p, "\tdst[1]=" DRM_RECT_FMT "\n",
>> DRM_RECT_ARG(&r_pipe_cfg->dst_rect));
>> + }
>> }
>> static void dpu_plane_reset(struct drm_plane *plane)
>> @@ -1412,6 +1512,10 @@ static void dpu_plane_reset(struct drm_plane
>> *plane)
>> * This is the place where the state is allocated, so fill it
>> fully.
>> */
>> pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
>> + pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>> + pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>> +
>> + pstate->r_pipe.sspp = NULL;
>> __drm_atomic_helper_plane_reset(plane, &pstate->base);
>> }
>> @@ -1428,6 +1532,8 @@ void dpu_plane_danger_signal_ctrl(struct
>> drm_plane *plane, bool enable)
>> pm_runtime_get_sync(&dpu_kms->pdev->dev);
>> _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable,
>> DPU_PLANE_QOS_PANIC_CTRL);
>> + if (pstate->r_pipe.sspp)
>> + _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable,
>> DPU_PLANE_QOS_PANIC_CTRL);
>> pm_runtime_put_sync(&dpu_kms->pdev->dev);
>> }
>> #endif
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> index 0ca9002015ff..7490ffd94d03 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> @@ -19,7 +19,9 @@
>> * @base: base drm plane state object
>> * @aspace: pointer to address space for input/output buffers
>> * @pipe: software pipe description
>> + * @r_pipe: software pipe description of the second pipe
>> * @pipe_cfg: software pipe configuration
>> + * @r_pipe_cfg: software pipe configuration for the second pipe
>> * @stage: assigned by crtc blender
>> * @needs_qos_remap: qos remap settings need to be updated
>> * @multirect_index: index of the rectangle of SSPP
>> @@ -34,7 +36,9 @@ struct dpu_plane_state {
>> struct drm_plane_state base;
>> struct msm_gem_address_space *aspace;
>> struct dpu_sw_pipe pipe;
>> + struct dpu_sw_pipe r_pipe;
>> struct dpu_sw_pipe_cfg pipe_cfg;
>> + struct dpu_sw_pipe_cfg r_pipe_cfg;
>> enum dpu_stage stage;
>> bool needs_qos_remap;
>> bool pending;
--
With best wishes
Dmitry
More information about the dri-devel
mailing list