Re: [etnaviv-next v13 7/7] drm/etnaviv: Add support for vivante GPU cores attached via PCI(e)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,


On 2024/2/9 19:02, Daniel Vetter wrote:
On Thu, Feb 08, 2024 at 04:27:02PM +0100, Maxime Ripard wrote:
On Wed, Feb 07, 2024 at 10:35:49AM +0100, Daniel Vetter wrote:
On Wed, Feb 07, 2024 at 01:27:59AM +0800, Sui Jingfeng wrote:
The component helper functions are the glue, which is used to bind multiple
GPU cores to a virtual master platform device. Which is fine and works well
for the SoCs who contains multiple GPU cores.

The problem is that usperspace programs (such as X server and Mesa) will
search the PCIe device to use if it is exist. In other words, usperspace
programs open the PCIe device with higher priority. Creating a virtual
master platform device for PCI(e) GPUs is unnecessary, as the PCI device
has been created by the time drm/etnaviv is loaded.

we create virtual platform devices as a representation for the vivante GPU
ip core. As all of subcomponent are attached via the PCIe master device,
we reflect this hardware layout by binding all of the virtual child to the
the real master.

Signed-off-by: Sui Jingfeng <sui.jingfeng@xxxxxxxxx>
Uh so my understanding is that drivers really shouldn't create platform
devices of their own. For this case here I think the aux-bus framework is
the right thing to use. Alternatively would be some infrastructure where
you feed a DT tree to driver core or pci subsystem and it instantiates it
all for you correctly, and especially with hotunplug all done right since
this is pci now, not actually part of the soc that cannot be hotunplugged.
I don't think we need intermediate platform devices at all. We just need
to register our GPU against the PCI device and that's it. We don't need
a platform device, we don't need the component framework.
Afaik that's what this series does. The component stuff is for the
internal structure of the gpu ip, so that the same modular approach that
works for arm-soc also works for pci chips.

Otherwise we end up with each driver hand-rolling that stuff, which is
defacto what both nouveau and amdgpu do (intel hw is too much a mess for
that component-driver based approach to actually work reasonably well).


Emmm, I spend years to achieve this, and only to find that you have fully
understand my patch within two days.


Cheers, Sima

I think I've seen some other pci devices from arm soc designs that would
benefit from this too, so lifting this logic into a pci function would
make sense imo.


Yes, as you said, we are trying to avoid the hand-rolling stuff.
I guess, extremely advanced drivers(like i915, amdgpu, and nouveau)
won't need this. So I'm not sure if lifting this logic into PCI
core would benefit to other people. While investigating the aux-bus
framework a few days, I find it is not as concise as this one. It
introduce a lot of new structure, I fear that it may cause namespace
pollution if adopt it. So, I thinks I should choose the alternative
way.

While taking a lot from contribution, we are really want to do some
feedback(pay-back). We are happy if there are other users(or new
drivers) would like to adopt this idea, I think, the idea itself
has already been conveyed. Which probably can be seen as a trivial
contribution. Other programmer are free to copy and modify.

But as a initial commit, I minimized the mount of changes  as required
by Locus. meanwhile, I'm willing to following the expectation in the
long term. If there are other users or other problem need to solve,
I would like help to improve and to cooperate to testing in the future.




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux