[Freedreno] [PATCH 10/25] drm/msm/dpu: introduce struct dpu_sw_pipe
Abhinav Kumar
quic_abhinavk at quicinc.com
Fri May 6 21:48:11 UTC 2022
On 5/6/2022 2:39 PM, Dmitry Baryshkov wrote:
> On 07/05/2022 00:30, Abhinav Kumar wrote:
>>
>>
>> On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote:
>>> Wrap SSPP and multirect index/mode into a single structure that
>>> represents software view on the pipe used.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 8 +-
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 16 ++-
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 136 ++++++++++----------
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 +-
>>> 4 files changed, 86 insertions(+), 78 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>> index ada7d5750536..751c64012058 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>> @@ -441,14 +441,14 @@ static void _dpu_crtc_blend_setup_ctl(struct
>>> drm_crtc *crtc)
>>> pstate = to_dpu_plane_state(state);
>>> - sspp_idx = pstate->pipe_hw->idx;
>>> + sspp_idx = pstate->pipe.sspp->idx;
>>> set_bit(sspp_idx, fetch_active);
>>> stage_idx = zpos_cnt[pstate->stage]++;
>>> stage_cfg.stage[pstate->stage][stage_idx] =
>>> sspp_idx;
>>> stage_cfg.multirect_index[pstate->stage][stage_idx] =
>>> - pstate->multirect_index;
>>> + pstate->pipe.multirect_index;
>>> /* blend config update */
>>> for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++)
>>> @@ -1118,7 +1118,7 @@ static int dpu_crtc_atomic_check(struct
>>> drm_crtc *crtc,
>>> pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate);
>>> pstates[cnt].drm_pstate = pstate;
>>> pstates[cnt].stage = pstate->normalized_zpos;
>>> - pstates[cnt].pipe_id =
>>> to_dpu_plane_state(pstate)->pipe_hw->idx;
>>> + pstates[cnt].pipe_id =
>>> to_dpu_plane_state(pstate)->pipe.sspp->idx;
>>> if (pipe_staged[pstates[cnt].pipe_id]) {
>>> multirect_plane[multirect_count].r0 =
>>> @@ -1389,7 +1389,7 @@ static int _dpu_debugfs_status_show(struct
>>> seq_file *s, void *data)
>>> state->crtc_x, state->crtc_y, state->crtc_w,
>>> state->crtc_h);
>>> seq_printf(s, "\tmultirect: mode: %d index: %d\n",
>>> - pstate->multirect_mode, pstate->multirect_index);
>>> + pstate->pipe.multirect_mode, pstate->pipe.multirect_index);
>>> seq_puts(s, "\n");
>>> }
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> index 674f311f99b4..0af2bc6e5df8 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>>> @@ -159,15 +159,11 @@ struct dpu_hw_pixel_ext {
>>> * @src_rect: src ROI, caller takes into account the different
>>> operations
>>> * such as decimation, flip etc to program this field
>>> * @dest_rect: destination ROI.
>>> - * @index: index of the rectangle of SSPP
>>> - * @mode: parallel or time multiplex multirect mode
>>> */
>>> struct dpu_hw_pipe_cfg {
>>> struct dpu_hw_fmt_layout layout;
>>> struct drm_rect src_rect;
>>> struct drm_rect dst_rect;
>>> - enum dpu_sspp_multirect_index index;
>>> - enum dpu_sspp_multirect_mode mode;
>>> };
>>> /**
>>> @@ -218,6 +214,18 @@ struct dpu_hw_pipe_ts_cfg {
>>> u64 time;
>>> };
>>> +/**
>>> + * struct dpu_sw_pipe - software pipe description
>>> + * @sspp: backing SSPP pipe
>>> + * @index: index of the rectangle of SSPP
>>> + * @mode: parallel or time multiplex multirect mode
>>> + */
>>> +struct dpu_sw_pipe {
>>> + struct dpu_hw_pipe *sspp;
>>> + enum dpu_sspp_multirect_index multirect_index;
>>> + enum dpu_sspp_multirect_mode multirect_mode;
>>> +};
>>> +
>>> /**
>>> * struct dpu_hw_sspp_ops - interface to the SSPP Hw driver functions
>>> * Caller must call the init function to get the pipe context for
>>> each pipe
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> index d1f9b4bc10ac..51b5e8a3182b 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> @@ -247,7 +247,7 @@ static int _dpu_plane_calc_fill_level(struct
>>> drm_plane *plane,
>>> ((src_width + 32) * fmt->bpp);
>>> }
>>> } else {
>>> - if (pstate->multirect_mode == DPU_SSPP_MULTIRECT_PARALLEL) {
>>> + if (pstate->pipe.multirect_mode ==
>>> DPU_SSPP_MULTIRECT_PARALLEL) {
>>> total_fl = (fixed_buff_size / 2) * 2 /
>>> ((src_width + 32) * fmt->bpp);
>>> } else {
>>> @@ -331,7 +331,7 @@ static void _dpu_plane_set_qos_lut(struct
>>> drm_plane *plane,
>>> fmt ? (char *)&fmt->base.pixel_format : NULL,
>>> pdpu->is_rt_pipe, total_fl, qos_lut);
>>> - pstate->pipe_hw->ops.setup_creq_lut(pstate->pipe_hw, qos_lut);
>>> + pstate->pipe.sspp->ops.setup_creq_lut(pstate->pipe.sspp, qos_lut);
>>> }
>>> /**
>>> @@ -383,7 +383,7 @@ static void _dpu_plane_set_danger_lut(struct
>>> drm_plane *plane,
>>> danger_lut,
>>> safe_lut);
>>> - pstate->pipe_hw->ops.setup_danger_safe_lut(pstate->pipe_hw,
>>> + pstate->pipe.sspp->ops.setup_danger_safe_lut(pstate->pipe.sspp,
>>> danger_lut, safe_lut);
>>> }
>>> @@ -403,9 +403,9 @@ static void _dpu_plane_set_qos_ctrl(struct
>>> drm_plane *plane,
>>> memset(&pipe_qos_cfg, 0, sizeof(pipe_qos_cfg));
>>> if (flags & DPU_PLANE_QOS_VBLANK_CTRL) {
>>> - pipe_qos_cfg.creq_vblank =
>>> pstate->pipe_hw->cap->sblk->creq_vblank;
>>> + pipe_qos_cfg.creq_vblank =
>>> pstate->pipe.sspp->cap->sblk->creq_vblank;
>>> pipe_qos_cfg.danger_vblank =
>>> - pstate->pipe_hw->cap->sblk->danger_vblank;
>>> + pstate->pipe.sspp->cap->sblk->danger_vblank;
>>> pipe_qos_cfg.vblank_en = enable;
>>> }
>>> @@ -431,7 +431,7 @@ static void _dpu_plane_set_qos_ctrl(struct
>>> drm_plane *plane,
>>> pipe_qos_cfg.danger_vblank,
>>> pdpu->is_rt_pipe);
>>> - pstate->pipe_hw->ops.setup_qos_ctrl(pstate->pipe_hw,
>>> + pstate->pipe.sspp->ops.setup_qos_ctrl(pstate->pipe.sspp,
>>> &pipe_qos_cfg);
>>> }
>>> @@ -450,14 +450,14 @@ static void _dpu_plane_set_ot_limit(struct
>>> drm_plane *plane,
>>> struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
>>> memset(&ot_params, 0, sizeof(ot_params));
>>> - ot_params.xin_id = pstate->pipe_hw->cap->xin_id;
>>> - ot_params.num = pstate->pipe_hw->idx - SSPP_NONE;
>>> + ot_params.xin_id = pstate->pipe.sspp->cap->xin_id;
>>> + ot_params.num = pstate->pipe.sspp->idx - SSPP_NONE;
>>> ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
>>> ot_params.height = drm_rect_height(&pipe_cfg->src_rect);
>>> ot_params.is_wfd = !pdpu->is_rt_pipe;
>>> ot_params.frame_rate = drm_mode_vrefresh(&crtc->mode);
>>> ot_params.vbif_idx = VBIF_RT;
>>> - ot_params.clk_ctrl = pstate->pipe_hw->cap->clk_ctrl;
>>> + ot_params.clk_ctrl = pstate->pipe.sspp->cap->clk_ctrl;
>>> ot_params.rd = true;
>>> dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
>>> @@ -476,9 +476,9 @@ static void _dpu_plane_set_qos_remap(struct
>>> drm_plane *plane)
>>> memset(&qos_params, 0, sizeof(qos_params));
>>> qos_params.vbif_idx = VBIF_RT;
>>> - qos_params.clk_ctrl = pstate->pipe_hw->cap->clk_ctrl;
>>> - qos_params.xin_id = pstate->pipe_hw->cap->xin_id;
>>> - qos_params.num = pstate->pipe_hw->idx - SSPP_VIG0;
>>> + qos_params.clk_ctrl = pstate->pipe.sspp->cap->clk_ctrl;
>>> + qos_params.xin_id = pstate->pipe.sspp->cap->xin_id;
>>> + qos_params.num = pstate->pipe.sspp->idx - SSPP_VIG0;
>>> qos_params.is_rt = pdpu->is_rt_pipe;
>>> DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d,
>>> clk_ctrl:%d\n",
>>> @@ -505,12 +505,12 @@ static void _dpu_plane_set_scanout(struct
>>> drm_plane *plane,
>>> DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
>>> else if (ret)
>>> DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n",
>>> ret);
>>> - else if (pstate->pipe_hw->ops.setup_sourceaddress) {
>>> - trace_dpu_plane_set_scanout(pstate->pipe_hw->idx,
>>> + else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
>>> + trace_dpu_plane_set_scanout(pstate->pipe.sspp->idx,
>>> &pipe_cfg->layout,
>>> - pstate->multirect_index);
>>> - pstate->pipe_hw->ops.setup_sourceaddress(pstate->pipe_hw,
>>> pipe_cfg,
>>> - pstate->multirect_index);
>>> + pstate->pipe.multirect_index);
>>> +
>>> pstate->pipe.sspp->ops.setup_sourceaddress(pstate->pipe.sspp, pipe_cfg,
>>> + pstate->pipe.multirect_index);
>>> }
>>> }
>>> @@ -553,7 +553,7 @@ static void _dpu_plane_setup_scaler3(struct
>>> dpu_plane *pdpu,
>>> scale_cfg->src_height[i] /= chroma_subsmpl_v;
>>> }
>>> - if (pstate->pipe_hw->cap->features &
>>> + if (pstate->pipe.sspp->cap->features &
>>> BIT(DPU_SSPP_SCALER_QSEED4)) {
>>> scale_cfg->preload_x[i] = DPU_QSEED4_DEFAULT_PRELOAD_H;
>>> scale_cfg->preload_y[i] = DPU_QSEED4_DEFAULT_PRELOAD_V;
>>> @@ -624,7 +624,7 @@ static const struct dpu_csc_cfg
>>> *_dpu_plane_get_csc(struct dpu_plane *pdpu, cons
>>> if (!DPU_FORMAT_IS_YUV(fmt))
>>> return NULL;
>>> - if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe_hw->cap->features)
>>> + if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe.sspp->cap->features)
>>> csc_ptr = &dpu_csc10_YUV2RGB_601L;
>>> else
>>> csc_ptr = &dpu_csc_YUV2RGB_601L;
>>> @@ -659,8 +659,8 @@ static void _dpu_plane_setup_scaler(struct
>>> dpu_plane *pdpu,
>>> &scaler3_cfg, &pixel_ext, fmt,
>>> info->hsub, info->vsub);
>>> - if (pstate->pipe_hw->ops.setup_pe)
>>> - pstate->pipe_hw->ops.setup_pe(pstate->pipe_hw,
>>> + if (pstate->pipe.sspp->ops.setup_pe)
>>> + pstate->pipe.sspp->ops.setup_pe(pstate->pipe.sspp,
>>> &pixel_ext);
>>> /**
>>> @@ -668,9 +668,9 @@ static void _dpu_plane_setup_scaler(struct
>>> dpu_plane *pdpu,
>>> * bypassed. Still we need to update alpha and bitwidth
>>> * ONLY for RECT0
>>> */
>>> - if (pstate->pipe_hw->ops.setup_scaler &&
>>> - pstate->multirect_index != DPU_SSPP_RECT_1)
>>> - pstate->pipe_hw->ops.setup_scaler(pstate->pipe_hw,
>>> + if (pstate->pipe.sspp->ops.setup_scaler &&
>>> + pstate->pipe.multirect_index != DPU_SSPP_RECT_1)
>>> + pstate->pipe.sspp->ops.setup_scaler(pstate->pipe.sspp,
>>> pipe_cfg,
>>> &scaler3_cfg);
>>> }
>>> @@ -699,10 +699,10 @@ static int _dpu_plane_color_fill(struct
>>> dpu_plane *pdpu,
>>> fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888);
>>> /* update sspp */
>>> - if (fmt && pstate->pipe_hw->ops.setup_solidfill) {
>>> - pstate->pipe_hw->ops.setup_solidfill(pstate->pipe_hw,
>>> + if (fmt && pstate->pipe.sspp->ops.setup_solidfill) {
>>> + pstate->pipe.sspp->ops.setup_solidfill(pstate->pipe.sspp,
>>> (color & 0xFFFFFF) | ((alpha & 0xFF) << 24),
>>> - pstate->multirect_index);
>>> + pstate->pipe.multirect_index);
>>> /* override scaler/decimation if solid fill */
>>> pipe_cfg.dst_rect = pstate->base.dst;
>>> @@ -714,15 +714,15 @@ static int _dpu_plane_color_fill(struct
>>> dpu_plane *pdpu,
>>> pipe_cfg.src_rect.y2 =
>>> drm_rect_height(&pipe_cfg.dst_rect);
>>> - if (pstate->pipe_hw->ops.setup_format)
>>> - pstate->pipe_hw->ops.setup_format(pstate->pipe_hw,
>>> + if (pstate->pipe.sspp->ops.setup_format)
>>> + pstate->pipe.sspp->ops.setup_format(pstate->pipe.sspp,
>>> fmt, DPU_SSPP_SOLID_FILL,
>>> - pstate->multirect_index);
>>> + pstate->pipe.multirect_index);
>>> - if (pstate->pipe_hw->ops.setup_rects)
>>> - pstate->pipe_hw->ops.setup_rects(pstate->pipe_hw,
>>> + if (pstate->pipe.sspp->ops.setup_rects)
>>> + pstate->pipe.sspp->ops.setup_rects(pstate->pipe.sspp,
>>> &pipe_cfg,
>>> - pstate->multirect_index);
>>> + pstate->pipe.multirect_index);
>>> _dpu_plane_setup_scaler(pdpu, pstate, fmt, true, &pipe_cfg);
>>> }
>>> @@ -734,8 +734,8 @@ void dpu_plane_clear_multirect(const struct
>>> drm_plane_state *drm_state)
>>> {
>>> struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
>>> - pstate->multirect_index = DPU_SSPP_RECT_SOLO;
>>> - pstate->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>> + pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
>>> + pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>>> }
>>> int dpu_plane_validate_multirect_v2(struct
>>> dpu_multirect_plane_states *plane)
>>> @@ -817,8 +817,8 @@ int dpu_plane_validate_multirect_v2(struct
>>> dpu_multirect_plane_states *plane)
>>> /* Prefer PARALLEL FETCH Mode over TIME_MX Mode */
>>> if (parallel_fetch_qualified) {
>>> - pstate[R0]->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>>> - pstate[R1]->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>>> + pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>>> + pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>>> goto done;
>>> }
>>> @@ -828,8 +828,8 @@ int dpu_plane_validate_multirect_v2(struct
>>> dpu_multirect_plane_states *plane)
>>> if (dst[R1].y1 >= dst[R0].y2 + buffer_lines ||
>>> dst[R0].y1 >= dst[R1].y2 + buffer_lines) {
>>> - pstate[R0]->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
>>> - pstate[R1]->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
>>> + pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
>>> + pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
>>> } else {
>>> DPU_ERROR(
>>> "No multirect mode possible for the planes (%d - %d)\n",
>>> @@ -839,13 +839,13 @@ int dpu_plane_validate_multirect_v2(struct
>>> dpu_multirect_plane_states *plane)
>>> }
>>> done:
>>> - pstate[R0]->multirect_index = DPU_SSPP_RECT_0;
>>> - pstate[R1]->multirect_index = DPU_SSPP_RECT_1;
>>> + pstate[R0]->pipe.multirect_index = DPU_SSPP_RECT_0;
>>> + pstate[R1]->pipe.multirect_index = DPU_SSPP_RECT_1;
>>> DPU_DEBUG_PLANE(dpu_plane[R0], "R0: %d - %d\n",
>>> - pstate[R0]->multirect_mode, pstate[R0]->multirect_index);
>>> + pstate[R0]->pipe.multirect_mode,
>>> pstate[R0]->pipe.multirect_index);
>>> DPU_DEBUG_PLANE(dpu_plane[R1], "R1: %d - %d\n",
>>> - pstate[R1]->multirect_mode, pstate[R1]->multirect_index);
>>> + pstate[R1]->pipe.multirect_mode,
>>> pstate[R1]->pipe.multirect_index);
>>> return 0;
>>> }
>>> @@ -937,6 +937,7 @@ static int dpu_plane_atomic_check(struct
>>> drm_plane *plane,
>>> int ret = 0, min_scale;
>>> struct dpu_plane *pdpu = to_dpu_plane(plane);
>>> struct dpu_plane_state *pstate =
>>> to_dpu_plane_state(new_plane_state);
>>> + struct dpu_sw_pipe *pipe = &pstate->pipe;
>>> const struct drm_crtc_state *crtc_state = NULL;
>>> const struct dpu_format *fmt;
>>> struct drm_rect src, dst, fb_rect = { 0 };
>>> @@ -946,10 +947,10 @@ static int dpu_plane_atomic_check(struct
>>> drm_plane *plane,
>>> crtc_state = drm_atomic_get_new_crtc_state(state,
>>> new_plane_state->crtc);
>>> - min_scale = FRAC_16_16(1, pstate->pipe_hw->cap->sblk->maxupscale);
>>> + min_scale = FRAC_16_16(1, pipe->sspp->cap->sblk->maxupscale);
>>> ret = drm_atomic_helper_check_plane_state(new_plane_state,
>>> crtc_state,
>>> min_scale,
>>> - pstate->pipe_hw->cap->sblk->maxdwnscale <<
>>> 16,
>>> + pipe->sspp->cap->sblk->maxdwnscale << 16,
>>> true, true);
>>> if (ret) {
>>> DPU_DEBUG_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
>>> @@ -975,8 +976,8 @@ static int dpu_plane_atomic_check(struct
>>> drm_plane *plane,
>>> min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
>>> if (DPU_FORMAT_IS_YUV(fmt) &&
>>> - (!(pstate->pipe_hw->cap->features & DPU_SSPP_SCALER) ||
>>> - !(pstate->pipe_hw->cap->features & DPU_SSPP_CSC_ANY))) {
>>> + (!(pipe->sspp->cap->features & DPU_SSPP_SCALER) ||
>>> + !(pipe->sspp->cap->features & DPU_SSPP_CSC_ANY))) {
>>> DPU_DEBUG_PLANE(pdpu,
>>> "plane doesn't have scaler/csc for yuv\n");
>>> return -EINVAL;
>>> @@ -1037,12 +1038,12 @@ void dpu_plane_flush(struct drm_plane *plane)
>>> else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
>>> /* force 100% alpha */
>>> _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
>>> - else if (pstate->pipe_hw && pstate->pipe_hw->ops.setup_csc) {
>>> + else if (pstate->pipe.sspp && pstate->pipe.sspp->ops.setup_csc) {
>>> const struct dpu_format *fmt =
>>> to_dpu_format(msm_framebuffer_format(plane->state->fb));
>>> const struct dpu_csc_cfg *csc_ptr =
>>> _dpu_plane_get_csc(pdpu, fmt);
>>> if (csc_ptr)
>>> - pstate->pipe_hw->ops.setup_csc(pstate->pipe_hw, csc_ptr);
>>> + pstate->pipe.sspp->ops.setup_csc(pstate->pipe.sspp,
>>> csc_ptr);
>>> }
>>> /* flag h/w flush complete */
>>> @@ -1072,6 +1073,7 @@ static void dpu_plane_sspp_atomic_update(struct
>>> drm_plane *plane)
>>> struct dpu_plane *pdpu = to_dpu_plane(plane);
>>> struct drm_plane_state *state = plane->state;
>>> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>> + struct dpu_sw_pipe *pipe = &pstate->pipe;
>>> struct drm_crtc *crtc = state->crtc;
>>> struct drm_framebuffer *fb = state->fb;
>>> bool is_rt_pipe, update_qos_remap;
>>> @@ -1109,21 +1111,21 @@ static void
>>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>> return;
>>> }
>>> - if (pstate->pipe_hw->ops.setup_rects) {
>>> - pstate->pipe_hw->ops.setup_rects(pstate->pipe_hw,
>>> + if (pipe->sspp->ops.setup_rects) {
>>> + pipe->sspp->ops.setup_rects(pipe->sspp,
>>> &pipe_cfg,
>>> - pstate->multirect_index);
>>> + pipe->multirect_index);
>>> }
>>> _dpu_plane_setup_scaler(pdpu, pstate, fmt, false, &pipe_cfg);
>>> - if (pstate->pipe_hw->ops.setup_multirect)
>>> - pstate->pipe_hw->ops.setup_multirect(
>>> - pstate->pipe_hw,
>>> - pstate->multirect_index,
>>> - pstate->multirect_mode);
>>> + if (pipe->sspp->ops.setup_multirect)
>>> + pipe->sspp->ops.setup_multirect(
>>> + pipe->sspp,
>>> + pipe->multirect_index,
>>> + pipe->multirect_mode);
>>> - if (pstate->pipe_hw->ops.setup_format) {
>>> + if (pipe->sspp->ops.setup_format) {
>>> unsigned int rotation;
>>> src_flags = 0x0;
>>> @@ -1140,10 +1142,10 @@ static void
>>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>> src_flags |= DPU_SSPP_FLIP_UD;
>>> /* update format */
>>> - pstate->pipe_hw->ops.setup_format(pstate->pipe_hw, fmt,
>>> src_flags,
>>> - pstate->multirect_index);
>>> + pipe->sspp->ops.setup_format(pipe->sspp, fmt, src_flags,
>>> + pipe->multirect_index);
>>> - if (pstate->pipe_hw->ops.setup_cdp) {
>>> + if (pipe->sspp->ops.setup_cdp) {
>>> struct dpu_hw_pipe_cdp_cfg cdp_cfg;
>>> memset(&cdp_cfg, 0, sizeof(struct dpu_hw_pipe_cdp_cfg));
>>> @@ -1157,7 +1159,7 @@ static void dpu_plane_sspp_atomic_update(struct
>>> drm_plane *plane)
>>> DPU_FORMAT_IS_TILE(fmt);
>>> cdp_cfg.preload_ahead = DPU_SSPP_CDP_PRELOAD_AHEAD_64;
>>> - pstate->pipe_hw->ops.setup_cdp(pstate->pipe_hw,
>>> &cdp_cfg, pstate->multirect_index);
>>> + pipe->sspp->ops.setup_cdp(pipe->sspp, &cdp_cfg,
>>> pipe->multirect_index);
>>> }
>>> }
>>> @@ -1191,7 +1193,7 @@ static void _dpu_plane_atomic_disable(struct
>>> drm_plane *plane)
>>> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>> trace_dpu_plane_disable(DRMID(plane), false,
>>> - pstate->multirect_mode);
>>> + pstate->pipe.multirect_mode);
>>> pstate->pending = true;
>>> }
>>> @@ -1305,9 +1307,9 @@ static void dpu_plane_atomic_print_state(struct
>>> drm_printer *p,
>>> const struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>>> drm_printf(p, "\tstage=%d\n", pstate->stage);
>>> - drm_printf(p, "\tsspp=%s\n", pstate->pipe_hw->cap->name);
>>> - drm_printf(p, "\tmultirect_mode=%s\n",
>>> dpu_get_multirect_mode(pstate->multirect_mode));
>>> - drm_printf(p, "\tmultirect_index=%s\n",
>>> dpu_get_multirect_index(pstate->multirect_index));
>>> + drm_printf(p, "\tsspp=%s\n", pstate->pipe.sspp->cap->name);
>>> + drm_printf(p, "\tmultirect_mode=%s\n",
>>> dpu_get_multirect_mode(pstate->pipe.multirect_mode));
>>> + drm_printf(p, "\tmultirect_index=%s\n",
>>> dpu_get_multirect_index(pstate->pipe.multirect_index));
>>> }
>>> static void dpu_plane_reset(struct drm_plane *plane)
>>> @@ -1336,7 +1338,7 @@ static void dpu_plane_reset(struct drm_plane
>>> *plane)
>>> return;
>>> }
>>> - pstate->pipe_hw = to_dpu_hw_pipe(kms->rm.sspp_blks[pdpu->pipe -
>>> SSPP_NONE]);
>>> + pstate->pipe.sspp = to_dpu_hw_pipe(kms->rm.sspp_blks[pdpu->pipe
>>> - SSPP_NONE]);
>>> __drm_atomic_helper_plane_reset(plane, &pstate->base);
>>> }
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>>> index d745cde4ea77..b6fd6f856d6a 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>>> @@ -31,11 +31,9 @@ struct dpu_plane_state {
>>> struct msm_gem_address_space *aspace;
>>> enum dpu_stage stage;
>>> bool needs_qos_remap;
>>> - uint32_t multirect_index;
>>> - uint32_t multirect_mode;
>>> bool pending;
>>> - struct dpu_hw_pipe *pipe_hw;
>>> + struct dpu_sw_pipe pipe;
>>
>> Are you sure we dont need this to be a pointer?
>>
>> Since this is now part of plane_state, its possible that across
>> commits, the hw pipe can change for this plane's state. So might be
>> better if this is pointer in that case ....
>
> A pointer means that it is allocated and deallocated by somebody.
> Probably you meant that it is a pointer into the dpu_hw_pipe member
> (with dpu_hw_pipe having sw_pipe's for rect0 and rect1).
>
> I thought about this. And ended up with the dpu_sw_pipe having a
> dpu_hw_pipe pointer inside, but itself being a member of plane state.
> This way the plane state contains such data as multirect mode and
> rectangle.
Right, so what I meant was, previously pipe_hw was a pointer but it was
not changed dynamically because it was assigned once in dpu_plane_init()
but after that was not updated to point to a different pipe_hw.
Now, pipe_hw is part of the plane state which means the actual hardware
which the plane state is using can change across commits with virtual
plane support.
So it made more sense to me that this was "struct dpu_sw_pipe *pipe"
because looking at it from a top level it tells me that the dpu_sw_pipe
can point to a different pipe. Just little more intuitive.
With it being just a member, sure it still means the same thing because
internal to it, pipe_hw is still a pointer.
Just felt it was more logical to have the whole dpu_sw_pipe as a pointer.
But its just a thought not a strong inclination against current patch.
>
>
>>
>>> u64 plane_fetch_bw;
>>> u64 plane_clk;
>
>
More information about the Freedreno
mailing list