diff mbox series

[1/2] arm64: dts: juno: add GPU subsystem

Message ID 88dc6386929b3dcd7a65ba8063628c62b66b330c.1569856049.git.robin.murphy@arm.com
State Not Applicable, archived
Headers show
Series [1/2] arm64: dts: juno: add GPU subsystem | expand

Checks

Context Check Description
robh/checkpatch warning "total: 0 errors, 3 warnings, 56 lines checked"

Commit Message

Robin Murphy Sept. 30, 2019, 3:24 p.m. UTC
Since we now have bindings for Mali Midgard GPUs, let's use them to
describe Juno's GPU subsystem, if only because we can. Juno sports a
Mali-T624 integrated behind an MMU-400 (as a gesture towards
virtualisation), in their own dedicated power domain with DVFS
controlled by the SCP.

CC: Liviu Dudau <liviu.dudau@arm.com>
CC: Sudeep Holla <sudeep.holla@arm.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 .../bindings/gpu/arm,mali-midgard.yaml        |  5 +++-
 arch/arm64/boot/dts/arm/juno-base.dtsi        | 27 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

Comments

Steven Price Sept. 30, 2019, 4:26 p.m. UTC | #1
On 30/09/2019 16:24, Robin Murphy wrote:
> Although going full "dma-coherent" ends badly due to GEM objects still
> being forcibly mapped non-cacheable, we can at least take advantage of
> Juno's ACE-lite integration to skip cache maintenance for pagetables.
> 
> CC: Rob Herring <robh@kernel.org>
> CC: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> 
> This isn't really meant as a series, I'm just sending it together
> with patch #1 for context.
> 
>  drivers/gpu/drm/panfrost/panfrost_mmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index bdd990568476..560439f63277 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -365,6 +365,9 @@ int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv)
>  		.iommu_dev	= pfdev->dev,
>  	};
>  
> +	if (of_device_is_compatible(pfdev->dev->of_node, "arm,juno-mali"))
> +		pfdev->mmu->pgtbl_cfg.coherent_walk = true;

Should be:
	mmu->pgtbl_cfg.coherent_walk = true;

Also I'm not sure whether we should do this based on a compatible
string. kbase has a "system-coherency" device-tree flag for it. In
theory we could end up with a long list of compatibles here...

Steve

> +
>  	mmu->pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &mmu->pgtbl_cfg,
>  					      priv);
>  	if (!mmu->pgtbl_ops)
>
Liviu Dudau Sept. 30, 2019, 5:27 p.m. UTC | #2
On Mon, Sep 30, 2019 at 04:24:58PM +0100, Robin Murphy wrote:
> Since we now have bindings for Mali Midgard GPUs, let's use them to
> describe Juno's GPU subsystem, if only because we can. Juno sports a
> Mali-T624 integrated behind an MMU-400 (as a gesture towards
> virtualisation), in their own dedicated power domain with DVFS
> controlled by the SCP.
> 
> CC: Liviu Dudau <liviu.dudau@arm.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks,
Liviu

