Re: [PATCH v10 03/11] drm/etnaviv: Add dedicated functions to create and destroy platform device

From: Sui Jingfeng
Date: Wed Jun 21 2023 - 12:12:17 EST


Hi

On 2023/6/21 23:20, Lucas Stach wrote:
Am Mittwoch, dem 21.06.2023 um 22:35 +0800 schrieb Sui Jingfeng:
Hi,

On 2023/6/21 22:00, Lucas Stach wrote:
Am Mittwoch, dem 21.06.2023 um 21:31 +0800 schrieb Sui Jingfeng:
On 2023/6/21 18:23, Lucas Stach wrote:
While back to the question you ask, I want etnaviv_create_platform_device() to be generic,

can be used by multiple place for multiple purpose.

I have successfully copy this to a another drm driver by simply renaming.

The body of the function itself does not need to change.
But it isn't shared,
This can be shared for drm/etnaviv in the future,

currently, we just need an opportunity to use this function.

I'm not convinced, yet.

I want to create a dummy platform device,

let this dummy platform be bound to the single PCI GPU master.


etnaviv_create_platform_device("dummy", &dummy_device);


1) To verify the component code path on PCI case.

My favorite option would be to just always use the component path even
when the GPU is on a PCI device to keep both paths mostly aligned. One
could easily image both a 3D and a 2D core being made available though
the same PCI device.
Component is for something that is possible not available. (or something
is optional)

Yes it provided flexibly, but don't forget, it rely on the DT.
The component framework itself doesn't rely on DT in any way.

Yes I know that, for example the HDMI audio stuff.

But *your implement* do rely on the DT, this is the point

By
providing a appropriate match function you can make it work with any
kind of device.
Yes, you are right.
In fact etnaviv supports platform devices instantiated
via board code today.
Nice,
They don't need to come from DT.
What about the various clock, sir?
If we could make the PCI stuff work the same way, that would be my
preferred option.


But for the PCIe device, it always the case that all of the hardware is
available at the same time

when the device driver(kernel module) is loaded.
That isn't the issue solved by the component framework. On the existing
SoCs all the hardware is available when the driver is probed. The
component framework just makes sure that we only expose the DRM device
after all GPU cores that should be managed by a single DRM device
instance are probed.

One could easily image a PCI device that containing a 2D and a 3D
Vivante GPU that should be made available through a single DRM device.
In that case you'll also need to use the component framework.


2) Possibly for create a device for some other tiny hardware logic
come with the platform

Do you have something in mind here? Until now I assumed that only the
GPU core is behind the PCI abstraction. Is there something else sharing
the MMIO space?
A display controller, HDMI phy, vga encoder etc


I have a discrete PCIe GPU card from another vendor,

It integrated display controller and vivante GPU and unknown VPUs.

All of the  hardware block mentioned above sharing the MMIO space.

There are available on the same time when you mount this discrete PCIe
GPU card on the mother board

But they surely should not all be made available through the etnaviv
driver. Etnaviv deals with the Vivante GPUs. If you have a PCI device
with multiple IP cores behind the shared MMIO space you should have a
PCI driver instantiating platform devices so the respective drivers for
those IP cores can bind to the platform device.

I have only one PCI device, let start from the simple case, OK?

I admire your fantastic idea.

let deal with it another patch in the future if such hardware emerged.

Accept the current implement, please ?

Etnaviv is not that
driver.

Yeah, but I notice that there is chipFeatures_DC defined in common.xml.h

I don't know how does this going to used, if a hardware marked it as true.

Regards,
Lucas

Regards,
Lucas

3) Revival component_compare_dev_name() function.

in this compilation unit this function is specific
to the etnaviv driver and I don't see why we shouldn't have etnaviv
specifics in there if it makes the code of this driver easier to
follow.

--
Jingfeng