mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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
> 


      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®