This series brings some niceties in preparation for A7xx introduction.
It should be fully independent of the GMU wrapper series.
Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (6):
drm/msm/a6xx: Explain CP_PROTECT_CNTL writes in a6xx_set_cp_protect
drm/msm/a6xx: Skip empty protection ranges entries
drm/msm/a6xx: Ensure clean GMU state in a6xx_gmu_fw_start
drm/msm/a6xx: Improve GMU force shutdown sequence
drm/msm/a6xx: Use GMU_ALWAYS_ON_COUNTER for GMU-equipped GPUs in timestamp
drm/msm/a6xx: Fix up GMU region reservations
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 21 +++++++++++++++++----
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 23 ++++++++++++-----------
2 files changed, 29 insertions(+), 15 deletions(-)
---
base-commit: 065efa589871e93b6610c70c1e9de274ef1f1ba2
change-id: 20230517-topic-a7xx_prep-787a69c7d0ff
Best regards,
--
Konrad Dybcio <[email protected]>
We have the necessary information, so explain which bit does what.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 9fb214f150dd..deed42675fe2 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -771,9 +771,10 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
}
/*
- * Enable access protection to privileged registers, fault on an access
- * protect violation and select the last span to protect from the start
- * address all the way to the end of the register address space
+ * BIT(0) - Enable access protection to privileged registers
+ * BIT(1) - Enable fault on an access protect violation
+ * BIT(3) - Select the last span to protect from the start
+ * address all the way to the end of the register address space
*/
gpu_write(gpu, REG_A6XX_CP_PROTECT_CNTL, BIT(0) | BIT(1) | BIT(3));
--
2.40.1
While it's not very well understood, there is some sort of a fault
handler implemented in the GMU firmware which triggers when a certain
bit is set, resulting in the M3 core not booting up the way we expect
it to.
Write a magic value to a magic register to hopefully prevent that
from happening.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index e16b4b3f8535..ea6d671e7c6c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -796,6 +796,12 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu, unsigned int state)
gmu_write(gmu, REG_A6XX_GMU_AHB_FENCE_RANGE_0,
(1 << 31) | (0xa << 18) | (0xa0));
+ /*
+ * Snapshots toggle the NMI bit which will result in a jump to the NMI
+ * handler instead of __main. Set the M3 config value to avoid that.
+ */
+ gmu_write(gmu, REG_A6XX_GMU_CM3_CFG, 0x4052);
+
chipid = adreno_gpu->rev.core << 24;
chipid |= adreno_gpu->rev.major << 16;
chipid |= adreno_gpu->rev.minor << 12;
--
2.40.1
Use the always-on counter provided by the GMU to skip having to
keep the GPU online.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 8707e8b6ac7e..d2a999b90589 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1664,12 +1664,9 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
mutex_lock(&a6xx_gpu->gmu.lock);
- /* Force the GPU power on so we can read this register */
- a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
-
- *value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER);
-
- a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
+ *value = gmu_read64(&a6xx_gpu->gmu,
+ REG_A6XX_GMU_ALWAYS_ON_COUNTER_L,
+ REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
mutex_unlock(&a6xx_gpu->gmu.lock);
--
2.40.1
Change the order of region allocations to make the addresses match
downstream. This shouldn't matter very much, but helps eliminate one
more difference when comparing register accesses.
Also, make the log region 16K long. That's what it is, unconditionally
on A6xx and A7xx.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 8004b582e45f..386c81e1a2f3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1614,13 +1614,13 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
goto err_memory;
}
- /* Allocate memory for for the HFI queues */
- ret = a6xx_gmu_memory_alloc(gmu, &gmu->hfi, SZ_16K, 0, "hfi");
+ /* Allocate memory for the GMU log region */
+ ret = a6xx_gmu_memory_alloc(gmu, &gmu->log, SZ_16K, 0, "log");
if (ret)
goto err_memory;
- /* Allocate memory for the GMU log region */
- ret = a6xx_gmu_memory_alloc(gmu, &gmu->log, SZ_4K, 0, "log");
+ /* Allocate memory for for the HFI queues */
+ ret = a6xx_gmu_memory_alloc(gmu, &gmu->hfi, SZ_16K, 0, "hfi");
if (ret)
goto err_memory;
--
2.40.1
Some specific SKUs leave certain protection range registers empty.
Allow for that behavior.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index deed42675fe2..8707e8b6ac7e 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -778,8 +778,11 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
*/
gpu_write(gpu, REG_A6XX_CP_PROTECT_CNTL, BIT(0) | BIT(1) | BIT(3));
- for (i = 0; i < count - 1; i++)
- gpu_write(gpu, REG_A6XX_CP_PROTECT(i), regs[i]);
+ for (i = 0; i < count - 1; i++) {
+ /* Intentionally skip writing to some registers */
+ if (regs[i])
+ gpu_write(gpu, REG_A6XX_CP_PROTECT(i), regs[i]);
+ }
/* last CP_PROTECT to have "infinite" length on the last entry */
gpu_write(gpu, REG_A6XX_CP_PROTECT(count_max - 1), regs[i]);
}
--
2.40.1
The GMU force shutdown sequence involves some additional register cleanup
which was not implemented previously. Do so.
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index ea6d671e7c6c..8004b582e45f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -930,6 +930,13 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
/* Make sure there are no outstanding RPMh votes */
a6xx_gmu_rpmh_off(gmu);
+ /* Clear the WRITEDROPPED fields and put fence into allow mode */
+ gmu_write(gmu, REG_A6XX_GMU_AHB_FENCE_STATUS_CLR, 0x7);
+ gmu_write(gmu, REG_A6XX_GMU_AO_AHB_FENCE_CTRL, 0);
+
+ /* Make sure the above writes go through */
+ wmb();
+
/* Halt the gmu cm3 core */
gmu_write(gmu, REG_A6XX_GMU_CM3_SYSRESET, 1);
--
2.40.1
AFAIK GMU_ALWAYS_ON_COUNTER does not have the same value as
CP_ALWAYS_ON_COUNTER (only the same frequency), so changing this would
break userspace expecting to be able to compare the value returned by
MSM_PARAM_TIMESTAMP with CP timestamp values.
On 5/17/23 12:50 PM, Konrad Dybcio wrote:
> Use the always-on counter provided by the GMU to skip having to
> keep the GPU online.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 8707e8b6ac7e..d2a999b90589 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1664,12 +1664,9 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>
> mutex_lock(&a6xx_gpu->gmu.lock);
>
> - /* Force the GPU power on so we can read this register */
> - a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
> -
> - *value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER);
> -
> - a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
> + *value = gmu_read64(&a6xx_gpu->gmu,
> + REG_A6XX_GMU_ALWAYS_ON_COUNTER_L,
> + REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
>
> mutex_unlock(&a6xx_gpu->gmu.lock);
>
>
On 17.05.2023 20:09, Jonathan Marek wrote:
> AFAIK GMU_ALWAYS_ON_COUNTER does not have the same value as CP_ALWAYS_ON_COUNTER (only the same frequency), so changing this would break userspace expecting to be able to compare the value returned by MSM_PARAM_TIMESTAMP with CP timestamp values.
FWIW A630 and A730 seem to work fine with this patch. Anything
in particular I should look out for?
Konrad
>
> On 5/17/23 12:50 PM, Konrad Dybcio wrote:
>> Use the always-on counter provided by the GMU to skip having to
>> keep the GPU online.
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> index 8707e8b6ac7e..d2a999b90589 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> @@ -1664,12 +1664,9 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>> mutex_lock(&a6xx_gpu->gmu.lock);
>> - /* Force the GPU power on so we can read this register */
>> - a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
>> -
>> - *value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER);
>> -
>> - a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
>> + *value = gmu_read64(&a6xx_gpu->gmu,
>> + REG_A6XX_GMU_ALWAYS_ON_COUNTER_L,
>> + REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
>> mutex_unlock(&a6xx_gpu->gmu.lock);
>>
On 5/17/23 3:08 PM, Konrad Dybcio wrote:
>
>
> On 17.05.2023 20:09, Jonathan Marek wrote:
>> AFAIK GMU_ALWAYS_ON_COUNTER does not have the same value as CP_ALWAYS_ON_COUNTER (only the same frequency), so changing this would break userspace expecting to be able to compare the value returned by MSM_PARAM_TIMESTAMP with CP timestamp values.
> FWIW A630 and A730 seem to work fine with this patch. Anything
> in particular I should look out for?
mesa uses it for perfetto tracing to synchronize the GPU/CPU timelines.
>
> Konrad
>>
>> On 5/17/23 12:50 PM, Konrad Dybcio wrote:
>>> Use the always-on counter provided by the GMU to skip having to
>>> keep the GPU online.
>>>
>>> Signed-off-by: Konrad Dybcio <[email protected]>
>>> ---
>>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 9 +++------
>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> index 8707e8b6ac7e..d2a999b90589 100644
>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> @@ -1664,12 +1664,9 @@ static int a6xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
>>> mutex_lock(&a6xx_gpu->gmu.lock);
>>> - /* Force the GPU power on so we can read this register */
>>> - a6xx_gmu_set_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
>>> -
>>> - *value = gpu_read64(gpu, REG_A6XX_CP_ALWAYS_ON_COUNTER);
>>> -
>>> - a6xx_gmu_clear_oob(&a6xx_gpu->gmu, GMU_OOB_PERFCOUNTER_SET);
>>> + *value = gmu_read64(&a6xx_gpu->gmu,
>>> + REG_A6XX_GMU_ALWAYS_ON_COUNTER_L,
>>> + REG_A6XX_GMU_ALWAYS_ON_COUNTER_H);
>>> mutex_unlock(&a6xx_gpu->gmu.lock);
>>>
Hi Konrad,
kernel test robot noticed the following build errors:
[auto build test ERROR on 065efa589871e93b6610c70c1e9de274ef1f1ba2]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/drm-msm-a6xx-Explain-CP_PROTECT_CNTL-writes-in-a6xx_set_cp_protect/20230518-010130
base: 065efa589871e93b6610c70c1e9de274ef1f1ba2
patch link: https://lore.kernel.org/r/20230517-topic-a7xx_prep-v1-4-7a964f2e99c2%40linaro.org
patch subject: [PATCH 4/6] drm/msm/a6xx: Improve GMU force shutdown sequence
config: mips-allyesconfig
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c8d08c2d02fc0a1ef0cdcc93d041e3802e01904e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Konrad-Dybcio/drm-msm-a6xx-Explain-CP_PROTECT_CNTL-writes-in-a6xx_set_cp_protect/20230518-010130
git checkout c8d08c2d02fc0a1ef0cdcc93d041e3802e01904e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
drivers/gpu/drm/msm/adreno/a6xx_gmu.c: In function 'a6xx_gmu_force_off':
>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c:934:24: error: 'REG_A6XX_GMU_AHB_FENCE_STATUS_CLR' undeclared (first use in this function); did you mean 'REG_A6XX_GMU_AHB_FENCE_STATUS'?
934 | gmu_write(gmu, REG_A6XX_GMU_AHB_FENCE_STATUS_CLR, 0x7);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| REG_A6XX_GMU_AHB_FENCE_STATUS
drivers/gpu/drm/msm/adreno/a6xx_gmu.c:934:24: note: each undeclared identifier is reported only once for each function it appears in
vim +934 drivers/gpu/drm/msm/adreno/a6xx_gmu.c
913
914 /* Force the GMU off in case it isn't responsive */
915 static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
916 {
917 struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
918 struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
919 struct msm_gpu *gpu = &adreno_gpu->base;
920
921 /* Flush all the queues */
922 a6xx_hfi_stop(gmu);
923
924 /* Stop the interrupts */
925 a6xx_gmu_irq_disable(gmu);
926
927 /* Force off SPTP in case the GMU is managing it */
928 a6xx_sptprac_disable(gmu);
929
930 /* Make sure there are no outstanding RPMh votes */
931 a6xx_gmu_rpmh_off(gmu);
932
933 /* Clear the WRITEDROPPED fields and put fence into allow mode */
> 934 gmu_write(gmu, REG_A6XX_GMU_AHB_FENCE_STATUS_CLR, 0x7);
935 gmu_write(gmu, REG_A6XX_GMU_AO_AHB_FENCE_CTRL, 0);
936
937 /* Make sure the above writes go through */
938 wmb();
939
940 /* Halt the gmu cm3 core */
941 gmu_write(gmu, REG_A6XX_GMU_CM3_SYSRESET, 1);
942
943 a6xx_bus_clear_pending_transactions(adreno_gpu, true);
944
945 /* Reset GPU core blocks */
946 gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1);
947 udelay(100);
948 }
949
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki