Re: [PATCH 3/9] gpu: nova-core: gsp: simplify sequencer opcode parsing

[lyude]

Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>

On Mon, 2025-12-08 at 18:26 +0900, Alexandre Courbot wrote:
> The opcodes are already the right type in the C union, so we can use
> them directly instead of converting them to a byte stream and back
> again
> using `FromBytes`.
> 
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gsp/fw.rs | 40 +++++--------------------------
> ---------
>  1 file changed, 5 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-
> core/gsp/fw.rs
> index 24e4eaaf1265..d06c0fdd6154 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -472,13 +472,7 @@ pub(crate) fn reg_write_payload(&self) ->
> Result<RegWritePayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegWrite`, so union
> contains valid `RegWritePayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regWrite).cast::<u8>(),
> -                core::mem::size_of::<RegWritePayload>(),
> -            )
> -        };
> -       
> Ok(*RegWritePayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegWritePayload(unsafe { self.0.payload.regWrite }))
>      }
>  
>      /// Returns the register modify payload by value.
> @@ -489,13 +483,7 @@ pub(crate) fn reg_modify_payload(&self) ->
> Result<RegModifyPayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegModify`, so union
> contains valid `RegModifyPayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regModify).cast::<u8>(),
> -                core::mem::size_of::<RegModifyPayload>(),
> -            )
> -        };
> -       
> Ok(*RegModifyPayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegModifyPayload(unsafe { self.0.payload.regModify }))
>      }
>  
>      /// Returns the register poll payload by value.
> @@ -506,13 +494,7 @@ pub(crate) fn reg_poll_payload(&self) ->
> Result<RegPollPayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegPoll`, so union
> contains valid `RegPollPayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regPoll).cast::<u8>(),
> -                core::mem::size_of::<RegPollPayload>(),
> -            )
> -        };
> -       
> Ok(*RegPollPayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegPollPayload(unsafe { self.0.payload.regPoll }))
>      }
>  
>      /// Returns the delay payload by value.
> @@ -523,13 +505,7 @@ pub(crate) fn delay_us_payload(&self) ->
> Result<DelayUsPayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `DelayUs`, so union
> contains valid `DelayUsPayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.delayUs).cast::<u8>(),
> -                core::mem::size_of::<DelayUsPayload>(),
> -            )
> -        };
> -       
> Ok(*DelayUsPayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(DelayUsPayload(unsafe { self.0.payload.delayUs }))
>      }
>  
>      /// Returns the register store payload by value.
> @@ -540,13 +516,7 @@ pub(crate) fn reg_store_payload(&self) ->
> Result<RegStorePayload> {
>              return Err(EINVAL);
>          }
>          // SAFETY: Opcode is verified to be `RegStore`, so union
> contains valid `RegStorePayload`.
> -        let payload_bytes = unsafe {
> -            core::slice::from_raw_parts(
> -               
> core::ptr::addr_of!(self.0.payload.regStore).cast::<u8>(),
> -                core::mem::size_of::<RegStorePayload>(),
> -            )
> -        };
> -       
> Ok(*RegStorePayload::from_bytes(payload_bytes).ok_or(EINVAL)?)
> +        Ok(RegStorePayload(unsafe { self.0.payload.regStore }))
>      }
>  }
>