diff mbox

[1/4] drm/msm: remove qcom,gpu-pwrlevels bindings

Message ID 20170130164921.20744-2-robdclark@gmail.com (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Rob Clark Jan. 30, 2017, 4:49 p.m. UTC
The plan is to use the OPP bindings.  For now, remove the documentation
for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
clock if the node is not present.

Note that no upstream dtb use this node.  For now we keep compatibility
with this node to avoid breaking compatibility with downstream android
dt files.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 Documentation/devicetree/bindings/display/msm/gpu.txt | 15 ---------------
 drivers/gpu/drm/msm/adreno/adreno_device.c            |  6 ++++--
 2 files changed, 4 insertions(+), 17 deletions(-)

Comments

Eric Anholt Jan. 30, 2017, 6:21 p.m. UTC | #1
Rob Clark <robdclark@gmail.com> writes:

> The plan is to use the OPP bindings.  For now, remove the documentation
> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
> clock if the node is not present.
>
> Note that no upstream dtb use this node.  For now we keep compatibility
> with this node to avoid breaking compatibility with downstream android
> dt files.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Will we need the bus frequency knobs that I see in the old pwrlevels
node?  If so, what would the plan be for doing that within OPP?
Rob Clark Jan. 30, 2017, 6:35 p.m. UTC | #2
On Mon, Jan 30, 2017 at 1:21 PM, Eric Anholt <eric@anholt.net> wrote:
> Rob Clark <robdclark@gmail.com> writes:
>
>> The plan is to use the OPP bindings.  For now, remove the documentation
>> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
>> clock if the node is not present.
>>
>> Note that no upstream dtb use this node.  For now we keep compatibility
>> with this node to avoid breaking compatibility with downstream android
>> dt files.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>
> Will we need the bus frequency knobs that I see in the old pwrlevels
> node?  If so, what would the plan be for doing that within OPP?

So, that I think is one of the open questions.  Jordan knows this
stuff a lot better than I, but my understanding is that bus and clk
scale *basically* independently, except that a given gpu clk we want a
different minimum bus clk.

(I'm not sure if that is a functional requirement, or just what qcom
arrived at after performance tuning..)

There is some work ongoing to get some sort of upstream bus scaling
scaling, although I'm not really sure yet what the bindings for this
would look like.

So basically short answer is "I don't know.. there are too many open
questions".  Maybe in the end we re-introduce qcom,gpu-pwrlevels.  But
I think for now the approach of not documenting it and have safe/slow
clk fallback in the driver is a reasonable way to move forward with
getting some basic gpu nodes into upstream dts files.

BR,
-R
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Anholt Jan. 30, 2017, 7:56 p.m. UTC | #3
Rob Clark <robdclark@gmail.com> writes:

> On Mon, Jan 30, 2017 at 1:21 PM, Eric Anholt <eric@anholt.net> wrote:
>> Rob Clark <robdclark@gmail.com> writes:
>>
>>> The plan is to use the OPP bindings.  For now, remove the documentation
>>> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
>>> clock if the node is not present.
>>>
>>> Note that no upstream dtb use this node.  For now we keep compatibility
>>> with this node to avoid breaking compatibility with downstream android
>>> dt files.
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>
>> Will we need the bus frequency knobs that I see in the old pwrlevels
>> node?  If so, what would the plan be for doing that within OPP?
>
> So, that I think is one of the open questions.  Jordan knows this
> stuff a lot better than I, but my understanding is that bus and clk
> scale *basically* independently, except that a given gpu clk we want a
> different minimum bus clk.
>
> (I'm not sure if that is a functional requirement, or just what qcom
> arrived at after performance tuning..)
>
> There is some work ongoing to get some sort of upstream bus scaling
> scaling, although I'm not really sure yet what the bindings for this
> would look like.
>
> So basically short answer is "I don't know.. there are too many open
> questions".  Maybe in the end we re-introduce qcom,gpu-pwrlevels.  But
> I think for now the approach of not documenting it and have safe/slow
> clk fallback in the driver is a reasonable way to move forward with
> getting some basic gpu nodes into upstream dts files.

Yeah, letting the upstream DT bind without the custom OPP stuff for now
seems like progress.  If we find that the safe fast freq is too high
then we can drop it down later, and that would just put more pressure on
getting the OPP work finished.

Reviewed-by: Eric Anholt <eric@anholt.net>
Rob Herring (Arm) Feb. 1, 2017, 5:01 p.m. UTC | #4
On Mon, Jan 30, 2017 at 11:49:18AM -0500, Rob Clark wrote:
> The plan is to use the OPP bindings.  For now, remove the documentation
> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
> clock if the node is not present.
> 
> Note that no upstream dtb use this node.  For now we keep compatibility
> with this node to avoid breaking compatibility with downstream android
> dt files.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  Documentation/devicetree/bindings/display/msm/gpu.txt | 15 ---------------
>  drivers/gpu/drm/msm/adreno/adreno_device.c            |  6 ++++--
>  2 files changed, 4 insertions(+), 17 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jordan Crouse Feb. 1, 2017, 11:26 p.m. UTC | #5
On Mon, Jan 30, 2017 at 01:35:47PM -0500, Rob Clark wrote:
> On Mon, Jan 30, 2017 at 1:21 PM, Eric Anholt <eric@anholt.net> wrote:
> > Rob Clark <robdclark@gmail.com> writes:
> >
> >> The plan is to use the OPP bindings.  For now, remove the documentation
> >> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
> >> clock if the node is not present.
> >>
> >> Note that no upstream dtb use this node.  For now we keep compatibility
> >> with this node to avoid breaking compatibility with downstream android
> >> dt files.
> >>
> >> Signed-off-by: Rob Clark <robdclark@gmail.com>
> >
> > Will we need the bus frequency knobs that I see in the old pwrlevels
> > node?  If so, what would the plan be for doing that within OPP?
> 
> So, that I think is one of the open questions.  Jordan knows this
> stuff a lot better than I, but my understanding is that bus and clk
> scale *basically* independently, except that a given gpu clk we want a
> different minimum bus clk.
> 
> (I'm not sure if that is a functional requirement, or just what qcom
> arrived at after performance tuning..)
> 
> There is some work ongoing to get some sort of upstream bus scaling
> scaling, although I'm not really sure yet what the bindings for this
> would look like.
> 
> So basically short answer is "I don't know.. there are too many open
> questions".  Maybe in the end we re-introduce qcom,gpu-pwrlevels.  But
> I think for now the approach of not documenting it and have safe/slow
> clk fallback in the driver is a reasonable way to move forward with
> getting some basic gpu nodes into upstream dts files.

Rob has it right.  On a fully optimized platform the bus does scale
independently from the GPU but when we switch GPU levels up we need to
immediately kick the bus to a new baseline level otherwise it underruns.

Eventually somebody will have to figure out how to make OPP work with both
device and bus frequency. Perhaps that will happen by the time useful bus
scaling hits the kernel, otherwise we will have to suffer along with two tables
and a closer relationship between the GPU driver and the devfreq governor than
any of us are comfortable with. Luckily for this discussion that someday is in
the future and we can focus on doing the frqeuency right.

Jordan
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 67d0a58..747b984 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -12,12 +12,6 @@  Required properties:
   * "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
-- qcom,gpu-pwrlevels: list of operating points
-  - compatible: "qcom,gpu-pwrlevels"
-  - for each qcom,gpu-pwrlevel:
-    - qcom,gpu-freq: requested gpu clock speed
-    - NOTE: downstream android driver defines additional parameters to
-      configure memory bandwidth scaling per OPP.
 
 Example:
 
@@ -39,14 +33,5 @@  Example:
 		    <&mmcc GFX3D_AHB_CLK>,
 		    <&mmcc MMSS_IMEM_AHB_CLK>;
 		qcom,chipid = <0x03020100>;
-		qcom,gpu-pwrlevels {
-			compatible = "qcom,gpu-pwrlevels";
-			qcom,gpu-pwrlevel@0 {
-				qcom,gpu-freq = <450000000>;
-			};
-			qcom,gpu-pwrlevel@1 {
-				qcom,gpu-freq = <27000000>;
-			};
-		};
 	};
 };
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index e130b5e..7b9181b2 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -224,8 +224,10 @@  static int adreno_bind(struct device *dev, struct device *master, void *data)
 	}
 
 	if (!config.fast_rate) {
-		dev_err(dev, "could not find clk rates\n");
-		return -ENXIO;
+		dev_warn(dev, "could not find clk rates\n");
+		/* This is a safe low speed for all devices: */
+		config.fast_rate = 200000000;
+		config.slow_rate = 27000000;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(quirks); i++)