| Message ID | 20190513144012.17243-1-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/panfrost: Add AArch64 page table format support | expand |
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
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
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
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
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
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)
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(-)