[PATCH v2 1/2] drm/msm: Fix A6XX support for opp-level

[PATCH v2 1/2] drm/msm: Fix A6XX support for opp-level

[Douglas Anderson]

The bindings for Qualcomm opp levels changed after being Acked but
before landing.  Thus the code in the GPU driver that was relying on
the old bindings is now broken.

Let's change the code to match the new bindings by adjusting the old
string 'qcom,level' to the new string 'opp-level'.  See the patch
("dt-bindings: opp: Introduce opp-level bindings").

NOTE: we will do additional cleanup to totally remove the string from
the code and use the new dev_pm_opp_get_level() but we'll do it in a
future patch.  This will facilitate getting the important code fix in
sooner without having to deal with cross-maintainer dependencies.

This patch needs to land before the patch ("arm64: dts: sdm845: Add
gpu and gmu device nodes") since if a tree contains the device tree
patch but not this one you'll get a crash at bootup.

Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v2:
- Split into two patches to facilitate landing.

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 5beb83d1cf87..ce1b3cc4bf6d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -944,7 +944,7 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
 	np = dev_pm_opp_get_of_node(opp);
 
 	if (np) {
-		of_property_read_u32(np, "qcom,level", &val);
+		of_property_read_u32(np, "opp-level", &val);
 		of_node_put(np);
 	}
 
-- 
2.20.1.97.g81188d93c3-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/2] drm/msm: Cleanup A6XX opp-level reading

[Douglas Anderson]

The patch ("OPP: Add support for parsing the 'opp-level' property")
adds an API enabling a cleaner way to read the opp-level.  Let's use
the new API.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>
---
Obviously this can't land until we have a tree that contains the patch
adding the API.  I believe that means we'll want to target this patch
for 5.2.  Luckily it's fine to wait since this patch has no functional
changes--it's all cleanup.

Changes in v2:
- Split into two patches to facilitate landing.

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index ce1b3cc4bf6d..900f18dc1577 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -928,25 +928,20 @@ static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
 }
 
 /* Return the 'arc-level' for the given frequency */
-static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
+static unsigned int a6xx_gmu_get_arc_level(struct device *dev,
+					   unsigned long freq)
 {
 	struct dev_pm_opp *opp;
-	struct device_node *np;
-	u32 val = 0;
+	unsigned int val;
 
 	if (!freq)
 		return 0;
 
-	opp  = dev_pm_opp_find_freq_exact(dev, freq, true);
+	opp = dev_pm_opp_find_freq_exact(dev, freq, true);
 	if (IS_ERR(opp))
 		return 0;
 
-	np = dev_pm_opp_get_of_node(opp);
-
-	if (np) {
-		of_property_read_u32(np, "opp-level", &val);
-		of_node_put(np);
-	}
+	val = dev_pm_opp_get_level(opp);
 
 	dev_pm_opp_put(opp);
 
@@ -982,7 +977,7 @@ static int a6xx_gmu_rpmh_arc_votes_init(struct device *dev, u32 *votes,
 	/* Construct a vote for each frequency */
 	for (i = 0; i < freqs_count; i++) {
 		u8 pindex = 0, sindex = 0;
-		u32 level = a6xx_gmu_get_arc_level(dev, freqs[i]);
+		unsigned int level = a6xx_gmu_get_arc_level(dev, freqs[i]);
 
 		/* Get the primary index that matches the arc level */
 		for (j = 0; j < pri_count; j++) {
-- 
2.20.1.97.g81188d93c3-goog

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 2/2] drm/msm: Cleanup A6XX opp-level reading

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

Hi Douglas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v5.0-rc2]
[also build test ERROR on next-20190116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Douglas-Anderson/drm-msm-Fix-A6XX-support-for-opp-level/20190118-042538
config: arm-imx_v6_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/msm/adreno/a6xx_gmu.c: In function 'a6xx_gmu_get_arc_level':
>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c:944:8: error: implicit declaration of function 'dev_pm_opp_get_level'; did you mean 'dev_pm_opp_get_freq'? [-Werror=implicit-function-declaration]
     val = dev_pm_opp_get_level(opp);
           ^~~~~~~~~~~~~~~~~~~~
           dev_pm_opp_get_freq
   cc1: some warnings being treated as errors

vim +944 drivers/gpu/drm/msm/adreno/a6xx_gmu.c

   929	
   930	/* Return the 'arc-level' for the given frequency */
   931	static unsigned int a6xx_gmu_get_arc_level(struct device *dev,
   932						   unsigned long freq)
   933	{
   934		struct dev_pm_opp *opp;
   935		unsigned int val;
   936	
   937		if (!freq)
   938			return 0;
   939	
   940		opp = dev_pm_opp_find_freq_exact(dev, freq, true);
   941		if (IS_ERR(opp))
   942			return 0;
   943	
 > 944		val = dev_pm_opp_get_level(opp);
   945	
   946		dev_pm_opp_put(opp);
   947	
   948		return val;
   949	}
   950	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34355 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 2/2] drm/msm: Cleanup A6XX opp-level reading

[Doug Anderson]

Hi,

On Wed, Jan 16, 2019 at 10:46 AM Douglas Anderson <dianders@chromium.org> wrote:
>
> The patch ("OPP: Add support for parsing the 'opp-level' property")
> adds an API enabling a cleaner way to read the opp-level.  Let's use
> the new API.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
> Obviously this can't land until we have a tree that contains the patch
> adding the API.  I believe that means we'll want to target this patch
> for 5.2.  Luckily it's fine to wait since this patch has no functional
> changes--it's all cleanup.
>
> Changes in v2:
> - Split into two patches to facilitate landing.
>
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)

FWIW I think this patch is ready to land any time.  That commit
5b93ac542301 ("OPP: Add support for parsing the 'opp-level' property")
is now in linux/master.


-Doug
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/2] drm/msm: Fix A6XX support for opp-level

