Message ID | 20230706211045.204925-11-robdclark@gmail.com |
---|---|
State | New |
Headers | show |
Series | drm/msm/adreno: Move away from legacy revision matching | expand |
On Thu, Jul 6, 2023 at 4:36 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 6.07.2023 23:10, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > This is used in a few places, including one that is parsed by userspace > > tools. So let's standardize it a bit better. > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > Userspace parsed this weird string instead of the hex-based chipid? > > weird^2 AFAICT it is just crashdec (the creatively named tool for parsing gpu devcore dumps) which parses using "%u.%u.%u.%u".. I suppose one _could_ make the argument that, since userspace doesn't yet support any device where "%x.%x.%x.%x" parsing would be different, we could get away with switching to hex without it being an ABI break.. BR, -R > Konrad > > drivers/gpu/drm/msm/adreno/adreno_device.c | 8 +++----- > > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 19 ++++++++----------- > > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 6 ++++++ > > 3 files changed, 17 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c > > index dcd6363ac7b0..fd2e183bce60 100644 > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c > > @@ -661,14 +661,12 @@ static int adreno_bind(struct device *dev, struct device *master, void *data) > > info = adreno_info(config.rev); > > > > if (!info) { > > - dev_warn(drm->dev, "Unknown GPU revision: %u.%u.%u.%u\n", > > - config.rev.core, config.rev.major, > > - config.rev.minor, config.rev.patchid); > > + dev_warn(drm->dev, "Unknown GPU revision: %"ADRENO_CHIPID_FMT"\n", > > + ADRENO_CHIPID_ARGS(config.rev)); > > return -ENXIO; > > } > > > > - DBG("Found GPU: %u.%u.%u.%u", config.rev.core, config.rev.major, > > - config.rev.minor, config.rev.patchid); > > + DBG("Found GPU: %"ADRENO_CHIPID_FMT, ADRENO_CHIPID_ARGS(config.rev)); > > > > priv->is_a2xx = info->family < ADRENO_3XX; > > priv->has_cached_coherent = > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > > index 75ff7fb46099..1a982a926f21 100644 > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > > @@ -847,10 +847,9 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, > > if (IS_ERR_OR_NULL(state)) > > return; > > > > - drm_printf(p, "revision: %d (%d.%d.%d.%d)\n", > > - adreno_gpu->info->revn, adreno_gpu->rev.core, > > - adreno_gpu->rev.major, adreno_gpu->rev.minor, > > - adreno_gpu->rev.patchid); > > + drm_printf(p, "revision: %u (%"ADRENO_CHIPID_FMT")\n", > > + adreno_gpu->info->revn, > > + ADRENO_CHIPID_ARGS(adreno_gpu->rev)); > > /* > > * If this is state collected due to iova fault, so fault related info > > * > > @@ -921,10 +920,9 @@ void adreno_dump_info(struct msm_gpu *gpu) > > struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > > int i; > > > > - printk("revision: %d (%d.%d.%d.%d)\n", > > - adreno_gpu->info->revn, adreno_gpu->rev.core, > > - adreno_gpu->rev.major, adreno_gpu->rev.minor, > > - adreno_gpu->rev.patchid); > > + printk("revision: %u (%"ADRENO_CHIPID_FMT")\n", > > + adreno_gpu->info->revn, > > + ADRENO_CHIPID_ARGS(adreno_gpu->rev)); > > > > for (i = 0; i < gpu->nr_rings; i++) { > > struct msm_ringbuffer *ring = gpu->rb[i]; > > @@ -1105,9 +1103,8 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > > speedbin = 0xffff; > > adreno_gpu->speedbin = (uint16_t) (0xffff & speedbin); > > > > - gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%d.%d.%d.%d", > > - rev->core, rev->major, rev->minor, > > - rev->patchid); > > + gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%"ADRENO_CHIPID_FMT, > > + ADRENO_CHIPID_ARGS(config->rev)); > > if (!gpu_name) > > return -ENOMEM; > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > index 2fa14dcd4e40..73e7155f164c 100644 > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > @@ -66,6 +66,12 @@ struct adreno_rev { > > #define ADRENO_REV(core, major, minor, patchid) \ > > ((struct adreno_rev){ core, major, minor, patchid }) > > > > +/* Helper for formating the chip_id in the way that userspace tools like > > + * crashdec expect. > > + */ > > +#define ADRENO_CHIPID_FMT "u.%u.%u.%u" > > +#define ADRENO_CHIPID_ARGS(_r) (_r).core, (_r).major, (_r).minor, (_r).patchid > > + > > struct adreno_gpu_funcs { > > struct msm_gpu_funcs base; > > int (*get_timestamp)(struct msm_gpu *gpu, uint64_t *value);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c index dcd6363ac7b0..fd2e183bce60 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_device.c +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c @@ -661,14 +661,12 @@ static int adreno_bind(struct device *dev, struct device *master, void *data) info = adreno_info(config.rev); if (!info) { - dev_warn(drm->dev, "Unknown GPU revision: %u.%u.%u.%u\n", - config.rev.core, config.rev.major, - config.rev.minor, config.rev.patchid); + dev_warn(drm->dev, "Unknown GPU revision: %"ADRENO_CHIPID_FMT"\n", + ADRENO_CHIPID_ARGS(config.rev)); return -ENXIO; } - DBG("Found GPU: %u.%u.%u.%u", config.rev.core, config.rev.major, - config.rev.minor, config.rev.patchid); + DBG("Found GPU: %"ADRENO_CHIPID_FMT, ADRENO_CHIPID_ARGS(config.rev)); priv->is_a2xx = info->family < ADRENO_3XX; priv->has_cached_coherent = diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 75ff7fb46099..1a982a926f21 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -847,10 +847,9 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, if (IS_ERR_OR_NULL(state)) return; - drm_printf(p, "revision: %d (%d.%d.%d.%d)\n", - adreno_gpu->info->revn, adreno_gpu->rev.core, - adreno_gpu->rev.major, adreno_gpu->rev.minor, - adreno_gpu->rev.patchid); + drm_printf(p, "revision: %u (%"ADRENO_CHIPID_FMT")\n", + adreno_gpu->info->revn, + ADRENO_CHIPID_ARGS(adreno_gpu->rev)); /* * If this is state collected due to iova fault, so fault related info * @@ -921,10 +920,9 @@ void adreno_dump_info(struct msm_gpu *gpu) struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); int i; - printk("revision: %d (%d.%d.%d.%d)\n", - adreno_gpu->info->revn, adreno_gpu->rev.core, - adreno_gpu->rev.major, adreno_gpu->rev.minor, - adreno_gpu->rev.patchid); + printk("revision: %u (%"ADRENO_CHIPID_FMT")\n", + adreno_gpu->info->revn, + ADRENO_CHIPID_ARGS(adreno_gpu->rev)); for (i = 0; i < gpu->nr_rings; i++) { struct msm_ringbuffer *ring = gpu->rb[i]; @@ -1105,9 +1103,8 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, speedbin = 0xffff; adreno_gpu->speedbin = (uint16_t) (0xffff & speedbin); - gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%d.%d.%d.%d", - rev->core, rev->major, rev->minor, - rev->patchid); + gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%"ADRENO_CHIPID_FMT, + ADRENO_CHIPID_ARGS(config->rev)); if (!gpu_name) return -ENOMEM; diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h index 2fa14dcd4e40..73e7155f164c 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -66,6 +66,12 @@ struct adreno_rev { #define ADRENO_REV(core, major, minor, patchid) \ ((struct adreno_rev){ core, major, minor, patchid }) +/* Helper for formating the chip_id in the way that userspace tools like + * crashdec expect. + */ +#define ADRENO_CHIPID_FMT "u.%u.%u.%u" +#define ADRENO_CHIPID_ARGS(_r) (_r).core, (_r).major, (_r).minor, (_r).patchid + struct adreno_gpu_funcs { struct msm_gpu_funcs base; int (*get_timestamp)(struct msm_gpu *gpu, uint64_t *value);