| Message ID | 20221231215006.211860-6-marijn.suijten@somainline.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dpu: Implement tearcheck support on INTF block | expand |
On 31/12/2022 23:50, Marijn Suijten wrote: > All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, > but excluding 7.x.x) have the tear interrupt and control registers moved > out of the PINGPONG block and into the INTF block. Wire up the > necessary interrupts and IRQ masks on all supported hardware. > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++-------- > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- > .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ > .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + > 5 files changed, 68 insertions(+), 33 deletions(-) > > 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 1cfe94494135..b9b9b5b0b615 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > @@ -86,6 +86,15 @@ > > #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) > > +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > + BIT(MDP_SSPP_TOP0_INTR2) | \ > + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > + BIT(MDP_INTF0_INTR) | \ > + BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF2_INTR) | \ > + BIT(MDP_INTF3_INTR) | \ > + BIT(MDP_INTF4_INTR)) > + > #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > @@ -100,13 +109,15 @@ > #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > - BIT(MDP_INTF1_INTR)) > + BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR)) > > #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > BIT(MDP_INTF0_INTR) | \ > - BIT(MDP_INTF1_INTR)) > + BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR)) > > #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > @@ -120,7 +131,9 @@ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > BIT(MDP_INTF0_INTR) | \ > BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR) | \ > BIT(MDP_INTF2_INTR) | \ > + BIT(MDP_INTF2_TEAR_INTR) | \ > BIT(MDP_INTF3_INTR) | \ > BIT(MDP_INTF4_INTR)) > > @@ -129,7 +142,9 @@ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > BIT(MDP_INTF0_INTR) | \ > BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR) | \ > BIT(MDP_INTF2_INTR) | \ > + BIT(MDP_INTF2_TEAR_INTR) | \ > BIT(MDP_INTF3_INTR) | \ > BIT(MDP_INTF4_INTR) | \ > BIT(MDP_INTF5_INTR) | \ > @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = { > /************************************************************* > * INTF sub blocks config > *************************************************************/ > -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit) \ > +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, _tear_rd_ptr_bit) \ > {\ > .name = _name, .id = _id, \ > - .base = _base, .len = 0x280, \ > + .base = _base, .len = _len, \ Please move .len setting to a separate patch, it is not direclty related to tear interrupt addition. > .features = _features, \ > .type = _type, \ > .controller_id = _ctrl_id, \ > .prog_fetch_lines_worst_case = _progfetch, \ > .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ > .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ > + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ Initially I added separate _reg and _bit settings because reg was common to both interrupts. However now as tear interrups use different reg it might be better to first move DPU_IRQ_IDX out of this macro () and then to add your tear_rd_ptr_intr as a single intr_idx. > } > > static const struct dpu_intf_cfg msm8998_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x268, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27, -1, -1), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x268, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29, -1, -1), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x268, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > }; > > static const struct dpu_intf_cfg sdm845_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x280, INTF_DSI, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27, -1, -1), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x280, INTF_DSI, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29, -1, -1), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > }; > > static const struct dpu_intf_cfg sc7180_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > }; > > static const struct dpu_intf_cfg sm8150_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x2b8, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29, MDP_INTF2_TEAR_INTR, 2), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > }; > > static const struct dpu_intf_cfg sc7280_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x34000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x35000, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23), > + INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x35000, 0x2b8, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > + INTF_BLK("intf_5", INTF_5, 0x39000, 0x280, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23, -1, -1), > }; > > static const struct dpu_intf_cfg sc8180x_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x2b8, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29, MDP_INTF2_TEAR_INTR, 2), > /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until this is supported */ > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > - INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 20, 21), > - INTF_BLK("intf_5", INTF_5, 0x6C800, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 22, 23), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 999, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > + INTF_BLK("intf_4", INTF_4, 0x6C000, 0x280, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 20, 21, -1, -1), > + INTF_BLK("intf_5", INTF_5, 0x6C800, 0x280, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 22, 23, -1, -1), > }; > > static const struct dpu_intf_cfg qcm2290_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x00000, INTF_NONE, 0, 0, 0, 0, 0, 0), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > + INTF_BLK("intf_0", INTF_0, 0x00000, 0x2b8, INTF_NONE, 0, 0, 0, 0, 0, 0, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF2_TEAR_INTR, 2), > }; > > /************************************************************* > @@ -1849,7 +1865,7 @@ static const struct dpu_mdss_cfg msm8998_dpu_cfg = { > .vbif = msm8998_vbif, > .reg_dma_count = 0, > .perf = &msm8998_perf_data, > - .mdss_irqs = IRQ_SM8250_MASK, > + .mdss_irqs = IRQ_MSM8998_MASK, > }; > > static const struct dpu_mdss_cfg sdm845_dpu_cfg = { > @@ -1947,7 +1963,7 @@ static const struct dpu_mdss_cfg sm8150_dpu_cfg = { > .reg_dma_count = 1, > .dma_cfg = &sm8150_regdma, > .perf = &sm8150_perf_data, > - .mdss_irqs = IRQ_SDM845_MASK, > + .mdss_irqs = IRQ_SM8250_MASK, > }; > > static const struct dpu_mdss_cfg sc8180x_dpu_cfg = { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > index e0e153889ab7..2ea17e4ef3e5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -638,8 +638,9 @@ struct dpu_dsc_cfg { > * @type: Interface type(DSI, DP, HDMI) > * @controller_id: Controller Instance ID in case of multiple of intf type > * @prog_fetch_lines_worst_case Worst case latency num lines needed to prefetch > - * @intr_underrun: index for INTF underrun interrupt > - * @intr_vsync: index for INTF VSYNC interrupt > + * @intr_underrun: index for INTF underrun interrupt > + * @intr_vsync: index for INTF VSYNC interrupt > + * @intr_tear_rd_ptr: index for INTF TEAR_RD_PTR interrupt > */ > struct dpu_intf_cfg { > DPU_HW_BLK_INFO; > @@ -648,6 +649,7 @@ struct dpu_intf_cfg { > u32 prog_fetch_lines_worst_case; > s32 intr_underrun; > s32 intr_vsync; > + s32 intr_tear_rd_ptr; > }; > > /** > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > index cf1b6d84c18a..044136a97fac 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > @@ -24,6 +24,8 @@ > #define MDP_INTF_3_OFF 0x6B800 > #define MDP_INTF_4_OFF 0x6C000 > #define MDP_INTF_5_OFF 0x6C800 > +#define MDP_INTF_1_TEAR_OFF 0x6D800 > +#define MDP_INTF_2_TEAR_OFF 0x6D900 > #define MDP_AD4_0_OFF 0x7C000 > #define MDP_AD4_1_OFF 0x7D000 > #define MDP_AD4_INTR_EN_OFF 0x41c > @@ -99,6 +101,16 @@ static const struct dpu_intr_reg dpu_intr_set[] = { > MDP_INTF_5_OFF+INTF_INTR_EN, > MDP_INTF_5_OFF+INTF_INTR_STATUS > }, > + [MDP_INTF1_TEAR_INTR] = { > + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_CLEAR, > + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_EN, > + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_STATUS > + }, > + [MDP_INTF2_TEAR_INTR] = { > + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_CLEAR, > + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_EN, > + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_STATUS > + }, > [MDP_AD4_0_INTR] = { > MDP_AD4_0_OFF + MDP_AD4_INTR_CLEAR_OFF, > MDP_AD4_0_OFF + MDP_AD4_INTR_EN_OFF, > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > index 46443955443c..b447e4a1d9f4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > @@ -23,6 +23,8 @@ enum dpu_hw_intr_reg { > MDP_INTF3_INTR, > MDP_INTF4_INTR, > MDP_INTF5_INTR, > + MDP_INTF1_TEAR_INTR, > + MDP_INTF2_TEAR_INTR, > MDP_AD4_0_INTR, > MDP_AD4_1_INTR, > MDP_INTF0_7xxx_INTR, > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h > index c8156ed4b7fb..6bdac515fd54 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h > @@ -10,6 +10,9 @@ > /** > * MDP TOP block Register and bit fields and defines > */ > +#define INTF_INTR_TEAR_EN 0x000 > +#define INTF_INTR_TEAR_STATUS 0x004 > +#define INTF_INTR_TEAR_CLEAR 0x008 > #define DISP_INTF_SEL 0x004 > #define INTR_EN 0x010 > #define INTR_STATUS 0x014
On 2023-01-01 15:12:35, Dmitry Baryshkov wrote: > On 31/12/2022 23:50, Marijn Suijten wrote: > > <snip> > > -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit) \ > > +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, _tear_rd_ptr_bit) \ > > {\ > > .name = _name, .id = _id, \ > > - .base = _base, .len = 0x280, \ > > + .base = _base, .len = _len, \ > > Please move .len setting to a separate patch, it is not direclty related > to tear interrupt addition. It is directly related in that the TE registers reside in the extra space beyond 0x280, but I can surely make that explicit in a separate patch. > > .features = _features, \ > > .type = _type, \ > > .controller_id = _ctrl_id, \ > > .prog_fetch_lines_worst_case = _progfetch, \ > > .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ > > .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ > > + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ > > Initially I added separate _reg and _bit settings because reg was common > to both interrupts. However now as tear interrups use different reg it > might be better to first move DPU_IRQ_IDX out of this macro () and then > to add your tear_rd_ptr_intr as a single intr_idx. I assumed as much; then we do get the duplication of _reg but I guess it's not too bad if the lines are nicely wrapped like in _pp[]. I'll do so in a separate patch. - Marijn <snip>
On 12/31/2022 1:50 PM, Marijn Suijten wrote: > All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, > but excluding 7.x.x) have the tear interrupt and control registers moved > out of the PINGPONG block and into the INTF block. Wire up the > necessary interrupts and IRQ masks on all supported hardware. Hi Marijn, Thanks for the patch. I saw that in your commit msg, you mentioned that 7.x doesn't have tearcheck in the INTF block -- can you double check that this is correct? I'm working on SM8350 (DPU v7) and I'm seeing that it does have tearcheck in INTF block. > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++-------- > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- > .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ > .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + > 5 files changed, 68 insertions(+), 33 deletions(-) > > 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 1cfe94494135..b9b9b5b0b615 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > @@ -86,6 +86,15 @@ > > #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) > > +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > + BIT(MDP_SSPP_TOP0_INTR2) | \ > + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > + BIT(MDP_INTF0_INTR) | \ > + BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF2_INTR) | \ > + BIT(MDP_INTF3_INTR) | \ > + BIT(MDP_INTF4_INTR)) > + > #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > @@ -100,13 +109,15 @@ > #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > - BIT(MDP_INTF1_INTR)) > + BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR)) > > #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > BIT(MDP_INTF0_INTR) | \ > - BIT(MDP_INTF1_INTR)) > + BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR)) > > #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > BIT(MDP_SSPP_TOP0_INTR2) | \ > @@ -120,7 +131,9 @@ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > BIT(MDP_INTF0_INTR) | \ > BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR) | \ > BIT(MDP_INTF2_INTR) | \ > + BIT(MDP_INTF2_TEAR_INTR) | \ > BIT(MDP_INTF3_INTR) | \ > BIT(MDP_INTF4_INTR)) > > @@ -129,7 +142,9 @@ > BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > BIT(MDP_INTF0_INTR) | \ > BIT(MDP_INTF1_INTR) | \ > + BIT(MDP_INTF1_TEAR_INTR) | \ > BIT(MDP_INTF2_INTR) | \ > + BIT(MDP_INTF2_TEAR_INTR) | \ > BIT(MDP_INTF3_INTR) | \ > BIT(MDP_INTF4_INTR) | \ > BIT(MDP_INTF5_INTR) | \ > @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = { > /************************************************************* > * INTF sub blocks config > *************************************************************/ > -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit) \ > +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, _tear_rd_ptr_bit) \ > {\ > .name = _name, .id = _id, \ > - .base = _base, .len = 0x280, \ > + .base = _base, .len = _len, \ > .features = _features, \ > .type = _type, \ > .controller_id = _ctrl_id, \ > .prog_fetch_lines_worst_case = _progfetch, \ > .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ > .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ > + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ > } > > static const struct dpu_intf_cfg msm8998_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), Just wondering, how were the lengths calculated for the INTF blocks? The values in general seem a little off to me. For example, I'm looking downstream and it seems to me that the length for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm seeing that length for INTF + tearcheck should be 0x2c4. Thanks, Jessica Zhang > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x268, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27, -1, -1), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x268, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29, -1, -1), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x268, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > }; > > static const struct dpu_intf_cfg sdm845_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x280, INTF_DSI, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27, -1, -1), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x280, INTF_DSI, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29, -1, -1), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > }; > > static const struct dpu_intf_cfg sc7180_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > }; > > static const struct dpu_intf_cfg sm8150_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x2b8, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29, MDP_INTF2_TEAR_INTR, 2), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > }; > > static const struct dpu_intf_cfg sc7280_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x34000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x35000, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23), > + INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x35000, 0x2b8, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > + INTF_BLK("intf_5", INTF_5, 0x39000, 0x280, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23, -1, -1), > }; > > static const struct dpu_intf_cfg sc8180x_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), > + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x2b8, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29, MDP_INTF2_TEAR_INTR, 2), > /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until this is supported */ > - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > - INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 20, 21), > - INTF_BLK("intf_5", INTF_5, 0x6C800, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 22, 23), > + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 999, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), > + INTF_BLK("intf_4", INTF_4, 0x6C000, 0x280, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 20, 21, -1, -1), > + INTF_BLK("intf_5", INTF_5, 0x6C800, 0x280, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 22, 23, -1, -1), > }; > > static const struct dpu_intf_cfg qcm2290_intf[] = { > - INTF_BLK("intf_0", INTF_0, 0x00000, INTF_NONE, 0, 0, 0, 0, 0, 0), > - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > + INTF_BLK("intf_0", INTF_0, 0x00000, 0x2b8, INTF_NONE, 0, 0, 0, 0, 0, 0, -1, -1), > + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF2_TEAR_INTR, 2), > }; > > /************************************************************* > @@ -1849,7 +1865,7 @@ static const struct dpu_mdss_cfg msm8998_dpu_cfg = { > .vbif = msm8998_vbif, > .reg_dma_count = 0, > .perf = &msm8998_perf_data, > - .mdss_irqs = IRQ_SM8250_MASK, > + .mdss_irqs = IRQ_MSM8998_MASK, > }; > > static const struct dpu_mdss_cfg sdm845_dpu_cfg = { > @@ -1947,7 +1963,7 @@ static const struct dpu_mdss_cfg sm8150_dpu_cfg = { > .reg_dma_count = 1, > .dma_cfg = &sm8150_regdma, > .perf = &sm8150_perf_data, > - .mdss_irqs = IRQ_SDM845_MASK, > + .mdss_irqs = IRQ_SM8250_MASK, > }; > > static const struct dpu_mdss_cfg sc8180x_dpu_cfg = { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > index e0e153889ab7..2ea17e4ef3e5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -638,8 +638,9 @@ struct dpu_dsc_cfg { > * @type: Interface type(DSI, DP, HDMI) > * @controller_id: Controller Instance ID in case of multiple of intf type > * @prog_fetch_lines_worst_case Worst case latency num lines needed to prefetch > - * @intr_underrun: index for INTF underrun interrupt > - * @intr_vsync: index for INTF VSYNC interrupt > + * @intr_underrun: index for INTF underrun interrupt > + * @intr_vsync: index for INTF VSYNC interrupt > + * @intr_tear_rd_ptr: index for INTF TEAR_RD_PTR interrupt > */ > struct dpu_intf_cfg { > DPU_HW_BLK_INFO; > @@ -648,6 +649,7 @@ struct dpu_intf_cfg { > u32 prog_fetch_lines_worst_case; > s32 intr_underrun; > s32 intr_vsync; > + s32 intr_tear_rd_ptr; > }; > > /** > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > index cf1b6d84c18a..044136a97fac 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > @@ -24,6 +24,8 @@ > #define MDP_INTF_3_OFF 0x6B800 > #define MDP_INTF_4_OFF 0x6C000 > #define MDP_INTF_5_OFF 0x6C800 > +#define MDP_INTF_1_TEAR_OFF 0x6D800 > +#define MDP_INTF_2_TEAR_OFF 0x6D900 > #define MDP_AD4_0_OFF 0x7C000 > #define MDP_AD4_1_OFF 0x7D000 > #define MDP_AD4_INTR_EN_OFF 0x41c > @@ -99,6 +101,16 @@ static const struct dpu_intr_reg dpu_intr_set[] = { > MDP_INTF_5_OFF+INTF_INTR_EN, > MDP_INTF_5_OFF+INTF_INTR_STATUS > }, > + [MDP_INTF1_TEAR_INTR] = { > + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_CLEAR, > + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_EN, > + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_STATUS > + }, > + [MDP_INTF2_TEAR_INTR] = { > + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_CLEAR, > + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_EN, > + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_STATUS > + }, > [MDP_AD4_0_INTR] = { > MDP_AD4_0_OFF + MDP_AD4_INTR_CLEAR_OFF, > MDP_AD4_0_OFF + MDP_AD4_INTR_EN_OFF, > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > index 46443955443c..b447e4a1d9f4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h > @@ -23,6 +23,8 @@ enum dpu_hw_intr_reg { > MDP_INTF3_INTR, > MDP_INTF4_INTR, > MDP_INTF5_INTR, > + MDP_INTF1_TEAR_INTR, > + MDP_INTF2_TEAR_INTR, > MDP_AD4_0_INTR, > MDP_AD4_1_INTR, > MDP_INTF0_7xxx_INTR, > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h > index c8156ed4b7fb..6bdac515fd54 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h > @@ -10,6 +10,9 @@ > /** > * MDP TOP block Register and bit fields and defines > */ > +#define INTF_INTR_TEAR_EN 0x000 > +#define INTF_INTR_TEAR_STATUS 0x004 > +#define INTF_INTR_TEAR_CLEAR 0x008 > #define DISP_INTF_SEL 0x004 > #define INTR_EN 0x010 > #define INTR_STATUS 0x014 > -- > 2.39.0 >
On 13/02/2023 21:37, Jessica Zhang wrote: > > > On 12/31/2022 1:50 PM, Marijn Suijten wrote: >> All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, >> but excluding 7.x.x) have the tear interrupt and control registers moved >> out of the PINGPONG block and into the INTF block. Wire up the >> necessary interrupts and IRQ masks on all supported hardware. > > Hi Marijn, > > Thanks for the patch. > > I saw that in your commit msg, you mentioned that 7.x doesn't have > tearcheck in the INTF block -- can you double check that this is correct? > > I'm working on SM8350 (DPU v7) and I'm seeing that it does have > tearcheck in INTF block. I confirm, according to the vendor drivers INTF TE should be used for all DPU >= 5.0, including 7.x and 8.x However I think I know what Marijn meant here. For 5.x and 6.x these IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU 7.x these IRQ registers were moved close to the main INTF block (0x36800 and 0x37800 = INTF + 0x800). > >> >> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> >> --- >> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++-------- >> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- >> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ >> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + >> 5 files changed, 68 insertions(+), 33 deletions(-) >> >> 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 1cfe94494135..b9b9b5b0b615 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> @@ -86,6 +86,15 @@ >> #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) >> +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >> + BIT(MDP_SSPP_TOP0_INTR2) | \ >> + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >> + BIT(MDP_INTF0_INTR) | \ >> + BIT(MDP_INTF1_INTR) | \ >> + BIT(MDP_INTF2_INTR) | \ >> + BIT(MDP_INTF3_INTR) | \ >> + BIT(MDP_INTF4_INTR)) >> + >> #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >> BIT(MDP_SSPP_TOP0_INTR2) | \ >> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >> @@ -100,13 +109,15 @@ >> #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >> BIT(MDP_SSPP_TOP0_INTR2) | \ >> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >> - BIT(MDP_INTF1_INTR)) >> + BIT(MDP_INTF1_INTR) | \ >> + BIT(MDP_INTF1_TEAR_INTR)) >> #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >> BIT(MDP_SSPP_TOP0_INTR2) | \ >> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >> BIT(MDP_INTF0_INTR) | \ >> - BIT(MDP_INTF1_INTR)) >> + BIT(MDP_INTF1_INTR) | \ >> + BIT(MDP_INTF1_TEAR_INTR)) >> #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >> BIT(MDP_SSPP_TOP0_INTR2) | \ >> @@ -120,7 +131,9 @@ >> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >> BIT(MDP_INTF0_INTR) | \ >> BIT(MDP_INTF1_INTR) | \ >> + BIT(MDP_INTF1_TEAR_INTR) | \ >> BIT(MDP_INTF2_INTR) | \ >> + BIT(MDP_INTF2_TEAR_INTR) | \ >> BIT(MDP_INTF3_INTR) | \ >> BIT(MDP_INTF4_INTR)) >> @@ -129,7 +142,9 @@ >> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >> BIT(MDP_INTF0_INTR) | \ >> BIT(MDP_INTF1_INTR) | \ >> + BIT(MDP_INTF1_TEAR_INTR) | \ >> BIT(MDP_INTF2_INTR) | \ >> + BIT(MDP_INTF2_TEAR_INTR) | \ >> BIT(MDP_INTF3_INTR) | \ >> BIT(MDP_INTF4_INTR) | \ >> BIT(MDP_INTF5_INTR) | \ >> @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = { >> /************************************************************* >> * INTF sub blocks config >> *************************************************************/ >> -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, >> _features, _reg, _underrun_bit, _vsync_bit) \ >> +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, >> _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, >> _tear_rd_ptr_bit) \ >> {\ >> .name = _name, .id = _id, \ >> - .base = _base, .len = 0x280, \ >> + .base = _base, .len = _len, \ >> .features = _features, \ >> .type = _type, \ >> .controller_id = _ctrl_id, \ >> .prog_fetch_lines_worst_case = _progfetch, \ >> .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ >> .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ >> + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ >> } >> static const struct dpu_intf_cfg msm8998_intf[] = { >> - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, >> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), >> - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, >> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), >> - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, >> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), >> - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, >> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), >> + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, >> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > > Just wondering, how were the lengths calculated for the INTF blocks? The > values in general seem a little off to me. > > For example, I'm looking downstream and it seems to me that the length > for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm > seeing that length for INTF + tearcheck should be 0x2c4. We have discussed INTF lengths in [1]. The current understanding of the block lengths can be found at [2]. Please comment there if any of the fixed lengths sounds incorrect to you. [1] https://patchwork.freedesktop.org/patch/522187/ [2] https://patchwork.freedesktop.org/patch/522227/ [skipped the rest]
On 2/13/2023 1:46 PM, Dmitry Baryshkov wrote: > On 13/02/2023 21:37, Jessica Zhang wrote: >> >> >> On 12/31/2022 1:50 PM, Marijn Suijten wrote: >>> All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, >>> but excluding 7.x.x) have the tear interrupt and control registers moved >>> out of the PINGPONG block and into the INTF block. Wire up the >>> necessary interrupts and IRQ masks on all supported hardware. >> >> Hi Marijn, >> >> Thanks for the patch. >> >> I saw that in your commit msg, you mentioned that 7.x doesn't have >> tearcheck in the INTF block -- can you double check that this is correct? >> >> I'm working on SM8350 (DPU v7) and I'm seeing that it does have >> tearcheck in INTF block. > > I confirm, according to the vendor drivers INTF TE should be used for > all DPU >= 5.0, including 7.x and 8.x > > However I think I know what Marijn meant here. For 5.x and 6.x these > IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means > offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU > 7.x these IRQ registers were moved close to the main INTF block (0x36800 > and 0x37800 = INTF + 0x800). > Got it, then the commit text should remove "control" and just say tear interrupt registers. It got a bit confusing. We will add the 7xxx intf tear check support on top of this series. >> >>> >>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> >>> --- >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++-------- >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + >>> 5 files changed, 68 insertions(+), 33 deletions(-) >>> >>> 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 1cfe94494135..b9b9b5b0b615 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>> @@ -86,6 +86,15 @@ >>> #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) >>> +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>> + BIT(MDP_SSPP_TOP0_INTR2) | \ >>> + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>> + BIT(MDP_INTF0_INTR) | \ >>> + BIT(MDP_INTF1_INTR) | \ >>> + BIT(MDP_INTF2_INTR) | \ >>> + BIT(MDP_INTF3_INTR) | \ >>> + BIT(MDP_INTF4_INTR)) >>> + >>> #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>> @@ -100,13 +109,15 @@ >>> #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>> - BIT(MDP_INTF1_INTR)) >>> + BIT(MDP_INTF1_INTR) | \ >>> + BIT(MDP_INTF1_TEAR_INTR)) >>> #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>> BIT(MDP_INTF0_INTR) | \ >>> - BIT(MDP_INTF1_INTR)) >>> + BIT(MDP_INTF1_INTR) | \ >>> + BIT(MDP_INTF1_TEAR_INTR)) >>> #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>> @@ -120,7 +131,9 @@ >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>> BIT(MDP_INTF0_INTR) | \ >>> BIT(MDP_INTF1_INTR) | \ >>> + BIT(MDP_INTF1_TEAR_INTR) | \ >>> BIT(MDP_INTF2_INTR) | \ >>> + BIT(MDP_INTF2_TEAR_INTR) | \ >>> BIT(MDP_INTF3_INTR) | \ >>> BIT(MDP_INTF4_INTR)) >>> @@ -129,7 +142,9 @@ >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>> BIT(MDP_INTF0_INTR) | \ >>> BIT(MDP_INTF1_INTR) | \ >>> + BIT(MDP_INTF1_TEAR_INTR) | \ >>> BIT(MDP_INTF2_INTR) | \ >>> + BIT(MDP_INTF2_TEAR_INTR) | \ >>> BIT(MDP_INTF3_INTR) | \ >>> BIT(MDP_INTF4_INTR) | \ >>> BIT(MDP_INTF5_INTR) | \ >>> @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = { >>> /************************************************************* >>> * INTF sub blocks config >>> *************************************************************/ >>> -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, >>> _features, _reg, _underrun_bit, _vsync_bit) \ >>> +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, >>> _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, >>> _tear_rd_ptr_bit) \ >>> {\ >>> .name = _name, .id = _id, \ >>> - .base = _base, .len = 0x280, \ >>> + .base = _base, .len = _len, \ >>> .features = _features, \ >>> .type = _type, \ >>> .controller_id = _ctrl_id, \ >>> .prog_fetch_lines_worst_case = _progfetch, \ >>> .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ >>> .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ >>> + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ >>> } >>> static const struct dpu_intf_cfg msm8998_intf[] = { >>> - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), >>> - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), >>> - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), >>> - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), >>> + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), >> >> Just wondering, how were the lengths calculated for the INTF blocks? >> The values in general seem a little off to me. >> >> For example, I'm looking downstream and it seems to me that the length >> for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm >> seeing that length for INTF + tearcheck should be 0x2c4. > > We have discussed INTF lengths in [1]. The current understanding of the > block lengths can be found at [2]. Please comment there if any of the > fixed lengths sounds incorrect to you. > > [1] https://patchwork.freedesktop.org/patch/522187/ > [2] https://patchwork.freedesktop.org/patch/522227/ > > [skipped the rest] > Please correct my understanding here, it was agreed to fix intf blocks to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont see this was merged? It was agreed to first land INTF_TE and then add the higher addresses but I dont see such a change, am i missing something?
On 2023-02-13 19:09:32, Abhinav Kumar wrote: > > > On 2/13/2023 1:46 PM, Dmitry Baryshkov wrote: > > On 13/02/2023 21:37, Jessica Zhang wrote: > >> > >> > >> On 12/31/2022 1:50 PM, Marijn Suijten wrote: > >>> All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, > >>> but excluding 7.x.x) have the tear interrupt and control registers moved > >>> out of the PINGPONG block and into the INTF block. Wire up the > >>> necessary interrupts and IRQ masks on all supported hardware. > >> > >> Hi Marijn, > >> > >> Thanks for the patch. > >> > >> I saw that in your commit msg, you mentioned that 7.x doesn't have > >> tearcheck in the INTF block -- can you double check that this is correct? It wasn't correct and has already been removed for v2 [1] after rebasing on top of SM8[345]50 support, where the registers reside at a different (named 7xxxx downstream) offset. [1] https://github.com/SoMainline/linux/commit/886d3fb9eed925e7e9c8d6ca63d2439eaec1c702 > >> I'm working on SM8350 (DPU v7) and I'm seeing that it does have > >> tearcheck in INTF block. > > > > I confirm, according to the vendor drivers INTF TE should be used for > > all DPU >= 5.0, including 7.x and 8.x > > > > However I think I know what Marijn meant here. For 5.x and 6.x these > > IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means > > offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU > > 7.x these IRQ registers were moved close to the main INTF block (0x36800 > > and 0x37800 = INTF + 0x800). That might have been the case. > Got it, then the commit text should remove "control" and just say tear > interrupt registers. It got a bit confusing. The wording here points to both the interrupt (MDP_INTFx_TEAR_INTR) registers and control (INTF_TEAR_xxx) registers separately. Feel free to bikeshed the wording in preliminary v2 [1]; should I drop the mention of the control registers being "moved" from PP to INTF entirely, leaving just the wording about the interrupt registers moving from MDP_SSPP_TOP0_INTR to a dedicated MDP_INTFx_TEAR_INTR region? > We will add the 7xxx intf tear check support on top of this series. No need, that is already taken care of in an impending v2 [1] (unless additional changes are required beyond the moved register offset). > >>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > >>> --- > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++-------- > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + > >>> 5 files changed, 68 insertions(+), 33 deletions(-) > >>> > >>> 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 1cfe94494135..b9b9b5b0b615 100644 > >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>> @@ -86,6 +86,15 @@ > >>> #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) > >>> +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> + BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> + BIT(MDP_INTF0_INTR) | \ > >>> + BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF2_INTR) | \ > >>> + BIT(MDP_INTF3_INTR) | \ > >>> + BIT(MDP_INTF4_INTR)) > >>> + > >>> #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> @@ -100,13 +109,15 @@ > >>> #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> - BIT(MDP_INTF1_INTR)) > >>> + BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF1_TEAR_INTR)) > >>> #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> BIT(MDP_INTF0_INTR) | \ > >>> - BIT(MDP_INTF1_INTR)) > >>> + BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF1_TEAR_INTR)) > >>> #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> @@ -120,7 +131,9 @@ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> BIT(MDP_INTF0_INTR) | \ > >>> BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF1_TEAR_INTR) | \ > >>> BIT(MDP_INTF2_INTR) | \ > >>> + BIT(MDP_INTF2_TEAR_INTR) | \ > >>> BIT(MDP_INTF3_INTR) | \ > >>> BIT(MDP_INTF4_INTR)) > >>> @@ -129,7 +142,9 @@ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> BIT(MDP_INTF0_INTR) | \ > >>> BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF1_TEAR_INTR) | \ > >>> BIT(MDP_INTF2_INTR) | \ > >>> + BIT(MDP_INTF2_TEAR_INTR) | \ > >>> BIT(MDP_INTF3_INTR) | \ > >>> BIT(MDP_INTF4_INTR) | \ > >>> BIT(MDP_INTF5_INTR) | \ > >>> @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = { > >>> /************************************************************* > >>> * INTF sub blocks config > >>> *************************************************************/ > >>> -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, > >>> _features, _reg, _underrun_bit, _vsync_bit) \ > >>> +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, > >>> _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, > >>> _tear_rd_ptr_bit) \ > >>> {\ > >>> .name = _name, .id = _id, \ > >>> - .base = _base, .len = 0x280, \ > >>> + .base = _base, .len = _len, \ > >>> .features = _features, \ > >>> .type = _type, \ > >>> .controller_id = _ctrl_id, \ > >>> .prog_fetch_lines_worst_case = _progfetch, \ > >>> .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ > >>> .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ > >>> + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ > >>> } > >>> static const struct dpu_intf_cfg msm8998_intf[] = { > >>> - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, > >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), > >>> - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, > >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), > >>> - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, > >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), > >>> - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, > >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), > >>> + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, > >>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), > >> > >> Just wondering, how were the lengths calculated for the INTF blocks? > >> The values in general seem a little off to me. These (for MSM8998) have been taken from downstream specifically; my series starts using INTF_STATUS at 0x26C which conveniently is the register right after 0x268, matching the fact that INTF TE and these registers weren't supported/available yet on MSM8998. > >> For example, I'm looking downstream and it seems to me that the length > >> for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm > >> seeing that length for INTF + tearcheck should be 0x2c4. There are many different downstream sources and tags with seemingly conflicting/confusing information. For v2 [2] I've picked the highest register used by the driver which is INTF_TEAR_AUTOREFRESH_CONFIG at 0x2B4 (but there might always be more registers that don't need to be poked at by the driver, but contain magic debug information and the like... those would be useful to capture in the dump going forward). [2]: https://github.com/SoMainline/linux/commit/2bbc609dd28aa0bd0a2dede20163e521912d0072 > > We have discussed INTF lengths in [1]. The current understanding of the > > block lengths can be found at [2]. Please comment there if any of the > > fixed lengths sounds incorrect to you. > > > > [1] https://patchwork.freedesktop.org/patch/522187/ > > [2] https://patchwork.freedesktop.org/patch/522227/ > > > > [skipped the rest] > > > > Please correct my understanding here, it was agreed to fix intf blocks > to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont > see this was merged? > > It was agreed to first land INTF_TE and then add the higher addresses Seems like it, at least if I interpret [3] correctly. My series adds a new define that will hardcode _len to 0x2B8 for now, and Dmitry/Konrad can later extend it to whatever is stated by the correct downstream source. [3]: https://lore.kernel.org/linux-arm-msm/6ad96cff-b91b-a4c7-4573-7bb8de7194f8@linaro.org/ > but I dont see such a change, am i missing something? This was discussed just yesterday. And it wouldn't make much sense to make such a change now, knowing that my v2 for this series - which isn't even on the lists yet - will already change the INTF_BLK macro resulting in unneeded conflicts. As requested by Dmitry, let's get INTF TE processed first before rebasing the block length change? - Marijn
On 2/14/2023 5:06 AM, Marijn Suijten wrote: > On 2023-02-13 19:09:32, Abhinav Kumar wrote: >> >> >> On 2/13/2023 1:46 PM, Dmitry Baryshkov wrote: >>> On 13/02/2023 21:37, Jessica Zhang wrote: >>>> >>>> >>>> On 12/31/2022 1:50 PM, Marijn Suijten wrote: >>>>> All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, >>>>> but excluding 7.x.x) have the tear interrupt and control registers moved >>>>> out of the PINGPONG block and into the INTF block. Wire up the >>>>> necessary interrupts and IRQ masks on all supported hardware. >>>> >>>> Hi Marijn, >>>> >>>> Thanks for the patch. >>>> >>>> I saw that in your commit msg, you mentioned that 7.x doesn't have >>>> tearcheck in the INTF block -- can you double check that this is correct? > > It wasn't correct and has already been removed for v2 [1] after rebasing > on top of SM8[345]50 support, where the registers reside at a different > (named 7xxxx downstream) offset. > > [1] https://github.com/SoMainline/linux/commit/886d3fb9eed925e7e9c8d6ca63d2439eaec1c702 > >>>> I'm working on SM8350 (DPU v7) and I'm seeing that it does have >>>> tearcheck in INTF block. >>> >>> I confirm, according to the vendor drivers INTF TE should be used for >>> all DPU >= 5.0, including 7.x and 8.x >>> >>> However I think I know what Marijn meant here. For 5.x and 6.x these >>> IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means >>> offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU >>> 7.x these IRQ registers were moved close to the main INTF block (0x36800 >>> and 0x37800 = INTF + 0x800). > > That might have been the case. > >> Got it, then the commit text should remove "control" and just say tear >> interrupt registers. It got a bit confusing. > > The wording here points to both the interrupt (MDP_INTFx_TEAR_INTR) > registers and control (INTF_TEAR_xxx) registers separately. Feel free > to bikeshed the wording in preliminary v2 [1]; should I drop the mention > of the control registers being "moved" from PP to INTF entirely, leaving > just the wording about the interrupt registers moving from > MDP_SSPP_TOP0_INTR to a dedicated MDP_INTFx_TEAR_INTR region? Yes, that makes more sense to me. Drop the mention on control registers. > >> We will add the 7xxx intf tear check support on top of this series. > > No need, that is already taken care of in an impending v2 [1] (unless > additional changes are required beyond the moved register offset). > Ok, we will wait till you post v2 and see if that works for us without any of our local changes. >>>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> >>>>> --- >>>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++-------- >>>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- >>>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ >>>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + >>>>> 5 files changed, 68 insertions(+), 33 deletions(-) >>>>> >>>>> 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 1cfe94494135..b9b9b5b0b615 100644 >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>>> @@ -86,6 +86,15 @@ >>>>> #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) >>>>> +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>>>> + BIT(MDP_SSPP_TOP0_INTR2) | \ >>>>> + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>>>> + BIT(MDP_INTF0_INTR) | \ >>>>> + BIT(MDP_INTF1_INTR) | \ >>>>> + BIT(MDP_INTF2_INTR) | \ >>>>> + BIT(MDP_INTF3_INTR) | \ >>>>> + BIT(MDP_INTF4_INTR)) >>>>> + >>>>> #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>>>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>>>> @@ -100,13 +109,15 @@ >>>>> #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>>>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>>>> - BIT(MDP_INTF1_INTR)) >>>>> + BIT(MDP_INTF1_INTR) | \ >>>>> + BIT(MDP_INTF1_TEAR_INTR)) >>>>> #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>>>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>>>> BIT(MDP_INTF0_INTR) | \ >>>>> - BIT(MDP_INTF1_INTR)) >>>>> + BIT(MDP_INTF1_INTR) | \ >>>>> + BIT(MDP_INTF1_TEAR_INTR)) >>>>> #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ >>>>> BIT(MDP_SSPP_TOP0_INTR2) | \ >>>>> @@ -120,7 +131,9 @@ >>>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>>>> BIT(MDP_INTF0_INTR) | \ >>>>> BIT(MDP_INTF1_INTR) | \ >>>>> + BIT(MDP_INTF1_TEAR_INTR) | \ >>>>> BIT(MDP_INTF2_INTR) | \ >>>>> + BIT(MDP_INTF2_TEAR_INTR) | \ >>>>> BIT(MDP_INTF3_INTR) | \ >>>>> BIT(MDP_INTF4_INTR)) >>>>> @@ -129,7 +142,9 @@ >>>>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ >>>>> BIT(MDP_INTF0_INTR) | \ >>>>> BIT(MDP_INTF1_INTR) | \ >>>>> + BIT(MDP_INTF1_TEAR_INTR) | \ >>>>> BIT(MDP_INTF2_INTR) | \ >>>>> + BIT(MDP_INTF2_TEAR_INTR) | \ >>>>> BIT(MDP_INTF3_INTR) | \ >>>>> BIT(MDP_INTF4_INTR) | \ >>>>> BIT(MDP_INTF5_INTR) | \ >>>>> @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = { >>>>> /************************************************************* >>>>> * INTF sub blocks config >>>>> *************************************************************/ >>>>> -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, >>>>> _features, _reg, _underrun_bit, _vsync_bit) \ >>>>> +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, >>>>> _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, >>>>> _tear_rd_ptr_bit) \ >>>>> {\ >>>>> .name = _name, .id = _id, \ >>>>> - .base = _base, .len = 0x280, \ >>>>> + .base = _base, .len = _len, \ >>>>> .features = _features, \ >>>>> .type = _type, \ >>>>> .controller_id = _ctrl_id, \ >>>>> .prog_fetch_lines_worst_case = _progfetch, \ >>>>> .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ >>>>> .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ >>>>> + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ >>>>> } >>>>> static const struct dpu_intf_cfg msm8998_intf[] = { >>>>> - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, >>>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), >>>>> - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, >>>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), >>>>> - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, >>>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), >>>>> - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, >>>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), >>>>> + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, >>>>> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), >>>> >>>> Just wondering, how were the lengths calculated for the INTF blocks? >>>> The values in general seem a little off to me. > > These (for MSM8998) have been taken from downstream specifically; my > series starts using INTF_STATUS at 0x26C which conveniently is the > register right after 0x268, matching the fact that INTF TE and these > registers weren't supported/available yet on MSM8998. > >>>> For example, I'm looking downstream and it seems to me that the length >>>> for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm >>>> seeing that length for INTF + tearcheck should be 0x2c4. > > There are many different downstream sources and tags with seemingly > conflicting/confusing information. For v2 [2] I've picked the highest > register used by the driver which is INTF_TEAR_AUTOREFRESH_CONFIG at > 0x2B4 (but there might always be more registers that don't need to be > poked at by the driver, but contain magic debug information and the > like... those would be useful to capture in the dump going forward). > > [2]: https://github.com/SoMainline/linux/commit/2bbc609dd28aa0bd0a2dede20163e521912d0072 > Not entirely correct.TEAR_AUTOREFRESH_STATUS is at 0x2c0 for sm8350 and sm8450 as well so 0x2b4 is a bit short. DPU code uses autorefresh status today.Esp after your changes it will use the autorefresh status from intf te which is at offset 0x2c0 >>> We have discussed INTF lengths in [1]. The current understanding of the >>> block lengths can be found at [2]. Please comment there if any of the >>> fixed lengths sounds incorrect to you. >>> >>> [1] https://patchwork.freedesktop.org/patch/522187/ >>> [2] https://patchwork.freedesktop.org/patch/522227/ >>> >>> [skipped the rest] >>> >> >> Please correct my understanding here, it was agreed to fix intf blocks >> to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont >> see this was merged? >> >> It was agreed to first land INTF_TE and then add the higher addresses > > Seems like it, at least if I interpret [3] correctly. My series adds a > new define that will hardcode _len to 0x2B8 for now, and Dmitry/Konrad > can later extend it to whatever is stated by the correct downstream > source. > Like mentioned above it should be 0x2c0 for intf block. If you face any conflicting information in downstream code, you can always check with me on IRC. > [3]: https://lore.kernel.org/linux-arm-msm/6ad96cff-b91b-a4c7-4573-7bb8de7194f8@linaro.org/ > >> but I dont see such a change, am i missing something? > > This was discussed just yesterday. And it wouldn't make much sense to > make such a change now, knowing that my v2 for this series - which isn't > even on the lists yet - will already change the INTF_BLK macro resulting > in unneeded conflicts. As requested by Dmitry, let's get INTF TE > processed first before rebasing the block length change? > Please see above comment on why it should be 0x2c4. > - Marijn
On 2023-02-14 09:54:57, Abhinav Kumar wrote: [..] > >>>> Just wondering, how were the lengths calculated for the INTF blocks? > >>>> The values in general seem a little off to me. > > > > These (for MSM8998) have been taken from downstream specifically; my > > series starts using INTF_STATUS at 0x26C which conveniently is the > > register right after 0x268, matching the fact that INTF TE and these > > registers weren't supported/available yet on MSM8998. > > > >>>> For example, I'm looking downstream and it seems to me that the length > >>>> for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm > >>>> seeing that length for INTF + tearcheck should be 0x2c4. > > > > There are many different downstream sources and tags with seemingly > > conflicting/confusing information. For v2 [2] I've picked the highest > > register used by the driver which is INTF_TEAR_AUTOREFRESH_CONFIG at > > 0x2B4 (but there might always be more registers that don't need to be > > poked at by the driver, but contain magic debug information and the > > like... those would be useful to capture in the dump going forward). > > > > [2]: https://github.com/SoMainline/linux/commit/2bbc609dd28aa0bd0a2dede20163e521912d0072 > > > > Not entirely correct.TEAR_AUTOREFRESH_STATUS is at 0x2c0 for sm8350 and > sm8450 as well so 0x2b4 is a bit short. DPU code uses autorefresh status > today.Esp after your changes it will use the autorefresh status from > intf te which is at offset 0x2c0 Revisiting this, I don't see current DPU code nor downstream 5.4 / 5.10 SDE techpack on some of my checkouts use this register, only INTF_TEAR_AUTOREFRESH_CONFIG at 0x2b4..0x2b7. Am I missing something critical here? > >>> We have discussed INTF lengths in [1]. The current understanding of the > >>> block lengths can be found at [2]. Please comment there if any of the > >>> fixed lengths sounds incorrect to you. > >>> > >>> [1] https://patchwork.freedesktop.org/patch/522187/ > >>> [2] https://patchwork.freedesktop.org/patch/522227/ > >>> > >>> [skipped the rest] > >>> > >> > >> Please correct my understanding here, it was agreed to fix intf blocks > >> to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont > >> see this was merged? > >> > >> It was agreed to first land INTF_TE and then add the higher addresses > > > > Seems like it, at least if I interpret [3] correctly. My series adds a > > new define that will hardcode _len to 0x2B8 for now, and Dmitry/Konrad > > can later extend it to whatever is stated by the correct downstream > > source. > > > > Like mentioned above it should be 0x2c0 for intf block. > > If you face any conflicting information in downstream code, you can > always check with me on IRC. Ack, it looks like others landed these changes for me now via the catalog rework, so I have just rebased and kept the lengths in. - Marijn
On 4/17/2023 12:41 PM, Marijn Suijten wrote: > On 2023-02-14 09:54:57, Abhinav Kumar wrote: > [..] >>>>>> Just wondering, how were the lengths calculated for the INTF blocks? >>>>>> The values in general seem a little off to me. >>> >>> These (for MSM8998) have been taken from downstream specifically; my >>> series starts using INTF_STATUS at 0x26C which conveniently is the >>> register right after 0x268, matching the fact that INTF TE and these >>> registers weren't supported/available yet on MSM8998. >>> >>>>>> For example, I'm looking downstream and it seems to me that the length >>>>>> for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm >>>>>> seeing that length for INTF + tearcheck should be 0x2c4. >>> >>> There are many different downstream sources and tags with seemingly >>> conflicting/confusing information. For v2 [2] I've picked the highest >>> register used by the driver which is INTF_TEAR_AUTOREFRESH_CONFIG at >>> 0x2B4 (but there might always be more registers that don't need to be >>> poked at by the driver, but contain magic debug information and the >>> like... those would be useful to capture in the dump going forward). >>> >>> [2]: https://github.com/SoMainline/linux/commit/2bbc609dd28aa0bd0a2dede20163e521912d0072 >>> >> >> Not entirely correct.TEAR_AUTOREFRESH_STATUS is at 0x2c0 for sm8350 and >> sm8450 as well so 0x2b4 is a bit short. DPU code uses autorefresh status >> today.Esp after your changes it will use the autorefresh status from >> intf te which is at offset 0x2c0 > > Revisiting this, I don't see current DPU code nor downstream 5.4 / 5.10 > SDE techpack on some of my checkouts use this register, only > INTF_TEAR_AUTOREFRESH_CONFIG at 0x2b4..0x2b7. Am I missing something > critical here? > Wow, I lost context since its been months since your last response. But, I refreshed some of it. You are right, we use the status bits present in the INTF_TEAR_AUTOREFRESH_CONFIG and INTF_TEAR_ AUTOREFRESH_STATUS is unused. I got confused between the status bit present in the two registers as both relate to autorefresh. But, the offset of of INTF_TEAR_ AUTOREFRESH_STATUS is still at 0x2c0 as i wrote before so 0x2c4 is the accurate length of this block. And yes, all the blk lengths should be accurate now in the hw catalog after the rework and reviews of that rework. >>>>> We have discussed INTF lengths in [1]. The current understanding of the >>>>> block lengths can be found at [2]. Please comment there if any of the >>>>> fixed lengths sounds incorrect to you. >>>>> >>>>> [1] https://patchwork.freedesktop.org/patch/522187/ >>>>> [2] https://patchwork.freedesktop.org/patch/522227/ >>>>> >>>>> [skipped the rest] >>>>> >>>> >>>> Please correct my understanding here, it was agreed to fix intf blocks >>>> to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont >>>> see this was merged? >>>> >>>> It was agreed to first land INTF_TE and then add the higher addresses >>> >>> Seems like it, at least if I interpret [3] correctly. My series adds a >>> new define that will hardcode _len to 0x2B8 for now, and Dmitry/Konrad >>> can later extend it to whatever is stated by the correct downstream >>> source. >>> >> >> Like mentioned above it should be 0x2c0 for intf block. >> >> If you face any conflicting information in downstream code, you can >> always check with me on IRC. > > Ack, it looks like others landed these changes for me now via the > catalog rework, so I have just rebased and kept the lengths in. > > - Marijn
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 1cfe94494135..b9b9b5b0b615 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -86,6 +86,15 @@ #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ + BIT(MDP_SSPP_TOP0_INTR2) | \ + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ + BIT(MDP_INTF0_INTR) | \ + BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF2_INTR) | \ + BIT(MDP_INTF3_INTR) | \ + BIT(MDP_INTF4_INTR)) + #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ @@ -100,13 +109,15 @@ #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ - BIT(MDP_INTF1_INTR)) + BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR)) #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ BIT(MDP_INTF0_INTR) | \ - BIT(MDP_INTF1_INTR)) + BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR)) #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ @@ -120,7 +131,9 @@ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ BIT(MDP_INTF0_INTR) | \ BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR) | \ BIT(MDP_INTF2_INTR) | \ + BIT(MDP_INTF2_TEAR_INTR) | \ BIT(MDP_INTF3_INTR) | \ BIT(MDP_INTF4_INTR)) @@ -129,7 +142,9 @@ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ BIT(MDP_INTF0_INTR) | \ BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR) | \ BIT(MDP_INTF2_INTR) | \ + BIT(MDP_INTF2_TEAR_INTR) | \ BIT(MDP_INTF3_INTR) | \ BIT(MDP_INTF4_INTR) | \ BIT(MDP_INTF5_INTR) | \ @@ -1300,63 +1315,64 @@ static struct dpu_dsc_cfg sdm845_dsc[] = { /************************************************************* * INTF sub blocks config *************************************************************/ -#define INTF_BLK(_name, _id, _base, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit) \ +#define INTF_BLK(_name, _id, _base, _len, _type, _ctrl_id, _progfetch, _features, _reg, _underrun_bit, _vsync_bit, _tear_reg, _tear_rd_ptr_bit) \ {\ .name = _name, .id = _id, \ - .base = _base, .len = 0x280, \ + .base = _base, .len = _len, \ .features = _features, \ .type = _type, \ .controller_id = _ctrl_id, \ .prog_fetch_lines_worst_case = _progfetch, \ .intr_underrun = DPU_IRQ_IDX(_reg, _underrun_bit), \ .intr_vsync = DPU_IRQ_IDX(_reg, _vsync_bit), \ + .intr_tear_rd_ptr = DPU_IRQ_IDX(_tear_reg, _tear_rd_ptr_bit), \ } static const struct dpu_intf_cfg msm8998_intf[] = { - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x268, INTF_DP, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x268, INTF_DSI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27, -1, -1), + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x268, INTF_DSI, 1, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29, -1, -1), + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x268, INTF_HDMI, 0, 25, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), }; static const struct dpu_intf_cfg sdm845_intf[] = { - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25), - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27), - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29), - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31), + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x280, INTF_DSI, 0, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27, -1, -1), + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x280, INTF_DSI, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29, -1, -1), + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 1, 24, INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), }; static const struct dpu_intf_cfg sc7180_intf[] = { - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), }; static const struct dpu_intf_cfg sm8150_intf[] = { - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29), - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31), + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x2b8, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29, MDP_INTF2_TEAR_INTR, 2), + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), }; static const struct dpu_intf_cfg sc7280_intf[] = { - INTF_BLK("intf_0", INTF_0, 0x34000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25), - INTF_BLK("intf_1", INTF_1, 0x35000, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27), - INTF_BLK("intf_5", INTF_5, 0x39000, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23), + INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), + INTF_BLK("intf_1", INTF_1, 0x35000, 0x2b8, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), + INTF_BLK("intf_5", INTF_5, 0x39000, 0x280, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 22, 23, -1, -1), }; static const struct dpu_intf_cfg sc8180x_intf[] = { - INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25), - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), - INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29), + INTF_BLK("intf_0", INTF_0, 0x6A000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25, -1, -1), + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF1_TEAR_INTR, 2), + INTF_BLK("intf_2", INTF_2, 0x6B000, 0x2b8, INTF_DSI, 1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 28, 29, MDP_INTF2_TEAR_INTR, 2), /* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until this is supported */ - INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31), - INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 20, 21), - INTF_BLK("intf_5", INTF_5, 0x6C800, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 22, 23), + INTF_BLK("intf_3", INTF_3, 0x6B800, 0x280, INTF_DP, 999, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 30, 31, -1, -1), + INTF_BLK("intf_4", INTF_4, 0x6C000, 0x280, INTF_DP, MSM_DP_CONTROLLER_1, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 20, 21, -1, -1), + INTF_BLK("intf_5", INTF_5, 0x6C800, 0x280, INTF_DP, MSM_DP_CONTROLLER_2, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 22, 23, -1, -1), }; static const struct dpu_intf_cfg qcm2290_intf[] = { - INTF_BLK("intf_0", INTF_0, 0x00000, INTF_NONE, 0, 0, 0, 0, 0, 0), - INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27), + INTF_BLK("intf_0", INTF_0, 0x00000, 0x2b8, INTF_NONE, 0, 0, 0, 0, 0, 0, -1, -1), + INTF_BLK("intf_1", INTF_1, 0x6A800, 0x2b8, INTF_DSI, 0, 24, INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 26, 27, MDP_INTF2_TEAR_INTR, 2), }; /************************************************************* @@ -1849,7 +1865,7 @@ static const struct dpu_mdss_cfg msm8998_dpu_cfg = { .vbif = msm8998_vbif, .reg_dma_count = 0, .perf = &msm8998_perf_data, - .mdss_irqs = IRQ_SM8250_MASK, + .mdss_irqs = IRQ_MSM8998_MASK, }; static const struct dpu_mdss_cfg sdm845_dpu_cfg = { @@ -1947,7 +1963,7 @@ static const struct dpu_mdss_cfg sm8150_dpu_cfg = { .reg_dma_count = 1, .dma_cfg = &sm8150_regdma, .perf = &sm8150_perf_data, - .mdss_irqs = IRQ_SDM845_MASK, + .mdss_irqs = IRQ_SM8250_MASK, }; static const struct dpu_mdss_cfg sc8180x_dpu_cfg = { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index e0e153889ab7..2ea17e4ef3e5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -638,8 +638,9 @@ struct dpu_dsc_cfg { * @type: Interface type(DSI, DP, HDMI) * @controller_id: Controller Instance ID in case of multiple of intf type * @prog_fetch_lines_worst_case Worst case latency num lines needed to prefetch - * @intr_underrun: index for INTF underrun interrupt - * @intr_vsync: index for INTF VSYNC interrupt + * @intr_underrun: index for INTF underrun interrupt + * @intr_vsync: index for INTF VSYNC interrupt + * @intr_tear_rd_ptr: index for INTF TEAR_RD_PTR interrupt */ struct dpu_intf_cfg { DPU_HW_BLK_INFO; @@ -648,6 +649,7 @@ struct dpu_intf_cfg { u32 prog_fetch_lines_worst_case; s32 intr_underrun; s32 intr_vsync; + s32 intr_tear_rd_ptr; }; /** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index cf1b6d84c18a..044136a97fac 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -24,6 +24,8 @@ #define MDP_INTF_3_OFF 0x6B800 #define MDP_INTF_4_OFF 0x6C000 #define MDP_INTF_5_OFF 0x6C800 +#define MDP_INTF_1_TEAR_OFF 0x6D800 +#define MDP_INTF_2_TEAR_OFF 0x6D900 #define MDP_AD4_0_OFF 0x7C000 #define MDP_AD4_1_OFF 0x7D000 #define MDP_AD4_INTR_EN_OFF 0x41c @@ -99,6 +101,16 @@ static const struct dpu_intr_reg dpu_intr_set[] = { MDP_INTF_5_OFF+INTF_INTR_EN, MDP_INTF_5_OFF+INTF_INTR_STATUS }, + [MDP_INTF1_TEAR_INTR] = { + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_CLEAR, + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_EN, + MDP_INTF_1_TEAR_OFF+INTF_INTR_TEAR_STATUS + }, + [MDP_INTF2_TEAR_INTR] = { + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_CLEAR, + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_EN, + MDP_INTF_2_TEAR_OFF+INTF_INTR_TEAR_STATUS + }, [MDP_AD4_0_INTR] = { MDP_AD4_0_OFF + MDP_AD4_INTR_CLEAR_OFF, MDP_AD4_0_OFF + MDP_AD4_INTR_EN_OFF, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h index 46443955443c..b447e4a1d9f4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h @@ -23,6 +23,8 @@ enum dpu_hw_intr_reg { MDP_INTF3_INTR, MDP_INTF4_INTR, MDP_INTF5_INTR, + MDP_INTF1_TEAR_INTR, + MDP_INTF2_TEAR_INTR, MDP_AD4_0_INTR, MDP_AD4_1_INTR, MDP_INTF0_7xxx_INTR, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h index c8156ed4b7fb..6bdac515fd54 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h @@ -10,6 +10,9 @@ /** * MDP TOP block Register and bit fields and defines */ +#define INTF_INTR_TEAR_EN 0x000 +#define INTF_INTR_TEAR_STATUS 0x004 +#define INTF_INTR_TEAR_CLEAR 0x008 #define DISP_INTF_SEL 0x004 #define INTR_EN 0x010 #define INTR_STATUS 0x014
All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, but excluding 7.x.x) have the tear interrupt and control registers moved out of the PINGPONG block and into the INTF block. Wire up the necessary interrupts and IRQ masks on all supported hardware. Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> --- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++++++++++-------- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + 5 files changed, 68 insertions(+), 33 deletions(-)