On Fri, 8 Sept 2023 at 21:56, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
>
> Currently, dpu_plane_atomic_check() does not check whether the
> plane can process the image without exceeding the per chipset
> limits for MDP clock. This leads to underflow issues because the
> SSPP is not able to complete the processing for the data rate of
> the display.
>
> Fail the dpu_plane_atomic_check() if the SSPP cannot process the
> image without exceeding the MDP clock limits.
>
> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 98c1b22e9bca..62dd9f9b4dce 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -733,9 +733,11 @@ static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
> static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
> struct dpu_sw_pipe *pipe,
> struct dpu_sw_pipe_cfg *pipe_cfg,
> - const struct dpu_format *fmt)
> + const struct dpu_format *fmt,
> + const struct drm_display_mode *mode)
> {
> uint32_t min_src_size;
> + struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>
> min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
>
> @@ -774,6 +776,12 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
> return -EINVAL;
> }
>
> + /* max clk check */
> + if (_dpu_plane_calc_clk(mode, pipe_cfg) > kms->perf.max_core_clk_rate) {
> + DPU_DEBUG_PLANE(pdpu, "plane exceeds max mdp core clk limits\n");
> + return -E2BIG;
> + }
> +
> return 0;
> }
>
> @@ -899,12 +907,13 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
> }
>
> - ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
> + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt, &crtc_state->mode);
> if (ret)
> return ret;
>
> if (r_pipe->sspp) {
> - ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt);
> + ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt,
> + &crtc_state->mode);
I think this should be adjusted_mode. In the end, according to the
docs CRTC should be programmed with the adjusted_mode, while the
state->mode is the mode at the end of the pipeline (if I got it
correct).
So e.g. if we add DS to the picture, state->mode will be screen
resolution, while adjusted_moe will be pre-scale resolution, which is
the one that matters from the bandwidth point of view.
--
With best wishes
Dmitry
