[Freedreno] [PATCH 06/12] drm/msm/adreno: Allow SoC specific gpu device table entries

[Dmitry Baryshkov]

On Wed, 26 Jul 2023 at 21:28, Rob Clark <robdclark at gmail.com> wrote:
>
> On Thu, Jul 13, 2023 at 1:26 PM Akhil P Oommen <quic_akhilpo at quicinc.com> wrote:
> >
> > On Fri, Jul 07, 2023 at 05:34:04AM +0300, Dmitry Baryshkov wrote:
> > >
> > > On 07/07/2023 00:10, Rob Clark wrote:
> > > > From: Rob Clark <robdclark at 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.
> > >
> > > I have the feeling that we are trying to circumvent the way DT works. I'd
> > > suggest adding explicit SoC-compatible strings to Adreno bindings and then
> > > using of_device_id::data and then of_device_get_match_data().
> > >
> > Just thinking, then how about a unique compatible string which we match
> > to identify gpu->info and drop chip-id check completely here?
>
> Ok, I think we could do this, so something like:
>
>   compatible = "qcom,sm4350-adreno-619.0", qcom,adreno-619.0", "qcom,adreno"
>
> ?
>
> It looks like we don't have gpu dt bits upstream yet for either sm4350
> or sm6375, so I suppose we could get away with this change

I think we can even skip the 619.0 part in the SoC compat string.
So it will be:

compatible = "qcom,sm4350-adreno", qcom,adreno-619.0", "qcom,adreno";

In future we can drop the chipid part completely and handle that as a
part of SoC data:

compatible = "qcom,sm4350-adreno", "qcom,adreno";

With the driver knowing that sm4350-adreno means ADRENO_ID(6,1,9,0)

>
> BR,
> -R
>
> > -Akhil
> >
> > > >
> > > > Signed-off-by: Rob Clark <robdclark at 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",
> > > > +           },
> > > > +           .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];
> > >
> > > --
> > > With best wishes
> > > Dmitry
> > >



-- 
With best wishes
Dmitry