[PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Fri Mar 3 12:18:26 UTC 2023
On 09/02/2023 01:44, Abhinav Kumar wrote:
>
>
> On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
>> Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
>> separate functon. This is a preparation for the r_pipe support.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 86 ++++++++++++++++-------
>> drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
>> 2 files changed, 63 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index 73e1a8c69ef0..0ca3bc38ff7e 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -400,6 +400,47 @@ static void
>> _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
>> }
>> }
>> +static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
>> + struct drm_plane *plane,
>> + struct dpu_crtc_mixer *mixer,
>> + u32 num_mixers,
>> + struct dpu_hw_stage_cfg *stage_cfg,
>> + enum dpu_stage stage,
>> + unsigned int stage_idx,
>> + unsigned long *fetch_active,
>> + struct dpu_sw_pipe *pipe
>> + )
>> +{
>> + uint32_t lm_idx;
>> + enum dpu_sspp sspp_idx;
>> + struct drm_plane_state *state;
>> +
>> + if (!pipe->sspp)
>> + return;
>> +
>> + sspp_idx = pipe->sspp->idx;
>> +
>> + state = plane->state;
>> +
>> + DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
>> + crtc->base.id,
>> + stage,
>> + plane->base.id,
>> + sspp_idx - SSPP_NONE,
>> + state->fb ? state->fb->base.id : -1);
>> +
>> + set_bit(sspp_idx, fetch_active);
>> +
>> + stage_cfg->stage[stage][stage_idx] = sspp_idx;
>> + stage_cfg->multirect_index[stage][stage_idx] =
>> + pipe->multirect_index;
>> +
>> + /* blend config update */
>> + for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
>> +
>> mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,
>> + sspp_idx);
>
> If you just pass the format to this function you can move rest of the
> for loop also to this function.
As a second thought, this would defeat the purpose of the split. We
don't have to call _dpu_crtc_setup_blend_cfg() or setup mixer_op_mode
for the second pipe separately. So, I'd leave the loops as is.
>
> Also, you will be able to add the trace_dpu_crtc_setup_mixer() with
> complete information.
>
> trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is
> important to debug blend issues.
>
>> +}
>> +
>> static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>> struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
>> struct dpu_hw_stage_cfg *stage_cfg)
>> @@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct
>> drm_crtc *crtc,
>> struct dpu_format *format;
>> struct dpu_hw_ctl *ctl = mixer->lm_ctl;
>> - uint32_t stage_idx, lm_idx;
>> - int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
>> + uint32_t lm_idx;
>> bool bg_alpha_enable = false;
>> DECLARE_BITMAP(fetch_active, SSPP_MAX);
>> memset(fetch_active, 0, sizeof(fetch_active));
>> drm_atomic_crtc_for_each_plane(plane, crtc) {
>> - enum dpu_sspp sspp_idx;
>> -
>> state = plane->state;
>> if (!state)
>> continue;
>> @@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct
>> drm_crtc *crtc,
>> pstate = to_dpu_plane_state(state);
>> fb = state->fb;
>> - sspp_idx = pstate->pipe.sspp->idx;
>> - set_bit(sspp_idx, fetch_active);
>> -
>> - DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
>> - crtc->base.id,
>> - pstate->stage,
>> - plane->base.id,
>> - sspp_idx - SSPP_VIG0,
>> - state->fb ? state->fb->base.id : -1);
>> -
>> format =
>> to_dpu_format(msm_framebuffer_format(pstate->base.fb));
>> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
>> bg_alpha_enable = true;
>> - stage_idx = zpos_cnt[pstate->stage]++;
>> - stage_cfg->stage[pstate->stage][stage_idx] =
>> - sspp_idx;
>> - stage_cfg->multirect_index[pstate->stage][stage_idx] =
>> - pstate->pipe.multirect_index;
>> -
>> trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
>> - state, pstate, stage_idx,
>> + state, pstate,
>> format->base.pixel_format,
>> fb ? fb->modifier : 0);
>> + _dpu_crtc_blend_setup_pipe(crtc, plane,
>> + mixer, cstate->num_mixers,
>> + stage_cfg, pstate->stage, 0,
>> + fetch_active,
>> + &pstate->pipe);
>> +
>> /* blend config update */
>> for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
>> - _dpu_crtc_setup_blend_cfg(mixer + lm_idx,
>> - pstate, format);
>> -
>> -
>> mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,
>> - sspp_idx);
>> + _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format);
>> if (bg_alpha_enable && !format->alpha_enable)
>> mixer[lm_idx].mixer_op_mode = 0;
>> @@ -1297,8 +1321,16 @@ static int _dpu_debugfs_status_show(struct
>> seq_file *s, void *data)
>> seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
>> state->crtc_x, state->crtc_y, state->crtc_w,
>> state->crtc_h);
>> - seq_printf(s, "\tmultirect: mode: %d index: %d\n",
>> + seq_printf(s, "\tsspp[0]:%d\n",
>> + pstate->pipe.sspp->idx - SSPP_NONE);
>> + 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]:%d\n",
>> + pstate->r_pipe.sspp->idx - SSPP_NONE);
>> + seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
>> + pstate->r_pipe.multirect_mode,
>> pstate->r_pipe.multirect_index);
>> + }
>> seq_puts(s, "\n");
>> }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
>> index 0ad148cc2fb8..5ec4f89e8814 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
>> @@ -633,9 +633,9 @@ TRACE_EVENT(dpu_enc_phys_vid_irq_ctrl,
>> 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,
>> + uint32_t pixel_format,
>> uint64_t modifier),
>> - TP_ARGS(crtc_id, plane_id, state, pstate, stage_idx,
>> + TP_ARGS(crtc_id, plane_id, state, pstate,
>> pixel_format, modifier),
>> TP_STRUCT__entry(
>> __field( uint32_t, crtc_id )
>> @@ -643,7 +643,6 @@ TRACE_EVENT(dpu_crtc_setup_mixer,
>> __field( uint32_t, fb_id )
>> __field_struct( struct drm_rect, src_rect )
>> __field_struct( struct drm_rect, dst_rect )
>> - __field( uint32_t, stage_idx )
>> __field( enum dpu_stage, stage )
>> __field( enum dpu_sspp, sspp )
>> __field( uint32_t, multirect_idx )
>> @@ -657,7 +656,6 @@ TRACE_EVENT(dpu_crtc_setup_mixer,
>> __entry->fb_id = state ? state->fb->base.id : 0;
>> __entry->src_rect = drm_plane_state_src(state);
>> __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;
>> @@ -666,13 +664,13 @@ TRACE_EVENT(dpu_crtc_setup_mixer,
>> __entry->modifier = modifier;
>> ),
>> TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:" DRM_RECT_FP_FMT
>> - " dst:" DRM_RECT_FMT " stage_idx:%u stage:%d, sspp:%d "
>> + " dst:" DRM_RECT_FMT " stage:%d, sspp:%d "
>> "multirect_index:%d multirect_mode:%u pix_format:%u "
>> "modifier:%llu",
>> __entry->crtc_id, __entry->plane_id, __entry->fb_id,
>> DRM_RECT_FP_ARG(&__entry->src_rect),
>> DRM_RECT_ARG(&__entry->dst_rect),
>> - __entry->stage_idx, __entry->stage, __entry->sspp,
>> + __entry->stage, __entry->sspp,
>> __entry->multirect_idx, __entry->multirect_mode,
>> __entry->pixel_format, __entry->modifier)
>> );
--
With best wishes
Dmitry
More information about the dri-devel
mailing list