* [PATCH 1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-15 12:41 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-15 12:41 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
linux-arm-msm, dri-devel, freedreno, AngeloGioacchino Del Regno,
Jami Kettunen
DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
planes). Correct corresponding SSPP declarations.
Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
Cc: Jami Kettunen <jami.kettunen@somainline.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_cfg sdm845_sspp[] = {
--
2.39.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-15 12:41 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-15 12:41 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, Jami Kettunen, linux-arm-msm, Bjorn Andersson,
dri-devel, Stephen Boyd, AngeloGioacchino Del Regno
DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
planes). Correct corresponding SSPP declarations.
Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
Cc: Jami Kettunen <jami.kettunen@somainline.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_cfg sdm845_sspp[] = {
--
2.39.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
2023-01-15 12:41 ` Dmitry Baryshkov
@ 2023-01-15 12:41 ` Dmitry Baryshkov
-1 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-15 12:41 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
linux-arm-msm, dri-devel, freedreno
DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
SSPP blocks used for the cursor planes. This has lead to the confusion
at least for the MSM8998 platform. In preparation to supporting the
cursor SSPP blocks, use proper enum values to index DMA SSPP clock
controls.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -522,9 +522,9 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -539,9 +539,9 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
.reg_off = 0x2AC, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_DMA0] = {
.reg_off = 0x2AC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -564,9 +564,9 @@ static const struct dpu_mdp_cfg sc8180x_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -602,9 +602,9 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2BC, .bit_off = 20},
@@ -631,9 +631,9 @@ static const struct dpu_mdp_cfg sm8350_mdp[] = {
.reg_off = 0x2ac, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2b4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2bc, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2c4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2bc, .bit_off = 20},
@@ -658,9 +658,9 @@ static const struct dpu_mdp_cfg sm8450_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2BC, .bit_off = 20},
@@ -676,9 +676,9 @@ static const struct dpu_mdp_cfg sc7280_mdp[] = {
.reg_off = 0x2AC, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_DMA0] = {
.reg_off = 0x2AC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -696,8 +696,8 @@ static const struct dpu_mdp_cfg sc8280xp_mdp[] = {
.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},
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2bc, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2c4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20},
},
};
@@ -724,9 +724,9 @@ static const struct dpu_mdp_cfg sm8550_mdp[] = {
.reg_off = 0x28330, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2a330, .bit_off = 0},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA4] = {
.reg_off = 0x2c330, .bit_off = 0},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA5] = {
.reg_off = 0x2e330, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2bc, .bit_off = 20},
@@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_sub_blks sc7180_vig_sblk_0 =
@@ -1216,9 +1216,9 @@ static const struct dpu_sspp_cfg sc7180_sspp[] = {
SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
};
static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
@@ -1254,9 +1254,9 @@ static const struct dpu_sspp_cfg sm8250_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_sub_blks sm8450_vig_sblk_0 =
@@ -1282,9 +1282,9 @@ static const struct dpu_sspp_cfg sm8450_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
@@ -1316,9 +1316,9 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_SDM845_MASK,
sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
SSPP_BLK("sspp_12", SSPP_DMA4, 0x2c000, DMA_CURSOR_SDM845_MASK,
- sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA4),
SSPP_BLK("sspp_13", SSPP_DMA5, 0x2e000, DMA_CURSOR_SDM845_MASK,
- sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA5),
};
static const struct dpu_sspp_cfg sc7280_sspp[] = {
@@ -1327,9 +1327,9 @@ static const struct dpu_sspp_cfg sc7280_sspp[] = {
SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
};
static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_0 =
@@ -1355,9 +1355,9 @@ static const struct dpu_sspp_cfg sc8280xp_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
#define _VIG_SBLK_NOSCALE(num, sdma_pri) \
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 56d98b4dd2ac..9c96920e1849 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -513,6 +513,8 @@ enum dpu_clk_ctrl_type {
DPU_CLK_CTRL_DMA1,
DPU_CLK_CTRL_DMA2,
DPU_CLK_CTRL_DMA3,
+ DPU_CLK_CTRL_DMA4,
+ DPU_CLK_CTRL_DMA5,
DPU_CLK_CTRL_CURSOR0,
DPU_CLK_CTRL_CURSOR1,
DPU_CLK_CTRL_INLINE_ROT0_SSPP,
--
2.39.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
@ 2023-01-15 12:41 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-15 12:41 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
SSPP blocks used for the cursor planes. This has lead to the confusion
at least for the MSM8998 platform. In preparation to supporting the
cursor SSPP blocks, use proper enum values to index DMA SSPP clock
controls.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -522,9 +522,9 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -539,9 +539,9 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
.reg_off = 0x2AC, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_DMA0] = {
.reg_off = 0x2AC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -564,9 +564,9 @@ static const struct dpu_mdp_cfg sc8180x_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -602,9 +602,9 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2BC, .bit_off = 20},
@@ -631,9 +631,9 @@ static const struct dpu_mdp_cfg sm8350_mdp[] = {
.reg_off = 0x2ac, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2b4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2bc, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2c4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2bc, .bit_off = 20},
@@ -658,9 +658,9 @@ static const struct dpu_mdp_cfg sm8450_mdp[] = {
.reg_off = 0x2AC, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2BC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2C4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2BC, .bit_off = 20},
@@ -676,9 +676,9 @@ static const struct dpu_mdp_cfg sc7280_mdp[] = {
.reg_off = 0x2AC, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_DMA0] = {
.reg_off = 0x2AC, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
.reg_off = 0x2B4, .bit_off = 8},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
.reg_off = 0x2C4, .bit_off = 8},
},
};
@@ -696,8 +696,8 @@ static const struct dpu_mdp_cfg sc8280xp_mdp[] = {
.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},
+ .clk_ctrls[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2bc, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2c4, .bit_off = 8},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20},
},
};
@@ -724,9 +724,9 @@ static const struct dpu_mdp_cfg sm8550_mdp[] = {
.reg_off = 0x28330, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_DMA3] = {
.reg_off = 0x2a330, .bit_off = 0},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA4] = {
.reg_off = 0x2c330, .bit_off = 0},
- .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .clk_ctrls[DPU_CLK_CTRL_DMA5] = {
.reg_off = 0x2e330, .bit_off = 0},
.clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
.reg_off = 0x2bc, .bit_off = 20},
@@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_sub_blks sc7180_vig_sblk_0 =
@@ -1216,9 +1216,9 @@ static const struct dpu_sspp_cfg sc7180_sspp[] = {
SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
};
static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
@@ -1254,9 +1254,9 @@ static const struct dpu_sspp_cfg sm8250_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_sub_blks sm8450_vig_sblk_0 =
@@ -1282,9 +1282,9 @@ static const struct dpu_sspp_cfg sm8450_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
@@ -1316,9 +1316,9 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_SDM845_MASK,
sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
SSPP_BLK("sspp_12", SSPP_DMA4, 0x2c000, DMA_CURSOR_SDM845_MASK,
- sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA4),
SSPP_BLK("sspp_13", SSPP_DMA5, 0x2e000, DMA_CURSOR_SDM845_MASK,
- sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA5),
};
static const struct dpu_sspp_cfg sc7280_sspp[] = {
@@ -1327,9 +1327,9 @@ static const struct dpu_sspp_cfg sc7280_sspp[] = {
SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
};
static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_0 =
@@ -1355,9 +1355,9 @@ static const struct dpu_sspp_cfg sc8280xp_sspp[] = {
SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
+ sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
- sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
+ sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
};
#define _VIG_SBLK_NOSCALE(num, sdma_pri) \
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 56d98b4dd2ac..9c96920e1849 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -513,6 +513,8 @@ enum dpu_clk_ctrl_type {
DPU_CLK_CTRL_DMA1,
DPU_CLK_CTRL_DMA2,
DPU_CLK_CTRL_DMA3,
+ DPU_CLK_CTRL_DMA4,
+ DPU_CLK_CTRL_DMA5,
DPU_CLK_CTRL_CURSOR0,
DPU_CLK_CTRL_CURSOR1,
DPU_CLK_CTRL_INLINE_ROT0_SSPP,
--
2.39.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
2023-01-15 12:41 ` Dmitry Baryshkov
@ 2023-01-16 15:12 ` Neil Armstrong
-1 siblings, 0 replies; 26+ messages in thread
From: Neil Armstrong @ 2023-01-16 15:12 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
On 15/01/2023 13:41, Dmitry Baryshkov wrote:
> DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
> SSPP blocks used for the cursor planes. This has lead to the confusion
> at least for the MSM8998 platform. In preparation to supporting the
> cursor SSPP blocks, use proper enum values to index DMA SSPP clock
> controls.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
> 2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -522,9 +522,9 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -539,9 +539,9 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> .reg_off = 0x2AC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -564,9 +564,9 @@ static const struct dpu_mdp_cfg sc8180x_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -602,9 +602,9 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2BC, .bit_off = 20},
> @@ -631,9 +631,9 @@ static const struct dpu_mdp_cfg sm8350_mdp[] = {
> .reg_off = 0x2ac, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2b4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2bc, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2c4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2bc, .bit_off = 20},
> @@ -658,9 +658,9 @@ static const struct dpu_mdp_cfg sm8450_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2BC, .bit_off = 20},
> @@ -676,9 +676,9 @@ static const struct dpu_mdp_cfg sc7280_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> .reg_off = 0x2AC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -696,8 +696,8 @@ static const struct dpu_mdp_cfg sc8280xp_mdp[] = {
> .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},
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2bc, .bit_off = 8},
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2c4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20},
> },
> };
> @@ -724,9 +724,9 @@ static const struct dpu_mdp_cfg sm8550_mdp[] = {
> .reg_off = 0x28330, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2a330, .bit_off = 0},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA4] = {
> .reg_off = 0x2c330, .bit_off = 0},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA5] = {
> .reg_off = 0x2e330, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2bc, .bit_off = 20},
> @@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sc7180_vig_sblk_0 =
> @@ -1216,9 +1216,9 @@ static const struct dpu_sspp_cfg sc7180_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
> @@ -1254,9 +1254,9 @@ static const struct dpu_sspp_cfg sm8250_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8450_vig_sblk_0 =
> @@ -1282,9 +1282,9 @@ static const struct dpu_sspp_cfg sm8450_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
> @@ -1316,9 +1316,9 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_SDM845_MASK,
> sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> SSPP_BLK("sspp_12", SSPP_DMA4, 0x2c000, DMA_CURSOR_SDM845_MASK,
> - sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA4),
> SSPP_BLK("sspp_13", SSPP_DMA5, 0x2e000, DMA_CURSOR_SDM845_MASK,
> - sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA5),
> };
>
> static const struct dpu_sspp_cfg sc7280_sspp[] = {
> @@ -1327,9 +1327,9 @@ static const struct dpu_sspp_cfg sc7280_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_0 =
> @@ -1355,9 +1355,9 @@ static const struct dpu_sspp_cfg sc8280xp_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> #define _VIG_SBLK_NOSCALE(num, sdma_pri) \
> 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 56d98b4dd2ac..9c96920e1849 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -513,6 +513,8 @@ enum dpu_clk_ctrl_type {
> DPU_CLK_CTRL_DMA1,
> DPU_CLK_CTRL_DMA2,
> DPU_CLK_CTRL_DMA3,
> + DPU_CLK_CTRL_DMA4,
> + DPU_CLK_CTRL_DMA5,
> DPU_CLK_CTRL_CURSOR0,
> DPU_CLK_CTRL_CURSOR1,
> DPU_CLK_CTRL_INLINE_ROT0_SSPP,
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550 on top of next-20230116
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
@ 2023-01-16 15:12 ` Neil Armstrong
0 siblings, 0 replies; 26+ messages in thread
From: Neil Armstrong @ 2023-01-16 15:12 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
linux-arm-msm, dri-devel, freedreno
On 15/01/2023 13:41, Dmitry Baryshkov wrote:
> DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
> SSPP blocks used for the cursor planes. This has lead to the confusion
> at least for the MSM8998 platform. In preparation to supporting the
> cursor SSPP blocks, use proper enum values to index DMA SSPP clock
> controls.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
> 2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -522,9 +522,9 @@ static const struct dpu_mdp_cfg sdm845_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -539,9 +539,9 @@ static const struct dpu_mdp_cfg sc7180_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> .reg_off = 0x2AC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -564,9 +564,9 @@ static const struct dpu_mdp_cfg sc8180x_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -602,9 +602,9 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2BC, .bit_off = 20},
> @@ -631,9 +631,9 @@ static const struct dpu_mdp_cfg sm8350_mdp[] = {
> .reg_off = 0x2ac, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2b4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2bc, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2c4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2bc, .bit_off = 20},
> @@ -658,9 +658,9 @@ static const struct dpu_mdp_cfg sm8450_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2BC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2C4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2BC, .bit_off = 20},
> @@ -676,9 +676,9 @@ static const struct dpu_mdp_cfg sc7280_mdp[] = {
> .reg_off = 0x2AC, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
> .reg_off = 0x2AC, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
> .reg_off = 0x2B4, .bit_off = 8},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = {
> .reg_off = 0x2C4, .bit_off = 8},
> },
> };
> @@ -696,8 +696,8 @@ static const struct dpu_mdp_cfg sc8280xp_mdp[] = {
> .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},
> + .clk_ctrls[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x2bc, .bit_off = 8},
> + .clk_ctrls[DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2c4, .bit_off = 8},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20},
> },
> };
> @@ -724,9 +724,9 @@ static const struct dpu_mdp_cfg sm8550_mdp[] = {
> .reg_off = 0x28330, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_DMA3] = {
> .reg_off = 0x2a330, .bit_off = 0},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA4] = {
> .reg_off = 0x2c330, .bit_off = 0},
> - .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
> + .clk_ctrls[DPU_CLK_CTRL_DMA5] = {
> .reg_off = 0x2e330, .bit_off = 0},
> .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
> .reg_off = 0x2bc, .bit_off = 20},
> @@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sc7180_vig_sblk_0 =
> @@ -1216,9 +1216,9 @@ static const struct dpu_sspp_cfg sc7180_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
> @@ -1254,9 +1254,9 @@ static const struct dpu_sspp_cfg sm8250_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8450_vig_sblk_0 =
> @@ -1282,9 +1282,9 @@ static const struct dpu_sspp_cfg sm8450_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
> @@ -1316,9 +1316,9 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_SDM845_MASK,
> sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> SSPP_BLK("sspp_12", SSPP_DMA4, 0x2c000, DMA_CURSOR_SDM845_MASK,
> - sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA4),
> SSPP_BLK("sspp_13", SSPP_DMA5, 0x2e000, DMA_CURSOR_SDM845_MASK,
> - sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA5),
> };
>
> static const struct dpu_sspp_cfg sc7280_sspp[] = {
> @@ -1327,9 +1327,9 @@ static const struct dpu_sspp_cfg sc7280_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_0 =
> @@ -1355,9 +1355,9 @@ static const struct dpu_sspp_cfg sc8280xp_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> #define _VIG_SBLK_NOSCALE(num, sdma_pri) \
> 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 56d98b4dd2ac..9c96920e1849 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -513,6 +513,8 @@ enum dpu_clk_ctrl_type {
> DPU_CLK_CTRL_DMA1,
> DPU_CLK_CTRL_DMA2,
> DPU_CLK_CTRL_DMA3,
> + DPU_CLK_CTRL_DMA4,
> + DPU_CLK_CTRL_DMA5,
> DPU_CLK_CTRL_CURSOR0,
> DPU_CLK_CTRL_CURSOR1,
> DPU_CLK_CTRL_INLINE_ROT0_SSPP,
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550 on top of next-20230116
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-15 12:41 ` Dmitry Baryshkov
@ 2023-01-24 9:59 ` Marijn Suijten
-1 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 9:59 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> planes). Correct corresponding SSPP declarations.
>
> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
Drop the _CURSOR mask here? And the double space....
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
However, my downstream sources still define cursor SSPPs that are
missing here (after all, there's clk-ctrl for these already), at xin ID
2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
Or should you/I send that as a separate folloup patch?
- Marijn
> };
>
> static const struct dpu_sspp_cfg sdm845_sspp[] = {
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-24 9:59 ` Marijn Suijten
0 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 9:59 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> planes). Correct corresponding SSPP declarations.
>
> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
Drop the _CURSOR mask here? And the double space....
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
However, my downstream sources still define cursor SSPPs that are
missing here (after all, there's clk-ctrl for these already), at xin ID
2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
Or should you/I send that as a separate folloup patch?
- Marijn
> };
>
> static const struct dpu_sspp_cfg sdm845_sspp[] = {
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
2023-01-15 12:41 ` Dmitry Baryshkov
@ 2023-01-24 10:13 ` Marijn Suijten
-1 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 10:13 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 2023-01-15 14:41:43, Dmitry Baryshkov wrote:
> DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
> SSPP blocks used for the cursor planes. This has lead to the confusion
> at least for the MSM8998 platform. In preparation to supporting the
> cursor SSPP blocks, use proper enum values to index DMA SSPP clock
> controls.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550 on top of next-20230116
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
One question follows...
> ---
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
> 2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
<snip>
> @@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
Are these DMA pipes with CURSOR support, or how should I see this? For
MSM8998 I suggested to remove the _CURSOR bit since it has two dedicated
cursor pipes (not yet represented in the catalog) but these SoCs don't
seem to have those.
- Marijn
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sc7180_vig_sblk_0 =
> @@ -1216,9 +1216,9 @@ static const struct dpu_sspp_cfg sc7180_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
> @@ -1254,9 +1254,9 @@ static const struct dpu_sspp_cfg sm8250_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8450_vig_sblk_0 =
> @@ -1282,9 +1282,9 @@ static const struct dpu_sspp_cfg sm8450_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
> @@ -1316,9 +1316,9 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_SDM845_MASK,
> sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> SSPP_BLK("sspp_12", SSPP_DMA4, 0x2c000, DMA_CURSOR_SDM845_MASK,
> - sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA4),
> SSPP_BLK("sspp_13", SSPP_DMA5, 0x2e000, DMA_CURSOR_SDM845_MASK,
> - sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA5),
> };
>
> static const struct dpu_sspp_cfg sc7280_sspp[] = {
> @@ -1327,9 +1327,9 @@ static const struct dpu_sspp_cfg sc7280_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_0 =
> @@ -1355,9 +1355,9 @@ static const struct dpu_sspp_cfg sc8280xp_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> #define _VIG_SBLK_NOSCALE(num, sdma_pri) \
> 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 56d98b4dd2ac..9c96920e1849 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -513,6 +513,8 @@ enum dpu_clk_ctrl_type {
> DPU_CLK_CTRL_DMA1,
> DPU_CLK_CTRL_DMA2,
> DPU_CLK_CTRL_DMA3,
> + DPU_CLK_CTRL_DMA4,
> + DPU_CLK_CTRL_DMA5,
> DPU_CLK_CTRL_CURSOR0,
> DPU_CLK_CTRL_CURSOR1,
> DPU_CLK_CTRL_INLINE_ROT0_SSPP,
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
@ 2023-01-24 10:13 ` Marijn Suijten
0 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 10:13 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, linux-arm-msm,
Bjorn Andersson, dri-devel, Stephen Boyd, Daniel Vetter,
David Airlie
On 2023-01-15 14:41:43, Dmitry Baryshkov wrote:
> DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
> SSPP blocks used for the cursor planes. This has lead to the confusion
> at least for the MSM8998 platform. In preparation to supporting the
> cursor SSPP blocks, use proper enum values to index DMA SSPP clock
> controls.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550 on top of next-20230116
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
One question follows...
> ---
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
> 2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
<snip>
> @@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
Are these DMA pipes with CURSOR support, or how should I see this? For
MSM8998 I suggested to remove the _CURSOR bit since it has two dedicated
cursor pipes (not yet represented in the catalog) but these SoCs don't
seem to have those.
- Marijn
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sc7180_vig_sblk_0 =
> @@ -1216,9 +1216,9 @@ static const struct dpu_sspp_cfg sc7180_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
> @@ -1254,9 +1254,9 @@ static const struct dpu_sspp_cfg sm8250_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8450_vig_sblk_0 =
> @@ -1282,9 +1282,9 @@ static const struct dpu_sspp_cfg sm8450_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
> @@ -1316,9 +1316,9 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_SDM845_MASK,
> sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> SSPP_BLK("sspp_12", SSPP_DMA4, 0x2c000, DMA_CURSOR_SDM845_MASK,
> - sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sm8550_dma_sblk_4, 14, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA4),
> SSPP_BLK("sspp_13", SSPP_DMA5, 0x2e000, DMA_CURSOR_SDM845_MASK,
> - sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sd8550_dma_sblk_5, 15, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA5),
> };
>
> static const struct dpu_sspp_cfg sc7280_sspp[] = {
> @@ -1327,9 +1327,9 @@ static const struct dpu_sspp_cfg sc7280_sspp[] = {
> SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
> sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> };
>
> static const struct dpu_sspp_sub_blks sc8280xp_vig_sblk_0 =
> @@ -1355,9 +1355,9 @@ static const struct dpu_sspp_cfg sc8280xp_sspp[] = {
> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_SDM845_MASK,
> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> };
>
> #define _VIG_SBLK_NOSCALE(num, sdma_pri) \
> 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 56d98b4dd2ac..9c96920e1849 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -513,6 +513,8 @@ enum dpu_clk_ctrl_type {
> DPU_CLK_CTRL_DMA1,
> DPU_CLK_CTRL_DMA2,
> DPU_CLK_CTRL_DMA3,
> + DPU_CLK_CTRL_DMA4,
> + DPU_CLK_CTRL_DMA5,
> DPU_CLK_CTRL_CURSOR0,
> DPU_CLK_CTRL_CURSOR1,
> DPU_CLK_CTRL_INLINE_ROT0_SSPP,
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-24 9:59 ` Marijn Suijten
@ 2023-01-24 10:19 ` Dmitry Baryshkov
-1 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 10:19 UTC (permalink / raw)
To: Marijn Suijten
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On 24/01/2023 11:59, Marijn Suijten wrote:
> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
>> planes). Correct corresponding SSPP declarations.
>>
>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
>
> Drop the _CURSOR mask here? And the double space....
Ack for the doublespace. By removing _CURSOR we would disallow using
these planes as hw cursor planes. This would switch all compositors into
sw cursor mode, thus damaging the performance.
>
>> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
>> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
>> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
>> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
>> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
>
> Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
> using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
>
> However, my downstream sources still define cursor SSPPs that are
> missing here (after all, there's clk-ctrl for these already), at xin ID
> 2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
>
> SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
> cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
I think this should not be the DMA_CURSOR_MSM8998_MASK, but don't bet on
my words, I haven't check actual cursor plane capabilities.
> cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
>
> Or should you/I send that as a separate folloup patch?
Ideally one can add these two planes and then switch two mentioned DMA
planes to plain DMA_MSM8998_MASK.
>
> - Marijn
>
>> };
>>
>> static const struct dpu_sspp_cfg sdm845_sspp[] = {
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-24 10:19 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 10:19 UTC (permalink / raw)
To: Marijn Suijten
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 24/01/2023 11:59, Marijn Suijten wrote:
> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
>> planes). Correct corresponding SSPP declarations.
>>
>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
>
> Drop the _CURSOR mask here? And the double space....
Ack for the doublespace. By removing _CURSOR we would disallow using
these planes as hw cursor planes. This would switch all compositors into
sw cursor mode, thus damaging the performance.
>
>> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
>> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
>> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
>> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
>> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
>
> Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
> using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
>
> However, my downstream sources still define cursor SSPPs that are
> missing here (after all, there's clk-ctrl for these already), at xin ID
> 2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
>
> SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
> cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
I think this should not be the DMA_CURSOR_MSM8998_MASK, but don't bet on
my words, I haven't check actual cursor plane capabilities.
> cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
>
> Or should you/I send that as a separate folloup patch?
Ideally one can add these two planes and then switch two mentioned DMA
planes to plain DMA_MSM8998_MASK.
>
> - Marijn
>
>> };
>>
>> static const struct dpu_sspp_cfg sdm845_sspp[] = {
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
2023-01-24 10:13 ` Marijn Suijten
@ 2023-01-24 10:20 ` Dmitry Baryshkov
-1 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 10:20 UTC (permalink / raw)
To: Marijn Suijten
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, linux-arm-msm,
Bjorn Andersson, dri-devel, Stephen Boyd, Daniel Vetter,
David Airlie
On 24/01/2023 12:13, Marijn Suijten wrote:
> On 2023-01-15 14:41:43, Dmitry Baryshkov wrote:
>> DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
>> SSPP blocks used for the cursor planes. This has lead to the confusion
>> at least for the MSM8998 platform. In preparation to supporting the
>> cursor SSPP blocks, use proper enum values to index DMA SSPP clock
>> controls.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
>> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550 on top of next-20230116
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> One question follows...
>
>> ---
>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
>> 2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> <snip>
>> @@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
>
> Are these DMA pipes with CURSOR support, or how should I see this? For
> MSM8998 I suggested to remove the _CURSOR bit since it has two dedicated
> cursor pipes (not yet represented in the catalog) but these SoCs don't
> seem to have those.
>
As I wrote earlier, this part just marks them to be used for HW cursor
support.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks
@ 2023-01-24 10:20 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 10:20 UTC (permalink / raw)
To: Marijn Suijten
Cc: Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 24/01/2023 12:13, Marijn Suijten wrote:
> On 2023-01-15 14:41:43, Dmitry Baryshkov wrote:
>> DPU driver has been using the DPU_CLK_CTRL_CURSOR prefix for the DMA
>> SSPP blocks used for the cursor planes. This has lead to the confusion
>> at least for the MSM8998 platform. In preparation to supporting the
>> cursor SSPP blocks, use proper enum values to index DMA SSPP clock
>> controls.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
>> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550 on top of next-20230116
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> One question follows...
>
>> ---
>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 64 +++++++++----------
>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +
>> 2 files changed, 34 insertions(+), 32 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 ad0c55464154..b0f6e071fe4b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> <snip>
>> @@ -1199,9 +1199,9 @@ static const struct dpu_sspp_cfg sdm845_sspp[] = {
>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK,
>
> Are these DMA pipes with CURSOR support, or how should I see this? For
> MSM8998 I suggested to remove the _CURSOR bit since it has two dedicated
> cursor pipes (not yet represented in the catalog) but these SoCs don't
> seem to have those.
>
As I wrote earlier, this part just marks them to be used for HW cursor
support.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-24 10:19 ` Dmitry Baryshkov
@ 2023-01-24 11:12 ` Marijn Suijten
-1 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 11:12 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> On 24/01/2023 11:59, Marijn Suijten wrote:
> > On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> >> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> >> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> >> planes). Correct corresponding SSPP declarations.
> >>
> >> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> >> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> >> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> >> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> >> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> >> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> >
> > Drop the _CURSOR mask here? And the double space....
>
> Ack for the doublespace. By removing _CURSOR we would disallow using
> these planes as hw cursor planes. This would switch all compositors into
> sw cursor mode, thus damaging the performance.
Doesn't that require special hardware support, or can any DMA pipe
support "hw cursor mode/planes", whatever that means? Sorry for not
being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
have a different set of features / capabilities.
Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
to believe that it's mostly to let userspace use these DMA pipes for
cursors (having cursor planes available in uapi) rather than requiring
any special hardware support (though semantics do seem to change in a
nontrivial way).
> >> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> >> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> >> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
> >> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> >> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> >
> > Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
> > using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
> >
> > However, my downstream sources still define cursor SSPPs that are
> > missing here (after all, there's clk-ctrl for these already), at xin ID
> > 2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
> >
> > SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
> > cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> > SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
>
> I think this should not be the DMA_CURSOR_MSM8998_MASK, but don't bet on
> my words, I haven't check actual cursor plane capabilities.
As we've seen in [1] (specifically [2]) there are a few more driver/hw
changes required to properly implement/support DPU_SSPP_CURSOR?
[1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
[2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
- Marijn
> > cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
> >
> > Or should you/I send that as a separate folloup patch?
>
> Ideally one can add these two planes and then switch two mentioned DMA
> planes to plain DMA_MSM8998_MASK.
>
> >
> > - Marijn
> >
> >> };
> >>
> >> static const struct dpu_sspp_cfg sdm845_sspp[] = {
>
> --
> With best wishes
> Dmitry
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-24 11:12 ` Marijn Suijten
0 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 11:12 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> On 24/01/2023 11:59, Marijn Suijten wrote:
> > On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> >> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> >> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> >> planes). Correct corresponding SSPP declarations.
> >>
> >> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> >> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> >> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> >> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> >> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> >> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> >
> > Drop the _CURSOR mask here? And the double space....
>
> Ack for the doublespace. By removing _CURSOR we would disallow using
> these planes as hw cursor planes. This would switch all compositors into
> sw cursor mode, thus damaging the performance.
Doesn't that require special hardware support, or can any DMA pipe
support "hw cursor mode/planes", whatever that means? Sorry for not
being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
have a different set of features / capabilities.
Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
to believe that it's mostly to let userspace use these DMA pipes for
cursors (having cursor planes available in uapi) rather than requiring
any special hardware support (though semantics do seem to change in a
nontrivial way).
> >> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> >> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> >> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
> >> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> >> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> >
> > Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
> > using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
> >
> > However, my downstream sources still define cursor SSPPs that are
> > missing here (after all, there's clk-ctrl for these already), at xin ID
> > 2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
> >
> > SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
> > cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> > SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
>
> I think this should not be the DMA_CURSOR_MSM8998_MASK, but don't bet on
> my words, I haven't check actual cursor plane capabilities.
As we've seen in [1] (specifically [2]) there are a few more driver/hw
changes required to properly implement/support DPU_SSPP_CURSOR?
[1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
[2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
- Marijn
> > cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
> >
> > Or should you/I send that as a separate folloup patch?
>
> Ideally one can add these two planes and then switch two mentioned DMA
> planes to plain DMA_MSM8998_MASK.
>
> >
> > - Marijn
> >
> >> };
> >>
> >> static const struct dpu_sspp_cfg sdm845_sspp[] = {
>
> --
> With best wishes
> Dmitry
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-24 11:12 ` Marijn Suijten
@ 2023-01-24 11:20 ` Dmitry Baryshkov
-1 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 11:20 UTC (permalink / raw)
To: Marijn Suijten
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> > On 24/01/2023 11:59, Marijn Suijten wrote:
> > > On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> > >> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> > >> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> > >> planes). Correct corresponding SSPP declarations.
> > >>
> > >> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> > >> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> > >> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >> ---
> > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> > >> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> > >> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> > >> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> > >> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> > >
> > > Drop the _CURSOR mask here? And the double space....
> >
> > Ack for the doublespace. By removing _CURSOR we would disallow using
> > these planes as hw cursor planes. This would switch all compositors into
> > sw cursor mode, thus damaging the performance.
>
> Doesn't that require special hardware support, or can any DMA pipe
> support "hw cursor mode/planes", whatever that means? Sorry for not
> being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
> have a different set of features / capabilities.
Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
used internally to tell the DRM core that the corresponding plane is
going to be used as a "userspace cursor" plane.
> Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
> to believe that it's mostly to let userspace use these DMA pipes for
> cursors (having cursor planes available in uapi) rather than requiring
> any special hardware support (though semantics do seem to change in a
> nontrivial way).
Correct.
DRM/userspace cursor planes = planes which userspace can use to draw
mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
using them
DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
additional features, targeting HW cursor only, not present since
sdm845
DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
'DRM/userspace cursor plane'.
>
> > >> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> > >> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> > >> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
> > >> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> > >> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> > >
> > > Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
> > > using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
> > >
> > > However, my downstream sources still define cursor SSPPs that are
> > > missing here (after all, there's clk-ctrl for these already), at xin ID
> > > 2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
> > >
> > > SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
> > > cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> > > SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
> >
> > I think this should not be the DMA_CURSOR_MSM8998_MASK, but don't bet on
> > my words, I haven't check actual cursor plane capabilities.
>
> As we've seen in [1] (specifically [2]) there are a few more driver/hw
> changes required to properly implement/support DPU_SSPP_CURSOR?
>
> [1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
> [2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
>
> - Marijn
>
> > > cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
> > >
> > > Or should you/I send that as a separate folloup patch?
> >
> > Ideally one can add these two planes and then switch two mentioned DMA
> > planes to plain DMA_MSM8998_MASK.
> >
> > >
> > > - Marijn
> > >
> > >> };
> > >>
> > >> static const struct dpu_sspp_cfg sdm845_sspp[] = {
> >
> > --
> > With best wishes
> > Dmitry
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-24 11:20 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 11:20 UTC (permalink / raw)
To: Marijn Suijten
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> > On 24/01/2023 11:59, Marijn Suijten wrote:
> > > On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> > >> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> > >> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> > >> planes). Correct corresponding SSPP declarations.
> > >>
> > >> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> > >> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> > >> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >> ---
> > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> > >> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > >> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> > >> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> > >> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> > >> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> > >
> > > Drop the _CURSOR mask here? And the double space....
> >
> > Ack for the doublespace. By removing _CURSOR we would disallow using
> > these planes as hw cursor planes. This would switch all compositors into
> > sw cursor mode, thus damaging the performance.
>
> Doesn't that require special hardware support, or can any DMA pipe
> support "hw cursor mode/planes", whatever that means? Sorry for not
> being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
> have a different set of features / capabilities.
Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
used internally to tell the DRM core that the corresponding plane is
going to be used as a "userspace cursor" plane.
> Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
> to believe that it's mostly to let userspace use these DMA pipes for
> cursors (having cursor planes available in uapi) rather than requiring
> any special hardware support (though semantics do seem to change in a
> nontrivial way).
Correct.
DRM/userspace cursor planes = planes which userspace can use to draw
mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
using them
DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
additional features, targeting HW cursor only, not present since
sdm845
DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
'DRM/userspace cursor plane'.
>
> > >> - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> > >> + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> > >> SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, DMA_CURSOR_MSM8998_MASK,
> > >> - sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> > >> + sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> > >
> > > Yes, msm8998_mdp defines both DMA2/3 and CURSOR0/1 clocks. R-b after
> > > using DMA_MSM8998_MASK without the DPU_SSPP_CURSOR bit.
> > >
> > > However, my downstream sources still define cursor SSPPs that are
> > > missing here (after all, there's clk-ctrl for these already), at xin ID
> > > 2 and 10 with addresses 0x3500 and 0x37000 downstream (-0x1000 here):
> > >
> > > SSPP_BLK("sspp_?", SSPP_CURSOR0, 0x34000, DMA_CURSOR_SM8998_MASK,
> > > cursor sblk?, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> > > SSPP_BLK("sspp_?", SSPP_CURSOR1, 0x36000, DMA_CURSOR_SM8998_MASK,
> >
> > I think this should not be the DMA_CURSOR_MSM8998_MASK, but don't bet on
> > my words, I haven't check actual cursor plane capabilities.
>
> As we've seen in [1] (specifically [2]) there are a few more driver/hw
> changes required to properly implement/support DPU_SSPP_CURSOR?
>
> [1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
> [2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
>
> - Marijn
>
> > > cursor sblk?, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
> > >
> > > Or should you/I send that as a separate folloup patch?
> >
> > Ideally one can add these two planes and then switch two mentioned DMA
> > planes to plain DMA_MSM8998_MASK.
> >
> > >
> > > - Marijn
> > >
> > >> };
> > >>
> > >> static const struct dpu_sspp_cfg sdm845_sspp[] = {
> >
> > --
> > With best wishes
> > Dmitry
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-24 11:20 ` Dmitry Baryshkov
@ 2023-01-24 12:06 ` Marijn Suijten
-1 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 12:06 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On 2023-01-24 13:20:57, Dmitry Baryshkov wrote:
> On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> > > On 24/01/2023 11:59, Marijn Suijten wrote:
> > > > On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> > > >> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> > > >> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> > > >> planes). Correct corresponding SSPP declarations.
> > > >>
> > > >> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> > > >> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> > > >> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> > > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > >> ---
> > > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> > > >> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> > > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > >> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> > > >> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> > > >> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> > > >> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> > > >
> > > > Drop the _CURSOR mask here? And the double space....
> > >
> > > Ack for the doublespace. By removing _CURSOR we would disallow using
> > > these planes as hw cursor planes. This would switch all compositors into
> > > sw cursor mode, thus damaging the performance.
> >
> > Doesn't that require special hardware support, or can any DMA pipe
> > support "hw cursor mode/planes", whatever that means? Sorry for not
> > being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
> > have a different set of features / capabilities.
>
> Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
> DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
> used internally to tell the DRM core that the corresponding plane is
> going to be used as a "userspace cursor" plane.
Different capabilities for userspace, but not in terms hardware (/driver
support, yet)? If so, then:
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> > Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
> > to believe that it's mostly to let userspace use these DMA pipes for
> > cursors (having cursor planes available in uapi) rather than requiring
> > any special hardware support (though semantics do seem to change in a
> > nontrivial way).
>
> Correct.
> DRM/userspace cursor planes = planes which userspace can use to draw
> mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
> using them
> DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
But these DMA pipes are _not_ lightweight/limited?
> additional features, targeting HW cursor only, not present since
> sdm845
> DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
> 'DRM/userspace cursor plane'.
Ack, so it's not toggling anything hardware specific /yet/. However,
does this prevent userspace from using these pipes/planes for other DMA
purposes as they're marked as a different _type_ of plane? And will
that change when we do end up "implementing more rigorous/strict
hardware support"? For the other SoCs, are their DMA pipes also
featureful and would the presence of DPU_SSPP_CURSOR severely limit its
functionality? And is this thing that "virtual planes" would be going
to "solve"?
<snip>
> > As we've seen in [1] (specifically [2]) there are a few more driver/hw
> > changes required to properly implement/support DPU_SSPP_CURSOR?
> >
> > [1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
> > [2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
Referring to changes like these ^.
- Marijn
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-24 12:06 ` Marijn Suijten
0 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 12:06 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 2023-01-24 13:20:57, Dmitry Baryshkov wrote:
> On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> > > On 24/01/2023 11:59, Marijn Suijten wrote:
> > > > On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> > > >> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> > > >> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> > > >> planes). Correct corresponding SSPP declarations.
> > > >>
> > > >> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> > > >> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> > > >> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> > > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > >> ---
> > > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> > > >> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> > > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > >> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> > > >> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> > > >> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> > > >> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> > > >
> > > > Drop the _CURSOR mask here? And the double space....
> > >
> > > Ack for the doublespace. By removing _CURSOR we would disallow using
> > > these planes as hw cursor planes. This would switch all compositors into
> > > sw cursor mode, thus damaging the performance.
> >
> > Doesn't that require special hardware support, or can any DMA pipe
> > support "hw cursor mode/planes", whatever that means? Sorry for not
> > being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
> > have a different set of features / capabilities.
>
> Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
> DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
> used internally to tell the DRM core that the corresponding plane is
> going to be used as a "userspace cursor" plane.
Different capabilities for userspace, but not in terms hardware (/driver
support, yet)? If so, then:
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> > Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
> > to believe that it's mostly to let userspace use these DMA pipes for
> > cursors (having cursor planes available in uapi) rather than requiring
> > any special hardware support (though semantics do seem to change in a
> > nontrivial way).
>
> Correct.
> DRM/userspace cursor planes = planes which userspace can use to draw
> mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
> using them
> DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
But these DMA pipes are _not_ lightweight/limited?
> additional features, targeting HW cursor only, not present since
> sdm845
> DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
> 'DRM/userspace cursor plane'.
Ack, so it's not toggling anything hardware specific /yet/. However,
does this prevent userspace from using these pipes/planes for other DMA
purposes as they're marked as a different _type_ of plane? And will
that change when we do end up "implementing more rigorous/strict
hardware support"? For the other SoCs, are their DMA pipes also
featureful and would the presence of DPU_SSPP_CURSOR severely limit its
functionality? And is this thing that "virtual planes" would be going
to "solve"?
<snip>
> > As we've seen in [1] (specifically [2]) there are a few more driver/hw
> > changes required to properly implement/support DPU_SSPP_CURSOR?
> >
> > [1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
> > [2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
Referring to changes like these ^.
- Marijn
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-24 12:06 ` Marijn Suijten
@ 2023-01-24 12:29 ` Dmitry Baryshkov
-1 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 12:29 UTC (permalink / raw)
To: Marijn Suijten
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On 24/01/2023 14:06, Marijn Suijten wrote:
> On 2023-01-24 13:20:57, Dmitry Baryshkov wrote:
>> On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
>> <marijn.suijten@somainline.org> wrote:
>>>
>>> On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
>>>> On 24/01/2023 11:59, Marijn Suijten wrote:
>>>>> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
>>>>>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
>>>>>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
>>>>>> planes). Correct corresponding SSPP declarations.
>>>>>>
>>>>>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
>>>>>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>>>>>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>> ---
>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
>>>>>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
>>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>>>>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
>>>>>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
>>>>>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>>>>>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
>>>>>
>>>>> Drop the _CURSOR mask here? And the double space....
>>>>
>>>> Ack for the doublespace. By removing _CURSOR we would disallow using
>>>> these planes as hw cursor planes. This would switch all compositors into
>>>> sw cursor mode, thus damaging the performance.
>>>
>>> Doesn't that require special hardware support, or can any DMA pipe
>>> support "hw cursor mode/planes", whatever that means? Sorry for not
>>> being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
>>> have a different set of features / capabilities.
>>
>> Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
>> DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
>> used internally to tell the DRM core that the corresponding plane is
>> going to be used as a "userspace cursor" plane.
>
> Different capabilities for userspace, but not in terms hardware (/driver
> support, yet)? If so, then:
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
>>> Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
>>> to believe that it's mostly to let userspace use these DMA pipes for
>>> cursors (having cursor planes available in uapi) rather than requiring
>>> any special hardware support (though semantics do seem to change in a
>>> nontrivial way).
>>
>> Correct.
>> DRM/userspace cursor planes = planes which userspace can use to draw
>> mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
>> using them
>> DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
>
> But these DMA pipes are _not_ lightweight/limited?
No, they are not.
>
>> additional features, targeting HW cursor only, not present since
>> sdm845
>> DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
>> 'DRM/userspace cursor plane'.
>
> Ack, so it's not toggling anything hardware specific /yet/. However,
> does this prevent userspace from using these pipes/planes for other DMA
> purposes as they're marked as a different _type_ of plane?
This is what 'universal planes' API is solving.
Historically there were three kinds of planes: primary (iow background),
cursor and overlay.
By default an application sees only the overlay planes and has some
additional API to manipulate cursors and backgrounds.
Then at some point it was found that this split is not worth all the
troubles, since applications can better utilize the hardware if they can
decide on their own what should be done. So now we still have all three
kinds of planes (for the legacy userspace), but behind the scenes they
all are the same. If an application knows how to knock the door, it will
see all the planes with the capabilities being exposed through plane
properties, etc.
Back to our case. We mark these planes as 'cursor' ones, to let the
legacy composers to use them for hardware cursor. I think it was decided
that not having the cursor is worse than requiring another blending
step. On the other hand newer composers see a full array of planes.
> And will
> that change when we do end up "implementing more rigorous/strict
> hardware support"?
Once implemented, there will be more planes for msm8998 (and eventually
sdm660/630, once we have them too). Some of them will be limited in size
or in the Z order (cursor), some will not (rgb, dma, vig).
> For the other SoCs, are their DMA pipes also
> featureful and would the presence of DPU_SSPP_CURSOR severely limit its
> functionality?
All DMA pipes have the same set of features (in the same generation of
course).
No, it's just a software marker.
> And is this thing that "virtual planes" would be going
> to "solve"?
Included. The virtual planes is trying to solve a slightly different
part of the story: to remove 1:1 correspondence between planes and
pipes. Sometimes it would be nice to use two HW pipes for a single DRM
plane (e.g. the kernel expects to have a single primary plane whose
resolution matches the resolution of the CRTC, 4k = two SSPP because of
hardware limitations). Sometimes a single HW pipe can be used to drive
two DRM planes (see multirect). So, pretty much in the same way as we
use one or two LMs to drive a CRTC, it is useful to use 1/2, 1 or 2
SSPPs to drive a single DRM plane.
>
> <snip>
>
>>> As we've seen in [1] (specifically [2]) there are a few more driver/hw
>>> changes required to properly implement/support DPU_SSPP_CURSOR?
>>>
>>> [1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
>>> [2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
>
> Referring to changes like these ^.
>
> - Marijn
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-24 12:29 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-01-24 12:29 UTC (permalink / raw)
To: Marijn Suijten
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 24/01/2023 14:06, Marijn Suijten wrote:
> On 2023-01-24 13:20:57, Dmitry Baryshkov wrote:
>> On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
>> <marijn.suijten@somainline.org> wrote:
>>>
>>> On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
>>>> On 24/01/2023 11:59, Marijn Suijten wrote:
>>>>> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
>>>>>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
>>>>>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
>>>>>> planes). Correct corresponding SSPP declarations.
>>>>>>
>>>>>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
>>>>>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>>>>>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>> ---
>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
>>>>>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
>>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>>>>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
>>>>>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
>>>>>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>>>>>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
>>>>>
>>>>> Drop the _CURSOR mask here? And the double space....
>>>>
>>>> Ack for the doublespace. By removing _CURSOR we would disallow using
>>>> these planes as hw cursor planes. This would switch all compositors into
>>>> sw cursor mode, thus damaging the performance.
>>>
>>> Doesn't that require special hardware support, or can any DMA pipe
>>> support "hw cursor mode/planes", whatever that means? Sorry for not
>>> being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
>>> have a different set of features / capabilities.
>>
>> Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
>> DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
>> used internally to tell the DRM core that the corresponding plane is
>> going to be used as a "userspace cursor" plane.
>
> Different capabilities for userspace, but not in terms hardware (/driver
> support, yet)? If so, then:
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
>>> Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
>>> to believe that it's mostly to let userspace use these DMA pipes for
>>> cursors (having cursor planes available in uapi) rather than requiring
>>> any special hardware support (though semantics do seem to change in a
>>> nontrivial way).
>>
>> Correct.
>> DRM/userspace cursor planes = planes which userspace can use to draw
>> mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
>> using them
>> DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
>
> But these DMA pipes are _not_ lightweight/limited?
No, they are not.
>
>> additional features, targeting HW cursor only, not present since
>> sdm845
>> DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
>> 'DRM/userspace cursor plane'.
>
> Ack, so it's not toggling anything hardware specific /yet/. However,
> does this prevent userspace from using these pipes/planes for other DMA
> purposes as they're marked as a different _type_ of plane?
This is what 'universal planes' API is solving.
Historically there were three kinds of planes: primary (iow background),
cursor and overlay.
By default an application sees only the overlay planes and has some
additional API to manipulate cursors and backgrounds.
Then at some point it was found that this split is not worth all the
troubles, since applications can better utilize the hardware if they can
decide on their own what should be done. So now we still have all three
kinds of planes (for the legacy userspace), but behind the scenes they
all are the same. If an application knows how to knock the door, it will
see all the planes with the capabilities being exposed through plane
properties, etc.
Back to our case. We mark these planes as 'cursor' ones, to let the
legacy composers to use them for hardware cursor. I think it was decided
that not having the cursor is worse than requiring another blending
step. On the other hand newer composers see a full array of planes.
> And will
> that change when we do end up "implementing more rigorous/strict
> hardware support"?
Once implemented, there will be more planes for msm8998 (and eventually
sdm660/630, once we have them too). Some of them will be limited in size
or in the Z order (cursor), some will not (rgb, dma, vig).
> For the other SoCs, are their DMA pipes also
> featureful and would the presence of DPU_SSPP_CURSOR severely limit its
> functionality?
All DMA pipes have the same set of features (in the same generation of
course).
No, it's just a software marker.
> And is this thing that "virtual planes" would be going
> to "solve"?
Included. The virtual planes is trying to solve a slightly different
part of the story: to remove 1:1 correspondence between planes and
pipes. Sometimes it would be nice to use two HW pipes for a single DRM
plane (e.g. the kernel expects to have a single primary plane whose
resolution matches the resolution of the CRTC, 4k = two SSPP because of
hardware limitations). Sometimes a single HW pipe can be used to drive
two DRM planes (see multirect). So, pretty much in the same way as we
use one or two LMs to drive a CRTC, it is useful to use 1/2, 1 or 2
SSPPs to drive a single DRM plane.
>
> <snip>
>
>>> As we've seen in [1] (specifically [2]) there are a few more driver/hw
>>> changes required to properly implement/support DPU_SSPP_CURSOR?
>>>
>>> [1]: https://github.com/rawoul/linux/commits/next_20220624-msm8998-hdmi
>>> [2]; https://github.com/rawoul/linux/commit/7d8d739cfbfa551120868986d5824f7b2b116ac1
>
> Referring to changes like these ^.
>
> - Marijn
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-24 12:29 ` Dmitry Baryshkov
@ 2023-01-24 14:00 ` Marijn Suijten
-1 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 14:00 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 2023-01-24 14:29:38, Dmitry Baryshkov wrote:
> On 24/01/2023 14:06, Marijn Suijten wrote:
> > On 2023-01-24 13:20:57, Dmitry Baryshkov wrote:
> >> On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
> >> <marijn.suijten@somainline.org> wrote:
> >>>
> >>> On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> >>>> On 24/01/2023 11:59, Marijn Suijten wrote:
> >>>>> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> >>>>>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> >>>>>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> >>>>>> planes). Correct corresponding SSPP declarations.
> >>>>>>
> >>>>>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> >>>>>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> >>>>>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> >>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>>> ---
> >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> >>>>>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> >>>>>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> >>>>>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> >>>>>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> >>>>>
> >>>>> Drop the _CURSOR mask here? And the double space....
> >>>>
> >>>> Ack for the doublespace. By removing _CURSOR we would disallow using
> >>>> these planes as hw cursor planes. This would switch all compositors into
> >>>> sw cursor mode, thus damaging the performance.
> >>>
> >>> Doesn't that require special hardware support, or can any DMA pipe
> >>> support "hw cursor mode/planes", whatever that means? Sorry for not
> >>> being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
> >>> have a different set of features / capabilities.
> >>
> >> Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
> >> DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
> >> used internally to tell the DRM core that the corresponding plane is
> >> going to be used as a "userspace cursor" plane.
> >
> > Different capabilities for userspace, but not in terms hardware (/driver
> > support, yet)? If so, then:
> >
> > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> >
> >>> Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
> >>> to believe that it's mostly to let userspace use these DMA pipes for
> >>> cursors (having cursor planes available in uapi) rather than requiring
> >>> any special hardware support (though semantics do seem to change in a
> >>> nontrivial way).
> >>
> >> Correct.
> >> DRM/userspace cursor planes = planes which userspace can use to draw
> >> mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
> >> using them
> >> DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
> >
> > But these DMA pipes are _not_ lightweight/limited?
>
> No, they are not.
>
> >
> >> additional features, targeting HW cursor only, not present since
> >> sdm845
> >> DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
> >> 'DRM/userspace cursor plane'.
> >
> > Ack, so it's not toggling anything hardware specific /yet/. However,
> > does this prevent userspace from using these pipes/planes for other DMA
> > purposes as they're marked as a different _type_ of plane?
>
> This is what 'universal planes' API is solving.
>
> Historically there were three kinds of planes: primary (iow background),
> cursor and overlay.
> By default an application sees only the overlay planes and has some
> additional API to manipulate cursors and backgrounds.
>
> Then at some point it was found that this split is not worth all the
> troubles, since applications can better utilize the hardware if they can
> decide on their own what should be done. So now we still have all three
> kinds of planes (for the legacy userspace), but behind the scenes they
> all are the same. If an application knows how to knock the door, it will
> see all the planes with the capabilities being exposed through plane
> properties, etc.
>
> Back to our case. We mark these planes as 'cursor' ones, to let the
> legacy composers to use them for hardware cursor. I think it was decided
> that not having the cursor is worse than requiring another blending
> step. On the other hand newer composers see a full array of planes.
Thanks so much for this backstory, that explains why it shouldn't harm
modern compositors; in modetest I see these planes have the cursor type
property, but no restriction on zpos for example.
> > And will
> > that change when we do end up "implementing more rigorous/strict
> > hardware support"?
>
> Once implemented, there will be more planes for msm8998 (and eventually
> sdm660/630, once we have them too). Some of them will be limited in size
> or in the Z order (cursor), some will not (rgb, dma, vig).
That's what I saw one of the linked patches; at that point we should add
separate feature bits for this so as to not limit the size or Z-order
range for these DMA-pipes-disguised-as-cursor-planes.
> > For the other SoCs, are their DMA pipes also
> > featureful and would the presence of DPU_SSPP_CURSOR severely limit its
> > functionality?
>
> All DMA pipes have the same set of features (in the same generation of
> course).
> No, it's just a software marker.
Ack yes, so again once the software marker starts limiting properties
for actual CURSOR support (on pre-msm8998) we should distinguish them
from DMA pipes that are simply marked as cursor planes...
> > And is this thing that "virtual planes" would be going
> > to "solve"?
>
> Included. The virtual planes is trying to solve a slightly different
> part of the story: to remove 1:1 correspondence between planes and
> pipes. Sometimes it would be nice to use two HW pipes for a single DRM
> plane (e.g. the kernel expects to have a single primary plane whose
> resolution matches the resolution of the CRTC, 4k = two SSPP because of
> hardware limitations). Sometimes a single HW pipe can be used to drive
> two DRM planes (see multirect). So, pretty much in the same way as we
> use one or two LMs to drive a CRTC, it is useful to use 1/2, 1 or 2
> SSPPs to drive a single DRM plane.
Ah, ack, that makes sense, and it wouldn't / shouldn't be up to
userspace to assign and use the pipes on its own.
- Marijn
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-01-24 14:00 ` Marijn Suijten
0 siblings, 0 replies; 26+ messages in thread
From: Marijn Suijten @ 2023-01-24 14:00 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On 2023-01-24 14:29:38, Dmitry Baryshkov wrote:
> On 24/01/2023 14:06, Marijn Suijten wrote:
> > On 2023-01-24 13:20:57, Dmitry Baryshkov wrote:
> >> On Tue, 24 Jan 2023 at 13:12, Marijn Suijten
> >> <marijn.suijten@somainline.org> wrote:
> >>>
> >>> On 2023-01-24 12:19:27, Dmitry Baryshkov wrote:
> >>>> On 24/01/2023 11:59, Marijn Suijten wrote:
> >>>>> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
> >>>>>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
> >>>>>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
> >>>>>> planes). Correct corresponding SSPP declarations.
> >>>>>>
> >>>>>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
> >>>>>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> >>>>>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
> >>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>>> ---
> >>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
> >>>>>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
> >>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >>>>>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> >>>>>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
> >>>>>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> >>>>>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
> >>>>>
> >>>>> Drop the _CURSOR mask here? And the double space....
> >>>>
> >>>> Ack for the doublespace. By removing _CURSOR we would disallow using
> >>>> these planes as hw cursor planes. This would switch all compositors into
> >>>> sw cursor mode, thus damaging the performance.
> >>>
> >>> Doesn't that require special hardware support, or can any DMA pipe
> >>> support "hw cursor mode/planes", whatever that means? Sorry for not
> >>> being well versed in this area, I'd expect DMA pipes and CURSOR pipes to
> >>> have a different set of features / capabilities.
> >>
> >> Yes, they have different capabilities. but DMA_CURSOR_MSM8998_MASK =
> >> DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR). And the DPU_SSPP_CURSOR is
> >> used internally to tell the DRM core that the corresponding plane is
> >> going to be used as a "userspace cursor" plane.
> >
> > Different capabilities for userspace, but not in terms hardware (/driver
> > support, yet)? If so, then:
> >
> > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> >
> >>> Commit 07ca1fc0f8a0 ("drm/msm/dpu: enable cursor plane on dpu") leads me
> >>> to believe that it's mostly to let userspace use these DMA pipes for
> >>> cursors (having cursor planes available in uapi) rather than requiring
> >>> any special hardware support (though semantics do seem to change in a
> >>> nontrivial way).
> >>
> >> Correct.
> >> DRM/userspace cursor planes = planes which userspace can use to draw
> >> mouse cursor. Legacy compositors and legacy cursor IOCTLs stick to
> >> using them
> >> DPU/MDP5 CURSOR plane (sspp_12/13) = lightweight limited plane without
> >
> > But these DMA pipes are _not_ lightweight/limited?
>
> No, they are not.
>
> >
> >> additional features, targeting HW cursor only, not present since
> >> sdm845
> >> DPU_SSPP_CURSOR = bit which tells DPU core to mark a plane as
> >> 'DRM/userspace cursor plane'.
> >
> > Ack, so it's not toggling anything hardware specific /yet/. However,
> > does this prevent userspace from using these pipes/planes for other DMA
> > purposes as they're marked as a different _type_ of plane?
>
> This is what 'universal planes' API is solving.
>
> Historically there were three kinds of planes: primary (iow background),
> cursor and overlay.
> By default an application sees only the overlay planes and has some
> additional API to manipulate cursors and backgrounds.
>
> Then at some point it was found that this split is not worth all the
> troubles, since applications can better utilize the hardware if they can
> decide on their own what should be done. So now we still have all three
> kinds of planes (for the legacy userspace), but behind the scenes they
> all are the same. If an application knows how to knock the door, it will
> see all the planes with the capabilities being exposed through plane
> properties, etc.
>
> Back to our case. We mark these planes as 'cursor' ones, to let the
> legacy composers to use them for hardware cursor. I think it was decided
> that not having the cursor is worse than requiring another blending
> step. On the other hand newer composers see a full array of planes.
Thanks so much for this backstory, that explains why it shouldn't harm
modern compositors; in modetest I see these planes have the cursor type
property, but no restriction on zpos for example.
> > And will
> > that change when we do end up "implementing more rigorous/strict
> > hardware support"?
>
> Once implemented, there will be more planes for msm8998 (and eventually
> sdm660/630, once we have them too). Some of them will be limited in size
> or in the Z order (cursor), some will not (rgb, dma, vig).
That's what I saw one of the linked patches; at that point we should add
separate feature bits for this so as to not limit the size or Z-order
range for these DMA-pipes-disguised-as-cursor-planes.
> > For the other SoCs, are their DMA pipes also
> > featureful and would the presence of DPU_SSPP_CURSOR severely limit its
> > functionality?
>
> All DMA pipes have the same set of features (in the same generation of
> course).
> No, it's just a software marker.
Ack yes, so again once the software marker starts limiting properties
for actual CURSOR support (on pre-msm8998) we should distinguish them
from DMA pipes that are simply marked as cursor planes...
> > And is this thing that "virtual planes" would be going
> > to "solve"?
>
> Included. The virtual planes is trying to solve a slightly different
> part of the story: to remove 1:1 correspondence between planes and
> pipes. Sometimes it would be nice to use two HW pipes for a single DRM
> plane (e.g. the kernel expects to have a single primary plane whose
> resolution matches the resolution of the CRTC, 4k = two SSPP because of
> hardware limitations). Sometimes a single HW pipe can be used to drive
> two DRM planes (see multirect). So, pretty much in the same way as we
> use one or two LMs to drive a CRTC, it is useful to use 1/2, 1 or 2
> SSPPs to drive a single DRM plane.
Ah, ack, that makes sense, and it wouldn't / shouldn't be up to
userspace to assign and use the pipes on its own.
- Marijn
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
2023-01-24 9:59 ` Marijn Suijten
@ 2023-02-10 13:38 ` Dmitry Baryshkov
-1 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-02-10 13:38 UTC (permalink / raw)
To: Marijn Suijten
Cc: Rob Clark, Sean Paul, Abhinav Kumar, freedreno, Jami Kettunen,
linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Daniel Vetter, AngeloGioacchino Del Regno, David Airlie
On 24/01/2023 11:59, Marijn Suijten wrote:
> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
>> planes). Correct corresponding SSPP declarations.
>>
>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
>
> Drop the _CURSOR mask here? And the double space....
As a second thought, I think I will keep the spacing in this patchset. I
will clean up spacing during the hw catalog split.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks
@ 2023-02-10 13:38 ` Dmitry Baryshkov
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Baryshkov @ 2023-02-10 13:38 UTC (permalink / raw)
To: Marijn Suijten
Cc: Jami Kettunen, Sean Paul, Bjorn Andersson,
AngeloGioacchino Del Regno, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 24/01/2023 11:59, Marijn Suijten wrote:
> On 2023-01-15 14:41:42, Dmitry Baryshkov wrote:
>> DMA2 and DMA3 planes on msm8998 should use corresponding DMA2 and DMA3
>> clocks rather than CURSOR0/1 clocks (which are used for the CURSOR
>> planes). Correct corresponding SSPP declarations.
>>
>> Fixes: 94391a14fc27 ("drm/msm/dpu1: Add MSM8998 to hw catalog")
>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
>> Cc: Jami Kettunen <jami.kettunen@somainline.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 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 0f3da480b066..ad0c55464154 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1180,9 +1180,9 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
>> SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_MSM8998_MASK,
>> sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
>> SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_MSM8998_MASK,
>
> Drop the _CURSOR mask here? And the double space....
As a second thought, I think I will keep the spacing in this patchset. I
will clean up spacing during the hw catalog split.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2023-02-10 13:38 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-15 12:41 [PATCH 1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks Dmitry Baryshkov
2023-01-15 12:41 ` Dmitry Baryshkov
2023-01-15 12:41 ` [PATCH 2/2] drm/msm/dpu: don't use DPU_CLK_CTRL_CURSORn for DMA SSPP clocks Dmitry Baryshkov
2023-01-15 12:41 ` Dmitry Baryshkov
2023-01-16 15:12 ` Neil Armstrong
2023-01-16 15:12 ` Neil Armstrong
2023-01-24 10:13 ` [2/2] " Marijn Suijten
2023-01-24 10:13 ` Marijn Suijten
2023-01-24 10:20 ` Dmitry Baryshkov
2023-01-24 10:20 ` Dmitry Baryshkov
2023-01-24 9:59 ` [1/2] drm/msm/dpu: fix clocks settings for msm8998 SSPP blocks Marijn Suijten
2023-01-24 9:59 ` Marijn Suijten
2023-01-24 10:19 ` Dmitry Baryshkov
2023-01-24 10:19 ` Dmitry Baryshkov
2023-01-24 11:12 ` Marijn Suijten
2023-01-24 11:12 ` Marijn Suijten
2023-01-24 11:20 ` Dmitry Baryshkov
2023-01-24 11:20 ` Dmitry Baryshkov
2023-01-24 12:06 ` Marijn Suijten
2023-01-24 12:06 ` Marijn Suijten
2023-01-24 12:29 ` Dmitry Baryshkov
2023-01-24 12:29 ` Dmitry Baryshkov
2023-01-24 14:00 ` Marijn Suijten
2023-01-24 14:00 ` Marijn Suijten
2023-02-10 13:38 ` Dmitry Baryshkov
2023-02-10 13:38 ` Dmitry Baryshkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.