| Message ID | 20230727212208.102501-1-robdclark@gmail.com |
|---|---|
| Headers | show |
| Series | drm/msm/adreno: Move away from legacy revision matching | expand |
On 27/07/2023 23:20, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > Upcoming GPUs use an opaque chip-id for identifying the GPU. Examples? Anyway, I think we should insist here of using something human-readable, even if Qualcomm/Adreno internally use some weird numbers. > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > Documentation/devicetree/bindings/display/msm/gpu.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml > index 58ca8912a8c3..56b9b247e8c2 100644 > --- a/Documentation/devicetree/bindings/display/msm/gpu.yaml > +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml > @@ -13,6 +13,12 @@ maintainers: > properties: > compatible: > oneOf: > + - description: | > + The driver is parsing the compat string for Adreno to > + figure out the chip-id. > + items: > + - pattern: '^qcom,adreno-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]$' {8} should work? Best regards, Krzysztof
On Fri, Jul 28, 2023 at 12:27 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 27/07/2023 23:20, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > Upcoming GPUs use an opaque chip-id for identifying the GPU. > > Examples? We'll know when we bring up the hw. But the main point is that we shouldn't expect, for example, the high 8 bits to tell us the generation, any more than we could if it was a pci id. We may not end up needing to use this new binding much, I _think_ we should be able to read it from the fw in most cases, at least for android devices. I'm unsure at this point about windows/chromebooks. > Anyway, I think we should insist here of using something human-readable, > even if Qualcomm/Adreno internally use some weird numbers. I mean qcom,sc8280cx-adreno is human readable but not really very informative. Encoding the chip-id is just a way to avoid the qcom,chipid field in the bindings, which qcom used downstream. The new pattern accomplishes the same thing as the existing one, but without trying to imply some meaning that becomes increasingly non-existent as qc moves to decouple the id from marketing names. > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > Documentation/devicetree/bindings/display/msm/gpu.yaml | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml > > index 58ca8912a8c3..56b9b247e8c2 100644 > > --- a/Documentation/devicetree/bindings/display/msm/gpu.yaml > > +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml > > @@ -13,6 +13,12 @@ maintainers: > > properties: > > compatible: > > oneOf: > > + - description: | > > + The driver is parsing the compat string for Adreno to > > + figure out the chip-id. > > + items: > > + - pattern: '^qcom,adreno-[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]$' > > {8} should work? > so '^qcom,adreno-[0-9a-f]{8}$' BR, -R > > > Best regards, > Krzysztof >
On Thu, Jul 27, 2023 at 3:12 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Fri, 28 Jul 2023 at 00:23, Rob Clark <robdclark@gmail.com> wrote: > > > > From: Rob Clark <robdclark@chromium.org> > > > > There are cases where there are differences due to SoC integration. > > Such as cache-coherency support, and (in the next patch) e-fuse to > > speedbin mappings. > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > drivers/gpu/drm/msm/adreno/adreno_device.c | 34 +++++++++++++++++++--- > > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 + > > 2 files changed, 31 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c > > index 3c531da417b9..e62bc895a31f 100644 > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c > > @@ -258,6 +258,32 @@ static const struct adreno_info gpulist[] = { > > .inactive_period = DRM_MSM_INACTIVE_PERIOD, > > .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT, > > .init = a6xx_gpu_init, > > + }, { > > + .machine = "qcom,sm4350", > > + .rev = ADRENO_REV(6, 1, 9, ANY_ID), > > + .revn = 619, > > + .fw = { > > + [ADRENO_FW_SQE] = "a630_sqe.fw", > > + [ADRENO_FW_GMU] = "a619_gmu.bin", > > If those are GMU-less platforms, do we need the ADRENO_FW_GMU firmware? ahh, good point, fixed that locally > Other than that: > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> thanks BR, -R > > + }, > > + .gmem = SZ_512K, > > + .inactive_period = DRM_MSM_INACTIVE_PERIOD, > > + .init = a6xx_gpu_init, > > + .zapfw = "a615_zap.mdt", > > + .hwcg = a615_hwcg, > > + }, { > > + .machine = "qcom,sm6375", > > + .rev = ADRENO_REV(6, 1, 9, ANY_ID), > > + .revn = 619, > > + .fw = { > > + [ADRENO_FW_SQE] = "a630_sqe.fw", > > + [ADRENO_FW_GMU] = "a619_gmu.bin", > > + }, > > + .gmem = SZ_512K, > > + .inactive_period = DRM_MSM_INACTIVE_PERIOD, > > + .init = a6xx_gpu_init, > > + .zapfw = "a615_zap.mdt", > > + .hwcg = a615_hwcg, > > }, { > > .rev = ADRENO_REV(6, 1, 9, ANY_ID), > > .revn = 619, > > @@ -409,6 +435,8 @@ const struct adreno_info *adreno_info(struct adreno_rev rev) > > /* identify gpu: */ > > for (i = 0; i < ARRAY_SIZE(gpulist); i++) { > > const struct adreno_info *info = &gpulist[i]; > > + if (info->machine && !of_machine_is_compatible(info->machine)) > > + continue; > > if (adreno_cmp_rev(info->rev, rev)) > > return info; > > } > > @@ -563,6 +591,8 @@ static int adreno_bind(struct device *dev, struct device *master, void *data) > > config.rev.minor, config.rev.patchid); > > > > priv->is_a2xx = config.rev.core == 2; > > + priv->has_cached_coherent = > > + !!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT); > > > > gpu = info->init(drm); > > if (IS_ERR(gpu)) { > > @@ -574,10 +604,6 @@ static int adreno_bind(struct device *dev, struct device *master, void *data) > > if (ret) > > return ret; > > > > - priv->has_cached_coherent = > > - !!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT) && > > - !adreno_has_gmu_wrapper(to_adreno_gpu(gpu)); > > - > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > index e08d41337169..d5335b99c64c 100644 > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > @@ -61,6 +61,7 @@ extern const struct adreno_reglist a612_hwcg[], a615_hwcg[], a630_hwcg[], a640_h > > extern const struct adreno_reglist a660_hwcg[], a690_hwcg[]; > > > > struct adreno_info { > > + const char *machine; > > struct adreno_rev rev; > > uint32_t revn; > > const char *fw[ADRENO_FW_MAX]; > > -- > > 2.41.0 > > > > > -- > With best wishes > Dmitry
From: Rob Clark <robdclark@chromium.org> Downstream seems to be moving to using the chip_id as simply an opaque identifier, and if we want to avoid headaches with userspace mesa supporting both kgsl and upstream, we should move away from the assumption that certain bits in the chip_id have a specific meaning. v2 adds a patch to move adreno_info to adreno_platform_config rather than needing to look it up in multiple places. Rob Clark (13): drm/msm/adreno: Remove GPU name drm/msm/adreno: Remove redundant gmem size param drm/msm/adreno: Remove redundant revn param drm/msm/adreno: Use quirk identify hw_apriv drm/msm/adreno: Use quirk to identify cached-coherent support drm/msm/adreno: Allow SoC specific gpu device table entries drm/msm/adreno: Move speedbin mapping to device table drm/msm/adreno: Bring the a630 family together drm/msm/adreno: Add adreno family drm/msm/adreno: Add helper for formating chip-id drm/msm/adreno: Move adreno info to config dt-bindings: drm/msm/gpu: Extend bindings for chip-id drm/msm/adreno: Switch to chip-id for identifying GPU .../devicetree/bindings/display/msm/gpu.yaml | 6 + drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 16 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 189 ++--------- drivers/gpu/drm/msm/adreno/adreno_device.c | 294 ++++++++++++------ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 53 ++-- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 154 ++++++--- 10 files changed, 365 insertions(+), 357 deletions(-)