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.

> 
> Signed-off-by: Rob Clark <robdcl...@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@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.

With that,

Acked-by: Rob Herring <r...@kernel.org>


> +             unsigned rev, patch;
> +
> +             if (sscanf(compat, "qcom,adreno-%u.%u", &rev, &patch) != 2)
> +                     continue;
> +
> +             *chipid = 0;
> +             *chipid |= (rev / 100) << 24;  /* core */
> +             rev %= 100;
> +             *chipid |= (rev / 10) << 16;   /* major */
> +             rev %= 10;
> +             *chipid |= rev << 8;           /* minor */
> +             *chipid |= patch;
> +
> +             return 0;
> +     }
> +
> +     /* and if that fails, fall back to legacy "qcom,chipid" property: */
> +     ret = of_property_read_u32(node, "qcom,chipid", chipid);
> +     if (ret)
> +             return ret;
> +
> +     dev_warn(dev, "Using legacy qcom,chipid binding!\n");
> +     dev_warn(dev, "Use compatible qcom,adreno-%u%u%u.%u instead.\n",
> +                     (*chipid >> 24) & 0xff, (*chipid >> 16) & 0xff,
> +                     (*chipid >> 8) & 0xff, *chipid & 0xff);
> +
> +     return 0;
> +}
> +
>  static int adreno_bind(struct device *dev, struct device *master, void *data)
>  {
>       static struct adreno_platform_config config = {};
> @@ -196,7 +236,7 @@ static int adreno_bind(struct device *dev, struct device 
> *master, void *data)
>       u32 val;
>       int ret, i;
>  
> -     ret = of_property_read_u32(node, "qcom,chipid", &val);
> +     ret = find_chipid(dev, &val);
>       if (ret) {
>               dev_err(dev, "could not find chipid: %d\n", ret);
>               return ret;
> @@ -265,6 +305,7 @@ static int adreno_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id dt_match[] = {
> +     { .compatible = "qcom,adreno" },
>       { .compatible = "qcom,adreno-3xx" },
>       /* for backwards compat w/ downstream kgsl DT files: */
>       { .compatible = "qcom,kgsl-3d0" },
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index e29bb66..6b85c41 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -985,6 +985,7 @@ static int add_display_components(struct device *dev,
>   * as components.
>   */
>  static const struct of_device_id msm_gpu_match[] = {
> +     { .compatible = "qcom,adreno" },
>       { .compatible = "qcom,adreno-3xx" },
>       { .compatible = "qcom,kgsl-3d0" },
>       { },
> -- 
> 2.9.3
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to