| Message ID | 20231009-topic-sm8550-graphics-sspp-split-clk-v1-5-806c0dee4e43@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split | expand |
On 09/10/2023 19:10, Dmitry Baryshkov wrote: > On 09/10/2023 19:36, Neil Armstrong wrote: >> The SM8550 has the SSPP clk_ctrl in the SSPP registers, move them >> out of the MDP top. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 ++++++++++------------ >> 1 file changed, 15 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h >> index 7bed819dfc39..527ec020fba4 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h >> @@ -24,16 +24,6 @@ static const struct dpu_mdp_cfg sm8550_mdp = { >> .base = 0, .len = 0x494, >> .features = BIT(DPU_MDP_PERIPH_0_REMOVED), >> .clk_ctrls = { >> - [DPU_CLK_CTRL_VIG0] = { .reg_off = 0x4330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_VIG1] = { .reg_off = 0x6330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_VIG2] = { .reg_off = 0x8330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_VIG3] = { .reg_off = 0xa330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA0] = { .reg_off = 0x24330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA1] = { .reg_off = 0x26330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA2] = { .reg_off = 0x28330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2a330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA4] = { .reg_off = 0x2c330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA5] = { .reg_off = 0x2e330, .bit_off = 0 }, >> [DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20 }, > > Hmm, interesting. I even double-checked this. SSPP and WB have their own clock registers now. But the REG_DMA uses the main area (0x2bc). yeah > >> }, >> }; >> @@ -73,6 +63,11 @@ static const struct dpu_ctl_cfg sm8550_ctl[] = { >> }, >> }; >> +static const struct dpu_clk_ctrl_reg sm8550_sspp_clk_ctrl = { >> + .reg_off = 0x330, >> + .bit_off = 0 >> +}; > > I don't think we even need this outside of dpu_hw_sspp. You can use core_major_rev to check whether the driver should use global clocks or per-SSPP / per-WB clocks register instead. Ack > >> + >> static const struct dpu_sspp_cfg sm8550_sspp[] = { >> { >> .name = "sspp_0", .id = SSPP_VIG0, >> @@ -81,7 +76,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_0, >> .xin_id = 0, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG0, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_1", .id = SSPP_VIG1, >> .base = 0x6000, .len = 0x344, >> @@ -89,7 +84,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_1, >> .xin_id = 4, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG1, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_2", .id = SSPP_VIG2, >> .base = 0x8000, .len = 0x344, >> @@ -97,7 +92,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_2, >> .xin_id = 8, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG2, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_3", .id = SSPP_VIG3, >> .base = 0xa000, .len = 0x344, >> @@ -105,7 +100,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_3, >> .xin_id = 12, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG3, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_8", .id = SSPP_DMA0, >> .base = 0x24000, .len = 0x344, >> @@ -113,7 +108,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_0, >> .xin_id = 1, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA0, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_9", .id = SSPP_DMA1, >> .base = 0x26000, .len = 0x344, >> @@ -121,7 +116,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_1, >> .xin_id = 5, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA1, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_10", .id = SSPP_DMA2, >> .base = 0x28000, .len = 0x344, >> @@ -129,7 +124,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_2, >> .xin_id = 9, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA2, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_11", .id = SSPP_DMA3, >> .base = 0x2a000, .len = 0x344, >> @@ -137,7 +132,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_3, >> .xin_id = 13, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA3, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_12", .id = SSPP_DMA4, >> .base = 0x2c000, .len = 0x344, >> @@ -145,7 +140,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_dma_sblk_4, >> .xin_id = 14, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA4, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_13", .id = SSPP_DMA5, >> .base = 0x2e000, .len = 0x344, >> @@ -153,7 +148,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_dma_sblk_5, >> .xin_id = 15, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA5, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, >> }; >> >
On 09/10/2023 19:10, Dmitry Baryshkov wrote: > On 09/10/2023 19:36, Neil Armstrong wrote: >> The SM8550 has the SSPP clk_ctrl in the SSPP registers, move them >> out of the MDP top. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 ++++++++++------------ >> 1 file changed, 15 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h >> index 7bed819dfc39..527ec020fba4 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h >> @@ -24,16 +24,6 @@ static const struct dpu_mdp_cfg sm8550_mdp = { >> .base = 0, .len = 0x494, >> .features = BIT(DPU_MDP_PERIPH_0_REMOVED), >> .clk_ctrls = { >> - [DPU_CLK_CTRL_VIG0] = { .reg_off = 0x4330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_VIG1] = { .reg_off = 0x6330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_VIG2] = { .reg_off = 0x8330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_VIG3] = { .reg_off = 0xa330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA0] = { .reg_off = 0x24330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA1] = { .reg_off = 0x26330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA2] = { .reg_off = 0x28330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2a330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA4] = { .reg_off = 0x2c330, .bit_off = 0 }, >> - [DPU_CLK_CTRL_DMA5] = { .reg_off = 0x2e330, .bit_off = 0 }, >> [DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20 }, > > Hmm, interesting. I even double-checked this. SSPP and WB have their own clock registers now. But the REG_DMA uses the main area (0x2bc). yeah > >> }, >> }; >> @@ -73,6 +63,11 @@ static const struct dpu_ctl_cfg sm8550_ctl[] = { >> }, >> }; >> +static const struct dpu_clk_ctrl_reg sm8550_sspp_clk_ctrl = { >> + .reg_off = 0x330, >> + .bit_off = 0 >> +}; > > I don't think we even need this outside of dpu_hw_sspp. You can use core_major_rev to check whether the driver should use global clocks or per-SSPP / per-WB clocks register instead. Ack > >> + >> static const struct dpu_sspp_cfg sm8550_sspp[] = { >> { >> .name = "sspp_0", .id = SSPP_VIG0, >> @@ -81,7 +76,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_0, >> .xin_id = 0, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG0, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_1", .id = SSPP_VIG1, >> .base = 0x6000, .len = 0x344, >> @@ -89,7 +84,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_1, >> .xin_id = 4, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG1, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_2", .id = SSPP_VIG2, >> .base = 0x8000, .len = 0x344, >> @@ -97,7 +92,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_2, >> .xin_id = 8, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG2, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_3", .id = SSPP_VIG3, >> .base = 0xa000, .len = 0x344, >> @@ -105,7 +100,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_vig_sblk_3, >> .xin_id = 12, >> .type = SSPP_TYPE_VIG, >> - .clk_ctrl = DPU_CLK_CTRL_VIG3, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_8", .id = SSPP_DMA0, >> .base = 0x24000, .len = 0x344, >> @@ -113,7 +108,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_0, >> .xin_id = 1, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA0, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_9", .id = SSPP_DMA1, >> .base = 0x26000, .len = 0x344, >> @@ -121,7 +116,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_1, >> .xin_id = 5, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA1, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_10", .id = SSPP_DMA2, >> .base = 0x28000, .len = 0x344, >> @@ -129,7 +124,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_2, >> .xin_id = 9, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA2, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_11", .id = SSPP_DMA3, >> .base = 0x2a000, .len = 0x344, >> @@ -137,7 +132,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sdm845_dma_sblk_3, >> .xin_id = 13, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA3, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_12", .id = SSPP_DMA4, >> .base = 0x2c000, .len = 0x344, >> @@ -145,7 +140,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_dma_sblk_4, >> .xin_id = 14, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA4, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, { >> .name = "sspp_13", .id = SSPP_DMA5, >> .base = 0x2e000, .len = 0x344, >> @@ -153,7 +148,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { >> .sblk = &sm8550_dma_sblk_5, >> .xin_id = 15, >> .type = SSPP_TYPE_DMA, >> - .clk_ctrl = DPU_CLK_CTRL_DMA5, >> + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, >> }, >> }; >> >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h index 7bed819dfc39..527ec020fba4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h @@ -24,16 +24,6 @@ static const struct dpu_mdp_cfg sm8550_mdp = { .base = 0, .len = 0x494, .features = BIT(DPU_MDP_PERIPH_0_REMOVED), .clk_ctrls = { - [DPU_CLK_CTRL_VIG0] = { .reg_off = 0x4330, .bit_off = 0 }, - [DPU_CLK_CTRL_VIG1] = { .reg_off = 0x6330, .bit_off = 0 }, - [DPU_CLK_CTRL_VIG2] = { .reg_off = 0x8330, .bit_off = 0 }, - [DPU_CLK_CTRL_VIG3] = { .reg_off = 0xa330, .bit_off = 0 }, - [DPU_CLK_CTRL_DMA0] = { .reg_off = 0x24330, .bit_off = 0 }, - [DPU_CLK_CTRL_DMA1] = { .reg_off = 0x26330, .bit_off = 0 }, - [DPU_CLK_CTRL_DMA2] = { .reg_off = 0x28330, .bit_off = 0 }, - [DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2a330, .bit_off = 0 }, - [DPU_CLK_CTRL_DMA4] = { .reg_off = 0x2c330, .bit_off = 0 }, - [DPU_CLK_CTRL_DMA5] = { .reg_off = 0x2e330, .bit_off = 0 }, [DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20 }, }, }; @@ -73,6 +63,11 @@ static const struct dpu_ctl_cfg sm8550_ctl[] = { }, }; +static const struct dpu_clk_ctrl_reg sm8550_sspp_clk_ctrl = { + .reg_off = 0x330, + .bit_off = 0 +}; + static const struct dpu_sspp_cfg sm8550_sspp[] = { { .name = "sspp_0", .id = SSPP_VIG0, @@ -81,7 +76,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sm8550_vig_sblk_0, .xin_id = 0, .type = SSPP_TYPE_VIG, - .clk_ctrl = DPU_CLK_CTRL_VIG0, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_1", .id = SSPP_VIG1, .base = 0x6000, .len = 0x344, @@ -89,7 +84,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sm8550_vig_sblk_1, .xin_id = 4, .type = SSPP_TYPE_VIG, - .clk_ctrl = DPU_CLK_CTRL_VIG1, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_2", .id = SSPP_VIG2, .base = 0x8000, .len = 0x344, @@ -97,7 +92,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sm8550_vig_sblk_2, .xin_id = 8, .type = SSPP_TYPE_VIG, - .clk_ctrl = DPU_CLK_CTRL_VIG2, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_3", .id = SSPP_VIG3, .base = 0xa000, .len = 0x344, @@ -105,7 +100,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sm8550_vig_sblk_3, .xin_id = 12, .type = SSPP_TYPE_VIG, - .clk_ctrl = DPU_CLK_CTRL_VIG3, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_8", .id = SSPP_DMA0, .base = 0x24000, .len = 0x344, @@ -113,7 +108,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sdm845_dma_sblk_0, .xin_id = 1, .type = SSPP_TYPE_DMA, - .clk_ctrl = DPU_CLK_CTRL_DMA0, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_9", .id = SSPP_DMA1, .base = 0x26000, .len = 0x344, @@ -121,7 +116,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sdm845_dma_sblk_1, .xin_id = 5, .type = SSPP_TYPE_DMA, - .clk_ctrl = DPU_CLK_CTRL_DMA1, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_10", .id = SSPP_DMA2, .base = 0x28000, .len = 0x344, @@ -129,7 +124,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sdm845_dma_sblk_2, .xin_id = 9, .type = SSPP_TYPE_DMA, - .clk_ctrl = DPU_CLK_CTRL_DMA2, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_11", .id = SSPP_DMA3, .base = 0x2a000, .len = 0x344, @@ -137,7 +132,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sdm845_dma_sblk_3, .xin_id = 13, .type = SSPP_TYPE_DMA, - .clk_ctrl = DPU_CLK_CTRL_DMA3, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_12", .id = SSPP_DMA4, .base = 0x2c000, .len = 0x344, @@ -145,7 +140,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sm8550_dma_sblk_4, .xin_id = 14, .type = SSPP_TYPE_DMA, - .clk_ctrl = DPU_CLK_CTRL_DMA4, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, { .name = "sspp_13", .id = SSPP_DMA5, .base = 0x2e000, .len = 0x344, @@ -153,7 +148,7 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = { .sblk = &sm8550_dma_sblk_5, .xin_id = 15, .type = SSPP_TYPE_DMA, - .clk_ctrl = DPU_CLK_CTRL_DMA5, + .clk_ctrl_reg = &sm8550_sspp_clk_ctrl, }, };
The SM8550 has the SSPP clk_ctrl in the SSPP registers, move them out of the MDP top. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 ++++++++++------------ 1 file changed, 15 insertions(+), 20 deletions(-)