[PATCH] drm/panfrost: Add AArch64 page table format support

[PATCH] drm/panfrost: Add AArch64 page table format support

[Rob Herring]

Bifrost GPUs use the standard format stage 1 LPAE page tables matching
the io-pgtable ARM_64_LPAE_S1 format. The one difference is the TCR or
TRANSCFG register as the Mali driver calls it has its own custom layout.

Signed-off-by: Rob Herring <robh@kernel.org>
---
This and compatible strings should be enough for enabling bifrost. 
There's some other feature and issue differences, but I think they all 
either don't matter (because of differences in panfrost) or I've already 
handled them.

This is only compile tested as I don't have h/w. Running the existing 
IGT tests may be sufficient to test this. We should get an error with 
the command stream rather than a MMU fault if this is working correctly.

Rob

 drivers/gpu/drm/panfrost/panfrost_mmu.c  | 28 +++++++++++++++++++++++-
 drivers/gpu/drm/panfrost/panfrost_regs.h |  3 +++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index 762b1bd2a8c2..41d184fab07f 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier:	GPL-2.0
 /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */
+/* Copyright (C) 2019 Arm Ltd. */
 #include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
@@ -111,6 +112,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
 	u64 transtab = cfg->arm_mali_lpae_cfg.transtab;
 	u64 memattr = cfg->arm_mali_lpae_cfg.memattr;
 
+	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
+		transtab = cfg->arm_lpae_s1_cfg.ttbr[0];
+		memattr = cfg->arm_lpae_s1_cfg.mair[0];
+	} else {
+		transtab = cfg->arm_mali_lpae_cfg.transtab;
+		memattr = cfg->arm_mali_lpae_cfg.memattr;
+	}
+
 	mmu_write(pfdev, MMU_INT_CLEAR, ~0);
 	mmu_write(pfdev, MMU_INT_MASK, ~0);
 
@@ -123,6 +132,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
 	mmu_write(pfdev, AS_MEMATTR_LO(as_nr), memattr & 0xffffffffUL);
 	mmu_write(pfdev, AS_MEMATTR_HI(as_nr), memattr >> 32);
 
+	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
+		/* TODO: handle system coherency */
+		mmu_write(pfdev, AS_TRANSCFG_LO(as_nr),
+			AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK |
+			AS_TRANSCFG_ADRMODE_AARCH64_4K);
+		mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
+	}
+
 	write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
 }
 
@@ -134,6 +151,11 @@ static void mmu_disable(struct panfrost_device *pfdev, u32 as_nr)
 	mmu_write(pfdev, AS_MEMATTR_LO(as_nr), 0);
 	mmu_write(pfdev, AS_MEMATTR_HI(as_nr), 0);
 
+	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
+		mmu_write(pfdev, AS_TRANSCFG_LO(as_nr), 0);
+		mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
+	}
+
 	write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
 }
 
@@ -335,6 +357,7 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)
 int panfrost_mmu_init(struct panfrost_device *pfdev)
 {
 	struct io_pgtable_ops *pgtbl_ops;
+	enum io_pgtable_fmt pgt_fmt = ARM_MALI_LPAE;
 	int err, irq;
 
 	pfdev->mmu = devm_kzalloc(pfdev->dev, sizeof(*pfdev->mmu), GFP_KERNEL);
@@ -365,7 +388,10 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
 		.iommu_dev	= pfdev->dev,
 	};
 
-	pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &pfdev->mmu->pgtbl_cfg,
+	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU))
+		pgt_fmt = ARM_64_LPAE_S1;
+
+	pgtbl_ops = alloc_io_pgtable_ops(pgt_fmt, &pfdev->mmu->pgtbl_cfg,
 					 pfdev);
 	if (!pgtbl_ops)
 		return -ENOMEM;
diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h
index 578c5fc2188b..31211df83c30 100644
--- a/drivers/gpu/drm/panfrost/panfrost_regs.h
+++ b/drivers/gpu/drm/panfrost/panfrost_regs.h
@@ -287,6 +287,9 @@
 #define AS_TRANSTAB_LPAE_READ_INNER		BIT(2)
 #define AS_TRANSTAB_LPAE_SHARE_OUTER		BIT(4)
 
+#define AS_TRANSCFG_ADRMODE_AARCH64_4K		6
+#define AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK	(2 << 24)
+
 #define AS_STATUS_AS_ACTIVE			0x01
 
 #define AS_FAULTSTATUS_ACCESS_TYPE_MASK		(0x3 << 8)
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panfrost: Add AArch64 page table format support

[Tomeu Vizoso]

Robin, Steven,

would you or someone else at Arm be able to run the IGT tests [0] on
5.2-rc2 with this patch on top?

I don't have any hw with Bifrost and am not planning to work on the
userspace any time soon, but I think it would be good to at least
check that the kernel doesn't have any obvious bugs.

[0] https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/tests/panfrost_submit.c

Thanks,

Tomeu

On Wed, 15 May 2019 at 10:11, Rob Herring <robh@kernel.org> wrote:
>
> Bifrost GPUs use the standard format stage 1 LPAE page tables matching
> the io-pgtable ARM_64_LPAE_S1 format. The one difference is the TCR or
> TRANSCFG register as the Mali driver calls it has its own custom layout.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> This and compatible strings should be enough for enabling bifrost.
> There's some other feature and issue differences, but I think they all
> either don't matter (because of differences in panfrost) or I've already
> handled them.
>
> This is only compile tested as I don't have h/w. Running the existing
> IGT tests may be sufficient to test this. We should get an error with
> the command stream rather than a MMU fault if this is working correctly.
>
> Rob
>
>  drivers/gpu/drm/panfrost/panfrost_mmu.c  | 28 +++++++++++++++++++++++-
>  drivers/gpu/drm/panfrost/panfrost_regs.h |  3 +++
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 762b1bd2a8c2..41d184fab07f 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier:    GPL-2.0
>  /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */
> +/* Copyright (C) 2019 Arm Ltd. */
>  #include <linux/bitfield.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> @@ -111,6 +112,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
>         u64 transtab = cfg->arm_mali_lpae_cfg.transtab;
>         u64 memattr = cfg->arm_mali_lpae_cfg.memattr;
>
> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
> +               transtab = cfg->arm_lpae_s1_cfg.ttbr[0];
> +               memattr = cfg->arm_lpae_s1_cfg.mair[0];
> +       } else {
> +               transtab = cfg->arm_mali_lpae_cfg.transtab;
> +               memattr = cfg->arm_mali_lpae_cfg.memattr;
> +       }
> +
>         mmu_write(pfdev, MMU_INT_CLEAR, ~0);
>         mmu_write(pfdev, MMU_INT_MASK, ~0);
>
> @@ -123,6 +132,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
>         mmu_write(pfdev, AS_MEMATTR_LO(as_nr), memattr & 0xffffffffUL);
>         mmu_write(pfdev, AS_MEMATTR_HI(as_nr), memattr >> 32);
>
> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
> +               /* TODO: handle system coherency */
> +               mmu_write(pfdev, AS_TRANSCFG_LO(as_nr),
> +                       AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK |
> +                       AS_TRANSCFG_ADRMODE_AARCH64_4K);
> +               mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
> +       }
> +
>         write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
>  }
>
> @@ -134,6 +151,11 @@ static void mmu_disable(struct panfrost_device *pfdev, u32 as_nr)
>         mmu_write(pfdev, AS_MEMATTR_LO(as_nr), 0);
>         mmu_write(pfdev, AS_MEMATTR_HI(as_nr), 0);
>
> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
> +               mmu_write(pfdev, AS_TRANSCFG_LO(as_nr), 0);
> +               mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
> +       }
> +
>         write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
>  }
>
> @@ -335,6 +357,7 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)
>  int panfrost_mmu_init(struct panfrost_device *pfdev)
>  {
>         struct io_pgtable_ops *pgtbl_ops;
> +       enum io_pgtable_fmt pgt_fmt = ARM_MALI_LPAE;
>         int err, irq;
>
>         pfdev->mmu = devm_kzalloc(pfdev->dev, sizeof(*pfdev->mmu), GFP_KERNEL);
> @@ -365,7 +388,10 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
>                 .iommu_dev      = pfdev->dev,
>         };
>
> -       pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &pfdev->mmu->pgtbl_cfg,
> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU))
> +               pgt_fmt = ARM_64_LPAE_S1;
> +
> +       pgtbl_ops = alloc_io_pgtable_ops(pgt_fmt, &pfdev->mmu->pgtbl_cfg,
>                                          pfdev);
>         if (!pgtbl_ops)
>                 return -ENOMEM;
> diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h
> index 578c5fc2188b..31211df83c30 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_regs.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_regs.h
> @@ -287,6 +287,9 @@
>  #define AS_TRANSTAB_LPAE_READ_INNER            BIT(2)
>  #define AS_TRANSTAB_LPAE_SHARE_OUTER           BIT(4)
>
> +#define AS_TRANSCFG_ADRMODE_AARCH64_4K         6
> +#define AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK     (2 << 24)
> +
>  #define AS_STATUS_AS_ACTIVE                    0x01
>
>  #define AS_FAULTSTATUS_ACCESS_TYPE_MASK                (0x3 << 8)
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panfrost: Add AArch64 page table format support

[Robin Murphy]

Hi Tomeu, Rob,

On 28/05/2019 08:03, Tomeu Vizoso wrote:
> Robin, Steven,
> 
> would you or someone else at Arm be able to run the IGT tests [0] on
> 5.2-rc2 with this patch on top?
> 
> I don't have any hw with Bifrost and am not planning to work on the
> userspace any time soon, but I think it would be good to at least
> check that the kernel doesn't have any obvious bugs.

I've managed to cobble something together which appears to sort-of-work, 
although I don't have the knowledge, time or patience to dive down the 
rabbit-hole of getting a working Arm DDK driver to actually prove the 
setup. The immediate observation is that I get a lot of this:

[  305.602001] panfrost 6e000000.gpu: error powering up gpu

Which appears to stem from the poking of STACK_PWRON_LO. Judging by 
CONFIG_MALI_CORESTACK in kbase, maybe we shouldn't always be going there 
anyway (Steve probably knows more, but is away for a few weeks ATM).

I can't make much sense of the IGT output, but trying 
"scripts/run-tests.sh -t panfrost" (because I also don't have the 
patience to watch it skip through all ~63,000 tests...) seems pretty 
much consistent with or without this patch. Same for trying kmscube with 
mesa/master; that produces lots of job errors:

[   42.409568] panfrost 6e000000.gpu: js fault, js=0, 
status=DATA_INVALID_FAULT, head=0x2400b00, tail=0x2400b00
[   42.419380] panfrost 6e000000.gpu: gpu sched timeout, js=0, 
status=0x58, head=0x2400b00, tail=0x2400b00, sched_job=00000000d17b91

rather than MMU faults either way, so at least this change doesn't seem 
to present any significant regression. It looks like without this patch 
we end up using AS_TRANSCFG_ADRMODE_LEGACY, which presumably means 
exactly what it sounds like, although whether that's sufficiently 
reliable I don't know; those kinds of backwards-compatibility features 
do have a habit of eventually disappearing, and what I've tried so far 
is certainly not the latest and greatest.

Robin.

