On 02/07/2023 17:31, Rob Clark wrote:
On Sat, Jul 1, 2023 at 5:24 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
On Sat, 1 Jul 2023 at 18:50, Rob Clark <robdclark@xxxxxxxxx> wrote:
On Fri, Jun 30, 2023 at 4:12 PM Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
Recently, a WARN_ON() was introduced to ensure that revn is filled before
adreno_is_aXYZ is called. This however doesn't work very well when revn is
0 by design (such as for A635). Fill it in as a stopgap solution for
-fixes.
Fixes: cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified before being set")
Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
Changes in v2:
- add fixes
- Link to v1: https://lore.kernel.org/r/20230628-topic-a635-v1-1-5056e09c08fb@xxxxxxxxxx
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index cb94cfd137a8..8ea7eae9fc52 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -345,6 +345,7 @@ static const struct adreno_info gpulist[] = {
.address_space_size = SZ_16G,
}, {
.rev = ADRENO_REV(6, 3, 5, ANY_ID),
+ .revn = 635,
.fw = {
[ADRENO_FW_SQE] = "a660_sqe.fw",
[ADRENO_FW_GMU] = "a660_gmu.bin",
hmm, I realized a problem with this, it would change what
MSM_PARAM_GPU_ID and more importantly MSM_PARAM_CHIP_ID return.. The
former should be "harmless", although it isn't a good idea for uabi
changes to be a side effect of a fix. The latter is more problematic.
I'd say MSM_PARAM_GPU_ID is broken for 635 anyway (won't it return 0
in this case)?
So the new value should be correct.
no, it is very much intentional that GPU_ID returns 0 for newer GPUs,
userspace should be matching on CHIP_ID. (Also, we should be moving
away from trying to infer generation/etc from CHIP_ID.. userspace is
farther ahead of the kernel on this.)
Thanks for the explanation. So in theory we can change this to always
return 0? Or must we keep it to keep UABI / compatibility?
I'm trying to understand if we can drop revn at all.
But more importantly, why are we exporting speedbin in
MSM_PARAM_CHIP_ID only if there is no revn? And why are we exporting
the speedbin at all as a part of CHIP_ID?
Basically just being paranoid about not changing uabi. It probably
would be ok to export the speedbin for all, but I'd have to double
check mesa version history.
Thanks!
BR,
-R
I think I'm leaning more towards reverting commit cc943f43ece7
("drm/msm/adreno: warn if chip revn is verified before being set") for
-fixes. I'm still thinking about options for a longer term fix.
BR,
-R
---
base-commit: 5c875096d59010cee4e00da1f9c7bdb07a025dc2
change-id: 20230628-topic-a635-1b3c2c987417
Best regards,
--
Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
--
With best wishes
Dmitry
--
With best wishes
Dmitry