> CC: Sudeep Holla <sudeep.holla@arm.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  .../bindings/gpu/arm,mali-midgard.yaml        |  5 +++-
>  arch/arm64/boot/dts/arm/juno-base.dtsi        | 27 +++++++++++++++++++
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
> index 47bc1ac36426..018f3ae4b43c 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
> @@ -22,6 +22,10 @@ properties:
>            - enum:
>               - amlogic,meson-gxm-mali
>            - const: arm,mali-t820
> +      - items:
> +          - enum:
> +             - arm,juno-mali
> +          - const: arm,mali-t624
>        - items:
>            - enum:
>               - rockchip,rk3288-mali
> @@ -39,7 +43,6 @@ properties:
>               - samsung,exynos5433-mali
>            - const: arm,mali-t760
>  
> -          # "arm,mali-t624"
>            # "arm,mali-t628"
>            # "arm,mali-t830"
>            # "arm,mali-t880"
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index 26a039a028b8..9e3e8ce6adfe 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -35,6 +35,18 @@
>  		clock-names = "apb_pclk";
>  	};
>  
> +	smmu_gpu: iommu@2b400000 {
> +		compatible = "arm,mmu-400", "arm,smmu-v1";
> +		reg = <0x0 0x2b400000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> +		#iommu-cells = <1>;
> +		#global-interrupts = <1>;
> +		power-domains = <&scpi_devpd 1>;
> +		dma-coherent;
> +		status = "disabled";
> +	};
> +
>  	smmu_pcie: iommu@2b500000 {
>  		compatible = "arm,mmu-401", "arm,smmu-v1";
>  		reg = <0x0 0x2b500000 0x0 0x10000>;
> @@ -487,6 +499,21 @@
>  		};
>  	};
>  
> +	gpu: gpu@2d000000 {
> +		compatible = "arm,juno-mali", "arm,mali-t624";
> +		reg = <0 0x2d000000 0 0x10000>;
> +		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "gpu", "job", "mmu";
> +		clocks = <&scpi_dvfs 2>;
> +		power-domains = <&scpi_devpd 1>;
> +		dma-coherent;
> +		/* The SMMU is only really of interest to bare-metal hypervisors */
> +		/* iommus = <&smmu_gpu 0>; */
> +		status = "disabled";
> +	};
> +
>  	sram: sram@2e000000 {
>  		compatible = "arm,juno-sram-ns", "mmio-sram";
>  		reg = <0x0 0x2e000000 0x0 0x8000>;
> -- 
> 2.21.0.dirty
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Rob Herring Sept. 30, 2019, 5:46 p.m. UTC | #3
On Mon, Sep 30, 2019 at 10:25 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> Since we now have bindings for Mali Midgard GPUs, let's use them to
> describe Juno's GPU subsystem, if only because we can. Juno sports a
> Mali-T624 integrated behind an MMU-400 (as a gesture towards
> virtualisation), in their own dedicated power domain with DVFS
> controlled by the SCP.
>
> CC: Liviu Dudau <liviu.dudau@arm.com>
> CC: Sudeep Holla <sudeep.holla@arm.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  .../bindings/gpu/arm,mali-midgard.yaml        |  5 +++-
>  arch/arm64/boot/dts/arm/juno-base.dtsi        | 27 +++++++++++++++++++
>  2 files changed, 31 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring <robh@kernel.org>
Rob Herring Sept. 30, 2019, 5:52 p.m. UTC | #4
On Mon, Sep 30, 2019 at 11:26 AM Steven Price <steven.price@arm.com> wrote:
>
> On 30/09/2019 16:24, Robin Murphy wrote:
> > Although going full "dma-coherent" ends badly due to GEM objects still
> > being forcibly mapped non-cacheable, we can at least take advantage of
> > Juno's ACE-lite integration to skip cache maintenance for pagetables.
> >
> > CC: Rob Herring <robh@kernel.org>
> > CC: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > ---
> >
> > This isn't really meant as a series, I'm just sending it together
> > with patch #1 for context.
> >
> >  drivers/gpu/drm/panfrost/panfrost_mmu.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > index bdd990568476..560439f63277 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > @@ -365,6 +365,9 @@ int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv)
> >               .iommu_dev      = pfdev->dev,
> >       };
> >
> > +     if (of_device_is_compatible(pfdev->dev->of_node, "arm,juno-mali"))
> > +             pfdev->mmu->pgtbl_cfg.coherent_walk = true;
>
> Should be:
>         mmu->pgtbl_cfg.coherent_walk = true;

IOW, base this on 5.4 or drm-misc-next.

> Also I'm not sure whether we should do this based on a compatible
> string. kbase has a "system-coherency" device-tree flag for it. In
> theory we could end up with a long list of compatibles here...

Why not use 'dma-coherent' which you set? If not, I'm confused as to
what 'dma-coherent' is supposed to mean. Is it possible for page table
walks to have different coherency than the rest of the accesses?

