[2/2] drm/panfrost: Use coherent pagetable walk on Juno
diff mbox series

Message ID 1e499f85220b735849126171e64ebdd1da0302ce.1569856049.git.robin.murphy@arm.com
State New
Headers show
Series
  • [1/2] arm64: dts: juno: add GPU subsystem
Related show

Commit Message

Robin Murphy Sept. 30, 2019, 3:24 p.m. UTC
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(+)

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)
>
Rob Herring Sept. 30, 2019, 5:52 p.m. UTC | #2
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 | #3
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)
>>
>

Patch
diff mbox series

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;
+
 	mmu->pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &mmu->pgtbl_cfg,
 					      priv);
 	if (!mmu->pgtbl_ops)