Re: [PATCH RFC v2 1/3] gpu: nova-core: register: use field type for Into implementation

[Joel Fernandes <joelagnelf@nvidia.com>]

On 10/10/2025 4:26 AM, Alexandre Courbot wrote:
> On Fri Oct 10, 2025 at 5:24 PM JST, Alexandre Courbot wrote:
>> On Fri Oct 10, 2025 at 12:41 AM JST, Joel Fernandes wrote:
>>>
>>>
>>>> On Oct 9, 2025, at 11:17 AM, Joel Fernandes <joelagnelf@nvidia.com> wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>>> On 10/9/2025 8:37 AM, Alexandre Courbot wrote:
>>>>> The getter method of a field works with the field type, but its setter
>>>>> expects the type of the register. This leads to an asymmetry in the
>>>>> From/Into implementations required for a field with a dedicated type.
>>>>> For instance, a field declared as
>>>>>
>>>>>    pub struct ControlReg(u32) {
>>>>>        3:0 mode as u8 ?=> Mode;
>>>>>        ...
>>>>>    }
>>>>>
>>>>> currently requires the following implementations:
>>>>>
>>>>>    impl TryFrom<u8> for Mode {
>>>>>      ...
>>>>>    }
>>>>>
>>>>>    impl From<Mode> for u32 {
>>>>>      ...
>>>>>    }
>>>>>
>>>>> Change this so the `From<Mode>` now needs to be implemented for `u8`,
>>>>> i.e. the primitive type of the field. This is more consistent, and will
>>>>> become a requirement once we start using the TryFrom/Into derive macros
>>>>> to implement these automatically.
>>>>>
>>>>> Reported-by: Edwin Peer <epeer@nvidia.com>
>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>>
>>>> As these are incremental improvements, could you please rebase on top of the v6
>>>> bitfield series so it does not conflict?
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/log/?h=nova.dev.bitfield.submitted.v6
>>>
>>> On second thought, I could just carry this patch on top of my v6 series and avoid too much conflict.
>>>
>>> So if it is ok with you, please only carry the last 2 patches of this series whenever applying.
>>>
>>> For this patch:
>>> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
>>>
>>> I will review the other two patches as well. Thanks.
>>
>> The idea is for this patch to go *before* your series, to avoid the
>> asymmetry in the From/Into implementions of bitfields. We could also put
>> it after, but it would become larger as a result and I think it can be
>> merge soon after -rc1 is tagged anyway.
> 
> I forgot to mention that this patch is not really part of the series ;
> it's just here to allow it to apply on top of drm-rust-next should
> anyone want to try it.

These are incremental improvements, I don't see the point in making the
improvements before bitfield extraction within Nova. Very least, we can apply
the bitfield extraction within nova and then make the improvements (within
nova), since the extraction work happened well before (and it was a non-trivial
amount of work), before any of these improvements. Let us work together to make
sure we don't have to throw away old work due to new patches please. :)

thanks,

 - Joel