From: "Maíra Canal" <mcanal@igalia.com>
To: Tvrtko Ursulin <tursulin@ursulin.net>,
Sidong Yang <sidong.yang@furiosa.ai>,
Tomeu Vizoso <tomeu@tomeuvizoso.net>,
Oded Gabbay <ogabbay@kernel.org>
Cc: Gyeyoung Baek <gye976@gmail.com>,
dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Philipp Stanner <phasta@kernel.org>,
Danilo Krummrich <dakr@kernel.org>
Subject: Re: [PATCH v4] accel/rocket: Report NPU usage through fdinfo
Date: Thu, 5 Nov 2026 08:07:57 -0300 [thread overview]
Message-ID: <bd2c65d2-c94c-4eca-9fc5-ec366e0be0fb@igalia.com> (raw)
In-Reply-To: <2ca12f1b-89f6-4069-aedf-4e5d645e8deb@ursulin.net>
On 05/11/26 08:04, Tvrtko Ursulin wrote:
>
> On 05/11/2026 10:43, Maíra Canal wrote:
>> Hi Tvrtko,
>>
>> On 05/11/26 06:22, Tvrtko Ursulin wrote:
>>>
>>> On 03/11/2026 15:53, Sidong Yang wrote:
>>>> There is no way to tell how much of the NPU a client uses. Implement
>>>> the
>>>> DRM usage stats: account the time each job occupies a core and report
>>>> it per client as drm-engine-npu, along with drm_show_memory_stats() for
>>>> the BOs.
>>>>
>>>> The cores are identical, so they are reported as a single engine with
>>>> drm-engine-capacity-npu set to rdev->max_cores, the number of core
>>>> slots. The time is summed over all cores, with an atomic counter as the
>>>> cores complete jobs concurrently.
>>>>
>>>> Jobs can outlive the file that submitted them, so the counter is
>>>> refcounted and each job holds a reference. The time covers the whole
>>>> period a job owns a core, so it also includes handling the completion
>>>> interrupt and the gaps between the tasks of a multi-task job.
>>>>
>>>> Document the keys in the rocket driver documentation.
>>>>
>>>> Signed-off-by: Sidong Yang <sidong.yang@furiosa.ai>
>>>> ---
>>>> v4: Report a single drm-engine-npu key with drm-engine-capacity-npu,
>>>> instead of drm-engine-npu0..2, as drm-usage-stats.rst specifies
>>>> for
>>>> a group of identical engines, and document the keys (Tomeu
>>>> Vizoso).
>>>> The subject no longer says per-core. Gyeyoung's tags are
>>>> dropped as
>>>> the accounting changed.
>>>>
>>>> Still depends on commit 6eff1d69dae8 ("accel/rocket: search every
>>>> core slot when a core is removed") for rdev->max_cores.
>>>>
>>>> v3: Add a rocket_engine_usage_create() helper for the counter
>>>> allocation in rocket_open(), to pair with
>>>> rocket_engine_usage_destroy() (Gyeyoung Baek).
>>>>
>>>> v2: Size the per-core counters by rdev->max_cores, the number of core
>>>> slots, instead of the number of cores bound when the file is
>>>> opened.
>>>> Sashiko reported the out-of-bounds read and Gyeyoung Baek
>>>> reproduced
>>>> it with KASAN.
>>>>
>>>> Link to v3: https://lore.kernel.org/all/20261103104804.1887489-1-
>>>> sidong.yang@furiosa.ai/
>>>> Link to v2: https://lore.kernel.org/all/20261101152723.1430312-1-
>>>> sidong.yang@furiosa.ai/
>>>> Link to v1: https://lore.kernel.org/all/20261031083642.1076356-1-
>>>> sidong.yang@furiosa.ai/
>>>>
>>>> Documentation/accel/rocket/index.rst | 28 +++++++++++
>>>> Documentation/gpu/drm-usage-stats.rst | 1 +
>>>> drivers/accel/rocket/rocket_drv.c | 70 +++++++++++++++++++++++
>>>> +++-
>>>> drivers/accel/rocket/rocket_drv.h | 15 ++++++
>>>> drivers/accel/rocket/rocket_job.c | 7 +++
>>>> drivers/accel/rocket/rocket_job.h | 4 ++
>>>> 6 files changed, 123 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/accel/rocket/index.rst b/Documentation/
>>>> accel/rocket/index.rst
>>>> index 70f97bccf100..53c9aa7e2229 100644
>>>> --- a/Documentation/accel/rocket/index.rst
>>>> +++ b/Documentation/accel/rocket/index.rst
>>>> @@ -17,3 +17,31 @@ project.
>>>> Hardware currently supported:
>>>> * RK3588
>>>> +
>>>> +.. _rocket-usage-stats:
>>>> +
>>>> +Rocket DRM client usage stats implementation
>>>> +============================================
>>>> +
>>>> +The accel/rocket driver implements the DRM client usage stats
>>>> specification as
>>>> +documented in :ref:`drm-client-usage-stats`.
>>>> +
>>>> +The NPU cores are reported as a single engine, npu, with a capacity
>>>> equal to the
>>>> +number of cores. The time is summed over all cores, and covers the
>>>> whole period
>>>> +a job occupies a core.
>>>> +
>>>> +Example of the output showing the implemented key value pairs:
>>>> +
>>>> +::
>>>> +
>>>> + pos: 0
>>>> + flags: 02400002
>>>> + mnt_id: 33
>>>> + ino: 749
>>>> + drm-driver: rocket
>>>> + drm-client-id: 246
>>>> + drm-engine-npu: 2296549300 ns
>>>> + drm-engine-capacity-npu: 3
>>>> + drm-total-memory: 24216 KiB
>>>> + drm-shared-memory: 0
>>>> + drm-active-memory: 1920 KiB
>>>> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/
>>>> gpu/drm-usage-stats.rst
>>>> index 70b7cfcc194f..288764730d23 100644
>>>> --- a/Documentation/gpu/drm-usage-stats.rst
>>>> +++ b/Documentation/gpu/drm-usage-stats.rst
>>>> @@ -218,3 +218,4 @@ Driver specific implementations
>>>> * :ref:`panthor-usage-stats`
>>>> * :ref:`xe-usage-stats`
>>>> * :ref:`amdxdna-usage-stats`
>>>> +* :ref:`rocket-usage-stats`
>>>> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/
>>>> rocket/ rocket_drv.c
>>>> index 2bcfe4ab3c68..e22e8623db64 100644
>>>> --- a/drivers/accel/rocket/rocket_drv.c
>>>> +++ b/drivers/accel/rocket/rocket_drv.c
>>>> @@ -3,8 +3,10 @@
>>>> #include <drm/drm_accel.h>
>>>> #include <drm/drm_drv.h>
>>>> +#include <drm/drm_file.h>
>>>> #include <drm/drm_gem.h>
>>>> #include <drm/drm_ioctl.h>
>>>> +#include <drm/drm_print.h>
>>>> #include <drm/rocket_accel.h>
>>>> #include <linux/clk.h>
>>>> #include <linux/err.h>
>>>> @@ -68,6 +70,38 @@ rocket_iommu_domain_put(struct
>>>> rocket_iommu_domain *domain)
>>>> kref_put(&domain->kref, rocket_iommu_domain_destroy);
>>>> }
>>>> +static struct rocket_engine_usage *
>>>> +rocket_engine_usage_create(void)
>>>> +{
>>>> + struct rocket_engine_usage *usage;
>>>> +
>>>> + usage = kzalloc_obj(*usage);
>>>> + if (!usage)
>>>> + return NULL;
>>>> + kref_init(&usage->kref);
>>>> +
>>>> + return usage;
>>>> +}
>>>> +
>>>> +static void
>>>> +rocket_engine_usage_destroy(struct kref *kref)
>>>> +{
>>>> + kfree(container_of(kref, struct rocket_engine_usage, kref));
>>>> +}
>>>> +
>>>> +struct rocket_engine_usage *
>>>> +rocket_engine_usage_get(struct rocket_file_priv *rocket_priv)
>>>> +{
>>>> + kref_get(&rocket_priv->engine_usage->kref);
>>>> + return rocket_priv->engine_usage;
>>>> +}
>>>> +
>>>> +void
>>>> +rocket_engine_usage_put(struct rocket_engine_usage *usage)
>>>> +{
>>>> + kref_put(&usage->kref, rocket_engine_usage_destroy);
>>>> +}
>>>> +
>>>> static int
>>>> rocket_open(struct drm_device *dev, struct drm_file *file)
>>>> {
>>>> @@ -86,10 +120,17 @@ rocket_open(struct drm_device *dev, struct
>>>> drm_file *file)
>>>> }
>>>> rocket_priv->rdev = rdev;
>>>> +
>>>> + rocket_priv->engine_usage = rocket_engine_usage_create();
>>>> + if (!rocket_priv->engine_usage) {
>>>> + ret = -ENOMEM;
>>>> + goto err_free;
>>>> + }
>>>
>>> Glanced over this patch by chance and it very reminded me of
>>> drm_sched_entity_stats tracking. That object is reference counted by
>>> the entity so I think it would satisfy your requirements. We would
>>> only need to add an accessor like drm_sched_entity_get_runtime() and
>>> just call it from rocket. Thoughts?
>>>
>>
>> I believe we tried something similar in the past (commit 97804a133c68)
>
> You mean df622729ddbf? But yes, I don't think I knew about that.
Yes, sorry for the copy/paste mistake.
>
>> and it ended up being reverted after this regression [1]. I would be
>> great to give this idea another change (specially now that we have
>> drm_sched_entity_stats), as it would also be useful for V3D.
>
> It makes sense to me if Philipp and Danilo will agree so adding them cc.
>
> It would just be adding u64 drm_sched_entity_get_runtime(entity).
> Nothing else would need to be exported and the implementation simply
> locks entity->stats and returns the runtime. As long as the caller knows
> they haven't destroyed the entity it is safe since entity holds a
> reference to the stats object.
>
> If rocket and v3d can use it that's a reasonable starting point for
> adding common API. Three users would be even better than two though,
> going by the long standing DRM tradition. :)
Considering commit 97804a133c68 (right commit this time), I believe that
Etnaviv could also benefit from it.
Best regards,
- Maíra
>
> Regards,
>
> Tvrtko
>
>> [1] https://lore.kernel.org/dri-devel/3e00d8a9-
>> b6c4-8202-4f2d-5a659c61d094@redhat.com/
>>
>> Best regards,
>> - Maíra
>
prev parent reply other threads:[~2026-11-05 11:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-03 15:53 Sidong Yang
2026-11-05 3:22 ` Gyeyoung Baek
2026-11-05 6:58 ` Tomeu Vizoso
2026-11-05 9:22 ` Tvrtko Ursulin
2026-11-05 10:43 ` Maíra Canal
2026-11-05 11:04 ` Tvrtko Ursulin
2026-11-05 11:07 ` Maíra Canal [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=bd2c65d2-c94c-4eca-9fc5-ec366e0be0fb@igalia.com \
--to=mcanal@igalia.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gye976@gmail.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ogabbay@kernel.org \
--cc=phasta@kernel.org \
--cc=sidong.yang@furiosa.ai \
--cc=tomeu@tomeuvizoso.net \
--cc=tursulin@ursulin.net \
/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
all inboxes | Powered by JetHome®