Re: [PATCH v5 5/5] gpu: nova-core: gsp: add mutex locking to Cmdq
From: Alexandre Courbot
Date: Fri Mar 20 2026 - 10:55:10 EST
On Thu Mar 19, 2026 at 12:40 AM JST, Danilo Krummrich wrote:
> On Wed Mar 18, 2026 at 4:27 PM CET, Alexandre Courbot wrote:
>> On Wed Mar 18, 2026 at 1:07 PM JST, Eliot Courtney wrote:
>> <snip>
>>> + /// Returns the DMA handle of the command queue's shared memory region.
>>> + pub(crate) fn dma_handle(&self) -> DmaAddress {
>>> + self.inner.lock().gsp_mem.0.dma_handle()
>>> + }
>>
>> Just noticed that we now need to lock to get the DMA handle. It's
>> inconsequential in practice but a bit inelegant.
>>
>> Since the DMA handle never changes, and is only ever needed during
>> initialization, I think I will just insert a patch before this one that
>> adds a `pub(super) dma_handle` member to `Cmdq`. That way we only need
>> to obtain the handle at construction time and can get rid of this
>> method, which keeps the public API focused on message handling.
>>
>> No need to resend, I will apply this patch on top of mine and merge.
>
> It is a minor inconvinience, but it may indicate that dma::Coherent should
> probaly support locking, i.e. you want to protect the data within the
> dma::Coherent allocation, not the dma::Coherent object itself.
Not quite sure I understand what you mean here - can you elaborate? This
sounds to me like you want to add a `Mutex` to every `dma::Coherent`, so
I am likely misunderstanding. :)