Replace magic register writes in msm_mdss_enable() with version that contains less magic and more variable names that can be traced back to the dpu_hw_catalog or the downstream dtsi files.
Reviewed-by: Abhinav Kumar quic_abhinavk@quicinc.com Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/msm_mdss.c | 80 ++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index 0454a571adf7..b41848bfff91 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -21,6 +21,7 @@ #define HW_REV 0x0 #define HW_INTR_STATUS 0x0010
+#define UBWC_DEC_HW_VERSION 0x58 #define UBWC_STATIC 0x144 #define UBWC_CTRL_2 0x150 #define UBWC_PREDICTION_MODE 0x154 @@ -132,9 +133,63 @@ static int _msm_mdss_irq_domain_add(struct msm_mdss *msm_mdss) return 0; }
+#define UBWC_1_0 0x10000000 +#define UBWC_2_0 0x20000000 +#define UBWC_3_0 0x30000000 +#define UBWC_4_0 0x40000000 + +static void msm_mdss_setup_ubwc_dec_20(struct msm_mdss *msm_mdss, + u32 ubwc_static) +{ + writel_relaxed(ubwc_static, msm_mdss->mmio + UBWC_STATIC); +} + +static void msm_mdss_setup_ubwc_dec_30(struct msm_mdss *msm_mdss, + unsigned int ubwc_version, + u32 ubwc_swizzle, + u32 highest_bank_bit, + u32 macrotile_mode) +{ + u32 value = (ubwc_swizzle & 0x1) | + (highest_bank_bit & 0x3) << 4 | + (macrotile_mode & 0x1) << 12; + + if (ubwc_version == UBWC_3_0) + value |= BIT(10); + + if (ubwc_version == UBWC_1_0) + value |= BIT(8); + + writel_relaxed(value, msm_mdss->mmio + UBWC_STATIC); +} + +static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss, + unsigned int ubwc_version, + u32 ubwc_swizzle, + u32 ubwc_static, + u32 highest_bank_bit, + u32 macrotile_mode) +{ + u32 value = (ubwc_swizzle & 0x7) | + (ubwc_static & 0x1) << 3 | + (highest_bank_bit & 0x7) << 4 | + (macrotile_mode & 0x1) << 12; + + writel_relaxed(value, msm_mdss->mmio + UBWC_STATIC); + + if (ubwc_version == UBWC_3_0) { + writel_relaxed(1, msm_mdss->mmio + UBWC_CTRL_2); + writel_relaxed(0, msm_mdss->mmio + UBWC_PREDICTION_MODE); + } else { + writel_relaxed(2, msm_mdss->mmio + UBWC_CTRL_2); + writel_relaxed(1, msm_mdss->mmio + UBWC_PREDICTION_MODE); + } +} + static int msm_mdss_enable(struct msm_mdss *msm_mdss) { int ret; + u32 hw_rev;
ret = clk_bulk_prepare_enable(msm_mdss->num_clocks, msm_mdss->clocks); if (ret) { @@ -149,26 +204,35 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss) if (msm_mdss->is_mdp5) return 0;
+ hw_rev = readl_relaxed(msm_mdss->mmio + HW_REV); + dev_dbg(msm_mdss->dev, "HW_REV: 0x%x\n", hw_rev); + dev_dbg(msm_mdss->dev, "UBWC_DEC_HW_VERSION: 0x%x\n", + readl_relaxed(msm_mdss->mmio + UBWC_DEC_HW_VERSION)); + /* * ubwc config is part of the "mdss" region which is not accessible * from the rest of the driver. hardcode known configurations here + * + * Decoder version can be read from the UBWC_DEC_HW_VERSION reg, + * UBWC_n and the rest of params comes from hw_catalog. + * Unforunately this driver can not access hw catalog, so we have to + * hardcode them here. */ - switch (readl_relaxed(msm_mdss->mmio + HW_REV)) { + switch (hw_rev) { case DPU_HW_VER_500: case DPU_HW_VER_501: - writel_relaxed(0x420, msm_mdss->mmio + UBWC_STATIC); + msm_mdss_setup_ubwc_dec_30(msm_mdss, UBWC_3_0, 0, 2, 0); break; case DPU_HW_VER_600: - /* TODO: 0x102e for LP_DDR4 */ - writel_relaxed(0x103e, msm_mdss->mmio + UBWC_STATIC); - writel_relaxed(2, msm_mdss->mmio + UBWC_CTRL_2); - writel_relaxed(1, msm_mdss->mmio + UBWC_PREDICTION_MODE); + /* TODO: highest_bank_bit = 2 for LP_DDR4 */ + msm_mdss_setup_ubwc_dec_40(msm_mdss, UBWC_4_0, 6, 1, 3, 1); break; case DPU_HW_VER_620: - writel_relaxed(0x1e, msm_mdss->mmio + UBWC_STATIC); + /* UBWC_2_0 */ + msm_mdss_setup_ubwc_dec_20(msm_mdss, 0x1e); break; case DPU_HW_VER_720: - writel_relaxed(0x101e, msm_mdss->mmio + UBWC_STATIC); + msm_mdss_setup_ubwc_dec_40(msm_mdss, UBWC_3_0, 6, 1, 1, 1); break; }
Enable (optional) core (MDP_CLK) clock that allows accessing HW_REV registers during the platform init.
Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/msm_mdss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index b41848bfff91..f7b4628986b8 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -288,7 +288,7 @@ static int msm_mdss_reset(struct device *dev) /* * MDP5 MDSS uses at most three specified clocks. */ -#define MDP5_MDSS_NUM_CLOCKS 3 +#define MDP5_MDSS_NUM_CLOCKS 4 static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_data **clocks) { struct clk_bulk_data *bulk; @@ -305,6 +305,7 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d bulk[num_clocks++].id = "iface"; bulk[num_clocks++].id = "bus"; bulk[num_clocks++].id = "vsync"; + bulk[num_clocks++].id = "core"; /* for hw_rev access */
ret = devm_clk_bulk_get_optional(&pdev->dev, num_clocks, bulk); if (ret)
Hi Dmitry,
On Mittwoch, 15. Juni 2022 15:59:32 CEST Dmitry Baryshkov wrote:
Enable (optional) core (MDP_CLK) clock that allows accessing HW_REV registers during the platform init.
I believe you also need to update Documentation/devicetree/bindings/display/ msm/mdp5.txt with the new clock.
Regards Luca
Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
drivers/gpu/drm/msm/msm_mdss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index b41848bfff91..f7b4628986b8 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -288,7 +288,7 @@ static int msm_mdss_reset(struct device *dev) /*
- MDP5 MDSS uses at most three specified clocks.
*/ -#define MDP5_MDSS_NUM_CLOCKS 3 +#define MDP5_MDSS_NUM_CLOCKS 4 static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_data **clocks) { struct clk_bulk_data *bulk; @@ -305,6 +305,7 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d bulk[num_clocks++].id = "iface"; bulk[num_clocks++].id = "bus"; bulk[num_clocks++].id = "vsync";
bulk[num_clocks++].id = "core"; /* for hw_rev access */
ret = devm_clk_bulk_get_optional(&pdev->dev, num_clocks, bulk); if (ret)
On Sat, 18 Jun 2022 at 17:24, Luca Weiss luca@z3ntu.xyz wrote:
Hi Dmitry,
On Mittwoch, 15. Juni 2022 15:59:32 CEST Dmitry Baryshkov wrote:
Enable (optional) core (MDP_CLK) clock that allows accessing HW_REV registers during the platform init.
I believe you also need to update Documentation/devicetree/bindings/display/ msm/mdp5.txt with the new clock.
I'm working on converting it to the yaml format.
Regards Luca
Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org
drivers/gpu/drm/msm/msm_mdss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index b41848bfff91..f7b4628986b8 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -288,7 +288,7 @@ static int msm_mdss_reset(struct device *dev) /*
- MDP5 MDSS uses at most three specified clocks.
*/ -#define MDP5_MDSS_NUM_CLOCKS 3 +#define MDP5_MDSS_NUM_CLOCKS 4 static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_data **clocks) { struct clk_bulk_data *bulk; @@ -305,6 +305,7 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d bulk[num_clocks++].id = "iface"; bulk[num_clocks++].id = "bus"; bulk[num_clocks++].id = "vsync";
bulk[num_clocks++].id = "core"; /* for hw_rev access */ ret = devm_clk_bulk_get_optional(&pdev->dev, num_clocks, bulk); if (ret)
Rather than checking whether the platform is an mdp5 or dpu platform, check if the MDP_CLK is provided or not before trying to access HW_REV (and skip reading the registers if the clock is not provided by the DT).
Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/msm_mdss.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index f7b4628986b8..d81d8fe3584e 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -32,7 +32,6 @@ struct msm_mdss { void __iomem *mmio; struct clk_bulk_data *clocks; size_t num_clocks; - bool is_mdp5; struct { unsigned long enabled_mask; struct irq_domain *domain; @@ -186,6 +185,19 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss, } }
+static bool msm_mdss_has_clock(struct msm_mdss *msm_mdss, const char *name) +{ + unsigned int i; + + for (i = 0; i < msm_mdss->num_clocks; i++) { + if (!strcmp(msm_mdss->clocks[i].id, name) && + msm_mdss->clocks[i].clk) + return true; + } + + return false; +} + static int msm_mdss_enable(struct msm_mdss *msm_mdss) { int ret; @@ -198,10 +210,11 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss) }
/* - * HW_REV requires MDSS_MDP_CLK, which is not enabled by the mdss on - * mdp5 hardware. Skip reading it for now. + * HW_REV requires MDSS_MDP_CLK, which is not used for MDSS device in + * older device trees. Skip accessing registers if the clock is not + * present. */ - if (msm_mdss->is_mdp5) + if (!msm_mdss_has_clock(msm_mdss, "core")) return 0;
hw_rev = readl_relaxed(msm_mdss->mmio + HW_REV); @@ -345,7 +358,6 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5 return ERR_PTR(ret); } msm_mdss->num_clocks = ret; - msm_mdss->is_mdp5 = is_mdp5;
msm_mdss->dev = &pdev->dev;
Move is_mdp5 check to a more logical place, to the msm_mdss_init(), rather than getting it in the mdss_probe() and passing it then as an argument.
Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- drivers/gpu/drm/msm/msm_mdss.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index d81d8fe3584e..ce8ff5bfe55a 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -329,8 +329,9 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d return num_clocks; }
-static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5) +static struct msm_mdss *msm_mdss_init(struct platform_device *pdev) { + bool is_mdp5 = of_device_is_compatible(pdev->dev.of_node, "qcom,mdss"); struct msm_mdss *msm_mdss; int ret; int irq; @@ -420,11 +421,10 @@ static const struct dev_pm_ops mdss_pm_ops = { static int mdss_probe(struct platform_device *pdev) { struct msm_mdss *mdss; - bool is_mdp5 = of_device_is_compatible(pdev->dev.of_node, "qcom,mdss"); struct device *dev = &pdev->dev; int ret;
- mdss = msm_mdss_init(pdev, is_mdp5); + mdss = msm_mdss_init(pdev); if (IS_ERR(mdss)) return PTR_ERR(mdss);
Add MDP_CLK ("core") clock to the mdss device to allow MDSS driver to access HW_REV/etc registers.
Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org --- arch/arm64/boot/dts/qcom/msm8996.dtsi | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi index f0f81c23c16f..3d8ecfe56fb3 100644 --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi @@ -773,8 +773,9 @@ mdss: mdss@900000 { interrupt-controller; #interrupt-cells = <1>;
- clocks = <&mmcc MDSS_AHB_CLK>; - clock-names = "iface"; + clocks = <&mmcc MDSS_AHB_CLK>, + <&mmcc MDSS_MDP_CLK>; + clock-names = "iface", "core";
#address-cells = <1>; #size-cells = <1>;
dri-devel@lists.freedesktop.org