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

[Rob Clark]

On Thu, Jul 27, 2023 at 12:51 AM Konrad Dybcio <konrad.dybcio at linaro.org> wrote:
>
> 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..

The newer rev of apq8064 would need its own dt w/ compatible override
in either case.  But if we continue with the current scheme parsing
the compatible, we don't need an extra gpu table entry for it.

BR,
-R

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