Rob
Robin Murphy Sept. 30, 2019, 5:54 p.m. UTC | #5
On 30/09/2019 17:26, Steven Price wrote:
> On 30/09/2019 16:24, Robin Murphy wrote:
>> Although going full "dma-coherent" ends badly due to GEM objects still
>> being forcibly mapped non-cacheable, we can at least take advantage of
>> Juno's ACE-lite integration to skip cache maintenance for pagetables.
>>
>> CC: Rob Herring <robh@kernel.org>
>> CC: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>
>> This isn't really meant as a series, I'm just sending it together
>> with patch #1 for context.
>>
>>   drivers/gpu/drm/panfrost/panfrost_mmu.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> index bdd990568476..560439f63277 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> @@ -365,6 +365,9 @@ int panfrost_mmu_pgtable_alloc(struct panfrost_file_priv *priv)
>>   		.iommu_dev	= pfdev->dev,
>>   	};
>>   
>> +	if (of_device_is_compatible(pfdev->dev->of_node, "arm,juno-mali"))
>> +		pfdev->mmu->pgtbl_cfg.coherent_walk = true;
> 
> Should be:
> 	mmu->pgtbl_cfg.coherent_walk = true;

Oops, yes, rebase error.

> Also I'm not sure whether we should do this based on a compatible
> string. kbase has a "system-coherency" device-tree flag for it. In
> theory we could end up with a long list of compatibles here...

Right, ideally we'd just infer everything we need from "dma-coherent", 
but to be able to set that without causing fireworks we need the GEM 
mmap implementation to be able to play along too. Maybe this can wait 
until I figure out how to address that at the same time...

And either way, it would probably be better to detect anything 
SoC-specific once at probe and stash it in a feature flag - TBH this 
patch is more of a proof-of-concept than anything.

Thanks,
Robin.

> 
> Steve
> 
>> +
>>   	mmu->pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &mmu->pgtbl_cfg,
>>   					      priv);
>>   	if (!mmu->pgtbl_ops)
>>
>
Sudeep Holla Oct. 1, 2019, 8:59 a.m. UTC | #6
On Mon, Sep 30, 2019 at 12:46:33PM -0500, Rob Herring wrote:
> On Mon, Sep 30, 2019 at 10:25 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > Since we now have bindings for Mali Midgard GPUs, let's use them to
> > describe Juno's GPU subsystem, if only because we can. Juno sports a
> > Mali-T624 integrated behind an MMU-400 (as a gesture towards
> > virtualisation), in their own dedicated power domain with DVFS
> > controlled by the SCP.
> >
> > CC: Liviu Dudau <liviu.dudau@arm.com>
> > CC: Sudeep Holla <sudeep.holla@arm.com>
> > CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > ---
> >  .../bindings/gpu/arm,mali-midgard.yaml        |  5 +++-
> >  arch/arm64/boot/dts/arm/juno-base.dtsi        | 27 +++++++++++++++++++
> >  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

If you plan to take it along with driver change,

Acked-by: Sudeep Holla <sudeep.holla@arm.com>

If not, please let us know. I can take it for v5.5

--
Regards,
Sudeep
Robin Murphy Oct. 1, 2019, 12:45 p.m. UTC | #7
On 01/10/2019 09:59, Sudeep Holla wrote:
> On Mon, Sep 30, 2019 at 12:46:33PM -0500, Rob Herring wrote:
>> On Mon, Sep 30, 2019 at 10:25 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>>
>>> Since we now have bindings for Mali Midgard GPUs, let's use them to
>>> describe Juno's GPU subsystem, if only because we can. Juno sports a
>>> Mali-T624 integrated behind an MMU-400 (as a gesture towards
>>> virtualisation), in their own dedicated power domain with DVFS
>>> controlled by the SCP.
>>>
>>> CC: Liviu Dudau <liviu.dudau@arm.com>
>>> CC: Sudeep Holla <sudeep.holla@arm.com>
>>> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>> ---
>>>   .../bindings/gpu/arm,mali-midgard.yaml        |  5 +++-
>>>   arch/arm64/boot/dts/arm/juno-base.dtsi        | 27 +++++++++++++++++++
>>>   2 files changed, 31 insertions(+), 1 deletion(-)
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> If you plan to take it along with driver change,
> 
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> If not, please let us know. I can take it for v5.5

The driver change is debatable and not strictly necessary, so it 
probably makes more sense to take this one through the VExpress tree on 
its own. I wouldn't suggest flipping the status to "enabled" just yet, 
but it seems worth putting the basic description in place as a 
jumping-off point for folks to hack on (e.g. it's another 'interesting' 
case for devfreq OPP stuff).

