[Freedreno] [PATCH 2/4] drm/msm: drop qcom,chipid

[Rob Clark]

On Wed, Feb 1, 2017 at 12:09 PM, Rob Herring <robh at kernel.org> wrote:
> On Mon, Jan 30, 2017 at 11:49:19AM -0500, Rob Clark wrote:
>> The original way we determined the gpu version was based on downstream
>> bindings from android kernel.  A cleaner way is to get the version from
>> the compatible string.
>>
>> Note that no upstream dtb uses these bindings.  But the code still
>> supports falling back to the legacy bindings (with a warning), so that
>> we are still compatible with the gpu dt node from android device
>> kernels.
>
> Fine for one driver/binding, but we wouldn't really want to do carry
> downstream compatibility for everything.

yeah, I'm not necessarily signing up to support the downstream
bindings forever.. but for now the benefit outweighs the cost.  We'll
see how that goes when we have OPP and bus scaling in the mix.

>>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>  .../devicetree/bindings/display/msm/gpu.txt        | 11 +++---
>>  drivers/gpu/drm/msm/adreno/adreno_device.c         | 43 +++++++++++++++++++++-
>>  drivers/gpu/drm/msm/msm_drv.c                      |  1 +
>>  3 files changed, 49 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
>> index 747b984..7ac3052 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,8 +14,6 @@ Required properties:
>>    * "core_clk"
>>    * "iface_clk"
>>    * "mem_iface_clk"
>> -- qcom,chipid: gpu chip-id.  Note this may become optional for future
>> -  devices if we can reliably read the chipid from hw
>>
>>  Example:
>>
>> @@ -19,7 +21,7 @@ Example:
>>       ...
>>
>>       gpu: qcom,kgsl-3d0 at 4300000 {
>> -             compatible = "qcom,adreno-3xx";
>> +             compatible = "qcom,adreno-320.2", "qcom,adreno";
>>               reg = <0x04300000 0x20000>;
>>               reg-names = "kgsl_3d0_reg_memory";
>>               interrupts = <GIC_SPI 80 0>;
>> @@ -32,6 +34,5 @@ Example:
>>                   <&mmcc GFX3D_CLK>,
>>                   <&mmcc GFX3D_AHB_CLK>,
>>                   <&mmcc MMSS_IMEM_AHB_CLK>;
>> -             qcom,chipid = <0x03020100>;
>>       };
>>  };
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 7b9181b2..fdaef67 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -189,6 +189,46 @@ static const struct {
>>       { "qcom,gpu-quirk-fault-detect-mask", ADRENO_QUIRK_FAULT_DETECT_MASK },
>>  };
>>
>> +static int find_chipid(struct device *dev, u32 *chipid)
>> +{
>> +     struct device_node *node = dev->of_node;
>> +     struct property *prop;
>> +     const char *compat;
>> +     int ret;
>> +
>> +     /* first search the compat strings for qcom,adreno-XYZ.W: */
>> +     prop = of_find_property(node, "compatible", NULL);
>> +     for (compat = of_prop_next_string(prop, NULL); compat;
>> +          compat = of_prop_next_string(prop, compat)) {
>
> of_property_for_each_string
>
> However, you specify in the binding it should be the 1st string, so you
> really don't need a loop here and could use
> of_property_read_string_index.

I suppose that I don't need to have that restriction about being 1st
string.. otoh, from looking at other dt nodes, that the more specific
is supposed to come first, so I guess it doesn't hurt to enforce it..

> With that,
>
> Acked-by: Rob Herring <robh at kernel.org>


Thx

BR,
-R