[PATCH v2 0/6] Add HiKey / HI6220 SoC Mali-450 support

[PATCH v2 0/6] Add HiKey / HI6220 SoC Mali-450 support

[Peter Griffin]

Hi folks,

This series adds support for the Mali450 MP4 GPU found on the
hi6220 SoC from HiSilicon. It has been tested with the
lima drm/mesa driver hosted on freedesktop.org gitlab,
and validated using Weston and kmscube.

As lima drm driver has now been merged this v2 series includes
one extra patch to avoid Oops when all Mali ip blocks share the
same irq.

regards,

Peter.

Changes since v1:
* Additional drm/lima patch to avoid Oops in shared irq case (Peter Griffin)
* Squash clock and reset driver parts to avoid bisection hole (Stephen Boyd)
* Add comment as to what other driver is probing same dt node (Stephen Boyd)
* Check regmap_write error code and return individually (Philipp Zabel)
* Add Rob Herring Reviewed by tags

Peter Griffin (6):
  dt-bindings: gpu: mali-utgard: add hisilicon,hi6220-mali compatible
  dt-bindings: reset: hisilicon: Update compatible documentation
  arm64: dts: hisilicon: Add Mali-450 MP4 GPU DT entry
  reset: hi6220: Add support for AO reset controller
  dt-bindings: reset: hisilicon: Add ao reset controller
  drm/lima: handle shared irq case for lima_pp_bcast_irq_handler

 .../devicetree/bindings/gpu/arm,mali-utgard.txt    |  5 ++
 .../bindings/reset/hisilicon,hi6220-reset.txt      |  1 +
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 38 +++++++++++++
 drivers/clk/hisilicon/clk-hi6220.c                 |  3 +-
 drivers/gpu/drm/lima/lima_pp.c                     |  8 ++-
 drivers/reset/hisilicon/hi6220_reset.c             | 65 +++++++++++++++++++++-
 include/dt-bindings/reset/hisi,hi6220-resets.h     |  7 +++
 7 files changed, 124 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH v2 1/6] dt-bindings: gpu: mali-utgard: add hisilicon,hi6220-mali compatible

[Peter Griffin]

The Hisilicon hi6220 uses a Mali-450MP4 with 4 PPs, so add
a compatible for it.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
index 3f128e4..2a16b0c 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
@@ -21,6 +21,7 @@ Required properties:
       + rockchip,rk3228-mali
       + rockchip,rk3328-mali
       + stericsson,db8500-mali
+      + hisilicon,hi6220-mali
 
   - reg: Physical base address and length of the GPU registers
 
@@ -91,6 +92,10 @@ to specify one more vendor-specific compatible, among:
       * interrupt-names and interrupts:
         + combined: combined interrupt of all of the above lines
 
+  - hisilicon,hi6220-mali
+    Required properties:
+      * resets: phandles to the reset lines for the GPU
+
 Example:
 
 mali: gpu@1c40000 {
-- 
2.7.4

[PATCH v2 2/6] dt-bindings: reset: hisilicon: Update compatible documentation

[Peter Griffin]

The reset driver now supports the ao reset controller, so update the
documentation to match.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt
index c25da39..ea0a6a9 100644
--- a/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt
+++ b/Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt
@@ -11,6 +11,7 @@ Required properties:
 - compatible: should be one of the following:
   - "hisilicon,hi6220-sysctrl", "syscon" : For peripheral reset controller.
   - "hisilicon,hi6220-mediactrl", "syscon" : For media reset controller.
+  - "hisilicon,hi6220-aoctrl", "syscon" : For ao reset controller.
 - reg: should be register base and length as documented in the
   datasheet
 - #reset-cells: 1, see below
-- 
2.7.4

[PATCH v2 3/6] arm64: dts: hisilicon: Add Mali-450 MP4 GPU DT entry

[Peter Griffin]

hi6220 has a Mali450 MP4 so lets add it into the DT.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 37 +++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index aec9e37..2defc19 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -1019,6 +1019,43 @@
 			clock-names = "apb_pclk";
 			cpu = <&cpu7>;
 		};
