Re: [PATCH v2 06/10] gpu: nova-core: convert PDISP registers to kernel register macro

[Alexandre Courbot]

On Sat Mar 21, 2026 at 2:33 AM JST, Joel Fernandes wrote:
>
>
> On 3/20/2026 8:19 AM, Alexandre Courbot wrote:
>> Convert all PDISP registers to use the kernel's register macro and
>> update the code accordingly.
>> 
>> Reviewed-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>> ---
>>  drivers/gpu/nova-core/fb.rs   |  6 +++++-
>>  drivers/gpu/nova-core/regs.rs | 12 ++++++++----
>>  2 files changed, 13 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
>> index 6536d0035cb1..62fc90fa6a84 100644
>> --- a/drivers/gpu/nova-core/fb.rs
>> +++ b/drivers/gpu/nova-core/fb.rs
>> @@ -8,6 +8,7 @@
>>  use kernel::{
>>      device,
>>      fmt,
>> +    io::Io,
>>      prelude::*,
>>      ptr::{
>>          Alignable,
>> @@ -189,7 +190,10 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>>                  let base = fb.end - NV_PRAMIN_SIZE;
>>  
>>                  if hal.supports_display(bar) {
>> -                    match regs::NV_PDISP_VGA_WORKSPACE_BASE::read(bar).vga_workspace_addr() {
>> +                    match bar
>> +                        .read(regs::NV_PDISP_VGA_WORKSPACE_BASE)
>> +                        .vga_workspace_addr()
>> +                    {
>>                          Some(addr) => {
>>                              if addr < base {
>>                                  const VBIOS_WORKSPACE_SIZE: u64 = usize_as_u64(SZ_128K);
>> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
>> index 61a8dba22d88..b051d5568cd8 100644
>> --- a/drivers/gpu/nova-core/regs.rs
>> +++ b/drivers/gpu/nova-core/regs.rs
>> @@ -250,10 +250,14 @@ pub(crate) fn usable_fb_size(self) -> u64 {
>>  
>>  // PDISP
>>  
>> -register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
>> -    3:3     status_valid as bool, "Set if the `addr` field is valid";
>> -    31:8    addr as u32, "VGA workspace base address divided by 0x10000";
>> -});
>> +io::register! {
>> +    pub(crate) NV_PDISP_VGA_WORKSPACE_BASE(u32) @ 0x00625f04 {
>> +        /// VGA workspace base address divided by 0x10000.
>> +        31:8    addr;
>> +        /// Set if the `addr` field is valid.
>> +        3:3     status_valid => bool;
>> +    }
>> +}
>
> Shouldn't this re-ordering of bit ranges be a separate patch? Also, what did we
> conclude on the ordering issue? I remember this was discussed, but I am not sure
> what the conclusion was.

The conclusion was descending order for both bitfields and the bits
themselves. This is part of the contract for using the `register!`
macro, so I don't think it is out of place in this series (and
re-shuffling things again won't make this diff smaller or more readable,
so I don't think there is a benefit).