Thanks,
Robin.
Sudeep Holla Oct. 1, 2019, 2:34 p.m. UTC | #8
On Tue, Oct 01, 2019 at 01:45:30PM +0100, Robin Murphy wrote:
> On 01/10/2019 09:59, Sudeep Holla wrote:
> > On Mon, Sep 30, 2019 at 12:46:33PM -0500, Rob Herring wrote:
> > > On Mon, Sep 30, 2019 at 10:25 AM Robin Murphy <robin.murphy@arm.com> wrote:
> > > > 
> > > > Since we now have bindings for Mali Midgard GPUs, let's use them to
> > > > describe Juno's GPU subsystem, if only because we can. Juno sports a
> > > > Mali-T624 integrated behind an MMU-400 (as a gesture towards
> > > > virtualisation), in their own dedicated power domain with DVFS
> > > > controlled by the SCP.
> > > > 
> > > > CC: Liviu Dudau <liviu.dudau@arm.com>
> > > > CC: Sudeep Holla <sudeep.holla@arm.com>
> > > > CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > > > ---
> > > >   .../bindings/gpu/arm,mali-midgard.yaml        |  5 +++-
> > > >   arch/arm64/boot/dts/arm/juno-base.dtsi        | 27 +++++++++++++++++++
> > > >   2 files changed, 31 insertions(+), 1 deletion(-)
> > > 
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > 
> > If you plan to take it along with driver change,
> > 
> > Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> > 
> > If not, please let us know. I can take it for v5.5
> 
> The driver change is debatable and not strictly necessary, so it probably
> makes more sense to take this one through the VExpress tree on its own. I
> wouldn't suggest flipping the status to "enabled" just yet, but it seems

Sure, make sense.

> worth putting the basic description in place as a jumping-off point for
> folks to hack on (e.g. it's another 'interesting' case for devfreq OPP
> stuff).

IIUC, the devfreq support in panfrost driver should work fine with SCPI
as it has clock provider for GPU DVFS.

With SCMI, we don't want to hook to clock framework, but I am yet to come
up with a sane generic way to add SCMI devfreq and integrate it with the
panfrost devfreq.

--
Regards,
Sudeep
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
index 47bc1ac36426..018f3ae4b43c 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
@@ -22,6 +22,10 @@  properties:
           - enum:
              - amlogic,meson-gxm-mali
           - const: arm,mali-t820
+      - items:
+          - enum:
+             - arm,juno-mali
+          - const: arm,mali-t624
       - items:
           - enum:
              - rockchip,rk3288-mali
@@ -39,7 +43,6 @@  properties:
              - samsung,exynos5433-mali
           - const: arm,mali-t760
 
-          # "arm,mali-t624"
           # "arm,mali-t628"
           # "arm,mali-t830"
           # "arm,mali-t880"
diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index 26a039a028b8..9e3e8ce6adfe 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -35,6 +35,18 @@ 
 		clock-names = "apb_pclk";
 	};
 
+	smmu_gpu: iommu@2b400000 {
+		compatible = "arm,mmu-400", "arm,smmu-v1";
+		reg = <0x0 0x2b400000 0x0 0x10000>;
+		interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		#global-interrupts = <1>;
+		power-domains = <&scpi_devpd 1>;
+		dma-coherent;
+		status = "disabled";
+	};
+
 	smmu_pcie: iommu@2b500000 {
 		compatible = "arm,mmu-401", "arm,smmu-v1";
 		reg = <0x0 0x2b500000 0x0 0x10000>;
@@ -487,6 +499,21 @@ 
 		};
 	};
 
+	gpu: gpu@2d000000 {
+		compatible = "arm,juno-mali", "arm,mali-t624";
+		reg = <0 0x2d000000 0 0x10000>;
+		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "gpu", "job", "mmu";
+		clocks = <&scpi_dvfs 2>;
+		power-domains = <&scpi_devpd 1>;
+		dma-coherent;
+		/* The SMMU is only really of interest to bare-metal hypervisors */
+		/* iommus = <&smmu_gpu 0>; */
+		status = "disabled";
+	};
+
 	sram: sram@2e000000 {
 		compatible = "arm,juno-sram-ns", "mmio-sram";
 		reg = <0x0 0x2e000000 0x0 0x8000>;