mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tursulin@ursulin.net>
To: "Maíra Canal" <mcanal@igalia.com>,
	"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 11:04:29 +0000	[thread overview]
Message-ID: <2ca12f1b-89f6-4069-aedf-4e5d645e8deb@ursulin.net> (raw)
In-Reply-To: <33ccceef-3498-426a-ae7c-a8bafbb88992@igalia.com>


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.

> 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. :)

Regards,

Tvrtko

> [1] https://lore.kernel.org/dri-devel/3e00d8a9- 
> b6c4-8202-4f2d-5a659c61d094@redhat.com/
> 
> Best regards,
> - Maíra


  reply	other threads:[~2026-11-05 11:04 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 [this message]
2026-11-05 11:07       ` Maíra Canal

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=2ca12f1b-89f6-4069-aedf-4e5d645e8deb@ursulin.net \
    --to=tursulin@ursulin.net \
    --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=mcanal@igalia.com \
    --cc=ogabbay@kernel.org \
    --cc=phasta@kernel.org \
    --cc=sidong.yang@furiosa.ai \
    --cc=tomeu@tomeuvizoso.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®