linux-kernel - Re: [PATCH] drm/msm/dpu: filter out too wide modes if no 3dmux is present

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ab8a9d0e-e786-47ea-833d-59d2f2988898@quicinc.com>
Date: Mon, 9 Dec 2024 14:30:09 -0800
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        Marijn
 Suijten <marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/msm/dpu: filter out too wide modes if no 3dmux is
 present

Hi Dmitry

On 12/9/2024 2:16 PM, Dmitry Baryshkov wrote:
> Hi Abhinav,
> 
> On Mon, 9 Dec 2024 at 22:07, Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>>
>>
>>
>> On 12/7/2024 9:29 PM, Dmitry Baryshkov wrote:
>>> On Fri, Dec 06, 2024 at 12:00:53PM -0800, Abhinav Kumar wrote:
>>>> On chipsets such as QCS615, there is no 3dmux present. In such
>>>> a case, a layer exceeding the max_mixer_width cannot be split,
>>>> hence cannot be supported.
>>>>
>>>> Filter out the modes which exceed the max_mixer_width when there
>>>> is no 3dmux present. Also, add a check in the dpu_crtc_atomic_check()
>>>> to return failure for such modes.
>>>>
>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>>>> ---
>>>> Note: this was only compile tested, so its pending validation on QCS615
>>>> ---
>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +++++++++++++
>>>>    1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>> index 9f6ffd344693ecfb633095772a31ada5613345dc..e6e5540aae83be7c20d8ae29115b8fdd42056e55 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>> @@ -732,6 +732,13 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc,
>>>>       struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
>>>>       int i;
>>>>
>>>> +    /* if we cannot merge 2 LMs (no 3d mux) better to fail earlier
>>>> +     * before even checking the width after the split
>>>> +     */
>>>> +    if (!dpu_kms->catalog->caps->has_3d_merge
>>>> +        && adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
>>>> +            return -E2BIG;
>>>
>>> Is it the same as checking that there are LMs which support
>>> DPU_MIXER_SOURCESPLIT ?
>>>
>>
>> DPU_MIXER_SOURCESPLIT tells whether we can have more than one SSPP in
>> the same blend stage.
> 
> Do we have a feature bit that corresponds to the ability to use 2 LMs?
> I mean, there are other *split topologies, not necessarily the 3DMux
> ones. E.g. PPSPLIT.
> 

A layer can always be split across LMs. There is not really any feature 
bit for this as it can always be done in pretty much all DPU chipsets.

Here the issue is we are not able to merge because there are no 3d mux 
blocks and hence we cannot split.

We need to merge because, the same display is requiring multiple LMs.

PP split will be a single LM going to two PPs and that going to two INTFs.

Hence the way to look at this patch would be we are avoiding split 
because we cannot merge and not that we cannot split.

>>
>> 494     if (test_bit(DPU_MIXER_SOURCESPLIT,
>> 495             &ctx->mixer_hw_caps->features))
>> 496             pipes_per_stage = PIPES_PER_STAGE;
>> 497     else
>> 498             pipes_per_stage = 1;
>>
>> That is different from this one. Here we are checking if we can actually
>> blend two LM outputs using the 3dmux (so its post blend).
>>
>>>> +
>>>>       for (i = 0; i < cstate->num_mixers; i++) {
>>>>               struct drm_rect *r = &cstate->lm_bounds[i];
>>>>               r->x1 = crtc_split_width * i;
>>>> @@ -1251,6 +1258,12 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
>>>>    {
>>>>       struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
>>>>
>>>> +    /* if there is no 3d_mux block we cannot merge LMs so we cannot
>>>> +     * split the large layer into 2 LMs, filter out such modes
>>>> +     */
>>>> +    if (!dpu_kms->catalog->caps->has_3d_merge
>>>> +        && mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
>>>> +            return MODE_BAD;
>>>
>>> This can be more specific, like MODE_BAD_HVALUE.
>>>
>>
>> Yes for sure, will fix this up.
>>
>>>>       /*
>>>>        * max crtc width is equal to the max mixer width * 2 and max height is 4K
>>>>        */
>>>>
>>>> ---
>>>> base-commit: af2ea8ab7a546b430726183458da0a173d331272
>>>> change-id: 20241206-no_3dmux-521a55ea0669
>>>>
>>>> Best regards,
>>>> --
>>>> Abhinav Kumar <quic_abhinavk@...cinc.com>
>>>>
>>>
> 
> 
>