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

Dmitry Baryshkov dmitry.baryshkov at linaro.org
Wed Jul 26 21:43:17 UTC 2023


On 26/07/2023 23:11, Rob Clark wrote:
> On Wed, Jul 26, 2023 at 1:00 PM Dmitry Baryshkov
> <dmitry.baryshkov at linaro.org> wrote:
>>
>> 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)
>>
> 
> I don't think we can do that, there are cases where the same SoC had
> multiple revisions of adreno.

Is that the case for the production versions of the SoC? In other 
subsystems what we usually do is that we add support only for the latest 
SoC revision (which would probably mean the latest GPU patch revision). 
Previous GPU revisions can be added in the following way (pure example):

qcom,sm4350-adreno -> 6,1,9,1 // assuming v2.0 or v1.1 is the commercial 
sample
qcom,sm4350-v1-adreno -> 6,1,9,0

>  We could possibly do that with future
> things where we can read the chip-id from fw.
> 
> What we _could_ do at the expense of making the compatible parsing a
> tiny bit more complex is something like:
> 
>     compatible = "qcom,sm4350-adreno-619.0", "qcom,adreno"
> 
> BR,
> -R
> 
>>>
>>> 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

-- 
With best wishes
Dmitry



More information about the dri-devel mailing list