> [0] https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/tests/panfrost_submit.c
> 
> Thanks,
> 
> Tomeu
> 
> On Wed, 15 May 2019 at 10:11, Rob Herring <robh@kernel.org> wrote:
>>
>> Bifrost GPUs use the standard format stage 1 LPAE page tables matching
>> the io-pgtable ARM_64_LPAE_S1 format. The one difference is the TCR or
>> TRANSCFG register as the Mali driver calls it has its own custom layout.
>>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> This and compatible strings should be enough for enabling bifrost.
>> There's some other feature and issue differences, but I think they all
>> either don't matter (because of differences in panfrost) or I've already
>> handled them.
>>
>> This is only compile tested as I don't have h/w. Running the existing
>> IGT tests may be sufficient to test this. We should get an error with
>> the command stream rather than a MMU fault if this is working correctly.
>>
>> Rob
>>
>>   drivers/gpu/drm/panfrost/panfrost_mmu.c  | 28 +++++++++++++++++++++++-
>>   drivers/gpu/drm/panfrost/panfrost_regs.h |  3 +++
>>   2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> index 762b1bd2a8c2..41d184fab07f 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> @@ -1,5 +1,6 @@
>>   // SPDX-License-Identifier:    GPL-2.0
>>   /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */
>> +/* Copyright (C) 2019 Arm Ltd. */
>>   #include <linux/bitfield.h>
>>   #include <linux/delay.h>
>>   #include <linux/interrupt.h>
>> @@ -111,6 +112,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
>>          u64 transtab = cfg->arm_mali_lpae_cfg.transtab;
>>          u64 memattr = cfg->arm_mali_lpae_cfg.memattr;
>>
>> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
>> +               transtab = cfg->arm_lpae_s1_cfg.ttbr[0];
>> +               memattr = cfg->arm_lpae_s1_cfg.mair[0];
>> +       } else {
>> +               transtab = cfg->arm_mali_lpae_cfg.transtab;
>> +               memattr = cfg->arm_mali_lpae_cfg.memattr;
>> +       }
>> +
>>          mmu_write(pfdev, MMU_INT_CLEAR, ~0);
>>          mmu_write(pfdev, MMU_INT_MASK, ~0);
>>
>> @@ -123,6 +132,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
>>          mmu_write(pfdev, AS_MEMATTR_LO(as_nr), memattr & 0xffffffffUL);
>>          mmu_write(pfdev, AS_MEMATTR_HI(as_nr), memattr >> 32);
>>
>> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
>> +               /* TODO: handle system coherency */
>> +               mmu_write(pfdev, AS_TRANSCFG_LO(as_nr),
>> +                       AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK |
>> +                       AS_TRANSCFG_ADRMODE_AARCH64_4K);
>> +               mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
>> +       }
>> +
>>          write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
>>   }
>>
>> @@ -134,6 +151,11 @@ static void mmu_disable(struct panfrost_device *pfdev, u32 as_nr)
>>          mmu_write(pfdev, AS_MEMATTR_LO(as_nr), 0);
>>          mmu_write(pfdev, AS_MEMATTR_HI(as_nr), 0);
>>
>> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
>> +               mmu_write(pfdev, AS_TRANSCFG_LO(as_nr), 0);
>> +               mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
>> +       }
>> +
>>          write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
>>   }
>>
>> @@ -335,6 +357,7 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)
>>   int panfrost_mmu_init(struct panfrost_device *pfdev)
>>   {
>>          struct io_pgtable_ops *pgtbl_ops;
>> +       enum io_pgtable_fmt pgt_fmt = ARM_MALI_LPAE;
>>          int err, irq;
>>
>>          pfdev->mmu = devm_kzalloc(pfdev->dev, sizeof(*pfdev->mmu), GFP_KERNEL);
>> @@ -365,7 +388,10 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
>>                  .iommu_dev      = pfdev->dev,
>>          };
>>
>> -       pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &pfdev->mmu->pgtbl_cfg,
>> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU))
>> +               pgt_fmt = ARM_64_LPAE_S1;
>> +
>> +       pgtbl_ops = alloc_io_pgtable_ops(pgt_fmt, &pfdev->mmu->pgtbl_cfg,
>>                                           pfdev);
>>          if (!pgtbl_ops)
>>                  return -ENOMEM;
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h
>> index 578c5fc2188b..31211df83c30 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_regs.h
>> +++ b/drivers/gpu/drm/panfrost/panfrost_regs.h
>> @@ -287,6 +287,9 @@
>>   #define AS_TRANSTAB_LPAE_READ_INNER            BIT(2)
>>   #define AS_TRANSTAB_LPAE_SHARE_OUTER           BIT(4)
>>
>> +#define AS_TRANSCFG_ADRMODE_AARCH64_4K         6
>> +#define AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK     (2 << 24)
>> +
>>   #define AS_STATUS_AS_ACTIVE                    0x01
>>
>>   #define AS_FAULTSTATUS_ACCESS_TYPE_MASK                (0x3 << 8)
>> --
>> 2.20.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panfrost: Add AArch64 page table format support

