All of lore.kernel.org
 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: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 15:53 [PATCH v4] accel/rocket: Report NPU usage through fdinfo 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]
2026-11-06 15:44       ` Sidong Yang
2026-11-12 13:46         ` Tvrtko Ursulin
2026-11-12 16:57           ` Sidong Yang

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.