diff mbox series

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

Message ID 1682033114-28483-6-git-send-email-quic_khsieh@quicinc.com
State New
Headers show
Series add DSC 1.2 dpu supports | expand

Commit Message

Kuogee Hsieh April 20, 2023, 11:25 p.m. UTC
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.

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(-)

Comments

Dmitry Baryshkov April 21, 2023, 12:07 a.m. UTC | #1
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?

> +	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, \
>   	}
>   
>   /*************************************************************
Kuogee Hsieh April 21, 2023, 10:05 p.m. UTC | #2
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

>
>> +    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, \
>>       }
>> /*************************************************************
>
Dmitry Baryshkov April 21, 2023, 10:16 p.m. UTC | #3
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?

> 
>>
>>> +    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, \
>>>       }
>>> /*************************************************************
>>
Kuogee Hsieh April 21, 2023, 11:08 p.m. UTC | #4
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)

>
>>
>>>
>>>> +    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, \
>>>>       }
>>>> /*************************************************************
>>>
>
Dmitry Baryshkov April 21, 2023, 11:11 p.m. UTC | #5
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?

> 
>>
>>>
>>>>
>>>>> +    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, \
>>>>>       }
>>>>> /*************************************************************
>>>>
>>
Kuogee Hsieh April 21, 2023, 11:16 p.m. UTC | #6
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.


>
>>
>>>
>>>>
>>>>>
>>>>>> +    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, \
>>>>>>       }
>>>>>> /*************************************************************
>>>>>
>>>
>
Dmitry Baryshkov April 21, 2023, 11:22 p.m. UTC | #7
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.

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, \
>>>>>>>       }
>>>>>>> /*************************************************************
>>>>>>
>>>>
>>
Kuogee Hsieh April 24, 2023, 4:58 p.m. UTC | #8
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, \
>>>>>>>>       }
>>>>>>>> /*************************************************************
>>>>>>>
>>>>>
>>>
>
diff mbox series

Patch

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 ca107ca..3f2dcc0 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,23 @@  static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
 	MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
 };
 
+static const struct dpu_dsc_sub_blks sm8350_dsc_sblk_0 = {
+	.enc = {.base = 0x100, .len = 0x100},
+	.ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks sm8350_dsc_sblk_1 = {
+	.enc = {.base = 0x200, .len = 0x100},
+	.ctl = {.base = 0xF80, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sm8350_dsc[] = {
+	DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8350_dsc_sblk_0),
+	DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8350_dsc_sblk_1),
+	DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_0),
+	DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_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 +222,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 5957de1..858577c 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,15 @@  static const struct dpu_pingpong_cfg sc7280_pp[] = {
 	PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
 };
 
+static const struct dpu_dsc_sub_blks sc7280_dsc_sblk_0 = {
+	.enc = {.base = 0x100, .len = 0x100},
+	.ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sc7280_dsc[] = {
+	DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sc7280_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 +151,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 9aab110..28443e2 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,25 @@  static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
 	MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
 };
 
+static const struct dpu_dsc_sub_blks sc8280xp_dsc_sblk_0 = {
+	.enc = {.base = 0x100, .len = 0x100},
+	.ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks sc8280xp_dsc_sblk_1 = {
+	.enc = {.base = 0x200, .len = 0x100},
+	.ctl = {.base = 0xF80, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
+	DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_0),
+	DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_1),
+	DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_0),
+	DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_1),
+	DSC_BLK_1_2("dsc_2", DSC_4, 0x82000, 0x100, 0, sm8350_dsc_sblk_0),
+	DSC_BLK_1_2("dsc_2", DSC_5, 0x82000, 0x100, 0, sm8350_dsc_sblk_1),
+};
+
 /* 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 +215,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 02a259b..7c25786 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,23 @@  static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
 	MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
 };
 
+static const struct dpu_dsc_sub_blks sm8450_dsc_sblk_0 = {
+	.enc = {.base = 0x100, .len = 0x100},
+	.ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks sm8450_dsc_sblk_1 = {
+	.enc = {.base = 0x200, .len = 0x100},
+	.ctl = {.base = 0xF80, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sm8450_dsc[] = {
+	DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8450_dsc_sblk_0),
+	DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8450_dsc_sblk_1),
+	DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8450_dsc_sblk_0),
+	DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8450_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 +230,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 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},
+};
+
+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),
+	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, \
 	}
 
 /*************************************************************