[PATCH v7 27/32] drm/msm/dpu: add support for wide planes
Abhinav Kumar
quic_abhinavk at quicinc.com
Thu Mar 30 04:51:15 UTC 2023
On 3/29/2023 8:30 PM, Dmitry Baryshkov wrote:
> 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).
>
Ack, I am satisfied with below response as well,
Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>>
>> 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;
>
More information about the dri-devel
mailing list