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]
Date:   Wed, 3 May 2023 11:54:03 -0700
From:   Abhinav Kumar <quic_abhinavk@...cinc.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Kuogee Hsieh <quic_khsieh@...cinc.com>,
        <dri-devel@...ts.freedesktop.org>, <robdclark@...il.com>,
        <sean@...rly.run>, <swboyd@...omium.org>, <dianders@...omium.org>,
        <vkoul@...nel.org>, <daniel@...ll.ch>, <airlied@...il.com>,
        <agross@...nel.org>, <andersson@...nel.org>
CC:     <quic_sbillaka@...cinc.com>, <marijn.suijten@...ainline.org>,
        <freedreno@...ts.freedesktop.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 7/7] drm/msm/dpu: add DSC 1.2 hw blocks for relevant
 chipsets



On 5/2/2023 2:42 PM, Dmitry Baryshkov wrote:
> On 03/05/2023 00:03, Kuogee Hsieh wrote:
>> From: Abhinav Kumar <quic_abhinavk@...cinc.com>
>>
>> Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and
>> feature flag information.  Each display compression engine (DCE) contains
>> dual hard slice DSC encoders so both share same base address but with
>> its own different sub block address.
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> 
> Minor question below.
> 
>> ---
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 +++++++++++
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h |  7 ++++++
>>   .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h   | 16 +++++++++++++
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 +++++++++++
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 +++++++++++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     | 27 
>> ++++++++++++++++++++--
>>   6 files changed, 90 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> index 4f6a965..f98c2a5 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> @@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg 
>> sm8350_merge_3d[] = {
>>       MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
>>   };
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8350_dsc[] = {
>> +    DSC_BLK_1_2("dce_0", DSC_0, 0x80000, 0x100, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1),
>> +};
>> +
>>   static const struct dpu_intf_cfg sm8350_intf[] = {
>>       INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, 
>> MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
>>       INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, 
>> INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
>> @@ -205,6 +217,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = {
>>       .dspp = sm8350_dspp,
>>       .pingpong_count = ARRAY_SIZE(sm8350_pp),
>>       .pingpong = sm8350_pp,
>> +    .dsc = sm8350_dsc,
>> +    .dsc_count = ARRAY_SIZE(sm8350_dsc),
>>       .merge_3d_count = ARRAY_SIZE(sm8350_merge_3d),
>>       .merge_3d = sm8350_merge_3d,
>>       .intf_count = ARRAY_SIZE(sm8350_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> index 6b2c7ea..3fd0498a 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> @@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
>>       PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, 
>> sc7280_pp_sblk, -1, -1),
>>   };
>> +/* NOTE: sc7280 only has one dsc hard slice encoder */
>> +static const struct dpu_dsc_cfg sc7280_dsc[] = {
>> +    DSC_BLK_1_2("dce_0", DSC_0, 0x80000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
>> +};
>> +
>>   static const struct dpu_intf_cfg sc7280_intf[] = {
>>       INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, 
>> MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
>>       INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, 
>> INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
>> @@ -142,6 +147,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
>>       .mixer = sc7280_lm,
>>       .pingpong_count = ARRAY_SIZE(sc7280_pp),
>>       .pingpong = sc7280_pp,
>> +    .dsc_count = ARRAY_SIZE(sc7280_dsc),
>> +    .dsc = sc7280_dsc,
>>       .intf_count = ARRAY_SIZE(sc7280_intf),
>>       .intf = sc7280_intf,
>>       .vbif_count = ARRAY_SIZE(sdm845_vbif),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> index 706d0f1..ce583eb 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> @@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg 
>> sc8280xp_merge_3d[] = {
>>       MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
>>   };
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
>> +    DSC_BLK_1_2("dce_0", DSC_0, 0x80000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_2", DSC_4, 0x82000, 0x100, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_2", DSC_5, 0x82000, 0x100, 0, dsc_sblk_1),
> 
> Just checking: all other platforms have non-422 encoders first, then the 
> 422-enabled ones. Is there any reason why this platform has them in the 
> opposite order?
> 

This turned out to be a mistake due to a confusion in the internal 
documents.

So this should be updated to :

DSC_0 pair -> only RGB
DSC_1 pair -> DPU_DSC_NATIVE_422_EN
DSC_2 pair -> only RGB

There was a generic statement in the document that with 3 DSC pairs 
first two are YUV 422 capable and third one is RGB. I assumed that was 
true for all chipsets with 3 DSC pairs and made this change but i was 
wrong as that was specific to some other chipset not present in this 
catalog yet.

Rest of the chipsets are correct as they have only two DSC pairs.

Will update this one.

>> +};
>> +
>>   /* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for 
>> now */
>>   static const struct dpu_intf_cfg sc8280xp_intf[] = {
>>       INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, 
>> MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
>> @@ -196,6 +210,8 @@ const struct dpu_mdss_cfg dpu_sc8280xp_cfg = {
>>       .dspp = sc8280xp_dspp,
>>       .pingpong_count = ARRAY_SIZE(sc8280xp_pp),
>>       .pingpong = sc8280xp_pp,
>> +    .dsc = sc8280xp_dsc,
>> +    .dsc_count = ARRAY_SIZE(sc8280xp_dsc),
>>       .merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d),
>>       .merge_3d = sc8280xp_merge_3d,
>>       .intf_count = ARRAY_SIZE(sc8280xp_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> index 4ecb3df..3950e7b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> @@ -161,6 +161,18 @@ static const struct dpu_merge_3d_cfg 
>> sm8450_merge_3d[] = {
>>       MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
>>   };
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8450_dsc[] = {
>> +    DSC_BLK_1_2("dce_0", DSC_0, 0x80000, 0x100, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1),
>> +};
>> +
>>   static const struct dpu_intf_cfg sm8450_intf[] = {
>>       INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, 
>> MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
>>       INTF_BLK("intf_1", INTF_1, 0x35000, 0x300, INTF_DSI, 0, 24, 
>> INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
>> @@ -213,6 +225,8 @@ const struct dpu_mdss_cfg dpu_sm8450_cfg = {
>>       .dspp = sm8450_dspp,
>>       .pingpong_count = ARRAY_SIZE(sm8450_pp),
>>       .pingpong = sm8450_pp,
>> +    .dsc = sm8450_dsc,
>> +    .dsc_count = ARRAY_SIZE(sm8450_dsc),
>>       .merge_3d_count = ARRAY_SIZE(sm8450_merge_3d),
>>       .merge_3d = sm8450_merge_3d,
>>       .intf_count = ARRAY_SIZE(sm8450_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h 
>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
>> index d0ab351..1b3f542 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
>> @@ -165,6 +165,18 @@ static const struct dpu_merge_3d_cfg 
>> sm8550_merge_3d[] = {
>>       MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>>   };
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>> +    DSC_BLK_1_2("dce_0", DSC_0, 0x80000, 0x100, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, 
>> BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1),
>> +};
>> +
>>   static const struct dpu_intf_cfg sm8550_intf[] = {
>>       INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, 
>> MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
>>       /* TODO TE sub-blocks for intf1 & intf2 */
>> @@ -218,6 +230,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
>>       .dspp = sm8550_dspp,
>>       .pingpong_count = ARRAY_SIZE(sm8550_pp),
>>       .pingpong = sm8550_pp,
>> +    .dsc = sm8550_dsc,
>> +    .dsc_count = ARRAY_SIZE(sm8550_dsc),
>>       .merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
>>       .merge_3d = sm8550_merge_3d,
>>       .intf_count = ARRAY_SIZE(sm8550_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> index 6ea1e9d..83c0cd9 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights 
>> reserved.
>> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All 
>> rights reserved.
>>    */
>>   #define pr_fmt(fmt)    "[drm:%s:%d] " fmt, __func__, __LINE__
>> @@ -536,11 +536,34 @@ static const struct dpu_pingpong_sub_blks 
>> sc7280_pp_sblk = {
>>   /*************************************************************
>>    * DSC sub blocks config
>>    *************************************************************/
>> +static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
>> +    .enc = {.base = 0x100, .len = 0x100},
>> +    .ctl = {.base = 0xF00, .len = 0x10},
>> +};
>> +
>> +static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
>> +    .enc = {.base = 0x200, .len = 0x100},
>> +    .ctl = {.base = 0xF80, .len = 0x10},
>> +};
>> +
>>   #define DSC_BLK(_name, _id, _base, _features) \
>>       {\
>>       .name = _name, .id = _id, \
>>       .base = _base, .len = 0x140, \
>> -    .features = _features, \
>> +    .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>> +    }
>> +
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
>> +    {\
>> +    .name = _name, .id = _id, \
>> +    .base = _base, .len = _len, \
>> +    .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
>> +    .sblk = &_sblk, \
>>       }
>>   /*************************************************************
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