diff mbox series

[3/4] drm/panfrost: Add permon acquire/release helpers

Message ID 20210527203804.12914-4-alyssa.rosenzweig@collabora.com (mailing list archive)
State New, archived
Headers show
Series drm/panfrost: Plumb cycle counters to userspace | expand

Commit Message

Alyssa Rosenzweig May 27, 2021, 8:38 p.m. UTC 
From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>

Wrap the underlying CYCLE_COUNT_START/STOP commands in a safe interface
that ensures the commands are only issued where required by guarding
behind an atomic counter. In particular, we need to be careful about
races between multiple in-flight jobs, where only some require cycle
counts.

Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
---
 drivers/gpu/drm/panfrost/panfrost_device.h |  3 +++
 drivers/gpu/drm/panfrost/panfrost_gpu.c    | 20 ++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_gpu.h    |  3 +++
 3 files changed, 26 insertions(+)

Comments

Steven Price June 2, 2021, 11:50 a.m. UTC | #1 
On 27/05/2021 21:38, alyssa.rosenzweig@collabora.com wrote:
> From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> 
> Wrap the underlying CYCLE_COUNT_START/STOP commands in a safe interface
> that ensures the commands are only issued where required by guarding
> behind an atomic counter. In particular, we need to be careful about
> races between multiple in-flight jobs, where only some require cycle
> counts.
> 
> Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.h |  3 +++
>  drivers/gpu/drm/panfrost/panfrost_gpu.c    | 20 ++++++++++++++++++++
>  drivers/gpu/drm/panfrost/panfrost_gpu.h    |  3 +++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index 597cf1459..8a89aa274 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -117,6 +117,9 @@ struct panfrost_device {
>  	struct shrinker shrinker;
>  
>  	struct panfrost_devfreq pfdevfreq;
> +
> +	/* Number of active jobs requiring performance monitoring */
> +	atomic_t permon_pending;

If the JD_REQ name is going to be changed then this should be renamed to
be consistent.

>  };
>  
>  struct panfrost_mmu {
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> index 2aae636f1..acacceb15 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> @@ -399,3 +399,23 @@ u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev)
>  
>  	return 0;
>  }
> +
> +void panfrost_acquire_permon(struct panfrost_device *pfdev)
> +{
> +	/* If another in-flight job enabled permon, we don't have to */
> +	if (atomic_inc_return(&pfdev->permon_pending) > 1)
> +		return;
> +
> +	/* Otherwise, we're the first user */
> +	gpu_write(pfdev, GPU_CMD, GPU_CMD_CYCLE_COUNT_START);
> +}

NIT: The check of atomic_inc_return() is usually of the style '== 1', i.e.:

	/* Only enable permon, if we're the first user */
	if (atomic_inc_return(&pfdev->permon_pending) == 1)
		gpu_write(pfdev, GPU_CMD, GPU_CMD_CYCLE_COUNT_START);

> +
> +void panfrost_release_permon(struct panfrost_device *pfdev)
> +{
> +	/* If another in-flight job needs permon, keep it active */
> +	if (atomic_dec_return(&pfdev->permon_pending) > 0)
> +		return;
> +
> +	/* Otherwise, we're the last user */
> +	gpu_write(pfdev, GPU_CMD, GPU_CMD_CYCLE_COUNT_STOP);
> +}

NIT: Similarly for disabling the usual style is '== 0'

Thanks,

Steve

> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.h b/drivers/gpu/drm/panfrost/panfrost_gpu.h
> index 468c51e7e..01a91af09 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.h
> @@ -18,4 +18,7 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev);
>  
>  void panfrost_gpu_amlogic_quirk(struct panfrost_device *pfdev);
>  
> +void panfrost_acquire_permon(struct panfrost_device *pfdev);
> +void panfrost_release_permon(struct panfrost_device *pfdev);
> +
>  #endif
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index 597cf1459..8a89aa274 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -117,6 +117,9 @@  struct panfrost_device {
 	struct shrinker shrinker;
 
 	struct panfrost_devfreq pfdevfreq;
+
+	/* Number of active jobs requiring performance monitoring */
+	atomic_t permon_pending;
 };
 
 struct panfrost_mmu {
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 2aae636f1..acacceb15 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -399,3 +399,23 @@  u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev)
 
 	return 0;
 }
+
+void panfrost_acquire_permon(struct panfrost_device *pfdev)
+{
+	/* If another in-flight job enabled permon, we don't have to */
+	if (atomic_inc_return(&pfdev->permon_pending) > 1)
+		return;
+
+	/* Otherwise, we're the first user */
+	gpu_write(pfdev, GPU_CMD, GPU_CMD_CYCLE_COUNT_START);
+}
+
+void panfrost_release_permon(struct panfrost_device *pfdev)
+{
+	/* If another in-flight job needs permon, keep it active */
+	if (atomic_dec_return(&pfdev->permon_pending) > 0)
+		return;
+
+	/* Otherwise, we're the last user */
+	gpu_write(pfdev, GPU_CMD, GPU_CMD_CYCLE_COUNT_STOP);
+}
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.h b/drivers/gpu/drm/panfrost/panfrost_gpu.h
index 468c51e7e..01a91af09 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.h
@@ -18,4 +18,7 @@  void panfrost_gpu_power_off(struct panfrost_device *pfdev);
 
 void panfrost_gpu_amlogic_quirk(struct panfrost_device *pfdev);
 
+void panfrost_acquire_permon(struct panfrost_device *pfdev);
+void panfrost_release_permon(struct panfrost_device *pfdev);
+
 #endif