+
+		mali: gpu@f4080000 {
+			compatible = "hisilicon,hi6220-mali", "arm,mali-450";
+			reg = <0x0 0xf4080000 0x0 0x00040000>;
+			interrupt-parent = <&gic>;
+			interrupts =	<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_PPI 126 IRQ_TYPE_LEVEL_HIGH>;
+
+			interrupt-names = "gp",
+					  "gpmmu",
+					  "pp",
+					  "pp0",
+					  "ppmmu0",
+					  "pp1",
+					  "ppmmu1",
+					  "pp2",
+					  "ppmmu2",
+					  "pp3",
+					  "ppmmu3";
+			clocks = <&media_ctrl HI6220_G3D_CLK>,
+				 <&media_ctrl HI6220_G3D_PCLK>;
+			clock-names = "core", "bus";
+			assigned-clocks = <&media_ctrl HI6220_G3D_CLK>,
+					  <&media_ctrl HI6220_G3D_PCLK>;
+			assigned-clock-rates = <500000000>, <144000000>;
+			reset-names = "ao_g3d", "media_g3d";
+			resets = <&ao_ctrl AO_G3D>, <&media_ctrl MEDIA_G3D>;
+		};
 	};
 };
 
-- 
2.7.4

[PATCH v2 4/6] reset: hi6220: Add support for AO reset controller

[Peter Griffin]

This is required to bring Mali450 gpu out of reset. Also
we now use CLK_OF_DECLARE_DRIVER to probe in both the
clock and reset drivers. The clock and reset parts have
been done as one atomic commit to avoid a bisection hole.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi |  1 +
 drivers/clk/hisilicon/clk-hi6220.c        |  3 +-
 drivers/reset/hisilicon/hi6220_reset.c    | 65 ++++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 2defc19..66a3746 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -260,6 +260,7 @@
 			compatible = "hisilicon,hi6220-aoctrl", "syscon";
 			reg = <0x0 0xf7800000 0x0 0x2000>;
 			#clock-cells = <1>;
+			#reset-cells = <1>;
 		};
 
 		sys_ctrl: sys_ctrl@f7030000 {
diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index a87809d..3dde9d3 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c
@@ -89,8 +89,9 @@ static void __init hi6220_clk_ao_init(struct device_node *np)
 	hisi_clk_register_gate_sep(hi6220_separated_gate_clks_ao,
 				ARRAY_SIZE(hi6220_separated_gate_clks_ao), clk_data_ao);
 }
-CLK_OF_DECLARE(hi6220_clk_ao, "hisilicon,hi6220-aoctrl", hi6220_clk_ao_init);
 
+/* reset driver also probes and uses these registers */
+CLK_OF_DECLARE_DRIVER(hi6220_clk_ao, "hisilicon,hi6220-aoctrl", hi6220_clk_ao_init);
 
 /* clocks in sysctrl */
 static const char *mmc0_mux0_p[] __initdata = { "pll_ddr_gate", "syspll", };
