| Message ID | 20230908185427.29026-1-quic_abhinavk@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on mdp clk limits | expand |
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.
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
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); if (ret) return ret; }
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(-)