Message ID | 20221231215006.211860-1-marijn.suijten@somainline.org |
---|---|
Headers | show |
Series | drm/msm/dpu: Implement tearcheck support on INTF block | expand |
On 31/12/2022 23:50, Marijn Suijten wrote: > Neither of these SoCs has INTF0, they only have a DSI interface on index > 1. Stop enabling an interrupt that can't fire. > > Fixes: 3581b7062cec ("drm/msm/disp/dpu1: add support for display on SM6115") > Fixes: 5334087ee743 ("drm/msm: add support for QCM2290 MDSS") > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Sat, 31 Dec 2022 at 23:50, Marijn Suijten <marijn.suijten@somainline.org> wrote: > > Now that newer DPU platforms use a readpointer-done interrupt on the > INTF block, stop providing the unused interrupt on the PINGPONG block. > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Sat, 31 Dec 2022 at 23:52, Marijn Suijten <marijn.suijten@somainline.org> wrote: > > On 2022-12-31 22:50:02, Marijn Suijten wrote: > > Since hardware revision 5.0.0 the TE configuration moved out of the > > PINGPONG block into the INTF block, including vsync source selection > > that was previously part of MDP top. Writing to the MDP_VSYNC_SEL > > register has no effect anymore and is omitted downstream via the > > DPU/SDE_MDP_VSYNC_SEL feature flag. This flag is only added to INTF > > blocks used by hardware prior to 5.0.0. > > > > The code that writes to these registers in the INTF block will follow in > > subsequent patches. > > > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > --- > > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 33 ++++++++++-- > > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 52 +++++++++++++------ > > 3 files changed, 66 insertions(+), 20 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 39d4b293710c..1cfe94494135 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > @@ -407,7 +407,7 @@ static const struct dpu_mdp_cfg msm8998_mdp[] = { > > { > > .name = "top_0", .id = MDP_TOP, > > .base = 0x0, .len = 0x458, > > - .features = 0, > > + .features = BIT(DPU_MDP_VSYNC_SEL), > > .highest_bank_bit = 0x2, > > .clk_ctrls[DPU_CLK_CTRL_VIG0] = { > > .reg_off = 0x2AC, .bit_off = 0}, > > @@ -436,7 +436,7 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = { > > { > > .name = "top_0", .id = MDP_TOP, > > .base = 0x0, .len = 0x45C, > > - .features = BIT(DPU_MDP_AUDIO_SELECT), > > + .features = BIT(DPU_MDP_AUDIO_SELECT) | BIT(DPU_MDP_VSYNC_SEL), > > .highest_bank_bit = 0x2, > > .clk_ctrls[DPU_CLK_CTRL_VIG0] = { > > .reg_off = 0x2AC, .bit_off = 0}, > > @@ -512,6 +512,31 @@ static const struct dpu_mdp_cfg sm6115_mdp[] = { > > }, > > }; > > > > +static const struct dpu_mdp_cfg sdm8150_mdp[] = { > > Sometimes it is only possible to spot such things _after_ sending, > probably the thing that makes us human :) > > sm8150_mdp*, not sdm. > With this name fixed: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
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 31.12.2022 22:52, Marijn Suijten wrote: > On 2022-12-31 22:50:02, Marijn Suijten wrote: >> Since hardware revision 5.0.0 the TE configuration moved out of the >> PINGPONG block into the INTF block, including vsync source selection >> that was previously part of MDP top. Writing to the MDP_VSYNC_SEL >> register has no effect anymore and is omitted downstream via the >> DPU/SDE_MDP_VSYNC_SEL feature flag. This flag is only added to INTF >> blocks used by hardware prior to 5.0.0. >> >> The code that writes to these registers in the INTF block will follow in >> subsequent patches. >> >> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> >> --- >> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 33 ++++++++++-- >> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 + >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 52 +++++++++++++------ >> 3 files changed, 66 insertions(+), 20 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 39d4b293710c..1cfe94494135 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >> @@ -407,7 +407,7 @@ static const struct dpu_mdp_cfg msm8998_mdp[] = { >> { >> .name = "top_0", .id = MDP_TOP, >> .base = 0x0, .len = 0x458, >> - .features = 0, >> + .features = BIT(DPU_MDP_VSYNC_SEL), >> .highest_bank_bit = 0x2, >> .clk_ctrls[DPU_CLK_CTRL_VIG0] = { >> .reg_off = 0x2AC, .bit_off = 0}, >> @@ -436,7 +436,7 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = { >> { >> .name = "top_0", .id = MDP_TOP, >> .base = 0x0, .len = 0x45C, >> - .features = BIT(DPU_MDP_AUDIO_SELECT), >> + .features = BIT(DPU_MDP_AUDIO_SELECT) | BIT(DPU_MDP_VSYNC_SEL), >> .highest_bank_bit = 0x2, >> .clk_ctrls[DPU_CLK_CTRL_VIG0] = { >> .reg_off = 0x2AC, .bit_off = 0}, >> @@ -512,6 +512,31 @@ static const struct dpu_mdp_cfg sm6115_mdp[] = { >> }, >> }; >> >> +static const struct dpu_mdp_cfg sdm8150_mdp[] = { > > Sometimes it is only possible to spot such things _after_ sending, > probably the thing that makes us human :) > > sm8150_mdp*, not sdm. > > - Marijn > >> + { >> + .name = "top_0", .id = MDP_TOP, >> + .base = 0x0, .len = 0x45C, >> + .features = BIT(DPU_MDP_AUDIO_SELECT), >> + .highest_bank_bit = 0x2, >> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { >> + .reg_off = 0x2AC, .bit_off = 0}, Keeping the hex values lowercase would be nice. Konrad >> + .clk_ctrls[DPU_CLK_CTRL_VIG1] = { >> + .reg_off = 0x2B4, .bit_off = 0}, >> + .clk_ctrls[DPU_CLK_CTRL_VIG2] = { >> + .reg_off = 0x2BC, .bit_off = 0}, >> + .clk_ctrls[DPU_CLK_CTRL_VIG3] = { >> + .reg_off = 0x2C4, .bit_off = 0}, >> + .clk_ctrls[DPU_CLK_CTRL_DMA0] = { >> + .reg_off = 0x2AC, .bit_off = 8}, >> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = { >> + .reg_off = 0x2B4, .bit_off = 8}, >> + .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = { >> + .reg_off = 0x2BC, .bit_off = 8}, >> + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { >> + .reg_off = 0x2C4, .bit_off = 8}, >> + }, >> +}; >> + >> static const struct dpu_mdp_cfg sm8250_mdp[] = { >> { >> .name = "top_0", .id = MDP_TOP, >> @@ -1901,8 +1926,8 @@ static const struct dpu_mdss_cfg sm6115_dpu_cfg = { >> >> static const struct dpu_mdss_cfg sm8150_dpu_cfg = { >> .caps = &sm8150_dpu_caps, >> - .mdp_count = ARRAY_SIZE(sdm845_mdp), >> - .mdp = sdm845_mdp, >> + .mdp_count = ARRAY_SIZE(sdm8150_mdp), >> + .mdp = sdm8150_mdp, >> .ctl_count = ARRAY_SIZE(sm8150_ctl), >> .ctl = sm8150_ctl, >> .sspp_count = ARRAY_SIZE(sdm845_sspp), >> 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 3b645d5aa9aa..e0e153889ab7 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h >> @@ -93,6 +93,7 @@ enum { >> DPU_MDP_UBWC_1_0, >> DPU_MDP_UBWC_1_5, >> DPU_MDP_AUDIO_SELECT, >> + DPU_MDP_VSYNC_SEL, >> DPU_MDP_MAX >> }; >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c >> index c3110a25a30d..2e699c9ad13c 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c >> @@ -151,28 +151,16 @@ static void dpu_hw_get_danger_status(struct dpu_hw_mdp *mdp, >> status->sspp[SSPP_CURSOR1] = (value >> 26) & 0x3; >> } >> >> -static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp *mdp, >> +static void dpu_hw_setup_vsync_source_v1(struct dpu_hw_mdp *mdp, >> struct dpu_vsync_source_cfg *cfg) >> { >> struct dpu_hw_blk_reg_map *c; >> - u32 reg, wd_load_value, wd_ctl, wd_ctl2, i; >> - static const u32 pp_offset[PINGPONG_MAX] = {0xC, 0x8, 0x4, 0x13, 0x18}; >> + u32 reg, wd_load_value, wd_ctl, wd_ctl2; >> >> - if (!mdp || !cfg || (cfg->pp_count > ARRAY_SIZE(cfg->ppnumber))) >> + if (!mdp || !cfg) >> return; >> >> c = &mdp->hw; >> - reg = DPU_REG_READ(c, MDP_VSYNC_SEL); >> - for (i = 0; i < cfg->pp_count; i++) { >> - int pp_idx = cfg->ppnumber[i] - PINGPONG_0; >> - >> - if (pp_idx >= ARRAY_SIZE(pp_offset)) >> - continue; >> - >> - reg &= ~(0xf << pp_offset[pp_idx]); >> - reg |= (cfg->vsync_source & 0xf) << pp_offset[pp_idx]; >> - } >> - DPU_REG_WRITE(c, MDP_VSYNC_SEL, reg); >> >> if (cfg->vsync_source >= DPU_VSYNC_SOURCE_WD_TIMER_4 && >> cfg->vsync_source <= DPU_VSYNC_SOURCE_WD_TIMER_0) { >> @@ -219,6 +207,33 @@ static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp *mdp, >> } >> } >> >> +static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp *mdp, >> + struct dpu_vsync_source_cfg *cfg) >> +{ >> + struct dpu_hw_blk_reg_map *c; >> + u32 reg, i; >> + static const u32 pp_offset[PINGPONG_MAX] = {0xC, 0x8, 0x4, 0x13, 0x18}; >> + >> + if (!mdp || !cfg || (cfg->pp_count > ARRAY_SIZE(cfg->ppnumber))) >> + return; >> + >> + c = &mdp->hw; >> + >> + reg = DPU_REG_READ(c, MDP_VSYNC_SEL); >> + for (i = 0; i < cfg->pp_count; i++) { >> + int pp_idx = cfg->ppnumber[i] - PINGPONG_0; >> + >> + if (pp_idx >= ARRAY_SIZE(pp_offset)) >> + continue; >> + >> + reg &= ~(0xf << pp_offset[pp_idx]); >> + reg |= (cfg->vsync_source & 0xf) << pp_offset[pp_idx]; >> + } >> + DPU_REG_WRITE(c, MDP_VSYNC_SEL, reg); >> + >> + dpu_hw_setup_vsync_source_v1(mdp, cfg); >> +} >> + >> static void dpu_hw_get_safe_status(struct dpu_hw_mdp *mdp, >> struct dpu_danger_safe_status *status) >> { >> @@ -266,7 +281,12 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops, >> ops->setup_split_pipe = dpu_hw_setup_split_pipe; >> ops->setup_clk_force_ctrl = dpu_hw_setup_clk_force_ctrl; >> ops->get_danger_status = dpu_hw_get_danger_status; >> - ops->setup_vsync_source = dpu_hw_setup_vsync_source; >> + >> + if (cap & BIT(DPU_MDP_VSYNC_SEL)) >> + ops->setup_vsync_source = dpu_hw_setup_vsync_source; >> + else >> + ops->setup_vsync_source = dpu_hw_setup_vsync_source_v1; >> + >> ops->get_safe_status = dpu_hw_get_safe_status; >> >> if (cap & BIT(DPU_MDP_AUDIO_SELECT)) >> -- >> 2.39.0 >>
On 2023-01-02 10:30:58, Konrad Dybcio wrote: > > > On 31.12.2022 22:52, Marijn Suijten wrote: > > On 2022-12-31 22:50:02, Marijn Suijten wrote: > >> Since hardware revision 5.0.0 the TE configuration moved out of the > >> PINGPONG block into the INTF block, including vsync source selection > >> that was previously part of MDP top. Writing to the MDP_VSYNC_SEL > >> register has no effect anymore and is omitted downstream via the > >> DPU/SDE_MDP_VSYNC_SEL feature flag. This flag is only added to INTF > >> blocks used by hardware prior to 5.0.0. > >> > >> The code that writes to these registers in the INTF block will follow in > >> subsequent patches. > >> > >> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > >> --- > >> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 33 ++++++++++-- > >> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 52 +++++++++++++------ > >> 3 files changed, 66 insertions(+), 20 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 39d4b293710c..1cfe94494135 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> @@ -407,7 +407,7 @@ static const struct dpu_mdp_cfg msm8998_mdp[] = { > >> { > >> .name = "top_0", .id = MDP_TOP, > >> .base = 0x0, .len = 0x458, > >> - .features = 0, > >> + .features = BIT(DPU_MDP_VSYNC_SEL), > >> .highest_bank_bit = 0x2, > >> .clk_ctrls[DPU_CLK_CTRL_VIG0] = { > >> .reg_off = 0x2AC, .bit_off = 0}, > >> @@ -436,7 +436,7 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = { > >> { > >> .name = "top_0", .id = MDP_TOP, > >> .base = 0x0, .len = 0x45C, > >> - .features = BIT(DPU_MDP_AUDIO_SELECT), > >> + .features = BIT(DPU_MDP_AUDIO_SELECT) | BIT(DPU_MDP_VSYNC_SEL), > >> .highest_bank_bit = 0x2, > >> .clk_ctrls[DPU_CLK_CTRL_VIG0] = { > >> .reg_off = 0x2AC, .bit_off = 0}, > >> @@ -512,6 +512,31 @@ static const struct dpu_mdp_cfg sm6115_mdp[] = { > >> }, > >> }; > >> > >> +static const struct dpu_mdp_cfg sdm8150_mdp[] = { > > > > Sometimes it is only possible to spot such things _after_ sending, > > probably the thing that makes us human :) > > > > sm8150_mdp*, not sdm. > > > > - Marijn > > > >> + { > >> + .name = "top_0", .id = MDP_TOP, > >> + .base = 0x0, .len = 0x45C, > >> + .features = BIT(DPU_MDP_AUDIO_SELECT), > >> + .highest_bank_bit = 0x2, > >> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { > >> + .reg_off = 0x2AC, .bit_off = 0}, > Keeping the hex values lowercase would be nice. Ack, this was copied verbatim from sdm845_mdp but will cleanup as we go. Looks like this file has a mixed approach towards lower and uppercase, when does the normalization patch land? - Marijn
On 2.01.2023 11:18, Marijn Suijten wrote: > On 2023-01-02 10:30:58, Konrad Dybcio wrote: >> >> >> On 31.12.2022 22:52, Marijn Suijten wrote: >>> On 2022-12-31 22:50:02, Marijn Suijten wrote: >>>> Since hardware revision 5.0.0 the TE configuration moved out of the >>>> PINGPONG block into the INTF block, including vsync source selection >>>> that was previously part of MDP top. Writing to the MDP_VSYNC_SEL >>>> register has no effect anymore and is omitted downstream via the >>>> DPU/SDE_MDP_VSYNC_SEL feature flag. This flag is only added to INTF >>>> blocks used by hardware prior to 5.0.0. >>>> >>>> The code that writes to these registers in the INTF block will follow in >>>> subsequent patches. >>>> >>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> >>>> --- >>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 33 ++++++++++-- >>>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 + >>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 52 +++++++++++++------ >>>> 3 files changed, 66 insertions(+), 20 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 39d4b293710c..1cfe94494135 100644 >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c >>>> @@ -407,7 +407,7 @@ static const struct dpu_mdp_cfg msm8998_mdp[] = { >>>> { >>>> .name = "top_0", .id = MDP_TOP, >>>> .base = 0x0, .len = 0x458, >>>> - .features = 0, >>>> + .features = BIT(DPU_MDP_VSYNC_SEL), >>>> .highest_bank_bit = 0x2, >>>> .clk_ctrls[DPU_CLK_CTRL_VIG0] = { >>>> .reg_off = 0x2AC, .bit_off = 0}, >>>> @@ -436,7 +436,7 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = { >>>> { >>>> .name = "top_0", .id = MDP_TOP, >>>> .base = 0x0, .len = 0x45C, >>>> - .features = BIT(DPU_MDP_AUDIO_SELECT), >>>> + .features = BIT(DPU_MDP_AUDIO_SELECT) | BIT(DPU_MDP_VSYNC_SEL), >>>> .highest_bank_bit = 0x2, >>>> .clk_ctrls[DPU_CLK_CTRL_VIG0] = { >>>> .reg_off = 0x2AC, .bit_off = 0}, >>>> @@ -512,6 +512,31 @@ static const struct dpu_mdp_cfg sm6115_mdp[] = { >>>> }, >>>> }; >>>> >>>> +static const struct dpu_mdp_cfg sdm8150_mdp[] = { >>> >>> Sometimes it is only possible to spot such things _after_ sending, >>> probably the thing that makes us human :) >>> >>> sm8150_mdp*, not sdm. >>> >>> - Marijn >>> >>>> + { >>>> + .name = "top_0", .id = MDP_TOP, >>>> + .base = 0x0, .len = 0x45C, >>>> + .features = BIT(DPU_MDP_AUDIO_SELECT), >>>> + .highest_bank_bit = 0x2, >>>> + .clk_ctrls[DPU_CLK_CTRL_VIG0] = { >>>> + .reg_off = 0x2AC, .bit_off = 0}, >> Keeping the hex values lowercase would be nice. > > Ack, this was copied verbatim from sdm845_mdp but will cleanup as we go. > Looks like this file has a mixed approach towards lower and uppercase, > when does the normalization patch land? Rob was against changing everything in one commit, as that would mess with git blame, so we settled on preventing adding new uppercase hex for now (outside of #defines ofc). Konrad > > - Marijn
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>