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

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

On Wed Nov 12, 2025 at 7:06 AM JST, Joel Fernandes
wrote:
>
>
> 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.
Yeah although I floated the idea I have to admit I am not a big fan of
that either. So I guess we could have accessor functions for the fields,
so the `GspSeqCmdRunner` implementation stays in the sequencer?

It will at least provide the level of abstraction we require against the
firmware types' internal structure.