Nouveau - Nov 2025 - [PATCH 03/11] gpu: nova-core: support header parsing on Turing/GA100

On Tue, Nov 18, 2025 at 12:52:47AM +0000, Timur Tabi wrote:
[...] 
> > > +
> > > ???????? Ok(Self {
> > > -??????????? imem_sec_load_target: FalconLoadTarget {
> > > -??????????????? src_start: app0.offset,
> > > -??????????????? dst_start: 0,
> > > -??????????????? len: app0.len,
> > > +??????????? imem_sec_load_target: if chipset > Chipset::GA100
{
> > > +??????????????? FalconLoadTarget {
> > > +??????????????????? src_start: app0.offset,
> > > +??????????????????? dst_start: 0,
> > > +??????????????????? len: app0.len,
> > > +??????????????? }
> > > +??????????? } else {
> > > +??????????????? FalconLoadTarget {
> > > +??????????????????? src_start: load_hdr.os_code_size,
> > > +??????????????????? dst_start: app0.offset,
> > > +??????????????????? len: app0.len,
> > > +??????????????? }
> > 
> > Can write more succinctly:
> > 
> > ? imem_sec_load_target: FalconLoadTarget {
> > ????? src_start: match chipset > Chipset::GA100? {
> > ????????? true => app0.offset,
> > ????????? false => load_hdr.os_code_size,
> > ????? },
> > ????? dst_start: match chipset > Chipset::GA100 {
> > ????????? true => 0,
> > ????????? false => app0.offset,
> > ????? 
> > ????? len:
app0.len,???????????????????????????????????????????????????????????????????????????
> > ? },
> 
> Do we really want to use "match" instead of "if", just
because we don't need "else"?
I don't care about the if/else as much as I care about the opportunity to
just specify FalconLoadTarget once instead twice. I think the match here is
cleaner for this
snippet, but I am Ok with the if/else as well.

Something like:
   imem_sec_load_target: FalconLoadTarget {
       src_start: if chipset > Chipset::GA100  {
          app0.offset
      } else {
           load_hdr.os_code_size
      },

That would be one more line of code, but pretty much the same.

thanks,

 - Joel

On Mon, 2025-11-17 at 20:04 -0500, Joel Fernandes wrote:
> I don't care about the if/else as much as I care about the opportunity
to
> just specify FalconLoadTarget once instead twice. I think the match here is
cleaner for this
> snippet, but I am Ok with the if/else as well.
> 
> Something like:
> ?? imem_sec_load_target: FalconLoadTarget {
> ?????? src_start: if chipset > Chipset::GA100? {
> ????????? app0.offset
> ????? } else {
> ?????????? load_hdr.os_code_size
> ????? },
> 
> That would be one more line of code, but pretty much the same.
Interesting.  I would have thought that duplicating the if-statement is the
higher offense.

On 11/17/25 5:04 PM, Joel Fernandes wrote:
> On Tue, Nov 18, 2025 at 12:52:47AM +0000, Timur Tabi wrote:
> [...] 
>>>> +
>>>> ???????? Ok(Self {
>>>> -??????????? imem_sec_load_target: FalconLoadTarget {
>>>> -??????????????? src_start: app0.offset,
>>>> -??????????????? dst_start: 0,
>>>> -??????????????? len: app0.len,
>>>> +??????????? imem_sec_load_target: if chipset >
Chipset::GA100 {
>>>> +??????????????? FalconLoadTarget {
>>>> +??????????????????? src_start: app0.offset,
>>>> +??????????????????? dst_start: 0,
>>>> +??????????????????? len: app0.len,
>>>> +??????????????? }
>>>> +??????????? } else {
>>>> +??????????????? FalconLoadTarget {
>>>> +??????????????????? src_start: load_hdr.os_code_size,
>>>> +??????????????????? dst_start: app0.offset,
>>>> +??????????????????? len: app0.len,
>>>> +??????????????? }
>>>
>>> Can write more succinctly:
>>>
>>> ? imem_sec_load_target: FalconLoadTarget {
>>> ????? src_start: match chipset > Chipset::GA100? {
>>> ????????? true => app0.offset,
>>> ????????? false => load_hdr.os_code_size,
>>> ????? },
>>> ????? dst_start: match chipset > Chipset::GA100 {
>>> ????????? true => 0,
>>> ????????? false => app0.offset,
>>> ????? 
>>> ????? len:
app0.len,???????????????????????????????????????????????????????????????????????????
>>> ? },
>>
>> Do we really want to use "match" instead of "if",
just because we don't need "else"?
> 
> I don't care about the if/else as much as I care about the opportunity
to
> just specify FalconLoadTarget once instead twice. I think the match here is
cleaner for this
> snippet, but I am Ok with the if/else as well.
> 
> Something like:
>    imem_sec_load_target: FalconLoadTarget {
>        src_start: if chipset > Chipset::GA100  {
>           app0.offset
>       } else {
>            load_hdr.os_code_size
>       },
> 
> That would be one more line of code, but pretty much the same.
> 
You know, this latest snippet looks a little better. The pattern of 

    match a > b {
	true => foo,
        false => bar,
    }

is actually not as nice as an if-else, because, well, anytime you
explicitly compare against true and false, you are likely doing 
something that the language (any language) has a construct for.

And in this case, it appears to be "if/else". :)


thanks,
-- 
John Hubbard