From: Sui Jingfeng <[email protected]>
There is a Vivante GC1000 (v5037) in LS2K1000 and LS7A1000, this GPU is a
PCI device, and it has 2D and 3D cores in the same core. Thus, this patch
set is trying to add PCI device driver support to etnaviv.
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)
Sui Jingfeng (7):
drm/etnaviv: add a dedicated function to register an irq handler
drm/etnaviv: add a dedicated function to get various clocks
drm/etnaviv: add dedicated functions to create and destroy platform
devices
drm/etnaviv: add helpers for private data construction and destruction
drm/etnaviv: allow bypass component framework
drm/etnaviv: add driver support for the PCI devices
drm/etnaviv: add support for the dma coherent device
drivers/gpu/drm/etnaviv/Kconfig | 9 +
drivers/gpu/drm/etnaviv/Makefile | 2 +
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 228 +++++++++++++++-----
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 10 +
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 22 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 7 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 170 ++++++++++-----
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 9 +
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 75 +++++++
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 9 +
include/uapi/drm/etnaviv_drm.h | 1 +
11 files changed, 422 insertions(+), 120 deletions(-)
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
--
2.25.1
From: Sui Jingfeng <[email protected]>
struct etnaviv_drm_private contains a lot of common resources that are
shared by all GPUs. This patch introduces two dedicated functions, which
is for the construction and destruction of instances of this structure.
The idea is to avoid leaking its members outside. The error handling code
can also be simplified.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 73 +++++++++++++++++----------
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 +
2 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index cec005035d0e..6a048be02857 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -24,9 +24,47 @@
#include "etnaviv_perfmon.h"
/*
- * DRM operations:
+ * etnaviv private data construction and destructions:
*/
+static struct etnaviv_drm_private *
+etnaviv_alloc_private(struct device *dev, struct drm_device *drm)
+{
+ struct etnaviv_drm_private *priv;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return ERR_PTR(-ENOMEM);
+
+ priv->drm = drm;
+
+ 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)
{
@@ -511,35 +549,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, drm);
+ 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);
@@ -551,10 +575,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);
@@ -570,12 +592,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);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index b3eb1662e90c..e58f82e698de 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;
--
2.25.1
From: Sui Jingfeng <[email protected]>
Adds another code path to allow bypass component frameworks, A platform
with a single GPU core could probably try the non-component code path.
This patch is for code sharing, no functional change.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 47 ++++++++++-----
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 84 +++++++++++++++++----------
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 3 +
3 files changed, 91 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6a048be02857..93ca240cd4c0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -536,10 +536,9 @@ static const struct drm_driver etnaviv_drm_driver = {
.minor = 3,
};
-/*
- * Platform driver:
- */
-static int etnaviv_bind(struct device *dev)
+static struct etnaviv_drm_private *etna_private_ptr;
+
+static int etnaviv_drm_bind(struct device *dev, bool component)
{
struct etnaviv_drm_private *priv;
struct drm_device *drm;
@@ -556,12 +555,15 @@ static int etnaviv_bind(struct device *dev)
}
drm->dev_private = priv;
+ etna_private_ptr = priv;
dma_set_max_seg_size(dev, SZ_2G);
- dev_set_drvdata(dev, drm);
+ if (component)
+ ret = component_bind_all(dev, drm);
+ else
+ ret = etnaviv_gpu_bind(dev, NULL, drm);
- ret = component_bind_all(dev, drm);
if (ret < 0)
goto out_free_priv;
@@ -574,7 +576,10 @@ static int etnaviv_bind(struct device *dev)
return 0;
out_unbind:
- component_unbind_all(dev, drm);
+ if (component)
+ component_unbind_all(dev, drm);
+ else
+ etnaviv_gpu_unbind(dev, NULL, drm);
out_free_priv:
etnaviv_free_private(priv);
out_put:
@@ -583,14 +588,17 @@ static int etnaviv_bind(struct device *dev)
return ret;
}
-static void etnaviv_unbind(struct device *dev)
+static void etnaviv_drm_unbind(struct device *dev, bool component)
{
- struct drm_device *drm = dev_get_drvdata(dev);
- struct etnaviv_drm_private *priv = drm->dev_private;
+ struct etnaviv_drm_private *priv = etna_private_ptr;
+ struct drm_device *drm = priv->drm;
drm_dev_unregister(drm);
- component_unbind_all(dev, drm);
+ if (component)
+ component_unbind_all(dev, drm);
+ else
+ etnaviv_gpu_unbind(dev, NULL, drm);
etnaviv_free_private(priv);
@@ -599,9 +607,22 @@ static void etnaviv_unbind(struct device *dev)
drm_dev_put(drm);
}
+/*
+ * Platform driver:
+ */
+static int etnaviv_master_bind(struct device *dev)
+{
+ return etnaviv_drm_bind(dev, true);
+}
+
+static void etnaviv_master_unbind(struct device *dev)
+{
+ return etnaviv_drm_unbind(dev, true);
+}
+
static const struct component_master_ops etnaviv_master_ops = {
- .bind = etnaviv_bind,
- .unbind = etnaviv_unbind,
+ .bind = etnaviv_master_bind,
+ .unbind = etnaviv_master_unbind,
};
static int etnaviv_pdev_probe(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 6c7aa9322468..126f0409aaed 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1737,8 +1737,7 @@ static const struct thermal_cooling_device_ops cooling_ops = {
.set_cur_state = etnaviv_gpu_cooling_set_cur_state,
};
-static int etnaviv_gpu_bind(struct device *dev, struct device *master,
- void *data)
+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;
@@ -1769,7 +1768,6 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
if (ret < 0)
goto out_sched;
-
gpu->drm = drm;
gpu->fence_context = dma_fence_context_alloc(1);
xa_init_flags(&gpu->user_fences, XA_FLAGS_ALLOC);
@@ -1798,8 +1796,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
return ret;
}
-static void etnaviv_gpu_unbind(struct device *dev, struct device *master,
- void *data)
+void etnaviv_gpu_unbind(struct device *dev, struct device *master, void *data)
{
struct etnaviv_gpu *gpu = dev_get_drvdata(dev);
@@ -1869,9 +1866,11 @@ static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq)
return 0;
}
-static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
+/* platform independent */
+
+static int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio,
+ int irq, bool component, bool has_clk)
{
- struct device *dev = &pdev->dev;
struct etnaviv_gpu *gpu;
int err;
@@ -1879,24 +1878,21 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
if (!gpu)
return -ENOMEM;
- gpu->dev = &pdev->dev;
+ gpu->dev = dev;
+ gpu->mmio = mmio;
mutex_init(&gpu->lock);
mutex_init(&gpu->sched_lock);
- /* Map registers: */
- gpu->mmio = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(gpu->mmio))
- return PTR_ERR(gpu->mmio);
-
/* Get Interrupt: */
- err = etnaviv_gpu_register_irq(gpu, platform_get_irq(pdev, 0));
+ err = etnaviv_gpu_register_irq(gpu, irq);
if (err)
return err;
- /* Get Clocks: */
- err = etnaviv_gpu_clk_get(gpu);
- if (err)
- return err;
+ if (has_clk) {
+ err = etnaviv_gpu_clk_get(gpu);
+ if (err)
+ return err;
+ }
/* TODO: figure out max mapped size */
dev_set_drvdata(dev, gpu);
@@ -1906,24 +1902,27 @@ 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);
-
- err = component_add(&pdev->dev, &gpu_ops);
- if (err < 0) {
- dev_err(&pdev->dev, "failed to register component: %d\n", err);
- return err;
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 200);
+ pm_runtime_enable(dev);
+
+ if (component) {
+ err = component_add(dev, &gpu_ops);
+ if (err < 0) {
+ dev_err(dev, "failed to register component: %d\n", err);
+ return err;
+ }
}
return 0;
}
-static int etnaviv_gpu_platform_remove(struct platform_device *pdev)
+static void etnaviv_gpu_driver_destroy(struct device *dev, bool component)
{
- component_del(&pdev->dev, &gpu_ops);
- pm_runtime_disable(&pdev->dev);
- return 0;
+ if (component)
+ component_del(dev, &gpu_ops);
+
+ pm_runtime_disable(dev);
}
static int etnaviv_gpu_rpm_suspend(struct device *dev)
@@ -1973,6 +1972,31 @@ static const struct dev_pm_ops etnaviv_gpu_pm_ops = {
RUNTIME_PM_OPS(etnaviv_gpu_rpm_suspend, etnaviv_gpu_rpm_resume, NULL)
};
+static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ void __iomem *mmio;
+ int irq;
+
+ /* Map registers: */
+ mmio = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(mmio))
+ return PTR_ERR(mmio);
+
+ irq = platform_get_irq(pdev, 0);
+
+ return etnaviv_gpu_driver_create(dev, mmio, irq, true, true);
+}
+
+static int etnaviv_gpu_platform_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ etnaviv_gpu_driver_destroy(dev, true);
+
+ return 0;
+}
+
struct platform_driver etnaviv_gpu_driver = {
.driver = {
.name = "etnaviv-gpu",
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 98c6f9c320fc..1ec829a649b5 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -206,6 +206,9 @@ void etnaviv_gpu_pm_put(struct etnaviv_gpu *gpu);
int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms);
void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32 address, u16 prefetch);
+int etnaviv_gpu_bind(struct device *dev, struct device *master, void *data);
+void etnaviv_gpu_unbind(struct device *dev, struct device *master, void *data);
+
extern struct platform_driver etnaviv_gpu_driver;
#endif /* __ETNAVIV_GPU_H__ */
--
2.25.1
From: Sui Jingfeng <[email protected]>
Because getting IRQ from a device is platform-dependent, PCI devices have
different methods for getting an IRQ. This patch is a preparation patch to
extend the driver for the PCI device support.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 34 ++++++++++++++++++++-------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index de8c9894967c..636d3f39ddcb 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1817,6 +1817,29 @@ static const struct of_device_id etnaviv_gpu_match[] = {
};
MODULE_DEVICE_TABLE(of, etnaviv_gpu_match);
+static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq)
+{
+ struct device *dev = gpu->dev;
+ int err;
+
+ if (irq < 0) {
+ dev_err(dev, "failed to get irq: %d\n", irq);
+ return irq;
+ }
+
+ err = devm_request_irq(dev, irq, irq_handler, 0, dev_name(dev), gpu);
+ if (err) {
+ dev_err(dev, "failed to request IRQ %u: %d\n", irq, err);
+ return err;
+ }
+
+ gpu->irq = irq;
+
+ dev_info(dev, "IRQ handler registered, irq = %d\n", irq);
+
+ return 0;
+}
+
static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1837,16 +1860,9 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
return PTR_ERR(gpu->mmio);
/* Get Interrupt: */
- gpu->irq = platform_get_irq(pdev, 0);
- if (gpu->irq < 0)
- return gpu->irq;
-
- err = devm_request_irq(&pdev->dev, gpu->irq, irq_handler, 0,
- dev_name(gpu->dev), gpu);
- if (err) {
- dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
+ err = etnaviv_gpu_register_irq(gpu, platform_get_irq(pdev, 0));
+ if (err)
return err;
- }
/* Get Clocks: */
gpu->clk_reg = devm_clk_get_optional(&pdev->dev, "reg");
--
2.25.1
From: Sui Jingfeng <[email protected]>
Loongson CPUs maintain cache coherency by hardware, which means that the
data in the CPU cache is identical to the data in main system memory. As
for the peripheral device, most of Loongson chips chose to define the
peripherals as DMA coherent by default, device drivers do not need to
maintain the coherency between a processor and an I/O device manually.
There are exceptions, for LS2K1000 SoC, part of peripheral device can be
configured as dma non-coherent. But there is no released version of such
firmware exist in the market. Peripherals of older ls2k1000 is also DMA
non-conherent, but they are nearly outdated. So, those are trivial cases.
Nevertheless, kernel space still need to do probe work, because vivante GPU
IP has been integrated into various platform. Hence, this patch add runtime
detection code to probe if a specific gpu is DMA coherent, If the answer is
yes, we are going to utilize such features. On Loongson platfform, When a
buffer is accesed by both the GPU and the CPU, The driver should prefer
ETNA_BO_CACHED over ETNA_BO_WC.
This patch also add a new parameter: etnaviv_param_gpu_coherent, which
allow userspace to know if such a feature is available. Because
write-combined BO is still preferred in some case, especially where don't
need CPU read, for example, uploading shader bin.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 36 +++++++++++++++++++++
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 6 ++++
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 22 ++++++++++---
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 7 +++-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 7 +++-
include/uapi/drm/etnaviv_drm.h | 1 +
6 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 033afe542a3a..3d05c33cce9f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -5,7 +5,9 @@
#include <linux/component.h>
#include <linux/dma-mapping.h>
+#include <linux/dma-map-ops.h>
#include <linux/module.h>
+#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/uaccess.h>
@@ -27,6 +29,36 @@
#include "etnaviv_pci_drv.h"
#endif
+static struct device_node *etnaviv_of_first_available_node(void)
+{
+ struct device_node *core_node;
+
+ for_each_compatible_node(core_node, NULL, "vivante,gc") {
+ if (!of_device_is_available(core_node))
+ continue;
+
+ return core_node;
+ }
+
+ return NULL;
+}
+
+static bool etnaviv_is_dma_coherent(struct device *dev)
+{
+ struct device_node *np;
+ bool coherent;
+
+ np = etnaviv_of_first_available_node();
+ if (np) {
+ coherent = of_dma_is_coherent(np);
+ of_node_put(np);
+ } else {
+ coherent = dev_is_dma_coherent(dev);
+ }
+
+ return coherent;
+}
+
/*
* etnaviv private data construction and destructions:
*/
@@ -55,6 +87,10 @@ etnaviv_alloc_private(struct device *dev, struct drm_device *drm)
return ERR_PTR(-ENOMEM);
}
+ priv->dma_coherent = etnaviv_is_dma_coherent(dev);
+
+ drm_info(drm, "%s is dma coherent\n", dev_name(dev));
+
return priv;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 9cd72948cfad..ad386ea21c3b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -46,6 +46,12 @@ struct etnaviv_drm_private {
struct xarray active_contexts;
u32 next_context_id;
+ /*
+ * If true, the GPU is capable of snoop cpu cache, here, it also
+ * means that cache coherency is enforced by the hardware.
+ */
+ bool dma_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 b5f73502e3dd..39bdc3774f2d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -343,6 +343,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);
@@ -350,8 +351,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)
@@ -369,6 +381,7 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
{
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
struct drm_device *dev = obj->dev;
+ struct etnaviv_drm_private *priv = dev->dev_private;
bool write = !!(op & ETNA_PREP_WRITE);
int ret;
@@ -395,7 +408,7 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
return ret == 0 ? -ETIMEDOUT : ret;
}
- if (etnaviv_obj->flags & ETNA_BO_CACHED) {
+ if (!priv->dma_coherent && etnaviv_obj->flags & ETNA_BO_CACHED) {
dma_sync_sgtable_for_cpu(dev->dev, etnaviv_obj->sgt,
etnaviv_op_to_dma_dir(op));
etnaviv_obj->last_cpu_prep_op = op;
@@ -408,8 +421,9 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
+ struct etnaviv_drm_private *priv = dev->dev_private;
- if (etnaviv_obj->flags & ETNA_BO_CACHED) {
+ if (!priv->dma_coherent && etnaviv_obj->flags & ETNA_BO_CACHED) {
/* fini without a prep is almost certainly a userspace error */
WARN_ON(etnaviv_obj->last_cpu_prep_op == 0);
dma_sync_sgtable_for_device(dev->dev, etnaviv_obj->sgt,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 3524b5811682..754126992264 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -112,11 +112,16 @@ static const struct etnaviv_gem_ops etnaviv_gem_prime_ops = {
struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment *attach, struct sg_table *sgt)
{
+ struct etnaviv_drm_private *priv = dev->dev_private;
struct etnaviv_gem_object *etnaviv_obj;
size_t size = PAGE_ALIGN(attach->dmabuf->size);
+ u32 cache_flags = ETNA_BO_WC;
int ret, npages;
- ret = etnaviv_gem_new_private(dev, size, ETNA_BO_WC,
+ if (priv->dma_coherent)
+ cache_flags = ETNA_BO_CACHED;
+
+ ret = etnaviv_gem_new_private(dev, size, cache_flags,
&etnaviv_gem_prime_ops, &etnaviv_obj);
if (ret < 0)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index f0eb808496e2..4dc4645a2e52 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -8,6 +8,7 @@
#include <linux/delay.h>
#include <linux/dma-fence.h>
#include <linux/dma-mapping.h>
+#include <linux/dma-map-ops.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
@@ -164,6 +165,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
*value = gpu->identity.eco_id;
break;
+ case ETNAVIV_PARAM_GPU_COHERENT:
+ *value = priv->dma_coherent;
+ break;
+
default:
DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
return -EINVAL;
@@ -1861,7 +1866,7 @@ static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq)
gpu->irq = irq;
- dev_info(dev, "IRQ handler registered, irq = %d\n", irq);
+ dev_info(dev, "irq(%d) handler registered\n", irq);
return 0;
}
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index af024d90453d..76baf45d7158 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -77,6 +77,7 @@ struct drm_etnaviv_timespec {
#define ETNAVIV_PARAM_GPU_PRODUCT_ID 0x1c
#define ETNAVIV_PARAM_GPU_CUSTOMER_ID 0x1d
#define ETNAVIV_PARAM_GPU_ECO_ID 0x1e
+#define ETNAVIV_PARAM_GPU_COHERENT 0x1f
#define ETNA_MAX_PIPES 4
--
2.25.1
From: Sui Jingfeng <[email protected]>
This patch adds PCI driver support on top of what already have. Take the
GC1000 in LS7A1000/LS2K1000 as the first instance of the PCI device driver.
There is only one GPU core for the GC1000 in the LS7A1000 and LS2K1000.
Therefore, component frameworks can be avoided.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/etnaviv/Kconfig | 9 +++
drivers/gpu/drm/etnaviv/Makefile | 2 +
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 20 +++++-
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 3 +
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 8 +--
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 ++
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 75 +++++++++++++++++++++++
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 9 +++
8 files changed, 125 insertions(+), 7 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..dbf948f99976 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -15,6 +15,15 @@ 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
+ default y
+ help
+ Compile in support for PCI GPUs of Vivante.
+ Say Y if you have such a hardware.
+
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 93ca240cd4c0..033afe542a3a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -23,6 +23,10 @@
#include "etnaviv_mmu.h"
#include "etnaviv_perfmon.h"
+#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER
+#include "etnaviv_pci_drv.h"
+#endif
+
/*
* etnaviv private data construction and destructions:
*/
@@ -538,7 +542,7 @@ static const struct drm_driver etnaviv_drm_driver = {
static struct etnaviv_drm_private *etna_private_ptr;
-static int etnaviv_drm_bind(struct device *dev, bool component)
+int etnaviv_drm_bind(struct device *dev, bool component)
{
struct etnaviv_drm_private *priv;
struct drm_device *drm;
@@ -588,7 +592,7 @@ static int etnaviv_drm_bind(struct device *dev, bool component)
return ret;
}
-static void etnaviv_drm_unbind(struct device *dev, bool component)
+void etnaviv_drm_unbind(struct device *dev, bool component)
{
struct etnaviv_drm_private *priv = etna_private_ptr;
struct drm_device *drm = priv->drm;
@@ -746,6 +750,12 @@ static int __init etnaviv_init(void)
if (ret != 0)
goto unregister_gpu_driver;
+#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER
+ ret = etnaviv_register_pci_driver();
+ if (ret != 0)
+ goto unregister_platform_driver;
+#endif
+
/*
* If the DT contains at least one available GPU device, instantiate
* the DRM platform device.
@@ -763,7 +773,7 @@ static int __init etnaviv_init(void)
break;
}
- return 0;
+ return ret;
unregister_platform_driver:
platform_driver_unregister(&etnaviv_platform_driver);
@@ -778,6 +788,10 @@ static void __exit etnaviv_exit(void)
etnaviv_destroy_platform_device(&etnaviv_platform_device);
platform_driver_unregister(&etnaviv_platform_driver);
platform_driver_unregister(&etnaviv_gpu_driver);
+
+#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER
+ etnaviv_unregister_pci_driver();
+#endif
}
module_exit(etnaviv_exit);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index e58f82e698de..9cd72948cfad 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -83,6 +83,9 @@ 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_drm_bind(struct device *dev, bool component);
+void etnaviv_drm_unbind(struct device *dev, bool component);
+
#ifdef CONFIG_DEBUG_FS
void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
struct seq_file *m);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 126f0409aaed..f0eb808496e2 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1868,8 +1868,8 @@ static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq)
/* platform independent */
-static int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio,
- int irq, bool component, bool has_clk)
+int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio,
+ int irq, bool component, bool has_clk)
{
struct etnaviv_gpu *gpu;
int err;
@@ -1917,7 +1917,7 @@ static int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio,
return 0;
}
-static void etnaviv_gpu_driver_destroy(struct device *dev, bool component)
+void etnaviv_gpu_driver_destroy(struct device *dev, bool component)
{
if (component)
component_del(dev, &gpu_ops);
@@ -1968,7 +1968,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev)
return 0;
}
-static const struct dev_pm_ops etnaviv_gpu_pm_ops = {
+const struct dev_pm_ops etnaviv_gpu_pm_ops = {
RUNTIME_PM_OPS(etnaviv_gpu_rpm_suspend, etnaviv_gpu_rpm_resume, NULL)
};
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 1ec829a649b5..8d9833996ed7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -209,6 +209,12 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32 address, u16 prefetch);
int etnaviv_gpu_bind(struct device *dev, struct device *master, void *data);
void etnaviv_gpu_unbind(struct device *dev, struct device *master, void *data);
+int etnaviv_gpu_driver_create(struct device *dev, void __iomem *mmio,
+ int irq, bool component, bool has_clk);
+
+void etnaviv_gpu_driver_destroy(struct device *dev, bool component);
+
extern struct platform_driver etnaviv_gpu_driver;
+extern const struct dev_pm_ops etnaviv_gpu_pm_ops;
#endif /* __ETNAVIV_GPU_H__ */
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..5de3e218274a
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/pci.h>
+
+#include "etnaviv_drv.h"
+#include "etnaviv_gpu.h"
+#include "etnaviv_pci_drv.h"
+
+static int etnaviv_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct device *dev = &pdev->dev;
+ void __iomem *mmio;
+ int ret;
+
+ ret = pcim_enable_device(pdev);
+ if (ret) {
+ dev_err(&pdev->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;
+
+ /* Map registers, assume the PCI bar 0 contain the registers */
+ mmio = pcim_iomap(pdev, 0, 0);
+ if (IS_ERR(mmio))
+ return PTR_ERR(mmio);
+
+ ret = etnaviv_gpu_driver_create(dev, mmio, pdev->irq, false, false);
+ if (ret)
+ return ret;
+
+ return etnaviv_drm_bind(dev, false);
+}
+
+static void etnaviv_pci_remove(struct pci_dev *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ etnaviv_drm_unbind(dev, false);
+
+ etnaviv_gpu_driver_destroy(dev, false);
+
+ pci_clear_master(pdev);
+}
+
+static const struct pci_device_id etnaviv_pci_id_lists[] = {
+ {PCI_VENDOR_ID_LOONGSON, 0x7a15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+ {PCI_VENDOR_ID_LOONGSON, 0x7a05, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+ { }
+};
+
+static struct pci_driver etnaviv_pci_driver = {
+ .name = "etnaviv",
+ .id_table = etnaviv_pci_id_lists,
+ .probe = etnaviv_pci_probe,
+ .remove = etnaviv_pci_remove,
+ .driver.pm = pm_ptr(&etnaviv_gpu_pm_ops),
+};
+
+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..25d07bdd2ea0
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ETNAVIV_PCI_DRV_H__
+#define __ETNAVIV_PCI_DRV_H__
+
+int etnaviv_register_pci_driver(void);
+void etnaviv_unregister_pci_driver(void);
+
+#endif
--
2.25.1
Hi,
Sorry for sending duplicated patches. Please don't get angry by me.
I'm just sending a patch set, which contain 7 patch. The command is as
following:
git send-email V7/ --to "Lucas Stach <[email protected]>" --to
"Christian Gmeiner <[email protected]>" --to "Daniel Vetter
<[email protected]>" --to "Bjorn Helgaas <[email protected]>" --cc
"[email protected]" --cc "[email protected]" --cc
"[email protected]" --cc "[email protected]"
After run the above about command with my company's mailbox, the
termial report "Too many commands"
which cause the last few patch of the whole patch set did not get send
out(get lost).
Then, I changed to another mail to send the patch with the same
command, then its works finally.
Please don't get angry by me. Thanks.
On 2023/6/3 18:59, Sui Jingfeng wrote:
> From: Sui Jingfeng <[email protected]>
>
> There is a Vivante GC1000 (v5037) in LS2K1000 and LS7A1000, this GPU is a
> PCI device, and it has 2D and 3D cores in the same core. Thus, this patch
> set is trying to add PCI device driver support to etnaviv.
>
> 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)
>
> Sui Jingfeng (7):
> drm/etnaviv: add a dedicated function to register an irq handler
> drm/etnaviv: add a dedicated function to get various clocks
> drm/etnaviv: add dedicated functions to create and destroy platform
> devices
> drm/etnaviv: add helpers for private data construction and destruction
> drm/etnaviv: allow bypass component framework
> drm/etnaviv: add driver support for the PCI devices
> drm/etnaviv: add support for the dma coherent device
>
> drivers/gpu/drm/etnaviv/Kconfig | 9 +
> drivers/gpu/drm/etnaviv/Makefile | 2 +
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 228 +++++++++++++++-----
> drivers/gpu/drm/etnaviv/etnaviv_drv.h | 10 +
> drivers/gpu/drm/etnaviv/etnaviv_gem.c | 22 +-
> drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 7 +-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 170 ++++++++++-----
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 9 +
> drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 75 +++++++
> drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 9 +
> include/uapi/drm/etnaviv_drm.h | 1 +
> 11 files changed, 422 insertions(+), 120 deletions(-)
> create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
> create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
>
--
Jingfeng
On Sat, Jun 03, 2023 at 06:59:43PM +0800, Sui Jingfeng wrote:
> From: Sui Jingfeng <[email protected]>
>
> Loongson CPUs maintain cache coherency by hardware, which means that the
> data in the CPU cache is identical to the data in main system memory. As
> for the peripheral device, most of Loongson chips chose to define the
> peripherals as DMA coherent by default, device drivers do not need to
> maintain the coherency between a processor and an I/O device manually.
>
> There are exceptions, for LS2K1000 SoC, part of peripheral device can be
> configured as dma non-coherent. But there is no released version of such
> firmware exist in the market. Peripherals of older ls2k1000 is also DMA
> non-conherent, but they are nearly outdated. So, those are trivial cases.
s/dma/DMA/
s/non-conherent/non-coherent/
s/ls2k1000/LS2K1000/
I guess when you say these are "trivial cases," you mean you don't
care about supporting those devices?
> Nevertheless, kernel space still need to do probe work, because vivante GPU
> IP has been integrated into various platform. Hence, this patch add runtime
> detection code to probe if a specific gpu is DMA coherent, If the answer is
> yes, we are going to utilize such features. On Loongson platfform, When a
> buffer is accesed by both the GPU and the CPU, The driver should prefer
> ETNA_BO_CACHED over ETNA_BO_WC.
s/gpu/GPU/
s/platfform/platform/
s/accesed/accessed/
I guess the only way to discover this coherency attribute is via the
DT "vivante,gc" property? Seems a little weird but I'm really not a
DT person.
> This patch also add a new parameter: etnaviv_param_gpu_coherent, which
> allow userspace to know if such a feature is available. Because
> write-combined BO is still preferred in some case, especially where don't
> need CPU read, for example, uploading shader bin.
> ...
> +static struct device_node *etnaviv_of_first_available_node(void)
> +{
> + struct device_node *core_node;
> +
> + for_each_compatible_node(core_node, NULL, "vivante,gc") {
> + if (!of_device_is_available(core_node))
> + continue;
> +
> + return core_node;
> + }
> +
> + return NULL;
Seems like this would be simpler as:
for_each_compatible_node(core_node, NULL, "vivante,gc") {
if (of_device_is_available(core_node))
return core_node;
}
return NULL;
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -8,6 +8,7 @@
> #include <linux/delay.h>
> #include <linux/dma-fence.h>
> #include <linux/dma-mapping.h>
> +#include <linux/dma-map-ops.h>
It looks like this #include might not be needed? You're only adding a
new reference to priv->dma_coherent, which looks like it was added to
etnaviv_drv.h.
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> @@ -164,6 +165,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
> *value = gpu->identity.eco_id;
> break;
>
> + case ETNAVIV_PARAM_GPU_COHERENT:
> + *value = priv->dma_coherent;
> + break;
> +
> default:
> DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
> return -EINVAL;
> @@ -1861,7 +1866,7 @@ static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq)
>
> gpu->irq = irq;
>
> - dev_info(dev, "IRQ handler registered, irq = %d\n", irq);
> + dev_info(dev, "irq(%d) handler registered\n", irq);
Looks possibly unnecessary, or at least unrelated to this patch.
> return 0;
> }
Hi, I love your reviews
On 2023/6/7 00:56, Bjorn Helgaas wrote:
> On Sat, Jun 03, 2023 at 06:59:43PM +0800, Sui Jingfeng wrote:
>> From: Sui Jingfeng <[email protected]>
>>
>> Loongson CPUs maintain cache coherency by hardware, which means that the
>> data in the CPU cache is identical to the data in main system memory. As
>> for the peripheral device, most of Loongson chips chose to define the
>> peripherals as DMA coherent by default, device drivers do not need to
>> maintain the coherency between a processor and an I/O device manually.
>>
>> There are exceptions, for LS2K1000 SoC, part of peripheral device can be
>> configured as dma non-coherent. But there is no released version of such
>> firmware exist in the market. Peripherals of older ls2k1000 is also DMA
>> non-conherent, but they are nearly outdated. So, those are trivial cases.
> s/dma/DMA/
> s/non-conherent/non-coherent/
> s/ls2k1000/LS2K1000/
So sharpen eyes, as before. :-)
> I guess when you say these are "trivial cases," you mean you don't
> care about supporting those devices?
Not exactly, Its just that its kernel side thing.
the kernel side should tell us whether a peripheral device is dma
coherent or not.
I do try to support the LSDC/GC1000 as DMA non-coherent peripheral
device in the past,
It's no fun, Only helps to study knowledge, experiment, testing and
comparison(with the dma coherent case).
It requires me compile the PMON firmware on myself. And flash it to the
ROM the board.
change firmware is complex, there a lot of address windows and cross
bar(control
a access either go the cached path or go the non cached path) setting which
only firmware engineer know about.
If there a user want ask me to do it, I will try to test this driver on
such old chip again.
Now, I believe that the support is *already* there.
As etnaviv already works on DMA non-coherent platform originally.
The DC in Loongson Mips/LoongArch SoC can scanout from cached system RAM.
low-end application relay on CPU software rendering only.
There no need to do something like flush cache(write the data in the cache
back to system ram), invalid the cache. This is pretty convenient.
The old Ingenic SoC(jz4770 for example, mips32) also has vivante gpu
integrated.
but they are dma non-coherent, see more addition material at [1].
Therefore, still need to consider the support from cross(various)
devices perspective.
[1] https://lkml.org/lkml/2021/5/15/177
>> Nevertheless, kernel space still need to do probe work, because vivante GPU
>> IP has been integrated into various platform. Hence, this patch add runtime
>> detection code to probe if a specific gpu is DMA coherent, If the answer is
>> yes, we are going to utilize such features. On Loongson platfform, When a
>> buffer is accesed by both the GPU and the CPU, The driver should prefer
>> ETNA_BO_CACHED over ETNA_BO_WC.
> s/gpu/GPU/
> s/platfform/platform/
> s/accesed/accessed/
OK, will be fixed at next version,
I don't find this on myself. :-(
> I guess the only way to discover this coherency attribute is via the
> DT "vivante,gc" property? Seems a little weird but I'm really not a
> DT person.
I'm not sure it is *only*, but it is very convenient to achieve such a
thing with DT.
Just need to put the "dma-coherent;" or "dma-noncoherent" inside the
"vivante,gc" node.
see of_dma_is_coherent() function.
DT is flexible. But this is just used to describe the hardware, it don't
not changing the hardware.
Put any property only has a influence to the software or driver side.
The hardware still as it is.
Changing hardware to a different working mode probably still need the
firmware(uefi, uboot, pmon etc) to do it
>> This patch also add a new parameter: etnaviv_param_gpu_coherent, which
>> allow userspace to know if such a feature is available. Because
>> write-combined BO is still preferred in some case, especially where don't
>> need CPU read, for example, uploading shader bin.
>> ...
>> +static struct device_node *etnaviv_of_first_available_node(void)
>> +{
>> + struct device_node *core_node;
>> +
>> + for_each_compatible_node(core_node, NULL, "vivante,gc") {
>> + if (!of_device_is_available(core_node))
>> + continue;
>> +
>> + return core_node;
>> + }
>> +
>> + return NULL;
> Seems like this would be simpler as:
>
> for_each_compatible_node(core_node, NULL, "vivante,gc") {
> if (of_device_is_available(core_node))
> return core_node;
> }
>
> return NULL;
Indeed, I don't realize this when I create this patch.
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> @@ -8,6 +8,7 @@
>> #include <linux/delay.h>
>> #include <linux/dma-fence.h>
>> #include <linux/dma-mapping.h>
>> +#include <linux/dma-map-ops.h>
> It looks like this #include might not be needed?
No,
the dev_is_dma_coherent() function is declared and defined in dma-map-ops.h.
if remove it, gcc will complain:
drivers/gpu/drm/etnaviv/etnaviv_drv.c: In function
‘etnaviv_is_dma_coherent’:
drivers/gpu/drm/etnaviv/etnaviv_drv.c:56:14: error: implicit declaration
of function ‘dev_is_dma_coherent’; did you mean
‘etnaviv_is_dma_coherent’? [-Werror=implicit-function-declaration]
56 | coherent = dev_is_dma_coherent(dev);
| ^~~~~~~~~~~~~~~~~~~
> You're only adding a
> new reference to priv->dma_coherent, which looks like it was added to
> etnaviv_drv.h.
>
>> #include <linux/module.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> @@ -164,6 +165,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
>> *value = gpu->identity.eco_id;
>> break;
>>
>> + case ETNAVIV_PARAM_GPU_COHERENT:
>> + *value = priv->dma_coherent;
>> + break;
>> +
>> default:
>> DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
>> return -EINVAL;
>> @@ -1861,7 +1866,7 @@ static int etnaviv_gpu_register_irq(struct etnaviv_gpu *gpu, int irq)
>>
>> gpu->irq = irq;
>>
>> - dev_info(dev, "IRQ handler registered, irq = %d\n", irq);
>> + dev_info(dev, "irq(%d) handler registered\n", irq);
> Looks possibly unnecessary, or at least unrelated to this patch.
Indeed, catched by you again.
>> return 0;
>> }
--
Jingfeng
On Wed, Jun 07, 2023 at 02:43:27AM +0800, Sui Jingfeng wrote:
> On 2023/6/7 00:56, Bjorn Helgaas wrote:
> > On Sat, Jun 03, 2023 at 06:59:43PM +0800, Sui Jingfeng wrote:
> > > From: Sui Jingfeng <[email protected]>
> > >
> > > Loongson CPUs maintain cache coherency by hardware, which means that the
> > > data in the CPU cache is identical to the data in main system memory. As
> > > for the peripheral device, most of Loongson chips chose to define the
> > > peripherals as DMA coherent by default, device drivers do not need to
> > > maintain the coherency between a processor and an I/O device manually.
> > ...
> > I guess the only way to discover this coherency attribute is via the
> > DT "vivante,gc" property? Seems a little weird but I'm really not a
> > DT person.
>
> I'm not sure it is *only*, but it is very convenient to achieve such a thing
> with DT.
I don't know if this is a property of the PCI device, or a property of
the system as a whole. I asked because PCI devices should be
self-describing (the Device/Vendor ID should be enough to identify the
correct driver, and the driver should know how to learn anything else
it needs to know about the device from PCI config space) and should
not require extra DT properties.
But if this is a CPU or system property, you probably have to use a
firmware interface like DT or ACPI.
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > @@ -8,6 +8,7 @@
> > > #include <linux/delay.h>
> > > #include <linux/dma-fence.h>
> > > #include <linux/dma-mapping.h>
> > > +#include <linux/dma-map-ops.h>
> >
> > It looks like this #include might not be needed?
>
> No, the dev_is_dma_coherent() function is declared and defined in
> dma-map-ops.h. if remove it, gcc will complain:
>
> drivers/gpu/drm/etnaviv/etnaviv_drv.c: In function
> ‘etnaviv_is_dma_coherent’:
> drivers/gpu/drm/etnaviv/etnaviv_drv.c:56:14: error: implicit declaration of
> function ‘dev_is_dma_coherent’; did you mean ‘etnaviv_is_dma_coherent’?
> [-Werror=implicit-function-declaration]
> 56 | coherent = dev_is_dma_coherent(dev);
> | ^~~~~~~~~~~~~~~~~~~
Of course, but that warning is for etnaviv_drv.c, not for
etnaviv_gpu.c. So etnaviv_drv.c needs to include dma-map-ops.h, but I
don't think etnaviv_gpu.c does. I removed this #include from
etnaviv_gpu.c and it still built without errors.
> > You're only adding a
> > new reference to priv->dma_coherent, which looks like it was added to
> > etnaviv_drv.h.