| Message ID | 20241009-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-21-v2-5-76d4f5d413bf@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/dpu: Support quad pipe with dual-DSI | expand |
On Wed, Oct 09, 2024 at 04:50:18PM GMT, Jun Nie wrote: > Store pipes in array with removing dedicated r_pipe. There are > 2 pipes in a drm plane at most currently. While 4 pipes are > needed for new usage case. This change generalize the handling > to pipe pair and ease handling to another pipe pair later. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 34 +++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 167 ++++++++++++++++-------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +-- > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- > 4 files changed, 111 insertions(+), 112 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 3e0e6e9757da5..9656b1df0f122 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -411,7 +411,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc, > > trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane), > state, to_dpu_plane_state(state), stage_idx, > - format->pixel_format, > + format->pixel_format, pipe, > modifier); Doesn't seem to be related. > > DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx %d\n", > @@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > const struct msm_format *format; > struct dpu_hw_ctl *ctl = mixer->lm_ctl; > > - uint32_t lm_idx; > + uint32_t lm_idx, i; > bool bg_alpha_enable = false; > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > @@ -463,20 +463,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) > bg_alpha_enable = true; > > - set_bit(pstate->pipe.sspp->idx, fetch_active); > - _dpu_crtc_blend_setup_pipe(crtc, plane, > - mixer, cstate->num_mixers, > - pstate->stage, > - format, fb ? fb->modifier : 0, > - &pstate->pipe, 0, stage_cfg); > - > - if (pstate->r_pipe.sspp) { > - set_bit(pstate->r_pipe.sspp->idx, fetch_active); > + for (i = 0; i < PIPES_PER_STAGE; i++) { > + if (!pstate->pipe[i].sspp) > + continue; > + set_bit(pstate->pipe[i].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); > + &pstate->pipe[i], i, stage_cfg); > } > > /* blend config update */ > @@ -1387,15 +1382,12 @@ 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[0]:%s\n", > - pstate->pipe.sspp->cap->name); > - 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); > + if (pstate->pipe[i].sspp) { > + seq_printf(s, "\tsspp[%d]:%s\n", > + i, pstate->pipe[i].sspp->cap->name); > + seq_printf(s, "\tmultirect[%d]: mode: %d index: %d\n", > + i, pstate->pipe[i].multirect_mode, > + pstate->pipe[i].multirect_index); > } I don't expect that this will work. > > 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 4df7cfed4d230..e7006fb8c7734 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -429,7 +429,7 @@ static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw, > uint32_t chroma_subsmpl_h, uint32_t chroma_subsmpl_v, > unsigned int rotation) > { > - uint32_t i; > + int i; Why? > bool inline_rotation = rotation & DRM_MODE_ROTATE_90; > > /* > @@ -619,6 +619,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > struct msm_drm_private *priv = plane->dev->dev_private; > struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); > u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24); > + int i; > > DPU_DEBUG_PLANE(pdpu, "\n"); > > @@ -632,12 +633,12 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > return; > > /* 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); > + for (i = 0; i < PIPES_PER_STAGE; i++) { > + if (pstate->pipe[i].sspp) > + _dpu_plane_color_fill_pipe(pstate, &pstate->pipe[i], > + &pstate->pipe_cfg[i].dst_rect, > + fill_color, fmt); > + } > } > > static int dpu_plane_prepare_fb(struct drm_plane *plane, > @@ -808,8 +809,8 @@ 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 = &pstate->pipe_cfg; > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > + struct dpu_sw_pipe_cfg *pipe_cfg; > + struct dpu_sw_pipe_cfg *r_pipe_cfg; > struct drm_rect fb_rect = { 0 }; > uint32_t max_linewidth; > > @@ -834,6 +835,9 @@ 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); > > @@ -916,11 +920,11 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane, > drm_atomic_get_new_plane_state(state, plane); > 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 msm_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 dpu_sw_pipe *pipe = &pstate->pipe[0]; > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > uint32_t supported_rotations; > const struct dpu_sspp_cfg *pipe_hw_caps; > const struct dpu_sspp_sub_blks *sblk; > @@ -975,10 +979,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > struct dpu_plane *pdpu = to_dpu_plane(plane); > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > - struct dpu_sw_pipe *pipe = &pstate->pipe; > - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe; > - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > + struct dpu_sw_pipe *pipe = &pstate->pipe[0]; > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > const struct drm_crtc_state *crtc_state = NULL; > > if (new_plane_state->crtc) > @@ -1056,7 +1060,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > drm_atomic_get_old_plane_state(state, plane); > struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state); > struct drm_crtc_state *crtc_state; > - int ret; > + int ret, i; > > if (plane_state->crtc) > crtc_state = drm_atomic_get_new_crtc_state(state, > @@ -1071,8 +1075,8 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > * resources are freed by dpu_crtc_assign_plane_resources(), > * but clean them here. > */ > - pstate->pipe.sspp = NULL; > - pstate->r_pipe.sspp = NULL; > + for (i = 0; i < PIPES_PER_STAGE; i++) > + pstate->pipe[i].sspp = NULL; > > return 0; > } > @@ -1110,19 +1114,22 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, > struct dpu_sw_pipe_cfg *r_pipe_cfg; > const struct msm_format *fmt; > uint32_t max_linewidth; > + int i; > > if (plane_state->crtc) > crtc_state = drm_atomic_get_new_crtc_state(state, > plane_state->crtc); > > pstate = to_dpu_plane_state(plane_state); > - pipe = &pstate->pipe; > - r_pipe = &pstate->r_pipe; > - pipe_cfg = &pstate->pipe_cfg; > - r_pipe_cfg = &pstate->r_pipe_cfg; > > - pipe->sspp = NULL; > - r_pipe->sspp = NULL; > + /* loop below code for another pair later */ ?? > + pipe = &pstate->pipe[0]; > + r_pipe = &pstate->pipe[1]; > + pipe_cfg = &pstate->pipe_cfg[0]; > + r_pipe_cfg = &pstate->pipe_cfg[1]; > + > + for (i = 0; i < PIPES_PER_STAGE; i++) > + pstate->pipe[i].sspp = NULL; > > if (!plane_state->fb) > return -EINVAL; > @@ -1228,6 +1235,7 @@ void dpu_plane_flush(struct drm_plane *plane) > { > struct dpu_plane *pdpu; > struct dpu_plane_state *pstate; > + int i; > > if (!plane || !plane->state) { > DPU_ERROR("invalid plane\n"); > @@ -1248,8 +1256,8 @@ void dpu_plane_flush(struct drm_plane *plane) > /* force 100% alpha */ > _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); > else { > - dpu_plane_flush_csc(pdpu, &pstate->pipe); > - dpu_plane_flush_csc(pdpu, &pstate->r_pipe); > + for (i = 0; i < PIPES_PER_STAGE; i++) > + dpu_plane_flush_csc(pdpu, &pstate->pipe[i]); > } > > /* flag h/w flush complete */ > @@ -1349,20 +1357,16 @@ 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 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 msm_format *fmt = > 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; > bool layout_valid = false; > - int ret; > + int ret, i; > > ret = dpu_format_populate_layout(aspace, fb, &layout); > if (ret) > @@ -1381,12 +1385,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > crtc->base.id, DRM_RECT_ARG(&state->dst), > &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt)); > > - dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt, > - 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, > + /* move the assignment here, to ease handling to another pairs later */ > + for (i = 0; i < PIPES_PER_STAGE; i++) { > + if (!pstate->pipe[i].sspp) > + continue; > + dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i], > + &pstate->pipe_cfg[i], fmt, > drm_mode_vrefresh(&crtc->mode), > layout_valid ? &layout : NULL); > } > @@ -1394,15 +1398,17 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > if (pstate->needs_qos_remap) > pstate->needs_qos_remap = false; > > - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, > - &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_fetch_bw = 0; > + pstate->plane_clk = 0; > + for (i = 0; i < PIPES_PER_STAGE; i++) { > + if (!pstate->pipe[i].sspp) > + continue; > + pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, > + &crtc->mode, &pstate->pipe_cfg[i]); Wrong indentation > > - pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg)); > + pstate->plane_clk = max(pstate->plane_clk, > + _dpu_plane_calc_clk(&crtc->mode, > + &pstate->pipe_cfg[i])); > } > } > > @@ -1410,17 +1416,24 @@ 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; > + struct dpu_sw_pipe *pipe; > + int i; > + > + for (i = 0; i < PIPES_PER_STAGE; i += 1) { > + pipe = &pstate->pipe[i]; > + if (!pipe->sspp) > + continue; > > - trace_dpu_plane_disable(DRMID(plane), false, > - pstate->pipe.multirect_mode); > + trace_dpu_plane_disable(DRMID(plane), false, > + pstate->pipe[i].multirect_mode); > > - if (r_pipe->sspp) { > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) { if (i > 1) > + pipe->multirect_index = DPU_SSPP_RECT_SOLO; > + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > - if (r_pipe->sspp->ops.setup_multirect) > - r_pipe->sspp->ops.setup_multirect(r_pipe); > + if (pipe->sspp->ops.setup_multirect) > + pipe->sspp->ops.setup_multirect(pipe); > + } > } > > pstate->pending = true; > @@ -1515,30 +1528,26 @@ 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; > + const struct dpu_sw_pipe *pipe; > + const struct dpu_sw_pipe_cfg *pipe_cfg; > + int i; > > drm_printf(p, "\tstage=%d\n", pstate->stage); > > - if (pipe->sspp) { > - 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", > + for (i = 0; i < PIPES_PER_STAGE; i++) { > + pipe = &pstate->pipe[i]; > + if (!pipe->sspp) > + continue; > + pipe_cfg = &pstate->pipe_cfg[i]; > + drm_printf(p, "\tsspp[%d]=%s\n", i, pipe->sspp->cap->name); > + drm_printf(p, "\tmultirect_mode[%d]=%s\n", i, > + dpu_get_multirect_mode(pipe->multirect_mode)); > + drm_printf(p, "\tmultirect_index[%d]=%s\n", i, > 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)); > + drm_printf(p, "\tsrc[%d]=" DRM_RECT_FMT "\n", i, > + DRM_RECT_ARG(&pipe_cfg->src_rect)); > + drm_printf(p, "\tdst[%d]=" DRM_RECT_FMT "\n", i, > + DRM_RECT_ARG(&pipe_cfg->dst_rect)); > } > } > > @@ -1576,14 +1585,16 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) > struct dpu_plane *pdpu = to_dpu_plane(plane); > struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > + int i; > > if (!pdpu->is_rt_pipe) > return; > > pm_runtime_get_sync(&dpu_kms->pdev->dev); > - _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable); > - if (pstate->r_pipe.sspp) > - _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable); > + for (i = 0; i < PIPES_PER_STAGE; i++) { > + if (pstate->pipe[i].sspp) > + _dpu_plane_set_qos_ctrl(plane, &pstate->pipe[i], enable); > + } > 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 e225d5baceb09..39945e0d1b851 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h > @@ -18,10 +18,8 @@ > * struct dpu_plane_state: Define dpu extension of drm plane state object > * @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 > + * @pipe: software pipe description array > + * @pipe_cfg: software pipe configuration array > * @stage: assigned by crtc blender > * @needs_qos_remap: qos remap settings need to be updated > * @multirect_index: index of the rectangle of SSPP > @@ -34,10 +32,8 @@ > 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; > + struct dpu_sw_pipe pipe[PIPES_PER_STAGE]; > + struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE]; > enum dpu_stage stage; > bool needs_qos_remap; > bool pending; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > index 5307cbc2007c5..cb24ad2a6d8d3 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > @@ -651,9 +651,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, > TP_PROTO(uint32_t crtc_id, uint32_t plane_id, > struct drm_plane_state *state, struct dpu_plane_state *pstate, > uint32_t stage_idx, uint32_t pixel_format, > - uint64_t modifier), > + struct dpu_sw_pipe *pipe, uint64_t modifier), > TP_ARGS(crtc_id, plane_id, state, pstate, stage_idx, > - pixel_format, modifier), > + pixel_format, pipe, modifier), > TP_STRUCT__entry( > __field( uint32_t, crtc_id ) > __field( uint32_t, plane_id ) > @@ -676,9 +676,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, > __entry->dst_rect = drm_plane_state_dest(state); > __entry->stage_idx = stage_idx; > __entry->stage = pstate->stage; > - __entry->sspp = pstate->pipe.sspp->idx; > - __entry->multirect_idx = pstate->pipe.multirect_index; > - __entry->multirect_mode = pstate->pipe.multirect_mode; > + __entry->sspp = pipe->sspp->idx; > + __entry->multirect_idx = pipe->multirect_index; > + __entry->multirect_mode = pipe->multirect_mode; > __entry->pixel_format = pixel_format; > __entry->modifier = modifier; > ), > > -- > 2.34.1 >
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:08写道： > > On Wed, Oct 09, 2024 at 04:50:18PM GMT, Jun Nie wrote: > > Store pipes in array with removing dedicated r_pipe. There are > > 2 pipes in a drm plane at most currently. While 4 pipes are > > needed for new usage case. This change generalize the handling > > to pipe pair and ease handling to another pipe pair later. > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 34 +++--- > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 167 ++++++++++++++++-------------- > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +-- > > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- > > 4 files changed, 111 insertions(+), 112 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > index 3e0e6e9757da5..9656b1df0f122 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > @@ -411,7 +411,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc, > > > > trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane), > > state, to_dpu_plane_state(state), stage_idx, > > - format->pixel_format, > > + format->pixel_format, pipe, > > modifier); > > Doesn't seem to be related. There is building error when converting r_pipe into pipe array. So I need to change trace code accordingly in the same patch. Otherwise, the error happens just after this patch is applied. > > > > > DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx %d\n", > > @@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > > const struct msm_format *format; > > struct dpu_hw_ctl *ctl = mixer->lm_ctl; > > > > - uint32_t lm_idx; > > + uint32_t lm_idx, i; > > bool bg_alpha_enable = false; > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > @@ -463,20 +463,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > > if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) > > bg_alpha_enable = true; > > > > - set_bit(pstate->pipe.sspp->idx, fetch_active); > > - _dpu_crtc_blend_setup_pipe(crtc, plane, > > - mixer, cstate->num_mixers, > > - pstate->stage, > > - format, fb ? fb->modifier : 0, > > - &pstate->pipe, 0, stage_cfg); > > - > > - if (pstate->r_pipe.sspp) { > > - set_bit(pstate->r_pipe.sspp->idx, fetch_active); > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > + if (!pstate->pipe[i].sspp) > > + continue; > > + set_bit(pstate->pipe[i].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); > > + &pstate->pipe[i], i, stage_cfg); > > } > > > > /* blend config update */ > > @@ -1387,15 +1382,12 @@ 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[0]:%s\n", > > - pstate->pipe.sspp->cap->name); > > - 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); > > + if (pstate->pipe[i].sspp) { > > + seq_printf(s, "\tsspp[%d]:%s\n", > > + i, pstate->pipe[i].sspp->cap->name); > > + seq_printf(s, "\tmultirect[%d]: mode: %d index: %d\n", > > + i, pstate->pipe[i].multirect_mode, > > + pstate->pipe[i].multirect_index); > > } > > I don't expect that this will work. Yes, the loop is missed. Will add it. > > > > > 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 4df7cfed4d230..e7006fb8c7734 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > @@ -429,7 +429,7 @@ static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw, > > uint32_t chroma_subsmpl_h, uint32_t chroma_subsmpl_v, > > unsigned int rotation) > > { > > - uint32_t i; > > + int i; > > Why? No need actually. It is a typo when converting type of many i. > > > bool inline_rotation = rotation & DRM_MODE_ROTATE_90; > > > > /* > > @@ -619,6 +619,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > > struct msm_drm_private *priv = plane->dev->dev_private; > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); > > u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24); > > + int i; > > > > DPU_DEBUG_PLANE(pdpu, "\n"); > > > > @@ -632,12 +633,12 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > > return; > > > > /* 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); > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > + if (pstate->pipe[i].sspp) > > + _dpu_plane_color_fill_pipe(pstate, &pstate->pipe[i], > > + &pstate->pipe_cfg[i].dst_rect, > > + fill_color, fmt); > > + } > > } > > > > static int dpu_plane_prepare_fb(struct drm_plane *plane, > > @@ -808,8 +809,8 @@ 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 = &pstate->pipe_cfg; > > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > > + struct dpu_sw_pipe_cfg *pipe_cfg; > > + struct dpu_sw_pipe_cfg *r_pipe_cfg; > > struct drm_rect fb_rect = { 0 }; > > uint32_t max_linewidth; > > > > @@ -834,6 +835,9 @@ 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); > > > > @@ -916,11 +920,11 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane, > > drm_atomic_get_new_plane_state(state, plane); > > 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 msm_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 dpu_sw_pipe *pipe = &pstate->pipe[0]; > > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > > uint32_t supported_rotations; > > const struct dpu_sspp_cfg *pipe_hw_caps; > > const struct dpu_sspp_sub_blks *sblk; > > @@ -975,10 +979,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > struct dpu_plane *pdpu = to_dpu_plane(plane); > > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); > > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > > - struct dpu_sw_pipe *pipe = &pstate->pipe; > > - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe; > > - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; > > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > > + struct dpu_sw_pipe *pipe = &pstate->pipe[0]; > > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > > const struct drm_crtc_state *crtc_state = NULL; > > > > if (new_plane_state->crtc) > > @@ -1056,7 +1060,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > > drm_atomic_get_old_plane_state(state, plane); > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state); > > struct drm_crtc_state *crtc_state; > > - int ret; > > + int ret, i; > > > > if (plane_state->crtc) > > crtc_state = drm_atomic_get_new_crtc_state(state, > > @@ -1071,8 +1075,8 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > > * resources are freed by dpu_crtc_assign_plane_resources(), > > * but clean them here. > > */ > > - pstate->pipe.sspp = NULL; > > - pstate->r_pipe.sspp = NULL; > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > + pstate->pipe[i].sspp = NULL; > > > > return 0; > > } > > @@ -1110,19 +1114,22 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, > > struct dpu_sw_pipe_cfg *r_pipe_cfg; > > const struct msm_format *fmt; > > uint32_t max_linewidth; > > + int i; > > > > if (plane_state->crtc) > > crtc_state = drm_atomic_get_new_crtc_state(state, > > plane_state->crtc); > > > > pstate = to_dpu_plane_state(plane_state); > > - pipe = &pstate->pipe; > > - r_pipe = &pstate->r_pipe; > > - pipe_cfg = &pstate->pipe_cfg; > > - r_pipe_cfg = &pstate->r_pipe_cfg; > > > > - pipe->sspp = NULL; > > - r_pipe->sspp = NULL; > > + /* loop below code for another pair later */ > > ?? A marker for TODO. The first pair are handled in this patch. A loop will be added later to loop the handing to the pair. > > > + pipe = &pstate->pipe[0]; > > + r_pipe = &pstate->pipe[1]; > > + pipe_cfg = &pstate->pipe_cfg[0]; > > + r_pipe_cfg = &pstate->pipe_cfg[1]; > > + > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > + pstate->pipe[i].sspp = NULL; > > > > if (!plane_state->fb) > > return -EINVAL; > > @@ -1228,6 +1235,7 @@ void dpu_plane_flush(struct drm_plane *plane) > > { > > struct dpu_plane *pdpu; > > struct dpu_plane_state *pstate; > > + int i; > > > > if (!plane || !plane->state) { > > DPU_ERROR("invalid plane\n"); > > @@ -1248,8 +1256,8 @@ void dpu_plane_flush(struct drm_plane *plane) > > /* force 100% alpha */ > > _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); > > else { > > - dpu_plane_flush_csc(pdpu, &pstate->pipe); > > - dpu_plane_flush_csc(pdpu, &pstate->r_pipe); > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > + dpu_plane_flush_csc(pdpu, &pstate->pipe[i]); > > } > > > > /* flag h/w flush complete */ > > @@ -1349,20 +1357,16 @@ 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 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 msm_format *fmt = > > 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; > > bool layout_valid = false; > > - int ret; > > + int ret, i; > > > > ret = dpu_format_populate_layout(aspace, fb, &layout); > > if (ret) > > @@ -1381,12 +1385,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > > crtc->base.id, DRM_RECT_ARG(&state->dst), > > &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt)); > > > > - dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt, > > - 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, > > + /* move the assignment here, to ease handling to another pairs later */ > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > + if (!pstate->pipe[i].sspp) > > + continue; > > + dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i], > > + &pstate->pipe_cfg[i], fmt, > > drm_mode_vrefresh(&crtc->mode), > > layout_valid ? &layout : NULL); > > } > > @@ -1394,15 +1398,17 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > > if (pstate->needs_qos_remap) > > pstate->needs_qos_remap = false; > > > > - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, > > - &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_fetch_bw = 0; > > + pstate->plane_clk = 0; > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > + if (!pstate->pipe[i].sspp) > > + continue; > > + pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, > > + &crtc->mode, &pstate->pipe_cfg[i]); > > Wrong indentation Could you help elaborate it? I assume it is a bandwidth audit. > > > > > - pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg)); > > + pstate->plane_clk = max(pstate->plane_clk, > > + _dpu_plane_calc_clk(&crtc->mode, > > + &pstate->pipe_cfg[i])); > > } > > } > > > > @@ -1410,17 +1416,24 @@ 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; > > + struct dpu_sw_pipe *pipe; > > + int i; > > + > > + for (i = 0; i < PIPES_PER_STAGE; i += 1) { > > + pipe = &pstate->pipe[i]; > > + if (!pipe->sspp) > > + continue; > > > > - trace_dpu_plane_disable(DRMID(plane), false, > > - pstate->pipe.multirect_mode); > > + trace_dpu_plane_disable(DRMID(plane), false, > > + pstate->pipe[i].multirect_mode); > > > > - if (r_pipe->sspp) { > > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) { > > if (i > 1) Is there any case that pipe->multirect_index == DPU_SSPP_RECT_1 and i == 0 ? > > > + pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > > > - if (r_pipe->sspp->ops.setup_multirect) > > - r_pipe->sspp->ops.setup_multirect(r_pipe); > > + if (pipe->sspp->ops.setup_multirect) > > + pipe->sspp->ops.setup_multirect(pipe); > > + } > > } > > > > pstate->pending = true; > > @@ -1515,30 +1528,26 @@ 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; > > + const struct dpu_sw_pipe *pipe; > > + const struct dpu_sw_pipe_cfg *pipe_cfg; > > + int i; > > > > drm_printf(p, "\tstage=%d\n", pstate->stage); > > > > - if (pipe->sspp) { > > - 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", > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > + pipe = &pstate->pipe[i]; > > + if (!pipe->sspp) > > + continue; > > + pipe_cfg = &pstate->pipe_cfg[i]; > > + drm_printf(p, "\tsspp[%d]=%s\n", i, pipe->sspp->cap->name); > > + drm_printf(p, "\tmultirect_mode[%d]=%s\n", i, > > + dpu_get_multirect_mode(pipe->multirect_mode)); > > + drm_printf(p, "\tmultirect_index[%d]=%s\n", i, > > 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)); > > + drm_printf(p, "\tsrc[%d]=" DRM_RECT_FMT "\n", i, > > + DRM_RECT_ARG(&pipe_cfg->src_rect)); > > + drm_printf(p, "\tdst[%d]=" DRM_RECT_FMT "\n", i, > > + DRM_RECT_ARG(&pipe_cfg->dst_rect)); > > } > > } > > > > @@ -1576,14 +1585,16 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) > > struct dpu_plane *pdpu = to_dpu_plane(plane); > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); > > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > > + int i; > > > > if (!pdpu->is_rt_pipe) > > return; > > > > pm_runtime_get_sync(&dpu_kms->pdev->dev); > > - _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable); > > - if (pstate->r_pipe.sspp) > > - _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable); > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > + if (pstate->pipe[i].sspp) > > + _dpu_plane_set_qos_ctrl(plane, &pstate->pipe[i], enable); > > + } > > 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 e225d5baceb09..39945e0d1b851 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h > > @@ -18,10 +18,8 @@ > > * struct dpu_plane_state: Define dpu extension of drm plane state object > > * @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 > > + * @pipe: software pipe description array > > + * @pipe_cfg: software pipe configuration array > > * @stage: assigned by crtc blender > > * @needs_qos_remap: qos remap settings need to be updated > > * @multirect_index: index of the rectangle of SSPP > > @@ -34,10 +32,8 @@ > > 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; > > + struct dpu_sw_pipe pipe[PIPES_PER_STAGE]; > > + struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE]; > > enum dpu_stage stage; > > bool needs_qos_remap; > > bool pending; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > > index 5307cbc2007c5..cb24ad2a6d8d3 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > > @@ -651,9 +651,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, > > TP_PROTO(uint32_t crtc_id, uint32_t plane_id, > > struct drm_plane_state *state, struct dpu_plane_state *pstate, > > uint32_t stage_idx, uint32_t pixel_format, > > - uint64_t modifier), > > + struct dpu_sw_pipe *pipe, uint64_t modifier), > > TP_ARGS(crtc_id, plane_id, state, pstate, stage_idx, > > - pixel_format, modifier), > > + pixel_format, pipe, modifier), > > TP_STRUCT__entry( > > __field( uint32_t, crtc_id ) > > __field( uint32_t, plane_id ) > > @@ -676,9 +676,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, > > __entry->dst_rect = drm_plane_state_dest(state); > > __entry->stage_idx = stage_idx; > > __entry->stage = pstate->stage; > > - __entry->sspp = pstate->pipe.sspp->idx; > > - __entry->multirect_idx = pstate->pipe.multirect_index; > > - __entry->multirect_mode = pstate->pipe.multirect_mode; > > + __entry->sspp = pipe->sspp->idx; > > + __entry->multirect_idx = pipe->multirect_index; > > + __entry->multirect_mode = pipe->multirect_mode; > > __entry->pixel_format = pixel_format; > > __entry->modifier = modifier; > > ), > > > > -- > > 2.34.1 > > > > -- > With best wishes > Dmitry
On Fri, 11 Oct 2024 at 09:49, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:08写道： > > > > On Wed, Oct 09, 2024 at 04:50:18PM GMT, Jun Nie wrote: > > > Store pipes in array with removing dedicated r_pipe. There are > > > 2 pipes in a drm plane at most currently. While 4 pipes are > > > needed for new usage case. This change generalize the handling > > > to pipe pair and ease handling to another pipe pair later. > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 34 +++--- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 167 ++++++++++++++++-------------- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +-- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- > > > 4 files changed, 111 insertions(+), 112 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > index 3e0e6e9757da5..9656b1df0f122 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > @@ -411,7 +411,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc, > > > > > > trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane), > > > state, to_dpu_plane_state(state), stage_idx, > > > - format->pixel_format, > > > + format->pixel_format, pipe, > > > modifier); > > > > Doesn't seem to be related. > > There is building error when converting r_pipe into pipe array. So I > need to change trace code > accordingly in the same patch. Otherwise, the error happens just after > this patch is applied. No. It is related to your changes that add pipe argument to trace_dpu_crtc_setup_mixer(). Please split that part. > > > > > > > > DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx %d\n", > > > @@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > > > const struct msm_format *format; > > > struct dpu_hw_ctl *ctl = mixer->lm_ctl; > > > > > > - uint32_t lm_idx; > > > + uint32_t lm_idx, i; > > > bool bg_alpha_enable = false; > > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > > > @@ -463,20 +463,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > > > if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) > > > bg_alpha_enable = true; > > > > > > - set_bit(pstate->pipe.sspp->idx, fetch_active); > > > - _dpu_crtc_blend_setup_pipe(crtc, plane, > > > - mixer, cstate->num_mixers, > > > - pstate->stage, > > > - format, fb ? fb->modifier : 0, > > > - &pstate->pipe, 0, stage_cfg); > > > - > > > - if (pstate->r_pipe.sspp) { > > > - set_bit(pstate->r_pipe.sspp->idx, fetch_active); > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > + if (!pstate->pipe[i].sspp) > > > + continue; > > > + set_bit(pstate->pipe[i].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); > > > + &pstate->pipe[i], i, stage_cfg); > > > } > > > > > > /* blend config update */ > > > @@ -1387,15 +1382,12 @@ 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[0]:%s\n", > > > - pstate->pipe.sspp->cap->name); > > > - 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); > > > + if (pstate->pipe[i].sspp) { > > > + seq_printf(s, "\tsspp[%d]:%s\n", > > > + i, pstate->pipe[i].sspp->cap->name); > > > + seq_printf(s, "\tmultirect[%d]: mode: %d index: %d\n", > > > + i, pstate->pipe[i].multirect_mode, > > > + pstate->pipe[i].multirect_index); > > > } > > > > I don't expect that this will work. > > Yes, the loop is missed. Will add it. > > > > > > > > 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 4df7cfed4d230..e7006fb8c7734 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > > @@ -429,7 +429,7 @@ static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw, > > > uint32_t chroma_subsmpl_h, uint32_t chroma_subsmpl_v, > > > unsigned int rotation) > > > { > > > - uint32_t i; > > > + int i; > > > > Why? > > No need actually. It is a typo when converting type of many i. > > > > > > bool inline_rotation = rotation & DRM_MODE_ROTATE_90; > > > > > > /* > > > @@ -619,6 +619,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > > > struct msm_drm_private *priv = plane->dev->dev_private; > > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); > > > u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24); > > > + int i; > > > > > > DPU_DEBUG_PLANE(pdpu, "\n"); > > > > > > @@ -632,12 +633,12 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > > > return; > > > > > > /* 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); > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > + if (pstate->pipe[i].sspp) > > > + _dpu_plane_color_fill_pipe(pstate, &pstate->pipe[i], > > > + &pstate->pipe_cfg[i].dst_rect, > > > + fill_color, fmt); > > > + } > > > } > > > > > > static int dpu_plane_prepare_fb(struct drm_plane *plane, > > > @@ -808,8 +809,8 @@ 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 = &pstate->pipe_cfg; > > > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > > > + struct dpu_sw_pipe_cfg *pipe_cfg; > > > + struct dpu_sw_pipe_cfg *r_pipe_cfg; > > > struct drm_rect fb_rect = { 0 }; > > > uint32_t max_linewidth; > > > > > > @@ -834,6 +835,9 @@ 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); > > > > > > @@ -916,11 +920,11 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane, > > > drm_atomic_get_new_plane_state(state, plane); > > > 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 msm_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 dpu_sw_pipe *pipe = &pstate->pipe[0]; > > > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > > > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > > > uint32_t supported_rotations; > > > const struct dpu_sspp_cfg *pipe_hw_caps; > > > const struct dpu_sspp_sub_blks *sblk; > > > @@ -975,10 +979,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > > struct dpu_plane *pdpu = to_dpu_plane(plane); > > > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); > > > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > > > - struct dpu_sw_pipe *pipe = &pstate->pipe; > > > - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe; > > > - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; > > > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > > > + struct dpu_sw_pipe *pipe = &pstate->pipe[0]; > > > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > > > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > > > const struct drm_crtc_state *crtc_state = NULL; > > > > > > if (new_plane_state->crtc) > > > @@ -1056,7 +1060,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > > > drm_atomic_get_old_plane_state(state, plane); > > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state); > > > struct drm_crtc_state *crtc_state; > > > - int ret; > > > + int ret, i; > > > > > > if (plane_state->crtc) > > > crtc_state = drm_atomic_get_new_crtc_state(state, > > > @@ -1071,8 +1075,8 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > > > * resources are freed by dpu_crtc_assign_plane_resources(), > > > * but clean them here. > > > */ > > > - pstate->pipe.sspp = NULL; > > > - pstate->r_pipe.sspp = NULL; > > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > > + pstate->pipe[i].sspp = NULL; > > > > > > return 0; > > > } > > > @@ -1110,19 +1114,22 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, > > > struct dpu_sw_pipe_cfg *r_pipe_cfg; > > > const struct msm_format *fmt; > > > uint32_t max_linewidth; > > > + int i; > > > > > > if (plane_state->crtc) > > > crtc_state = drm_atomic_get_new_crtc_state(state, > > > plane_state->crtc); > > > > > > pstate = to_dpu_plane_state(plane_state); > > > - pipe = &pstate->pipe; > > > - r_pipe = &pstate->r_pipe; > > > - pipe_cfg = &pstate->pipe_cfg; > > > - r_pipe_cfg = &pstate->r_pipe_cfg; > > > > > > - pipe->sspp = NULL; > > > - r_pipe->sspp = NULL; > > > + /* loop below code for another pair later */ > > > > ?? > > A marker for TODO. The first pair are handled in this patch. A loop will be > added later to loop the handing to the pair. A marker for TODO is 'TODO: foo bar'. > > > > > + pipe = &pstate->pipe[0]; > > > + r_pipe = &pstate->pipe[1]; > > > + pipe_cfg = &pstate->pipe_cfg[0]; > > > + r_pipe_cfg = &pstate->pipe_cfg[1]; > > > + > > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > > + pstate->pipe[i].sspp = NULL; > > > > > > if (!plane_state->fb) > > > return -EINVAL; > > > @@ -1228,6 +1235,7 @@ void dpu_plane_flush(struct drm_plane *plane) > > > { > > > struct dpu_plane *pdpu; > > > struct dpu_plane_state *pstate; > > > + int i; > > > > > > if (!plane || !plane->state) { > > > DPU_ERROR("invalid plane\n"); > > > @@ -1248,8 +1256,8 @@ void dpu_plane_flush(struct drm_plane *plane) > > > /* force 100% alpha */ > > > _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); > > > else { > > > - dpu_plane_flush_csc(pdpu, &pstate->pipe); > > > - dpu_plane_flush_csc(pdpu, &pstate->r_pipe); > > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > > + dpu_plane_flush_csc(pdpu, &pstate->pipe[i]); > > > } > > > > > > /* flag h/w flush complete */ > > > @@ -1349,20 +1357,16 @@ 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 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 msm_format *fmt = > > > 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; > > > bool layout_valid = false; > > > - int ret; > > > + int ret, i; > > > > > > ret = dpu_format_populate_layout(aspace, fb, &layout); > > > if (ret) > > > @@ -1381,12 +1385,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > > > crtc->base.id, DRM_RECT_ARG(&state->dst), > > > &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt)); > > > > > > - dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt, > > > - 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, > > > + /* move the assignment here, to ease handling to another pairs later */ > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > + if (!pstate->pipe[i].sspp) > > > + continue; > > > + dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i], > > > + &pstate->pipe_cfg[i], fmt, > > > drm_mode_vrefresh(&crtc->mode), > > > layout_valid ? &layout : NULL); > > > } > > > @@ -1394,15 +1398,17 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > > > if (pstate->needs_qos_remap) > > > pstate->needs_qos_remap = false; > > > > > > - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, > > > - &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_fetch_bw = 0; > > > + pstate->plane_clk = 0; > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > + if (!pstate->pipe[i].sspp) > > > + continue; > > > + pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, > > > + &crtc->mode, &pstate->pipe_cfg[i]); > > > > Wrong indentation > > Could you help elaborate it? I assume it is a bandwidth audit. Indent to the opening bracket please. > > > > > > > > - pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg)); > > > + pstate->plane_clk = max(pstate->plane_clk, > > > + _dpu_plane_calc_clk(&crtc->mode, > > > + &pstate->pipe_cfg[i])); > > > } > > > } > > > > > > @@ -1410,17 +1416,24 @@ 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; > > > + struct dpu_sw_pipe *pipe; > > > + int i; > > > + > > > + for (i = 0; i < PIPES_PER_STAGE; i += 1) { > > > + pipe = &pstate->pipe[i]; > > > + if (!pipe->sspp) > > > + continue; > > > > > > - trace_dpu_plane_disable(DRMID(plane), false, > > > - pstate->pipe.multirect_mode); > > > + trace_dpu_plane_disable(DRMID(plane), false, > > > + pstate->pipe[i].multirect_mode); > > > > > > - if (r_pipe->sspp) { > > > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) { > > > > if (i > 1) > > Is there any case that pipe->multirect_index == DPU_SSPP_RECT_1 and i == 0 ? You are converting the code. Please don't change the logic. Original code had separate handling for r_pipe. After your conversion it should be if (i == 1) or if (i > 0), which means a typo in my review comment. > > > > > + pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > > + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > > > > > - if (r_pipe->sspp->ops.setup_multirect) > > > - r_pipe->sspp->ops.setup_multirect(r_pipe); > > > + if (pipe->sspp->ops.setup_multirect) > > > + pipe->sspp->ops.setup_multirect(pipe); > > > + } > > > } > > > > > > pstate->pending = true; > > > @@ -1515,30 +1528,26 @@ 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; > > > + const struct dpu_sw_pipe *pipe; > > > + const struct dpu_sw_pipe_cfg *pipe_cfg; > > > + int i; > > > > > > drm_printf(p, "\tstage=%d\n", pstate->stage); > > > > > > - if (pipe->sspp) { > > > - 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", > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > + pipe = &pstate->pipe[i]; > > > + if (!pipe->sspp) > > > + continue; > > > + pipe_cfg = &pstate->pipe_cfg[i]; > > > + drm_printf(p, "\tsspp[%d]=%s\n", i, pipe->sspp->cap->name); > > > + drm_printf(p, "\tmultirect_mode[%d]=%s\n", i, > > > + dpu_get_multirect_mode(pipe->multirect_mode)); > > > + drm_printf(p, "\tmultirect_index[%d]=%s\n", i, > > > 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)); > > > + drm_printf(p, "\tsrc[%d]=" DRM_RECT_FMT "\n", i, > > > + DRM_RECT_ARG(&pipe_cfg->src_rect)); > > > + drm_printf(p, "\tdst[%d]=" DRM_RECT_FMT "\n", i, > > > + DRM_RECT_ARG(&pipe_cfg->dst_rect)); > > > } > > > } > > > > > > @@ -1576,14 +1585,16 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) > > > struct dpu_plane *pdpu = to_dpu_plane(plane); > > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); > > > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > > > + int i; > > > > > > if (!pdpu->is_rt_pipe) > > > return; > > > > > > pm_runtime_get_sync(&dpu_kms->pdev->dev); > > > - _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable); > > > - if (pstate->r_pipe.sspp) > > > - _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable); > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > + if (pstate->pipe[i].sspp) > > > + _dpu_plane_set_qos_ctrl(plane, &pstate->pipe[i], enable); > > > + } > > > 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 e225d5baceb09..39945e0d1b851 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h > > > @@ -18,10 +18,8 @@ > > > * struct dpu_plane_state: Define dpu extension of drm plane state object > > > * @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 > > > + * @pipe: software pipe description array > > > + * @pipe_cfg: software pipe configuration array > > > * @stage: assigned by crtc blender > > > * @needs_qos_remap: qos remap settings need to be updated > > > * @multirect_index: index of the rectangle of SSPP > > > @@ -34,10 +32,8 @@ > > > 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; > > > + struct dpu_sw_pipe pipe[PIPES_PER_STAGE]; > > > + struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE]; > > > enum dpu_stage stage; > > > bool needs_qos_remap; > > > bool pending; > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > > > index 5307cbc2007c5..cb24ad2a6d8d3 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h > > > @@ -651,9 +651,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, > > > TP_PROTO(uint32_t crtc_id, uint32_t plane_id, > > > struct drm_plane_state *state, struct dpu_plane_state *pstate, > > > uint32_t stage_idx, uint32_t pixel_format, > > > - uint64_t modifier), > > > + struct dpu_sw_pipe *pipe, uint64_t modifier), > > > TP_ARGS(crtc_id, plane_id, state, pstate, stage_idx, > > > - pixel_format, modifier), > > > + pixel_format, pipe, modifier), > > > TP_STRUCT__entry( > > > __field( uint32_t, crtc_id ) > > > __field( uint32_t, plane_id ) > > > @@ -676,9 +676,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, > > > __entry->dst_rect = drm_plane_state_dest(state); > > > __entry->stage_idx = stage_idx; > > > __entry->stage = pstate->stage; > > > - __entry->sspp = pstate->pipe.sspp->idx; > > > - __entry->multirect_idx = pstate->pipe.multirect_index; > > > - __entry->multirect_mode = pstate->pipe.multirect_mode; > > > + __entry->sspp = pipe->sspp->idx; > > > + __entry->multirect_idx = pipe->multirect_index; > > > + __entry->multirect_mode = pipe->multirect_mode; > > > __entry->pixel_format = pixel_format; > > > __entry->modifier = modifier; > > > ), > > > > > > -- > > > 2.34.1 > > > > > > > -- > > With best wishes > > Dmitry
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:10写道： > > On Fri, 11 Oct 2024 at 09:49, Jun Nie <jun.nie@linaro.org> wrote: > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:08写道： > > > > > > On Wed, Oct 09, 2024 at 04:50:18PM GMT, Jun Nie wrote: > > > > Store pipes in array with removing dedicated r_pipe. There are > > > > 2 pipes in a drm plane at most currently. While 4 pipes are > > > > needed for new usage case. This change generalize the handling > > > > to pipe pair and ease handling to another pipe pair later. > > > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > > --- > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 34 +++--- > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 167 ++++++++++++++++-------------- > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +-- > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- > > > > 4 files changed, 111 insertions(+), 112 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > > index 3e0e6e9757da5..9656b1df0f122 100644 > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > > > @@ -411,7 +411,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc, > > > > > > > > trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane), > > > > state, to_dpu_plane_state(state), stage_idx, > > > > - format->pixel_format, > > > > + format->pixel_format, pipe, > > > > modifier); > > > > > > Doesn't seem to be related. > > > > There is building error when converting r_pipe into pipe array. So I > > need to change trace code > > accordingly in the same patch. Otherwise, the error happens just after > > this patch is applied. > > No. It is related to your changes that add pipe argument to > trace_dpu_crtc_setup_mixer(). Please split that part. OK. > > > > > > > > > > > > DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx %d\n", > > > > @@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > > > > const struct msm_format *format; > > > > struct dpu_hw_ctl *ctl = mixer->lm_ctl; > > > > > > > > - uint32_t lm_idx; > > > > + uint32_t lm_idx, i; > > > > bool bg_alpha_enable = false; > > > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > > > > > @@ -463,20 +463,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, > > > > if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) > > > > bg_alpha_enable = true; > > > > > > > > - set_bit(pstate->pipe.sspp->idx, fetch_active); > > > > - _dpu_crtc_blend_setup_pipe(crtc, plane, > > > > - mixer, cstate->num_mixers, > > > > - pstate->stage, > > > > - format, fb ? fb->modifier : 0, > > > > - &pstate->pipe, 0, stage_cfg); > > > > - > > > > - if (pstate->r_pipe.sspp) { > > > > - set_bit(pstate->r_pipe.sspp->idx, fetch_active); > > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > > + if (!pstate->pipe[i].sspp) > > > > + continue; > > > > + set_bit(pstate->pipe[i].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); > > > > + &pstate->pipe[i], i, stage_cfg); > > > > } > > > > > > > > /* blend config update */ > > > > @@ -1387,15 +1382,12 @@ 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[0]:%s\n", > > > > - pstate->pipe.sspp->cap->name); > > > > - 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); > > > > + if (pstate->pipe[i].sspp) { > > > > + seq_printf(s, "\tsspp[%d]:%s\n", > > > > + i, pstate->pipe[i].sspp->cap->name); > > > > + seq_printf(s, "\tmultirect[%d]: mode: %d index: %d\n", > > > > + i, pstate->pipe[i].multirect_mode, > > > > + pstate->pipe[i].multirect_index); > > > > } > > > > > > I don't expect that this will work. > > > > Yes, the loop is missed. Will add it. > > > > > > > > > > > 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 4df7cfed4d230..e7006fb8c7734 100644 > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > > > @@ -429,7 +429,7 @@ static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw, > > > > uint32_t chroma_subsmpl_h, uint32_t chroma_subsmpl_v, > > > > unsigned int rotation) > > > > { > > > > - uint32_t i; > > > > + int i; > > > > > > Why? > > > > No need actually. It is a typo when converting type of many i. > > > > > > > > > bool inline_rotation = rotation & DRM_MODE_ROTATE_90; > > > > > > > > /* > > > > @@ -619,6 +619,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > > > > struct msm_drm_private *priv = plane->dev->dev_private; > > > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); > > > > u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24); > > > > + int i; > > > > > > > > DPU_DEBUG_PLANE(pdpu, "\n"); > > > > > > > > @@ -632,12 +633,12 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, > > > > return; > > > > > > > > /* 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); > > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > > + if (pstate->pipe[i].sspp) > > > > + _dpu_plane_color_fill_pipe(pstate, &pstate->pipe[i], > > > > + &pstate->pipe_cfg[i].dst_rect, > > > > + fill_color, fmt); > > > > + } > > > > } > > > > > > > > static int dpu_plane_prepare_fb(struct drm_plane *plane, > > > > @@ -808,8 +809,8 @@ 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 = &pstate->pipe_cfg; > > > > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > > > > + struct dpu_sw_pipe_cfg *pipe_cfg; > > > > + struct dpu_sw_pipe_cfg *r_pipe_cfg; > > > > struct drm_rect fb_rect = { 0 }; > > > > uint32_t max_linewidth; > > > > > > > > @@ -834,6 +835,9 @@ 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); > > > > > > > > @@ -916,11 +920,11 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane, > > > > drm_atomic_get_new_plane_state(state, plane); > > > > 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 msm_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 dpu_sw_pipe *pipe = &pstate->pipe[0]; > > > > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > > > > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > > > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > > > > uint32_t supported_rotations; > > > > const struct dpu_sspp_cfg *pipe_hw_caps; > > > > const struct dpu_sspp_sub_blks *sblk; > > > > @@ -975,10 +979,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, > > > > struct dpu_plane *pdpu = to_dpu_plane(plane); > > > > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); > > > > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > > > > - struct dpu_sw_pipe *pipe = &pstate->pipe; > > > > - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe; > > > > - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; > > > > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; > > > > + struct dpu_sw_pipe *pipe = &pstate->pipe[0]; > > > > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; > > > > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; > > > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; > > > > const struct drm_crtc_state *crtc_state = NULL; > > > > > > > > if (new_plane_state->crtc) > > > > @@ -1056,7 +1060,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > > > > drm_atomic_get_old_plane_state(state, plane); > > > > struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state); > > > > struct drm_crtc_state *crtc_state; > > > > - int ret; > > > > + int ret, i; > > > > > > > > if (plane_state->crtc) > > > > crtc_state = drm_atomic_get_new_crtc_state(state, > > > > @@ -1071,8 +1075,8 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, > > > > * resources are freed by dpu_crtc_assign_plane_resources(), > > > > * but clean them here. > > > > */ > > > > - pstate->pipe.sspp = NULL; > > > > - pstate->r_pipe.sspp = NULL; > > > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > > > + pstate->pipe[i].sspp = NULL; > > > > > > > > return 0; > > > > } > > > > @@ -1110,19 +1114,22 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, > > > > struct dpu_sw_pipe_cfg *r_pipe_cfg; > > > > const struct msm_format *fmt; > > > > uint32_t max_linewidth; > > > > + int i; > > > > > > > > if (plane_state->crtc) > > > > crtc_state = drm_atomic_get_new_crtc_state(state, > > > > plane_state->crtc); > > > > > > > > pstate = to_dpu_plane_state(plane_state); > > > > - pipe = &pstate->pipe; > > > > - r_pipe = &pstate->r_pipe; > > > > - pipe_cfg = &pstate->pipe_cfg; > > > > - r_pipe_cfg = &pstate->r_pipe_cfg; > > > > > > > > - pipe->sspp = NULL; > > > > - r_pipe->sspp = NULL; > > > > + /* loop below code for another pair later */ > > > > > > ?? > > > > A marker for TODO. The first pair are handled in this patch. A loop will be > > added later to loop the handing to the pair. > > A marker for TODO is 'TODO: foo bar'. > > > > > > > > + pipe = &pstate->pipe[0]; > > > > + r_pipe = &pstate->pipe[1]; > > > > + pipe_cfg = &pstate->pipe_cfg[0]; > > > > + r_pipe_cfg = &pstate->pipe_cfg[1]; > > > > + > > > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > > > + pstate->pipe[i].sspp = NULL; > > > > > > > > if (!plane_state->fb) > > > > return -EINVAL; > > > > @@ -1228,6 +1235,7 @@ void dpu_plane_flush(struct drm_plane *plane) > > > > { > > > > struct dpu_plane *pdpu; > > > > struct dpu_plane_state *pstate; > > > > + int i; > > > > > > > > if (!plane || !plane->state) { > > > > DPU_ERROR("invalid plane\n"); > > > > @@ -1248,8 +1256,8 @@ void dpu_plane_flush(struct drm_plane *plane) > > > > /* force 100% alpha */ > > > > _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); > > > > else { > > > > - dpu_plane_flush_csc(pdpu, &pstate->pipe); > > > > - dpu_plane_flush_csc(pdpu, &pstate->r_pipe); > > > > + for (i = 0; i < PIPES_PER_STAGE; i++) > > > > + dpu_plane_flush_csc(pdpu, &pstate->pipe[i]); > > > > } > > > > > > > > /* flag h/w flush complete */ > > > > @@ -1349,20 +1357,16 @@ 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 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 msm_format *fmt = > > > > 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; > > > > bool layout_valid = false; > > > > - int ret; > > > > + int ret, i; > > > > > > > > ret = dpu_format_populate_layout(aspace, fb, &layout); > > > > if (ret) > > > > @@ -1381,12 +1385,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > > > > crtc->base.id, DRM_RECT_ARG(&state->dst), > > > > &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt)); > > > > > > > > - dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt, > > > > - 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, > > > > + /* move the assignment here, to ease handling to another pairs later */ > > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > > + if (!pstate->pipe[i].sspp) > > > > + continue; > > > > + dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i], > > > > + &pstate->pipe_cfg[i], fmt, > > > > drm_mode_vrefresh(&crtc->mode), > > > > layout_valid ? &layout : NULL); > > > > } > > > > @@ -1394,15 +1398,17 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) > > > > if (pstate->needs_qos_remap) > > > > pstate->needs_qos_remap = false; > > > > > > > > - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, > > > > - &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_fetch_bw = 0; > > > > + pstate->plane_clk = 0; > > > > + for (i = 0; i < PIPES_PER_STAGE; i++) { > > > > + if (!pstate->pipe[i].sspp) > > > > + continue; > > > > + pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, > > > > + &crtc->mode, &pstate->pipe_cfg[i]); > > > > > > Wrong indentation > > > > Could you help elaborate it? I assume it is a bandwidth audit. > > Indent to the opening bracket please. > > > > > > > > > > > > - pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg)); > > > > + pstate->plane_clk = max(pstate->plane_clk, > > > > + _dpu_plane_calc_clk(&crtc->mode, > > > > + &pstate->pipe_cfg[i])); > > > > } > > > > } > > > > > > > > @@ -1410,17 +1416,24 @@ 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; > > > > + struct dpu_sw_pipe *pipe; > > > > + int i; > > > > + > > > > + for (i = 0; i < PIPES_PER_STAGE; i += 1) { > > > > + pipe = &pstate->pipe[i]; > > > > + if (!pipe->sspp) > > > > + continue; > > > > > > > > - trace_dpu_plane_disable(DRMID(plane), false, > > > > - pstate->pipe.multirect_mode); > > > > + trace_dpu_plane_disable(DRMID(plane), false, > > > > + pstate->pipe[i].multirect_mode); > > > > > > > > - if (r_pipe->sspp) { > > > > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > > > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > > > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) { > > > > > > if (i > 1) > > > > Is there any case that pipe->multirect_index == DPU_SSPP_RECT_1 and i == 0 ? > > You are converting the code. Please don't change the logic. Original > code had separate handling for r_pipe. After your conversion it should > be if (i == 1) or if (i > 0), which means a typo in my review comment. > I see. You want to keep logic unchanged, with only handling the pipe in array. Handling to multiple pipe pairs can be added later. - Jun
On Fri, 11 Oct 2024 at 10:18, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:10写道： > > > > On Fri, 11 Oct 2024 at 09:49, Jun Nie <jun.nie@linaro.org> wrote: > > > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:08写道： > > > > > > > > On Wed, Oct 09, 2024 at 04:50:18PM GMT, Jun Nie wrote: > > > > > Store pipes in array with removing dedicated r_pipe. There are > > > > > 2 pipes in a drm plane at most currently. While 4 pipes are > > > > > needed for new usage case. This change generalize the handling > > > > > to pipe pair and ease handling to another pipe pair later. > > > > > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > > > --- > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 34 +++--- > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 167 ++++++++++++++++-------------- > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +-- > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- > > > > > 4 files changed, 111 insertions(+), 112 deletions(-) > > > > > > > > > > @@ -1410,17 +1416,24 @@ 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; > > > > > + struct dpu_sw_pipe *pipe; > > > > > + int i; > > > > > + > > > > > + for (i = 0; i < PIPES_PER_STAGE; i += 1) { > > > > > + pipe = &pstate->pipe[i]; > > > > > + if (!pipe->sspp) > > > > > + continue; > > > > > > > > > > - trace_dpu_plane_disable(DRMID(plane), false, > > > > > - pstate->pipe.multirect_mode); > > > > > + trace_dpu_plane_disable(DRMID(plane), false, > > > > > + pstate->pipe[i].multirect_mode); > > > > > > > > > > - if (r_pipe->sspp) { > > > > > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; > > > > > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; > > > > > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) { > > > > > > > > if (i > 1) > > > > > > Is there any case that pipe->multirect_index == DPU_SSPP_RECT_1 and i == 0 ? > > > > You are converting the code. Please don't change the logic. Original > > code had separate handling for r_pipe. After your conversion it should > > be if (i == 1) or if (i > 0), which means a typo in my review comment. > > > I see. You want to keep logic unchanged, with only handling the pipe in array. > Handling to multiple pipe pairs can be added later. Yes. Please never mix "rewrite" and "change the logic" together. It complicates the review and is generally error-prone.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 3e0e6e9757da5..9656b1df0f122 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -411,7 +411,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc, trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane), state, to_dpu_plane_state(state), stage_idx, - format->pixel_format, + format->pixel_format, pipe, modifier); DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx %d\n", @@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, const struct msm_format *format; struct dpu_hw_ctl *ctl = mixer->lm_ctl; - uint32_t lm_idx; + uint32_t lm_idx, i; bool bg_alpha_enable = false; DECLARE_BITMAP(fetch_active, SSPP_MAX); @@ -463,20 +463,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc, if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable) bg_alpha_enable = true; - set_bit(pstate->pipe.sspp->idx, fetch_active); - _dpu_crtc_blend_setup_pipe(crtc, plane, - mixer, cstate->num_mixers, - pstate->stage, - format, fb ? fb->modifier : 0, - &pstate->pipe, 0, stage_cfg); - - if (pstate->r_pipe.sspp) { - set_bit(pstate->r_pipe.sspp->idx, fetch_active); + for (i = 0; i < PIPES_PER_STAGE; i++) { + if (!pstate->pipe[i].sspp) + continue; + set_bit(pstate->pipe[i].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); + &pstate->pipe[i], i, stage_cfg); } /* blend config update */ @@ -1387,15 +1382,12 @@ 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[0]:%s\n", - pstate->pipe.sspp->cap->name); - 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); + if (pstate->pipe[i].sspp) { + seq_printf(s, "\tsspp[%d]:%s\n", + i, pstate->pipe[i].sspp->cap->name); + seq_printf(s, "\tmultirect[%d]: mode: %d index: %d\n", + i, pstate->pipe[i].multirect_mode, + pstate->pipe[i].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 4df7cfed4d230..e7006fb8c7734 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -429,7 +429,7 @@ static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw, uint32_t chroma_subsmpl_h, uint32_t chroma_subsmpl_v, unsigned int rotation) { - uint32_t i; + int i; bool inline_rotation = rotation & DRM_MODE_ROTATE_90; /* @@ -619,6 +619,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, struct msm_drm_private *priv = plane->dev->dev_private; struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24); + int i; DPU_DEBUG_PLANE(pdpu, "\n"); @@ -632,12 +633,12 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu, return; /* 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); + for (i = 0; i < PIPES_PER_STAGE; i++) { + if (pstate->pipe[i].sspp) + _dpu_plane_color_fill_pipe(pstate, &pstate->pipe[i], + &pstate->pipe_cfg[i].dst_rect, + fill_color, fmt); + } } static int dpu_plane_prepare_fb(struct drm_plane *plane, @@ -808,8 +809,8 @@ 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 = &pstate->pipe_cfg; - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; + struct dpu_sw_pipe_cfg *pipe_cfg; + struct dpu_sw_pipe_cfg *r_pipe_cfg; struct drm_rect fb_rect = { 0 }; uint32_t max_linewidth; @@ -834,6 +835,9 @@ 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); @@ -916,11 +920,11 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane, drm_atomic_get_new_plane_state(state, plane); 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 msm_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 dpu_sw_pipe *pipe = &pstate->pipe[0]; + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; uint32_t supported_rotations; const struct dpu_sspp_cfg *pipe_hw_caps; const struct dpu_sspp_sub_blks *sblk; @@ -975,10 +979,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state); struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); - struct dpu_sw_pipe *pipe = &pstate->pipe; - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe; - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg; - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg; + struct dpu_sw_pipe *pipe = &pstate->pipe[0]; + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1]; + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0]; + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1]; const struct drm_crtc_state *crtc_state = NULL; if (new_plane_state->crtc) @@ -1056,7 +1060,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, drm_atomic_get_old_plane_state(state, plane); struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state); struct drm_crtc_state *crtc_state; - int ret; + int ret, i; if (plane_state->crtc) crtc_state = drm_atomic_get_new_crtc_state(state, @@ -1071,8 +1075,8 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane, * resources are freed by dpu_crtc_assign_plane_resources(), * but clean them here. */ - pstate->pipe.sspp = NULL; - pstate->r_pipe.sspp = NULL; + for (i = 0; i < PIPES_PER_STAGE; i++) + pstate->pipe[i].sspp = NULL; return 0; } @@ -1110,19 +1114,22 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc, struct dpu_sw_pipe_cfg *r_pipe_cfg; const struct msm_format *fmt; uint32_t max_linewidth; + int i; if (plane_state->crtc) crtc_state = drm_atomic_get_new_crtc_state(state, plane_state->crtc); pstate = to_dpu_plane_state(plane_state); - pipe = &pstate->pipe; - r_pipe = &pstate->r_pipe; - pipe_cfg = &pstate->pipe_cfg; - r_pipe_cfg = &pstate->r_pipe_cfg; - pipe->sspp = NULL; - r_pipe->sspp = NULL; + /* loop below code for another pair later */ + pipe = &pstate->pipe[0]; + r_pipe = &pstate->pipe[1]; + pipe_cfg = &pstate->pipe_cfg[0]; + r_pipe_cfg = &pstate->pipe_cfg[1]; + + for (i = 0; i < PIPES_PER_STAGE; i++) + pstate->pipe[i].sspp = NULL; if (!plane_state->fb) return -EINVAL; @@ -1228,6 +1235,7 @@ void dpu_plane_flush(struct drm_plane *plane) { struct dpu_plane *pdpu; struct dpu_plane_state *pstate; + int i; if (!plane || !plane->state) { DPU_ERROR("invalid plane\n"); @@ -1248,8 +1256,8 @@ void dpu_plane_flush(struct drm_plane *plane) /* force 100% alpha */ _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); else { - dpu_plane_flush_csc(pdpu, &pstate->pipe); - dpu_plane_flush_csc(pdpu, &pstate->r_pipe); + for (i = 0; i < PIPES_PER_STAGE; i++) + dpu_plane_flush_csc(pdpu, &pstate->pipe[i]); } /* flag h/w flush complete */ @@ -1349,20 +1357,16 @@ 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 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 msm_format *fmt = 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; bool layout_valid = false; - int ret; + int ret, i; ret = dpu_format_populate_layout(aspace, fb, &layout); if (ret) @@ -1381,12 +1385,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) crtc->base.id, DRM_RECT_ARG(&state->dst), &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt)); - dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt, - 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, + /* move the assignment here, to ease handling to another pairs later */ + for (i = 0; i < PIPES_PER_STAGE; i++) { + if (!pstate->pipe[i].sspp) + continue; + dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i], + &pstate->pipe_cfg[i], fmt, drm_mode_vrefresh(&crtc->mode), layout_valid ? &layout : NULL); } @@ -1394,15 +1398,17 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) if (pstate->needs_qos_remap) pstate->needs_qos_remap = false; - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, - &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_fetch_bw = 0; + pstate->plane_clk = 0; + for (i = 0; i < PIPES_PER_STAGE; i++) { + if (!pstate->pipe[i].sspp) + continue; + pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, + &crtc->mode, &pstate->pipe_cfg[i]); - pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg)); + pstate->plane_clk = max(pstate->plane_clk, + _dpu_plane_calc_clk(&crtc->mode, + &pstate->pipe_cfg[i])); } } @@ -1410,17 +1416,24 @@ 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; + struct dpu_sw_pipe *pipe; + int i; + + for (i = 0; i < PIPES_PER_STAGE; i += 1) { + pipe = &pstate->pipe[i]; + if (!pipe->sspp) + continue; - trace_dpu_plane_disable(DRMID(plane), false, - pstate->pipe.multirect_mode); + trace_dpu_plane_disable(DRMID(plane), false, + pstate->pipe[i].multirect_mode); - if (r_pipe->sspp) { - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO; - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) { + pipe->multirect_index = DPU_SSPP_RECT_SOLO; + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE; - if (r_pipe->sspp->ops.setup_multirect) - r_pipe->sspp->ops.setup_multirect(r_pipe); + if (pipe->sspp->ops.setup_multirect) + pipe->sspp->ops.setup_multirect(pipe); + } } pstate->pending = true; @@ -1515,30 +1528,26 @@ 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; + const struct dpu_sw_pipe *pipe; + const struct dpu_sw_pipe_cfg *pipe_cfg; + int i; drm_printf(p, "\tstage=%d\n", pstate->stage); - if (pipe->sspp) { - 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", + for (i = 0; i < PIPES_PER_STAGE; i++) { + pipe = &pstate->pipe[i]; + if (!pipe->sspp) + continue; + pipe_cfg = &pstate->pipe_cfg[i]; + drm_printf(p, "\tsspp[%d]=%s\n", i, pipe->sspp->cap->name); + drm_printf(p, "\tmultirect_mode[%d]=%s\n", i, + dpu_get_multirect_mode(pipe->multirect_mode)); + drm_printf(p, "\tmultirect_index[%d]=%s\n", i, 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)); + drm_printf(p, "\tsrc[%d]=" DRM_RECT_FMT "\n", i, + DRM_RECT_ARG(&pipe_cfg->src_rect)); + drm_printf(p, "\tdst[%d]=" DRM_RECT_FMT "\n", i, + DRM_RECT_ARG(&pipe_cfg->dst_rect)); } } @@ -1576,14 +1585,16 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) struct dpu_plane *pdpu = to_dpu_plane(plane); struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state); struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); + int i; if (!pdpu->is_rt_pipe) return; pm_runtime_get_sync(&dpu_kms->pdev->dev); - _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable); - if (pstate->r_pipe.sspp) - _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable); + for (i = 0; i < PIPES_PER_STAGE; i++) { + if (pstate->pipe[i].sspp) + _dpu_plane_set_qos_ctrl(plane, &pstate->pipe[i], enable); + } 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 e225d5baceb09..39945e0d1b851 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h @@ -18,10 +18,8 @@ * struct dpu_plane_state: Define dpu extension of drm plane state object * @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 + * @pipe: software pipe description array + * @pipe_cfg: software pipe configuration array * @stage: assigned by crtc blender * @needs_qos_remap: qos remap settings need to be updated * @multirect_index: index of the rectangle of SSPP @@ -34,10 +32,8 @@ 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; + struct dpu_sw_pipe pipe[PIPES_PER_STAGE]; + struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE]; enum dpu_stage stage; bool needs_qos_remap; bool pending; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 5307cbc2007c5..cb24ad2a6d8d3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -651,9 +651,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, TP_PROTO(uint32_t crtc_id, uint32_t plane_id, struct drm_plane_state *state, struct dpu_plane_state *pstate, uint32_t stage_idx, uint32_t pixel_format, - uint64_t modifier), + struct dpu_sw_pipe *pipe, uint64_t modifier), TP_ARGS(crtc_id, plane_id, state, pstate, stage_idx, - pixel_format, modifier), + pixel_format, pipe, modifier), TP_STRUCT__entry( __field( uint32_t, crtc_id ) __field( uint32_t, plane_id ) @@ -676,9 +676,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer, __entry->dst_rect = drm_plane_state_dest(state); __entry->stage_idx = stage_idx; __entry->stage = pstate->stage; - __entry->sspp = pstate->pipe.sspp->idx; - __entry->multirect_idx = pstate->pipe.multirect_index; - __entry->multirect_mode = pstate->pipe.multirect_mode; + __entry->sspp = pipe->sspp->idx; + __entry->multirect_idx = pipe->multirect_index; + __entry->multirect_mode = pipe->multirect_mode; __entry->pixel_format = pixel_format; __entry->modifier = modifier; ),
Store pipes in array with removing dedicated r_pipe. There are 2 pipes in a drm plane at most currently. While 4 pipes are needed for new usage case. This change generalize the handling to pipe pair and ease handling to another pipe pair later. Signed-off-by: Jun Nie <jun.nie@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 34 +++--- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 167 ++++++++++++++++-------------- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- 4 files changed, 111 insertions(+), 112 deletions(-)