[Tomeu Vizoso]

On Wed, 29 May 2019 at 15:00, Robin Murphy <robin.murphy@arm.com> wrote:
>
> Hi Tomeu, Rob,
>
> On 28/05/2019 08:03, Tomeu Vizoso wrote:
> > Robin, Steven,
> >
> > would you or someone else at Arm be able to run the IGT tests [0] on
> > 5.2-rc2 with this patch on top?
> >
> > I don't have any hw with Bifrost and am not planning to work on the
> > userspace any time soon, but I think it would be good to at least
> > check that the kernel doesn't have any obvious bugs.
>
> I've managed to cobble something together which appears to sort-of-work,
> although I don't have the knowledge, time or patience to dive down the
> rabbit-hole of getting a working Arm DDK driver to actually prove the
> setup. The immediate observation is that I get a lot of this:
>
> [  305.602001] panfrost 6e000000.gpu: error powering up gpu
>
> Which appears to stem from the poking of STACK_PWRON_LO. Judging by
> CONFIG_MALI_CORESTACK in kbase, maybe we shouldn't always be going there
> anyway (Steve probably knows more, but is away for a few weeks ATM).
>
> I can't make much sense of the IGT output, but trying
> "scripts/run-tests.sh -t panfrost" (because I also don't have the
> patience to watch it skip through all ~63,000 tests...) seems pretty
> much consistent with or without this patch.

Oops, sorry about that. It would have been sufficient to directly run
these executables:

tests/panfrost_gem_new
tests/panfrost_get_param
tests/panfrost_prime
tests/panfrost_submit

> Same for trying kmscube with
> mesa/master; that produces lots of job errors:
>
> [   42.409568] panfrost 6e000000.gpu: js fault, js=0,
> status=DATA_INVALID_FAULT, head=0x2400b00, tail=0x2400b00
> [   42.419380] panfrost 6e000000.gpu: gpu sched timeout, js=0,
> status=0x58, head=0x2400b00, tail=0x2400b00, sched_job=00000000d17b91
>
> rather than MMU faults either way, so at least this change doesn't seem
> to present any significant regression.

That sounds pretty good to me. We know that the cmdstream and compiler
aren't ready yet for Bifrost.

> It looks like without this patch
> we end up using AS_TRANSCFG_ADRMODE_LEGACY, which presumably means
> exactly what it sounds like, although whether that's sufficiently
> reliable I don't know; those kinds of backwards-compatibility features
> do have a habit of eventually disappearing, and what I've tried so far
> is certainly not the latest and greatest.

One for Rob when he's back, I think  :)

Thanks a lot!

Tomeu

