Re: [PATCH 4/5] drm/panfrost: add amlogic reset quirk callback

Hi,
On 09/09/2020 14:23, Steven Price wrote:

On 08/09/2020 16:18, Neil Armstrong wrote:

quoted

The T820, G31 & G52 GPUs integratewd by Amlogic in the respective GXM, G12A/SM1 & G12B
SoCs needs a quirk in the PWR registers at the GPU reset time.

Since the documentation of the GPU cores are not public, we do not know what does these
values, but they permit having a fully functional GPU running with Panfrost.

Signed-off-by: Neil Armstrong <redacted>
---
  drivers/gpu/drm/panfrost/panfrost_gpu.c  | 13 +++++++++++++
  drivers/gpu/drm/panfrost/panfrost_gpu.h  |  2 ++
  drivers/gpu/drm/panfrost/panfrost_regs.h |  3 +++
  3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index c129aaf77790..018737bd4ac6 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c

@@ -80,6 +80,19 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev)

      return 0;
  }
  +void panfrost_gpu_amlogic_quirks(struct panfrost_device *pfdev)
+{
+    /*
+     * The Amlogic integrated Mali-T820, Mali-G31 & Mali-G52 needs
+     * these undocumented bits to be set in order to operate
+     * correctly.
+     * These GPU_PWR registers contains:
+     * "device-specific power control value"
+     */
+    gpu_write(pfdev, GPU_PWR_KEY, 0x2968A819);

As Alyssa has mentioned this magic value is not Amlogic specific, but is just the unlock key value, so please add the define in panfrost-gpu.h

Acked

quoted

+    gpu_write(pfdev, GPU_PWR_OVERRIDE1, 0xfff | (0x20 << 16));

But PWR_OVERRIDE1 is indeed device specific so I can't offer an insight here.

Yep.

quoted

+}
+
  static void panfrost_gpu_init_quirks(struct panfrost_device *pfdev)
  {
      u32 quirks = 0;

diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.h b/drivers/gpu/drm/panfrost/panfrost_gpu.h
index 4112412087b2..a881d7dc812f 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.h

@@ -16,4 +16,6 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev);

  void panfrost_gpu_power_on(struct panfrost_device *pfdev);
  void panfrost_gpu_power_off(struct panfrost_device *pfdev);
  +void panfrost_gpu_amlogic_reset_quirk(struct panfrost_device *pfdev);

You need to be consistent about the name - this has _reset_, the above function doesn't.

Yep, will be fixed in next version.

Thanks for the review,
Neil

Steve

quoted

+
  #endif

diff --git a/drivers/gpu/drm/panfrost/panfrost_regs.h b/drivers/gpu/drm/panfrost/panfrost_regs.h
index ea38ac60581c..fa0d02f3c830 100644
--- a/drivers/gpu/drm/panfrost/panfrost_regs.h
+++ b/drivers/gpu/drm/panfrost/panfrost_regs.h

@@ -51,6 +51,9 @@

  #define GPU_STATUS            0x34
  #define   GPU_STATUS_PRFCNT_ACTIVE    BIT(2)
  #define GPU_LATEST_FLUSH_ID        0x38
+#define GPU_PWR_KEY            0x050    /* (WO) Power manager key register */
+#define GPU_PWR_OVERRIDE0        0x054    /* (RW) Power manager override settings */
+#define GPU_PWR_OVERRIDE1        0x058    /* (RW) Power manager override settings */
  #define GPU_FAULT_STATUS        0x3C
  #define GPU_FAULT_ADDRESS_LO        0x40
  #define GPU_FAULT_ADDRESS_HI        0x44

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel