| Message ID | 1683218805-23419-4-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add DSC 1.2 dpu supports | expand |
PP_BLK_TE is no longer there. marcos -> macros. On 2023-05-04 09:46:41, Kuogee Hsieh wrote: > At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate I may have not made this clear, but the comments on patch 2/7 (introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify DPU 7.0.0 exactly in favour of "legacy", which has no definition at all and changes over time. > pingpong ops functions are required to complete DSC data path setup if > this chipset has DSC hardware block presented. This patch add > DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC > hardware block presented. Strictly speaking this patch together with 2/7 is not bisectable, as 2/7 first disables the callbacks for _all_ hardware and then this patch adds it back by adding the flag to all DPU < 7.0.0 catalog descriptions. To solve that, as we do in other DPU patch-series, just squash this patch into 2/7. That way you also don't have to spend extra time rewording this commit message either to match the improvements we made in 2/7 (for example, you mention that "ops functions are required to complete DSC data path setup", but those were already available before 2/7, despite sounding as if this is a new thing that was previously missing entirely). But please wait at least a couple days before sending v6. I only have a few hours every day/week but would appreciate to review and test all the other patches. > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++-------- > .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++-------- > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++----------- > .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++----------- > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++----------- > 5 files changed, 52 insertions(+), 52 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > index 521cfd5..ef92545 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = { > }; > > static const struct dpu_pingpong_cfg msm8998_pp[] = { > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), This should be added to the MASK (add new #define's where necessary). - Marijn > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h > index b109757..697fbd8 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h > @@ -110,17 +110,17 @@ static const struct dpu_lm_cfg sdm845_lm[] = { > }; > > static const struct dpu_pingpong_cfg sdm845_pp[] = { > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h > index 30aff2b..cb117ca 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h > @@ -128,23 +128,23 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = { > }; > > static const struct dpu_pingpong_cfg sm8150_pp[] = { > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > -1), > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > -1), > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > -1), > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > -1), > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > -1), > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > -1), > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > index fec1665..27eda6a 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > @@ -116,23 +116,23 @@ static const struct dpu_lm_cfg sc8180x_lm[] = { > }; > > static const struct dpu_pingpong_cfg sc8180x_pp[] = { > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > -1), > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > -1), > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > -1), > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > -1), > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > -1), > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > -1), > }; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h > index 37716b8..70fdd4d 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h > @@ -129,23 +129,23 @@ static const struct dpu_dspp_cfg sm8250_dspp[] = { > }; > > static const struct dpu_pingpong_cfg sm8250_pp[] = { > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > -1), > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > -1), > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > -1), > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > -1), > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > -1), > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > -1), > }; > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On Thu, 4 May 2023 at 20:49, Marijn Suijten <marijn.suijten@somainline.org> wrote: > > PP_BLK_TE is no longer there. > > marcos -> macros. > > On 2023-05-04 09:46:41, Kuogee Hsieh wrote: > > At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate > > I may have not made this clear, but the comments on patch 2/7 > (introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify > DPU 7.0.0 exactly in favour of "legacy", which has no definition at all > and changes over time. > > > pingpong ops functions are required to complete DSC data path setup if > > this chipset has DSC hardware block presented. This patch add > > DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC > > hardware block presented. > > Strictly speaking this patch together with 2/7 is not bisectable, as 2/7 > first disables the callbacks for _all_ hardware and then this patch adds > it back by adding the flag to all DPU < 7.0.0 catalog descriptions. I asked to split these into two patches, but I see your point and partially agree with it. However if we mix the catalog changes with functional changes, it is too easy to overlook or misjudge the functional changes. As you are correct about bisectability, I'd probably suggest either having three patches (define flag, update catalog, handle flag in the driver) or squashing first two patches to have two patches (add flag + catalog, separate functional changes). > > To solve that, as we do in other DPU patch-series, just squash this > patch into 2/7. That way you also don't have to spend extra time > rewording this commit message either to match the improvements we made > in 2/7 (for example, you mention that "ops functions are required to > complete DSC data path setup", but those were already available before > 2/7, despite sounding as if this is a new thing that was previously > missing entirely). > > But please wait at least a couple days before sending v6. I only have a > few hours every day/week but would appreciate to review and test all the > other patches. > > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > > --- > > .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++-------- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++-------- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++----------- > > .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++----------- > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++----------- > > 5 files changed, 52 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > > index 521cfd5..ef92545 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > > @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = { > > }; > > > > static const struct dpu_pingpong_cfg msm8998_pp[] = { > > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > > This should be added to the MASK (add new #define's where necessary). > > - Marijn > > > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), > > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), > > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), > > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), > > }; > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h > > index b109757..697fbd8 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h > > @@ -110,17 +110,17 @@ static const struct dpu_lm_cfg sdm845_lm[] = { > > }; > > > > static const struct dpu_pingpong_cfg sdm845_pp[] = { > > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), > > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), > > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), > > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), > > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), > > }; > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h > > index 30aff2b..cb117ca 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h > > @@ -128,23 +128,23 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = { > > }; > > > > static const struct dpu_pingpong_cfg sm8150_pp[] = { > > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > -1), > > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > -1), > > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > -1), > > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > -1), > > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > > -1), > > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > > -1), > > }; > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > > index fec1665..27eda6a 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > > @@ -116,23 +116,23 @@ static const struct dpu_lm_cfg sc8180x_lm[] = { > > }; > > > > static const struct dpu_pingpong_cfg sc8180x_pp[] = { > > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > -1), > > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > -1), > > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > -1), > > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > -1), > > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > > -1), > > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > > -1), > > }; > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h > > index 37716b8..70fdd4d 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h > > @@ -129,23 +129,23 @@ static const struct dpu_dspp_cfg sm8250_dspp[] = { > > }; > > > > static const struct dpu_pingpong_cfg sm8250_pp[] = { > > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > -1), > > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), > > -1), > > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), > > -1), > > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), > > -1), > > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), > > -1), > > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), > > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), > > -1), > > }; > > > > -- > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > > a Linux Foundation Collaborative Project > >
On 2023-05-04 20:53:33, Dmitry Baryshkov wrote: > On Thu, 4 May 2023 at 20:49, Marijn Suijten > <marijn.suijten@somainline.org> wrote: > > > > PP_BLK_TE is no longer there. > > > > marcos -> macros. > > > > On 2023-05-04 09:46:41, Kuogee Hsieh wrote: > > > At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate > > > > I may have not made this clear, but the comments on patch 2/7 > > (introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify > > DPU 7.0.0 exactly in favour of "legacy", which has no definition at all > > and changes over time. > > > > > pingpong ops functions are required to complete DSC data path setup if > > > this chipset has DSC hardware block presented. This patch add > > > DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC > > > hardware block presented. > > > > Strictly speaking this patch together with 2/7 is not bisectable, as 2/7 > > first disables the callbacks for _all_ hardware and then this patch adds > > it back by adding the flag to all DPU < 7.0.0 catalog descriptions. > > I asked to split these into two patches, but I see your point and > partially agree with it. However if we mix the catalog changes with > functional changes, it is too easy to overlook or misjudge the > functional changes. I did the same in the INTF TE series for patches that have very little and/or very obvious functional changes: exactly this combination of guarding a few callbacks behind a feature bit, and setting that feature bit on a few specific catalog entries. > As you are correct about bisectability, I'd probably suggest either > having three patches (define flag, update catalog, handle flag in the > driver) or squashing first two patches to have two patches (add flag + > catalog, separate functional changes). Sure, if you really prefer a split I'd go for two patches: 1. Add the flag to the enum and catalog; 2. Add the ops guard (functional change). Then don't forget to reword the commit message, following the guidelines below and the suggestion for 2/7. - Marijn > > To solve that, as we do in other DPU patch-series, just squash this > > patch into 2/7. That way you also don't have to spend extra time > > rewording this commit message either to match the improvements we made > > in 2/7 (for example, you mention that "ops functions are required to > > complete DSC data path setup", but those were already available before > > 2/7, despite sounding as if this is a new thing that was previously > > missing entirely). > > > > But please wait at least a couple days before sending v6. I only have a > > few hours every day/week but would appreciate to review and test all the > > other patches. > > > > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > > > --- > > > .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++-------- > > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++-------- > > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++----------- > > > .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++----------- > > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++----------- > > > 5 files changed, 52 insertions(+), 52 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > > > index 521cfd5..ef92545 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > > > @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = { > > > }; > > > > > > static const struct dpu_pingpong_cfg msm8998_pp[] = { > > > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, > > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), > > > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), > > > > This should be added to the MASK (add new #define's where necessary). > > > > - Marijn <snip>
On 5/4/2023 11:23 AM, Marijn Suijten wrote: > On 2023-05-04 20:53:33, Dmitry Baryshkov wrote: >> On Thu, 4 May 2023 at 20:49, Marijn Suijten >> <marijn.suijten@somainline.org> wrote: >>> >>> PP_BLK_TE is no longer there. >>> >>> marcos -> macros. >>> >>> On 2023-05-04 09:46:41, Kuogee Hsieh wrote: >>>> At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate >>> >>> I may have not made this clear, but the comments on patch 2/7 >>> (introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify >>> DPU 7.0.0 exactly in favour of "legacy", which has no definition at all >>> and changes over time. >>> >>>> pingpong ops functions are required to complete DSC data path setup if >>>> this chipset has DSC hardware block presented. This patch add >>>> DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC >>>> hardware block presented. >>> >>> Strictly speaking this patch together with 2/7 is not bisectable, as 2/7 >>> first disables the callbacks for _all_ hardware and then this patch adds >>> it back by adding the flag to all DPU < 7.0.0 catalog descriptions. >> >> I asked to split these into two patches, but I see your point and >> partially agree with it. However if we mix the catalog changes with >> functional changes, it is too easy to overlook or misjudge the >> functional changes. > > I did the same in the INTF TE series for patches that have very little > and/or very obvious functional changes: exactly this combination of > guarding a few callbacks behind a feature bit, and setting that feature > bit on a few specific catalog entries. > >> As you are correct about bisectability, I'd probably suggest either >> having three patches (define flag, update catalog, handle flag in the >> driver) or squashing first two patches to have two patches (add flag + >> catalog, separate functional changes). > > Sure, if you really prefer a split I'd go for two patches: > 1. Add the flag to the enum and catalog; > 2. Add the ops guard (functional change). > > Then don't forget to reword the commit message, following the guidelines > below and the suggestion for 2/7. > > - Marijn Plan sounds good to me. Marijn, we will wait for a couple of days to post the next rev but would be hard more than that as we need to pick up other things which are pending on top of this. Hence would appreciate if you can finish reviews by then. > >>> To solve that, as we do in other DPU patch-series, just squash this >>> patch into 2/7. That way you also don't have to spend extra time >>> rewording this commit message either to match the improvements we made >>> in 2/7 (for example, you mention that "ops functions are required to >>> complete DSC data path setup", but those were already available before >>> 2/7, despite sounding as if this is a new thing that was previously >>> missing entirely). >>> >>> But please wait at least a couple days before sending v6. I only have a >>> few hours every day/week but would appreciate to review and test all the >>> other patches. >>> >>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> >>>> --- >>>> .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++-------- >>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++-------- >>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++----------- >>>> .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++----------- >>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++----------- >>>> 5 files changed, 52 insertions(+), 52 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h >>>> index 521cfd5..ef92545 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h >>>> @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = { >>>> }; >>>> >>>> static const struct dpu_pingpong_cfg msm8998_pp[] = { >>>> - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, >>>> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), >>>> + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), >>> >>> This should be added to the MASK (add new #define's where necessary). >>> >>> - Marijn > > <snip>
On 2023-05-04 11:25:44, Abhinav Kumar wrote: <snip> > > Sure, if you really prefer a split I'd go for two patches: > > 1. Add the flag to the enum and catalog; > > 2. Add the ops guard (functional change). > > > > Then don't forget to reword the commit message, following the guidelines > > below and the suggestion for 2/7. > > > > - Marijn > > Plan sounds good to me. > > Marijn, we will wait for a couple of days to post the next rev but would > be hard more than that as we need to pick up other things which are > pending on top of this. Hence would appreciate if you can finish reviews > by then. It depends on how many more revisions are needed after that, and not all patches in this series have an r-b just yet. Given the amount of review comments that are still trickling in (also on patches that already have maintainer r-b) I don't think we're quite there to start thinging about picking this up in drm-msm just yet. I doubt anyone wants a repeat of the original DSC series, which went through many review rounds yet still required multiple series of bugfixes (some of which were pointed out and ignored in review) to be brought to a working state. But the split across topics per series already makes this a lot less likely, many thanks for that. In other words, let's take it slow and do things properly this time. And who knows, perhaps the rest of these patches are more straightforward. - Marijn <snip>
On 5/4/2023 12:36 PM, Marijn Suijten wrote: > On 2023-05-04 11:25:44, Abhinav Kumar wrote: > <snip> >>> Sure, if you really prefer a split I'd go for two patches: >>> 1. Add the flag to the enum and catalog; >>> 2. Add the ops guard (functional change). >>> >>> Then don't forget to reword the commit message, following the guidelines >>> below and the suggestion for 2/7. >>> >>> - Marijn >> >> Plan sounds good to me. >> >> Marijn, we will wait for a couple of days to post the next rev but would >> be hard more than that as we need to pick up other things which are >> pending on top of this. Hence would appreciate if you can finish reviews >> by then. > > It depends on how many more revisions are needed after that, and not all > patches in this series have an r-b just yet. Given the amount of review > comments that are still trickling in (also on patches that already have > maintainer r-b) I don't think we're quite there to start thinging about > picking this up in drm-msm just yet. I doubt anyone wants a repeat of > the original DSC series, which went through many review rounds yet still > required multiple series of bugfixes (some of which were pointed out and > ignored in review) to be brought to a working state. But the split > across topics per series already makes this a lot less likely, many > thanks for that. > I think the outstanding comments shouldnt last more than 1-2 revs more on this one as its mostly due to multiple patches on the list touching catalog at the same time. I have been monitoring the comments closely even though I dont respond to all of them. One of the major reasons of the number of issues with DSC 1.1 was QC didn't really have the devices or panels to support it. Thats why I changed that this time around to take more control of validation of DSC 1.2 and ofcourse decided to break up of series into the least amount of functionality needed to keep the DPU driver intact. All that being said, we still value your comments and would gladly wait for a couple of days like I already wrote. But there are more incremental series on top of this: -> DSI changes for DSC 1.2 -> proper teardown for DSC -> DSC pair allocation support -> DSC 1.2 over DP We will be posting all of these within next couple of weeks on top of this. > In other words, let's take it slow and do things properly this time. And > who knows, perhaps the rest of these patches are more straightforward. > Ack. the intent is always to do things right the first time. > - Marijn > > <snip>
On 04/05/2023 22:50, Abhinav Kumar wrote: > > > On 5/4/2023 12:36 PM, Marijn Suijten wrote: >> On 2023-05-04 11:25:44, Abhinav Kumar wrote: >> <snip> >>>> Sure, if you really prefer a split I'd go for two patches: >>>> 1. Add the flag to the enum and catalog; >>>> 2. Add the ops guard (functional change). >>>> >>>> Then don't forget to reword the commit message, following the >>>> guidelines >>>> below and the suggestion for 2/7. >>>> >>>> - Marijn >>> >>> Plan sounds good to me. >>> >>> Marijn, we will wait for a couple of days to post the next rev but would >>> be hard more than that as we need to pick up other things which are >>> pending on top of this. Hence would appreciate if you can finish reviews >>> by then. >> >> It depends on how many more revisions are needed after that, and not all >> patches in this series have an r-b just yet. Given the amount of review >> comments that are still trickling in (also on patches that already have >> maintainer r-b) I don't think we're quite there to start thinging about >> picking this up in drm-msm just yet. I doubt anyone wants a repeat of >> the original DSC series, which went through many review rounds yet still >> required multiple series of bugfixes (some of which were pointed out and >> ignored in review) to be brought to a working state. But the split >> across topics per series already makes this a lot less likely, many >> thanks for that. >> > > I think the outstanding comments shouldnt last more than 1-2 revs more > on this one as its mostly due to multiple patches on the list touching > catalog at the same time. I have been monitoring the comments closely > even though I dont respond to all of them. > > One of the major reasons of the number of issues with DSC 1.1 was QC > didn't really have the devices or panels to support it. Thats why I > changed that this time around to take more control of validation of DSC > 1.2 and ofcourse decided to break up of series into the least amount of > functionality needed to keep the DPU driver intact. > > All that being said, we still value your comments and would gladly wait > for a couple of days like I already wrote. But there are more > incremental series on top of this: > > -> DSI changes for DSC 1.2 > -> proper teardown for DSC > -> DSC pair allocation support > -> DSC 1.2 over DP > > We will be posting all of these within next couple of weeks on top of this. I'd say, it's fine to post them now, as we have more or less agreed on the helper series. The interface between the series should be stable then. The RM series is probably the one having bigger dependencies/conflicts on other pending patches (include virtual wide planes) > >> In other words, let's take it slow and do things properly this time. And >> who knows, perhaps the rest of these patches are more straightforward. >> > > Ack. the intent is always to do things right the first time. > >> - Marijn >> >> <snip>
On 5/4/2023 12:59 PM, Dmitry Baryshkov wrote: > On 04/05/2023 22:50, Abhinav Kumar wrote: >> >> >> On 5/4/2023 12:36 PM, Marijn Suijten wrote: >>> On 2023-05-04 11:25:44, Abhinav Kumar wrote: >>> <snip> >>>>> Sure, if you really prefer a split I'd go for two patches: >>>>> 1. Add the flag to the enum and catalog; >>>>> 2. Add the ops guard (functional change). >>>>> >>>>> Then don't forget to reword the commit message, following the >>>>> guidelines >>>>> below and the suggestion for 2/7. >>>>> >>>>> - Marijn >>>> >>>> Plan sounds good to me. >>>> >>>> Marijn, we will wait for a couple of days to post the next rev but >>>> would >>>> be hard more than that as we need to pick up other things which are >>>> pending on top of this. Hence would appreciate if you can finish >>>> reviews >>>> by then. >>> >>> It depends on how many more revisions are needed after that, and not all >>> patches in this series have an r-b just yet. Given the amount of review >>> comments that are still trickling in (also on patches that already have >>> maintainer r-b) I don't think we're quite there to start thinging about >>> picking this up in drm-msm just yet. I doubt anyone wants a repeat of >>> the original DSC series, which went through many review rounds yet still >>> required multiple series of bugfixes (some of which were pointed out and >>> ignored in review) to be brought to a working state. But the split >>> across topics per series already makes this a lot less likely, many >>> thanks for that. >>> >> >> I think the outstanding comments shouldnt last more than 1-2 revs more >> on this one as its mostly due to multiple patches on the list touching >> catalog at the same time. I have been monitoring the comments closely >> even though I dont respond to all of them. >> >> One of the major reasons of the number of issues with DSC 1.1 was QC >> didn't really have the devices or panels to support it. Thats why I >> changed that this time around to take more control of validation of >> DSC 1.2 and ofcourse decided to break up of series into the least >> amount of functionality needed to keep the DPU driver intact. >> >> All that being said, we still value your comments and would gladly >> wait for a couple of days like I already wrote. But there are more >> incremental series on top of this: >> >> -> DSI changes for DSC 1.2 >> -> proper teardown for DSC >> -> DSC pair allocation support >> -> DSC 1.2 over DP >> >> We will be posting all of these within next couple of weeks on top of >> this. > > I'd say, it's fine to post them now, as we have more or less agreed on > the helper series. The interface between the series should be stable then. > > The RM series is probably the one having bigger dependencies/conflicts > on other pending patches (include virtual wide planes) > 1 is already posted, will keep fixing review comments 2 will be posted pretty soon DSC1.2 over DSI will be complete with this set. I will finish up virtual planes review by early next week. Already underway ... 3 & 4 will be posted soon after that. >> >>> In other words, let's take it slow and do things properly this time. And >>> who knows, perhaps the rest of these patches are more straightforward. >>> >> >> Ack. the intent is always to do things right the first time. >> >>> - Marijn >>> >>> <snip> >
On 2023-05-04 12:50:57, Abhinav Kumar wrote: > > > On 5/4/2023 12:36 PM, Marijn Suijten wrote: > > On 2023-05-04 11:25:44, Abhinav Kumar wrote: > > <snip> > >>> Sure, if you really prefer a split I'd go for two patches: > >>> 1. Add the flag to the enum and catalog; > >>> 2. Add the ops guard (functional change). > >>> > >>> Then don't forget to reword the commit message, following the guidelines > >>> below and the suggestion for 2/7. > >>> > >>> - Marijn > >> > >> Plan sounds good to me. > >> > >> Marijn, we will wait for a couple of days to post the next rev but would > >> be hard more than that as we need to pick up other things which are > >> pending on top of this. Hence would appreciate if you can finish reviews > >> by then. > > > > It depends on how many more revisions are needed after that, and not all > > patches in this series have an r-b just yet. Given the amount of review > > comments that are still trickling in (also on patches that already have > > maintainer r-b) I don't think we're quite there to start thinging about > > picking this up in drm-msm just yet. I doubt anyone wants a repeat of > > the original DSC series, which went through many review rounds yet still > > required multiple series of bugfixes (some of which were pointed out and > > ignored in review) to be brought to a working state. But the split > > across topics per series already makes this a lot less likely, many > > thanks for that. > > > > I think the outstanding comments shouldnt last more than 1-2 revs more > on this one as its mostly due to multiple patches on the list touching > catalog at the same time. I have been monitoring the comments closely > even though I dont respond to all of them. > > One of the major reasons of the number of issues with DSC 1.1 was QC > didn't really have the devices or panels to support it. Thats why I > changed that this time around to take more control of validation of DSC > 1.2 and ofcourse decided to break up of series into the least amount of > functionality needed to keep the DPU driver intact. Really glad that you are able to test and validate it now, that goes a long way. Does that also mean you can post the panel patches quickly, so that everyone has a point of reference? As you said that is one of the main points where DSC 1.1 "went wrong" (a misunderstanding of drm_dsc_config). > All that being said, we still value your comments and would gladly wait > for a couple of days like I already wrote. But there are more > incremental series on top of this: > > -> DSI changes for DSC 1.2 > -> proper teardown for DSC > -> DSC pair allocation support > -> DSC 1.2 over DP Yeah, I'm familiar with the concept of having many dependent series, and now DSC series are even rebasing on DPU (catalog) cleanups to preempt conflicts, which is really hard to follow. Dmitry just pushed v5 of "drm/i915/dsc: change DSC param tables to follow the DSC model" [1] and seems to have dropped some patches that some of these series are depending on, resulting in compilation failures. Other series don't seem to fully mention all their dependencies. [1]: https://lore.kernel.org/linux-arm-msm/20230504153511.4007320-1-dmitry.baryshkov@linaro.org/T/#u So, for this to go as convenient as possible, do you have a list of series, in a desired order that they should be reviewed and tested? That way I can direct my priorities and help achieve the goal of picking base dependencies as early as possible. For example, one of the many series regresses DSC on the Xperia XZ3 (SDM845), but I have yet to bisect and understand which patch it is. Will let you know as soon as I get my tree in order. - Marijn
On 05/05/2023 00:39, Marijn Suijten wrote: > On 2023-05-04 12:50:57, Abhinav Kumar wrote: >> >> >> On 5/4/2023 12:36 PM, Marijn Suijten wrote: >>> On 2023-05-04 11:25:44, Abhinav Kumar wrote: >>> <snip> >>>>> Sure, if you really prefer a split I'd go for two patches: >>>>> 1. Add the flag to the enum and catalog; >>>>> 2. Add the ops guard (functional change). >>>>> >>>>> Then don't forget to reword the commit message, following the guidelines >>>>> below and the suggestion for 2/7. >>>>> >>>>> - Marijn >>>> >>>> Plan sounds good to me. >>>> >>>> Marijn, we will wait for a couple of days to post the next rev but would >>>> be hard more than that as we need to pick up other things which are >>>> pending on top of this. Hence would appreciate if you can finish reviews >>>> by then. >>> >>> It depends on how many more revisions are needed after that, and not all >>> patches in this series have an r-b just yet. Given the amount of review >>> comments that are still trickling in (also on patches that already have >>> maintainer r-b) I don't think we're quite there to start thinging about >>> picking this up in drm-msm just yet. I doubt anyone wants a repeat of >>> the original DSC series, which went through many review rounds yet still >>> required multiple series of bugfixes (some of which were pointed out and >>> ignored in review) to be brought to a working state. But the split >>> across topics per series already makes this a lot less likely, many >>> thanks for that. >>> >> >> I think the outstanding comments shouldnt last more than 1-2 revs more >> on this one as its mostly due to multiple patches on the list touching >> catalog at the same time. I have been monitoring the comments closely >> even though I dont respond to all of them. >> >> One of the major reasons of the number of issues with DSC 1.1 was QC >> didn't really have the devices or panels to support it. Thats why I >> changed that this time around to take more control of validation of DSC >> 1.2 and ofcourse decided to break up of series into the least amount of >> functionality needed to keep the DPU driver intact. > > Really glad that you are able to test and validate it now, that goes a > long way. Does that also mean you can post the panel patches quickly, > so that everyone has a point of reference? As you said that is one of > the main points where DSC 1.1 "went wrong" (a misunderstanding of > drm_dsc_config). > >> All that being said, we still value your comments and would gladly wait >> for a couple of days like I already wrote. But there are more >> incremental series on top of this: >> >> -> DSI changes for DSC 1.2 >> -> proper teardown for DSC >> -> DSC pair allocation support >> -> DSC 1.2 over DP > > Yeah, I'm familiar with the concept of having many dependent series, and > now DSC series are even rebasing on DPU (catalog) cleanups to preempt > conflicts, which is really hard to follow. > Dmitry just pushed v5 of "drm/i915/dsc: change DSC param tables to > follow the DSC model" [1] and seems to have dropped some patches that > some of these series are depending on, resulting in compilation > failures. Other series don't seem to fully mention all their > dependencies. We'd have to pick them into our code or submit directly to drm-misc. I removed the patches which we can get in w/o Intel review. > > [1]: https://lore.kernel.org/linux-arm-msm/20230504153511.4007320-1-dmitry.baryshkov@linaro.org/T/#u > > So, for this to go as convenient as possible, do you have a list of > series, in a desired order that they should be reviewed and tested? > That way I can direct my priorities and help achieve the goal of picking > base dependencies as early as possible. > > For example, one of the many series regresses DSC on the Xperia XZ3 > (SDM845), but I have yet to bisect and understand which patch it is. > Will let you know as soon as I get my tree in order. > > - Marijn
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h index 521cfd5..ef92545 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = { }; static const struct dpu_pingpong_cfg msm8998_pp[] = { - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h index b109757..697fbd8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h @@ -110,17 +110,17 @@ static const struct dpu_lm_cfg sdm845_lm[] = { }; static const struct dpu_pingpong_cfg sdm845_pp[] = { - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)), - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC), + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h index 30aff2b..cb117ca 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h @@ -128,23 +128,23 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = { }; static const struct dpu_pingpong_cfg sm8150_pp[] = { - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), -1), - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), -1), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), -1), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), -1), - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), -1), - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), -1), }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h index fec1665..27eda6a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h @@ -116,23 +116,23 @@ static const struct dpu_lm_cfg sc8180x_lm[] = { }; static const struct dpu_pingpong_cfg sc8180x_pp[] = { - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), -1), - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), -1), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), -1), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), -1), - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), -1), - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), -1), }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h index 37716b8..70fdd4d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h @@ -129,23 +129,23 @@ static const struct dpu_dspp_cfg sm8250_dspp[] = { }; static const struct dpu_pingpong_cfg sm8250_pp[] = { - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), -1), - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), -1), - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), -1), - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), -1), - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), -1), - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk, - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC), + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), -1), };
At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate pingpong ops functions are required to complete DSC data path setup if this chipset has DSC hardware block presented. This patch add DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC hardware block presented. Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> --- .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++-------- .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++-------- .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++----------- .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++----------- .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++----------- 5 files changed, 52 insertions(+), 52 deletions(-)