On Thu, Jan 26, 2017 at 4:09 PM, Rob Herring <robh@xxxxxxxxxx> wrote: > On Thu, Jan 26, 2017 at 1:51 PM, Rob Clark <robdclark@xxxxxxxxx> wrote: >> On Thu, Jan 26, 2017 at 2:11 PM, Rob Herring <robh@xxxxxxxxxx> wrote: >>> On Tue, Jan 24, 2017 at 11:11 AM, Rob Clark <robdclark@xxxxxxxxx> wrote: >>>> So, cleaning up the GPU bindings is something that has been on my TODO >>>> list for a while, but always $bigger_fires. Existing bindings are a bit >>>> ugly, but served a purpose when too many of the other drivers the GPU >>>> depends on where still working their way upstream. But now enough of >>>> that is in place for a few devices, that we should start trying to get >>>> the GPU nodes into the upstream dts files. >>>> >>>> This drops the "qcom,chipid" property and parses the GPU revision out of >>>> the compat string. It does mean you need to list both "qcom,adreno" and >>>> the more specific string, for example "qcom,adreno-530.2". I'm not sure >>>> if that is "weird" or anyone has better ideas (hence this RFC). >>> >>> Is version and SoC close to a 1-1 mapping? If one version is in lots >>> of different SoCs, then you should have an SoC specific compatible. >> >> I'm not sure how common it is, but I'm pretty sure in the past I've >> seen same gpu (but maybe not same patchlevel). >> >> Also, there tend to be multiple revisions of the SoC which may or may >> not bump the gpu patchlevel. I think it is quite likely to be >> insanity to try and figure out gpu revision from SoC.. > > I'm not saying you should. My point is gpu revision alone may not be > enough. Things can get integrated or configured differently. You could > use an SoC based compatible, read the revision registers for the > revision, and override wrong revision registers based on SoC > compatible (assuming that is rare). Hmm, I'll probably defer to Jordan on this, since he has seen more combinations over the years. I think any integration differences would just amount to which clks/gdsc/etc are wired up. At least what I've seen in downstream kgsl driver, all the things that are conditional are purely keyed to the revision+patchlevel. As far as revision registers, IIRC back in the a3xx days they were not to be trusted. I'm not entirely sure when they became trustworthy. Last I checked, android kgsl driver was not using them. BR, -R >>>> It also drops "qcom,gpu-pwrlevels". Jordan tells me we can probably >>>> replace that w/ OPP bindings, which seems more sane. For now, the code >>>> just falls back to a super-slow safe clock until we get the OPP bindings >>>> sorted. >>>> >>>> In both cases, the code is still compatible with the old bindings, so >>>> dts files that exist on top of upstream will still work. But it is >>>> removed from the documentation, and for merging GPU nodes in upstream >>>> dts files, we should use the new bindings. >>> >>> Good. >> >> jfyi, I don't believe we have any adreno gpu nodes upstream.. > > Okay, missed that detail. In that case, one more thing below. > >> (but as long as it is easy, I like to keep backwards compat since >> otherwise I'll end up helping someone debug a bindings mismatch issue >> sooner or later.. and I'm lazy that way ;-)) > > Glad *someone* sees the value. > >>>> (I'll ofc split out the .dtsi changes, but figured for an RFC it was >>>> easier to include them in this patch for discussion.) >>>> >>>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx> >>>> --- >>>> .../devicetree/bindings/display/msm/gpu.txt | 24 +++---------- >>>> arch/arm64/boot/dts/qcom/msm8916.dtsi | 3 +- >>>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 8 +---- >>> >>> Bindings need to go to DT list. >>> >>>> drivers/gpu/drm/msm/adreno/adreno_device.c | 42 ++++++++++++++++++++-- >>>> drivers/gpu/drm/msm/msm_drv.c | 1 + >>>> 5 files changed, 47 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt >>>> index 67d0a58..760194d 100644 >>>> --- a/Documentation/devicetree/bindings/display/msm/gpu.txt >>>> +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt >>>> @@ -1,7 +1,11 @@ >>>> Qualcomm adreno/snapdragon GPU >>>> >>>> Required properties: >>>> -- compatible: "qcom,adreno-3xx" >>>> +- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" >>>> + for example: "qcom,adreno-306.0", "qcom,adreno" >>>> + Note that you need to list the less specific "qcom,adreno" (since this >>>> + is what the device is matched on), in addition to the more specific >>>> + with the chip-id. >>>> - reg: Physical base address and length of the controller's registers. >>>> - interrupts: The interrupt signal from the gpu. >>>> - clocks: device clocks >>>> @@ -10,14 +14,6 @@ Required properties: >>>> * "core_clk" >>>> * "iface_clk" >>>> * "mem_iface_clk" > > "_clk" here is redundant. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
