mbox series

[0/4] drm/panfrost: Plumb cycle counters to userspace

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

Message

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

Mali has hardware cycle counters (and GPU timestamps) available for
profiling. These are exposed in various ways:

- Kernel: As CYCLE_COUNT and TIMESTAMP registers 
- Job chain: As WRITE_VALUE descriptors
- Shader (Midgard): As LD_SPECIAL selectors
- Shader (Bifrost): As the LD_GCLK.u64 instruction

These form building blocks for profiling features, for example the
ARB_shader_clock extension which accesses the counters from an
application's shader.

The counters consume power, so it is recommended to disable the counters
when not in use. To do so, we follow the strategy from mali_kbase: add a
counter requirement to the job, start the counters only when required,
and stop them as quickly as possible.

The new UABI will be used in Mesa. An implementation of ARB_shader_clock
using this UABI is available as a pending upstream merge request [1].
The implementation passes the relevant piglit test, validating both the
kernel and mesa.

The main outstanding questing is the proper name. Performance monitoring
("PERMON") is the name used by kbase, but it's jargon-y and risks
confusion with performance counters, an orthogonal mechanism. Cycle
count is more descriptive and matches the actual hardware name, but
obscures that the same mechanism is required for GPU timestamps. This
bit of bikeshedding aside, I'm pleased with the patches.

[1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/11051

Alyssa Rosenzweig (4):
  drm/panfrost: Add cycle counter job requirement
  drm/panfrost: Add CYCLE_COUNT_START/STOP commands
  drm/panfrost: Add permon acquire/release helpers
  drm/panfrost: Handle PANFROST_JD_REQ_PERMON

 drivers/gpu/drm/panfrost/panfrost_device.h |  3 +++
 drivers/gpu/drm/panfrost/panfrost_drv.c    | 10 +++++++---
 drivers/gpu/drm/panfrost/panfrost_gpu.c    | 20 ++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_gpu.h    |  3 +++
 drivers/gpu/drm/panfrost/panfrost_job.c    |  6 ++++++
 drivers/gpu/drm/panfrost/panfrost_regs.h   |  2 ++
 include/uapi/drm/panfrost_drm.h            |  3 ++-
 7 files changed, 43 insertions(+), 4 deletions(-)

Comments

Alyssa Rosenzweig May 27, 2021, 9:14 p.m. UTC | #1
> The main outstanding questing is the proper name. Performance monitoring
> ("PERMON") is the name used by kbase, but it's jargon-y and risks
> confusion with performance counters, an orthogonal mechanism. Cycle
> count is more descriptive and matches the actual hardware name, but
> obscures that the same mechanism is required for GPU timestamps. This
> bit of bikeshedding aside, I'm pleased with the patches.

PANFROST_JD_REQ_CLOCK might be the clearest.
Tomeu Vizoso May 28, 2021, 6:07 a.m. UTC | #2
Hi Alyssa,

Will this be enough to implement GL_TIMESTAMP and GL_TIME_ELAPSED queries?

Guess the DDK implements these as WRITE_VALUE jobs, and there's also a 
soft job BASE_JD_REQ_SOFT_DUMP_CPU_GPU_TIME that I guess is used for 
glGet*(GL_TIMESTAMP). Other DRM drivers use an ioctl for that instead.

Regards,

Tomeu

On 5/27/21 10:38 PM, alyssa.rosenzweig@collabora.com wrote:
> From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> 
> Mali has hardware cycle counters (and GPU timestamps) available for
> profiling. These are exposed in various ways:
> 
> - Kernel: As CYCLE_COUNT and TIMESTAMP registers
> - Job chain: As WRITE_VALUE descriptors
> - Shader (Midgard): As LD_SPECIAL selectors
> - Shader (Bifrost): As the LD_GCLK.u64 instruction
> 
> These form building blocks for profiling features, for example the
> ARB_shader_clock extension which accesses the counters from an
> application's shader.
> 
> The counters consume power, so it is recommended to disable the counters
> when not in use. To do so, we follow the strategy from mali_kbase: add a
> counter requirement to the job, start the counters only when required,
> and stop them as quickly as possible.
> 
> The new UABI will be used in Mesa. An implementation of ARB_shader_clock
> using this UABI is available as a pending upstream merge request [1].
> The implementation passes the relevant piglit test, validating both the
> kernel and mesa.
> 
> The main outstanding questing is the proper name. Performance monitoring
> ("PERMON") is the name used by kbase, but it's jargon-y and risks
> confusion with performance counters, an orthogonal mechanism. Cycle
> count is more descriptive and matches the actual hardware name, but
> obscures that the same mechanism is required for GPU timestamps. This
> bit of bikeshedding aside, I'm pleased with the patches.
> 
> [1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/11051
> 
> Alyssa Rosenzweig (4):
>    drm/panfrost: Add cycle counter job requirement
>    drm/panfrost: Add CYCLE_COUNT_START/STOP commands
>    drm/panfrost: Add permon acquire/release helpers
>    drm/panfrost: Handle PANFROST_JD_REQ_PERMON
> 
>   drivers/gpu/drm/panfrost/panfrost_device.h |  3 +++
>   drivers/gpu/drm/panfrost/panfrost_drv.c    | 10 +++++++---
>   drivers/gpu/drm/panfrost/panfrost_gpu.c    | 20 ++++++++++++++++++++
>   drivers/gpu/drm/panfrost/panfrost_gpu.h    |  3 +++
>   drivers/gpu/drm/panfrost/panfrost_job.c    |  6 ++++++
>   drivers/gpu/drm/panfrost/panfrost_regs.h   |  2 ++
>   include/uapi/drm/panfrost_drm.h            |  3 ++-
>   7 files changed, 43 insertions(+), 4 deletions(-)
>
Alyssa Rosenzweig May 28, 2021, 1:31 p.m. UTC | #3
Hi Tomeu,

> Will this be enough to implement GL_TIMESTAMP and GL_TIME_ELAPSED queries?
> 
> Guess the DDK implements these as WRITE_VALUE jobs, and there's also a soft
> job BASE_JD_REQ_SOFT_DUMP_CPU_GPU_TIME that I guess is used for
> glGet*(GL_TIMESTAMP). Other DRM drivers use an ioctl for that instead.

For anything implemented as WRITE_VALUE jobs, this is necessary and
sufficient on the kernel side. If an out-of-band soft job or ioctl is
truly needed (I haven't looked), of course that needs additional piping.

Thanks,

Alyssa