[1/2] panfrost: Make sure GPU is powered on when reading GPU_LATEST_FLUSH_ID

Message ID 20200611085900.49740-1-tomeu.vizoso@collabora.com
State New
Headers show
Series
  • [1/2] panfrost: Make sure GPU is powered on when reading GPU_LATEST_FLUSH_ID
Related show

Commit Message

Tomeu Vizoso June 11, 2020, 8:58 a.m.
Bifrost devices do support the flush reduction feature, so on first job
submit we were trying to read the register while still powered off.

If the GPU is powered off, the feature doesn't bring any benefit, so
don't try to read.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

---
 drivers/gpu/drm/panfrost/panfrost_gpu.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

-- 
2.21.0

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

Comments

Heiko Stübner June 11, 2020, 9:05 a.m. | #1
Am Donnerstag, 11. Juni 2020, 10:58:43 CEST schrieb Tomeu Vizoso:
> Bifrost devices do support the flush reduction feature, so on first job

> submit we were trying to read the register while still powered off.

> 

> If the GPU is powered off, the feature doesn't bring any benefit, so

> don't try to read.

> 

> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>


on a PX30-board (sibling of rk3326) with a 720x1280 panel and
glmark2 running all tests

Tested-by: Heiko Stuebner <heiko@sntech.de>



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alyssa Rosenzweig June 11, 2020, 2:11 p.m. | #2
Both patches are Reviewed-by: Alyssa Rosenzweig
<alyssa.rosenzweig@collabora.com>

On Thu, Jun 11, 2020 at 10:58:43AM +0200, Tomeu Vizoso wrote:
> Bifrost devices do support the flush reduction feature, so on first job

> submit we were trying to read the register while still powered off.

> 

> If the GPU is powered off, the feature doesn't bring any benefit, so

> don't try to read.

> 

> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

> ---

>  drivers/gpu/drm/panfrost/panfrost_gpu.c | 14 ++++++++++++--

>  1 file changed, 12 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c

> index f2c1ddc41a9b..e0f190e43813 100644

> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c

> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c

> @@ -10,6 +10,7 @@

>  #include <linux/io.h>

>  #include <linux/iopoll.h>

>  #include <linux/platform_device.h>

> +#include <linux/pm_runtime.h>

>  

>  #include "panfrost_device.h"

>  #include "panfrost_features.h"

> @@ -368,7 +369,16 @@ void panfrost_gpu_fini(struct panfrost_device *pfdev)

>  

>  u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev)

>  {

> -	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION))

> -		return gpu_read(pfdev, GPU_LATEST_FLUSH_ID);

> +	u32 flush_id;

> +

> +	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) {

> +		/* Flush reduction only makes sense when the GPU is kept powered on between jobs */

> +		if (pm_runtime_get_if_in_use(pfdev->dev)) {

> +			flush_id = gpu_read(pfdev, GPU_LATEST_FLUSH_ID);

> +			pm_runtime_put(pfdev->dev);

> +			return flush_id;

> +		}

> +	}

> +

>  	return 0;

>  }

> -- 

> 2.21.0

>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Steven Price June 19, 2020, 8:52 a.m. | #3
On 11/06/2020 09:58, Tomeu Vizoso wrote:
> Bifrost devices do support the flush reduction feature, so on first job

> submit we were trying to read the register while still powered off.

> 

> If the GPU is powered off, the feature doesn't bring any benefit, so

> don't try to read.

> 

> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>


Reviewed-by: Steven Price <steven.price@arm.com>


> ---

>   drivers/gpu/drm/panfrost/panfrost_gpu.c | 14 ++++++++++++--

>   1 file changed, 12 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c

> index f2c1ddc41a9b..e0f190e43813 100644

> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c

> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c

> @@ -10,6 +10,7 @@

>   #include <linux/io.h>

>   #include <linux/iopoll.h>

>   #include <linux/platform_device.h>

> +#include <linux/pm_runtime.h>

>   

>   #include "panfrost_device.h"

>   #include "panfrost_features.h"

> @@ -368,7 +369,16 @@ void panfrost_gpu_fini(struct panfrost_device *pfdev)

>   

>   u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev)

>   {

> -	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION))

> -		return gpu_read(pfdev, GPU_LATEST_FLUSH_ID);

> +	u32 flush_id;

> +

> +	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) {

> +		/* Flush reduction only makes sense when the GPU is kept powered on between jobs */

> +		if (pm_runtime_get_if_in_use(pfdev->dev)) {

> +			flush_id = gpu_read(pfdev, GPU_LATEST_FLUSH_ID);

> +			pm_runtime_put(pfdev->dev);

> +			return flush_id;

> +		}

> +	}

> +

>   	return 0;

>   }

> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Rob Herring Aug. 7, 2020, 7:47 p.m. | #4
On Thu, Jun 11, 2020 at 2:59 AM Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>

> Bifrost devices do support the flush reduction feature, so on first job

> submit we were trying to read the register while still powered off.

>

> If the GPU is powered off, the feature doesn't bring any benefit, so

> don't try to read.

>

> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

> ---

>  drivers/gpu/drm/panfrost/panfrost_gpu.c | 14 ++++++++++++--

>  1 file changed, 12 insertions(+), 2 deletions(-)


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

Patch

diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index f2c1ddc41a9b..e0f190e43813 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -10,6 +10,7 @@ 
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 
 #include "panfrost_device.h"
 #include "panfrost_features.h"
@@ -368,7 +369,16 @@  void panfrost_gpu_fini(struct panfrost_device *pfdev)
 
 u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev)
 {
-	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION))
-		return gpu_read(pfdev, GPU_LATEST_FLUSH_ID);
+	u32 flush_id;
+
+	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) {
+		/* Flush reduction only makes sense when the GPU is kept powered on between jobs */
+		if (pm_runtime_get_if_in_use(pfdev->dev)) {
+			flush_id = gpu_read(pfdev, GPU_LATEST_FLUSH_ID);
+			pm_runtime_put(pfdev->dev);
+			return flush_id;
+		}
+	}
+
 	return 0;
 }