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

[Rob Herring]

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 <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.

With that,

Acked-by: Rob Herring <robh at 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
>