diff --git a/drivers/reset/hisilicon/hi6220_reset.c b/drivers/reset/hisilicon/hi6220_reset.c
index d5e5229..ccd5e0a 100644
--- a/drivers/reset/hisilicon/hi6220_reset.c
+++ b/drivers/reset/hisilicon/hi6220_reset.c
@@ -36,6 +36,7 @@
 enum hi6220_reset_ctrl_type {
 	PERIPHERAL,
 	MEDIA,
+	AO,
 };
 
 struct hi6220_reset_data {
@@ -95,6 +96,61 @@ static const struct reset_control_ops hi6220_media_reset_ops = {
 	.deassert = hi6220_media_deassert,
 };
 
+#define AO_SCTRL_SC_PW_CLKEN0     0x800
+#define AO_SCTRL_SC_PW_CLKDIS0    0x804
+
+#define AO_SCTRL_SC_PW_RSTEN0     0x810
+#define AO_SCTRL_SC_PW_RSTDIS0    0x814
+
+#define AO_SCTRL_SC_PW_ISOEN0     0x820
+#define AO_SCTRL_SC_PW_ISODIS0    0x824
+#define AO_MAX_INDEX              12
+
+static int hi6220_ao_assert(struct reset_controller_dev *rc_dev,
+			       unsigned long idx)
+{
+	struct hi6220_reset_data *data = to_reset_data(rc_dev);
+	struct regmap *regmap = data->regmap;
+	int ret;
+
+	ret = regmap_write(regmap, AO_SCTRL_SC_PW_RSTEN0, BIT(idx));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, AO_SCTRL_SC_PW_ISOEN0, BIT(idx));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, AO_SCTRL_SC_PW_CLKDIS0, BIT(idx));
+	if (ret)
+		return ret;
+}
+
+static int hi6220_ao_deassert(struct reset_controller_dev *rc_dev,
+				 unsigned long idx)
+{
+	struct hi6220_reset_data *data = to_reset_data(rc_dev);
+	struct regmap *regmap = data->regmap;
+	int ret;
+
+	ret = regmap_write(regmap, AO_SCTRL_SC_PW_RSTDIS0, BIT(idx));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, AO_SCTRL_SC_PW_ISODIS0, BIT(idx));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, AO_SCTRL_SC_PW_CLKEN0, BIT(idx));
+	if (ret)
+		return ret;
+}
+
+static const struct reset_control_ops hi6220_ao_reset_ops = {
+	.assert = hi6220_ao_assert,
+	.deassert = hi6220_ao_deassert,
+};
+
 static int hi6220_reset_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -120,9 +176,12 @@ static int hi6220_reset_probe(struct platform_device *pdev)
 	if (type == MEDIA) {
 		data->rc_dev.ops = &hi6220_media_reset_ops;
 		data->rc_dev.nr_resets = MEDIA_MAX_INDEX;
-	} else {
+	} else if (type == PERIPHERAL) {
 		data->rc_dev.ops = &hi6220_peripheral_reset_ops;
 		data->rc_dev.nr_resets = PERIPH_MAX_INDEX;
+	} else {
+		data->rc_dev.ops = &hi6220_ao_reset_ops;
+		data->rc_dev.nr_resets = AO_MAX_INDEX;
 	}
 
 	return reset_controller_register(&data->rc_dev);
@@ -137,6 +196,10 @@ static const struct of_device_id hi6220_reset_match[] = {
 		.compatible = "hisilicon,hi6220-mediactrl",
 		.data = (void *)MEDIA,
 	},
+	{
+		.compatible = "hisilicon,hi6220-aoctrl",
+		.data = (void *)AO,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, hi6220_reset_match);
-- 
2.7.4

[PATCH v2 5/6] dt-bindings: reset: hisilicon: Add ao reset controller

[Peter Griffin]

This is required to bring Mali450 gpu out of reset.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 include/dt-bindings/reset/hisi,hi6220-resets.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/dt-bindings/reset/hisi,hi6220-resets.h b/include/dt-bindings/reset/hisi,hi6220-resets.h
index e7c362a8..63aff7d 100644
--- a/include/dt-bindings/reset/hisi,hi6220-resets.h
+++ b/include/dt-bindings/reset/hisi,hi6220-resets.h
@@ -73,4 +73,11 @@
 #define MEDIA_MMU                       6
 #define MEDIA_XG2RAM1                   7
 
+#define AO_G3D                          1
+#define AO_CODECISP                     2
+#define AO_MCPU                         4
+#define AO_BBPHARQMEM                   5
+#define AO_HIFI                         8
+#define AO_ACPUSCUL2C                   12
+
 #endif /*_DT_BINDINGS_RESET_CONTROLLER_HI6220*/
-- 
2.7.4

[PATCH v2 6/6] drm/lima: handle shared irq case for lima_pp_bcast_irq_handler

[Peter Griffin]

On Hikey board all lima ip blocks are shared with one irq.
This patch avoids a NULL ptr deref crash on this platform
on startup. Tested with Weston and kmscube.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Qiang Yu <yuq825@gmail.com>
---
 drivers/gpu/drm/lima/lima_pp.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/lima/lima_pp.c b/drivers/gpu/drm/lima/lima_pp.c
index d29721e..8fef224 100644
--- a/drivers/gpu/drm/lima/lima_pp.c
+++ b/drivers/gpu/drm/lima/lima_pp.c
@@ -64,7 +64,13 @@ static irqreturn_t lima_pp_bcast_irq_handler(int irq, void *data)
 	struct lima_ip *pp_bcast = data;
 	struct lima_device *dev = pp_bcast->dev;
 	struct lima_sched_pipe *pipe = dev->pipe + lima_pipe_pp;
