Messages in this thread | | | Date | Mon, 24 Apr 2023 09:58:49 -0700 | Subject | Re: [PATCH v1 5/5] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets | From | Kuogee Hsieh <> |
| |
On 4/21/2023 4:22 PM, Dmitry Baryshkov wrote: > On 22/04/2023 02:16, Kuogee Hsieh wrote: >> >> On 4/21/2023 4:11 PM, Dmitry Baryshkov wrote: >>> On 22/04/2023 02:08, Kuogee Hsieh wrote: >>>> >>>> On 4/21/2023 3:16 PM, Dmitry Baryshkov wrote: >>>>> On 22/04/2023 01:05, Kuogee Hsieh wrote: >>>>>> >>>>>> On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote: >>>>>>> On 21/04/2023 02:25, Kuogee Hsieh wrote: >>>>>>>> From: Abhinav Kumar <quic_abhinavk@quicinc.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. >>>>>>> >>>>>>> Please correct line wrapping. 72-75 is usually the preferred width >>>>>>> >>>>>>>> >>>>>>>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>>>>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >>>>>>>> --- >>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19 >>>>>>>> +++++++++++++++++++ >>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 >>>>>>>> +++++++++++ >>>>>>>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21 >>>>>>>> +++++++++++++++++++++ >>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19 >>>>>>>> +++++++++++++++++++ >>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19 >>>>>>>> +++++++++++++++++++ >>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++++++++++-- >>>>>>>> 6 files changed, 99 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>> >>>>>>> >>>>>>> [I commented on sm8550, it applies to all the rest of platforms] >>>>>>> >>>>>>>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg >>>>>>>> sm8550_merge_3d[] = { >>>>>>>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700), >>>>>>>> }; >>>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = { >>>>>>>> + .enc = {.base = 0x100, .len = 0x100}, >>>>>>>> + .ctl = {.base = 0xF00, .len = 0x10}, >>>>>>>> +}; >>>>>>>> + >>>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = { >>>>>>>> + .enc = {.base = 0x200, .len = 0x100}, >>>>>>>> + .ctl = {.base = 0xF80, .len = 0x10}, >>>>>>>> +}; >>>>>>> >>>>>>> Please keep sblk in dpu_hw_catalog for now. >>>>>>> >>>>>>>> + >>>>>>>> +static const struct dpu_dsc_cfg sm8550_dsc[] = { >>>>>>>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, >>>>>>>> sm8550_dsc_sblk_0), >>>>>>>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, >>>>>>>> sm8550_dsc_sblk_1), >>>>>>> >>>>>>> Is there a reason why index in "dsc_N" doesn't match the DSC_n >>>>>>> which comes next to it? >>>>>> >>>>>> usually each DCE (display compression engine) contains two hard >>>>>> slice encoders. >>>>>> >>>>>> DSC_0 and DSC_1 (index) is belong to dsc_0. >>>>>> >>>>>> If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1 >>>>> >>>>> Ah, I see now. So, the block register space is the following: >>>>> DCEi -> >>>>> common >>>>> dsc0_enc >>>>> dsc1_enc >>>>> dsc0_ctl >>>>> dsc1_ctl >>>>> >>>>> Instead of declaring a single DCE unit with two DSC blocks, we >>>>> declare two distinct DSC blocks. This raises a question, how >>>>> independent are these two parts of a single DCE block? For >>>>> example, can we use them to perform compression with different >>>>> parameters? Or use one of them for the DP DSC and another one for >>>>> DSI DSC? Can we have the following configuration: >>>>> >>>>> DSC_0 => DP DSC >>>>> DSC_1, DSC_2 => DSI DSC in DSC_MERGE topology? >>>> >>>> no, For merge mode you have to use same DCE, such as DSC_2 and DSC3 >>>> (pair) >>> >>> Ok, this is for the merge mode. So the dpu_rm should be extended to >>> allocate them in pairs if merge mode is requested. >>> >>> What about using DSC_0 for DP and DSC_1 for DSI? Is it possible? >> >> I never do that, but i think it should works since they can work >> independently. > > Good, thanks for the confirmation. For v2, could you please describe > this arrangement of DCE -> 2xDSC in a comment close to DSC_BLK_1_2 and > corresponding sblk definitions? > > Also could you please fix dpu_rm to allocate DSC blocks in pairs for > DSC_MERGE mode. yes, I will fix DSC_MERGE mode at next patch serial. > > Last, but not least, would it make sense to name the blocks as "dceN" > instead of "dscN"? > >> >> >>> >>>> >>>>> >>>>>> >>>>>>> >>>>>>>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, >>>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0), >>>>>>>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, >>>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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 +235,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 03f162a..be08158 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__ >>>>>>>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks >>>>>>>> sc7280_pp_sblk = { >>>>>>>> {\ >>>>>>>> .name = _name, .id = _id, \ >>>>>>>> .base = _base, .len = 0x140, \ >>>>>>>> - .features = _features, \ >>>>>>>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \ >>>>>>>> + } >>>>>>>> + >>>>>>>> +#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, \ >>>>>>>> } >>>>>>>> /************************************************************* >>>>>>> >>>>> >>> >
| |