Nouveau - Nov 2025 - [PATCH v2 06/12] nova-core: Add bindings required by GSP sequencer

Hi Joel,

On Mon Nov 3, 2025 at 8:59 AM JST, Joel Fernandes wrote:
> Add several firmware bindings required by GSP sequencer code.
>
> Co-developed-by: Alistair Popple <apopple at nvidia.com>
> Signed-off-by: Alistair Popple <apopple at nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/fw.rs               | 45 ++++++++++
>  .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 85 +++++++++++++++++++
>  2 files changed, 130 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs
b/drivers/gpu/nova-core/gsp/fw.rs
> index 687749bdbb45..53e28458cd7d 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -543,6 +543,51 @@ pub(crate) fn element_count(&self) -> u32 {
>      }
>  }
>  
> +#[expect(unused)]
> +pub(crate) use r570_144::{
We are trying to avoid that (direct reexports from the generated
bindings) - some explanations for how to abstract these follow.
> +    // GSP sequencer run structure with information on how to run the
sequencer.
> +    rpc_run_cpu_sequencer_v17_00,
This type should be wrapped in a transparent type similarly to e.g.
`GspArgumentsCached`: its internal structure hidden, and the needed
functionality made accessible through a constructor and (if needed)
other methods.
> +
> +    // GSP sequencer structures.
> +    GSP_SEQUENCER_BUFFER_CMD,
> +    GSP_SEQ_BUF_OPCODE,
> +
> +    // GSP sequencer core operation opcodes.
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_CORE_RESET,
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_CORE_RESUME,
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_CORE_START,
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_CORE_WAIT_FOR_HALT,
> +
> +    // GSP sequencer delay opcode and payload.
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_DELAY_US,
> +
> +    // GSP sequencer register opcodes.
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_REG_MODIFY,
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_REG_POLL,
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_REG_STORE,
> +    GSP_SEQ_BUF_OPCODE_GSP_SEQ_BUF_OPCODE_REG_WRITE,
The opcodes can be exposed through a dedicated enum like `MsgFunction`.
> +
> +    // GSP sequencer delay payload structure.
> +    GSP_SEQ_BUF_PAYLOAD_DELAY_US,
> +
> +    // GSP sequencer register payload structures.
> +    GSP_SEQ_BUF_PAYLOAD_REG_MODIFY,
> +    GSP_SEQ_BUF_PAYLOAD_REG_POLL,
> +    GSP_SEQ_BUF_PAYLOAD_REG_STORE,
> +    GSP_SEQ_BUF_PAYLOAD_REG_WRITE, //
These ones are a bit trickier to abstract. Since they ever only use
`bar` from the sequencer, I guess we can have their semantics in the
`fw` module, exposed through a method that receives the `bar`? That way
the sequencer won't have to access their members which are private to
it.

On 11/10/2025 8:39 AM, Alexandre Courbot wrote:
>> +    // GSP sequencer delay payload structure.
>> +    GSP_SEQ_BUF_PAYLOAD_DELAY_US,
>> +
>> +    // GSP sequencer register payload structures.
>> +    GSP_SEQ_BUF_PAYLOAD_REG_MODIFY,
>> +    GSP_SEQ_BUF_PAYLOAD_REG_POLL,
>> +    GSP_SEQ_BUF_PAYLOAD_REG_STORE,
>> +    GSP_SEQ_BUF_PAYLOAD_REG_WRITE, //
>> These ones are a bit trickier to abstract. Since they ever only use
> `bar` from the sequencer, I guess we can have their semantics in the
> `fw` module, exposed through a method that receives the `bar`? That way
> the sequencer won't have to access their members which are private to
> it.
The sequencer does need access to the private fields, because the logic of what
to write to the bar should be in the sequencer, and that logic depends on the
fields.

Example:

impl GspSeqCmdRunner for fw::GSP_SEQ_BUF_PAYLOAD_REG_MODIFY {
    fn run(&self, sequencer: &GspSequencer<'_>) -> Result {
        let addr = self.addr as usize;
        if let Ok(temp) = sequencer.bar.try_read32(addr) {
            let _ = sequencer
                .bar
                .try_write32((temp & !self.mask) | self.val, addr);
        }
        Ok(())
    }
}

Here, the sequencer needs access to `.addr`, `.mask` and `.val` to craft the
address and the value to write.

I could expose access to those fields as functions, but I think we should not
move sequencer logic to fw.rs, that should live in the sequencer.

Or am I missing something? thanks,

 - Joel