-	struct drm_lima_m450_pp_frame *frame = pipe->current_task->frame;
+	struct drm_lima_m450_pp_frame *frame;
+
+	/* for shared irq case */
+	if (!pipe->current_task)
+		return IRQ_NONE;
+
+	frame = pipe->current_task->frame;
 
 	for (i = 0; i < frame->num_pp; i++) {
 		struct lima_ip *ip = pipe->processor[i];
-- 
2.7.4

Re: [PATCH v2 6/6] drm/lima: handle shared irq case for lima_pp_bcast_irq_handler

[Qiang Yu]

Looks good for me, patch is:
Reviewed-by: Qiang Yu <yuq825@gmail.com>

I'll push this patch to drm-misc-next.

Regards,
Qiang


On Fri, Apr 19, 2019 at 4:35 PM Peter Griffin <peter.griffin@linaro.org> wrote:
>
> On Hikey board all lima ip blocks are shared with one irq.
> This patch avoids a NULL ptr deref crash on this platform
> on startup. Tested with Weston and kmscube.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Qiang Yu <yuq825@gmail.com>
> ---
>  drivers/gpu/drm/lima/lima_pp.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/lima/lima_pp.c b/drivers/gpu/drm/lima/lima_pp.c
> index d29721e..8fef224 100644
> --- a/drivers/gpu/drm/lima/lima_pp.c
> +++ b/drivers/gpu/drm/lima/lima_pp.c
> @@ -64,7 +64,13 @@ static irqreturn_t lima_pp_bcast_irq_handler(int irq, void *data)
>         struct lima_ip *pp_bcast = data;
>         struct lima_device *dev = pp_bcast->dev;
>         struct lima_sched_pipe *pipe = dev->pipe + lima_pipe_pp;
> -       struct drm_lima_m450_pp_frame *frame = pipe->current_task->frame;
> +       struct drm_lima_m450_pp_frame *frame;
> +
> +       /* for shared irq case */
> +       if (!pipe->current_task)
> +               return IRQ_NONE;
> +
> +       frame = pipe->current_task->frame;
>
>         for (i = 0; i < frame->num_pp; i++) {
>                 struct lima_ip *ip = pipe->processor[i];
> --
> 2.7.4
>

Re: [PATCH v2 4/6] reset: hi6220: Add support for AO reset controller

[Stephen Boyd]

Quoting Peter Griffin (2019-04-19 01:32:59)
> This is required to bring Mali450 gpu out of reset. Also
> we now use CLK_OF_DECLARE_DRIVER to probe in both the
> clock and reset drivers. The clock and reset parts have
> been done as one atomic commit to avoid a bisection hole.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> ---

>  drivers/clk/hisilicon/clk-hi6220.c        |  3 +-

Acked-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v2 6/6] drm/lima: handle shared irq case for lima_pp_bcast_irq_handler

[Qiang Yu]

Pushed to drm-misc-next.

Thanks,
Qiang

On Fri, Apr 19, 2019 at 4:35 PM Peter Griffin <peter.griffin@linaro.org> wrote:
>
> On Hikey board all lima ip blocks are shared with one irq.
> This patch avoids a NULL ptr deref crash on this platform
> on startup. Tested with Weston and kmscube.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Qiang Yu <yuq825@gmail.com>
> ---
>  drivers/gpu/drm/lima/lima_pp.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/lima/lima_pp.c b/drivers/gpu/drm/lima/lima_pp.c
> index d29721e..8fef224 100644
> --- a/drivers/gpu/drm/lima/lima_pp.c
> +++ b/drivers/gpu/drm/lima/lima_pp.c
> @@ -64,7 +64,13 @@ static irqreturn_t lima_pp_bcast_irq_handler(int irq, void *data)
>         struct lima_ip *pp_bcast = data;
>         struct lima_device *dev = pp_bcast->dev;
>         struct lima_sched_pipe *pipe = dev->pipe + lima_pipe_pp;
> -       struct drm_lima_m450_pp_frame *frame = pipe->current_task->frame;
> +       struct drm_lima_m450_pp_frame *frame;
> +
> +       /* for shared irq case */
> +       if (!pipe->current_task)
> +               return IRQ_NONE;
> +
> +       frame = pipe->current_task->frame;
>
>         for (i = 0; i < frame->num_pp; i++) {
>                 struct lima_ip *ip = pipe->processor[i];
> --
> 2.7.4
>