From: Iago Toral <itoral@igalia.com>
To: "Maíra Canal" <mcanal@igalia.com>,
"Melissa Wen" <mwen@igalia.com>,
"Jose Maria Casanova Crespo" <jmcasanova@igalia.com>,
"Juan A . Suárez" <jasuarez@igalia.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"David Airlie" <airlied@gmail.com>,
"Daniel Vetter" <daniel@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org, kernel-dev@igalia.com
Subject: Re: [PATCH 0/6] drm/v3d: Improve Performance Counters handling
Date: Thu, 09 May 2024 08:15:14 +0200 [thread overview]
Message-ID: <073ff0b7737845a12099ba39fc0cab0bdc730799.camel@igalia.com> (raw)
In-Reply-To: <20240508143306.2435304-2-mcanal@igalia.com>
Hi Maíra,
I made a couple of minor comments, with those addressed the series is:
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
Thanks a lot for this!,
Iago
El mié, 08-05-2024 a las 11:30 -0300, Maíra Canal escribió:
> This series has the intention to address two issues with Performance
> Counters
> on V3D:
>
> 1. Update the number of Performance Counters for V3D 7.1
>
> V3D 7.1 has 93 performance counters, while V3D 4.2 has only 87.
> Although the
> series [1] enabled support for V3D 7.1, it didn’t replace the maximum
> number of
> performance counters. This led to errors in user space as the Vulkan
> driver
> updated the maximum number of performance counters, but the kernel
> didn’t.
>
> Currently, the user space can request values for performance counters
> that
> are greater than 87 and the kernel will return an error instead of
> the values.
> That’s why `dEQP-VK.query_pool.performance_query.*` currently fails
> on Mesa
> CI [2]. This series intends to fix the `dEQP-
> VK.query_pool.performance_query.*`
> fail.
>
> 2. Make the kernel able to provide the Performance Counter
> descriptions
>
> Although all the management of the Performance Monitors is done
> through IOCTLs,
> which means that the code is in the kernel, the performance counter
> descriptions
> are in Mesa. This means two things: (#1) only Mesa has access to the
> descriptions
> and (#2) we can have inconsistencies between the information provided
> by Mesa
> and the kernel, as seen in the first issue addressed by this series.
>
> To minimize the risk of inconsistencies, this series proposes to use
> the kernel
> as a “single source of truth”. Therefore, if there are any changes to
> the
> performance monitors, all the changes must be done only in the
> kernel. This
> means that all information about the maximum number of performance
> counters and
> all the descriptions will now be retrieved from the kernel.
>
> This series is coupled with a Mesa series [3] that enabled the use of
> the new
> IOCTL. I appreciate any feedback from both the kernel and Mesa
> implementations.
>
> [1]
> https://lore.kernel.org/dri-devel/20231031073859.25298-1-itoral@igalia.com/
> [2]
> https://gitlab.freedesktop.org/mesa/mesa/-/commit/ea1f09a5f21839f4f3b93610b58507c4bd9b9b81
> [3]
> https://gitlab.freedesktop.org/mairacanal/mesa/-/tree/v3dv/fix-perfcnt
>
> Best Regards,
> - Maíra Canal
>
> Maíra Canal (6):
> drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
> drm/v3d: Different V3D versions can have different number of
> perfcnt
> drm/v3d: Create a new V3D parameter for the maximum number of
> perfcnt
> drm/v3d: Create new IOCTL to expose performance counters
> information
> drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
> drm/v3d: Deprecate the use of the Performance Counters enum
>
> drivers/gpu/drm/v3d/v3d_drv.c | 11 +
> drivers/gpu/drm/v3d/v3d_drv.h | 14 +-
> drivers/gpu/drm/v3d/v3d_perfmon.c | 36 ++-
> .../gpu/drm/v3d/v3d_performance_counters.h | 208
> ++++++++++++++++++
> drivers/gpu/drm/v3d/v3d_sched.c | 2 +-
> include/uapi/drm/v3d_drm.h | 44 ++++
> 6 files changed, 312 insertions(+), 3 deletions(-)
> create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h
>
prev parent reply other threads:[~2024-05-09 6:15 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 14:30 [PATCH 0/6] drm/v3d: Improve Performance Counters handling Maíra Canal
2024-05-08 14:30 ` [PATCH 1/6] drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1 Maíra Canal
2024-05-08 14:30 ` [PATCH 2/6] drm/v3d: Different V3D versions can have different number of perfcnt Maíra Canal
2024-05-08 14:30 ` [PATCH 3/6] drm/v3d: Create a new V3D parameter for the maximum " Maíra Canal
2024-05-08 14:30 ` [PATCH 4/6] drm/v3d: Create new IOCTL to expose performance counters information Maíra Canal
2024-05-08 14:30 ` [PATCH 5/6] drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM Maíra Canal
2024-05-09 6:11 ` Iago Toral
2024-05-08 14:30 ` [PATCH 6/6] drm/v3d: Deprecate the use of the Performance Counters enum Maíra Canal
2024-05-09 6:14 ` Iago Toral
2024-05-09 6:15 ` Iago Toral [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=073ff0b7737845a12099ba39fc0cab0bdc730799.camel@igalia.com \
--to=itoral@igalia.com \
--cc=airlied@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=jasuarez@igalia.com \
--cc=jmcasanova@igalia.com \
--cc=kernel-dev@igalia.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mcanal@igalia.com \
--cc=mripard@kernel.org \
--cc=mwen@igalia.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).