Re: [PATCH v1 5/5] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets

[Kuogee Hsieh]

On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
> On 21/04/2023 02:25, Kuogee Hsieh wrote:
> > From: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
> > ---
> >   .../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

> > +    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, \
> >       }
> > /*************************************************************