Re: [RFC PATCH 0/3] gpu: nova-core: add basic timer subdevice implementation

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Feb 17, 2025 at 04:48:13PM +0100, Simona Vetter wrote:
> On Mon, Feb 17, 2025 at 11:04:45PM +0900, Alexandre Courbot wrote:
> > Hi everyone,
> > 
> > This short RFC is based on top of Danilo's initial driver stub series
> > [1] and has for goal to initiate discussions and hopefully some design
> > decisions using the simplest subdevice of the GPU (the timer) as an
> > example, before implementing more devices allowing the GPU
> > initialization sequence to progress (Falcon being the logical next step
> > so we can get the GSP rolling).
> > 
> > It is kept simple and short for that purpose, and to avoid bumping into
> > a wall with much more device code because my assumptions were incorrect.
> > 
> > This is my first time trying to write Rust kernel code, and some of my
> > questions below are probably due to me not understanding yet how to use
> > the core kernel interfaces. So before going further I thought it would
> > make sense to raise the most obvious questions that came to my mind
> > while writing this draft:
> > 
> > - Where and how to store subdevices. The timer device is currently a
> >   direct member of the GPU structure. It might work for GSP devices
> >   which are IIUC supposed to have at least a few fixed devices required
> >   to bring the GSP up ; but as a general rule this probably won't scale
> >   as not all subdevices are present on all GPU variants, or in the same
> >   numbers. So we will probably need to find an equivalent to the
> >   `subdev` linked list in Nouveau.
> > 
> > - BAR sharing between subdevices. Right now each subdevice gets access
> >   to the full BAR range. I am wondering whether we could not split it
> >   into the relevant slices for each-subdevice, and transfer ownership of
> >   each slice to the device that is supposed to use it. That way each
> >   register would have a single owner, which is arguably safer - but
> >   maybe not as flexible as we will need down the road?
> > 
> > - On a related note, since the BAR is behind a Devres its availability
> >   must first be secured before any hardware access using try_access().
> >   Doing this on a per-register or per-operation basis looks overkill, so
> >   all methods that access the BAR take a reference to it, allowing to
> >   call try_access() from the highest-level caller and thus reducing the
> >   number of times this needs to be performed. Doing so comes at the cost
> >   of an extra argument to most subdevice methods ; but also with the
> >   benefit that we don't need to put the BAR behind another Arc and share
> >   it across all subdevices. I don't know which design is better here,
> >   and input would be very welcome.
> > 
> > - We will probably need sometime like a `Subdevice` trait or something
> >   down the road, but I'll wait until we have more than one subdevice to
> >   think about it.
> 
> It might make sense to go with a full-blown aux bus. Gives you a lot of
> structures and answers to these questions, but also might be way too much.

No, it's not too much, that's exactly what the auxbus code is for
(splitting a real device into child ones where they all share the same
physical resources.)  So good suggestion.

thanks,

greg k-h