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