Boris,

On 3/15/25 5:43 AM, Boris Brezillon wrote:
On Fri, 14 Mar 2025 14:38:56 -0300
Ariel D'Alessandro <ariel.dalessan...@collabora.com> wrote:

Currently, Panfrost only supports MMU configuration in "LEGACY" (as
Bifrost calls it) mode, a (modified) version of LPAE "Large Physical
Address Extension", which in Linux we've called "mali_lpae".

This commit adds support for conditionally enabling AARCH64_4K page
table format. To achieve that, a "GPU optional quirks" field was added
to `struct panfrost_features` with the related flag.

Note that, in order to enable AARCH64_4K mode, the GPU variant must have
the HW_FEATURE_AARCH64_MMU feature flag present.

Signed-off-by: Ariel D'Alessandro <ariel.dalessan...@collabora.com>
---
  drivers/gpu/drm/panfrost/panfrost_device.h |  16 +++
  drivers/gpu/drm/panfrost/panfrost_mmu.c    | 136 +++++++++++++++++++--
  drivers/gpu/drm/panfrost/panfrost_regs.h   |  34 ++++++
  3 files changed, 177 insertions(+), 9 deletions(-)
[snip]
diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
b/drivers/gpu/drm/panfrost/panfrost_mmu.c
index 294f86b3c25e7..f24c23e1f67b8 100644
--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -26,6 +26,48 @@
  #define mmu_write(dev, reg, data) writel(data, dev->iomem + reg)
  #define mmu_read(dev, reg) readl(dev->iomem + reg)
[snip]
  u32 panfrost_mmu_as_get(struct panfrost_device *pfdev, struct panfrost_mmu 
*mmu)
  {
        int as;
@@ -618,6 +720,18 @@ struct panfrost_mmu *panfrost_mmu_ctx_create(struct 
panfrost_device *pfdev)
        u32 va_bits = GPU_MMU_FEATURES_VA_BITS(pfdev->features.mmu_features);
        u32 pa_bits = GPU_MMU_FEATURES_PA_BITS(pfdev->features.mmu_features);
        struct panfrost_mmu *mmu;
+       enum io_pgtable_fmt fmt;
+
+       if (pfdev->comp->gpu_quirks & BIT(GPU_QUIRK_FORCE_AARCH64_PGTABLE)) {
+               if (!panfrost_has_hw_feature(pfdev, HW_FEATURE_AARCH64_MMU)) {
+                       dev_err_once(pfdev->dev,
+                                    "AARCH64_4K page table not supported\n");
+                       return ERR_PTR(-EINVAL);
+               }
+               fmt = ARM_64_LPAE_S1;
+       } else {
+               fmt = ARM_MALI_LPAE;
+       }
mmu = kzalloc(sizeof(*mmu), GFP_KERNEL);
        if (!mmu)
@@ -642,16 +756,20 @@ struct panfrost_mmu *panfrost_mmu_ctx_create(struct 
panfrost_device *pfdev)
                .iommu_dev      = pfdev->dev,
        };
- mmu->pgtbl_ops = alloc_io_pgtable_ops(ARM_MALI_LPAE, &mmu->pgtbl_cfg,
-                                             mmu);
-       if (!mmu->pgtbl_ops) {
-               kfree(mmu);
-               return ERR_PTR(-EINVAL);
-       }
+       mmu->pgtbl_ops = alloc_io_pgtable_ops(fmt, &mmu->pgtbl_cfg, mmu);
+       if (!mmu->pgtbl_ops)
+               goto err_free_mmu;
+
+       if (panfrost_mmu_cfg_init(mmu, fmt))
+               goto err_free_mmu;

How about propagating the error returned by panfrost_mmu_cfg_init()
instead of assuming it's always -EINVAL on failure? Oh, and you need to
call free_io_pgtable_ops(), not just kfree().

Ah, totally, thanks for the heads up. Will fix in v3 right away.

--
Ariel D'Alessandro
Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK Registered in England & Wales, no. 5513718

Reply via email to