[Jordan Crouse]

On Wed, Jan 16, 2019 at 10:46:21AM -0800, Douglas Anderson wrote:
> The bindings for Qualcomm opp levels changed after being Acked but
> before landing.  Thus the code in the GPU driver that was relying on
> the old bindings is now broken.
> 
> Let's change the code to match the new bindings by adjusting the old
> string 'qcom,level' to the new string 'opp-level'.  See the patch
> ("dt-bindings: opp: Introduce opp-level bindings").
> 
> NOTE: we will do additional cleanup to totally remove the string from
> the code and use the new dev_pm_opp_get_level() but we'll do it in a
> future patch.  This will facilitate getting the important code fix in
> sooner without having to deal with cross-maintainer dependencies.
> 
> This patch needs to land before the patch ("arm64: dts: sdm845: Add
> gpu and gmu device nodes") since if a tree contains the device tree
> patch but not this one you'll get a crash at bootup.
> 
> Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

I agree that splitting these out make sense for the workflow.

Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
> 
> Changes in v2:
> - Split into two patches to facilitate landing.
> 
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 5beb83d1cf87..ce1b3cc4bf6d 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -944,7 +944,7 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
>  	np = dev_pm_opp_get_of_node(opp);
>  
>  	if (np) {
> -		of_property_read_u32(np, "qcom,level", &val);
> +		of_property_read_u32(np, "opp-level", &val);
>  		of_node_put(np);
>  	}
>  
> -- 
> 2.20.1.97.g81188d93c3-goog
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/2] drm/msm: Fix A6XX support for opp-level

[Rob Clark]

s

On Wed, Jan 16, 2019 at 1:46 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> The bindings for Qualcomm opp levels changed after being Acked but
> before landing.  Thus the code in the GPU driver that was relying on
> the old bindings is now broken.
>
> Let's change the code to match the new bindings by adjusting the old
> string 'qcom,level' to the new string 'opp-level'.  See the patch
> ("dt-bindings: opp: Introduce opp-level bindings").
>
> NOTE: we will do additional cleanup to totally remove the string from
> the code and use the new dev_pm_opp_get_level() but we'll do it in a
> future patch.  This will facilitate getting the important code fix in
> sooner without having to deal with cross-maintainer dependencies.
>
> This patch needs to land before the patch ("arm64: dts: sdm845: Add
> gpu and gmu device nodes") since if a tree contains the device tree
> patch but not this one you'll get a crash at bootup.
>
> Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

thanks, I've pulled the first into msm-next and I'll grab the 2nd once
dev_pm_opp_get_level() lands

BR,
-R

> ---
>
> Changes in v2:
> - Split into two patches to facilitate landing.
>
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 5beb83d1cf87..ce1b3cc4bf6d 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -944,7 +944,7 @@ static u32 a6xx_gmu_get_arc_level(struct device *dev, unsigned long freq)
>         np = dev_pm_opp_get_of_node(opp);
>
>         if (np) {
> -               of_property_read_u32(np, "qcom,level", &val);
> +               of_property_read_u32(np, "opp-level", &val);
>                 of_node_put(np);
>         }
>
> --
> 2.20.1.97.g81188d93c3-goog
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno