On Tue Feb 18, 2025 at 5:07 PM JST, Greg KH wrote: > 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. Dave's comments have somehow convinced me that we probably won't need to do something as complex as I initially planned, so hopefully it won't come to that. :) But thanks for the suggestion, I'll keep it in mind just in case.
