Messages in this thread | | | Date | Mon, 11 Sep 2023 09:49:03 -0700 | Subject | Re: [PATCH 1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on mdp clk limits | From | Abhinav Kumar <> |
| |
On 9/8/2023 4:06 PM, Dmitry Baryshkov wrote: > On Fri, 8 Sept 2023 at 21:56, Abhinav Kumar <quic_abhinavk@quicinc.com> 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@quicinc.com> >> --- >> 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. > >
Ack, I should change this to adjusted_mode although today this would be the same behavior as dpu_crtc doesn't have a mode_fixup.
so mode = adjusted_mode
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/drm_atomic_helper.c#L425
| |