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: <BN0PR02MB81421A761275502A5A17AEEE96C79@BN0PR02MB8142.namprd02.prod.outlook.com>
Date:   Wed, 18 Jan 2023 03:30:51 +0000
From:   Kalyan Thota <kalyant@....qualcomm.com>
To:     "dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>,
        "Kalyan Thota (QUIC)" <quic_kalyant@...cinc.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "freedreno@...ts.freedesktop.org" <freedreno@...ts.freedesktop.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "Abhinav Kumar (QUIC)" <quic_abhinavk@...cinc.com>,
        Doug Anderson <dianders@...gle.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robdclark@...omium.org" <robdclark@...omium.org>,
        "dianders@...omium.org" <dianders@...omium.org>,
        "swboyd@...omium.org" <swboyd@...omium.org>,
        "Vinod Polimera (QUIC)" <quic_vpolimer@...cinc.com>
Subject: RE: [PATCH 2/3] drm/msm/disp/dpu1: allow dspp selection for all the
 interfaces



>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>Sent: Tuesday, January 17, 2023 10:26 PM
>To: Kalyan Thota (QUIC) <quic_kalyant@...cinc.com>; dri-
>devel@...ts.freedesktop.org; linux-arm-msm@...r.kernel.org;
>freedreno@...ts.freedesktop.org; devicetree@...r.kernel.org
>Cc: linux-kernel@...r.kernel.org; robdclark@...omium.org;
>dianders@...omium.org; swboyd@...omium.org; Vinod Polimera (QUIC)
><quic_vpolimer@...cinc.com>; Abhinav Kumar (QUIC)
><quic_abhinavk@...cinc.com>
>Subject: Re: [PATCH 2/3] drm/msm/disp/dpu1: allow dspp selection for all the
>interfaces
>
>WARNING: This email originated from outside of Qualcomm. Please be wary of
>any links or attachments, and do not enable macros.
>
>On 17/01/2023 18:21, Kalyan Thota wrote:
>> Allow dspps to be populated as a requirement for all the encoder types
>> it need not be just DSI. If for any encoder the dspp allocation
>> doesn't go through then there can be an option to fallback for color
>> features.
>>
>> Signed-off-by: Kalyan Thota <quic_kalyant@...cinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 9c6817b..e39b345 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -545,7 +545,8 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder
>*drm_enc)
>>   static struct msm_display_topology dpu_encoder_get_topology(
>>                       struct dpu_encoder_virt *dpu_enc,
>>                       struct dpu_kms *dpu_kms,
>> -                     struct drm_display_mode *mode)
>> +                     struct drm_display_mode *mode,
>> +                     struct drm_crtc_state *crtc_state)
>
>Is this new argument used at all?
>
>>   {
>>       struct msm_display_topology topology = {0};
>>       int i, intf_count = 0;
>> @@ -563,8 +564,9 @@ static struct msm_display_topology
>dpu_encoder_get_topology(
>>        * 1 LM, 1 INTF
>>        * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
>>        *
>> -      * Adding color blocks only to primary interface if available in
>> -      * sufficient number
>> +      * dspp blocks are made optional. If RM manager cannot allocate
>> +      * dspp blocks, then reservations will still go through with non dspp LM's
>> +      * so as to allow color management support via composer
>> + fallbacks
>>        */
>
>No, this is not the way to go.
>
>First, RM should prefer non-DSPP-enabled LMs if DSPP blocks are not required.
>Right now your patch makes it possible to allocate LMs, that have DSPP attached,
>for non-CTM-enabled encoder and later fail allocation of DSPP for the CRTC
>which has CTM blob attached.
>
>Second, the decision on using DSPPs should come from dpu_crtc_atomic_check().
>Pass 'bool need_dspp' to this function from dpu_atomic_check(). Fail if the
>need_dspp constraint can't be fulfilled.
>
We may not get color_mgmt_changed property set during modeset commit, where as our resource allocation happens during modeset.
With this approach, dspps will get allocated on first come first serve basis

@Rob, is it possible to send color management property during modeset, in that case, we can use it for dspp allocation to the datapath. The current approach doesn't assume it.
On chrome compositor, I see that color property was being set in the subsequent commits but not in modeset.

>
>>       if (intf_count == 2)
>>               topology.num_lm = 2;
>> @@ -573,11 +575,9 @@ static struct msm_display_topology
>dpu_encoder_get_topology(
>>       else
>>               topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT)
>> ? 2 : 1;
>>
>> -     if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
>> -             if (dpu_kms->catalog->dspp &&
>> -                     (dpu_kms->catalog->dspp_count >= topology.num_lm))
>> -                     topology.num_dspp = topology.num_lm;
>> -     }
>> +     if (dpu_kms->catalog->dspp &&
>> +         (dpu_kms->catalog->dspp_count >= topology.num_lm))
>> +             topology.num_dspp = topology.num_lm;
>>
>>       topology.num_enc = 0;
>>       topology.num_intf = intf_count;
>> @@ -643,7 +643,7 @@ static int dpu_encoder_virt_atomic_check(
>>               }
>>       }
>>
>> -     topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
>> +     topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode,
>> + crtc_state);
>>
>>       /* Reserve dynamic resources now. */
>>       if (!ret) {
>
>--
>With best wishes
>Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