[Freedreno] [PATCH 1/2] drm/msm/dpu: fix DSC 1.2 block lengths

Fri Jun 23 11:25:14 UTC 2023

On 23/06/2023 08:47, Abhinav Kumar wrote:
> 
> 
> On 6/22/2023 6:37 PM, Dmitry Baryshkov wrote:
>> All DSC_BLK_1_2 declarations incorrectly pass 0x29c as the block length.
>> This includes the common block itself, enc subblocks and some empty
>> space around. Change that to pass 0x4 instead, the length of common
>> register block itself.
>>
>> Fixes: 0d1b10c63346 ("drm/msm/dpu: add DSC 1.2 hw blocks for relevant 
>> chipsets")
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> 
> There is no need of a fixes tag for this.
> 
> This is not a bug but was intentional.

We have other subblocks which are not dumped withoyt Ryan's patchset. So 
this declaration should be corrected.

> 
> Till we added sub-block parsing support we had to dump the whole block.
> 
> And hence I would suggest this change should be merged after the 
> sub-block parsing change otherwise we wont have full register dumps for 
> DSC.

No, the order should be opposite: this is merged first, then subblocks 
dumping can use block->len in all the cases.

> 
> Also, please add :
> 
> Suggested-by: Ryan McCann <quic_rmccann at quicinc.com>

More likely:

Reported-by: Ryan McCann <quic_rmccann at quicinc.com>

> 
> 
>> ---
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h   |  8 ++++----
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h   |  2 +-
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 12 ++++++------
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h   |  8 ++++----
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   |  8 ++++----
>>   5 files changed, 19 insertions(+), 19 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 8da424eaee6a..6edf323f381f 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
>> @@ -159,10 +159,10 @@ static const struct dpu_merge_3d_cfg 
>> sm8350_merge_3d[] = {
>>    * its own different sub block address.
>>    */
>>   static const struct dpu_dsc_cfg sm8350_dsc[] = {
>> -    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> -    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x4, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x4, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>>   };
>>   static const struct dpu_intf_cfg 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 900fee410e11..5354003aa8be 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
>> @@ -104,7 +104,7 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
>>   /* NOTE: sc7280 only has one DSC hard slice encoder */
>>   static const struct dpu_dsc_cfg sc7280_dsc[] = {
>> -    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>>   };
>>   static const struct dpu_wb_cfg sc7280_wb[] = {
>> 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 f6ce6b090f71..1d374abec1fd 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
>> @@ -148,12 +148,12 @@ static const struct dpu_merge_3d_cfg 
>> sc8280xp_merge_3d[] = {
>>    * its own different sub block address.
>>    */
>>   static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
>> -    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> -    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> -    DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x29c, 0, dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x29c, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x4, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x4, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x4, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x4, 0, dsc_sblk_1),
>>   };
>>   /* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for 
>> now */
>> 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 8d13c369213c..79447d8cab05 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
>> @@ -167,10 +167,10 @@ static const struct dpu_merge_3d_cfg 
>> sm8450_merge_3d[] = {
>>    * its own different sub block address.
>>    */
>>   static const struct dpu_dsc_cfg sm8450_dsc[] = {
>> -    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> -    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x4, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x4, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>>   };
>>   static const struct dpu_intf_cfg 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 f17b9a7fee85..26e3c28003f7 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
>> @@ -171,10 +171,10 @@ static const struct dpu_merge_3d_cfg 
>> sm8550_merge_3d[] = {
>>    * its own different sub block address.
>>    */
>>   static const struct dpu_dsc_cfg sm8550_dsc[] = {
>> -    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> -    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> -    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x4, 0, dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x4, 0, dsc_sblk_1),
>> +    DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> +    DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x4, 
>> BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>>   };
>>   static const struct dpu_intf_cfg sm8550_intf[] = {

-- 
With best wishes
Dmitry