On Thu, Jul 27, 2023 at 12:51 AM Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote: > > On 27.07.2023 00:53, Rob Clark wrote: > > On Wed, Jul 26, 2023 at 3:33 PM Dmitry Baryshkov > > <dmitry.baryshkov@xxxxxxxxxx> wrote: > >> > >> On Thu, 27 Jul 2023 at 01:04, Rob Clark <robdclark@xxxxxxxxx> wrote: > >>> > >>> On Wed, Jul 26, 2023 at 2:43 PM Dmitry Baryshkov > >>> <dmitry.baryshkov@xxxxxxxxxx> wrote: > >>>> > >>>> On 26/07/2023 23:11, Rob Clark wrote: > >>>>> On Wed, Jul 26, 2023 at 1:00 PM Dmitry Baryshkov > >>>>> <dmitry.baryshkov@xxxxxxxxxx> wrote: > >>>>>> > >>>>>> On Wed, 26 Jul 2023 at 21:28, Rob Clark <robdclark@xxxxxxxxx> wrote: > >>>>>>> > >>>>>>> On Thu, Jul 13, 2023 at 1:26 PM Akhil P Oommen <quic_akhilpo@xxxxxxxxxxx> 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@xxxxxxxxxxxx> > >>>>>>>>>> > >>>>>>>>>> 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 >