> Robin.
>
> > [0] https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/tests/panfrost_submit.c
> >
> > Thanks,
> >
> > Tomeu
> >
> > On Wed, 15 May 2019 at 10:11, Rob Herring <robh@kernel.org> wrote:
> >>
> >> Bifrost GPUs use the standard format stage 1 LPAE page tables matching
> >> the io-pgtable ARM_64_LPAE_S1 format. The one difference is the TCR or
> >> TRANSCFG register as the Mali driver calls it has its own custom layout.
> >>
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >> This and compatible strings should be enough for enabling bifrost.
> >> There's some other feature and issue differences, but I think they all
> >> either don't matter (because of differences in panfrost) or I've already
> >> handled them.
> >>
> >> This is only compile tested as I don't have h/w. Running the existing
> >> IGT tests may be sufficient to test this. We should get an error with
> >> the command stream rather than a MMU fault if this is working correctly.
> >>
> >> Rob
> >>
> >>   drivers/gpu/drm/panfrost/panfrost_mmu.c  | 28 +++++++++++++++++++++++-
> >>   drivers/gpu/drm/panfrost/panfrost_regs.h |  3 +++
> >>   2 files changed, 30 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> >> index 762b1bd2a8c2..41d184fab07f 100644
> >> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> >> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> >> @@ -1,5 +1,6 @@
> >>   // SPDX-License-Identifier:    GPL-2.0
> >>   /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */
> >> +/* Copyright (C) 2019 Arm Ltd. */
> >>   #include <linux/bitfield.h>
> >>   #include <linux/delay.h>
> >>   #include <linux/interrupt.h>
> >> @@ -111,6 +112,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
> >>          u64 transtab = cfg->arm_mali_lpae_cfg.transtab;
> >>          u64 memattr = cfg->arm_mali_lpae_cfg.memattr;
> >>
> >> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
> >> +               transtab = cfg->arm_lpae_s1_cfg.ttbr[0];
> >> +               memattr = cfg->arm_lpae_s1_cfg.mair[0];
> >> +       } else {
> >> +               transtab = cfg->arm_mali_lpae_cfg.transtab;
> >> +               memattr = cfg->arm_mali_lpae_cfg.memattr;
> >> +       }
> >> +
> >>          mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> >>          mmu_write(pfdev, MMU_INT_MASK, ~0);
> >>
> >> @@ -123,6 +132,14 @@ void panfrost_mmu_enable(struct panfrost_device *pfdev, u32 as_nr)
> >>          mmu_write(pfdev, AS_MEMATTR_LO(as_nr), memattr & 0xffffffffUL);
> >>          mmu_write(pfdev, AS_MEMATTR_HI(as_nr), memattr >> 32);
> >>
> >> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
> >> +               /* TODO: handle system coherency */
> >> +               mmu_write(pfdev, AS_TRANSCFG_LO(as_nr),
> >> +                       AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK |
> >> +                       AS_TRANSCFG_ADRMODE_AARCH64_4K);
> >> +               mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
> >> +       }
> >> +
> >>          write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
> >>   }
> >>
> >> @@ -134,6 +151,11 @@ static void mmu_disable(struct panfrost_device *pfdev, u32 as_nr)
> >>          mmu_write(pfdev, AS_MEMATTR_LO(as_nr), 0);
> >>          mmu_write(pfdev, AS_MEMATTR_HI(as_nr), 0);
> >>
> >> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
> >> +               mmu_write(pfdev, AS_TRANSCFG_LO(as_nr), 0);
> >> +               mmu_write(pfdev, AS_TRANSCFG_HI(as_nr), 0);
> >> +       }
> >> +
> >>          write_cmd(pfdev, as_nr, AS_COMMAND_UPDATE);
> >>   }
> >>
> >> @@ -335,6 +357,7 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)
> >>   int panfrost_mmu_init(struct panfrost_device *pfdev)
> >>   {
> >>          struct io_pgtable_ops *pgtbl_ops;
> >> +       enum io_pgtable_fmt pgt_fmt = ARM_MALI_LPAE;
> >>          int err, irq;
> >>
> >>          pfdev->mmu = devm_kzalloc(pfdev->dev, sizeof(*pfdev->mmu), GFP_KERNEL);
> >> @@ -365,7 +388,10 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
> >>                  .iommu_dev      = pfdev->dev,
> >>          };
> >>
> >> -       pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &pfdev->mmu->pgtbl_cfg,
> >> +       if (panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU))
> >> +               pgt_fmt = ARM_64_LPAE_S1;
> >> +
> >> +       pgtbl_ops = alloc_io_pgtable_ops(pgt_fmt, &pfdev->mmu->pgtbl_cfg,
> >>                                           pfdev);
> >>          if (!pgtbl_ops)
> >>                  return -ENOMEM;
> >> diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h
> >> index 578c5fc2188b..31211df83c30 100644
> >> --- a/drivers/gpu/drm/panfrost/panfrost_regs.h
> >> +++ b/drivers/gpu/drm/panfrost/panfrost_regs.h
> >> @@ -287,6 +287,9 @@
> >>   #define AS_TRANSTAB_LPAE_READ_INNER            BIT(2)
> >>   #define AS_TRANSTAB_LPAE_SHARE_OUTER           BIT(4)
> >>
> >> +#define AS_TRANSCFG_ADRMODE_AARCH64_4K         6
> >> +#define AS_TRANSCFG_PTW_MEMATTR_WRITE_BACK     (2 << 24)
> >> +
> >>   #define AS_STATUS_AS_ACTIVE                    0x01
> >>
> >>   #define AS_FAULTSTATUS_ACCESS_TYPE_MASK                (0x3 << 8)
> >> --
> >> 2.20.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panfrost: Add AArch64 page table format support

