Hi, On 09/09/2020 14:23, Steven Price wrote: > On 08/09/2020 16:18, Neil Armstrong wrote: >> 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 <narmstr...@baylibre.com> >> --- >> 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 > >> + 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. > >> +} >> + >> 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 > >> + >> #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 >> >