On Tue, 2025-09-30 at 23:16 +1000, Alistair Popple wrote:
> From: Joel Fernandes <joelagnelf@nvidia.com>
>
> Add definition for RISCV_CPUCTL register and use it in a new falcon API
> to check if the RISC-V core of a Falcon is active. It is required by
> the sequencer to know if the GSP's RISCV processor is active.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
You didn't make any of my previously suggested changes.
> ---
> drivers/gpu/nova-core/falcon.rs | 9 +++++++++
> drivers/gpu/nova-core/regs.rs | 5 +++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 37e6298195e4..c7907f16bcf4 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -610,4 +610,13 @@ pub(crate) fn signature_reg_fuse_version(
> self.hal
> .signature_reg_fuse_version(self, bar, engine_id_mask, ucode_id)
> }
> +
> + /// Check if the RISC-V core is active.
> + ///
> + /// Returns `true` if the RISC-V core is active, `false` otherwise.
> + #[expect(unused)]
> + pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> Result<bool> {
> + let cpuctl = regs::NV_PRISCV_RISCV_CPUCTL::read(bar, &E::ID);
> + Ok(cpuctl.active_stat())
> + }
This should return just bool, not Result<bool>. Either it's active or it's not. There is no third
option.
> }
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 0585699ae951..5df6a2bf42ad 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -324,6 +324,11 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
>
> // PRISCV
>
> +register!(NV_PRISCV_RISCV_CPUCTL @ PFalconBase[0x00001388] {
> + 7:7 active_stat as bool;
> + 0:0 halted as bool;
> +});
I think the convention is to list the bits in increase position. That is, 'active_stat' should be
on the line below 'halted''
Also I think that this should actually be PFalcon2Base[0x00000388]
> +
> register!(NV_PRISCV_RISCV_BCR_CTRL @ PFalconBase[0x00001668] {
> 0:0 valid as bool;
> 4:4 core_select as bool => PeregrineCoreSelect;
And this should be PFalcon2Base[0x00000668]