On 6/2/25 17:25, Tvrtko Ursulin wrote:
>
> On 02/06/2025 15:42, Christian König wrote:
>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>>
>>> Hi,
>>>
>>> On 15/05/2025 14:15, Christian König wrote:
>>>> Hey drm-misc maintainers,
>>>>
>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>
>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>
>>> Looks like the backmerge is still pending?
>>
>> Yes, @Maarten, @Maxime and @Thomas ping on this.
>>
>>> In the meantime, Christian, any chance you will have some bandwith to think about the tail end of the series? Specifically patch 6 and how that is used onward.
>>
>> Well the RCU grace period is quite a nifty hack. I wanted to go over it again after merging the first patches from this series.
>>
>> In general looks like a good idea to me, I just don't like that we explicitely need to expose dma_fence_access_begin() and dma_fence_access_end().
>>
>> Especially we can't do that while calling fence->ops->release.
>
> Hm why not? You think something will take offence of the rcu_read_lock()?
Yes, especially it is perfectly legitimate to call synchronize_rcu() or lock semaphores/mutexes from that callback.
Either keep the RCU critical section only for the trace or even better come up with some different approach, e.g. copying the string under the RCU lock or something like that.
Regards,
Christian.
>
> Regards,
>
> Tvrtko
>
>>>> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>>>>> With the goal of reducing the need for drivers to touch (and dereference)
>>>>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>>>>> the fence->flags.
>>>>>
>>>>> Drivers which were setting this flag are changed to use new
>>>>> dma_fence_init64() instead of dma_fence_init().
>>>>>
>>>>> v2:
>>>>> * Streamlined init and added kerneldoc.
>>>>> * Rebase for amdgpu userq which landed since.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com>
>>>>> Reviewed-by: Christian König <christian.koenig(a)amd.com> # v1
>>>>> ---
>>>>> drivers/dma-buf/dma-fence-chain.c | 5 +-
>>>>> drivers/dma-buf/dma-fence.c | 69 ++++++++++++++-----
>>>>> .../drm/amd/amdgpu/amdgpu_eviction_fence.c | 7 +-
>>>>> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 5 +-
>>>>> .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c | 5 +-
>>>>> include/linux/dma-fence.h | 14 ++--
>>>>> 6 files changed, 64 insertions(+), 41 deletions(-)
>>>>>
>>>>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>>>>> index 90424f23fd73..a8a90acf4f34 100644
>>>>> --- a/drivers/dma-buf/dma-fence-chain.c
>>>>> +++ b/drivers/dma-buf/dma-fence-chain.c
>>>>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>>>> }
>>>>> const struct dma_fence_ops dma_fence_chain_ops = {
>>>>> - .use_64bit_seqno = true,
>>>>> .get_driver_name = dma_fence_chain_get_driver_name,
>>>>> .get_timeline_name = dma_fence_chain_get_timeline_name,
>>>>> .enable_signaling = dma_fence_chain_enable_signaling,
>>>>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>>>> seqno = max(prev->seqno, seqno);
>>>>> }
>>>>> - dma_fence_init(&chain->base, &dma_fence_chain_ops,
>>>>> - &chain->lock, context, seqno);
>>>>> + dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>>>>> + context, seqno);
>>>>> /*
>>>>> * Chaining dma_fence_chain container together is only allowed through
>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>> index f0cdd3e99d36..705b59787731 100644
>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>>>> }
>>>>> EXPORT_SYMBOL(dma_fence_describe);
>>>>> -/**
>>>>> - * dma_fence_init - Initialize a custom fence.
>>>>> - * @fence: the fence to initialize
>>>>> - * @ops: the dma_fence_ops for operations on this fence
>>>>> - * @lock: the irqsafe spinlock to use for locking this fence
>>>>> - * @context: the execution context this fence is run on
>>>>> - * @seqno: a linear increasing sequence number for this context
>>>>> - *
>>>>> - * Initializes an allocated fence, the caller doesn't have to keep its
>>>>> - * refcount after committing with this fence, but it will need to hold a
>>>>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>> - *
>>>>> - * context and seqno are used for easy comparison between fences, allowing
>>>>> - * to check which fence is later by simply using dma_fence_later().
>>>>> - */
>>>>> -void
>>>>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>> - spinlock_t *lock, u64 context, u64 seqno)
>>>>> +static void
>>>>> +__dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>> + spinlock_t *lock, u64 context, u64 seqno, unsigned long flags)
>>>>> {
>>>>> BUG_ON(!lock);
>>>>> BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
>>>>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>> fence->lock = lock;
>>>>> fence->context = context;
>>>>> fence->seqno = seqno;
>>>>> - fence->flags = 0UL;
>>>>> + fence->flags = flags;
>>>>> fence->error = 0;
>>>>> trace_dma_fence_init(fence);
>>>>> }
>>>>> +
>>>>> +/**
>>>>> + * dma_fence_init - Initialize a custom fence.
>>>>> + * @fence: the fence to initialize
>>>>> + * @ops: the dma_fence_ops for operations on this fence
>>>>> + * @lock: the irqsafe spinlock to use for locking this fence
>>>>> + * @context: the execution context this fence is run on
>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>> + *
>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>> + *
>>>>> + * context and seqno are used for easy comparison between fences, allowing
>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>> + */
>>>>> +void
>>>>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>> + spinlock_t *lock, u64 context, u64 seqno)
>>>>> +{
>>>>> + __dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>>>>> +}
>>>>> EXPORT_SYMBOL(dma_fence_init);
>>>>> +
>>>>> +/**
>>>>> + * dma_fence_init64 - Initialize a custom fence with 64-bit seqno support.
>>>>> + * @fence: the fence to initialize
>>>>> + * @ops: the dma_fence_ops for operations on this fence
>>>>> + * @lock: the irqsafe spinlock to use for locking this fence
>>>>> + * @context: the execution context this fence is run on
>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>> + *
>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>> + *
>>>>> + * Context and seqno are used for easy comparison between fences, allowing
>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>> + */
>>>>> +void
>>>>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>> + spinlock_t *lock, u64 context, u64 seqno)
>>>>> +{
>>>>> + __dma_fence_init(fence, ops, lock, context, seqno,
>>>>> + BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>>>> +}
>>>>> +EXPORT_SYMBOL(dma_fence_init64);
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>> index 1a7469543db5..79713421bffe 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>>>> }
>>>>> static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>>>>> - .use_64bit_seqno = true,
>>>>> .get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>>>> .get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>>>> .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>>>>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>>>> ev_fence->evf_mgr = evf_mgr;
>>>>> get_task_comm(ev_fence->timeline_name, current);
>>>>> spin_lock_init(&ev_fence->lock);
>>>>> - dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>> - &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>> - atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>> + dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>> + &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>> + atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>> return ev_fence;
>>>>> }
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>> index 029cb24c28b3..5e92d00a591f 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>>>> fence = &userq_fence->base;
>>>>> userq_fence->fence_drv = fence_drv;
>>>>> - dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>> - fence_drv->context, seq);
>>>>> + dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>> + fence_drv->context, seq);
>>>>> amdgpu_userq_fence_driver_get(fence_drv);
>>>>> dma_fence_get(fence);
>>>>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>>>> }
>>>>> static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>>>> - .use_64bit_seqno = true,
>>>>> .get_driver_name = amdgpu_userq_fence_get_driver_name,
>>>>> .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>>>> .signaled = amdgpu_userq_fence_signaled,
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>> index 51cddfa3f1e8..5d26797356a3 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>>>> }
>>>>> static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>>>>> - .use_64bit_seqno = true,
>>>>> .get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>>>> .get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>>>> };
>>>>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>>>> INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>>>> spin_lock_init(&f->lock);
>>>>> - dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>> - vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>> + dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>> + vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>> /* TODO: We probably need a separate wq here */
>>>>> dma_fence_get(&f->base);
>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>> index 48b5202c531d..a34a0dcdc446 100644
>>>>> --- a/include/linux/dma-fence.h
>>>>> +++ b/include/linux/dma-fence.h
>>>>> @@ -97,6 +97,7 @@ struct dma_fence {
>>>>> };
>>>>> enum dma_fence_flag_bits {
>>>>> + DMA_FENCE_FLAG_SEQNO64_BIT,
>>>>> DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>> DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>>> DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>>>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>>>> *
>>>>> */
>>>>> struct dma_fence_ops {
>>>>> - /**
>>>>> - * @use_64bit_seqno:
>>>>> - *
>>>>> - * True if this dma_fence implementation uses 64bit seqno, false
>>>>> - * otherwise.
>>>>> - */
>>>>> - bool use_64bit_seqno;
>>>>> -
>>>>> /**
>>>>> * @get_driver_name:
>>>>> *
>>>>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>>>> void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>> spinlock_t *lock, u64 context, u64 seqno);
>>>>> +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>> + spinlock_t *lock, u64 context, u64 seqno);
>>>>> +
>>>>> void dma_fence_release(struct kref *kref);
>>>>> void dma_fence_free(struct dma_fence *fence);
>>>>> void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>>>>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>>> * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>>> * do so.
>>>>> */
>>>>> - if (fence->ops->use_64bit_seqno)
>>>>> + if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>>> return f1 > f2;
>>>>> return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>>>
>>>
>>
>
6.6-stable review patch. If anyone has any objections, please let me know.
------------------
From: Thomas Zimmermann <tzimmermann(a)suse.de>
commit 8260731ccad0451207b45844bb66eb161a209218 upstream.
Test struct drm_gem_object.import_attach to detect imported objects.
During object clenanup, the dma_buf field might be NULL. Testing it in
an object's free callback then incorrectly does a cleanup as for native
objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that
clears the dma_buf field in drm_gem_object_exported_dma_buf_free().
v3:
- only test for import_attach (Boris)
v2:
- use import_attach.dmabuf instead of dma_buf (Christian)
Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de>
Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper")
Reported-by: Andy Yan <andyshrk(a)163.com>
Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh…
Tested-by: Andy Yan <andyshrk(a)163.com>
Cc: Thomas Zimmermann <tzimmermann(a)suse.de>
Cc: Anusha Srivatsa <asrivats(a)redhat.com>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com>
Cc: Maxime Ripard <mripard(a)kernel.org>
Cc: David Airlie <airlied(a)gmail.com>
Cc: Simona Vetter <simona(a)ffwll.ch>
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: "Christian König" <christian.koenig(a)amd.com>
Cc: dri-devel(a)lists.freedesktop.org
Cc: linux-media(a)vger.kernel.org
Cc: linaro-mm-sig(a)lists.linaro.org
Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com>
Reviewed-by: Simona Vetter <simona.vetter(a)ffwll.ch>
Link: https://lore.kernel.org/r/20250416065820.26076-1-tzimmermann@suse.de
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
include/drm/drm_gem.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -567,8 +567,7 @@ static inline bool drm_gem_object_is_sha
*/
static inline bool drm_gem_is_imported(const struct drm_gem_object *obj)
{
- /* The dma-buf's priv field points to the original GEM object. */
- return obj->dma_buf && (obj->dma_buf->priv != obj);
+ return !!obj->import_attach;
}
#ifdef CONFIG_LOCKDEP
On Thu, May 29, 2025 at 01:34:43PM +0800, Xu Yilun wrote:
> This series has 3 sections:
I really think this is too big to try to progress, even in RFC
form.
> Patch 1 - 11 deal with the private MMIO mapping in KVM MMU via DMABUF.
> Leverage Jason & Vivek's latest VFIO dmabuf series [3], see Patch 2 - 4.
> The concern for get_pfn() kAPI [4] is not addressed so are marked as
> HACK, will investigate later.
I would probably split this out entirely into its own topic. It
doesn't seem directly related to TSM as KVM can use DMABUF for good
reasons independently .
> Patch 12 - 22 is about TSM Bind/Unbind/Guest request management in VFIO
> & IOMMUFD. Picks some of Shameer's patch in [5], see Patch 12 & 14.
This is some reasonable topic on its own after Dan's series
> Patch 23 - 30 is a solution to meet the TDX specific sequence
> enforcement on various device Unbind cases, including converting device
> back to shared, hot unplug, TD destroy. Start with a tdx_tsm driver
> prototype and finally implement the Unbind enforcement inside the
> driver. To be honest it is still awkward to me, but I need help.
Then you have a series or two to implement TDX using the infrastructure.
Jason
On Thu, May 29, 2025 at 01:34:53PM +0800, Xu Yilun wrote:
> Export vfio dma-buf specific info by attaching vfio_dma_buf_data in
> struct dma_buf::priv. Provide a helper vfio_dma_buf_get_data() for
> importers to fetch these data. Exporters identify VFIO dma-buf by
> successfully getting these data.
>
> VFIO dma-buf supports disabling host access to these exported MMIO
> regions when the device is converted to private. Exporters like KVM
> need to identify this type of dma-buf to decide if it is good to use.
> KVM only allows host unaccessible MMIO regions been mapped in private
> roots.
>
> Export struct kvm * handler attached to the vfio device. This
> allows KVM to do another sanity check. MMIO should only be assigned to
> a CoCo VM if its owner device is already assigned to the same VM.
This doesn't seem right, it should be encapsulated into the standard
DMABUF API in some way.
Jason
On Thu, May 29, 2025 at 10:41:15PM +0800, Xu Yilun wrote:
> > On AMD, the host can "revoke" at any time, at worst it'll see RMP
> > events from IOMMU. Thanks,
>
> Is the RMP event firstly detected by host or guest? If by host,
> host could fool guest by just suppress the event. Guest thought the
> DMA writting is successful but it is not and may cause security issue.
Is that in scope of the threat model though? Host must not be able to
change DMAs or target them to different memory, but the host can stop
DMA and loose it, surely?
Host controls the PCI memory enable bit, doesn't it?
Jason
On Tue, May 20, 2025 at 5:27 AM Tomeu Vizoso <tomeu(a)tomeuvizoso.net> wrote:
>
> Add the bindings for the Neural Processing Unit IP from Rockchip.
>
> v2:
> - Adapt to new node structure (one node per core, each with its own
> IOMMU)
> - Several misc. fixes from Sebastian Reichel
>
> v3:
> - Split register block in its constituent subblocks, and only require
> the ones that the kernel would ever use (Nicolas Frattaroli)
> - Group supplies (Rob Herring)
> - Explain the way in which the top core is special (Rob Herring)
>
> v4:
> - Change required node name to npu@ (Rob Herring and Krzysztof Kozlowski)
> - Remove unneeded items: (Krzysztof Kozlowski)
> - Fix use of minItems/maxItems (Krzysztof Kozlowski)
> - Add reg-names to list of required properties (Krzysztof Kozlowski)
> - Fix example (Krzysztof Kozlowski)
>
> v5:
> - Rename file to rockchip,rk3588-rknn-core.yaml (Krzysztof Kozlowski)
> - Streamline compatible property (Krzysztof Kozlowski)
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel(a)collabora.com>
> Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net>
> ---
> .../bindings/npu/rockchip,rk3588-rknn-core.yaml | 147 +++++++++++++++++++++
> 1 file changed, 147 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..9eb426367afcbc03c387d43c4b8250cdd1b9ee86
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> @@ -0,0 +1,147 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/npu/rockchip,rk3588-rknn-core.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Neural Processing Unit IP from Rockchip
> +
> +maintainers:
> + - Tomeu Vizoso <tomeu(a)tomeuvizoso.net>
> +
> +description:
> + Rockchip IP for accelerating inference of neural networks, based on NVIDIA's
> + open source NVDLA IP.
> +
> + There is to be a node per each core in the NPU. In Rockchip's design there
> + will be one core that is special and needs to be powered on before any of the
> + other cores can be used. This special core is called the top core and should
> + have the compatible string that corresponds to top cores.
Is this really a distinction in the h/w? If you change which core is
the top one in the DT, does it still work?
> +
> +properties:
> + $nodename:
> + pattern: '^npu@[a-f0-9]+$'
> +
> + compatible:
> + enum:
> + - rockchip,rk3588-rknn-core-top
> + - rockchip,rk3588-rknn-core
> +
> + reg:
> + maxItems: 3
> +
> + reg-names:
> + items:
> + - const: pc
> + - const: cna
> + - const: core
> +
> + clocks:
> + minItems: 2
> + maxItems: 4
> +
> + clock-names:
> + items:
> + - const: aclk
> + - const: hclk
> + - const: npu
> + - const: pclk
> + minItems: 2
It is odd that the non-top cores only have bus clocks and no module
clock. But based on the clock names, I'm guessing the aclk/hclk are
not shared, but the npu and pclk are shared. Since you make the top
core probe first, then it will enable the shared clocks and the
non-top cores don't have to worry about them. If so, that is wrong as
it is letting the software design define the bindings.
Rob