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

[Alexandre Courbot]

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@xxxxxxxxxx>
> Signed-off-by: Alistair Popple <apopple@xxxxxxxxxx>
> Signed-off-by: Joel Fernandes <joelagnelf@xxxxxxxxxx>
> ---
>  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.