This patch adds PCI driver wrapper on the top of what drm/etnaviv already
have, with the expectation that to keep component framework untoughed and
to preserve code sharing.
drm/etnaviv use the component framework to bind multiple GPU cores to a
virtual master platform device, the virtual master is create during driver
load time. For the PCI(e) device who integrate vivante GPU ip, the process
to going to be reverse. Create virtual platform device as a representation
for the vivante GPU ip core attached on the real PCIe master. The master
already there by the time the driver is loaded, what we need to do is to
devide the big MMIO resource and assign it tp the child. All the virtual
childs are bind to the master with component framework.
v6:
* Fix build issue on system without CONFIG_PCI enabled
v7:
* Add a separate patch for the platform driver rearrangement (Bjorn)
* Switch to runtime check if the GPU is dma coherent or not (Lucas)
* Add ETNAVIV_PARAM_GPU_COHERENT to allow userspace to query (Lucas)
* Remove etnaviv_gpu.no_clk member (Lucas)
* Various Typos and coding style fixed (Bjorn)
v8:
* Fix typos and remove unnecessary header included (Bjorn).
* Add a dedicated function to create the virtual master platform
device.
v9:
* Use PCI_VDEVICE() macro (Bjorn)
* Add trivial stubs for the PCI driver (Bjorn)
* Remove a redundant dev_err() usage (Bjorn)
* Clean up etnaviv_pdev_probe() with etnaviv_of_first_available_node()
v10:
* Add one more cleanup patch
* Resolve the conflict with a patch from Rob
* Make the dummy PCI stub inlined
* Print only if the platform is dma-coherrent
V11:
* Drop unnecessary changes (Lucas)
* Tweak according to other reviews of v10.
V12:
* Create a virtual platform device for the subcomponent GPU cores
* Bind all subordinate GPU cores to the real PCI master via component.
V13:
* Drop the non-component code path (side-by-side implement), always
use the component framework to bind subcomponent GPU core. Even
though this is only one core.
* Defer the irq handler register.
* Rebase and improve the commit message
The whole patch series have been tested on X86-64 and LoongArch platform.
Tested with JD9230P GPU, only brief product overview is available[1].
[1] http://www.goldendisk.cn/index.php?s=index/show/index&id=273
$ dmesg | grep etnaviv:
etnaviv 0000:05:00.0: JingJia Micro JD9230P has 2 GPU cores
etnaviv 0000:05:00.0: bound etnaviv-gpu,3d.0 (ops gpu_ops [etnaviv])
etnaviv 0000:05:00.0: bound etnaviv-gpu,3d.1 (ops gpu_ops [etnaviv])
etnaviv-gpu etnaviv-gpu,3d.0: model: GC9200, revision: 6304
etnaviv-gpu etnaviv-gpu,3d.1: model: GC9200, revision: 6304
[drm] Initialized etnaviv 1.3.0 20151214 for 0000:05:00.0 on minor 0
$ cd /sys/kernel/debug/dri/0
$ cat gpu
etnaviv-gpu,3d.0 Status:
identity
model: 0x9200
revision: 0x6304
product_id: 0x92004
customer_id: 0x49
eco_id: 0x0
features
major_features: 0xe0287cad
minor_features0: 0xc9799eff
minor_features1: 0xfefbfadb
minor_features2: 0xeb9d6fbf
minor_features3: 0xedfffcfd
minor_features4: 0xdb0dafc7
minor_features5: 0xbb5ac733
minor_features6: 0x00000000
minor_features7: 0x00000000
minor_features8: 0x00000000
minor_features9: 0x00000000
minor_features10: 0x00000000
minor_features11: 0x00000000
specs
stream_count: 16
register_max: 64
thread_count: 4096
vertex_cache_size: 16
shader_core_count: 16
nn_core_count: 0
pixel_pipes: 1
vertex_output_buffer_size: 1024
buffer_size: 0
instruction_count: 256
num_constants: 320
varyings_count: 16
axi: 0x00000000
idle: 0x7fffffff
DMA seems to be stuck
address 0: 0xccd2a7e8
address 1: 0xccd2a7e8
state 0: 0x00000000
state 1: 0x00000000
last fetch 64 bit word: 0x08ddbdb6 0x85ec1955
etnaviv-gpu,3d.1 Status:
identity
model: 0x9200
revision: 0x6304
product_id: 0x92004
customer_id: 0x49
eco_id: 0x0
features
major_features: 0xe0287cad
minor_features0: 0xc9799eff
minor_features1: 0xfefbfadb
minor_features2: 0xeb9d6fbf
minor_features3: 0xedfffcfd
minor_features4: 0xdb0dafc7
minor_features5: 0xbb5ac733
minor_features6: 0x00000000
minor_features7: 0x00000000
minor_features8: 0x00000000
minor_features9: 0x00000000
minor_features10: 0x00000000
minor_features11: 0x00000000
specs
stream_count: 16
register_max: 64
thread_count: 4096
vertex_cache_size: 16
shader_core_count: 16
nn_core_count: 0
pixel_pipes: 1
vertex_output_buffer_size: 1024
buffer_size: 0
instruction_count: 256
num_constants: 320
varyings_count: 16
axi: 0x00000000
idle: 0x7fffffff
DMA seems to be stuck
address 0: 0x5756fef8
address 1: 0x5756fef8
state 0: 0x00000000
state 1: 0x00000000
last fetch 64 bit word: 0x68de88ef 0xcffa3d57
Tested with LingJiu GP102 GPU, only brief product overview is available[2].
[2] http://www.goldendisk.cn/index.php?s=index/show/index&id=3028
$ dmesg | grep etnaviv:
etnaviv 0000:06:00.0: LingJiu GP102 has 2 GPU cores
etnaviv 0000:06:00.0: bound etnaviv-gpu,3d.0 (ops gpu_ops [etnaviv])
etnaviv 0000:06:00.0: bound etnaviv-gpu,2d.0 (ops gpu_ops [etnaviv])
etnaviv-gpu etnaviv-gpu,3d.0: model: GC860, revision: 4601
etnaviv-gpu etnaviv-gpu,2d.0: model: GC300, revision: 2000
[drm] Initialized etnaviv 1.3.0 20151214 for 0000:06:00.0 on minor 0
$ cd /sys/kernel/debug/dri/0
$ cat gpu
etnaviv-gpu,3d.0 Status:
identity
model: 0x860
revision: 0x4601
product_id: 0x0
customer_id: 0x0
eco_id: 0x0
features
major_features: 0xe02c2ced
minor_features0: 0xcbf99fff
minor_features1: 0x00000001
minor_features2: 0x00000000
minor_features3: 0x00000000
minor_features4: 0x00000000
minor_features5: 0x00000000
minor_features6: 0x00000000
minor_features7: 0x00000000
minor_features8: 0x00000000
minor_features9: 0x00000000
minor_features10: 0x00000000
minor_features11: 0x00000000
specs
stream_count: 1
register_max: 64
thread_count: 256
vertex_cache_size: 8
shader_core_count: 1
nn_core_count: 0
pixel_pipes: 1
vertex_output_buffer_size: 512
buffer_size: 0
instruction_count: 256
num_constants: 168
varyings_count: 8
axi: 0x00000000
idle: 0x7fffffff
DMA seems to be stuck
address 0: 0x00000000
address 1: 0x00000000
state 0: 0x00000000
state 1: 0x00000000
last fetch 64 bit word: 0x00000000 0x00000000
etnaviv-gpu,2d.0 Status:
identity
model: 0x300
revision: 0x2000
product_id: 0x0
customer_id: 0x0
eco_id: 0x0
features
major_features: 0xe02c2ee9
minor_features0: 0xcbf99fff
minor_features1: 0x00000001
minor_features2: 0x00000000
minor_features3: 0x00000000
minor_features4: 0x00000000
minor_features5: 0x00000000
minor_features6: 0x00000000
minor_features7: 0x00000000
minor_features8: 0x00000000
minor_features9: 0x00000000
minor_features10: 0x00000000
minor_features11: 0x00000000
specs
stream_count: 1
register_max: 64
thread_count: 256
vertex_cache_size: 8
shader_core_count: 1
nn_core_count: 0
pixel_pipes: 1
vertex_output_buffer_size: 512
buffer_size: 0
instruction_count: 256
num_constants: 168
varyings_count: 8
axi: 0x00000000
idle: 0x7fffffff
DMA seems to be stuck
address 0: 0x00000000
address 1: 0x00000000
state 0: 0x00000000
state 1: 0x00000000
last fetch 64 bit word: 0x00000000 0x00000000
Sui Jingfeng (7):
drm/etnaviv: Add a helper function to get clocks
drm/etnaviv: Add constructor and destructor for the
etnaviv_drm_private struct
drm/etnaviv: Embed struct drm_device in struct etnaviv_drm_private
drm/etnaviv: Add support for cached coherent caching mode
drm/etnaviv: Replace the '&pdev->dev' with 'dev'
drm/etnaviv: Update the implement of etnaviv_create_platform_device()
drm/etnaviv: Add support for vivante GPU cores attached via PCI(e)
drivers/gpu/drm/etnaviv/Kconfig | 8 +
drivers/gpu/drm/etnaviv/Makefile | 2 +
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 157 ++++++++-----
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 24 ++
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 22 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 146 ++++++++----
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 4 +
drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 4 +-
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 224 +++++++++++++++++++
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 18 ++
include/uapi/drm/etnaviv_drm.h | 1 +
12 files changed, 502 insertions(+), 110 deletions(-)
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
--
2.34.1
Because the current implementation is DT-based, this only works when the
host platform has the DT support. The problem is that some host platforms
does not provide DT-based clocks drivers, as a result, the driver rage
quit. For example, the X86-64 and LoongArch desktop platform.
Typically, PLL hardware is provided by the host platform, which is part
of the entire clock tree, the PLL hardware provide clock pulse to the GPU
core, but it's not belong to the GPU core. PLL registers can be manipulated
directly by the device driver. Hence, add a dedicated helper function to
get the various clocks, which make it possible to call this function only
on the platform where DT support is available.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 53 ++++++++++++++++-----------
1 file changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index ce1e970f0209..654bf2631755 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1617,6 +1617,35 @@ static irqreturn_t irq_handler(int irq, void *data)
return ret;
}
+static int etnaviv_gpu_clk_get(struct etnaviv_gpu *gpu)
+{
+ struct device *dev = gpu->dev;
+
+ gpu->clk_reg = devm_clk_get_optional(dev, "reg");
+ DBG("clk_reg: %p", gpu->clk_reg);
+ if (IS_ERR(gpu->clk_reg))
+ return PTR_ERR(gpu->clk_reg);
+
+ gpu->clk_bus = devm_clk_get_optional(dev, "bus");
+ DBG("clk_bus: %p", gpu->clk_bus);
+ if (IS_ERR(gpu->clk_bus))
+ return PTR_ERR(gpu->clk_bus);
+
+ gpu->clk_core = devm_clk_get(dev, "core");
+ DBG("clk_core: %p", gpu->clk_core);
+ if (IS_ERR(gpu->clk_core))
+ return PTR_ERR(gpu->clk_core);
+ gpu->base_rate_core = clk_get_rate(gpu->clk_core);
+
+ gpu->clk_shader = devm_clk_get_optional(dev, "shader");
+ DBG("clk_shader: %p", gpu->clk_shader);
+ if (IS_ERR(gpu->clk_shader))
+ return PTR_ERR(gpu->clk_shader);
+ gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+
+ return 0;
+}
+
static int etnaviv_gpu_clk_enable(struct etnaviv_gpu *gpu)
{
int ret;
@@ -1892,27 +1921,9 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
}
/* Get Clocks: */
- gpu->clk_reg = devm_clk_get_optional(&pdev->dev, "reg");
- DBG("clk_reg: %p", gpu->clk_reg);
- if (IS_ERR(gpu->clk_reg))
- return PTR_ERR(gpu->clk_reg);
-
- gpu->clk_bus = devm_clk_get_optional(&pdev->dev, "bus");
- DBG("clk_bus: %p", gpu->clk_bus);
- if (IS_ERR(gpu->clk_bus))
- return PTR_ERR(gpu->clk_bus);
-
- gpu->clk_core = devm_clk_get(&pdev->dev, "core");
- DBG("clk_core: %p", gpu->clk_core);
- if (IS_ERR(gpu->clk_core))
- return PTR_ERR(gpu->clk_core);
- gpu->base_rate_core = clk_get_rate(gpu->clk_core);
-
- gpu->clk_shader = devm_clk_get_optional(&pdev->dev, "shader");
- DBG("clk_shader: %p", gpu->clk_shader);
- if (IS_ERR(gpu->clk_shader))
- return PTR_ERR(gpu->clk_shader);
- gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+ err = etnaviv_gpu_clk_get(gpu);
+ if (err)
+ return err;
/* TODO: figure out max mapped size */
dev_set_drvdata(dev, gpu);
--
2.34.1
There are a lot of data members in the struct etnaviv_drm_private, which
are intended to be shared by all GPU cores. Introduces two dedicated helper
functions for the purpose of construction and destruction.
After applied this patch, the error handling of the etnaviv_bind() gets
simplified a lot. Another potential benefit is that we can also put the
struct drm_device into struct etnaviv_drm_private in the future, as the
instance of struct drm_device is also unique across all GPU cores.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 70 +++++++++++++++++----------
1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 4bdfb1c81eff..035ac0c6884f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,6 +41,43 @@ static struct device_node *etnaviv_of_first_available_node(void)
return NULL;
}
+static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+{
+ struct etnaviv_drm_private *priv;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return ERR_PTR(-ENOMEM);
+
+ xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
+
+ mutex_init(&priv->gem_lock);
+ INIT_LIST_HEAD(&priv->gem_list);
+ priv->num_gpus = 0;
+ priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
+
+ priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
+ if (IS_ERR(priv->cmdbuf_suballoc)) {
+ kfree(priv);
+ dev_err(dev, "Failed to create cmdbuf suballocator\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return priv;
+}
+
+static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+{
+ if (!priv)
+ return;
+
+ etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
+
+ xa_destroy(&priv->active_contexts);
+
+ kfree(priv);
+}
+
static void load_gpu(struct drm_device *dev)
{
struct etnaviv_drm_private *priv = dev->dev_private;
@@ -521,35 +558,21 @@ static int etnaviv_bind(struct device *dev)
if (IS_ERR(drm))
return PTR_ERR(drm);
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv) {
- dev_err(dev, "failed to allocate private data\n");
- ret = -ENOMEM;
+ priv = etnaviv_alloc_private(dev);
+ if (IS_ERR(priv)) {
+ ret = PTR_ERR(priv);
goto out_put;
}
+
drm->dev_private = priv;
dma_set_max_seg_size(dev, SZ_2G);
- xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
-
- mutex_init(&priv->gem_lock);
- INIT_LIST_HEAD(&priv->gem_list);
- priv->num_gpus = 0;
- priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
-
- priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
- if (IS_ERR(priv->cmdbuf_suballoc)) {
- dev_err(drm->dev, "Failed to create cmdbuf suballocator\n");
- ret = PTR_ERR(priv->cmdbuf_suballoc);
- goto out_free_priv;
- }
-
dev_set_drvdata(dev, drm);
ret = component_bind_all(dev, drm);
if (ret < 0)
- goto out_destroy_suballoc;
+ goto out_free_priv;
load_gpu(drm);
@@ -561,10 +584,8 @@ static int etnaviv_bind(struct device *dev)
out_unbind:
component_unbind_all(dev, drm);
-out_destroy_suballoc:
- etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
out_free_priv:
- kfree(priv);
+ etnaviv_free_private(priv);
out_put:
drm_dev_put(drm);
@@ -580,12 +601,9 @@ static void etnaviv_unbind(struct device *dev)
component_unbind_all(dev, drm);
- etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
-
- xa_destroy(&priv->active_contexts);
+ etnaviv_free_private(priv);
drm->dev_private = NULL;
- kfree(priv);
drm_dev_put(drm);
}
--
2.34.1
Both the instance of struct drm_device and the instance of struct
etnaviv_drm_private are intended to be shared by all GPU cores. They
both have only one instance across drm/etnaviv, therefore, embed struct
drm_device into struct etnaviv_drm_private, and use container_of() to
retrieve pointer for the containing structure. The benifit is that the
DRM device can be initialized with devm_drm_dev_alloc() and the DRM device
created is automatically put on driver detach, so don't need us to call
drm_dev_put() explicitly on driver teardown. It's also eliminate the need
to use the .dev_private member, which is deprecated according to the drm
document.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 65 ++++++++------------
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 7 +++
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 6 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 6 +-
drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 4 +-
6 files changed, 40 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 035ac0c6884f..5ba2b3a386b3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,14 +41,9 @@ static struct device_node *etnaviv_of_first_available_node(void)
return NULL;
}
-static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+static int etnaviv_private_init(struct device *dev,
+ struct etnaviv_drm_private *priv)
{
- struct etnaviv_drm_private *priv;
-
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return ERR_PTR(-ENOMEM);
-
xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
mutex_init(&priv->gem_lock);
@@ -58,15 +53,14 @@ static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
if (IS_ERR(priv->cmdbuf_suballoc)) {
- kfree(priv);
dev_err(dev, "Failed to create cmdbuf suballocator\n");
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
}
- return priv;
+ return 0;
}
-static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+static void etnaviv_private_fini(struct etnaviv_drm_private *priv)
{
if (!priv)
return;
@@ -74,13 +68,11 @@ static void etnaviv_free_private(struct etnaviv_drm_private *priv)
etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
xa_destroy(&priv->active_contexts);
-
- kfree(priv);
}
static void load_gpu(struct drm_device *dev)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
unsigned int i;
for (i = 0; i < ETNA_MAX_PIPES; i++) {
@@ -98,7 +90,7 @@ static void load_gpu(struct drm_device *dev)
static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_file_private *ctx;
int ret, i;
@@ -141,7 +133,7 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
static void etnaviv_postclose(struct drm_device *dev, struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_file_private *ctx = file->driver_priv;
unsigned int i;
@@ -166,7 +158,7 @@ static void etnaviv_postclose(struct drm_device *dev, struct drm_file *file)
#ifdef CONFIG_DEBUG_FS
static int etnaviv_gem_show(struct drm_device *dev, struct seq_file *m)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
etnaviv_gem_describe_objects(priv, m);
@@ -260,7 +252,7 @@ static int show_each_gpu(struct seq_file *m, void *arg)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_gpu *gpu;
int (*show)(struct etnaviv_gpu *gpu, struct seq_file *m) =
node->info_ent->data;
@@ -303,7 +295,7 @@ static void etnaviv_debugfs_init(struct drm_minor *minor)
static int etnaviv_ioctl_get_param(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_param *args = data;
struct etnaviv_gpu *gpu;
@@ -396,7 +388,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
struct drm_file *file)
{
struct drm_etnaviv_wait_fence *args = data;
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_timespec *timeout = &args->timeout;
struct etnaviv_gpu *gpu;
@@ -444,7 +436,7 @@ static int etnaviv_ioctl_gem_userptr(struct drm_device *dev, void *data,
static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_gem_wait *args = data;
struct drm_etnaviv_timespec *timeout = &args->timeout;
struct drm_gem_object *obj;
@@ -478,7 +470,7 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
static int etnaviv_ioctl_pm_query_dom(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_pm_domain *args = data;
struct etnaviv_gpu *gpu;
@@ -495,7 +487,7 @@ static int etnaviv_ioctl_pm_query_dom(struct drm_device *dev, void *data,
static int etnaviv_ioctl_pm_query_sig(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_pm_signal *args = data;
struct etnaviv_gpu *gpu;
@@ -554,17 +546,14 @@ static int etnaviv_bind(struct device *dev)
struct drm_device *drm;
int ret;
- drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
- if (IS_ERR(drm))
- return PTR_ERR(drm);
+ priv = devm_drm_dev_alloc(dev, &etnaviv_drm_driver,
+ struct etnaviv_drm_private, drm);
+ if (IS_ERR(priv))
+ return PTR_ERR(priv);
- priv = etnaviv_alloc_private(dev);
- if (IS_ERR(priv)) {
- ret = PTR_ERR(priv);
- goto out_put;
- }
+ etnaviv_private_init(dev, priv);
- drm->dev_private = priv;
+ drm = &priv->drm;
dma_set_max_seg_size(dev, SZ_2G);
@@ -585,9 +574,7 @@ static int etnaviv_bind(struct device *dev)
out_unbind:
component_unbind_all(dev, drm);
out_free_priv:
- etnaviv_free_private(priv);
-out_put:
- drm_dev_put(drm);
+ etnaviv_private_fini(priv);
return ret;
}
@@ -595,17 +582,13 @@ static int etnaviv_bind(struct device *dev)
static void etnaviv_unbind(struct device *dev)
{
struct drm_device *drm = dev_get_drvdata(dev);
- struct etnaviv_drm_private *priv = drm->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(drm);
drm_dev_unregister(drm);
component_unbind_all(dev, drm);
- etnaviv_free_private(priv);
-
- drm->dev_private = NULL;
-
- drm_dev_put(drm);
+ etnaviv_private_fini(priv);
}
static const struct component_master_ops etnaviv_master_ops = {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index b3eb1662e90c..1f9b50b5a6aa 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -35,6 +35,7 @@ struct etnaviv_file_private {
};
struct etnaviv_drm_private {
+ struct drm_device drm;
int num_gpus;
struct etnaviv_gpu *gpu[ETNA_MAX_PIPES];
gfp_t shm_gfp_mask;
@@ -50,6 +51,12 @@ struct etnaviv_drm_private {
struct list_head gem_list;
};
+static inline struct etnaviv_drm_private *
+to_etnaviv_priv(struct drm_device *ddev)
+{
+ return container_of(ddev, struct etnaviv_drm_private, drm);
+}
+
int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
struct drm_file *file);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 71a6d2b1c80f..aa95a5e98374 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -486,7 +486,7 @@ static const struct etnaviv_gem_ops etnaviv_gem_shmem_ops = {
void etnaviv_gem_free_object(struct drm_gem_object *obj)
{
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
- struct etnaviv_drm_private *priv = obj->dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(obj->dev);
struct etnaviv_vram_mapping *mapping, *tmp;
/* object should not be active */
@@ -517,7 +517,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
mutex_lock(&priv->gem_lock);
@@ -584,7 +584,7 @@ static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags,
int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
u32 size, u32 flags, u32 *handle)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_gem_object *obj = NULL;
int ret;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 3d0f8d182506..6b40a39fb8cd 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -413,7 +413,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
struct drm_file *file)
{
struct etnaviv_file_private *ctx = file->driver_priv;
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_gem_submit *args = data;
struct drm_etnaviv_gem_submit_reloc *relocs;
struct drm_etnaviv_gem_submit_pmr *pmrs;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 654bf2631755..a407c2c9e140 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -38,7 +38,7 @@ static const struct platform_device_id gpu_ids[] = {
int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
{
- struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
switch (param) {
case ETNAVIV_PARAM_GPU_MODEL:
@@ -801,7 +801,7 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
{
- struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
dma_addr_t cmdbuf_paddr;
int ret, i;
@@ -1791,7 +1791,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
void *data)
{
struct drm_device *drm = data;
- struct etnaviv_drm_private *priv = drm->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(drm);
struct etnaviv_gpu *gpu = dev_get_drvdata(dev);
int ret;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index 1661d589bf3e..c38272868328 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -490,7 +490,7 @@ void etnaviv_iommu_dump(struct etnaviv_iommu_context *context, void *buf)
int etnaviv_iommu_global_init(struct etnaviv_gpu *gpu)
{
enum etnaviv_iommu_version version = ETNAVIV_IOMMU_V1;
- struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
struct etnaviv_iommu_global *global;
struct device *dev = gpu->drm->dev;
@@ -550,7 +550,7 @@ int etnaviv_iommu_global_init(struct etnaviv_gpu *gpu)
void etnaviv_iommu_global_fini(struct etnaviv_gpu *gpu)
{
- struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
struct etnaviv_iommu_global *global = priv->mmu_global;
if (!global)
--
2.34.1
In the etnaviv_gem_vmap_impl(), update the page property from writecombine
to PAGE_KERNEL on cached mapping. Previously, it use writecombine page
property to vmap cached buffer unconditionally. Many modern CPUs choose to
define the peripheral devices as DMA coherent by default, to be specific,
the peripheral devices are capable of snooping CPU's cache. Therefore,
cached buffers should be accessed with cached mapping.
While at it, probe cached coherent support at the host platform with the
dev_is_dma_coherent() function. This allows userspace to query on the
runtime, which avoid compile-time macros (#ifdefs). Modern CPUs choose to
define the peripheral devices as DMA coherent by default, In other words,
the peripheral devices are capable of snooping CPU's cache. This means
that device drivers do not need to maintain the coherency issue between
a processor and an peripheral I/O for the cached mapping buffers. Such a
hardware feature is implementation-defined by host CPU platform, not the
vivante GPU IP core itself. X86-64, LoongArch and Loongson Mips CPU and
some ARM64 CPU has the hardware maintain cache coherency, but ARM CPU is
not. So provide mechanism to let userspace know.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 8 ++++++++
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 16 ++++++++++++++--
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 ++++
include/uapi/drm/etnaviv_drm.h | 1 +
5 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 5ba2b3a386b3..e3a05b8b9330 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -5,6 +5,7 @@
#include <linux/component.h>
#include <linux/dma-mapping.h>
+#include <linux/dma-map-ops.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
@@ -57,6 +58,8 @@ static int etnaviv_private_init(struct device *dev,
return -ENOMEM;
}
+ priv->cached_coherent = dev_is_dma_coherent(dev);
+
return 0;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 1f9b50b5a6aa..9c05f503747a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -46,6 +46,14 @@ struct etnaviv_drm_private {
struct xarray active_contexts;
u32 next_context_id;
+ /*
+ * If true, the cached mapping is consistent for all CPU cores and
+ * peripheral bus masters in the system. It means that vboth of the
+ * CPU and GPU will see the same data if the buffer being access is
+ * cached. And coherency is guaranteed by the arch specific hardware.
+ */
+ bool cached_coherent;
+
/* list of GEM objects: */
struct mutex gem_lock;
struct list_head gem_list;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index aa95a5e98374..eed98bb9e446 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -342,6 +342,7 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
{
struct page **pages;
+ pgprot_t prot;
lockdep_assert_held(&obj->lock);
@@ -349,8 +350,19 @@ static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
if (IS_ERR(pages))
return NULL;
- return vmap(pages, obj->base.size >> PAGE_SHIFT,
- VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+ switch (obj->flags) {
+ case ETNA_BO_CACHED:
+ prot = PAGE_KERNEL;
+ break;
+ case ETNA_BO_UNCACHED:
+ prot = pgprot_noncached(PAGE_KERNEL);
+ break;
+ case ETNA_BO_WC:
+ default:
+ prot = pgprot_writecombine(PAGE_KERNEL);
+ }
+
+ return vmap(pages, obj->base.size >> PAGE_SHIFT, VM_MAP, prot);
}
static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index a407c2c9e140..306973660653 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -184,6 +184,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
*value = gpu->identity.axi_sram_size;
break;
+ case ETNAVIV_PARAM_CACHED_COHERENT:
+ *value = priv->cached_coherent;
+ break;
+
default:
DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
return -EINVAL;
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index d87410a8443a..7c5d988a5b9f 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -82,6 +82,7 @@ struct drm_etnaviv_timespec {
#define ETNAVIV_PARAM_GPU_TP_CORE_COUNT 0x21
#define ETNAVIV_PARAM_GPU_ON_CHIP_SRAM_SIZE 0x22
#define ETNAVIV_PARAM_GPU_AXI_SRAM_SIZE 0x23
+#define ETNAVIV_PARAM_CACHED_COHERENT 0x24
#define ETNA_MAX_PIPES 4
--
2.34.1
In the etnaviv_pdev_probe() and the etnaviv_gpu_platform_probe() function,
the value of '&pdev->dev' has been cached to a local auto variable (which
is named as 'dev'), but part of caller functions use 'dev' as argument,
but the rest use '&pdev->dev'. To keep it consistent, use 'dev' uniformly.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 10 +++++-----
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 16 ++++++++--------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index e3a05b8b9330..d5a5fcc30341 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -612,7 +612,7 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
if (!of_device_is_available(core_node))
continue;
- drm_of_component_match_add(&pdev->dev, &match,
+ drm_of_component_match_add(dev, &match,
component_compare_of, core_node);
}
} else {
@@ -635,9 +635,9 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
* bit to make sure we are allocating the command buffers and
* TLBs in the lower 4 GiB address space.
*/
- if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(40)) ||
- dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32))) {
- dev_dbg(&pdev->dev, "No suitable DMA available\n");
+ if (dma_set_mask(dev, DMA_BIT_MASK(40)) ||
+ dma_set_coherent_mask(dev, DMA_BIT_MASK(32))) {
+ dev_dbg(dev, "No suitable DMA available\n");
return -ENODEV;
}
@@ -648,7 +648,7 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
*/
first_node = etnaviv_of_first_available_node();
if (first_node) {
- of_dma_configure(&pdev->dev, first_node, true);
+ of_dma_configure(dev, first_node, true);
of_node_put(first_node);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 306973660653..3fd637c17797 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1903,7 +1903,7 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
if (!gpu)
return -ENOMEM;
- gpu->dev = &pdev->dev;
+ gpu->dev = dev;
mutex_init(&gpu->lock);
mutex_init(&gpu->sched_lock);
@@ -1917,8 +1917,8 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
if (gpu->irq < 0)
return gpu->irq;
- err = devm_request_irq(&pdev->dev, gpu->irq, irq_handler, 0,
- dev_name(gpu->dev), gpu);
+ err = devm_request_irq(dev, gpu->irq, irq_handler, 0,
+ dev_name(dev), gpu);
if (err) {
dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
return err;
@@ -1937,13 +1937,13 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
* autosuspend delay is rather arbitary: no measurements have
* yet been performed to determine an appropriate value.
*/
- pm_runtime_use_autosuspend(gpu->dev);
- pm_runtime_set_autosuspend_delay(gpu->dev, 200);
- pm_runtime_enable(gpu->dev);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 200);
+ pm_runtime_enable(dev);
- err = component_add(&pdev->dev, &gpu_ops);
+ err = component_add(dev, &gpu_ops);
if (err < 0) {
- dev_err(&pdev->dev, "failed to register component: %d\n", err);
+ dev_err(dev, "failed to register component: %d\n", err);
return err;
}
--
2.34.1
Because we need this function to create virtial child.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 33 +++++++++++++++++++++++----
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 9 ++++++++
2 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index d5a5fcc30341..5f65f2dead44 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -670,16 +670,36 @@ static struct platform_driver etnaviv_platform_driver = {
},
};
-static int etnaviv_create_platform_device(const char *name,
- struct platform_device **ppdev)
+int etnaviv_create_platform_device(struct device *parent,
+ const char *name, int id,
+ struct resource *pres,
+ void *data,
+ struct platform_device **ppdev)
{
struct platform_device *pdev;
int ret;
- pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
+ pdev = platform_device_alloc(name, id);
if (!pdev)
return -ENOMEM;
+ pdev->dev.parent = parent;
+
+ if (pres) {
+ ret = platform_device_add_resources(pdev, pres, 1);
+ if (ret) {
+ platform_device_put(pdev);
+ return ret;
+ }
+ }
+
+ if (data) {
+ void *pdata = kmalloc(sizeof(void *), GFP_KERNEL);
+
+ *(void **)pdata = data;
+ pdev->dev.platform_data = pdata;
+ }
+
ret = platform_device_add(pdev);
if (ret) {
platform_device_put(pdev);
@@ -691,7 +711,7 @@ static int etnaviv_create_platform_device(const char *name,
return 0;
}
-static void etnaviv_destroy_platform_device(struct platform_device **ppdev)
+void etnaviv_destroy_platform_device(struct platform_device **ppdev)
{
struct platform_device *pdev = *ppdev;
@@ -728,7 +748,10 @@ static int __init etnaviv_init(void)
if (np) {
of_node_put(np);
- ret = etnaviv_create_platform_device("etnaviv", &etnaviv_drm);
+ ret = etnaviv_create_platform_device(NULL, "etnaviv",
+ PLATFORM_DEVID_NONE,
+ NULL, NULL,
+ &etnaviv_drm);
if (ret)
goto unregister_platform_driver;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 9c05f503747a..bd8ac64dbaf3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -9,6 +9,7 @@
#include <linux/io.h>
#include <linux/list.h>
#include <linux/mm_types.h>
+#include <linux/platform_device.h>
#include <linux/sizes.h>
#include <linux/time64.h>
#include <linux/types.h>
@@ -97,6 +98,14 @@ bool etnaviv_cmd_validate_one(struct etnaviv_gpu *gpu,
u32 *stream, unsigned int size,
struct drm_etnaviv_gem_submit_reloc *relocs, unsigned int reloc_size);
+int etnaviv_create_platform_device(struct device *parent,
+ const char *name, int id,
+ struct resource *pres,
+ void *data,
+ struct platform_device **ppdev);
+
+void etnaviv_destroy_platform_device(struct platform_device **ppdev);
+
#ifdef CONFIG_DEBUG_FS
void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
struct seq_file *m);
--
2.34.1
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 <[email protected]>
---
drivers/gpu/drm/etnaviv/Kconfig | 8 +
drivers/gpu/drm/etnaviv/Makefile | 2 +
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 12 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 75 ++++++--
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 4 +
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 224 ++++++++++++++++++++++
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 18 ++
7 files changed, 327 insertions(+), 16 deletions(-)
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index faa7fc68b009..38c251585ec1 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -15,6 +15,14 @@ config DRM_ETNAVIV
help
DRM driver for Vivante GPUs.
+config DRM_ETNAVIV_PCI_DRIVER
+ bool "enable ETNAVIV PCI driver support"
+ depends on DRM_ETNAVIV
+ depends on PCI
+ help
+ Compile in support for Vivante GPUs attached via PCI(e).
+ Say Y if you have such hardwares.
+
config DRM_ETNAVIV_THERMAL
bool "enable ETNAVIV thermal throttling"
depends on DRM_ETNAVIV
diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
index 46e5ffad69a6..6829e1ebf2db 100644
--- a/drivers/gpu/drm/etnaviv/Makefile
+++ b/drivers/gpu/drm/etnaviv/Makefile
@@ -16,4 +16,6 @@ etnaviv-y := \
etnaviv_perfmon.o \
etnaviv_sched.o
+etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o
+
obj-$(CONFIG_DRM_ETNAVIV) += etnaviv.o
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 5f65f2dead44..48e2402adbe3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -24,6 +24,7 @@
#include "etnaviv_gpu.h"
#include "etnaviv_gem.h"
#include "etnaviv_mmu.h"
+#include "etnaviv_pci_drv.h"
#include "etnaviv_perfmon.h"
/*
@@ -566,6 +567,10 @@ static int etnaviv_bind(struct device *dev)
if (ret < 0)
goto out_free_priv;
+ ret = etnaviv_register_irq_handler(dev, priv);
+ if (ret)
+ goto out_unbind;
+
load_gpu(drm);
ret = drm_dev_register(drm, 0);
@@ -594,7 +599,7 @@ static void etnaviv_unbind(struct device *dev)
etnaviv_private_fini(priv);
}
-static const struct component_master_ops etnaviv_master_ops = {
+const struct component_master_ops etnaviv_master_ops = {
.bind = etnaviv_bind,
.unbind = etnaviv_unbind,
};
@@ -740,6 +745,10 @@ static int __init etnaviv_init(void)
if (ret != 0)
goto unregister_gpu_driver;
+ ret = etnaviv_register_pci_driver();
+ if (ret != 0)
+ goto unregister_platform_driver;
+
/*
* If the DT contains at least one available GPU device, instantiate
* the DRM platform device.
@@ -769,6 +778,7 @@ module_init(etnaviv_init);
static void __exit etnaviv_exit(void)
{
etnaviv_destroy_platform_device(&etnaviv_drm);
+ etnaviv_unregister_pci_driver();
platform_driver_unregister(&etnaviv_platform_driver);
platform_driver_unregister(&etnaviv_gpu_driver);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 3fd637c17797..221020e98900 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -10,6 +10,7 @@
#include <linux/dma-mapping.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
@@ -29,6 +30,7 @@
static const struct platform_device_id gpu_ids[] = {
{ .name = "etnaviv-gpu,2d" },
+ { .name = "etnaviv-gpu,3d" },
{ },
};
@@ -1555,14 +1557,22 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
static irqreturn_t irq_handler(int irq, void *data)
{
- struct etnaviv_gpu *gpu = data;
+ struct etnaviv_drm_private *priv = data;
irqreturn_t ret = IRQ_NONE;
+ int i;
- u32 intr = gpu_read(gpu, VIVS_HI_INTR_ACKNOWLEDGE);
-
- if (intr != 0) {
+ for (i = 0; i < priv->num_gpus; i++) {
+ struct etnaviv_gpu *gpu = priv->gpu[i];
+ u32 intr;
int event;
+ if (!gpu)
+ continue;
+
+ intr = gpu_read(gpu, VIVS_HI_INTR_ACKNOWLEDGE);
+ if (!intr)
+ continue;
+
pm_runtime_mark_last_busy(gpu->dev);
dev_dbg(gpu->dev, "intr 0x%08x\n", intr);
@@ -1893,10 +1903,44 @@ static const struct of_device_id etnaviv_gpu_match[] = {
};
MODULE_DEVICE_TABLE(of, etnaviv_gpu_match);
+/*
+ * dev point to the master. For platform device, it is virtual.
+ * For PCI(e) device, it is real.
+ */
+int etnaviv_register_irq_handler(struct device *dev,
+ struct etnaviv_drm_private *priv)
+{
+ bool is_pci = dev_is_pci(dev);
+ int ret = 0;
+
+ if (is_pci) {
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ ret = request_irq(pdev->irq, irq_handler, IRQF_SHARED,
+ dev_name(dev), priv);
+ } else {
+ int i;
+
+ for (i = 0; i < priv->num_gpus; i++) {
+ struct etnaviv_gpu *gpu = priv->gpu[i];
+
+ ret = devm_request_irq(gpu->dev, gpu->irq, irq_handler,
+ 0, dev_name(dev), priv);
+ if (ret) {
+ dev_err(dev, "failed to request IRQ handler: %d\n", ret);
+ break;
+ }
+ }
+ }
+
+ return ret;
+}
+
static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct etnaviv_gpu *gpu;
+ bool is_pci;
int err;
gpu = devm_kzalloc(dev, sizeof(*gpu), GFP_KERNEL);
@@ -1912,22 +1956,23 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
if (IS_ERR(gpu->mmio))
return PTR_ERR(gpu->mmio);
+ is_pci = dev->parent ? dev_is_pci(dev->parent) : false;
+
/* Get Interrupt: */
- gpu->irq = platform_get_irq(pdev, 0);
+ if (is_pci)
+ gpu->irq = to_pci_dev(dev->parent)->irq;
+ else
+ gpu->irq = platform_get_irq(pdev, 0);
+
if (gpu->irq < 0)
return gpu->irq;
- err = devm_request_irq(dev, gpu->irq, irq_handler, 0,
- dev_name(dev), gpu);
- if (err) {
- dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
- return err;
- }
-
/* Get Clocks: */
- err = etnaviv_gpu_clk_get(gpu);
- if (err)
- return err;
+ if (!is_pci) {
+ err = etnaviv_gpu_clk_get(gpu);
+ if (err)
+ return err;
+ }
/* TODO: figure out max mapped size */
dev_set_drvdata(dev, gpu);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 7d5e9158e13c..d354e096652c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -208,6 +208,10 @@ static inline u32 gpu_read_power(struct etnaviv_gpu *gpu, u32 reg)
int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value);
int etnaviv_gpu_init(struct etnaviv_gpu *gpu);
+
+int etnaviv_register_irq_handler(struct device *dev,
+ struct etnaviv_drm_private *priv);
+
bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu);
#ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
new file mode 100644
index 000000000000..ec5039866eda
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/component.h>
+#include <linux/pci.h>
+
+#include "etnaviv_drv.h"
+#include "etnaviv_pci_drv.h"
+
+enum etnaviv_pci_gpu_chip_id {
+ GC_CORE_UNKNOWN = 0,
+ JM9100 = 1,
+ JD9230P = 2,
+ GP102 = 3,
+ GC1000_IN_LS7A1000 = 4,
+ GC1000_IN_LS2K1000 = 5,
+ GC_CORE_PCI_LAST,
+};
+
+struct etnaviv_pci_gpu_data {
+ enum etnaviv_pci_gpu_chip_id chip_id;
+ u32 num_core;
+ u32 num_vram;
+ u32 vram_bars[2];
+ u32 mmio_bar;
+ struct {
+ u32 id;
+ u32 offset;
+ u32 size;
+ char compatible[20];
+ } cores[ETNA_MAX_PIPES];
+
+ bool has_dedicated_vram;
+ char market_name[24];
+};
+
+static const struct etnaviv_pci_gpu_data
+gc_core_plaform_data[GC_CORE_PCI_LAST] = {
+ {
+ .chip_id = GC_CORE_UNKNOWN,
+ },
+ {
+ .chip_id = JM9100,
+ .num_core = 1,
+ .num_vram = 2,
+ .vram_bars = {0, 2},
+ .mmio_bar = 1,
+ .cores = {{0, 0x00900000, 0x00010000, "etnaviv-gpu,3d"},},
+ .has_dedicated_vram = true,
+ .market_name = "JingJia Micro JM9100",
+ },
+ {
+ .chip_id = JD9230P,
+ .num_core = 2,
+ .num_vram = 2,
+ .vram_bars = {0, 2},
+ .mmio_bar = 1,
+ .cores = {{0, 0x00900000, 0x00010000, "etnaviv-gpu,3d"},
+ {1, 0x00910000, 0x00010000, "etnaviv-gpu,3d"},},
+ .has_dedicated_vram = true,
+ .market_name = "JingJia Micro JD9230P",
+ },
+ {
+ .chip_id = GP102,
+ .num_core = 2,
+ .num_vram = 1,
+ .vram_bars = {0,},
+ .mmio_bar = 2,
+ .cores = {{0, 0x00040000, 0x00010000, "etnaviv-gpu,3d"},
+ {0, 0x000C0000, 0x00010000, "etnaviv-gpu,2d"},},
+ .has_dedicated_vram = true,
+ .market_name = "LingJiu GP102",
+ },
+ {
+ .chip_id = GC1000_IN_LS7A1000,
+ .num_core = 1,
+ .num_vram = 1,
+ .vram_bars = {2,},
+ .mmio_bar = 0,
+ .cores = {{0, 0, 0x00010000, "etnaviv-gpu,3d"}, {}, {}, {}},
+ .has_dedicated_vram = true,
+ .market_name = "GC1000 in LS7A1000",
+ },
+ {
+ .chip_id = GC1000_IN_LS2K1000,
+ .num_core = 1,
+ .num_vram = 0,
+ .mmio_bar = 0,
+ .cores = {{0, 0, 0x00010000, "etnaviv-gpu,3d"}, {}, {}, {}},
+ .has_dedicated_vram = false,
+ .market_name = "GC1000 in LS2K1000",
+ },
+};
+
+static const struct etnaviv_pci_gpu_data *
+etnaviv_pci_get_platform_data(const struct pci_device_id *entity)
+{
+ enum etnaviv_pci_gpu_chip_id chip_id = entity->driver_data;
+ static const struct etnaviv_pci_gpu_data *pdata;
+
+ pdata = &gc_core_plaform_data[chip_id];
+ if (!pdata || pdata->chip_id == GC_CORE_UNKNOWN)
+ return NULL;
+
+ return pdata;
+}
+
+extern const struct component_master_ops etnaviv_master_ops;
+
+static int etnaviv_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ const struct etnaviv_pci_gpu_data *pdata;
+ struct device *dev = &pdev->dev;
+ struct component_match *matches = NULL;
+ unsigned int i;
+ unsigned int num_core;
+ int ret;
+
+ ret = pcim_enable_device(pdev);
+ if (ret) {
+ dev_err(dev, "failed to enable\n");
+ return ret;
+ }
+
+ pci_set_master(pdev);
+
+ ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+
+ pdata = etnaviv_pci_get_platform_data(ent);
+ if (!pdata)
+ return -ENODEV;
+
+ num_core = pdata->num_core;
+
+ dev_info(dev, "%s has %u GPU cores\n", pdata->market_name, num_core);
+
+ /*
+ * Create a virtual platform device for the sub-component,
+ * a sub-component is refer to a single vivante GPU core.
+ * But it can also be extended to stand for a display controller
+ * or any other IP core attached via the same PCIe master.
+ */
+ for (i = 0; i < num_core; i++) {
+ struct platform_device *virtual_child;
+ resource_size_t start, offset, size;
+ struct resource res;
+
+ start = pci_resource_start(pdev, pdata->mmio_bar);
+ offset = pdata->cores[i].offset;
+ size = pdata->cores[i].size;
+
+ memset(&res, 0, sizeof(res));
+ res.flags = IORESOURCE_MEM;
+ res.name = "reg";
+ res.start = start + offset;
+ res.end = start + offset + size - 1;
+
+ ret = etnaviv_create_platform_device(dev,
+ pdata->cores[i].compatible,
+ pdata->cores[i].id,
+ &res,
+ (void *)pdata,
+ &virtual_child);
+ if (ret)
+ return ret;
+
+ component_match_add(dev, &matches, component_compare_dev,
+ &virtual_child->dev);
+ }
+
+ ret = component_master_add_with_match(dev, &etnaviv_master_ops, matches);
+
+ return ret;
+}
+
+static int platform_device_remove_callback(struct device *dev, void *data)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ etnaviv_destroy_platform_device(&pdev);
+
+ return 0;
+}
+
+static void etnaviv_pci_remove(struct pci_dev *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ component_master_del(dev, &etnaviv_master_ops);
+
+ device_for_each_child(dev, NULL, platform_device_remove_callback);
+
+ pci_clear_master(pdev);
+}
+
+static const struct pci_device_id etnaviv_pci_id_lists[] = {
+ {0x0731, 0x9100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, JM9100},
+ {0x0731, 0x9230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, JD9230P},
+ {0x0709, 0x0001, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GP102},
+ {0x0014, 0x7A15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GC1000_IN_LS7A1000},
+ {0x0014, 0x7A05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GC1000_IN_LS2K1000},
+ { }
+};
+
+static struct pci_driver etnaviv_pci_driver = {
+ .name = "etnaviv",
+ .id_table = etnaviv_pci_id_lists,
+ .probe = etnaviv_pci_probe,
+ .remove = etnaviv_pci_remove,
+};
+
+int etnaviv_register_pci_driver(void)
+{
+ return pci_register_driver(&etnaviv_pci_driver);
+}
+
+void etnaviv_unregister_pci_driver(void)
+{
+ pci_unregister_driver(&etnaviv_pci_driver);
+}
+
+MODULE_DEVICE_TABLE(pci, etnaviv_pci_id_lists);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
new file mode 100644
index 000000000000..1db559ee5e9b
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ETNAVIV_PCI_DRV_H__
+#define __ETNAVIV_PCI_DRV_H__
+
+#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER
+
+int etnaviv_register_pci_driver(void);
+void etnaviv_unregister_pci_driver(void);
+
+#else
+
+static inline int etnaviv_register_pci_driver(void) { return 0; }
+static inline void etnaviv_unregister_pci_driver(void) { }
+
+#endif
+
+#endif
--
2.34.1
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 <[email protected]>
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 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.
Cheers, Sima
> ---
> drivers/gpu/drm/etnaviv/Kconfig | 8 +
> drivers/gpu/drm/etnaviv/Makefile | 2 +
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 12 +-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 75 ++++++--
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 4 +
> drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 224 ++++++++++++++++++++++
> drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 18 ++
> 7 files changed, 327 insertions(+), 16 deletions(-)
> create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
> create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
>
> diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
> index faa7fc68b009..38c251585ec1 100644
> --- a/drivers/gpu/drm/etnaviv/Kconfig
> +++ b/drivers/gpu/drm/etnaviv/Kconfig
> @@ -15,6 +15,14 @@ config DRM_ETNAVIV
> help
> DRM driver for Vivante GPUs.
>
> +config DRM_ETNAVIV_PCI_DRIVER
> + bool "enable ETNAVIV PCI driver support"
> + depends on DRM_ETNAVIV
> + depends on PCI
> + help
> + Compile in support for Vivante GPUs attached via PCI(e).
> + Say Y if you have such hardwares.
> +
> config DRM_ETNAVIV_THERMAL
> bool "enable ETNAVIV thermal throttling"
> depends on DRM_ETNAVIV
> diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
> index 46e5ffad69a6..6829e1ebf2db 100644
> --- a/drivers/gpu/drm/etnaviv/Makefile
> +++ b/drivers/gpu/drm/etnaviv/Makefile
> @@ -16,4 +16,6 @@ etnaviv-y := \
> etnaviv_perfmon.o \
> etnaviv_sched.o
>
> +etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o
> +
> obj-$(CONFIG_DRM_ETNAVIV) += etnaviv.o
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 5f65f2dead44..48e2402adbe3 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -24,6 +24,7 @@
> #include "etnaviv_gpu.h"
> #include "etnaviv_gem.h"
> #include "etnaviv_mmu.h"
> +#include "etnaviv_pci_drv.h"
> #include "etnaviv_perfmon.h"
>
> /*
> @@ -566,6 +567,10 @@ static int etnaviv_bind(struct device *dev)
> if (ret < 0)
> goto out_free_priv;
>
> + ret = etnaviv_register_irq_handler(dev, priv);
> + if (ret)
> + goto out_unbind;
> +
> load_gpu(drm);
>
> ret = drm_dev_register(drm, 0);
> @@ -594,7 +599,7 @@ static void etnaviv_unbind(struct device *dev)
> etnaviv_private_fini(priv);
> }
>
> -static const struct component_master_ops etnaviv_master_ops = {
> +const struct component_master_ops etnaviv_master_ops = {
> .bind = etnaviv_bind,
> .unbind = etnaviv_unbind,
> };
> @@ -740,6 +745,10 @@ static int __init etnaviv_init(void)
> if (ret != 0)
> goto unregister_gpu_driver;
>
> + ret = etnaviv_register_pci_driver();
> + if (ret != 0)
> + goto unregister_platform_driver;
> +
> /*
> * If the DT contains at least one available GPU device, instantiate
> * the DRM platform device.
> @@ -769,6 +778,7 @@ module_init(etnaviv_init);
> static void __exit etnaviv_exit(void)
> {
> etnaviv_destroy_platform_device(&etnaviv_drm);
> + etnaviv_unregister_pci_driver();
> platform_driver_unregister(&etnaviv_platform_driver);
> platform_driver_unregister(&etnaviv_gpu_driver);
> }
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 3fd637c17797..221020e98900 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -10,6 +10,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> +#include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> @@ -29,6 +30,7 @@
>
> static const struct platform_device_id gpu_ids[] = {
> { .name = "etnaviv-gpu,2d" },
> + { .name = "etnaviv-gpu,3d" },
> { },
> };
>
> @@ -1555,14 +1557,22 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
>
> static irqreturn_t irq_handler(int irq, void *data)
> {
> - struct etnaviv_gpu *gpu = data;
> + struct etnaviv_drm_private *priv = data;
> irqreturn_t ret = IRQ_NONE;
> + int i;
>
> - u32 intr = gpu_read(gpu, VIVS_HI_INTR_ACKNOWLEDGE);
> -
> - if (intr != 0) {
> + for (i = 0; i < priv->num_gpus; i++) {
> + struct etnaviv_gpu *gpu = priv->gpu[i];
> + u32 intr;
> int event;
>
> + if (!gpu)
> + continue;
> +
> + intr = gpu_read(gpu, VIVS_HI_INTR_ACKNOWLEDGE);
> + if (!intr)
> + continue;
> +
> pm_runtime_mark_last_busy(gpu->dev);
>
> dev_dbg(gpu->dev, "intr 0x%08x\n", intr);
> @@ -1893,10 +1903,44 @@ static const struct of_device_id etnaviv_gpu_match[] = {
> };
> MODULE_DEVICE_TABLE(of, etnaviv_gpu_match);
>
> +/*
> + * dev point to the master. For platform device, it is virtual.
> + * For PCI(e) device, it is real.
> + */
> +int etnaviv_register_irq_handler(struct device *dev,
> + struct etnaviv_drm_private *priv)
> +{
> + bool is_pci = dev_is_pci(dev);
> + int ret = 0;
> +
> + if (is_pci) {
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + ret = request_irq(pdev->irq, irq_handler, IRQF_SHARED,
> + dev_name(dev), priv);
> + } else {
> + int i;
> +
> + for (i = 0; i < priv->num_gpus; i++) {
> + struct etnaviv_gpu *gpu = priv->gpu[i];
> +
> + ret = devm_request_irq(gpu->dev, gpu->irq, irq_handler,
> + 0, dev_name(dev), priv);
> + if (ret) {
> + dev_err(dev, "failed to request IRQ handler: %d\n", ret);
> + break;
> + }
> + }
> + }
> +
> + return ret;
> +}
> +
> static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct etnaviv_gpu *gpu;
> + bool is_pci;
> int err;
>
> gpu = devm_kzalloc(dev, sizeof(*gpu), GFP_KERNEL);
> @@ -1912,22 +1956,23 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
> if (IS_ERR(gpu->mmio))
> return PTR_ERR(gpu->mmio);
>
> + is_pci = dev->parent ? dev_is_pci(dev->parent) : false;
> +
> /* Get Interrupt: */
> - gpu->irq = platform_get_irq(pdev, 0);
> + if (is_pci)
> + gpu->irq = to_pci_dev(dev->parent)->irq;
> + else
> + gpu->irq = platform_get_irq(pdev, 0);
> +
> if (gpu->irq < 0)
> return gpu->irq;
>
> - err = devm_request_irq(dev, gpu->irq, irq_handler, 0,
> - dev_name(dev), gpu);
> - if (err) {
> - dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
> - return err;
> - }
> -
> /* Get Clocks: */
> - err = etnaviv_gpu_clk_get(gpu);
> - if (err)
> - return err;
> + if (!is_pci) {
> + err = etnaviv_gpu_clk_get(gpu);
> + if (err)
> + return err;
> + }
>
> /* TODO: figure out max mapped size */
> dev_set_drvdata(dev, gpu);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 7d5e9158e13c..d354e096652c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -208,6 +208,10 @@ static inline u32 gpu_read_power(struct etnaviv_gpu *gpu, u32 reg)
> int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value);
>
> int etnaviv_gpu_init(struct etnaviv_gpu *gpu);
> +
> +int etnaviv_register_irq_handler(struct device *dev,
> + struct etnaviv_drm_private *priv);
> +
> bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu);
>
> #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
> new file mode 100644
> index 000000000000..ec5039866eda
> --- /dev/null
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/component.h>
> +#include <linux/pci.h>
> +
> +#include "etnaviv_drv.h"
> +#include "etnaviv_pci_drv.h"
> +
> +enum etnaviv_pci_gpu_chip_id {
> + GC_CORE_UNKNOWN = 0,
> + JM9100 = 1,
> + JD9230P = 2,
> + GP102 = 3,
> + GC1000_IN_LS7A1000 = 4,
> + GC1000_IN_LS2K1000 = 5,
> + GC_CORE_PCI_LAST,
> +};
> +
> +struct etnaviv_pci_gpu_data {
> + enum etnaviv_pci_gpu_chip_id chip_id;
> + u32 num_core;
> + u32 num_vram;
> + u32 vram_bars[2];
> + u32 mmio_bar;
> + struct {
> + u32 id;
> + u32 offset;
> + u32 size;
> + char compatible[20];
> + } cores[ETNA_MAX_PIPES];
> +
> + bool has_dedicated_vram;
> + char market_name[24];
> +};
> +
> +static const struct etnaviv_pci_gpu_data
> +gc_core_plaform_data[GC_CORE_PCI_LAST] = {
> + {
> + .chip_id = GC_CORE_UNKNOWN,
> + },
> + {
> + .chip_id = JM9100,
> + .num_core = 1,
> + .num_vram = 2,
> + .vram_bars = {0, 2},
> + .mmio_bar = 1,
> + .cores = {{0, 0x00900000, 0x00010000, "etnaviv-gpu,3d"},},
> + .has_dedicated_vram = true,
> + .market_name = "JingJia Micro JM9100",
> + },
> + {
> + .chip_id = JD9230P,
> + .num_core = 2,
> + .num_vram = 2,
> + .vram_bars = {0, 2},
> + .mmio_bar = 1,
> + .cores = {{0, 0x00900000, 0x00010000, "etnaviv-gpu,3d"},
> + {1, 0x00910000, 0x00010000, "etnaviv-gpu,3d"},},
> + .has_dedicated_vram = true,
> + .market_name = "JingJia Micro JD9230P",
> + },
> + {
> + .chip_id = GP102,
> + .num_core = 2,
> + .num_vram = 1,
> + .vram_bars = {0,},
> + .mmio_bar = 2,
> + .cores = {{0, 0x00040000, 0x00010000, "etnaviv-gpu,3d"},
> + {0, 0x000C0000, 0x00010000, "etnaviv-gpu,2d"},},
> + .has_dedicated_vram = true,
> + .market_name = "LingJiu GP102",
> + },
> + {
> + .chip_id = GC1000_IN_LS7A1000,
> + .num_core = 1,
> + .num_vram = 1,
> + .vram_bars = {2,},
> + .mmio_bar = 0,
> + .cores = {{0, 0, 0x00010000, "etnaviv-gpu,3d"}, {}, {}, {}},
> + .has_dedicated_vram = true,
> + .market_name = "GC1000 in LS7A1000",
> + },
> + {
> + .chip_id = GC1000_IN_LS2K1000,
> + .num_core = 1,
> + .num_vram = 0,
> + .mmio_bar = 0,
> + .cores = {{0, 0, 0x00010000, "etnaviv-gpu,3d"}, {}, {}, {}},
> + .has_dedicated_vram = false,
> + .market_name = "GC1000 in LS2K1000",
> + },
> +};
> +
> +static const struct etnaviv_pci_gpu_data *
> +etnaviv_pci_get_platform_data(const struct pci_device_id *entity)
> +{
> + enum etnaviv_pci_gpu_chip_id chip_id = entity->driver_data;
> + static const struct etnaviv_pci_gpu_data *pdata;
> +
> + pdata = &gc_core_plaform_data[chip_id];
> + if (!pdata || pdata->chip_id == GC_CORE_UNKNOWN)
> + return NULL;
> +
> + return pdata;
> +}
> +
> +extern const struct component_master_ops etnaviv_master_ops;
> +
> +static int etnaviv_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + const struct etnaviv_pci_gpu_data *pdata;
> + struct device *dev = &pdev->dev;
> + struct component_match *matches = NULL;
> + unsigned int i;
> + unsigned int num_core;
> + int ret;
> +
> + ret = pcim_enable_device(pdev);
> + if (ret) {
> + dev_err(dev, "failed to enable\n");
> + return ret;
> + }
> +
> + pci_set_master(pdev);
> +
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> +
> + pdata = etnaviv_pci_get_platform_data(ent);
> + if (!pdata)
> + return -ENODEV;
> +
> + num_core = pdata->num_core;
> +
> + dev_info(dev, "%s has %u GPU cores\n", pdata->market_name, num_core);
> +
> + /*
> + * Create a virtual platform device for the sub-component,
> + * a sub-component is refer to a single vivante GPU core.
> + * But it can also be extended to stand for a display controller
> + * or any other IP core attached via the same PCIe master.
> + */
> + for (i = 0; i < num_core; i++) {
> + struct platform_device *virtual_child;
> + resource_size_t start, offset, size;
> + struct resource res;
> +
> + start = pci_resource_start(pdev, pdata->mmio_bar);
> + offset = pdata->cores[i].offset;
> + size = pdata->cores[i].size;
> +
> + memset(&res, 0, sizeof(res));
> + res.flags = IORESOURCE_MEM;
> + res.name = "reg";
> + res.start = start + offset;
> + res.end = start + offset + size - 1;
> +
> + ret = etnaviv_create_platform_device(dev,
> + pdata->cores[i].compatible,
> + pdata->cores[i].id,
> + &res,
> + (void *)pdata,
> + &virtual_child);
> + if (ret)
> + return ret;
> +
> + component_match_add(dev, &matches, component_compare_dev,
> + &virtual_child->dev);
> + }
> +
> + ret = component_master_add_with_match(dev, &etnaviv_master_ops, matches);
> +
> + return ret;
> +}
> +
> +static int platform_device_remove_callback(struct device *dev, void *data)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + etnaviv_destroy_platform_device(&pdev);
> +
> + return 0;
> +}
> +
> +static void etnaviv_pci_remove(struct pci_dev *pdev)
> +{
> + struct device *dev = &pdev->dev;
> +
> + component_master_del(dev, &etnaviv_master_ops);
> +
> + device_for_each_child(dev, NULL, platform_device_remove_callback);
> +
> + pci_clear_master(pdev);
> +}
> +
> +static const struct pci_device_id etnaviv_pci_id_lists[] = {
> + {0x0731, 0x9100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, JM9100},
> + {0x0731, 0x9230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, JD9230P},
> + {0x0709, 0x0001, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GP102},
> + {0x0014, 0x7A15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GC1000_IN_LS7A1000},
> + {0x0014, 0x7A05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GC1000_IN_LS2K1000},
> + { }
> +};
> +
> +static struct pci_driver etnaviv_pci_driver = {
> + .name = "etnaviv",
> + .id_table = etnaviv_pci_id_lists,
> + .probe = etnaviv_pci_probe,
> + .remove = etnaviv_pci_remove,
> +};
> +
> +int etnaviv_register_pci_driver(void)
> +{
> + return pci_register_driver(&etnaviv_pci_driver);
> +}
> +
> +void etnaviv_unregister_pci_driver(void)
> +{
> + pci_unregister_driver(&etnaviv_pci_driver);
> +}
> +
> +MODULE_DEVICE_TABLE(pci, etnaviv_pci_id_lists);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
> new file mode 100644
> index 000000000000..1db559ee5e9b
> --- /dev/null
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __ETNAVIV_PCI_DRV_H__
> +#define __ETNAVIV_PCI_DRV_H__
> +
> +#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER
> +
> +int etnaviv_register_pci_driver(void);
> +void etnaviv_unregister_pci_driver(void);
> +
> +#else
> +
> +static inline int etnaviv_register_pci_driver(void) { return 0; }
> +static inline void etnaviv_unregister_pci_driver(void) { }
> +
> +#endif
> +
> +#endif
> --
> 2.34.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Hi,
On 2024/2/7 17:35, 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 <[email protected]>
> Uh so my understanding is that drivers really shouldn't create platform
> devices of their own.
>
Yes,
At least for DT-based systems, this driver can be modified
to let the core to create the virtual master for us. We don't
have to create platform devices by our own(refer to the drm/etnaviv
driver).
I means that we could put the following example device node
into the .dts file.
gpu_2d: gpu@A0000 {
compatible = "vivante,gc";
reg = <0xA0000 0x4000>;
};
gpu_3d: gpu@90000 {
compatible = "vivante,gc";
reg = <0x90000 0x4000>;
};
gpu@0 {
compatible = "etnaviv";
cores = <&gpu_2d &gpu_3d>;
dma-coherent;
dma-mask = <0xffffffff>
virtual_master;
};
But now, I'm afraid it's too late. Because the DTS/DTB may already have been
burned into board's BIOS for years. I guess, nowadays, modifying(changes)
this driver have to take the backward compatibility constraint into consideration.
Since we only have one chance to form the spec, that happens when this driver was
initially merged. Apparently, we miss it.
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 <[email protected]>
>
> 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.
> 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.
Nouveau supports both iirc.
Maxime
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 <[email protected]>
> >
> > 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).
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.
>
> Nouveau supports both iirc.
>
> Maxime
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Fri, Feb 09, 2024 at 12:02:48PM +0100, 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 <[email protected]>
> > >
> > > 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.
But there should be a single PCI device, while we have multiple "DT"
devices, right? Or is there several PCI devices too on that PCI card?
Maxime
Hi,
On 2024/2/9 23:15, Maxime Ripard wrote:
> On Fri, Feb 09, 2024 at 12:02:48PM +0100, 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 <[email protected]>
>>>> 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.
> But there should be a single PCI device, while we have multiple "DT"
> devices, right? Or is there several PCI devices too on that PCI card?
There is only a single PCI(e) device on that PCI(e) card, this single
PCI(e) device is selected as the component master. All other Hardware IP
blocks are shipped by the single PCI(e) master. It may includes Display
controllers, GPUs, video decoders, HDMI display bridges hardware unit etc.
But all of those Hardware IP share the same MMIO registers PCI BAR, this
PCI BAR is a kind of PCI(e) MEM resource. It is a relative *big* chunk,
as large as 32MB in address ranges for the JingJia Macro dGPU. Therefore,
I break the whole registers memory(MMIO) resource into smaller pieces by
creating platform device manually, manually created platform device is
called as virtual child in this series.
In short, we cut the whole into smaller piece, each smaller piece is a
single hardware IP block, thus deserve a single device driver. We will
have multiple platform devices if the dGPU contains multiple hardware
IP block. On the driver side, we bind all of the scattered driver module
with component.
Bind with another PCI(e) device is also possible, if it is going to be
used under the same driver. It is just that this will not need us to
create a platform device for it manually. We won't set its parent, they
are siblings then.
> Maxime
On Sat, Feb 10, 2024 at 12:25:33AM +0800, Sui Jingfeng wrote:
> On 2024/2/9 23:15, Maxime Ripard wrote:
> > On Fri, Feb 09, 2024 at 12:02:48PM +0100, 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 <[email protected]>
> > > > > 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.
> > But there should be a single PCI device, while we have multiple "DT"
> > devices, right? Or is there several PCI devices too on that PCI card?
>
>
> There is only a single PCI(e) device on that PCI(e) card, this single
> PCI(e) device is selected as the component master. All other Hardware IP
> blocks are shipped by the single PCI(e) master. It may includes Display
> controllers, GPUs, video decoders, HDMI display bridges hardware unit etc.
>
> But all of those Hardware IP share the same MMIO registers PCI BAR, this
> PCI BAR is a kind of PCI(e) MEM resource. It is a relative *big* chunk,
> as large as 32MB in address ranges for the JingJia Macro dGPU. Therefore,
> I break the whole registers memory(MMIO) resource into smaller pieces by
> creating platform device manually, manually created platform device is
> called as virtual child in this series.
>
> In short, we cut the whole into smaller piece, each smaller piece is a
> single hardware IP block, thus deserve a single device driver. We will
> have multiple platform devices if the dGPU contains multiple hardware
> IP block. On the driver side, we bind all of the scattered driver module
> with component.
That's kind of my point then. If there's a single device, there's no
need to create intermediate devices and use the component framework to
tie them all together. You can have a simpler approach where you create
a function that takes the memory area it operates on (and whatever
additional resource it needs: interrupt, clocks, etc.) and call that
directly from the PCIe device probe, and the MMIO device bind.
Maxime
Hi,
On 2024/2/13 22:38, Maxime Ripard wrote:
> On Sat, Feb 10, 2024 at 12:25:33AM +0800, Sui Jingfeng wrote:
>> On 2024/2/9 23:15, Maxime Ripard wrote:
>>> On Fri, Feb 09, 2024 at 12:02:48PM +0100, 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 <[email protected]>
>>>>>> 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.
>>> But there should be a single PCI device, while we have multiple "DT"
>>> devices, right? Or is there several PCI devices too on that PCI card?
>>
>> There is only a single PCI(e) device on that PCI(e) card, this single
>> PCI(e) device is selected as the component master. All other Hardware IP
>> blocks are shipped by the single PCI(e) master. It may includes Display
>> controllers, GPUs, video decoders, HDMI display bridges hardware unit etc.
>>
>> But all of those Hardware IP share the same MMIO registers PCI BAR, this
>> PCI BAR is a kind of PCI(e) MEM resource. It is a relative *big* chunk,
>> as large as 32MB in address ranges for the JingJia Macro dGPU. Therefore,
>> I break the whole registers memory(MMIO) resource into smaller pieces by
>> creating platform device manually, manually created platform device is
>> called as virtual child in this series.
>>
>> In short, we cut the whole into smaller piece, each smaller piece is a
>> single hardware IP block, thus deserve a single device driver. We will
>> have multiple platform devices if the dGPU contains multiple hardware
>> IP block. On the driver side, we bind all of the scattered driver module
>> with component.
> That's kind of my point then. If there's a single device, there's no
> need to create intermediate devices and use the component framework to
> tie them all together. You can have a simpler approach where you create
> a function that takes the memory area it operates on (and whatever
> additional resource it needs: interrupt, clocks, etc.) and call that
> directly from the PCIe device probe, and the MMIO device bind.
Yes, you are right. I have implemented the method just as you told me at
V12 of this series (see 0004 patch at [1]). But at V13, I suddenly realized
that the component code path plus(+) non-component code path yield a
"side-by-side" implement. The old non-component approach can not support
binding multiple sub-core, it can only support one Vivante GPU IP core case.
But there are dGPU which shipping two identical core.
While, the component-based approach implemented at this version is the most
universal and the most flexible and extensible. We could bind a virtual
display driver and/or a real display driver to the real master (refer to the
PCI(e) device). The PCI(e) device is responsible for present the DRM service
to userspace, like a leader or agent. All other sub hardware and software are
hiding behind of the master.
Besides, Lucas asked me implement the driver like this way at V10 (see [2])
Lucas said:
"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."
Lucas, are you watching? How about this version? Can you help to review please?
[1] https://patchwork.freedesktop.org/series/127084/
[2]
https://lore.kernel.org/dri-devel/[email protected]/
> Maxime
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 <[email protected]>
>>> 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.