[PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Thu Feb 9 11:46:32 UTC 2023
On 07/02/2023 02:22, Abhinav Kumar wrote:
>
>
> On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
>> Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the
>
> sspp-dependent?
>
>> separate function dpu_plane_sspp_update_pipe(). This is one of
>> preparational steps to add r_pipe support.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>
> Just a couple of minor comments below but otherwise this split up lgtm
>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 ++++++++++++----------
>> 1 file changed, 63 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index 0986e740b978..f94e132733f3 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct
>> drm_plane *plane,
>> * _dpu_plane_set_ot_limit - set OT limit for the given plane
>> * @plane: Pointer to drm plane
>> * @pipe: Pointer to software pipe
>> - * @crtc: Pointer to drm crtc
>> * @pipe_cfg: Pointer to pipe configuration
>> + * @frame_rate: CRTC's frame rate
>
> Can you please check the spacing here. There seems to be an extra tab
> before the CRTC's frame rate
I checked, the ident is correct here. It uses tabs, maybe that confuses
your mailer.
>
>> */
>> static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>> struct dpu_sw_pipe *pipe,
>> - struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
>> + struct dpu_hw_sspp_cfg *pipe_cfg,
>> + int frame_rate)
>> {
>> struct dpu_plane *pdpu = to_dpu_plane(plane);
>> struct dpu_vbif_set_ot_params ot_params;
>> @@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct
>> drm_plane *plane,
>> ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
>> ot_params.height = drm_rect_height(&pipe_cfg->src_rect);
>> ot_params.is_wfd = !pdpu->is_rt_pipe;
>> - ot_params.frame_rate = drm_mode_vrefresh(&crtc->mode);
>> + ot_params.frame_rate = frame_rate;
>> ot_params.vbif_idx = VBIF_RT;
>> ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>> ot_params.rd = true;
>> @@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct
>> drm_plane *plane,
>> dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
>> }
>> -static void _dpu_plane_set_scanout(struct drm_plane *plane,
>> - struct dpu_plane_state *pstate,
>> - struct drm_framebuffer *fb)
>> -{
>> - struct dpu_plane *pdpu = to_dpu_plane(plane);
>> - 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;
>> - int ret;
>> -
>> - ret = dpu_format_populate_layout(aspace, fb, &layout);
>> - if (ret)
>> - DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
>> - else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
>> - trace_dpu_plane_set_scanout(&pstate->pipe,
>> - &layout);
>> - pstate->pipe.sspp->ops.setup_sourceaddress(&pstate->pipe,
>> &layout);
>> - }
>> -}
>> -
>> static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
>> uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
>> struct dpu_hw_scaler3_cfg *scale_cfg,
>> @@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane
>> *plane, bool error)
>> pdpu->is_error = error;
>> }
>> -static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>> +static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
>> + struct dpu_sw_pipe *pipe,
>> + struct dpu_hw_sspp_cfg *pipe_cfg,
>
> You can call this parameter sspp_cfg instead of pipe_cfg?
>
>> + const struct dpu_format *fmt,
>> + int frame_rate,
>> + struct dpu_hw_fmt_layout *layout)
>> {
>> uint32_t src_flags;
>> struct dpu_plane *pdpu = to_dpu_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 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_hw_sspp_cfg *pipe_cfg = &pstate->pipe_cfg;
>> - _dpu_plane_set_scanout(plane, pstate, fb);
>> -
>> - pstate->pending = true;
>> -
>> - is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>> - pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
>> - pdpu->is_rt_pipe = is_rt_pipe;
>> + if (layout && pipe->sspp->ops.setup_sourceaddress) {
>> + trace_dpu_plane_set_scanout(pipe, layout);
>> + pipe->sspp->ops.setup_sourceaddress(pipe, layout);
>> + }
>> _dpu_plane_set_qos_ctrl(plane, pipe, false,
>> DPU_PLANE_QOS_PANIC_CTRL);
>> - DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
>> DRM_RECT_FMT
>> - ", %4.4s ubwc %d\n", fb->base.id,
>> DRM_RECT_FP_ARG(&state->src),
>> - crtc->base.id, DRM_RECT_ARG(&state->dst),
>> - (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt));
>> -
>> /* override for color fill */
>> if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
>> /* skip remaining processing on color fill */
>> @@ -1183,22 +1154,64 @@ static void
>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>> }
>> }
>> - _dpu_plane_set_qos_lut(plane, pipe, fmt, &pstate->pipe_cfg);
>> + _dpu_plane_set_qos_lut(plane, pipe, fmt, pipe_cfg);
>> _dpu_plane_set_danger_lut(plane, pipe, fmt);
>> if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>> _dpu_plane_set_qos_ctrl(plane, pipe, true,
>> DPU_PLANE_QOS_PANIC_CTRL);
>> - _dpu_plane_set_ot_limit(plane, pipe, crtc, &pstate->pipe_cfg);
>> + _dpu_plane_set_ot_limit(plane, pipe, pipe_cfg, frame_rate);
>> }
>> - if (pstate->needs_qos_remap) {
>> - pstate->needs_qos_remap = false;
>> + if (pstate->needs_qos_remap)
>> _dpu_plane_set_qos_remap(plane, pipe);
>> - }
>> +}
>> +
>> +static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>> +{
>> + struct dpu_plane *pdpu = to_dpu_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 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_hw_sspp_cfg *pipe_cfg = &pstate->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;
>> + bool layout_valid = false;
>> + int ret;
>> +
>> + ret = dpu_format_populate_layout(aspace, fb, &layout);
>> + if (ret)
>> + DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
>> + else
>> + layout_valid = true;
>> +
>> + pstate->pending = true;
>> +
>> + is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>> + pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
>> + pdpu->is_rt_pipe = is_rt_pipe;
>> +
>> + DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
>> DRM_RECT_FMT
>> + ", %4.4s ubwc %d\n", fb->base.id,
>> DRM_RECT_FP_ARG(&state->src),
>> + crtc->base.id, DRM_RECT_ARG(&state->dst),
>> + (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt));
>> +
>> + dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
>> + drm_mode_vrefresh(&crtc->mode),
>> + layout_valid ? &layout: NULL);
>> +
>> + if (pstate->needs_qos_remap)
>> + pstate->needs_qos_remap = false;
>> - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt,
>> &crtc->mode, &pstate->pipe_cfg);
>> + pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt,
>> &crtc->mode, pipe_cfg);
>> - pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode,
>> &pstate->pipe_cfg);
>> + pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
>> }
>> static void _dpu_plane_atomic_disable(struct drm_plane *plane)
--
With best wishes
Dmitry
More information about the dri-devel
mailing list