drm/panfrost: Add AArch64 page table format support
diff mbox series

Message ID 20190513144012.17243-1-robh@kernel.org
State New
Headers show
Series
  • drm/panfrost: Add AArch64 page table format support
Related show

Commit Message

Rob Herring May 13, 2019, 2:40 p.m. UTC
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(-)

Comments

Tomeu Vizoso May 28, 2019, 7:03 a.m. UTC | #1
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
Robin Murphy May 29, 2019, 1 p.m. UTC | #2
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
Tomeu Vizoso May 29, 2019, 3:38 p.m. UTC | #3
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
Rob Herring June 10, 2019, 1:20 p.m. UTC | #4
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
Steven Price June 27, 2019, 1:54 p.m. UTC | #5
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

Patch
diff mbox series

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)