[PATCH 1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on mdp clk limits

[PATCH 1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on mdp clk limits

[Abhinav Kumar]

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);
 		if (ret)
 			return ret;
 	}
-- 
2.40.1

[PATCH 2/2] drm/msm/dpu: try multirect based on mdp clock limits

[Abhinav Kumar]

It's certainly possible that for large resolutions a single DPU SSPP
cannot process the image without exceeding the MDP clock limits but
it can still process it in multirect mode because the source rectangles
will get divided and can fall within the MDP clock limits.

If the SSPP cannot process the image even in multirect mode, then it
will be rejected in dpu_plane_atomic_check_pipe().

Hence try using multirect for resolutions which cannot be processed
by a single SSPP without exceeding the MDP clock limits.

Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 62dd9f9b4dce..85072328cd53 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -792,6 +792,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 										 plane);
 	int ret = 0, min_scale;
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
+	struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
 	struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
 	struct dpu_sw_pipe *pipe = &pstate->pipe;
 	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
@@ -860,7 +861,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 
 	max_linewidth = pdpu->catalog->caps->max_linewidth;
 
-	if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
+	if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) ||
+	     _dpu_plane_calc_clk(&crtc_state->mode, pipe_cfg) > kms->perf.max_core_clk_rate) {
 		/*
 		 * In parallel multirect case only the half of the usual width
 		 * is supported for tiled formats. If we are here, we know that
-- 
2.40.1

Re: [PATCH 1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on mdp clk limits

[Dmitry Baryshkov]

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.


-- 
With best wishes
Dmitry

Re: [PATCH 2/2] drm/msm/dpu: try multirect based on mdp clock limits

[Dmitry Baryshkov]

On Fri, 8 Sept 2023 at 21:55, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
> It's certainly possible that for large resolutions a single DPU SSPP
> cannot process the image without exceeding the MDP clock limits but
> it can still process it in multirect mode because the source rectangles
> will get divided and can fall within the MDP clock limits.
>
> If the SSPP cannot process the image even in multirect mode, then it
> will be rejected in dpu_plane_atomic_check_pipe().
>
> Hence try using multirect for resolutions which cannot be processed
> by a single SSPP without exceeding the MDP clock limits.
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 62dd9f9b4dce..85072328cd53 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -792,6 +792,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>                                                                                  plane);
>         int ret = 0, min_scale;
>         struct dpu_plane *pdpu = to_dpu_plane(plane);
> +       struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>         struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
>         struct dpu_sw_pipe *pipe = &pstate->pipe;
>         struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> @@ -860,7 +861,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>
>         max_linewidth = pdpu->catalog->caps->max_linewidth;
>
> -       if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
> +       if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) ||
> +            _dpu_plane_calc_clk(&crtc_state->mode, pipe_cfg) > kms->perf.max_core_clk_rate) {

First, I think this should be an adjusted_mode too. And this probably
needs some more attention in the next few lines of code, since .e.g
the UBWC case also needs to be adjusted.

>                 /*
>                  * In parallel multirect case only the half of the usual width
>                  * is supported for tiled formats. If we are here, we know that
> --
> 2.40.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on mdp clk limits

[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

Re: [PATCH 2/2] drm/msm/dpu: try multirect based on mdp clock limits

[Abhinav Kumar]

On 9/8/2023 4:30 PM, Dmitry Baryshkov wrote:
> On Fri, 8 Sept 2023 at 21:55, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>>
>> It's certainly possible that for large resolutions a single DPU SSPP
>> cannot process the image without exceeding the MDP clock limits but
>> it can still process it in multirect mode because the source rectangles
>> will get divided and can fall within the MDP clock limits.
>>
>> If the SSPP cannot process the image even in multirect mode, then it
>> will be rejected in dpu_plane_atomic_check_pipe().
>>
>> Hence try using multirect for resolutions which cannot be processed
>> by a single SSPP without exceeding the MDP clock limits.
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index 62dd9f9b4dce..85072328cd53 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -792,6 +792,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>>                                                                                   plane);
>>          int ret = 0, min_scale;
>>          struct dpu_plane *pdpu = to_dpu_plane(plane);
>> +       struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>>          struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
>>          struct dpu_sw_pipe *pipe = &pstate->pipe;
>>          struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
>> @@ -860,7 +861,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>>
>>          max_linewidth = pdpu->catalog->caps->max_linewidth;
>>
>> -       if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
>> +       if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) ||
>> +            _dpu_plane_calc_clk(&crtc_state->mode, pipe_cfg) > kms->perf.max_core_clk_rate) {
> 
> First, I think this should be an adjusted_mode too. And this probably
> needs some more attention in the next few lines of code, since .e.g
> the UBWC case also needs to be adjusted.
> 

Ack, will change this to adjusted_mode as well

Yes, need to update UBWC check like below, thanks for catching it.

@@ -869,7 +878,7 @@ static int dpu_plane_atomic_check(struct drm_plane 
*plane,
                  * full width is more than max_linewidth, thus each rect is
                  * wider than allowed.
                  */
-               if (DPU_FORMAT_IS_UBWC(fmt)) {
+               if (DPU_FORMAT_IS_UBWC(fmt) && 
drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
                         DPU_DEBUG_PLANE(pdpu, "invalid src " 
DRM_RECT_FMT " line:%u, tiled format\n",
 
DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
                         return -E2BIG;

>>                  /*
>>                   * In parallel multirect case only the half of the usual width
>>                   * is supported for tiled formats. If we are here, we know that
>> --
>> 2.40.1
>>
> 
>