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

[Konrad Dybcio]

On 27.07.2023 00:53, Rob Clark wrote:
> On Wed, Jul 26, 2023 at 3:33 PM Dmitry Baryshkov
> <dmitry.baryshkov at linaro.org> wrote:
>>
>> On Thu, 27 Jul 2023 at 01:04, Rob Clark <robdclark at gmail.com> wrote:
>>>
>>> On Wed, Jul 26, 2023 at 2:43 PM Dmitry Baryshkov
>>> <dmitry.baryshkov at linaro.org> wrote:
>>>>
>>>> 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
>>>>
>>>
>>> My recollection was that nexus4 shipped with an early version of 8064
>>> which needed userspace workarounds that later 8064 did not.  Not sure
>>> if that is the only such example, but it is one that userspace needed
>>> to be aware of.
>>
>> Good question. I don't have nexus4, and both nexus7 and ifc6410 work fine.
>>
>> And this is a perfect use case for "qcom,apq8064-v1.1-adreno" compat string.
> 
> I no longer have a n4 that boots.. but if I did both it and the later
> ones should work properly if they expose the appropriate chip id
> 
> I do still prefer parsing the chip-id out of the compatible.  It
> avoids needing separate table entries just to have a different
> chip-id.  Maybe the scheme that is used elsewhere makes sense when it
> is only the kernel that needs to be aware of the device-id.  And maybe
> we could just done matching based on compat-id in userspace as well,
> but (a) msm and freedreno pre-date dt, and (b) that ship has already
> sailed.
I think a per-soc dt would be the better approach..

We could probably solve the revision issue with a socid readout of
the silicon revision and override based on that?

Konrad