Re: [PATCH v2 0/1] Add support for EU stall sampling

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Sun, Jul 07, 2024 at 03:41:40PM -0700, Ashutosh Dixit wrote:
> The following patch adds support for EU stall sampling, a new hardware
>feature first added in PVC and is being supported in XE2 and later
>architecture GPUs. This feature would enable capturing of EU stall
>data which include the IP address of the instruction stalled and
>various stall reason counts. More details are explained in the patch
>commit message.
>
>I am posting this patch as an RFC to get early feedback in the uAPI

maybe not an RFC anymore?

>while support for this feature is being added into Mesa. A new test
>in the IGT repo: https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
>is also under development to test this feature in the driver. This
>patch has undergone basic testing with the new IGT test that is under
>development.
>
>The EU stall data from the driver include a header with additional
>information about the data that follows the header. The header
>includes flags one of which indicate if data has been dropped has

s/has been dropped//

>been dropped by the hardware due to buffer being full. While read
>returns the total bytes read, any data dropped by the hardware will
>be indicated in the flags.

>One feedback received so far is to make
>read return an error when data has been dropped by the hardware
>instead of setting a flag in the header.

if dropped data can be mapped to an intuitive error in the return of 
read, we should definitely do that. If multiple headers are returned in 
a read call and only some header blobs are affected, then, IMO, we 
should also set the flag for the relevant headers.

>One suggestion received
>is to consider two FDs per EU stall data stream with one fd to read
>data and other fd to pass any errors. please comment on this idea
>that's not represented in the code

If it is just one stream of data, then I would avoid an additional FD 
for passing errors. Read should be able to handle returning errors. Is 
there a limitation in the generic file ops (like read()) that EU stall 
supports? Are errors very common when consuming EU stall data?

Regards,
Umesh
>
>Thank You.
>
>v2: Rename xe perf layer as xe observation layer
>
>Harish Chegondi (1):
>  drm/xe/eustall: Add support for EU stall sampling
>
> drivers/gpu/drm/xe/Makefile                |    1 +
> drivers/gpu/drm/xe/regs/xe_eu_stall_regs.h |   33 +
> drivers/gpu/drm/xe/xe_eustall_cntr.c       | 1005 ++++++++++++++++++++
> drivers/gpu/drm/xe/xe_eustall_cntr.h       |   62 ++
> drivers/gpu/drm/xe/xe_gt.c                 |    3 +
> drivers/gpu/drm/xe/xe_gt_topology.c        |    9 +
> drivers/gpu/drm/xe/xe_gt_topology.h        |    3 +
> drivers/gpu/drm/xe/xe_gt_types.h           |    4 +
> drivers/gpu/drm/xe/xe_observation.c        |   14 +
> drivers/gpu/drm/xe/xe_trace.h              |   35 +
> include/uapi/drm/xe_drm.h                  |   77 ++
> 11 files changed, 1246 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/regs/xe_eu_stall_regs.h
> create mode 100644 drivers/gpu/drm/xe/xe_eustall_cntr.c
> create mode 100644 drivers/gpu/drm/xe/xe_eustall_cntr.h
>
>-- 
>2.41.0
>