[Rob Herring]

On Wed, May 29, 2019 at 10:38 AM Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>
> On Wed, 29 May 2019 at 15:00, Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > Hi Tomeu, Rob,
> >
> > On 28/05/2019 08:03, Tomeu Vizoso wrote:
> > > Robin, Steven,
> > >
> > > would you or someone else at Arm be able to run the IGT tests [0] on
> > > 5.2-rc2 with this patch on top?
> > >
> > > I don't have any hw with Bifrost and am not planning to work on the
> > > userspace any time soon, but I think it would be good to at least
> > > check that the kernel doesn't have any obvious bugs.
> >
> > I've managed to cobble something together which appears to sort-of-work,
> > although I don't have the knowledge, time or patience to dive down the
> > rabbit-hole of getting a working Arm DDK driver to actually prove the
> > setup. The immediate observation is that I get a lot of this:
> >
> > [  305.602001] panfrost 6e000000.gpu: error powering up gpu
> >
> > Which appears to stem from the poking of STACK_PWRON_LO. Judging by
> > CONFIG_MALI_CORESTACK in kbase, maybe we shouldn't always be going there
> > anyway (Steve probably knows more, but is away for a few weeks ATM).
> >
> > I can't make much sense of the IGT output, but trying
> > "scripts/run-tests.sh -t panfrost" (because I also don't have the
> > patience to watch it skip through all ~63,000 tests...) seems pretty
> > much consistent with or without this patch.
>
> Oops, sorry about that. It would have been sufficient to directly run
> these executables:
>
> tests/panfrost_gem_new
> tests/panfrost_get_param
> tests/panfrost_prime
> tests/panfrost_submit
>
> > Same for trying kmscube with
> > mesa/master; that produces lots of job errors:
> >
> > [   42.409568] panfrost 6e000000.gpu: js fault, js=0,
> > status=DATA_INVALID_FAULT, head=0x2400b00, tail=0x2400b00
> > [   42.419380] panfrost 6e000000.gpu: gpu sched timeout, js=0,
> > status=0x58, head=0x2400b00, tail=0x2400b00, sched_job=00000000d17b91
> >
> > rather than MMU faults either way, so at least this change doesn't seem
> > to present any significant regression.
>
> That sounds pretty good to me. We know that the cmdstream and compiler
> aren't ready yet for Bifrost.
>
> > It looks like without this patch
> > we end up using AS_TRANSCFG_ADRMODE_LEGACY, which presumably means
> > exactly what it sounds like, although whether that's sufficiently
> > reliable I don't know; those kinds of backwards-compatibility features
> > do have a habit of eventually disappearing, and what I've tried so far
> > is certainly not the latest and greatest.
>
> One for Rob when he's back, I think  :)

I wouldn't have expected AS_TRANSCFG_ADRMODE_LEGACY to work and if it
did it was by chance. So I don't think it is something we want to
support.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panfrost: Add AArch64 page table format support

[Steven Price]

On 10/06/2019 14:20, Rob Herring wrote:
[...]
> I wouldn't have expected AS_TRANSCFG_ADRMODE_LEGACY to work and if it
> did it was by chance. So I don't think it is something we want to
> support.

Actually legacy mode is supported on (most?) Bifrost GPUs. But best to
follow the lead of kbase here as that will be what has been tested.

Steve
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel