linux-kernel - Re: [PATCH 1/2] drm/msm/dpu: fail dpu_plane_atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 11 Sep 2023 09:49:03 -0700
From:   Abhinav Kumar <quic_abhinavk@...cinc.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC:     <freedreno@...ts.freedesktop.org>, Rob Clark <robdclark@...il.com>,
        "Sean Paul" <sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        "David Airlie" <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Jordan Crouse <jordan@...micpenguin.net>,
        Jeykumar Sankaran <jsanka@...eaurora.org>,
        Chandan Uddaraju <chandanu@...eaurora.org>,
        Sravanthi Kollukuduru <skolluku@...eaurora.org>,
        <dri-devel@...ts.freedesktop.org>, <quic_jesszhan@...cinc.com>,
        <quic_parellan@...cinc.com>, <nespera@...lia.com>,
        Rajesh Yadav <ryadav@...eaurora.org>,
        Archit Taneja <architt@...eaurora.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] drm/msm/dpu: fail dpu_plane_atomic_check() based on
 mdp clk limits



On 9/8/2023 4:06 PM, Dmitry Baryshkov wrote:
> On Fri, 8 Sept 2023 at 21:56, Abhinav Kumar <quic_abhinavk@...cinc.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@...cinc.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