Re: [PATCH] drm/etnaviv: Create an accel device node if compute-only
Hi Tomeu,
>
> If we expose a render node for NPUs without rendering capabilities, the
> userspace stack will offer it to compositors and applications for
> rendering, which of course won't work.
>
> Userspace is probably right in not questioning whether a render node
> might not be capable of supporting rendering, so change it in the kernel
> instead by exposing a /dev/accel node.
>
> Before we bring the device up we don't know whether it is capable of
> rendering or not (depends on the features of its blocks), so first try
> to probe a rendering node, and if we find out that there is no rendering
> hardware, abort and retry with an accel node.
>
I really love this idea of moving away from a render node. What needs to be done
on the userspace side?
> Signed-off-by: Tomeu Vizoso
> Cc: Oded Gabbay
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 46 ++-
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..8e7dd23115f4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -11,6 +11,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -488,10 +489,10 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
> ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> };
>
> -DEFINE_DRM_GEM_FOPS(fops);
> +DEFINE_DRM_GEM_FOPS(render_fops);
> +DEFINE_DRM_ACCEL_FOPS(accel_fops);
>
> -static const struct drm_driver etnaviv_drm_driver = {
> - .driver_features= DRIVER_GEM | DRIVER_RENDER,
> +static struct drm_driver etnaviv_drm_driver = {
> .open = etnaviv_open,
> .postclose = etnaviv_postclose,
> .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> @@ -500,7 +501,6 @@ static const struct drm_driver etnaviv_drm_driver = {
> #endif
> .ioctls = etnaviv_ioctls,
> .num_ioctls = DRM_ETNAVIV_NUM_IOCTLS,
> - .fops = ,
> .name = "etnaviv",
> .desc = "etnaviv DRM",
> .date = "20151214",
> @@ -508,15 +508,20 @@ static const struct drm_driver etnaviv_drm_driver = {
> .minor = 4,
> };
>
> -/*
> - * Platform driver:
> - */
> -static int etnaviv_bind(struct device *dev)
> +static int etnaviv_bind_with_type(struct device *dev, u32 type)
> {
> struct etnaviv_drm_private *priv;
> struct drm_device *drm;
> + bool is_compute_only = true;
> int ret;
>
> + etnaviv_drm_driver.driver_features = DRIVER_GEM | type;
> +
> + if (type == DRIVER_RENDER)
> + etnaviv_drm_driver.fops = _fops;
> + else
> + etnaviv_drm_driver.fops = _fops;
> +
> drm = drm_dev_alloc(_drm_driver, dev);
> if (IS_ERR(drm))
> return PTR_ERR(drm);
> @@ -553,6 +558,18 @@ static int etnaviv_bind(struct device *dev)
>
> load_gpu(drm);
>
> + for (unsigned int i = 0; i < ETNA_MAX_PIPES; i++) {
> + struct etnaviv_gpu *g = priv->gpu[i];
> +
> + if (g && (g->identity.minor_features8 &
> chipMinorFeatures8_COMPUTE_ONLY) == 0)
> + is_compute_only = false;
> + }
> +
> + if (type == DRIVER_RENDER && is_compute_only) {
> + ret = -EINVAL;
> + goto out_unbind;
> + }
> +
> ret = drm_dev_register(drm, 0);
> if (ret)
> goto out_unbind;
> @@ -571,6 +588,19 @@ static int etnaviv_bind(struct device *dev)
> return ret;
> }
>
> +/*
> + * Platform driver:
> + */
> +static int etnaviv_bind(struct device *dev)
> +{
> + int ret = etnaviv_bind_with_type(dev, DRIVER_RENDER);
> +
> + if (ret == -EINVAL)
> + return etnaviv_bind_with_type(dev, DRIVER_COMPUTE_ACCEL);
> +
> + return ret;
> +}
> +
> static void etnaviv_unbind(struct device *dev)
> {
> struct drm_device *drm = dev_get_drvdata(dev);
> --
> 2.44.0
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
[PATCH] Revert "drm/etnaviv: Expose a few more chipspecs to userspace"
From: Christian Gmeiner
This reverts commit 1dccdba084897443d116508a8ed71e0ac8a031a4.
In userspace a different approach was choosen - hwdb. As a result, there
is no need for these values.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 20 ---
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 12 -
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 34 --
include/uapi/drm/etnaviv_drm.h | 5
4 files changed, 71 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 734412aae94d..e47e5562535a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -164,26 +164,6 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32
param, u64 *value)
*value = gpu->identity.eco_id;
break;
- case ETNAVIV_PARAM_GPU_NN_CORE_COUNT:
- *value = gpu->identity.nn_core_count;
- break;
-
- case ETNAVIV_PARAM_GPU_NN_MAD_PER_CORE:
- *value = gpu->identity.nn_mad_per_core;
- break;
-
- case ETNAVIV_PARAM_GPU_TP_CORE_COUNT:
- *value = gpu->identity.tp_core_count;
- break;
-
- case ETNAVIV_PARAM_GPU_ON_CHIP_SRAM_SIZE:
- *value = gpu->identity.on_chip_sram_size;
- break;
-
- case ETNAVIV_PARAM_GPU_AXI_SRAM_SIZE:
- *value = gpu->identity.axi_sram_size;
- break;
-
default:
DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
return -EINVAL;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 7d5e9158e13c..197e0037732e 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -54,18 +54,6 @@ struct etnaviv_chip_identity {
/* Number of Neural Network cores. */
u32 nn_core_count;
- /* Number of MAD units per Neural Network core. */
- u32 nn_mad_per_core;
-
- /* Number of Tensor Processing cores. */
- u32 tp_core_count;
-
- /* Size in bytes of the SRAM inside the NPU. */
- u32 on_chip_sram_size;
-
- /* Size in bytes of the SRAM across the AXI bus. */
- u32 axi_sram_size;
-
/* Size of the vertex cache. */
u32 vertex_cache_size;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index d8e7334de8ce..8665f2658d51 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -17,10 +17,6 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
.thread_count = 128,
.shader_core_count = 1,
.nn_core_count = 0,
- .nn_mad_per_core = 0,
- .tp_core_count = 0,
- .on_chip_sram_size = 0,
- .axi_sram_size = 0,
.vertex_cache_size = 8,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -52,11 +48,6 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 256,
.shader_core_count = 1,
- .nn_core_count = 0,
- .nn_mad_per_core = 0,
- .tp_core_count = 0,
- .on_chip_sram_size = 0,
- .axi_sram_size = 0,
.vertex_cache_size = 8,
.vertex_output_buffer_size = 512,
.pixel_pipes = 1,
@@ -89,10 +80,6 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
.thread_count = 512,
.shader_core_count = 2,
.nn_core_count = 0,
- .nn_mad_per_core = 0,
- .tp_core_count = 0,
- .on_chip_sram_size = 0,
- .axi_sram_size = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -125,10 +112,6 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
.thread_count = 512,
.shader_core_count = 2,
.nn_core_count = 0,
- .nn_mad_per_core = 0,
- .tp_core_count = 0,
- .on_chip_sram_size = 0,
- .axi_sram_size = 0,
.vertex_cache_size = 16,
.vertex_output_buffer_size = 1024,
.pixel_pipes = 1,
@@ -160,11 +143,6 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
.register_max = 64,
.thread_count = 512,
.shader_core_count = 2,
- .nn_core_count = 0,
- .nn_mad_per_core = 0,
- .tp_core_count = 0,
- .on_chip_sram_size = 0,
-
Re: [PATCH v3 2/2] misc: sram: Add DMA-BUF Heap exporting of SRAM areas
Am Di., 9. Apr. 2024 um 14:14 Uhr schrieb Greg Kroah-Hartman : > > On Tue, Apr 09, 2024 at 02:06:05PM +0200, Pascal FONTAIN wrote: > > From: Andrew Davis > > > > This new export type exposes to userspace the SRAM area as a DMA-BUF > > Heap, > > this allows for allocations of DMA-BUFs that can be consumed by various > > DMA-BUF supporting devices. > > > > Signed-off-by: Andrew Davis > > Tested-by: Pascal Fontain > > When sending on a patch from someone else, you too must sign off on it > as per our documenation. Please read it and understand what you are > agreeing to when you do that for a new version please. > > > --- > > drivers/misc/Kconfig | 7 + > > drivers/misc/Makefile| 1 + > > drivers/misc/sram-dma-heap.c | 246 +++ > > drivers/misc/sram.c | 6 + > > drivers/misc/sram.h | 16 +++ > > 5 files changed, 276 insertions(+) > > create mode 100644 drivers/misc/sram-dma-heap.c > > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > > index 9e4ad4d61f06..e6674e913168 100644 > > --- a/drivers/misc/Kconfig > > +++ b/drivers/misc/Kconfig > > @@ -448,6 +448,13 @@ config SRAM > > config SRAM_EXEC > > bool > > > > +config SRAM_DMA_HEAP > > + bool "Export on-chip SRAM pools using DMA-Heaps" > > + depends on DMABUF_HEAPS && SRAM > > + help > > + This driver allows the export of on-chip SRAM marked as both pool > > + and exportable to userspace using the DMA-Heaps interface. > > What will use this in userspace? > I could imagine a way it might be used. Imagine a SoC like TI's AM64x series, where some cores (A53) run Linux and others (R5F) are managed by remoteproc to execute a RTOS. When it comes to efficiently exchanging large data sets, the conventional method involves using rpmsg, which has limitations due to message size and could potentially slow down data transfer. However, an alternative approach could be to allocate a sizable chunk of SRAM memory in userspace. By utilizing memcpy() to copy data into this memory, followed by a single rpmsg signal to notify the RTOS that the data is ready, we can leverage the faster access speed of SRAM compared to DDR from the remoteproc. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: drop driver owner assignment
>
> Core in platform_driver_register() already sets the .owner, so driver
> does not need to. Whatever is set here will be anyway overwritten by
> main driver calling platform_driver_register().
>
> Signed-off-by: Krzysztof Kozlowski
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 734412aae94d..87b31cfee41d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1995,7 +1995,6 @@ static const struct dev_pm_ops etnaviv_gpu_pm_ops = {
> struct platform_driver etnaviv_gpu_driver = {
> .driver = {
> .name = "etnaviv-gpu",
> - .owner = THIS_MODULE,
> .pm = pm_ptr(_gpu_pm_ops),
> .of_match_table = etnaviv_gpu_match,
> },
> --
> 2.34.1
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] etnaviv: Restore some id values
Hi Lucas,
>
> prefix for etnaviv kernel patches should be "drm/etnaviv: ..."
Ah yeah .. corrected in v2.
>
> Am Donnerstag, dem 29.02.2024 um 20:55 +0100 schrieb Christian Gmeiner:
> > From: Christian Gmeiner
> >
> > The hwdb selection logic as a feature that allows it to mark some fields
> > as 'don't care'. If we match with such a field we memcpy(..)
> > the current etnaviv_chip_identity into ident.
> >
> > This step can overwrite some id values read from the GPU with the
> > 'don't care' value.
> >
> > Fix this issue by restoring the affected values after the memcpy(..).
> >
> > As this is crucial for user space to know when this feature works as
> > expected increment the minor version too.
>
> Uh, right. Lying to userspace about the GPU identity doesn't seem like
> a good idea.
>
> >
> > Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Christian Gmeiner
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
> > drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 14 ++
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 6228ce603248..9a2965741dab 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
> > .desc = "etnaviv DRM",
> > .date = "20151214",
> > .major = 1,
> > - .minor = 3,
> > + .minor = 4,
> > };
> >
> > /*
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > index 67201242438b..1e38d66702f1 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > @@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity
> > etnaviv_chip_identities[] = {
> > bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> > {
> > struct etnaviv_chip_identity *ident = >identity;
> > + const u32 product_id = ident->product_id;
> > + const u32 customer_id = ident->customer_id;
> > + const u32 eco_id = ident->eco_id;
> > int i;
> >
> > for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
> > @@ -278,6 +281,17 @@ bool etnaviv_fill_identity_from_hwdb(struct
> > etnaviv_gpu *gpu)
> >etnaviv_chip_identities[i].eco_id == ~0U)) {
> > memcpy(ident, _chip_identities[i],
> > sizeof(*ident));
> > +
> > + /* Restore some id values if ~0U aka 'don't care' is
> > used. */
> > + if (etnaviv_chip_identities[i].product_id == ~0U)
>
> You can drop all those if clauses. Either the hwdb value is an exact
> match and the value is the same or it's the don't care value and you
> want to restore it, so a simple unconditional assignment would make
> this a bit more compact.
>
Makes sense - thanks for your review!
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
[PATCH v2] drm/etnaviv: Restore some id values
From: Christian Gmeiner
The hwdb selection logic as a feature that allows it to mark some fields
as 'don't care'. If we match with such a field we memcpy(..)
the current etnaviv_chip_identity into ident.
This step can overwrite some id values read from the GPU with the
'don't care' value.
Fix this issue by restoring the affected values after the memcpy(..).
As this is crucial for user space to know when this feature works as
expected increment the minor version too.
Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
Cc: sta...@vger.kernel.org
Signed-off-by: Christian Gmeiner
---
V1 -> V2: Fixed patch subject line and removed not needed if clauses.
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 9 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6228ce603248..9a2965741dab 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
.desc = "etnaviv DRM",
.date = "20151214",
.major = 1,
- .minor = 3,
+ .minor = 4,
};
/*
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 67201242438b..8665f2658d51 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
{
struct etnaviv_chip_identity *ident = >identity;
+ const u32 product_id = ident->product_id;
+ const u32 customer_id = ident->customer_id;
+ const u32 eco_id = ident->eco_id;
int i;
for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
@@ -278,6 +281,12 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu
*gpu)
etnaviv_chip_identities[i].eco_id == ~0U)) {
memcpy(ident, _chip_identities[i],
sizeof(*ident));
+
+ /* Restore some id values as ~0U aka 'don't care' might
been used. */
+ ident->product_id = product_id;
+ ident->customer_id = customer_id;
+ ident->eco_id = eco_id;
+
return true;
}
}
--
2.44.0
[PATCH] etnaviv: Restore some id values
From: Christian Gmeiner
The hwdb selection logic as a feature that allows it to mark some fields
as 'don't care'. If we match with such a field we memcpy(..)
the current etnaviv_chip_identity into ident.
This step can overwrite some id values read from the GPU with the
'don't care' value.
Fix this issue by restoring the affected values after the memcpy(..).
As this is crucial for user space to know when this feature works as
expected increment the minor version too.
Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
Cc: sta...@vger.kernel.org
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 14 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6228ce603248..9a2965741dab 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
.desc = "etnaviv DRM",
.date = "20151214",
.major = 1,
- .minor = 3,
+ .minor = 4,
};
/*
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 67201242438b..1e38d66702f1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity
etnaviv_chip_identities[] = {
bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
{
struct etnaviv_chip_identity *ident = >identity;
+ const u32 product_id = ident->product_id;
+ const u32 customer_id = ident->customer_id;
+ const u32 eco_id = ident->eco_id;
int i;
for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
@@ -278,6 +281,17 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu
*gpu)
etnaviv_chip_identities[i].eco_id == ~0U)) {
memcpy(ident, _chip_identities[i],
sizeof(*ident));
+
+ /* Restore some id values if ~0U aka 'don't care' is
used. */
+ if (etnaviv_chip_identities[i].product_id == ~0U)
+ ident->product_id = product_id;
+
+ if (etnaviv_chip_identities[i].customer_id == ~0U)
+ ident->customer_id = customer_id;
+
+ if (etnaviv_chip_identities[i].eco_id == ~0U)
+ ident->eco_id = eco_id;
+
return true;
}
}
--
2.44.0
Re: [PATCH v2 2/3] drm/etnaviv: Turn etnaviv_is_model_rev() into a function
etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> }
>
> /* Another special case */
> - if (etnaviv_is_model_rev(gpu, GC300, 0x2201)) {
> + if (etnaviv_is_model_rev(gpu, 0x300, 0x2201)) {
> u32 chipTime = gpu_read(gpu, VIVS_HI_CHIP_TIME);
>
> if (chipDate == 0x20080814 && chipTime == 0x12051100)
> {
> @@ -387,15 +389,15 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> * Fix model/rev here, so all other places can refer to this
> * core by its real identity.
> */
> - if (etnaviv_is_model_rev(gpu, GC2000, 0x5450)) {
> + if (etnaviv_is_model_rev(gpu, 0x2000, 0x5450)) {
> gpu->identity.model = chipModel_GC3000;
> gpu->identity.revision &= 0x;
> }
>
> - if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate ==
> 0x20120617))
> + if (etnaviv_is_model_rev(gpu, 0x1000, 0x5037) && (chipDate ==
> 0x20120617))
> gpu->identity.eco_id = 1;
>
> - if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate ==
> 0x20140511))
> + if (etnaviv_is_model_rev(gpu, 0x320, 0x5303) && (chipDate ==
> 0x20140511))
> gpu->identity.eco_id = 1;
> }
>
> @@ -630,14 +632,14 @@ static void etnaviv_gpu_enable_mlcg(struct etnaviv_gpu
> *gpu)
> pmc |= BIT(15); /* Unknown bit */
>
> /* Disable TX clock gating on affected core revisions. */
> - if (etnaviv_is_model_rev(gpu, GC4000, 0x5222) ||
> - etnaviv_is_model_rev(gpu, GC2000, 0x5108) ||
> - etnaviv_is_model_rev(gpu, GC2000, 0x6202) ||
> - etnaviv_is_model_rev(gpu, GC2000, 0x6203))
> + if (etnaviv_is_model_rev(gpu, 0x4000, 0x5222) ||
> + etnaviv_is_model_rev(gpu, 0x2000, 0x5108) ||
> + etnaviv_is_model_rev(gpu, 0x2000, 0x6202) ||
> + etnaviv_is_model_rev(gpu, 0x2000, 0x6203))
> pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_TX;
>
> /* Disable SE and RA clock gating on affected core revisions. */
> - if (etnaviv_is_model_rev(gpu, GC7000, 0x6202))
> + if (etnaviv_is_model_rev(gpu, 0x7000, 0x6202))
> pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_SE
> |
>VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA;
>
> @@ -690,14 +692,14 @@ static void etnaviv_gpu_setup_pulse_eater(struct
> etnaviv_gpu *gpu)
> */
> u32 pulse_eater = 0x01590880;
>
> - if (etnaviv_is_model_rev(gpu, GC4000, 0x5208) ||
> - etnaviv_is_model_rev(gpu, GC4000, 0x5222)) {
> + if (etnaviv_is_model_rev(gpu, 0x4000, 0x5208) ||
> + etnaviv_is_model_rev(gpu, 0x4000, 0x5222)) {
> pulse_eater |= BIT(23);
>
> }
>
> - if (etnaviv_is_model_rev(gpu, GC1000, 0x5039) ||
> - etnaviv_is_model_rev(gpu, GC1000, 0x5040)) {
> + if (etnaviv_is_model_rev(gpu, 0x1000, 0x5039) ||
> + etnaviv_is_model_rev(gpu, 0x1000, 0x5040)) {
> pulse_eater &= ~BIT(16);
> pulse_eater |= BIT(17);
> }
> @@ -718,8 +720,8 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
> WARN_ON(!(gpu->state == ETNA_GPU_STATE_IDENTIFIED ||
> gpu->state == ETNA_GPU_STATE_RESET));
>
> - if ((etnaviv_is_model_rev(gpu, GC320, 0x5007) ||
> -etnaviv_is_model_rev(gpu, GC320, 0x5220)) &&
> + if ((etnaviv_is_model_rev(gpu, 0x320, 0x5007) ||
> +etnaviv_is_model_rev(gpu, 0x320, 0x5220)) &&
> gpu_read(gpu, VIVS_HI_CHIP_TIME) != 0x2062400) {
> u32 mc_memory_debug;
>
> @@ -745,7 +747,7 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
> VIVS_HI_AXI_CONFIG_ARCACHE(2));
>
> /* GC2000 rev 5108 needs a special bus config */
> - if (etnaviv_is_model_rev(gpu, GC2000, 0x5108)) {
> + if (etnaviv_is_model_rev(gpu, 0x2000, 0x5108)) {
> u32 bus_config = gpu_read(gpu, VIVS_MC_BUS_CONFIG);
> bus_config &= ~(VIVS_MC_BUS_CONFIG_FE_BUS_CONFIG__MASK |
> VIVS_MC_BUS_CONFIG_TX_BUS_CONFIG__MASK);
>
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH v2 1/3] drm/etnaviv: Update hardware headers from rnndb
>
> Update the state HI header from the rnndb commit
> 8d7ee714cfe2 ("Merge pull request #24 from pH5/unknown-3950").
>
> Signed-off-by: Philipp Zabel
You missed my R-b from the v1 series for this patch - please include
it the next time!
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/cmdstream.xml.h | 52 ++--
> drivers/gpu/drm/etnaviv/common.xml.h| 12 ++--
> drivers/gpu/drm/etnaviv/state.xml.h | 101
> +++-
> drivers/gpu/drm/etnaviv/state_blt.xml.h | 20 +++
> drivers/gpu/drm/etnaviv/state_hi.xml.h | 28 +
> 5 files changed, 170 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/cmdstream.xml.h
> b/drivers/gpu/drm/etnaviv/cmdstream.xml.h
> index 65f1ba1099bd..a96597a27ae2 100644
> --- a/drivers/gpu/drm/etnaviv/cmdstream.xml.h
> +++ b/drivers/gpu/drm/etnaviv/cmdstream.xml.h
> @@ -8,11 +8,11 @@ This file was generated by the rules-ng-ng headergen tool
> in this git repository
> git clone git://0x04.net/rules-ng-ng
>
> The rules-ng-ng source files this header was generated from are:
> -- cmdstream.xml ( 14094 bytes, from 2016-11-11 06:55:14)
> -- copyright.xml ( 1597 bytes, from 2016-10-29 07:29:22)
> -- common.xml( 23344 bytes, from 2016-11-10 15:14:07)
> +- cmdstream.xml ( 16933 bytes, from 2023-12-11 15:50:17)
> +- copyright.xml ( 1597 bytes, from 2016-11-10 13:58:32)
> +- common.xml( 35664 bytes, from 2023-12-06 10:55:32)
>
> -Copyright (C) 2012-2016 by the following authors:
> +Copyright (C) 2012-2023 by the following authors:
> - Wladimir J. van der Laan
> - Christian Gmeiner
> - Lucas Stach
> @@ -52,6 +52,9 @@ DEALINGS IN THE SOFTWARE.
> #define FE_OPCODE_RETURN 0x000b
> #define FE_OPCODE_DRAW_INSTANCED 0x000c
> #define FE_OPCODE_CHIP_SELECT 0x000d
> +#define FE_OPCODE_WAIT_FENCE 0x000f
> +#define FE_OPCODE_DRAW_INDIRECT
> 0x0010
> +#define FE_OPCODE_SNAP_PAGES 0x0013
> #define PRIMITIVE_TYPE_POINTS 0x0001
> #define PRIMITIVE_TYPE_LINES 0x0002
> #define PRIMITIVE_TYPE_LINE_STRIP 0x0003
> @@ -192,6 +195,9 @@ DEALINGS IN THE SOFTWARE.
> #define VIV_FE_STALL_TOKEN_TO__MASK0x1f00
> #define VIV_FE_STALL_TOKEN_TO__SHIFT 8
> #define VIV_FE_STALL_TOKEN_TO(x) (((x) <<
> VIV_FE_STALL_TOKEN_TO__SHIFT) & VIV_FE_STALL_TOKEN_TO__MASK)
> +#define VIV_FE_STALL_TOKEN_UNK28__MASK 0x3000
> +#define VIV_FE_STALL_TOKEN_UNK28__SHIFT28
> +#define VIV_FE_STALL_TOKEN_UNK28(x)(((x) <<
> VIV_FE_STALL_TOKEN_UNK28__SHIFT) & VIV_FE_STALL_TOKEN_UNK28__MASK)
>
> #define VIV_FE_CALL0x
>
> @@ -266,5 +272,43 @@ DEALINGS IN THE SOFTWARE.
> #define VIV_FE_DRAW_INSTANCED_START_INDEX__SHIFT 0
> #define VIV_FE_DRAW_INSTANCED_START_INDEX(x) (((x) <<
> VIV_FE_DRAW_INSTANCED_START_INDEX__SHIFT) &
> VIV_FE_DRAW_INSTANCED_START_INDEX__MASK)
>
> +#define VIV_FE_WAIT_FENCE 0x
> +
> +#define VIV_FE_WAIT_FENCE_HEADER 0x
> +#define VIV_FE_WAIT_FENCE_HEADER_OP__MASK 0xf800
> +#define VIV_FE_WAIT_FENCE_HEADER_OP__SHIFT 27
> +#define VIV_FE_WAIT_FENCE_HEADER_OP_WAIT_FENCE 0x7800
> +#define VIV_FE_WAIT_FENCE_HEADER_UNK16__MASK 0x0003
> +#define VIV_FE_WAIT_FENCE_HEADER_UNK16__SHIFT 16
> +#define VIV_FE_WAIT_FENCE_HEADER_UNK16(x) (((x) <<
> VIV_FE_WAIT_FENCE_HEADER_UNK16__SHIFT) & VIV_FE_WAIT_FENCE_HEADER_UNK16__MASK)
> +#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__MASK 0x
> +#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__SHIFT 0
> +#define VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT(x) (((x) <<
> VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__SHIFT) &
> VIV_FE_WAIT_FENCE_HEADER_WAITCOUNT__MASK)
> +
> +#define VIV_FE_WAIT_FENCE_ADDRESS 0x0004
> +
> +#define VIV_FE_DRAW_INDIRECT 0x
> +
> +#define VIV_FE_DRAW_INDIRECT_HEADER0x
> +#define VIV_FE_DRAW_INDIR
Re: [PATCH 0/2] drm/etnaviv: Disable SH_EU clock gating on the i.MX8MP NPU
Am Mi., 24. Jan. 2024 um 10:22 Uhr schrieb Philipp Zabel : > > The vendor kernel sets a previously unknown clock gating bit in the > VIVS_PM_MODULE_CONTROLS register to disable SH_EU clock gating. > > Import new headers from rnndb for the definition and set the bit > for the VIPNano-Si+ NPU on i.MX8MP. > > Signed-off-by: Philipp Zabel Thanks - series is Reviewed-by: Christian Gmeiner > --- > Philipp Zabel (2): > drm/etnaviv: Update hardware headers from rnndb > drm/etnaviv: Disable SH_EU clock gating on VIPNano-Si+ > > drivers/gpu/drm/etnaviv/cmdstream.xml.h | 52 ++-- > drivers/gpu/drm/etnaviv/common.xml.h| 12 ++-- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 ++ > drivers/gpu/drm/etnaviv/state.xml.h | 101 > +++- > drivers/gpu/drm/etnaviv/state_blt.xml.h | 20 +++ > drivers/gpu/drm/etnaviv/state_hi.xml.h | 28 + > 6 files changed, 174 insertions(+), 43 deletions(-) > --- > base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d > change-id: 20240124-etnaviv-npu-627f6881322c > > Best regards, > -- > Philipp Zabel > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: add sensitive state for PE_RT_ADDR_4_PIPE(3, 0|1) address
Hi Lucas > > Am Donnerstag, dem 07.12.2023 um 08:43 +0100 schrieb Christian Gmeiner: > > From: Christian Gmeiner > > > > 0x1540 is the address of 4th render target address pair (two pixel pipes). > > > Are there any corresponding TS states for this region? I will add some more ranges .. Do you want one patch per "region" or one patch with everything? -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
[PATCH] drm/etnaviv: add sensitive state for PE_RT_ADDR_4_PIPE(3, 0|1) address
From: Christian Gmeiner
0x1540 is the address of 4th render target address pair (two pixel pipes).
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c
b/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c
index b106e8b288ad..9bf47327f436 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_cmd_parser.c
@@ -54,6 +54,7 @@ static const struct {
ST(0x1480, 8),
ST(0x1500, 8),
ST(0x1520, 8),
+ ST(0x1540, 8),
ST(0x1608, 1),
ST(0x1610, 1),
ST(0x1658, 1),
--
2.43.0
Re: [PATCH v2] drm/etnaviv: Expose a few more chipspecs to userspace
Am Di., 21. Nov. 2023 um 07:33 Uhr schrieb Tomeu Vizoso :
>
> These ones will be needed to make use fo the NN and TP units in the NPUs
> based on Vivante IP.
>
> Also fix the number of NN cores in the VIPNano-qi.
>
> Signed-off-by: Tomeu Vizoso
>
I have not checked all the new values but it looks fine to me.
Acked-by: Christian Gmeiner
> ---
>
> v2: Update a few chipspecs that I had missed before. (Christian)
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 20 +++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 12 +
> drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 34 ++
> include/uapi/drm/etnaviv_drm.h | 5
> 4 files changed, 71 insertions(+)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 9276756e1397..9055ed08cd7b 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -164,6 +164,26 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32
> param, u64 *value)
> *value = gpu->identity.eco_id;
> break;
>
> + case ETNAVIV_PARAM_GPU_NN_CORE_COUNT:
> + *value = gpu->identity.nn_core_count;
> + break;
> +
> + case ETNAVIV_PARAM_GPU_NN_MAD_PER_CORE:
> + *value = gpu->identity.nn_mad_per_core;
> + break;
> +
> + case ETNAVIV_PARAM_GPU_TP_CORE_COUNT:
> + *value = gpu->identity.tp_core_count;
> + break;
> +
> + case ETNAVIV_PARAM_GPU_ON_CHIP_SRAM_SIZE:
> + *value = gpu->identity.on_chip_sram_size;
> + break;
> +
> + case ETNAVIV_PARAM_GPU_AXI_SRAM_SIZE:
> + *value = gpu->identity.axi_sram_size;
> + break;
> +
> default:
> DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 197e0037732e..7d5e9158e13c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -54,6 +54,18 @@ struct etnaviv_chip_identity {
> /* Number of Neural Network cores. */
> u32 nn_core_count;
>
> + /* Number of MAD units per Neural Network core. */
> + u32 nn_mad_per_core;
> +
> + /* Number of Tensor Processing cores. */
> + u32 tp_core_count;
> +
> + /* Size in bytes of the SRAM inside the NPU. */
> + u32 on_chip_sram_size;
> +
> + /* Size in bytes of the SRAM across the AXI bus. */
> + u32 axi_sram_size;
> +
> /* Size of the vertex cache. */
> u32 vertex_cache_size;
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index 67201242438b..9eb8ca7c5034 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -17,6 +17,10 @@ static const struct etnaviv_chip_identity
> etnaviv_chip_identities[] = {
> .thread_count = 128,
> .shader_core_count = 1,
> .nn_core_count = 0,
> + .nn_mad_per_core = 0,
> + .tp_core_count = 0,
> + .on_chip_sram_size = 0,
> + .axi_sram_size = 0,
> .vertex_cache_size = 8,
> .vertex_output_buffer_size = 1024,
> .pixel_pipes = 1,
> @@ -48,6 +52,11 @@ static const struct etnaviv_chip_identity
> etnaviv_chip_identities[] = {
> .register_max = 64,
> .thread_count = 256,
> .shader_core_count = 1,
> + .nn_core_count = 0,
> + .nn_mad_per_core = 0,
> + .tp_core_count = 0,
> + .on_chip_sram_size = 0,
> + .axi_sram_size = 0,
> .vertex_cache_size = 8,
> .vertex_output_buffer_size = 512,
> .pixel_pipes = 1,
> @@ -80,6 +89,10 @@ static const struct etnaviv_chip_identity
> etnaviv_chip_identities[] = {
> .thread_count = 512,
> .shader_core_count = 2,
> .nn_core_count = 0,
> + .nn_mad_per_core = 0,
> + .tp_core_count = 0,
> + .on_chip_sram_size = 0,
> + .axi_sram_size = 0,
> .vertex_cache_size = 16,
> .vertex_output_buffer_size = 1024,
> .pixel_pipes = 1,
> @@ -112,6 +125,10 @@ static const struct etnaviv_chip_identity
> etnaviv_chip_
Re: [PATCH] drm/etnaviv: Expose a few more chipspecs to userspace
size = 0,
> + .axi_sram_size = 0,
> .vertex_cache_size = 16,
> .vertex_output_buffer_size = 1024,
> .pixel_pipes = 2,
> @@ -144,7 +160,11 @@ static const struct etnaviv_chip_identity
> etnaviv_chip_identities[] = {
> .register_max = 64,
> .thread_count = 256,
> .shader_core_count = 1,
> - .nn_core_count = 1,
> + .nn_core_count = 8,
> + .nn_mad_per_core = 64,
> + .tp_core_count = 4,
> + .on_chip_sram_size = 524288,
> + .axi_sram_size = 1048576,
> .vertex_cache_size = 16,
> .vertex_output_buffer_size = 1024,
> .pixel_pipes = 1,
I am counting 8 entries in the etnaviv_chip_identities array and you
are just updating 5 of them.
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index af024d90453d..d87410a8443a 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -77,6 +77,11 @@ struct drm_etnaviv_timespec {
> #define ETNAVIV_PARAM_GPU_PRODUCT_ID0x1c
> #define ETNAVIV_PARAM_GPU_CUSTOMER_ID 0x1d
> #define ETNAVIV_PARAM_GPU_ECO_ID0x1e
> +#define ETNAVIV_PARAM_GPU_NN_CORE_COUNT 0x1f
> +#define ETNAVIV_PARAM_GPU_NN_MAD_PER_CORE 0x20
> +#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 ETNA_MAX_PIPES 4
>
> --
> 2.41.0
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH v2] drm/etnaviv: refactor deprecated strncpy
Am Fr., 6. Okt. 2023 um 22:23 Uhr schrieb Kees Cook : > > On Mon, Sep 18, 2023 at 01:34:08PM +, Justin Stitt wrote: > > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > > > We should prefer more robust and less ambiguous string interfaces. > > > > A suitable replacement is `strscpy_pad` due to the fact that it > > guarantees NUL-termination on the destination buffer whilst maintaining > > the NUL-padding behavior that strncpy provides. > > Friend ping. Who can pick this change up? > Lucas is the one who is responsible for this job. > > Thanks! > > -Kees > > > > > Link: > > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > > [1] > > Link: https://github.com/KSPP/linux/issues/90 > > Cc: linux-harden...@vger.kernel.org > > Cc: Bo YU > > Signed-off-by: Justin Stitt Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH v2 02/12] drm/etnaviv: Convert to platform remove callback returning void
Uwe Kleine-König schrieb am Di., 1. Aug.
2023, 13:02:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
>
> Trivially convert the etnaviv drm driver from always returning zero in
> the remove callback to the void returning variant.
>
> Reviewed-by: Thomas Zimmermann
> Reviewed-by: Jyri Sarha
> Signed-off-by: Uwe Kleine-König
>
Reviewed-by: Christian Gmeiner
---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 6 ++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 5 ++---
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index ea55f6b7b744..20305df01b57 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -638,16 +638,14 @@ static int etnaviv_pdev_probe(struct platform_device
> *pdev)
> return component_master_add_with_match(dev, _master_ops,
> match);
> }
>
> -static int etnaviv_pdev_remove(struct platform_device *pdev)
> +static void etnaviv_pdev_remove(struct platform_device *pdev)
> {
> component_master_del(>dev, _master_ops);
> -
> - return 0;
> }
>
> static struct platform_driver etnaviv_platform_driver = {
> .probe = etnaviv_pdev_probe,
> - .remove = etnaviv_pdev_remove,
> + .remove_new = etnaviv_pdev_remove,
> .driver = {
> .name = "etnaviv",
> },
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index bbc9c54871f4..43426ffe4d77 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1892,11 +1892,10 @@ static int etnaviv_gpu_platform_probe(struct
> platform_device *pdev)
> return 0;
> }
>
> -static int etnaviv_gpu_platform_remove(struct platform_device *pdev)
> +static void etnaviv_gpu_platform_remove(struct platform_device *pdev)
> {
> component_del(>dev, _ops);
> pm_runtime_disable(>dev);
> - return 0;
> }
>
> static int etnaviv_gpu_rpm_suspend(struct device *dev)
> @@ -1954,6 +1953,6 @@ struct platform_driver etnaviv_gpu_driver = {
> .of_match_table = etnaviv_gpu_match,
> },
> .probe = etnaviv_gpu_platform_probe,
> - .remove = etnaviv_gpu_platform_remove,
> + .remove_new = etnaviv_gpu_platform_remove,
> .id_table = gpu_ids,
> };
> --
> 2.39.2
>
>
Re: [PATCH] drm/etnaviv: fix dumping of active MMU context
Hi Lucas, Am Mi., 21. Juni 2023 um 17:44 Uhr schrieb Christian Gmeiner : > > Hi Lucas, > > Am Mo., 17. Apr. 2023 um 19:42 Uhr schrieb Christian Gmeiner > : > > > > Hi Lucas > > > > > > > > gpu->mmu_context is the MMU context of the last job in the HW queue, which > > > isn't necessarily the same as the context from the bad job. Dump the MMU > > > context from the scheduler determined bad submit to make it work as > > > intended. > > > > > > > Good catch! > > > > I think this patch did not land yet. Do you have plans to add it to > etnaviv/next? > I have seen you added it today - great! -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: fix dumping of active MMU context
Hi Lucas,
Am Mo., 17. Apr. 2023 um 19:42 Uhr schrieb Christian Gmeiner
:
>
> Hi Lucas
>
> >
> > gpu->mmu_context is the MMU context of the last job in the HW queue, which
> > isn't necessarily the same as the context from the bad job. Dump the MMU
> > context from the scheduler determined bad submit to make it work as
> > intended.
> >
>
> Good catch!
>
I think this patch did not land yet. Do you have plans to add it to
etnaviv/next?
> > Fixes: 17e4660ae3d7 ("drm/etnaviv: implement per-process address spaces on
> > MMUv2")
> > Signed-off-by: Lucas Stach
>
> Reviewed-by: Christian Gmeiner
>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_dump.c | 14 +++---
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> > index 44b5f3c35aab..898f84a0fc30 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> > @@ -130,9 +130,9 @@ void etnaviv_core_dump(struct etnaviv_gem_submit
> > *submit)
> > return;
> > etnaviv_dump_core = false;
> >
> > - mutex_lock(>mmu_context->lock);
> > + mutex_lock(>mmu_context->lock);
> >
> > - mmu_size = etnaviv_iommu_dump_size(gpu->mmu_context);
> > + mmu_size = etnaviv_iommu_dump_size(submit->mmu_context);
> >
> > /* We always dump registers, mmu, ring, hanging cmdbuf and end
> > marker */
> > n_obj = 5;
> > @@ -162,7 +162,7 @@ void etnaviv_core_dump(struct etnaviv_gem_submit
> > *submit)
> > iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
> > __GFP_NORETRY);
> > if (!iter.start) {
> > - mutex_unlock(>mmu_context->lock);
> > + mutex_unlock(>mmu_context->lock);
> > dev_warn(gpu->dev, "failed to allocate devcoredump file\n");
> > return;
> > }
> > @@ -174,18 +174,18 @@ void etnaviv_core_dump(struct etnaviv_gem_submit
> > *submit)
> > memset(iter.hdr, 0, iter.data - iter.start);
> >
> > etnaviv_core_dump_registers(, gpu);
> > - etnaviv_core_dump_mmu(, gpu->mmu_context, mmu_size);
> > + etnaviv_core_dump_mmu(, submit->mmu_context, mmu_size);
> > etnaviv_core_dump_mem(, ETDUMP_BUF_RING, gpu->buffer.vaddr,
> > gpu->buffer.size,
> > etnaviv_cmdbuf_get_va(>buffer,
> > - >mmu_context->cmdbuf_mapping));
> > +
> > >mmu_context->cmdbuf_mapping));
> >
> > etnaviv_core_dump_mem(, ETDUMP_BUF_CMD,
> > submit->cmdbuf.vaddr, submit->cmdbuf.size,
> > etnaviv_cmdbuf_get_va(>cmdbuf,
> > - >mmu_context->cmdbuf_mapping));
> > +
> > >mmu_context->cmdbuf_mapping));
> >
> > - mutex_unlock(>mmu_context->lock);
> > + mutex_unlock(>mmu_context->lock);
> >
> > /* Reserve space for the bomap */
> > if (n_bomap_pages) {
> > --
> > 2.39.2
> >
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH v10 00/11] drm/etnaviv: Add pci device driver support
Hi > > From: Sui Jingfeng > > 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. This series is > trying to add PCI device driver support to drm/etnaviv. > Is it possible to get the lspci output for the GPU? Something like this: sudo lspci -vvv -s ... thanks -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH 8/8] drm/etnaviv: expedited MMU fault handling
Hi Lucas
>
> The GPU is halted when it hits a MMU exception, so there is no point in
> waiting for the job timeout to expire or try to work out if the GPU is
> still making progress in the timeout handler, as we know that the GPU
> won't make any more progress.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 ++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 +
> drivers/gpu/drm/etnaviv/etnaviv_sched.c | 5 +++--
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index e62761032afe..74fdcaf52fc5 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1531,6 +1531,8 @@ static irqreturn_t irq_handler(int irq, void *data)
>
> if (intr & VIVS_HI_INTR_ACKNOWLEDGE_MMU_EXCEPTION) {
> dump_mmu_fault(gpu);
> + gpu->state = ETNA_GPU_STATE_FAULT;
> + drm_sched_fault(>sched);
> intr &= ~VIVS_HI_INTR_ACKNOWLEDGE_MMU_EXCEPTION;
> }
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index a4a9253f0d52..d4b9a97f2c72 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -101,6 +101,7 @@ enum etnaviv_gpu_state {
> ETNA_GPU_STATE_RESET,
> ETNA_GPU_STATE_INITIALIZED,
> ETNA_GPU_STATE_RUNNING,
> + ETNA_GPU_STATE_FAULT,
> };
>
> struct etnaviv_gpu {
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> index 1ae87dfd19c4..345fec6cb1a4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> @@ -55,8 +55,9 @@ static enum drm_gpu_sched_stat
> etnaviv_sched_timedout_job(struct drm_sched_job
> */
> dma_addr = gpu_read(gpu, VIVS_FE_DMA_ADDRESS);
> change = dma_addr - gpu->hangcheck_dma_addr;
> - if (gpu->completed_fence != gpu->hangcheck_fence ||
> - change < 0 || change > 16) {
> + if (gpu->state == ETNA_GPU_STATE_RUNNING &&
> + (gpu->completed_fence != gpu->hangcheck_fence ||
> +change < 0 || change > 16)) {
> gpu->hangcheck_dma_addr = dma_addr;
> gpu->hangcheck_fence = gpu->completed_fence;
> goto out_no_timeout;
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 7/8] drm/etnaviv: drop GPU initialized property
Hi Lucas
Am Mi., 7. Juni 2023 um 15:02 Uhr schrieb Lucas Stach :
>
> Now that it is only used to track the driver internal state of
> the MMU global and cmdbuf objects, we can get rid of this property
> by making the free/finit functions of those objects safe to call
> on an uninitialized object.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 3 +++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 9 ++---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 1 -
> drivers/gpu/drm/etnaviv/etnaviv_mmu.c| 3 +++
> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> index 9dc20d892c15..721d633aece9 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> @@ -121,6 +121,9 @@ void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf)
> int order = order_base_2(ALIGN(cmdbuf->size, SUBALLOC_GRANULE) /
> SUBALLOC_GRANULE);
>
> + if (!suballoc)
> + return;
> +
> mutex_lock(>lock);
> bitmap_release_region(suballoc->granule_map,
> cmdbuf->suballoc_offset / SUBALLOC_GRANULE,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 96cbb290b869..e62761032afe 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -868,8 +868,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> pm_runtime_mark_last_busy(gpu->dev);
> pm_runtime_put_autosuspend(gpu->dev);
>
> - gpu->initialized = true;
> -
> return 0;
>
> fail:
> @@ -1797,11 +1795,8 @@ static void etnaviv_gpu_unbind(struct device *dev,
> struct device *master,
> if (gpu->mmu_context)
> etnaviv_iommu_context_put(gpu->mmu_context);
>
> - if (gpu->initialized) {
> - etnaviv_cmdbuf_free(>buffer);
> - etnaviv_iommu_global_fini(gpu);
> - gpu->initialized = false;
> - }
> + etnaviv_cmdbuf_free(>buffer);
> + etnaviv_iommu_global_fini(gpu);
>
> gpu->drm = NULL;
> xa_destroy(>user_fences);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 33ecc1bf84b1..a4a9253f0d52 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -114,7 +114,6 @@ struct etnaviv_gpu {
> struct mutex sched_lock;
> struct drm_gpu_scheduler sched;
> enum etnaviv_gpu_state state;
> - bool initialized;
>
> /* 'ring'-buffer: */
> struct etnaviv_cmdbuf buffer;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index 67bdce5326c6..4fa72567183a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -553,6 +553,9 @@ void etnaviv_iommu_global_fini(struct etnaviv_gpu *gpu)
> struct etnaviv_drm_private *priv = gpu->drm->dev_private;
> struct etnaviv_iommu_global *global = priv->mmu_global;
>
> + if (!global)
> + return;
> +
> if (--global->use > 0)
> return;
>
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 6/8] drm/etnaviv: better track GPU state
Hi Lucas
>
> Instead of only tracking if the FE is running, use a enum to better
> describe the various states the GPU can be in. This allows some
> additional validation to make sure that functions that expect a
> certain GPU state are only called when the GPU is actually in that
> state.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 25 ++---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 10 +-
> 2 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index fb07d0e73802..96cbb290b869 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -576,7 +576,7 @@ static int etnaviv_hw_reset(struct etnaviv_gpu *gpu)
> /* We rely on the GPU running, so program the clock */
> etnaviv_gpu_update_clock(gpu);
>
> - gpu->fe_running = false;
> + gpu->state = ETNA_GPU_STATE_RESET;
> gpu->exec_state = -1;
> if (gpu->mmu_context)
> etnaviv_iommu_context_put(gpu->mmu_context);
> @@ -651,8 +651,6 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32
> address, u16 prefetch)
> VIVS_MMUv2_SEC_COMMAND_CONTROL_ENABLE |
> VIVS_MMUv2_SEC_COMMAND_CONTROL_PREFETCH(prefetch));
> }
> -
> - gpu->fe_running = true;
> }
>
> static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
> @@ -661,6 +659,8 @@ static void etnaviv_gpu_start_fe_idleloop(struct
> etnaviv_gpu *gpu,
> u16 prefetch;
> u32 address;
>
> + WARN_ON(gpu->state != ETNA_GPU_STATE_INITIALIZED);
> +
> /* setup the MMU */
> etnaviv_iommu_restore(gpu, context);
>
> @@ -670,6 +670,8 @@ static void etnaviv_gpu_start_fe_idleloop(struct
> etnaviv_gpu *gpu,
> >mmu_context->cmdbuf_mapping);
>
> etnaviv_gpu_start_fe(gpu, address, prefetch);
> +
> + gpu->state = ETNA_GPU_STATE_RUNNING;
> }
>
> static void etnaviv_gpu_setup_pulse_eater(struct etnaviv_gpu *gpu)
> @@ -705,6 +707,9 @@ static void etnaviv_gpu_setup_pulse_eater(struct
> etnaviv_gpu *gpu)
>
> static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
> {
> + WARN_ON(!(gpu->state == ETNA_GPU_STATE_IDENTIFIED ||
> + gpu->state == ETNA_GPU_STATE_RESET));
> +
> if ((etnaviv_is_model_rev(gpu, GC320, 0x5007) ||
> etnaviv_is_model_rev(gpu, GC320, 0x5220)) &&
> gpu_read(gpu, VIVS_HI_CHIP_TIME) != 0x2062400) {
> @@ -751,6 +756,8 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
> etnaviv_gpu_setup_pulse_eater(gpu);
>
> gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
> +
> + gpu->state = ETNA_GPU_STATE_INITIALIZED;
> }
>
> int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> @@ -793,6 +800,8 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> (gpu->identity.minor_features10 &
> chipMinorFeatures10_SECURITY_AHB))
> gpu->sec_mode = ETNA_SEC_KERNEL;
>
> + gpu->state = ETNA_GPU_STATE_IDENTIFIED;
> +
> ret = etnaviv_hw_reset(gpu);
> if (ret) {
> dev_err(gpu->dev, "GPU reset failed\n");
> @@ -1368,7 +1377,7 @@ struct dma_fence *etnaviv_gpu_submit(struct
> etnaviv_gem_submit *submit)
> goto out_unlock;
> }
>
> - if (!gpu->fe_running)
> + if (gpu->state == ETNA_GPU_STATE_INITIALIZED)
> etnaviv_gpu_start_fe_idleloop(gpu, submit->mmu_context);
>
> if (submit->prev_mmu_context)
> @@ -1634,7 +1643,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu,
> unsigned int timeout_ms)
>
> static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
> {
> - if (gpu->initialized && gpu->fe_running) {
> + if (gpu->state == ETNA_GPU_STATE_RUNNING) {
> /* Replace the last WAIT with END */
> mutex_lock(>lock);
> etnaviv_buffer_end(gpu);
> @@ -1647,7 +1656,7 @@ static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu
> *gpu)
> */
> etnaviv_gpu_wait_idle(gpu, 100);
>
> - gpu->fe_running = false;
> + gpu->state = ETNA_GPU_STATE_INITIALIZED;
> }
>
> gpu->exec_state = -1;
> @@ -1918,6 +1927,8 @@ static int etnaviv_gpu_rpm_suspend(struct device *dev)
>
>
Re: [PATCH 5/8] drm/etnaviv: avoid runtime PM usage in etnaviv_gpu_bind
Hi Lucas
>
> Nothing in this callpath actually touches the GPU, so there is no reason
> to get it out of suspend state here. Only if runtime PM isn't enabled at
> all we must make sure to enable the clocks, so the GPU init routine can
> access the GPU later on.
>
> This also removes the need to guard against the state where the driver
> isn't fully initialized yet in the runtime PM resume handler.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 15 +--
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 57cf77ed2fcf..fb07d0e73802 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1735,13 +1735,11 @@ static int etnaviv_gpu_bind(struct device *dev,
> struct device *master,
> if (ret)
> goto out_workqueue;
>
> - if (IS_ENABLED(CONFIG_PM))
> - ret = pm_runtime_get_sync(gpu->dev);
> - else
> + if (!IS_ENABLED(CONFIG_PM)) {
> ret = etnaviv_gpu_clk_enable(gpu);
> - if (ret < 0)
> - goto out_sched;
> -
> + if (ret < 0)
> + goto out_sched;
> + }
>
> gpu->drm = drm;
> gpu->fence_context = dma_fence_context_alloc(1);
> @@ -1753,9 +1751,6 @@ static int etnaviv_gpu_bind(struct device *dev, struct
> device *master,
>
> priv->gpu[priv->num_gpus++] = gpu;
>
> - pm_runtime_mark_last_busy(gpu->dev);
> - pm_runtime_put_autosuspend(gpu->dev);
> -
> return 0;
>
> out_sched:
> @@ -1936,7 +1931,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev)
> return ret;
>
> /* Re-initialise the basic hardware state */
> - if (gpu->drm && gpu->initialized) {
> + if (gpu->initialized) {
> ret = etnaviv_gpu_hw_resume(gpu);
> if (ret) {
> etnaviv_gpu_clk_disable(gpu);
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 3/8] drm/etnaviv: move runtime PM handling to events
Hi Lucas,
Am Do., 15. Juni 2023 um 11:37 Uhr schrieb Lucas Stach :
>
> Hi Christian,
>
> Am Mittwoch, dem 14.06.2023 um 20:41 +0200 schrieb Christian Gmeiner:
> > Hi Lucas
> >
> > >
> > > Conceptually events are the right abstraction to handle the GPU
> > > runtime PM state: as long as any event is pending the GPU can not
> > > be idle. Events are also properly freed and reallocated when the
> > > GPU has been reset after a hang.
> > >
> > > Signed-off-by: Lucas Stach
> > > ---
> > > drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 -
> > > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 3 ---
> > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 27
> > > 3 files changed, 16 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > > b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > > index baa81cbf701a..a42d260cac2c 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > > @@ -97,7 +97,6 @@ struct etnaviv_gem_submit {
> > > struct list_head node; /* GPU active submit list */
> > > struct etnaviv_cmdbuf cmdbuf;
> > > struct pid *pid; /* submitting process */
> > > - bool runtime_resumed;
> > > u32 exec_state;
> > > u32 flags;
> > > unsigned int nr_pmrs;
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > index 45403ea38906..2416c526f9b0 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > @@ -362,9 +362,6 @@ static void submit_cleanup(struct kref *kref)
> > > container_of(kref, struct etnaviv_gem_submit,
> > > refcount);
> > > unsigned i;
> > >
> > > - if (submit->runtime_resumed)
> > > - pm_runtime_put_autosuspend(submit->gpu->dev);
> > > -
> > > if (submit->cmdbuf.suballoc)
> > > etnaviv_cmdbuf_free(>cmdbuf);
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > index 4e18aa8566c6..54a1249c5bca 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > @@ -1139,7 +1139,8 @@ static int event_alloc(struct etnaviv_gpu *gpu,
> > > unsigned nr_events,
> > > unsigned int *events)
> > > {
> > > unsigned long timeout = msecs_to_jiffies(10 * 1);
> > > - unsigned i, acquired = 0;
> > > + unsigned i, acquired = 0, rpm_count = 0;
> >
> > rpm is the short form of runtime power management?
> >
> Yes, it's used in this way in multiple places in the kernel. Do you
> think it's clear enough from the code what's going on to keep it that
> way or should I change it to a longer name?
>
Yes it is clear what is going on - even with short variable name :)
Reviewed-by: Christian Gmeiner
> Regards,
> Lucas
>
> > > + int ret;
> > >
> > > for (i = 0; i < nr_events; i++) {
> > > unsigned long ret;
> > > @@ -1148,6 +1149,7 @@ static int event_alloc(struct etnaviv_gpu *gpu,
> > > unsigned nr_events,
> > >
> > > if (!ret) {
> > > dev_err(gpu->dev, "wait_for_completion_timeout
> > > failed");
> > > + ret = -EBUSY;
> > > goto out;
> > > }
> > >
> > > @@ -1167,13 +1169,23 @@ static int event_alloc(struct etnaviv_gpu *gpu,
> > > unsigned nr_events,
> > >
> > > spin_unlock(>event_spinlock);
> > >
> > > + for (i = 0; i < nr_events; i++) {
> > > + ret = pm_runtime_resume_and_get(gpu->dev);
> > > + if (ret)
> > > + goto out_rpm;
> > > + rpm_count++;
> > > + }
> > > +
> > > return 0;
> > >
> > > +out_rpm:
> > > + for (i = 0; i < rpm_count; i++)
> > > + pm_runtime_put_autosuspend(gpu->dev);
> > > out:
> > > for (i = 0; i < acquired; i++)
>
Re: [PATCH v2] drm/etnaviv: slow down FE idle polling
>
> Currently the FE is spinning way too fast when polling for new work in
> the FE idleloop. As each poll fetches 16 bytes from memory, a GPU running
> at 1GHz with the current setting of 200 wait cycle between fetches causes
> 80 MB/s of memory traffic just to check for new work when the GPU is
> otherwise idle, which is more FE traffic than in some GPU loaded cases.
>
> Significantly increase the number of wait cycles to slow down the poll
> interval to ~30µs, limiting the FE idle memory traffic to 512 KB/s, while
> providing a max latency which should not hurt most use-cases. The FE WAIT
> command seems to have some unknown discrete steps in the wait cycles so
> we may over/undershoot the target a bit, but that should be harmless.
>
> If the GPU core base frequency is unknown keep the 200 wait cycles as
> a sane default.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> v2: Don't speed up when the GPU core frequency is unknown.
> ---
> drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 11 ++-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 8
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 1 +
> 3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> index cf741c5c82d2..384df1659be6 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> @@ -53,11 +53,12 @@ static inline void CMD_END(struct etnaviv_cmdbuf *buffer)
> OUT(buffer, VIV_FE_END_HEADER_OP_END);
> }
>
> -static inline void CMD_WAIT(struct etnaviv_cmdbuf *buffer)
> +static inline void CMD_WAIT(struct etnaviv_cmdbuf *buffer,
> + unsigned int waitcycles)
> {
> buffer->user_size = ALIGN(buffer->user_size, 8);
>
> - OUT(buffer, VIV_FE_WAIT_HEADER_OP_WAIT | 200);
> + OUT(buffer, VIV_FE_WAIT_HEADER_OP_WAIT | waitcycles);
> }
>
> static inline void CMD_LINK(struct etnaviv_cmdbuf *buffer,
> @@ -168,7 +169,7 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu)
> /* initialize buffer */
> buffer->user_size = 0;
>
> - CMD_WAIT(buffer);
> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> CMD_LINK(buffer, 2,
> etnaviv_cmdbuf_get_va(buffer,
> >mmu_context->cmdbuf_mapping)
> + buffer->user_size - 4);
> @@ -320,7 +321,7 @@ void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu,
> unsigned int event)
> CMD_END(buffer);
>
> /* Append waitlink */
> - CMD_WAIT(buffer);
> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> CMD_LINK(buffer, 2,
> etnaviv_cmdbuf_get_va(buffer,
> >mmu_context->cmdbuf_mapping)
> + buffer->user_size - 4);
> @@ -503,7 +504,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32
> exec_state,
>
> CMD_LOAD_STATE(buffer, VIVS_GL_EVENT, VIVS_GL_EVENT_EVENT_ID(event) |
>VIVS_GL_EVENT_FROM_PE);
> - CMD_WAIT(buffer);
> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> CMD_LINK(buffer, 2,
> etnaviv_cmdbuf_get_va(buffer,
> >mmu_context->cmdbuf_mapping)
> + buffer->user_size - 4);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index de8c9894967c..63b929b1e5d1 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -493,6 +493,14 @@ static void etnaviv_gpu_update_clock(struct etnaviv_gpu
> *gpu)
> clock |= VIVS_HI_CLOCK_CONTROL_FSCALE_VAL(fscale);
> etnaviv_gpu_load_clock(gpu, clock);
> }
> +
> + /*
> +* Choose number of wait cycles to target a ~30us (1/32768) max
> latency
> +* until new work is picked up by the FE when it polls in the idle
> loop.
> +* If the GPU base frequency is unknown use 200 wait cycles.
> +*/
> + gpu->fe_waitcycles = clamp(gpu->base_rate_core >> (15 -
> gpu->freq_scale),
> + 200UL, 0xUL);
> }
>
> static int etnaviv_hw_reset(struct etnaviv_gpu *gpu)
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 98c6f9c320fc..e1e1de59c38d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -150,6 +150,7 @@ struct etnaviv_gpu {
> struct clk *clk_shader;
>
> unsigned int freq_scale;
> + unsigned int fe_waitcycles;
> unsigned long base_rate_core;
> unsigned long base_rate_shader;
> };
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: drm/etnaviv: slow down FE idle polling
Hi
Am Do., 15. Juni 2023 um 11:16 Uhr schrieb Sui Jingfeng
:
>
> Hi,
>
> On 2023/6/15 17:04, Lucas Stach wrote:
> > Am Donnerstag, dem 15.06.2023 um 12:09 +0800 schrieb Sui Jingfeng:
> >> Hi,
> >>
> >> On 2023/6/7 20:59, Lucas Stach wrote:
> >>> Currently the FE is spinning way too fast when polling for new work in
> >> 'way' -> 'away'
> >>> the FE idleloop.
> >> 'idleloop' -> 'idle loop'
> >>>As each poll fetches 16 bytes from memory, a GPU running
> >>> at 1GHz with the current setting of 200 wait cycle between fetches causes
> >>> 80 MB/s of memory traffic just to check for new work when the GPU is
> >>> otherwise idle, which is more FE traffic than in some GPU loaded cases.
> >>>
> >>> Significantly increase the number of wait cycles to slow down the poll
> >>> interval to ~30µs, limiting the FE idle memory traffic to 512 KB/s, while
> >>> providing a max latency which should not hurt most use-cases. The FE WAIT
> >>> command seems to have some unknown discrete steps in the wait cycles
> >> add a comma here.
> >>>so
> >>> we may over/undershoot the target a bit, but that should be harmless.
> >> overshoot or undershoot
> >>> Signed-off-by: Lucas Stach
> >>> Reviewed-by: Christian Gmeiner
> >>> ---
> >>>drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 11 ++-
> >>>drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 7 +++
> >>>drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 1 +
> >>>3 files changed, 14 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> >>> b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> >>> index cf741c5c82d2..384df1659be6 100644
> >>> --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> >>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> >>> @@ -53,11 +53,12 @@ static inline void CMD_END(struct etnaviv_cmdbuf
> >>> *buffer)
> >>> OUT(buffer, VIV_FE_END_HEADER_OP_END);
> >>>}
> >>>
> >>> -static inline void CMD_WAIT(struct etnaviv_cmdbuf *buffer)
> >>> +static inline void CMD_WAIT(struct etnaviv_cmdbuf *buffer,
> >>> + unsigned int waitcycles)
> >>>{
> >>> buffer->user_size = ALIGN(buffer->user_size, 8);
> >>>
> >>> - OUT(buffer, VIV_FE_WAIT_HEADER_OP_WAIT | 200);
> >>> + OUT(buffer, VIV_FE_WAIT_HEADER_OP_WAIT | waitcycles);
> >>>}
> >>>
> >>>static inline void CMD_LINK(struct etnaviv_cmdbuf *buffer,
> >>> @@ -168,7 +169,7 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu)
> >>> /* initialize buffer */
> >>> buffer->user_size = 0;
> >>>
> >>> - CMD_WAIT(buffer);
> >>> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> >>> CMD_LINK(buffer, 2,
> >>> etnaviv_cmdbuf_get_va(buffer,
> >>> >mmu_context->cmdbuf_mapping)
> >>> + buffer->user_size - 4);
> >>> @@ -320,7 +321,7 @@ void etnaviv_sync_point_queue(struct etnaviv_gpu
> >>> *gpu, unsigned int event)
> >>> CMD_END(buffer);
> >>>
> >>> /* Append waitlink */
> >>> - CMD_WAIT(buffer);
> >>> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> >>> CMD_LINK(buffer, 2,
> >>> etnaviv_cmdbuf_get_va(buffer,
> >>> >mmu_context->cmdbuf_mapping)
> >>> + buffer->user_size - 4);
> >>> @@ -503,7 +504,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu,
> >>> u32 exec_state,
> >>>
> >>> CMD_LOAD_STATE(buffer, VIVS_GL_EVENT, VIVS_GL_EVENT_EVENT_ID(event) |
> >>>VIVS_GL_EVENT_FROM_PE);
> >>> - CMD_WAIT(buffer);
> >>> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> >>> CMD_LINK(buffer, 2,
> >>> etnaviv_cmdbuf_get_va(buffer,
> >>> >mmu_context->cmdbuf_mapping)
> >>> + buffer->user_size - 4);
> >>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> >>> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> >>> index 41aab1aa330b..8c20dff32240 100644
> >>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> >>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> >>> @@ -493
Re: [PATCH 4/8] drm/etnaviv: make clock handling symetric between runtime resume and suspend
Hi Lucas
>
> Currently the clock is enabled in the runtime resume function, but are
> disabled a level further down in the callstack in the suspend function.
> Move the clock disable into the suspend function to make handling
> symmetrical between resume and suspend.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 54a1249c5bca..57cf77ed2fcf 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1632,7 +1632,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu,
> unsigned int timeout_ms)
> } while (1);
> }
>
> -static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
> +static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
> {
> if (gpu->initialized && gpu->fe_running) {
> /* Replace the last WAIT with END */
> @@ -1651,8 +1651,6 @@ static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu
> *gpu)
> }
>
> gpu->exec_state = -1;
> -
> - return etnaviv_gpu_clk_disable(gpu);
> }
>
> static int etnaviv_gpu_hw_resume(struct etnaviv_gpu *gpu)
> @@ -1789,6 +1787,7 @@ static void etnaviv_gpu_unbind(struct device *dev,
> struct device *master,
> pm_runtime_put_sync_suspend(gpu->dev);
> } else {
> etnaviv_gpu_hw_suspend(gpu);
> + etnaviv_gpu_clk_disable(gpu);
> }
>
> if (gpu->mmu_context)
> @@ -1922,7 +1921,9 @@ static int etnaviv_gpu_rpm_suspend(struct device *dev)
> return -EBUSY;
> }
>
> - return etnaviv_gpu_hw_suspend(gpu);
> + etnaviv_gpu_hw_suspend(gpu);
> +
> + return etnaviv_gpu_clk_disable(gpu);
> }
>
> static int etnaviv_gpu_rpm_resume(struct device *dev)
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 3/8] drm/etnaviv: move runtime PM handling to events
Hi Lucas
>
> Conceptually events are the right abstraction to handle the GPU
> runtime PM state: as long as any event is pending the GPU can not
> be idle. Events are also properly freed and reallocated when the
> GPU has been reset after a hang.
>
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 -
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 3 ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 27
> 3 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index baa81cbf701a..a42d260cac2c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -97,7 +97,6 @@ struct etnaviv_gem_submit {
> struct list_head node; /* GPU active submit list */
> struct etnaviv_cmdbuf cmdbuf;
> struct pid *pid; /* submitting process */
> - bool runtime_resumed;
> u32 exec_state;
> u32 flags;
> unsigned int nr_pmrs;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 45403ea38906..2416c526f9b0 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -362,9 +362,6 @@ static void submit_cleanup(struct kref *kref)
> container_of(kref, struct etnaviv_gem_submit,
> refcount);
> unsigned i;
>
> - if (submit->runtime_resumed)
> - pm_runtime_put_autosuspend(submit->gpu->dev);
> -
> if (submit->cmdbuf.suballoc)
> etnaviv_cmdbuf_free(>cmdbuf);
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 4e18aa8566c6..54a1249c5bca 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1139,7 +1139,8 @@ static int event_alloc(struct etnaviv_gpu *gpu,
> unsigned nr_events,
> unsigned int *events)
> {
> unsigned long timeout = msecs_to_jiffies(10 * 1);
> - unsigned i, acquired = 0;
> + unsigned i, acquired = 0, rpm_count = 0;
rpm is the short form of runtime power management?
> + int ret;
>
> for (i = 0; i < nr_events; i++) {
> unsigned long ret;
> @@ -1148,6 +1149,7 @@ static int event_alloc(struct etnaviv_gpu *gpu,
> unsigned nr_events,
>
> if (!ret) {
> dev_err(gpu->dev, "wait_for_completion_timeout
> failed");
> + ret = -EBUSY;
> goto out;
> }
>
> @@ -1167,13 +1169,23 @@ static int event_alloc(struct etnaviv_gpu *gpu,
> unsigned nr_events,
>
> spin_unlock(>event_spinlock);
>
> + for (i = 0; i < nr_events; i++) {
> + ret = pm_runtime_resume_and_get(gpu->dev);
> + if (ret)
> + goto out_rpm;
> + rpm_count++;
> + }
> +
> return 0;
>
> +out_rpm:
> + for (i = 0; i < rpm_count; i++)
> + pm_runtime_put_autosuspend(gpu->dev);
> out:
> for (i = 0; i < acquired; i++)
> complete(>event_free);
>
> - return -EBUSY;
> + return ret;
> }
>
> static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
> @@ -1185,6 +1197,8 @@ static void event_free(struct etnaviv_gpu *gpu,
> unsigned int event)
> clear_bit(event, gpu->event_bitmap);
> complete(>event_free);
> }
> +
> + pm_runtime_put_autosuspend(gpu->dev);
> }
>
> /*
> @@ -1327,15 +1341,6 @@ struct dma_fence *etnaviv_gpu_submit(struct
> etnaviv_gem_submit *submit)
> unsigned int i, nr_events = 1, event[3];
> int ret;
>
> - if (!submit->runtime_resumed) {
> - ret = pm_runtime_get_sync(gpu->dev);
> - if (ret < 0) {
> - pm_runtime_put_noidle(gpu->dev);
> - return NULL;
> - }
> - submit->runtime_resumed = true;
> - }
> -
> /*
> * if there are performance monitor requests we need to have
> * - a sync point to re-configure gpu and process ETNA_PM_PROCESS_PRE
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 2/8] drm/etnaviv: free events the usual way in recover worker
Hi Lucas > > Clearing the whole bitmap at once is only a minor optimization in > a path that should be extremely cold. Free the events by calling > event_free() instead of directly manipulating the completion count > and event bitmap. > > Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 6d4df9f1aeff..4e18aa8566c6 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -1442,8 +1442,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit > *submit) > /* complete all events, the GPU won't do it after the reset */ > spin_lock(>event_spinlock); > for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS) > - complete(>event_free); > - bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS); > + event_free(gpu, i); > spin_unlock(>event_spinlock); > > etnaviv_gpu_hw_init(gpu); > -- > 2.39.2 > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH 1/8] drm/etnaviv: move down etnaviv_gpu_recover_hang() in file
Hi Lucas
>
> So it can use the event_free function without adding another
> forward declaration. No functional change.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 88 +--
> 1 file changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index de8c9894967c..6d4df9f1aeff 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1059,50 +1059,6 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu,
> struct seq_file *m)
> }
> #endif
>
> -void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
> -{
> - struct etnaviv_gpu *gpu = submit->gpu;
> - char *comm = NULL, *cmd = NULL;
> - struct task_struct *task;
> - unsigned int i;
> -
> - dev_err(gpu->dev, "recover hung GPU!\n");
> -
> - task = get_pid_task(submit->pid, PIDTYPE_PID);
> - if (task) {
> - comm = kstrdup(task->comm, GFP_KERNEL);
> - cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
> - put_task_struct(task);
> - }
> -
> - if (comm && cmd)
> - dev_err(gpu->dev, "offending task: %s (%s)\n", comm, cmd);
> -
> - kfree(cmd);
> - kfree(comm);
> -
> - if (pm_runtime_get_sync(gpu->dev) < 0)
> - goto pm_put;
> -
> - mutex_lock(>lock);
> -
> - etnaviv_hw_reset(gpu);
> -
> - /* complete all events, the GPU won't do it after the reset */
> - spin_lock(>event_spinlock);
> - for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
> - complete(>event_free);
> - bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
> - spin_unlock(>event_spinlock);
> -
> - etnaviv_gpu_hw_init(gpu);
> -
> - mutex_unlock(>lock);
> - pm_runtime_mark_last_busy(gpu->dev);
> -pm_put:
> - pm_runtime_put_autosuspend(gpu->dev);
> -}
> -
> /* fence object management */
> struct etnaviv_fence {
> struct etnaviv_gpu *gpu;
> @@ -1454,6 +1410,50 @@ static void sync_point_worker(struct work_struct *work)
> etnaviv_gpu_start_fe(gpu, addr + 2, 2);
> }
>
> +void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
> +{
> + struct etnaviv_gpu *gpu = submit->gpu;
> + char *comm = NULL, *cmd = NULL;
> + struct task_struct *task;
> + unsigned int i;
> +
> + dev_err(gpu->dev, "recover hung GPU!\n");
> +
> + task = get_pid_task(submit->pid, PIDTYPE_PID);
> + if (task) {
> + comm = kstrdup(task->comm, GFP_KERNEL);
> + cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
> + put_task_struct(task);
> + }
> +
> + if (comm && cmd)
> + dev_err(gpu->dev, "offending task: %s (%s)\n", comm, cmd);
> +
> + kfree(cmd);
> + kfree(comm);
> +
> + if (pm_runtime_get_sync(gpu->dev) < 0)
> + goto pm_put;
> +
> + mutex_lock(>lock);
> +
> + etnaviv_hw_reset(gpu);
> +
> + /* complete all events, the GPU won't do it after the reset */
> + spin_lock(>event_spinlock);
> + for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
> + complete(>event_free);
> + bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
> + spin_unlock(>event_spinlock);
> +
> + etnaviv_gpu_hw_init(gpu);
> +
> + mutex_unlock(>lock);
> + pm_runtime_mark_last_busy(gpu->dev);
> +pm_put:
> + pm_runtime_put_autosuspend(gpu->dev);
> +}
> +
> static void dump_mmu_fault(struct etnaviv_gpu *gpu)
> {
> static const char *fault_reasons[] = {
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: slow down FE idle polling
Hi Lucas
>
> Currently the FE is spinning way too fast when polling for new work in
> the FE idleloop. As each poll fetches 16 bytes from memory, a GPU running
> at 1GHz with the current setting of 200 wait cycle between fetches causes
> 80 MB/s of memory traffic just to check for new work when the GPU is
> otherwise idle, which is more FE traffic than in some GPU loaded cases.
>
> Significantly increase the number of wait cycles to slow down the poll
> interval to ~30µs, limiting the FE idle memory traffic to 512 KB/s, while
> providing a max latency which should not hurt most use-cases. The FE WAIT
> command seems to have some unknown discrete steps in the wait cycles so
> we may over/undershoot the target a bit, but that should be harmless.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 11 ++-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 7 +++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 1 +
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> index cf741c5c82d2..384df1659be6 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> @@ -53,11 +53,12 @@ static inline void CMD_END(struct etnaviv_cmdbuf *buffer)
> OUT(buffer, VIV_FE_END_HEADER_OP_END);
> }
>
> -static inline void CMD_WAIT(struct etnaviv_cmdbuf *buffer)
> +static inline void CMD_WAIT(struct etnaviv_cmdbuf *buffer,
> + unsigned int waitcycles)
> {
> buffer->user_size = ALIGN(buffer->user_size, 8);
>
> - OUT(buffer, VIV_FE_WAIT_HEADER_OP_WAIT | 200);
> + OUT(buffer, VIV_FE_WAIT_HEADER_OP_WAIT | waitcycles);
> }
>
> static inline void CMD_LINK(struct etnaviv_cmdbuf *buffer,
> @@ -168,7 +169,7 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu)
> /* initialize buffer */
> buffer->user_size = 0;
>
> - CMD_WAIT(buffer);
> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> CMD_LINK(buffer, 2,
> etnaviv_cmdbuf_get_va(buffer,
> >mmu_context->cmdbuf_mapping)
> + buffer->user_size - 4);
> @@ -320,7 +321,7 @@ void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu,
> unsigned int event)
> CMD_END(buffer);
>
> /* Append waitlink */
> - CMD_WAIT(buffer);
> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> CMD_LINK(buffer, 2,
> etnaviv_cmdbuf_get_va(buffer,
> >mmu_context->cmdbuf_mapping)
> + buffer->user_size - 4);
> @@ -503,7 +504,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32
> exec_state,
>
> CMD_LOAD_STATE(buffer, VIVS_GL_EVENT, VIVS_GL_EVENT_EVENT_ID(event) |
>VIVS_GL_EVENT_FROM_PE);
> - CMD_WAIT(buffer);
> + CMD_WAIT(buffer, gpu->fe_waitcycles);
> CMD_LINK(buffer, 2,
> etnaviv_cmdbuf_get_va(buffer,
> >mmu_context->cmdbuf_mapping)
> + buffer->user_size - 4);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 41aab1aa330b..8c20dff32240 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -493,6 +493,13 @@ static void etnaviv_gpu_update_clock(struct etnaviv_gpu
> *gpu)
> clock |= VIVS_HI_CLOCK_CONTROL_FSCALE_VAL(fscale);
> etnaviv_gpu_load_clock(gpu, clock);
> }
> +
> + /*
> +* Choose number of wait cycles to target a ~30us (1/32768) max
> latency
> +* until new work is picked up by the FE when it polls in the idle
> loop.
> +*/
> + gpu->fe_waitcycles = min(gpu->base_rate_core >> (15 -
> gpu->freq_scale),
> +0xUL);
> }
>
> static int etnaviv_hw_reset(struct etnaviv_gpu *gpu)
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 98c6f9c320fc..e1e1de59c38d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -150,6 +150,7 @@ struct etnaviv_gpu {
> struct clk *clk_shader;
>
> unsigned int freq_scale;
> + unsigned int fe_waitcycles;
> unsigned long base_rate_core;
> unsigned long base_rate_shader;
> };
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: disable MLCG and pulse eater on GPU reset
>
> Module level clock gating and the pulse eater might interfere with
> the GPU reset, as they both have the potential to stop the clock
> and thus reset propagation to parts of the GPU.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> I'm not aware of any cases where this would have been an issue, but
> it is what the downstream driver does and fundametally seems like
> the right thing to do.
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index de8c9894967c..41aab1aa330b 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -505,8 +505,19 @@ static int etnaviv_hw_reset(struct etnaviv_gpu *gpu)
> timeout = jiffies + msecs_to_jiffies(1000);
>
> while (time_is_after_jiffies(timeout)) {
> - /* enable clock */
> unsigned int fscale = 1 << (6 - gpu->freq_scale);
> + u32 pulse_eater = 0x01590880;
> +
> + /* disable clock gating */
> + gpu_write_power(gpu, VIVS_PM_POWER_CONTROLS, 0x0);
> +
> + /* disable pulse eater */
> + pulse_eater |= BIT(17);
> + gpu_write_power(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
> + pulse_eater |= BIT(0);
> + gpu_write_power(gpu, VIVS_PM_PULSE_EATER, pulse_eater);
> +
> + /* enable clock */
> control = VIVS_HI_CLOCK_CONTROL_FSCALE_VAL(fscale);
> etnaviv_gpu_load_clock(gpu, control);
>
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: fix dumping of active MMU context
Hi Lucas
>
> gpu->mmu_context is the MMU context of the last job in the HW queue, which
> isn't necessarily the same as the context from the bad job. Dump the MMU
> context from the scheduler determined bad submit to make it work as intended.
>
Good catch!
> Fixes: 17e4660ae3d7 ("drm/etnaviv: implement per-process address spaces on
> MMUv2")
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_dump.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> index 44b5f3c35aab..898f84a0fc30 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
> @@ -130,9 +130,9 @@ void etnaviv_core_dump(struct etnaviv_gem_submit *submit)
> return;
> etnaviv_dump_core = false;
>
> - mutex_lock(>mmu_context->lock);
> + mutex_lock(>mmu_context->lock);
>
> - mmu_size = etnaviv_iommu_dump_size(gpu->mmu_context);
> + mmu_size = etnaviv_iommu_dump_size(submit->mmu_context);
>
> /* We always dump registers, mmu, ring, hanging cmdbuf and end marker
> */
> n_obj = 5;
> @@ -162,7 +162,7 @@ void etnaviv_core_dump(struct etnaviv_gem_submit *submit)
> iter.start = __vmalloc(file_size, GFP_KERNEL | __GFP_NOWARN |
> __GFP_NORETRY);
> if (!iter.start) {
> - mutex_unlock(>mmu_context->lock);
> + mutex_unlock(>mmu_context->lock);
> dev_warn(gpu->dev, "failed to allocate devcoredump file\n");
> return;
> }
> @@ -174,18 +174,18 @@ void etnaviv_core_dump(struct etnaviv_gem_submit
> *submit)
> memset(iter.hdr, 0, iter.data - iter.start);
>
> etnaviv_core_dump_registers(, gpu);
> - etnaviv_core_dump_mmu(, gpu->mmu_context, mmu_size);
> + etnaviv_core_dump_mmu(, submit->mmu_context, mmu_size);
> etnaviv_core_dump_mem(, ETDUMP_BUF_RING, gpu->buffer.vaddr,
> gpu->buffer.size,
> etnaviv_cmdbuf_get_va(>buffer,
> - >mmu_context->cmdbuf_mapping));
> +
> >mmu_context->cmdbuf_mapping));
>
> etnaviv_core_dump_mem(, ETDUMP_BUF_CMD,
> submit->cmdbuf.vaddr, submit->cmdbuf.size,
> etnaviv_cmdbuf_get_va(>cmdbuf,
> - >mmu_context->cmdbuf_mapping));
> +
> >mmu_context->cmdbuf_mapping));
>
> - mutex_unlock(>mmu_context->lock);
> + mutex_unlock(>mmu_context->lock);
>
> /* Reserve space for the bomap */
> if (n_bomap_pages) {
> --
> 2.39.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] misc: sram: Add dma-heap-export reserved SRAM area type
Hi Andrew Am Di., 4. Apr. 2023 um 17:02 Uhr schrieb Christian Gmeiner : > > > > Hi Andrew > > > > > >> > > >> > > >> Okay, will split for v2. > > >> > > >> > > > > > > Was there a follow-up v2 of this patchset? AFAICT this series did not > > > make it into the mainline kernel. > > > Do you have any plans to work on it? If not I would like to help out > > > as we have a use case where we want to > > > use a dma-buf sram exporter. > > > > > > > > > > Sure, I've been keeping it alive in our evil vendor tree, but if > > there is interest upstream now I'll post a v2 and CC you. > > That would be great! > Did you find time to prepare a v2? If not, can you point me to the evil vendor tree? -- thanks -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] misc: sram: Add dma-heap-export reserved SRAM area type
> > Hi Andrew > > > >> > >> > >> Okay, will split for v2. > >> > >> > > > > Was there a follow-up v2 of this patchset? AFAICT this series did not > > make it into the mainline kernel. > > Do you have any plans to work on it? If not I would like to help out > > as we have a use case where we want to > > use a dma-buf sram exporter. > > > > > > Sure, I've been keeping it alive in our evil vendor tree, but if > there is interest upstream now I'll post a v2 and CC you. That would be great! -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] misc: sram: Add dma-heap-export reserved SRAM area type
Hi Andrew > > > Okay, will split for v2. > > Was there a follow-up v2 of this patchset? AFAICT this series did not make it into the mainline kernel. Do you have any plans to work on it? If not I would like to help out as we have a use case where we want to use a dma-buf sram exporter. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: fix reference leak when mmaping imported buffer
>
> drm_gem_prime_mmap() takes a reference on the GEM object, but before that
> drm_gem_mmap_obj() already takes a reference, which will be leaked as only
> one reference is dropped when the mapping is closed. Drop the extra
> reference when dma_buf_mmap() succeeds.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index 7031db145a77..3524b5811682 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -91,7 +91,15 @@ static void *etnaviv_gem_prime_vmap_impl(struct
> etnaviv_gem_object *etnaviv_obj)
> static int etnaviv_gem_prime_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
> struct vm_area_struct *vma)
> {
> - return dma_buf_mmap(etnaviv_obj->base.dma_buf, vma, 0);
> + int ret;
> +
> + ret = dma_buf_mmap(etnaviv_obj->base.dma_buf, vma, 0);
> + if (!ret) {
> + /* Drop the reference acquired by drm_gem_mmap_obj(). */
> + drm_gem_object_put(_obj->base);
> + }
> +
> + return ret;
> }
>
> static const struct etnaviv_gem_ops etnaviv_gem_prime_ops = {
> --
> 2.30.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: print MMU exception cause
Hi Lucas Am Mi., 30. Nov. 2022 um 19:53 Uhr schrieb Lucas Stach : > > From: Christian Gmeiner > > The MMU tells us the fault status. While the raw register value is > already printed, it's a bit more user friendly to translate the > fault reasons into human readable format. > > Signed-off-by: Christian Gmeiner > Signed-off-by: Lucas Stach > --- > I've rewritten parts of the patch to properly cover multiple > MMUs and squashed the reason into the existing message. Christian, > please tell me if you are fine with having your name attached to > this patch. > --- Uff.. and old patch I forgot to send an updated version - sorry for that. I am happy to finally see this patch with your improvements landing - thanks! -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/fourcc: add Vivante tile status modifiers
Hi Lucas Am Fr., 9. Sept. 2022 um 11:30 Uhr schrieb Lucas Stach : > > The tile status modifiers can be combined with all of the usual > color buffer modifiers. When they are present an additional plane > is added to the surfaces to share the tile status buffer. The > TS modifiers describe the interpretation of the tag bits in this > buffer. > > Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH 2/2] drm/etnaviv: fix power register offset on GC300
Hi
>
> Older GC300 revisions have their power registers at an offset of 0x200
> rather than 0x100.
>
> Signed-off-by: Doug Brown
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 85eddd492774..b375612df862 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -10,6 +10,8 @@
> #include "etnaviv_gem.h"
> #include "etnaviv_mmu.h"
> #include "etnaviv_drv.h"
> +#include "common.xml.h"
> +#include "state_hi.xml.h"
>
> struct etnaviv_gem_submit;
> struct etnaviv_vram_mapping;
> @@ -149,14 +151,24 @@ struct etnaviv_gpu {
> unsigned long base_rate_shader;
> };
>
> +static inline u32 gpu_fix_reg_address(struct etnaviv_gpu *gpu, u32 reg)
> +{
> + /* Power registers in GC300 < 2.0 are offset by 0x100 */
> + if (reg >= VIVS_PM_POWER_CONTROLS && reg <= VIVS_PM_PULSE_EATER &&
> + gpu->identity.model == chipModel_GC300 &&
> + gpu->identity.revision < 0x2000)
> + reg += 0x100;
> + return reg;
> +}
> +
> static inline void gpu_write(struct etnaviv_gpu *gpu, u32 reg, u32 data)
> {
> - writel(data, gpu->mmio + reg);
> + writel(data, gpu->mmio + gpu_fix_reg_address(gpu, reg));
> }
>
> static inline u32 gpu_read(struct etnaviv_gpu *gpu, u32 reg)
> {
> - return readl(gpu->mmio + reg);
> + return readl(gpu->mmio + gpu_fix_reg_address(gpu, reg));
> }
>
I had a quick look at what vivantes kernel driver did. It uses a per
gpu instance variable powerBaseAddress
that gets set accordingly. I am not sure if I really like the
gpu_fix_reg_address(..) idea, as it gets called on every
register read and write. For me I see two other possible solutions:
1) Add two seperate helpers ala gpu_read_power() and gpu_write_power()
where we do the if beast.
2) Add a power register offset variable to etnaviv_gpu and explicitly
use it on for reads and writes - like the Vivante driver does.
But that's just my personal opinion. Can't wait to hear what Lucas thinks.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 1/2] drm/etnaviv: add missing quirks for GC300
> > The GC300's features register doesn't specify that a 2D pipe is > available, and like the GC600, its idle register reports zero bits where > modules aren't present. > > Signed-off-by: Doug Brown Reviewed-by: Christian Gmeiner > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 37018bc55810..f667e7906d1f 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -416,6 +416,12 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu) > if (gpu->identity.model == chipModel_GC700) > gpu->identity.features &= ~chipFeatures_FAST_CLEAR; > > + /* These models/revisions don't have the 2D pipe bit */ > + if ((gpu->identity.model == chipModel_GC500 && > +gpu->identity.revision <= 2) || > + gpu->identity.model == chipModel_GC300) > + gpu->identity.features |= chipFeatures_PIPE_2D; > + > if ((gpu->identity.model == chipModel_GC500 && > gpu->identity.revision < 2) || > (gpu->identity.model == chipModel_GC300 && > @@ -449,8 +455,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu) > gpu_read(gpu, VIVS_HI_CHIP_MINOR_FEATURE_5); > } > > - /* GC600 idle register reports zero bits where modules aren't present > */ > - if (gpu->identity.model == chipModel_GC600) > + /* GC600/300 idle register reports zero bits where modules aren't > present */ > + if (gpu->identity.model == chipModel_GC600 || > + gpu->identity.model == chipModel_GC300) > gpu->idle_mask = VIVS_HI_IDLE_STATE_TX | > VIVS_HI_IDLE_STATE_RA | > VIVS_HI_IDLE_STATE_SE | > -- > 2.25.1 > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH 2/2] drm/etnaviv: disable tx clock gating for GC7000 rev6203
> The i.MX8MN SoC errata sheet mentions ERR050226: "GPU: Texture L2 Cache > idle signal may incorrectly clock gate the texture engine in GPU". > > The workaround is to disable the corresponding clock gatings. > > While on it move the clock gating check for rev6202 into the same check > to bundle them. > > Signed-off-by: Marco Felsch > Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 37018bc55810..430fcc11d668 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -616,14 +616,15 @@ static void etnaviv_gpu_enable_mlcg(struct etnaviv_gpu > *gpu) > > /* Disable TX clock gating on affected core revisions. */ > if (etnaviv_is_model_rev(gpu, GC4000, 0x5222) || > - etnaviv_is_model_rev(gpu, GC2000, 0x5108)) > + etnaviv_is_model_rev(gpu, GC2000, 0x5108) || > + etnaviv_is_model_rev(gpu, GC2000, 0x6202) || > + etnaviv_is_model_rev(gpu, GC2000, 0x6203)) > pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_TX; > > - /* Disable SE, RA and TX clock gating on affected core revisions. */ > + /* Disable SE and RA clock gating on affected core revisions. */ > if (etnaviv_is_model_rev(gpu, GC7000, 0x6202)) > pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_SE > | > - VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA > | > - VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_TX; > + VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA; > > pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_HZ; > pmc |= VIVS_PM_MODULE_CONTROLS_DISABLE_MODULE_CLOCK_GATING_RA_EZ; > -- > 2.30.2 > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH 1/2] drm/etnaviv: add HWDB entry for GC7000 r6203
Hi all Am Fr., 26. Aug. 2022 um 21:07 Uhr schrieb Lucas Stach : > > From: Marco Felsch > > The GPU is found on the NXP i.MX8MN SoC. The feature bits are taken from > the NXP downstream kernel driver 6.4.3.p2. > Can we stop adding new entries to the kernel hwdb and start to establish a hwdb in the userspace aka mesa? The kernel provides all the needed information to the user space so "all" that's left is to add the mesa side. You might ask why? It is much simpler to maintain such a database in the user space (thanks to stable patch releases of mesa) than forcing users to update their kernels to get this new database entry. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH v2 4/4] drm/etnaviv: export loadavg via perfmon
Hi Lucas > You need to explain a bit more how you intend to use those. > > Contrary to all other perfmon values, where we go to great lengths to > only count the load put onto the GPU by the specific process requesting > the perfmon, the loadavg values also include the load caused by other > submits. Due to this difference in behavior I fear that those new > counters might be confusing to use. But maybe you have a use-case in > mind that I don't see right now. I see these values as total load avg of a sub-GPU component. Sure it is not per process but it is a starting point. I have no problem introducing per process load avg values .. lets see how the next version of this series will look like, -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH v2 2/4] drm/etnaviv: add loadavg accounting
Am Fr., 24. Juni 2022 um 11:38 Uhr schrieb Lucas Stach :
>
> Am Dienstag, dem 21.06.2022 um 09:20 +0200 schrieb Christian Gmeiner:
> > The GPU has an idle state register where each bit represents the idle
> > state of a sub-GPU component like FE or TX. Sample this register
> > every 10ms and calculate a simple moving average over the sub-GPU
> > component idle states with a total observation time frame of 1s.
> >
> > This provides us with a percentage based load of each sub-GPU
> > component.
> >
> > Signed-off-by: Christian Gmeiner
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14 ++
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 64 ++-
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 37
> > 3 files changed, 114 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 1d2b4fb4bcf8..d5c6115e56bd 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -46,6 +46,19 @@ static void load_gpu(struct drm_device *dev)
> > }
> > }
> >
> > +static void unload_gpu(struct drm_device *dev)
> > +{
> > + struct etnaviv_drm_private *priv = dev->dev_private;
> > + unsigned int i;
> > +
> > + for (i = 0; i < ETNA_MAX_PIPES; i++) {
> > + struct etnaviv_gpu *g = priv->gpu[i];
> > +
> > + if (g)
> > + etnaviv_gpu_shutdown(g);
> > + }
> > +}
> > +
> > static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
> > {
> > struct etnaviv_drm_private *priv = dev->dev_private;
> > @@ -557,6 +570,7 @@ static void etnaviv_unbind(struct device *dev)
> > struct drm_device *drm = dev_get_drvdata(dev);
> > struct etnaviv_drm_private *priv = drm->dev_private;
> >
> > + unload_gpu(drm);
> > drm_dev_unregister(drm);
> >
> > component_unbind_all(dev, drm);
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index 37018bc55810..202002ae75ee 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -27,6 +27,8 @@
> > #include "state_hi.xml.h"
> > #include "cmdstream.xml.h"
> >
> > +static const ktime_t loadavg_polling_frequency = 10 * NSEC_PER_MSEC;
> > +
> Feeling like a nitpicker, but the thing defined here isn't a frequency,
> but a time delta/interval.
>
Will rename it in the next version of the patch series.
>
> > static const struct platform_device_id gpu_ids[] = {
> > { .name = "etnaviv-gpu,2d" },
> > { },
> > @@ -745,6 +747,32 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu
> > *gpu)
> > gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
> > }
> >
> > +static enum hrtimer_restart etnaviv_loadavg_function(struct hrtimer *t)
> > +{
> > + struct etnaviv_gpu *gpu = container_of(t, struct etnaviv_gpu,
> > loadavg_timer);
> > + const u32 idle = gpu_read(gpu, VIVS_HI_IDLE_STATE);
> > + int i;
> > +
> > + gpu->loadavg_last_sample_time = ktime_get();
> > +
> > + for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> > + if ((idle & etna_idle_module_names[i].bit))
> > + sma_loadavg_add(>loadavg_value[i], 0);
> > + else
> > + sma_loadavg_add(>loadavg_value[i], 100);
> > +
> > + spin_lock(>loadavg_spinlock);
> > +
> > + for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> > + gpu->loadavg_percentage[i] =
> > sma_loadavg_read(>loadavg_value[i]);
> > +
> > + spin_unlock(>loadavg_spinlock);
>
> After pondering this for a bit, I don't think we need this spinlock.
> The percentage is a single value per engine, so they are already single
> write atomic. The worst thing that can happen without this spinlock is
> that on read of the loadavg some engines already have the value of
> sample period n+1 integrated, while another set is still at n, which I
> don't think we care much about, as those load values are already quite
> coarse.
>
Okay.. will remove the spinlock.
> > +
> > + hrtimer_forward_now(t, loadavg_polling_frequency);
> > +
> > + return HRTIMER_RESTART;
> > +}
> > +
> > int etnaviv_gpu_init(struct etnaviv_gpu *g
Re: [PATCH v2 1/4] drm/etnaviv: add simple moving average (SMA)
Hi Lucas > > Am Dienstag, dem 21.06.2022 um 09:20 +0200 schrieb Christian Gmeiner: > > This adds a SMA algorithm inspired by Exponentially weighted moving > > average (EWMA) algorithm found in the kernel. > > > Still not sure about this one. I _feel_ that a simple moving average > over a period of one second does not do a good job of reflecting the > real GPU load for a bursty workload, where EWMA might be better suited. > But then I also don't have a real informed opinion to offer on this. > I will play with EWMA and see what happens. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
[PATCH v2 4/4] drm/etnaviv: export loadavg via perfmon
Make it possible to access the sub-GPU component load value from
user space with the perfmon infrastructure.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 79 +++
1 file changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index bafdfe49c1d8..d65d9af3a74a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -120,6 +120,19 @@ static u32 hi_total_idle_cycle_read(struct etnaviv_gpu
*gpu,
return gpu_read(gpu, reg);
}
+static u32 load_read(struct etnaviv_gpu *gpu,
+ const struct etnaviv_pm_domain *domain,
+ const struct etnaviv_pm_signal *signal)
+{
+ u32 load;
+
+ spin_lock_bh(>loadavg_spinlock);
+ load = gpu->loadavg_percentage[signal->data];
+ spin_unlock_bh(>loadavg_spinlock);
+
+ return load;
+}
+
static const struct etnaviv_pm_domain doms_3d[] = {
{
.name = "HI",
@@ -419,6 +432,72 @@ static const struct etnaviv_pm_domain doms_3d[] = {
_reg_read
}
}
+ },
+ {
+ .name = "LOAD",
+ .nr_signals = 12,
+ .signal = (const struct etnaviv_pm_signal[]) {
+ {
+ "FE",
+ 0,
+ _read
+ },
+ {
+ "DE",
+ 1,
+ _read
+ },
+ {
+ "PE",
+ 2,
+ _read
+ },
+ {
+ "SH",
+ 3,
+ _read
+ },
+ {
+ "PA",
+ 4,
+ _read
+ },
+ {
+ "SE",
+ 5,
+ _read
+ },
+ {
+ "RA",
+ 6,
+ _read
+ },
+ {
+ "TX",
+ 7,
+ _read
+ },
+ {
+ "VG",
+ 8,
+ _read
+ },
+ {
+ "IM",
+ 9,
+ _read
+ },
+ {
+ "FP",
+ 10,
+ _read
+ },
+ {
+ "TS",
+ 11,
+ _read
+ }
+ }
}
};
--
2.36.1
[PATCH v2 2/4] drm/etnaviv: add loadavg accounting
The GPU has an idle state register where each bit represents the idle
state of a sub-GPU component like FE or TX. Sample this register
every 10ms and calculate a simple moving average over the sub-GPU
component idle states with a total observation time frame of 1s.
This provides us with a percentage based load of each sub-GPU
component.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14 ++
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 64 ++-
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 37
3 files changed, 114 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 1d2b4fb4bcf8..d5c6115e56bd 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -46,6 +46,19 @@ static void load_gpu(struct drm_device *dev)
}
}
+static void unload_gpu(struct drm_device *dev)
+{
+ struct etnaviv_drm_private *priv = dev->dev_private;
+ unsigned int i;
+
+ for (i = 0; i < ETNA_MAX_PIPES; i++) {
+ struct etnaviv_gpu *g = priv->gpu[i];
+
+ if (g)
+ etnaviv_gpu_shutdown(g);
+ }
+}
+
static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
{
struct etnaviv_drm_private *priv = dev->dev_private;
@@ -557,6 +570,7 @@ static void etnaviv_unbind(struct device *dev)
struct drm_device *drm = dev_get_drvdata(dev);
struct etnaviv_drm_private *priv = drm->dev_private;
+ unload_gpu(drm);
drm_dev_unregister(drm);
component_unbind_all(dev, drm);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..202002ae75ee 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -27,6 +27,8 @@
#include "state_hi.xml.h"
#include "cmdstream.xml.h"
+static const ktime_t loadavg_polling_frequency = 10 * NSEC_PER_MSEC;
+
static const struct platform_device_id gpu_ids[] = {
{ .name = "etnaviv-gpu,2d" },
{ },
@@ -745,6 +747,32 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
}
+static enum hrtimer_restart etnaviv_loadavg_function(struct hrtimer *t)
+{
+ struct etnaviv_gpu *gpu = container_of(t, struct etnaviv_gpu,
loadavg_timer);
+ const u32 idle = gpu_read(gpu, VIVS_HI_IDLE_STATE);
+ int i;
+
+ gpu->loadavg_last_sample_time = ktime_get();
+
+ for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
+ if ((idle & etna_idle_module_names[i].bit))
+ sma_loadavg_add(>loadavg_value[i], 0);
+ else
+ sma_loadavg_add(>loadavg_value[i], 100);
+
+ spin_lock(>loadavg_spinlock);
+
+ for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
+ gpu->loadavg_percentage[i] =
sma_loadavg_read(>loadavg_value[i]);
+
+ spin_unlock(>loadavg_spinlock);
+
+ hrtimer_forward_now(t, loadavg_polling_frequency);
+
+ return HRTIMER_RESTART;
+}
+
int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
{
struct etnaviv_drm_private *priv = gpu->drm->dev_private;
@@ -839,6 +867,11 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
for (i = 0; i < ARRAY_SIZE(gpu->event); i++)
complete(>event_free);
+ /* Setup loadavg timer */
+ hrtimer_init(>loadavg_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_ABS_SOFT);
+ gpu->loadavg_timer.function = etnaviv_loadavg_function;
+ hrtimer_start(>loadavg_timer, loadavg_polling_frequency,
HRTIMER_MODE_ABS_SOFT);
+
/* Now program the hardware */
mutex_lock(>lock);
etnaviv_gpu_hw_init(gpu);
@@ -859,6 +892,11 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
return ret;
}
+void etnaviv_gpu_shutdown(struct etnaviv_gpu *gpu)
+{
+ hrtimer_cancel(>loadavg_timer);
+}
+
#ifdef CONFIG_DEBUG_FS
struct dma_debug {
u32 address[2];
@@ -1585,6 +1623,8 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu,
unsigned int timeout_ms)
static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
{
if (gpu->initialized && gpu->fe_running) {
+ hrtimer_cancel(>loadavg_timer);
+
/* Replace the last WAIT with END */
mutex_lock(>lock);
etnaviv_buffer_end(gpu);
@@ -1608,7 +1648,8 @@ static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
#ifdef CONFIG_PM
static int etnaviv_gpu_hw_resume(struct etnaviv_gpu *gpu)
{
- int ret;
+ s64 missing_samples;
+ int ret, i, j;
ret = mutex_lock_killable(>lock);
if (ret)
@@ -1617,7 +1658,27 @@ static int etnaviv_gpu_hw_resume(struct etnaviv_gpu *gpu)
etnavi
[PATCH v2 3/4] drm/etnaviv: show loadavg in debugfs
Might be helpful to see the loadavg in debugfs.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 202002ae75ee..113faff9b02e 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -926,7 +926,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct
seq_file *m)
{
struct dma_debug debug;
u32 dma_lo, dma_hi, axi, idle;
- int ret;
+ int ret, i;
seq_printf(m, "%s Status:\n", dev_name(gpu->dev));
@@ -1044,6 +1044,16 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct
seq_file *m)
if (idle & VIVS_HI_IDLE_STATE_AXI_LP)
seq_puts(m, "\t AXI low power mode\n");
+ seq_printf(m, "\tload:\n");
+ spin_lock_bh(>loadavg_spinlock);
+
+ for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
+ seq_printf(m, "\t %s: %u%%\n",
+ etna_idle_module_names[i].name,
+ gpu->loadavg_percentage[i]);
+
+ spin_unlock_bh(>loadavg_spinlock);
+
if (gpu->identity.features & chipFeatures_DEBUG_MODE) {
u32 read0 = gpu_read(gpu, VIVS_MC_DEBUG_READ0);
u32 read1 = gpu_read(gpu, VIVS_MC_DEBUG_READ1);
--
2.36.1
[PATCH v2 1/4] drm/etnaviv: add simple moving average (SMA)
This adds a SMA algorithm inspired by Exponentially weighted moving
average (EWMA) algorithm found in the kernel.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_sma.h | 53 +++
1 file changed, 53 insertions(+)
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_sma.h
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sma.h
b/drivers/gpu/drm/etnaviv/etnaviv_sma.h
new file mode 100644
index ..81564d5cbdc3
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sma.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 Etnaviv Project
+ */
+
+#ifndef __ETNAVIV_SMA_H__
+#define __ETNAVIV_SMA_H__
+
+#include
+#include
+
+/*
+ * Simple moving average (SMA)
+ *
+ * This implements a fixed-size SMA algorithm.
+ *
+ * The first argument to the macro is the name that will be used
+ * for the struct and helper functions.
+ *
+ * The second argument, the samples, expresses how many samples are
+ * used for the SMA algorithm.
+ */
+
+#define DECLARE_SMA(name, _samples) \
+struct sma_##name { \
+unsigned long pos; \
+unsigned long sum; \
+unsigned long samples[_samples]; \
+}; \
+static inline void sma_##name##_init(struct sma_##name *s) \
+{ \
+BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
+memset(s, 0, sizeof(struct sma_##name)); \
+} \
+static inline unsigned long sma_##name##_read(struct sma_##name *s) \
+{ \
+BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
+return s->sum / _samples; \
+} \
+static inline void sma_##name##_add(struct sma_##name *s, unsigned long
val) \
+{ \
+unsigned long pos = READ_ONCE(s->pos); \
+unsigned long sum = READ_ONCE(s->sum); \
+unsigned long sample = READ_ONCE(s->samples[pos]); \
+ \
+BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
+ \
+ WRITE_ONCE(s->sum, sum - sample + val); \
+ WRITE_ONCE(s->samples[pos], val); \
+ WRITE_ONCE(s->pos, pos + 1 == _samples ? 0 : pos + 1); \
+}
+
+#endif /* __ETNAVIV_SMA_H__ */
--
2.36.1
[PATCH v2 0/4] Add support for GPU load values
This patch series add support for loadavg values for GPU sub-components. I am adding a SMA algorithm as I was not really sure if EWMA would be a good fit for this use case. Changes v2: - Addressed feedback from Lucas Christian Gmeiner (4): drm/etnaviv: add simple moving average (SMA) drm/etnaviv: add loadavg accounting drm/etnaviv: show loadavg in debugfs drm/etnaviv: export loadavg via perfmon drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 76 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 37 +++ drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 79 +++ drivers/gpu/drm/etnaviv/etnaviv_sma.h | 53 +++ 5 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_sma.h -- 2.36.1
[PATCH] drm/etnaviv: print offender task information on hangcheck recovery
Track the pid per submit, so we can print the name and cmdline of
the task which submitted the batch that caused the gpu to hang.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 +
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 6 ++
drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 18 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 2 +-
drivers/gpu/drm/etnaviv/etnaviv_sched.c | 2 +-
5 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index 63688e6e4580..baa81cbf701a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -96,6 +96,7 @@ struct etnaviv_gem_submit {
int out_fence_id;
struct list_head node; /* GPU active submit list */
struct etnaviv_cmdbuf cmdbuf;
+ struct pid *pid; /* submitting process */
bool runtime_resumed;
u32 exec_state;
u32 flags;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 1ac916b24891..1491159d0d20 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -399,6 +399,9 @@ static void submit_cleanup(struct kref *kref)
mutex_unlock(>gpu->fence_lock);
dma_fence_put(submit->out_fence);
}
+
+ put_pid(submit->pid);
+
kfree(submit->pmrs);
kfree(submit);
}
@@ -422,6 +425,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
*data,
struct sync_file *sync_file = NULL;
struct ww_acquire_ctx ticket;
int out_fence_fd = -1;
+ struct pid *pid = get_pid(task_pid(current));
void *stream;
int ret;
@@ -519,6 +523,8 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void
*data,
goto err_submit_ww_acquire;
}
+ submit->pid = pid;
+
ret = etnaviv_cmdbuf_init(priv->cmdbuf_suballoc, >cmdbuf,
ALIGN(args->stream_size, 8) + 8);
if (ret)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 37018bc55810..7d9bf4673e2d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1045,12 +1045,28 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct
seq_file *m)
}
#endif
-void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu)
+void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
{
+ struct etnaviv_gpu *gpu = submit->gpu;
+ char *comm = NULL, *cmd = NULL;
+ struct task_struct *task;
unsigned int i;
dev_err(gpu->dev, "recover hung GPU!\n");
+ task = get_pid_task(submit->pid, PIDTYPE_PID);
+ if (task) {
+ comm = kstrdup(task->comm, GFP_KERNEL);
+ cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
+ put_task_struct(task);
+ }
+
+ if (comm && cmd)
+ dev_err(gpu->dev, "offending task: %s (%s)\n", comm, cmd);
+
+ kfree(cmd);
+ kfree(comm);
+
if (pm_runtime_get_sync(gpu->dev) < 0)
goto pm_put;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 85eddd492774..b3a0941d56fd 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -168,7 +168,7 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu
*gpu);
int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m);
#endif
-void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu);
+void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit);
void etnaviv_gpu_retire(struct etnaviv_gpu *gpu);
int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
u32 fence, struct drm_etnaviv_timespec *timeout);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 72e2553fbc98..d29f467eee13 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -67,7 +67,7 @@ static enum drm_gpu_sched_stat
etnaviv_sched_timedout_job(struct drm_sched_job
/* get the GPU back into the init state */
etnaviv_core_dump(submit);
- etnaviv_gpu_recover_hang(gpu);
+ etnaviv_gpu_recover_hang(submit);
drm_sched_resubmit_jobs(>sched);
--
2.36.1
Re: [PATCH] drm/etnaviv: relax submit size limits
Am Do., 6. Jan. 2022 um 19:10 Uhr schrieb Lucas Stach :
>
> While all userspace tried to limit commandstreams to 64K in size,
> a bug in the Mesa driver lead to command streams of up to 128K
> being submitted. Allow those to avoid breaking existing userspace.
>
> Fixes: 6dfa2fab8ddd ("drm/etnaviv: limit submit sizes")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: consider completed fence seqno in hang check
Am Mi., 22. Dez. 2021 um 01:17 Uhr schrieb Lucas Stach : > > Some GPU heavy test programs manage to trigger the hangcheck quite often. > If there are no other GPU users in the system and the test program > exhibits a very regular structure in the commandstreams that are being > submitted, we can end up with two distinct submits managing to trigger > the hangcheck with the FE in a very similar address range. This leads > the hangcheck to believe that the GPU is stuck, while in reality the GPU > is already busy working on a different job. To avoid those spurious > GPU resets, also remember and consider the last completed fence seqno > in the hang check. > > Reported-by: Joerg Albert > Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: limit submit sizes
Am Fr., 17. Dez. 2021 um 11:59 Uhr schrieb Lucas Stach : > > Currently we allow rediculous ammounts of kernel memory being allocated > via the etnaviv GEM_SUBMIT ioctl, which is a pretty easy DoS vector. Put > some reasonable limits in to fix this. > > The commandstream size is limited to 64KB, which was already a soft limit > on older kernels after which the kernel only took submits on a best effort > base, so there is no userspace that tries to submit commandstreams larger > than this. Even if the whole commandstream is a single incrementing address > load, the size limit also limits the number of potential relocs and > referenced buffers to slightly under 64K, so use the same limit for those > arguments. The performance monitoring infrastructure currently supports > less than 50 performance counter signals, so limiting them to 128 on a > single submit seems like a reasonably future-proof number for now. This > number can be bumped if needed without breaking the interface. > > Cc: sta...@vger.kernel.org > Reported-by: Dan Carpenter > Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: constify static struct cooling_ops
Am So., 28. Nov. 2021 um 21:20 Uhr schrieb Rikard Falkeborn
:
>
> The only usage of cooling_ops is to pass its address to
> thermal_of_cooling_device_register(), which takes a pointer to const
> struct thermal_cooling_device_ops as input. Make it const to allow the
> compiler to put it in read-only memory.
>
> Signed-off-by: Rikard Falkeborn
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 06bde46df451..37018bc55810 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1658,7 +1658,7 @@ etnaviv_gpu_cooling_set_cur_state(struct
> thermal_cooling_device *cdev,
> return 0;
> }
>
> -static struct thermal_cooling_device_ops cooling_ops = {
> +static const struct thermal_cooling_device_ops cooling_ops = {
> .get_max_state = etnaviv_gpu_cooling_get_max_state,
> .get_cur_state = etnaviv_gpu_cooling_get_cur_state,
> .set_cur_state = etnaviv_gpu_cooling_set_cur_state,
> --
> 2.34.1
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm: Remove redundant 'flush_workqueue()' calls
Am So., 10. Okt. 2021 um 16:08 Uhr schrieb Christophe JAILLET : > > 'destroy_workqueue()' already drains the queue before destroying it, so > there is no need to flush it explicitly. > > Remove the redundant 'flush_workqueue()' calls. > > This was generated with coccinelle: > > @@ > expression E; > @@ > - flush_workqueue(E); > destroy_workqueue(E); > > Signed-off-by: Christophe JAILLET For drm/etnaviv: Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH 1/3] drm/etnaviv: use PLATFORM_DEVID_NONE
Am Do., 26. Aug. 2021 um 14:10 Uhr schrieb Michael Walle :
>
> There is already a macro for the magic value. Use it.
>
> Signed-off-by: Michael Walle
Reviewed-by: Christian Gmeiner
I will wait for v2 for the rest of the changes to review.
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 7dcc6392792d..2509b3e85709 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -653,7 +653,7 @@ static int __init etnaviv_init(void)
> if (!of_device_is_available(np))
> continue;
>
> - pdev = platform_device_alloc("etnaviv", -1);
> + pdev = platform_device_alloc("etnaviv", PLATFORM_DEVID_NONE);
> if (!pdev) {
> ret = -ENOMEM;
> of_node_put(np);
> --
> 2.30.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 8/8] drm/etnaviv: add missing MMU context put when reaping MMU mapping
Am Fr., 20. Aug. 2021 um 22:18 Uhr schrieb Lucas Stach :
>
> When we forcefully evict a mapping from the the address space and thus the
> MMU context, the MMU context is leaked, as the mapping no longer points to
> it, so it doesn't get freed when the GEM object is destroyed. Add the
> mssing context put to fix the leak.
>
> Cc: sta...@vger.kernel.org # 5.4
> Signed-off-by: Lucas Stach
> Tested-by: Michael Walle
Series is:
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index dab1b58006d8..9fb1a2aadbcb 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -199,6 +199,7 @@ static int etnaviv_iommu_find_iova(struct
> etnaviv_iommu_context *context,
> */
> list_for_each_entry_safe(m, n, , scan_node) {
> etnaviv_iommu_remove_mapping(context, m);
> + etnaviv_iommu_context_put(m->context);
> m->context = NULL;
> list_del_init(>mmu_node);
> list_del_init(>scan_node);
> --
> 2.30.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Re: [PATCH 7/8] drm/etnaviv: reference MMU context when setting up hardware state
Hi Lucas
Am Di., 24. Aug. 2021 um 09:54 Uhr schrieb Lucas Stach :
>
> Am Dienstag, dem 24.08.2021 um 09:24 +0200 schrieb Christian Gmeiner:
> > Am Fr., 20. Aug. 2021 um 22:18 Uhr schrieb Lucas Stach
> > :
> > >
> > > Move the refcount manipulation of the MMU context to the point where the
> > > hardware state is programmed. At that point it is also known if a previous
> > > MMU state is still there, or the state needs to be reprogrammed with a
> > > potentially different context.
> > >
> > > Cc: sta...@vger.kernel.org # 5.4
> > > Signed-off-by: Lucas Stach
> > > Tested-by: Michael Walle
> > > ---
> > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 24 +++---
> > > drivers/gpu/drm/etnaviv/etnaviv_iommu.c| 4
> > > drivers/gpu/drm/etnaviv/etnaviv_iommu_v2.c | 8
> > > 3 files changed, 24 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > index f420c4f14657..1fa98ce870f7 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > @@ -641,17 +641,19 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu,
> > > u32 address, u16 prefetch)
> > > gpu->fe_running = true;
> > > }
> > >
> > > -static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu)
> > > +static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
> > > + struct etnaviv_iommu_context
> > > *context)
> > > {
> > > - u32 address = etnaviv_cmdbuf_get_va(>buffer,
> > > - >mmu_context->cmdbuf_mapping);
> > > u16 prefetch;
> > > + u32 address;
> > >
> > > /* setup the MMU */
> > > - etnaviv_iommu_restore(gpu, gpu->mmu_context);
> > > + etnaviv_iommu_restore(gpu, context);
> > >
> > > /* Start command processor */
> > > prefetch = etnaviv_buffer_init(gpu);
> > > + address = etnaviv_cmdbuf_get_va(>buffer,
> > > +
> > > >mmu_context->cmdbuf_mapping);
> > >
> > > etnaviv_gpu_start_fe(gpu, address, prefetch);
> > > }
> > > @@ -1369,14 +1371,12 @@ struct dma_fence *etnaviv_gpu_submit(struct
> > > etnaviv_gem_submit *submit)
> > > goto out_unlock;
> > > }
> > >
> > > - if (!gpu->fe_running) {
> > > - gpu->mmu_context =
> > > etnaviv_iommu_context_get(submit->mmu_context);
> > > - etnaviv_gpu_start_fe_idleloop(gpu);
> > > - } else {
> > > - if (submit->prev_mmu_context)
> > > -
> > > etnaviv_iommu_context_put(submit->prev_mmu_context);
> > > - submit->prev_mmu_context =
> > > etnaviv_iommu_context_get(gpu->mmu_context);
> > > - }
> > > + if (!gpu->fe_running)
> > > + etnaviv_gpu_start_fe_idleloop(gpu, submit->mmu_context);
> > > +
> > > + if (submit->prev_mmu_context)
> > > + etnaviv_iommu_context_put(submit->prev_mmu_context);
> > > + submit->prev_mmu_context =
> > > etnaviv_iommu_context_get(gpu->mmu_context);
> > >
> > > if (submit->nr_pmrs) {
> > > gpu->event[event[1]].sync_point =
> > > _point_perfmon_sample_pre;
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> > > index 1a7c89a67bea..afe5dd6a9925 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_iommu.c
> > > @@ -92,6 +92,10 @@ static void etnaviv_iommuv1_restore(struct etnaviv_gpu
> > > *gpu,
> > > struct etnaviv_iommuv1_context *v1_context =
> > > to_v1_context(context);
> > > u32 pgtable;
> > >
> > > + if (gpu->mmu_context)
> > > + etnaviv_iommu_context_put(gpu->mmu_context);
> > > + gpu->mmu_context = etnaviv_iommu_context_get(context);
> > > +
> > > /* set base addresses */
> > > gpu_write(gpu, VIVS_MC_MEMORY_BASE_ADDR_RA,
> >
Re: [PATCH 7/8] drm/etnaviv: reference MMU context when setting up hardware state
if (gpu_read(gpu, VIVS_MMUv2_SEC_CONTROL) & > VIVS_MMUv2_SEC_CONTROL_ENABLE) > return; > > + if (gpu->mmu_context) > + etnaviv_iommu_context_put(gpu->mmu_context); > + gpu->mmu_context = etnaviv_iommu_context_get(context); > + I have seen this pattern now more than two times - maybe put the assignment of a new mmu context into its own function? > gpu_write(gpu, VIVS_MMUv2_PTA_ADDRESS_LOW, > lower_32_bits(context->global->v2.pta_dma)); > gpu_write(gpu, VIVS_MMUv2_PTA_ADDRESS_HIGH, > -- > 2.30.2 > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: remove NULL check which is not needed
Am Sa., 5. Juni 2021 um 14:18 Uhr schrieb Bernard Zhao : > > NULL check before kvfree functions is not needed. > > Signed-off-by: Bernard Zhao > --- > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c > b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c > index b390dd4d60b7..d741b1d735f7 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c > @@ -80,8 +80,7 @@ static void etnaviv_gem_prime_release(struct > etnaviv_gem_object *etnaviv_obj) > /* Don't drop the pages for imported dmabuf, as they are not > * ours, just free the array we allocated: > */ > - if (etnaviv_obj->pages) > - kvfree(etnaviv_obj->pages); > + kvfree(etnaviv_obj->pages); > > drm_prime_gem_destroy(_obj->base, etnaviv_obj->sgt); > } > -- > 2.31.0 > Thanks for the patch, but there is an other one queued up in etnaviv/next that fixes the same issue: https://git.pengutronix.de/cgit/lst/linux/commit/?h=etnaviv/next=7d614ab2f20503ed8766363d41f8607337571adf -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: remove no need NULL check
Am Sa., 5. Juni 2021 um 14:24 Uhr schrieb Bernard Zhao : > > NULL check before kvfree functions is not needed. > > Signed-off-by: Bernard Zhao > --- > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 12 > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > index d05c35994579..bd0d66ebf314 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > @@ -612,14 +612,10 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, > void *data, > err_submit_cmds: > if (ret && (out_fence_fd >= 0)) > put_unused_fd(out_fence_fd); > - if (stream) > - kvfree(stream); > - if (bos) > - kvfree(bos); > - if (relocs) > - kvfree(relocs); > - if (pmrs) > - kvfree(pmrs); > + kvfree(stream); > + kvfree(bos); > + kvfree(relocs); > + kvfree(pmrs); > > return ret; > } > -- > 2.31.0 > Thanks for the patch, but there is an other one queued up in etnaviv/next that fixes the same issue: https://git.pengutronix.de/cgit/lst/linux/commit/?h=etnaviv/next=bdf622e0fade2cec72c948c708763378b656c01d -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH -next] drm/etnaviv: Remove unneeded if-null-free check
Am Di., 1. Juni 2021 um 16:06 Uhr schrieb Zheng Yongjun : > > Eliminate the following coccicheck warning: > > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c:84:2-8: WARNING: > NULL check before some freeing functions is not needed. > > Signed-off-by: Zheng Yongjun Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
Re: [PATCH] drm/etnaviv: rework linear window offset calculation
Am Mo., 3. Mai 2021 um 12:24 Uhr schrieb Lucas Stach :
>
> The current calculation based on the required_dma mask can be significantly
> off, so that the linear window only overlaps a small part of the DRAM
> address space. This can lead to the command buffer being unmappable, which
> is obviously bad.
>
> Rework the linear window offset calculation to be based on the command buffer
> physical address, making sure that the command buffer is always mappable.
>
> Signed-off-by: Lucas Stach
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 52 +--
> 1 file changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index c6404b8d067f..a454b13e8106 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -27,10 +27,6 @@
> #include "state_hi.xml.h"
> #include "cmdstream.xml.h"
>
> -#ifndef PHYS_OFFSET
> -#define PHYS_OFFSET 0
> -#endif
> -
> static const struct platform_device_id gpu_ids[] = {
> { .name = "etnaviv-gpu,2d" },
> { },
> @@ -724,6 +720,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;
> + dma_addr_t cmdbuf_paddr;
> int ret, i;
>
> ret = pm_runtime_get_sync(gpu->dev);
> @@ -766,28 +763,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> if (ret)
> goto fail;
>
> - /*
> -* Set the GPU linear window to be at the end of the DMA window, where
> -* the CMA area is likely to reside. This ensures that we are able to
> -* map the command buffers while having the linear window overlap as
> -* much RAM as possible, so we can optimize mappings for other
> buffers.
> -*
> -* For 3D cores only do this if MC2.0 is present, as with MC1.0 it
> leads
> -* to different views of the memory on the individual engines.
> -*/
> - if (!(gpu->identity.features & chipFeatures_PIPE_3D) ||
> - (gpu->identity.minor_features0 & chipMinorFeatures0_MC20)) {
> - u32 dma_mask = (u32)dma_get_required_mask(gpu->dev);
> - if (dma_mask < PHYS_OFFSET + SZ_2G)
> - priv->mmu_global->memory_base = PHYS_OFFSET;
> - else
> - priv->mmu_global->memory_base = dma_mask - SZ_2G + 1;
> - } else if (PHYS_OFFSET >= SZ_2G) {
> - dev_info(gpu->dev, "Need to move linear window on MC1.0,
> disabling TS\n");
> - priv->mmu_global->memory_base = PHYS_OFFSET;
> - gpu->identity.features &= ~chipFeatures_FAST_CLEAR;
> - }
> -
> /*
> * If the GPU is part of a system with DMA addressing limitations,
> * request pages for our SHM backend buffers from the DMA32 zone to
> @@ -804,6 +779,31 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> goto fail;
> }
>
> + /*
> +* Set the GPU linear window to cover the cmdbuf region, as the GPU
> +* won't be able to start execution otherwise. The alignment to 128M
> is
> +* chosen arbitrarily but helps in debugging, as the MMU offset
> +* calculations are much more straight forward this way.
> +*
> +* On MC1.0 cores the linear window offset is ignored by the TS
> engine,
> +* leading to inconsistent memory views. Avoid using the offset on
> those
> +* cores if possible, otherwise disable the TS feature.
> +*/
> + cmdbuf_paddr = ALIGN_DOWN(etnaviv_cmdbuf_get_pa(>buffer),
> SZ_128M);
> +
> + if (!(gpu->identity.features & chipFeatures_PIPE_3D) ||
> + (gpu->identity.minor_features0 & chipMinorFeatures0_MC20)) {
> + if (cmdbuf_paddr >= SZ_2G)
> + priv->mmu_global->memory_base = SZ_2G;
> + else
> + priv->mmu_global->memory_base = cmdbuf_paddr;
> + } else if (cmdbuf_paddr + SZ_128M >= SZ_2G) {
> + dev_info(gpu->dev,
> +"Need to move linear window on MC1.0, disabling
> TS\n");
> + gpu->identity.features &= ~chipFeatures_FAST_CLEAR;
> + priv->mmu_global->memory_base = SZ_2G;
> + }
> +
> /* Setup event management */
> spin_lock_init(>event_spinlock);
> init_completion(>event_free);
> --
> 2.29.2
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: Remove useless error message
Am Do., 25. März 2021 um 13:33 Uhr schrieb Tian Tao : > > Fix the following coccicheck report: > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c:1775:2-9: > line 1775 is redundant because platform_get_irq() already prints an error > > Remove dev_err() messages after platform_get_irq() failures. > > Signed-off-by: Tian Tao > Signed-off-by: Zihao Tang > Signed-off-by: Jay Fang Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/etnaviv: remove unneeded if-null-free check
Am Fr., 9. Apr. 2021 um 14:23 Uhr schrieb Qiheng Lin : > > Eliminate the following coccicheck warning: > > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:616:2-8: WARNING: > NULL check before some freeing functions is not needed. > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:618:2-8: WARNING: > NULL check before some freeing functions is not needed. > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:620:2-8: WARNING: > NULL check before some freeing functions is not needed. > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:622:2-8: WARNING: > NULL check before some freeing functions is not needed. > > Signed-off-by: Qiheng Lin Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: Remove redundant NULL check
Am Di., 23. März 2021 um 03:46 Uhr schrieb Jiapeng Chong : > > Fix the following coccicheck warnings: > > ./drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:622:2-8: WARNING: NULL > check before some freeing functions is not needed. > > ./drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:618:2-8: WARNING: NULL > check before some freeing functions is not needed. > > ./drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:616:2-8: WARNING: NULL > check before some freeing functions is not needed. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fourcc: add Vivante TS modifiers
Am Sa., 20. März 2021 um 20:11 Uhr schrieb Daniel Vetter : > > On Sat, Mar 20, 2021 at 10:28 AM Christian Gmeiner > wrote: > > > > Hi Lucas > > > > Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach > > : > > > > > > Vivante TS (tile-status) buffer modifiers. They can be combined with all > > > of > > > the Vivante color buffer tiling modifiers, so they are kind of a modifier > > > modifier. If a TS modifier is present we have a additional plane for the > > > TS buffer and the modifier defines the layout of this TS buffer. > > > > > > > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can > > you share some insight on this? > > It's the official registry for drm_fourcc codes and drm modifiers. > Whether the kernel ever uses it or not doesn't matter. Fair point.. but I do not see any usage of these TS modifiers in mesa - that's why I am asking. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fourcc: add Vivante TS modifiers
Hi Lucas Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach : > > Vivante TS (tile-status) buffer modifiers. They can be combined with all of > the Vivante color buffer tiling modifiers, so they are kind of a modifier > modifier. If a TS modifier is present we have a additional plane for the > TS buffer and the modifier defines the layout of this TS buffer. > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can you share some insight on this? -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/etnaviv: add HWDB entry for GC7000 rev 6204
Am Fr., 19. März 2021 um 20:02 Uhr schrieb Lucas Stach : > > From: Sascha Hauer > > This is the 3D GPU found on the i.MX8MP SoC. The feature bits are > taken from the NXP downstream kernel driver 6.4.3.p1.305572. > > Signed-off-by: Sascha Hauer > Signed-off-by: Lucas Stach Reviewed-by: Christian Gmeiner -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: add HWDB entry for GC7000 rev 6204
Am Mi., 24. Feb. 2021 um 16:55 Uhr schrieb Lucas Stach :
>
> From: Sascha Hauer
>
> This is the 3D GPU found on the i.MX8MP SoC.
>
Can you provide some information about where you have taken these
values (galcore version,
database entry name)?
> Signed-off-by: Sascha Hauer
> Signed-off-by: Lucas Stach
> ---
> drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index 167971a09be7..dfc0f536b3b9 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -37,6 +37,37 @@ static const struct etnaviv_chip_identity
> etnaviv_chip_identities[] = {
> .minor_features10 = 0x0,
> .minor_features11 = 0x0,
> },
> + {
> + .model = 0x7000,
> + .revision = 0x6204,
> + .product_id = ~0U,
> + .customer_id = ~0U,
> + .eco_id = 0,
> + .stream_count = 16,
> + .register_max = 64,
> + .thread_count = 512,
> + .shader_core_count = 2,
> + .vertex_cache_size = 16,
> + .vertex_output_buffer_size = 1024,
> + .pixel_pipes = 1,
> + .instruction_count = 512,
> + .num_constants = 320,
> + .buffer_size = 0,
> + .varyings_count = 16,
> + .features = 0xe0287c8d,
> + .minor_features0 = 0xc1589eff,
> + .minor_features1 = 0xfefbfad9,
> + .minor_features2 = 0xeb9d4fbf,
> + .minor_features3 = 0xedfffced,
> + .minor_features4 = 0xdb0dafc7,
> + .minor_features5 = 0x3b5ac333,
> + .minor_features6 = 0xfcce6000,
> + .minor_features7 = 0xfffbfa6f,
> + .minor_features8 = 0x00e10ef3,
> + .minor_features9 = 0x04c8003c,
> + .minor_features10 = 0x4060,
> + .minor_features11 = 0x0024,
> + },
> {
> .model = 0x7000,
> .revision = 0x6214,
> --
> 2.29.2
>
I really would love to see a hwdb in mesa instead of adding more
entries to the kernel one.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/etnaviv: provide more ID values via GET_PARAM ioctl.
Make it possible for the user space to access these ID values.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12
include/uapi/drm/etnaviv_drm.h| 3 +++
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index c6404b8d067f..ec16991ba8b6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -156,6 +156,18 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32
param, u64 *value)
*value = ~0ULL;
break;
+ case ETNAVIV_PARAM_GPU_PRODUCT_ID:
+ *value = gpu->identity.product_id;
+ break;
+
+ case ETNAVIV_PARAM_GPU_CUSTOMER_ID:
+ *value = gpu->identity.customer_id;
+ break;
+
+ case ETNAVIV_PARAM_GPU_ECO_ID:
+ *value = gpu->identity.eco_id;
+ 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 09d0df8b71c5..af024d90453d 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -74,6 +74,9 @@ struct drm_etnaviv_timespec {
#define ETNAVIV_PARAM_GPU_NUM_CONSTANTS 0x19
#define ETNAVIV_PARAM_GPU_NUM_VARYINGS 0x1a
#define ETNAVIV_PARAM_SOFTPIN_START_ADDR0x1b
+#define ETNAVIV_PARAM_GPU_PRODUCT_ID0x1c
+#define ETNAVIV_PARAM_GPU_CUSTOMER_ID 0x1d
+#define ETNAVIV_PARAM_GPU_ECO_ID0x1e
#define ETNA_MAX_PIPES 4
--
2.29.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 3/3] tests/etnaviv_2d_test: check whether the rendering is correct
Am Di., 1. Dez. 2020 um 21:38 Uhr schrieb Lubomir Rintel :
>
> Instead of always dumping the rendered picture, check whether it matches
> the expectations. This makes more sense for automated testing.
>
> Retain the ability to dump the picture instead of checking it when a
> file name is given as an argument. This also removes use of a hardcoded
> file name in a world writable directory, which is an unsafe thing to
> do anyway.
>
> Signed-off-by: Lubomir Rintel
Reviewed-by: Christian Gmeiner
> ---
> tests/etnaviv/etnaviv_2d_test.c | 29 +++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/tests/etnaviv/etnaviv_2d_test.c b/tests/etnaviv/etnaviv_2d_test.c
> index a75a4a89..9fcdae18 100644
> --- a/tests/etnaviv/etnaviv_2d_test.c
> +++ b/tests/etnaviv/etnaviv_2d_test.c
> @@ -147,6 +147,27 @@ static void gen_cmd_stream(struct etna_cmd_stream
> *stream, struct etna_bo *bmp,
> etna_set_state(stream, VIVS_GL_FLUSH_CACHE, VIVS_GL_FLUSH_CACHE_PE2D);
> }
>
> +int etna_check_image(uint32_t *p, int width, int height)
> +{
> + int i;
> + uint32_t expected;
> +
> + for (i = 0; i < width * height; i++) {
> + if (i%8 < 4 && i%(width*8) < width*4 && i%width < 8*16 && i <
> width*8*16)
> + expected = 0xff40ff40;
> + else
> + expected = 0x;
> +
> + if (p[i] != expected) {
> + fprintf(stderr, "Offset %d: expected: 0x%08x, got:
> 0x%08x\n",
> + i, expected, p[i]);
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> int main(int argc, char *argv[])
> {
> const int width = 256;
> @@ -165,7 +186,7 @@ int main(int argc, char *argv[])
> int core = 0;
>
> if (argc < 2) {
> - fprintf(stderr, "Usage: %s /dev/dri/\n", argv[0]);
> + fprintf(stderr, "Usage: %s /dev/dri/ []\n",
> argv[0]);
> return 1;
> }
>
> @@ -242,7 +263,11 @@ int main(int argc, char *argv[])
>
> etna_cmd_stream_finish(stream);
>
> - bmp_dump32(etna_bo_map(bmp), width, height, false, "/tmp/etna.bmp");
> + if (argc > 2)
> + bmp_dump32(etna_bo_map(bmp), width, height, false, argv[2]);
> +
> + if (etna_check_image(etna_bo_map(bmp), width, height))
> + ret = 7;
>
> etna_cmd_stream_del(stream);
>
> --
> 2.28.0
>
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 2/3] tests/etnaviv_2d_test: pick the 2D core
Am Di., 1. Dez. 2020 um 21:38 Uhr schrieb Lubomir Rintel :
>
> Run the test on a core capable of 2D rendering instead of hardcoding to
> core zero.
>
Thanks - I should have done this before landing this test :)
> Signed-off-by: Lubomir Rintel
Reviewed-by: Christian Gmeiner
> ---
> tests/etnaviv/etnaviv_2d_test.c | 31 ---
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/tests/etnaviv/etnaviv_2d_test.c b/tests/etnaviv/etnaviv_2d_test.c
> index eb9dfa59..a75a4a89 100644
> --- a/tests/etnaviv/etnaviv_2d_test.c
> +++ b/tests/etnaviv/etnaviv_2d_test.c
> @@ -161,6 +161,8 @@ int main(int argc, char *argv[])
>
> drmVersionPtr version;
> int fd, ret = 0;
> + uint64_t feat;
> + int core = 0;
>
> if (argc < 2) {
> fprintf(stderr, "Usage: %s /dev/dri/\n", argv[0]);
> @@ -190,13 +192,28 @@ int main(int argc, char *argv[])
> goto out;
> }
>
> - /* TODO: we assume that core 0 is a 2D capable one */
> - gpu = etna_gpu_new(dev, 0);
> - if (!gpu) {
> - perror("etna_gpu_new");
> - ret = 3;
> - goto out_device;
> - }
> + do {
> + gpu = etna_gpu_new(dev, core);
> + if (!gpu) {
> + perror("etna_gpu_new");
> + ret = 3;
> + goto out_device;
> + }
> +
> + if (etna_gpu_get_param(gpu, ETNA_GPU_FEATURES_0, )) {
> + perror("etna_gpu_get_param");
> + ret = 4;
> + goto out_device;
> + }
> +
> + if ((feat & (1 << 9)) == 0) {
> + /* GPU not 2D capable. */
> + etna_gpu_del(gpu);
> + gpu = NULL;
> + }
> +
> + core++;
> + } while (!gpu);
>
> pipe = etna_pipe_new(gpu, ETNA_PIPE_2D);
> if (!pipe) {
> --
> 2.28.0
>
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 1/3] tests/etnaviv_2d_test: explain the errors
Am Di., 1. Dez. 2020 um 21:38 Uhr schrieb Lubomir Rintel :
>
> Just so that it's obvious what failed and why.
>
> Signed-off-by: Lubomir Rintel
Reviewed-by: Christian Gmeiner
> ---
> tests/etnaviv/etnaviv_2d_test.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/tests/etnaviv/etnaviv_2d_test.c b/tests/etnaviv/etnaviv_2d_test.c
> index 8dd77b66..eb9dfa59 100644
> --- a/tests/etnaviv/etnaviv_2d_test.c
> +++ b/tests/etnaviv/etnaviv_2d_test.c
> @@ -162,9 +162,16 @@ int main(int argc, char *argv[])
> drmVersionPtr version;
> int fd, ret = 0;
>
> - fd = open(argv[1], O_RDWR);
> - if (fd < 0)
> + if (argc < 2) {
> + fprintf(stderr, "Usage: %s /dev/dri/\n", argv[0]);
> return 1;
> + }
> +
> + fd = open(argv[1], O_RDWR);
> + if (fd < 0) {
> + perror(argv[1]);
> + return 1;
> + }
>
> version = drmGetVersion(fd);
> if (version) {
> @@ -178,6 +185,7 @@ int main(int argc, char *argv[])
>
> dev = etna_device_new(fd);
> if (!dev) {
> + perror("etna_device_new");
> ret = 2;
> goto out;
> }
> @@ -185,18 +193,21 @@ int main(int argc, char *argv[])
> /* TODO: we assume that core 0 is a 2D capable one */
> gpu = etna_gpu_new(dev, 0);
> if (!gpu) {
> + perror("etna_gpu_new");
> ret = 3;
> goto out_device;
> }
>
> pipe = etna_pipe_new(gpu, ETNA_PIPE_2D);
> if (!pipe) {
> + perror("etna_pipe_new");
> ret = 4;
> goto out_gpu;
> }
>
> bmp = etna_bo_new(dev, bmp_size, ETNA_BO_UNCACHED);
> if (!bmp) {
> + perror("etna_bo_new");
> ret = 5;
> goto out_pipe;
> }
> @@ -204,6 +215,7 @@ int main(int argc, char *argv[])
>
> stream = etna_cmd_stream_new(pipe, 0x300, NULL, NULL);
> if (!stream) {
> + perror("etna_cmd_stream_new");
> ret = 6;
> goto out_bo;
> }
> --
> 2.28.0
>
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] drm/etnaviv: add total hi bandwidth perf counters
Hi Lucas, almost one month has passed since I sent this little patch series - is there any update on this? Am Fr., 14. Aug. 2020 um 11:05 Uhr schrieb Christian Gmeiner : > > This little patch set adds support for the total bandwidth used by HI. The > basic hi bandwidth read-out is quite simple but I needed to add some little > clean-ups to make it nice looking. > > Christian Gmeiner (4): > drm/etnaviv: rename pipe_reg_read(..) > drm/etnaviv: call perf_reg_read(..) > drm/etnaviv: add total hi bandwidth perfcounter > drm/etnaviv: add pipe_select(..) helper > > drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 78 --- > 1 file changed, 55 insertions(+), 23 deletions(-) > > -- > 2.26.2 > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: fix external abort seen on GC600 rev 0x19
Hi > I have formally tested the patch with 5.7.10 - and it doesn't resolve > the issue - sadly :( > > From my testing, the reads on > VIVS_HI_CHIP_PRODUCT_ID > VIVS_HI_CHIP_ECO_ID > need to be conditional - while > VIVS_HI_CHIP_CUSTOMER_ID > seems to be okay. > Uhh.. okay.. just send a V2 - thanks for testing :) -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/etnaviv: fix external abort seen on GC600 rev 0x19
It looks like that this GPU core triggers an abort when
reading VIVS_HI_CHIP_PRODUCT_ID and/or VIVS_HI_CHIP_ECO_ID.
I looked at different versions of Vivante's kernel driver and did
not found anything about this issue or what feature flag can be
used. So go the simplest route and do not read these two registers
on the affected GPU core.
Signed-off-by: Christian Gmeiner
Reported-by: Josua Mayer
Fixes: 815e45bbd4d3 ("drm/etnaviv: determine product, customer and eco id")
Cc: sta...@vger.kernel.org
---
Changelog:
V2:
- use correct register for conditional reads.
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index d5a4cd85a0f6..c6404b8d067f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -337,9 +337,16 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
gpu->identity.model = gpu_read(gpu, VIVS_HI_CHIP_MODEL);
gpu->identity.revision = gpu_read(gpu, VIVS_HI_CHIP_REV);
- gpu->identity.product_id = gpu_read(gpu,
VIVS_HI_CHIP_PRODUCT_ID);
gpu->identity.customer_id = gpu_read(gpu,
VIVS_HI_CHIP_CUSTOMER_ID);
- gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
+
+ /*
+* Reading these two registers on GC600 rev 0x19 result in a
+* unhandled fault: external abort on non-linefetch
+*/
+ if (!etnaviv_is_model_rev(gpu, GC600, 0x19)) {
+ gpu->identity.product_id = gpu_read(gpu,
VIVS_HI_CHIP_PRODUCT_ID);
+ gpu->identity.eco_id = gpu_read(gpu,
VIVS_HI_CHIP_ECO_ID);
+ }
/*
* HACK ALERT
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/etnaviv: fix external abort seen on GC600 rev 0x19
It looks like that this GPU core triggers an abort when
reading VIVS_HI_CHIP_PRODUCT_ID and/or VIVS_HI_CHIP_CUSTOMER_ID.
I looked at different versions of Vivante's kernel driver and did
not found anything about this issue or what feature flag can be
used. So go the simplest route and do not read these two registers
on the affected GPU core.
Signed-off-by: Christian Gmeiner
Reported-by: Josua Mayer
Fixes: 815e45bbd4d3 ("drm/etnaviv: determine product, customer and eco id")
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index d5a4cd85a0f6..d3906688c2b3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -337,10 +337,17 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
gpu->identity.model = gpu_read(gpu, VIVS_HI_CHIP_MODEL);
gpu->identity.revision = gpu_read(gpu, VIVS_HI_CHIP_REV);
- gpu->identity.product_id = gpu_read(gpu,
VIVS_HI_CHIP_PRODUCT_ID);
- gpu->identity.customer_id = gpu_read(gpu,
VIVS_HI_CHIP_CUSTOMER_ID);
gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
+ /*
+* Reading these two registers on GC600 rev 0x19 result in a
+* unhandled fault: external abort on non-linefetch
+*/
+ if (!etnaviv_is_model_rev(gpu, GC600, 0x19)) {
+ gpu->identity.product_id = gpu_read(gpu,
VIVS_HI_CHIP_PRODUCT_ID);
+ gpu->identity.customer_id = gpu_read(gpu,
VIVS_HI_CHIP_CUSTOMER_ID);
+ }
+
/*
* HACK ALERT
* Because people change device IDs without letting software
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/etnaviv: call perf_reg_read(..)
Replace the open coded access pattern with a function call. Signed-off-by: Christian Gmeiner --- drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c index 1f0402f7a7de..782732e6ce72 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c @@ -58,8 +58,7 @@ static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu, clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK); clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i); gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock); - gpu_write(gpu, domain->profile_config, signal->data); - value += gpu_read(gpu, domain->profile_read); + value += perf_reg_read(gpu, domain, signal); } /* switch back to pixel pipe 0 to prevent GPU hang */ -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] drm/etnaviv: add total hi bandwidth perf counters
This little patch set adds support for the total bandwidth used by HI. The basic hi bandwidth read-out is quite simple but I needed to add some little clean-ups to make it nice looking. Christian Gmeiner (4): drm/etnaviv: rename pipe_reg_read(..) drm/etnaviv: call perf_reg_read(..) drm/etnaviv: add total hi bandwidth perfcounter drm/etnaviv: add pipe_select(..) helper drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 78 --- 1 file changed, 55 insertions(+), 23 deletions(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/etnaviv: add total hi bandwidth perfcounter
These two perf counters represent the total read and write
GPU bandwidth in terms of 64bits.
The used sequence was taken from Vivante kernel driver.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 35 ++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index 782732e6ce72..b37459f022d7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -69,6 +69,29 @@ static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
return value;
}
+static u32 pipe_reg_read(struct etnaviv_gpu *gpu,
+ const struct etnaviv_pm_domain *domain,
+ const struct etnaviv_pm_signal *signal)
+{
+ u32 clock = gpu_read(gpu, VIVS_HI_CLOCK_CONTROL);
+ u32 value = 0;
+ unsigned i;
+
+ for (i = 0; i < gpu->identity.pixel_pipes; i++) {
+ clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
+ clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
+ gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+ value += gpu_read(gpu, signal->data);
+ }
+
+ /* switch back to pixel pipe 0 to prevent GPU hang */
+ clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
+ clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(0);
+ gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+
+ return value;
+}
+
static u32 hi_total_cycle_read(struct etnaviv_gpu *gpu,
const struct etnaviv_pm_domain *domain,
const struct etnaviv_pm_signal *signal)
@@ -102,8 +125,18 @@ static const struct etnaviv_pm_domain doms_3d[] = {
.name = "HI",
.profile_read = VIVS_MC_PROFILE_HI_READ,
.profile_config = VIVS_MC_PROFILE_CONFIG2,
- .nr_signals = 5,
+ .nr_signals = 7,
.signal = (const struct etnaviv_pm_signal[]) {
+ {
+ "TOTAL_READ_BYTES8",
+ VIVS_HI_PROFILE_READ_BYTES8,
+ _reg_read,
+ },
+ {
+ "TOTAL_WRITE_BYTES8",
+ VIVS_HI_PROFILE_WRITE_BYTES8,
+ _reg_read,
+ },
{
"TOTAL_CYCLES",
0,
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/etnaviv: rename pipe_reg_read(..)
pipe_reg_read(..) iterates over all pixel pipes, selects a perf counter
register and sums the actual perf counter value. Rename the function
to reflect more what it is actual doing.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 30 +++
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index 75f9db8f7bec..1f0402f7a7de 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -46,7 +46,7 @@ static u32 perf_reg_read(struct etnaviv_gpu *gpu,
return gpu_read(gpu, domain->profile_read);
}
-static u32 pipe_reg_read(struct etnaviv_gpu *gpu,
+static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
const struct etnaviv_pm_domain *domain,
const struct etnaviv_pm_signal *signal)
{
@@ -141,22 +141,22 @@ static const struct etnaviv_pm_domain doms_3d[] = {
{
"PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_KILLED_BY_COLOR_PIPE,
- _reg_read
+ _perf_reg_read
},
{
"PIXEL_COUNT_KILLED_BY_DEPTH_PIPE",
VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_KILLED_BY_DEPTH_PIPE,
- _reg_read
+ _perf_reg_read
},
{
"PIXEL_COUNT_DRAWN_BY_COLOR_PIPE",
VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_DRAWN_BY_COLOR_PIPE,
- _reg_read
+ _perf_reg_read
},
{
"PIXEL_COUNT_DRAWN_BY_DEPTH_PIPE",
VIVS_MC_PROFILE_CONFIG0_PE_PIXEL_COUNT_DRAWN_BY_DEPTH_PIPE,
- _reg_read
+ _perf_reg_read
}
}
},
@@ -184,32 +184,32 @@ static const struct etnaviv_pm_domain doms_3d[] = {
{
"VS_INST_COUNTER",
VIVS_MC_PROFILE_CONFIG0_SH_VS_INST_COUNTER,
- _reg_read
+ _perf_reg_read
},
{
"RENDERED_VERTICE_COUNTER",
VIVS_MC_PROFILE_CONFIG0_SH_RENDERED_VERTICE_COUNTER,
- _reg_read
+ _perf_reg_read
},
{
"VTX_BRANCH_INST_COUNTER",
VIVS_MC_PROFILE_CONFIG0_SH_VTX_BRANCH_INST_COUNTER,
- _reg_read
+ _perf_reg_read
},
{
"VTX_TEXLD_INST_COUNTER",
VIVS_MC_PROFILE_CONFIG0_SH_VTX_TEXLD_INST_COUNTER,
- _reg_read
+ _perf_reg_read
},
{
"PXL_BRANCH_INST_COUNTER",
VIVS_MC_PROFILE_CONFIG0_SH_PXL_BRANCH_INST_COUNTER,
- _reg_read
+ _perf_reg_read
},
{
"PXL_TEXLD_INST_COUNTER",
VIVS_MC_PROFILE_CONFIG0_SH_PXL_TEXLD_INST_COUNTER,
- _reg_read
+ _perf_reg_read
}
}
},
@@ -237,17 +237,17 @@ static const struct etnaviv_pm_domain doms_3d[] = {
{
"DEPTH_CLIPPED_COUNTER",
VIVS_MC_PROFILE_CONFIG1_PA_DEPTH_CLIPPED_COUNTER,
- _reg_read
+ _perf_reg_read
},
{
"TRIVIAL_REJECTED_COUNTER",
VIVS_MC_PROFILE_CONFIG1_PA_TRIVIAL_REJECTED_COUNTER,
- _reg_read
+ _perf_reg_read
},
{
"CULLED_COUNTER",
VIVS_MC_PROFILE_CONFIG1_PA_CULLED_COUNTER,
- _reg_read
+
[PATCH 4/4] drm/etnaviv: add pipe_select(..) helper
Replace the open coded pixel pipe selection pattern with a function.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 24 +++
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index b37459f022d7..bafdfe49c1d8 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -46,6 +46,14 @@ static u32 perf_reg_read(struct etnaviv_gpu *gpu,
return gpu_read(gpu, domain->profile_read);
}
+static inline void pipe_select(struct etnaviv_gpu *gpu, u32 clock, unsigned
pipe)
+{
+ clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
+ clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(pipe);
+
+ gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+}
+
static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
const struct etnaviv_pm_domain *domain,
const struct etnaviv_pm_signal *signal)
@@ -55,16 +63,12 @@ static u32 pipe_perf_reg_read(struct etnaviv_gpu *gpu,
unsigned i;
for (i = 0; i < gpu->identity.pixel_pipes; i++) {
- clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
- clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
- gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+ pipe_select(gpu, clock, i);
value += perf_reg_read(gpu, domain, signal);
}
/* switch back to pixel pipe 0 to prevent GPU hang */
- clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
- clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(0);
- gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+ pipe_select(gpu, clock, 0);
return value;
}
@@ -78,16 +82,12 @@ static u32 pipe_reg_read(struct etnaviv_gpu *gpu,
unsigned i;
for (i = 0; i < gpu->identity.pixel_pipes; i++) {
- clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
- clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(i);
- gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+ pipe_select(gpu, clock, i);
value += gpu_read(gpu, signal->data);
}
/* switch back to pixel pipe 0 to prevent GPU hang */
- clock &= ~(VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE__MASK);
- clock |= VIVS_HI_CLOCK_CONTROL_DEBUG_PIXEL_PIPE(0);
- gpu_write(gpu, VIVS_HI_CLOCK_CONTROL, clock);
+ pipe_select(gpu, clock, 0);
return value;
}
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] Add support for GPU load values
Hi Lucas, Am Fr., 10. Juli 2020 um 10:44 Uhr schrieb Christian Gmeiner : > > Hoi Lucas > > Am Fr., 10. Juli 2020 um 10:31 Uhr schrieb Lucas Stach > : > > > > Hi Christian, > > > > Am Freitag, den 10.07.2020, 09:41 +0200 schrieb Christian Gmeiner: > > > This patch series add support for loadavg values for GPU > > > sub-components. I am adding a SMA algorithm as I was not > > > really sure if EWMA would be a good fit for this use case. > > > > 1 second is a pretty long window in GPU time. Why do you feel that a > > simple moving average is more appropriate than a exponentially > > weighted one here? Note that I haven't given this any thought myself > > and haven't made up my mind yet, so this is a honest question to > > understand the reasoning behind your choice. > > > I have v2 ready except for this point. If you want to go with EWMA could you provide me with a good weight reciprocal value to use? -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/etnaviv: add loadavg accounting
Hoi Lucas,
Am Fr., 10. Juli 2020 um 10:19 Uhr schrieb Lucas Stach :
>
> Hi Christian,
>
> Am Freitag, den 10.07.2020, 09:41 +0200 schrieb Christian Gmeiner:
> > The GPU has an idle state register where each bit represents the idle
> > state of a sub-GPU component like FE or TX. Sample this register
> > every 10ms and calculate a simple moving average over the sub-GPU
> > component idle states with a total observation time frame of 1s.
> >
> > This provides us with a percentage based load of each sub-GPU
> > component.
> >
> > Signed-off-by: Christian Gmeiner
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 32 +++
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 29
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index f9afe11c50f0..b31920241c86 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -46,6 +46,19 @@ static void load_gpu(struct drm_device *dev)
> > }
> > }
> >
> > +static void unload_gpu(struct drm_device *dev)
> > +{
> > + struct etnaviv_drm_private *priv = dev->dev_private;
> > + unsigned int i;
> > +
> > + for (i = 0; i < ETNA_MAX_PIPES; i++) {
> > + struct etnaviv_gpu *g = priv->gpu[i];
> > +
> > + if (g)
> > + etnaviv_gpu_shutdown(g);
> > + }
> > +}
> > +
> > static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
> > {
> > struct etnaviv_drm_private *priv = dev->dev_private;
> > @@ -581,6 +594,7 @@ static void etnaviv_unbind(struct device *dev)
> > struct drm_device *drm = dev_get_drvdata(dev);
> > struct etnaviv_drm_private *priv = drm->dev_private;
> >
> > + unload_gpu(drm);
> > drm_dev_unregister(drm);
> >
> > component_unbind_all(dev, drm);
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index a31eeff2b297..1f0eb7e00657 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -714,6 +714,28 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu
> > *gpu)
> > gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
> > }
> >
> > +static void etnaviv_loadavg_function(struct timer_list *t)
> > +{
> > + struct etnaviv_gpu *gpu = from_timer(gpu, t, loadavg_timer);
> > + const u32 idle = gpu_read(gpu, VIVS_HI_IDLE_STATE);
>
> This isn't guaranteed to work on a clock/power gated GPU. Also we
> surely don't want to wake a idle system every 10ms just to sample a "no
> load" value, so this needs some integration with runtime PM, to disable
> the sampling when the GPU is powered down and enable when powered up.
> The loadavg must be able to adapt to jumps in the sampling interval
> while idle.
>
Oh yea.. runtime PM.. I thought I was missing something. Will tackle this in the
next version.
>
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> > + if ((idle & etna_idle_module_names[i].bit))
> > + sma_loadavg_add(>loadavg_value[i], 0);
> > + else
> > + sma_loadavg_add(>loadavg_value[i], 100);
> > +
> > + spin_lock_bh(>loadavg_spinlock);
> > +
> > + for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
> > + gpu->loadavg_percentage[i] =
> > sma_loadavg_read(>loadavg_value[i]);
> > +
> > + spin_unlock_bh(>loadavg_spinlock);
> > +
> > + mod_timer(t, jiffies + msecs_to_jiffies(10));
>
> A jiffies based timer is much too coarse for a regular 10ms sampling.
> On a typical 100Hz system 10ms is a single jiffy, so your timer will
> fire anywhere in the range of ~0ms...~20ms. This won't get us a usable
> measurement.
>
Makes sense.. will switch to hrtimers.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] Add support for GPU load values
Hoi Lucas Am Fr., 10. Juli 2020 um 10:31 Uhr schrieb Lucas Stach : > > Hi Christian, > > Am Freitag, den 10.07.2020, 09:41 +0200 schrieb Christian Gmeiner: > > This patch series add support for loadavg values for GPU > > sub-components. I am adding a SMA algorithm as I was not > > really sure if EWMA would be a good fit for this use case. > > 1 second is a pretty long window in GPU time. Why do you feel that a > simple moving average is more appropriate than a exponentially > weighted one here? Note that I haven't given this any thought myself > and haven't made up my mind yet, so this is a honest question to > understand the reasoning behind your choice. > I played with both variants but I 'feel' that SMA might be a better fit. To be honest I have no background in signal processing and stuff like this so.. I will go the route you guide me to :) I have kept the "interface" for SMA equal to the one EWMA uses so I can easily switch between them. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/etnaviv: show loadavg in debugfs
Might be helpful to see the loadavg in debugfs.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 1f0eb7e00657..82fe4aafed57 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -884,7 +884,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct
seq_file *m)
{
struct dma_debug debug;
u32 dma_lo, dma_hi, axi, idle;
- int ret;
+ int ret, i;
seq_printf(m, "%s Status:\n", dev_name(gpu->dev));
@@ -1002,6 +1002,16 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct
seq_file *m)
if (idle & VIVS_HI_IDLE_STATE_AXI_LP)
seq_puts(m, "\t AXI low power mode\n");
+ seq_printf(m, "\tload:\n");
+ spin_lock_bh(>loadavg_spinlock);
+
+ for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
+ seq_printf(m, "\t %s: %u%%\n",
+ etna_idle_module_names[i].name,
+ gpu->loadavg_percentage[i]);
+
+ spin_unlock_bh(>loadavg_spinlock);
+
if (gpu->identity.features & chipFeatures_DEBUG_MODE) {
u32 read0 = gpu_read(gpu, VIVS_MC_DEBUG_READ0);
u32 read1 = gpu_read(gpu, VIVS_MC_DEBUG_READ1);
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/etnaviv: export loadavg via perfmon
Make it possible to access the sub-GPU component load value from
user space with the perfmon infrastructure.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 79 +++
1 file changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index 75f9db8f7bec..614d86e2802d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -98,6 +98,19 @@ static u32 hi_total_idle_cycle_read(struct etnaviv_gpu *gpu,
return gpu_read(gpu, reg);
}
+static u32 load_read(struct etnaviv_gpu *gpu,
+ const struct etnaviv_pm_domain *domain,
+ const struct etnaviv_pm_signal *signal)
+{
+ u32 load;
+
+ spin_lock_bh(>loadavg_spinlock);
+ load = gpu->loadavg_percentage[signal->data];
+ spin_unlock_bh(>loadavg_spinlock);
+
+ return load;
+}
+
static const struct etnaviv_pm_domain doms_3d[] = {
{
.name = "HI",
@@ -387,6 +400,72 @@ static const struct etnaviv_pm_domain doms_3d[] = {
_reg_read
}
}
+ },
+ {
+ .name = "LOAD",
+ .nr_signals = 12,
+ .signal = (const struct etnaviv_pm_signal[]) {
+ {
+ "FE",
+ 0,
+ _read
+ },
+ {
+ "DE",
+ 1,
+ _read
+ },
+ {
+ "PE",
+ 2,
+ _read
+ },
+ {
+ "SH",
+ 3,
+ _read
+ },
+ {
+ "PA",
+ 4,
+ _read
+ },
+ {
+ "SE",
+ 5,
+ _read
+ },
+ {
+ "RA",
+ 6,
+ _read
+ },
+ {
+ "TX",
+ 7,
+ _read
+ },
+ {
+ "VG",
+ 8,
+ _read
+ },
+ {
+ "IM",
+ 9,
+ _read
+ },
+ {
+ "FP",
+ 10,
+ _read
+ },
+ {
+ "TS",
+ 11,
+ _read
+ }
+ }
}
};
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/etnaviv: add simple moving average (SMA)
This adds a SMA algorithm inspired by Exponentially weighted moving
average (EWMA) algorithm found in the kernel.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_sma.h | 53 +++
1 file changed, 53 insertions(+)
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_sma.h
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sma.h
b/drivers/gpu/drm/etnaviv/etnaviv_sma.h
new file mode 100644
index ..81564d5cbdc3
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sma.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 Etnaviv Project
+ */
+
+#ifndef __ETNAVIV_SMA_H__
+#define __ETNAVIV_SMA_H__
+
+#include
+#include
+
+/*
+ * Simple moving average (SMA)
+ *
+ * This implements a fixed-size SMA algorithm.
+ *
+ * The first argument to the macro is the name that will be used
+ * for the struct and helper functions.
+ *
+ * The second argument, the samples, expresses how many samples are
+ * used for the SMA algorithm.
+ */
+
+#define DECLARE_SMA(name, _samples) \
+struct sma_##name { \
+unsigned long pos; \
+unsigned long sum; \
+unsigned long samples[_samples]; \
+}; \
+static inline void sma_##name##_init(struct sma_##name *s) \
+{ \
+BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
+memset(s, 0, sizeof(struct sma_##name)); \
+} \
+static inline unsigned long sma_##name##_read(struct sma_##name *s) \
+{ \
+BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
+return s->sum / _samples; \
+} \
+static inline void sma_##name##_add(struct sma_##name *s, unsigned long
val) \
+{ \
+unsigned long pos = READ_ONCE(s->pos); \
+unsigned long sum = READ_ONCE(s->sum); \
+unsigned long sample = READ_ONCE(s->samples[pos]); \
+ \
+BUILD_BUG_ON(!__builtin_constant_p(_samples)); \
+ \
+ WRITE_ONCE(s->sum, sum - sample + val); \
+ WRITE_ONCE(s->samples[pos], val); \
+ WRITE_ONCE(s->pos, pos + 1 == _samples ? 0 : pos + 1); \
+}
+
+#endif /* __ETNAVIV_SMA_H__ */
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/etnaviv: add loadavg accounting
The GPU has an idle state register where each bit represents the idle
state of a sub-GPU component like FE or TX. Sample this register
every 10ms and calculate a simple moving average over the sub-GPU
component idle states with a total observation time frame of 1s.
This provides us with a percentage based load of each sub-GPU
component.
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 32 +++
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 29
3 files changed, 75 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index f9afe11c50f0..b31920241c86 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -46,6 +46,19 @@ static void load_gpu(struct drm_device *dev)
}
}
+static void unload_gpu(struct drm_device *dev)
+{
+ struct etnaviv_drm_private *priv = dev->dev_private;
+ unsigned int i;
+
+ for (i = 0; i < ETNA_MAX_PIPES; i++) {
+ struct etnaviv_gpu *g = priv->gpu[i];
+
+ if (g)
+ etnaviv_gpu_shutdown(g);
+ }
+}
+
static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
{
struct etnaviv_drm_private *priv = dev->dev_private;
@@ -581,6 +594,7 @@ static void etnaviv_unbind(struct device *dev)
struct drm_device *drm = dev_get_drvdata(dev);
struct etnaviv_drm_private *priv = drm->dev_private;
+ unload_gpu(drm);
drm_dev_unregister(drm);
component_unbind_all(dev, drm);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index a31eeff2b297..1f0eb7e00657 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -714,6 +714,28 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
}
+static void etnaviv_loadavg_function(struct timer_list *t)
+{
+ struct etnaviv_gpu *gpu = from_timer(gpu, t, loadavg_timer);
+ const u32 idle = gpu_read(gpu, VIVS_HI_IDLE_STATE);
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
+ if ((idle & etna_idle_module_names[i].bit))
+ sma_loadavg_add(>loadavg_value[i], 0);
+ else
+ sma_loadavg_add(>loadavg_value[i], 100);
+
+ spin_lock_bh(>loadavg_spinlock);
+
+ for (i = 0; i < ARRAY_SIZE(etna_idle_module_names); i++)
+ gpu->loadavg_percentage[i] =
sma_loadavg_read(>loadavg_value[i]);
+
+ spin_unlock_bh(>loadavg_spinlock);
+
+ mod_timer(t, jiffies + msecs_to_jiffies(10));
+}
+
int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
{
struct etnaviv_drm_private *priv = gpu->drm->dev_private;
@@ -804,6 +826,10 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
for (i = 0; i < ARRAY_SIZE(gpu->event); i++)
complete(>event_free);
+ /* Setup loadavg timer */
+ timer_setup(>loadavg_timer, etnaviv_loadavg_function, 0);
+ mod_timer(>loadavg_timer, jiffies + msecs_to_jiffies(10));
+
/* Now program the hardware */
mutex_lock(>lock);
etnaviv_gpu_hw_init(gpu);
@@ -824,6 +850,11 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
return ret;
}
+void etnaviv_gpu_shutdown(struct etnaviv_gpu *gpu)
+{
+ del_timer(>loadavg_timer);
+}
+
#ifdef CONFIG_DEBUG_FS
struct dma_debug {
u32 address[2];
@@ -1762,6 +1793,7 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
gpu->dev = >dev;
mutex_init(>lock);
mutex_init(>fence_lock);
+ spin_lock_init(>loadavg_spinlock);
/* Map registers: */
gpu->mmio = devm_platform_ioremap_resource(pdev, 0);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 8ea48697d132..a5b9c89c6744 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -10,6 +10,8 @@
#include "etnaviv_gem.h"
#include "etnaviv_mmu.h"
#include "etnaviv_drv.h"
+#include "etnaviv_sma.h"
+#include "state_hi.xml.h"
struct etnaviv_gem_submit;
struct etnaviv_vram_mapping;
@@ -91,6 +93,26 @@ struct clk;
#define ETNA_NR_EVENTS 30
+DECLARE_SMA(loadavg, 100)
+
+static const struct {
+const char *name;
+u32 bit;
+} etna_idle_module_names[] = {
+{ "FE", VIVS_HI_IDLE_STATE_FE },
+{ "DE", VIVS_HI_IDLE_STATE_DE },
+{ "PE", VIVS_HI_IDLE_STATE_PE },
+{ "SH", VIVS_HI_IDLE_STATE_SH },
+{ "PA", VIVS_HI_IDLE_STATE_PA },
+{ "SE", VIVS_HI_IDLE_STATE_SE },
+{ "RA", VIVS_HI_IDLE_STATE_R
[PATCH 0/4] Add support for GPU load values
This patch series add support for loadavg values for GPU sub-components. I am adding a SMA algorithm as I was not really sure if EWMA would be a good fit for this use case. Christian Gmeiner (4): drm/etnaviv: add simple moving average (SMA) drm/etnaviv: add loadavg accounting drm/etnaviv: show loadavg in debugfs drm/etnaviv: export loadavg via perfmon drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 44 - drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 29 + drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 79 +++ drivers/gpu/drm/etnaviv/etnaviv_sma.h | 53 +++ 5 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_sma.h -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/etnaviv: fix perfmon domain interation
The GC860 has one GPU device which has a 2d and 3d core. In this case
we want to expose perfmon information for both cores.
The driver has one array which contains all possible perfmon domains
with some meta data - doms_meta. Here we can see that for the GC860
two elements of that array are relevant:
doms_3d: is at index 0 in the doms_meta array with 8 perfmon domains
doms_2d: is at index 1 in the doms_meta array with 1 perfmon domain
The userspace driver wants to get a list of all perfmon domains and
their perfmon signals. This is done by iterating over all domains and
their signals. If the userspace driver wants to access the domain with
id 8 the kernel driver fails and returns invalid data from doms_3d with
and invalid offset.
This results in:
Unable to handle kernel paging request at virtual address
On such a device it is not possible to use the userspace driver at all.
The fix for this off-by-one error is quite simple.
Reported-by: Paul Cercueil
Tested-by: Paul Cercueil
Fixes: ed1dd899baa3 ("drm/etnaviv: rework perfmon query infrastructure")
Cc: sta...@vger.kernel.org
Signed-off-by: Christian Gmeiner
---
drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index e6795bafcbb9..75f9db8f7bec 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -453,7 +453,7 @@ static const struct etnaviv_pm_domain *pm_domain(const
struct etnaviv_gpu *gpu,
if (!(gpu->identity.features & meta->feature))
continue;
- if (meta->nr_domains < (index - offset)) {
+ if (index - offset >= meta->nr_domains) {
offset += meta->nr_domains;
continue;
}
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: fix perfmon domain interation
Hi Lucas,
Am So., 17. Mai 2020 um 14:03 Uhr schrieb Lucas Stach :
>
> Hi Christian,
>
> Am Montag, den 11.05.2020, 14:37 +0200 schrieb Christian Gmeiner:
> > The GC860 has one GPU device which has a 2d and 3d core. In this case
> > we want to expose perfmon information for both cores.
> >
> > The driver has one array which contains all possible perfmon domains
> > with some meta data - doms_meta. Here we can see that for the GC860
> > two elements of that array are relevant:
> >
> > doms_3d: is at index 0 in the doms_meta array with 8 perfmon domains
> > doms_2d: is at index 1 in the doms_meta array with 1 perfmon domain
> >
> > The userspace driver wants to get a list of all perfmon domains and
> > their perfmon signals. This is done by iterating over all domains and
> > their signals. If the userspace driver wants to access the domain with
> > id 8 the kernel driver fails and returns invalid data from doms_3d with
> > and invalid offset.
> >
> > This results in:
> > Unable to handle kernel paging request at virtual address
> >
> > On such a device it is not possible to use the userspace driver at all.
> >
> > The fix for this off-by-one error is quite simple.
> >
> > Reported-by: Paul Cercueil
> > Tested-by: Paul Cercueil
> > Fixes: ed1dd899baa3 ("drm/etnaviv: rework perfmon query infrastructure")
> > Cc: sta...@vger.kernel.or
>
> Missing last letter of the TLD.
>
> > Signed-off-by: Christian Gmeiner
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > index e6795bafcbb9..35f7171e779a 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > @@ -453,7 +453,7 @@ static const struct etnaviv_pm_domain *pm_domain(const
> > struct etnaviv_gpu *gpu,
> > if (!(gpu->identity.features & meta->feature))
> > continue;
> >
> > - if (meta->nr_domains < (index - offset)) {
> > + if ((meta->nr_domains - 1) < (index - offset)) {
>
> While the logic is correct, I find this quite hard to read. A more
> idiomatic way to write this (which is much easier to grok when reading
> the code IMHO) would be:
>
> if (index - offset >= meta->nr_domains)
>
> If you agree, please send a v2 of this patch.
>
Works for me - will send a v2 in the evening.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: fix perfmon domain interation
Am Fr., 15. Mai 2020 um 12:33 Uhr schrieb Lucas Stach :
>
> Am Freitag, den 15.05.2020, 12:27 +0200 schrieb Christian Gmeiner:
> > Am Fr., 15. Mai 2020 um 12:24 Uhr schrieb Lucas Stach
> > :
> > > Am Freitag, den 15.05.2020, 12:12 +0200 schrieb Paul Cercueil:
> > > > Hi Christian,
> > > >
> > > > Le ven. 15 mai 2020 à 12:09, Christian Gmeiner
> > > > a écrit :
> > > > > Am Mo., 11. Mai 2020 um 14:38 Uhr schrieb Christian Gmeiner
> > > > > :
> > > > > > The GC860 has one GPU device which has a 2d and 3d core. In this
> > > > > > case
> > > > > > we want to expose perfmon information for both cores.
> > > > > >
> > > > > > The driver has one array which contains all possible perfmon
> > > > > > domains
> > > > > > with some meta data - doms_meta. Here we can see that for the GC860
> > > > > > two elements of that array are relevant:
> > > > > >
> > > > > >doms_3d: is at index 0 in the doms_meta array with 8 perfmon
> > > > > > domains
> > > > > >doms_2d: is at index 1 in the doms_meta array with 1 perfmon
> > > > > > domain
> > > > > >
> > > > > > The userspace driver wants to get a list of all perfmon domains and
> > > > > > their perfmon signals. This is done by iterating over all domains
> > > > > > and
> > > > > > their signals. If the userspace driver wants to access the domain
> > > > > > with
> > > > > > id 8 the kernel driver fails and returns invalid data from doms_3d
> > > > > > with
> > > > > > and invalid offset.
> > > > > >
> > > > > > This results in:
> > > > > >Unable to handle kernel paging request at virtual address
> > > > > >
> > > > > >
> > > > > > On such a device it is not possible to use the userspace driver at
> > > > > > all.
> > > > > >
> > > > > > The fix for this off-by-one error is quite simple.
> > > > > >
> > > > > > Reported-by: Paul Cercueil
> > > > > > Tested-by: Paul Cercueil
> > > > > > Fixes: ed1dd899baa3 ("drm/etnaviv: rework perfmon query
> > > > > > infrastructure")
> > > > > > Cc: sta...@vger.kernel.org
> > > > > > Signed-off-by: Christian Gmeiner
> > > > > > ---
> > > > > > drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > > > b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > > > index e6795bafcbb9..35f7171e779a 100644
> > > > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > > > @@ -453,7 +453,7 @@ static const struct etnaviv_pm_domain
> > > > > > *pm_domain(const struct etnaviv_gpu *gpu,
> > > > > > if (!(gpu->identity.features & meta->feature))
> > > > > > continue;
> > > > > >
> > > > > > - if (meta->nr_domains < (index - offset)) {
> > > > > > + if ((meta->nr_domains - 1) < (index - offset)) {
> > > > > > offset += meta->nr_domains;
> > > > > > continue;
> > > > > > }
> > > > > > --
> > > > > > 2.26.2
> > > > > >
> > > > >
> > > > > ping
> > > >
> > > > I'll merge it tomorrow if there's no further feedback.
> > >
> > > Huh? Etnaviv patches are going through the etnaviv tree.
> > >
> > > We now have two different solutions to the same issue. I first want to
> > > dig into the code to see why two developers can get confused enough by
> > > the code to come up with totally different fixes.
> > >
> >
> > You will see that the solutions are not totally different. I really hoped to
> > get this fixed in the 5.7 release.. but I think its now too late.
>
> I didn't have time to look at the full picture, yet. We still have at
> least a week until the final 5.7 release, why would it be too late to
> get a fix upstream?
>
Great - so I count on you that we will have a fix in 5.7 release.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: fix perfmon domain interation
Am Fr., 15. Mai 2020 um 12:24 Uhr schrieb Lucas Stach :
>
> Am Freitag, den 15.05.2020, 12:12 +0200 schrieb Paul Cercueil:
> > Hi Christian,
> >
> > Le ven. 15 mai 2020 à 12:09, Christian Gmeiner
> > a écrit :
> > > Am Mo., 11. Mai 2020 um 14:38 Uhr schrieb Christian Gmeiner
> > > :
> > > > The GC860 has one GPU device which has a 2d and 3d core. In this
> > > > case
> > > > we want to expose perfmon information for both cores.
> > > >
> > > > The driver has one array which contains all possible perfmon domains
> > > > with some meta data - doms_meta. Here we can see that for the GC860
> > > > two elements of that array are relevant:
> > > >
> > > >doms_3d: is at index 0 in the doms_meta array with 8 perfmon
> > > > domains
> > > >doms_2d: is at index 1 in the doms_meta array with 1 perfmon
> > > > domain
> > > >
> > > > The userspace driver wants to get a list of all perfmon domains and
> > > > their perfmon signals. This is done by iterating over all domains
> > > > and
> > > > their signals. If the userspace driver wants to access the domain
> > > > with
> > > > id 8 the kernel driver fails and returns invalid data from doms_3d
> > > > with
> > > > and invalid offset.
> > > >
> > > > This results in:
> > > >Unable to handle kernel paging request at virtual address
> > > >
> > > > On such a device it is not possible to use the userspace driver at
> > > > all.
> > > >
> > > > The fix for this off-by-one error is quite simple.
> > > >
> > > > Reported-by: Paul Cercueil
> > > > Tested-by: Paul Cercueil
> > > > Fixes: ed1dd899baa3 ("drm/etnaviv: rework perfmon query
> > > > infrastructure")
> > > > Cc: sta...@vger.kernel.org
> > > > Signed-off-by: Christian Gmeiner
> > > > ---
> > > > drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > index e6795bafcbb9..35f7171e779a 100644
> > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > @@ -453,7 +453,7 @@ static const struct etnaviv_pm_domain
> > > > *pm_domain(const struct etnaviv_gpu *gpu,
> > > > if (!(gpu->identity.features & meta->feature))
> > > > continue;
> > > >
> > > > - if (meta->nr_domains < (index - offset)) {
> > > > + if ((meta->nr_domains - 1) < (index - offset)) {
> > > > offset += meta->nr_domains;
> > > > continue;
> > > > }
> > > > --
> > > > 2.26.2
> > > >
> > >
> > > ping
> >
> > I'll merge it tomorrow if there's no further feedback.
>
> Huh? Etnaviv patches are going through the etnaviv tree.
>
> We now have two different solutions to the same issue. I first want to
> dig into the code to see why two developers can get confused enough by
> the code to come up with totally different fixes.
>
You will see that the solutions are not totally different. I really hoped to
get this fixed in the 5.7 release.. but I think its now too late.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: fix perfmon domain interation
Hi Paul
Am Fr., 15. Mai 2020 um 12:12 Uhr schrieb Paul Cercueil :
>
> Hi Christian,
>
> Le ven. 15 mai 2020 à 12:09, Christian Gmeiner
> a écrit :
> > Am Mo., 11. Mai 2020 um 14:38 Uhr schrieb Christian Gmeiner
> > :
> >>
> >> The GC860 has one GPU device which has a 2d and 3d core. In this
> >> case
> >> we want to expose perfmon information for both cores.
> >>
> >> The driver has one array which contains all possible perfmon domains
> >> with some meta data - doms_meta. Here we can see that for the GC860
> >> two elements of that array are relevant:
> >>
> >>doms_3d: is at index 0 in the doms_meta array with 8 perfmon
> >> domains
> >>doms_2d: is at index 1 in the doms_meta array with 1 perfmon
> >> domain
> >>
> >> The userspace driver wants to get a list of all perfmon domains and
> >> their perfmon signals. This is done by iterating over all domains
> >> and
> >> their signals. If the userspace driver wants to access the domain
> >> with
> >> id 8 the kernel driver fails and returns invalid data from doms_3d
> >> with
> >> and invalid offset.
> >>
> >> This results in:
> >>Unable to handle kernel paging request at virtual address
> >>
> >> On such a device it is not possible to use the userspace driver at
> >> all.
> >>
> >> The fix for this off-by-one error is quite simple.
> >>
> >> Reported-by: Paul Cercueil
> >> Tested-by: Paul Cercueil
> >> Fixes: ed1dd899baa3 ("drm/etnaviv: rework perfmon query
> >> infrastructure")
> >> Cc: sta...@vger.kernel.org
> >> Signed-off-by: Christian Gmeiner
> >> ---
> >> drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> >> b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> >> index e6795bafcbb9..35f7171e779a 100644
> >> --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> >> +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> >> @@ -453,7 +453,7 @@ static const struct etnaviv_pm_domain
> >> *pm_domain(const struct etnaviv_gpu *gpu,
> >> if (!(gpu->identity.features & meta->feature))
> >> continue;
> >>
> >> - if (meta->nr_domains < (index - offset)) {
> >> + if ((meta->nr_domains - 1) < (index - offset)) {
> >> offset += meta->nr_domains;
> >> continue;
> >> }
> >> --
> >> 2.26.2
> >>
> >
> > ping
>
> I'll merge it tomorrow if there's no further feedback.
>
Works for me too.. as far as Lucas (the maintainer) is happy with it.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: fix perfmon domain interation
Am Mo., 11. Mai 2020 um 14:38 Uhr schrieb Christian Gmeiner
:
>
> The GC860 has one GPU device which has a 2d and 3d core. In this case
> we want to expose perfmon information for both cores.
>
> The driver has one array which contains all possible perfmon domains
> with some meta data - doms_meta. Here we can see that for the GC860
> two elements of that array are relevant:
>
> doms_3d: is at index 0 in the doms_meta array with 8 perfmon domains
> doms_2d: is at index 1 in the doms_meta array with 1 perfmon domain
>
> The userspace driver wants to get a list of all perfmon domains and
> their perfmon signals. This is done by iterating over all domains and
> their signals. If the userspace driver wants to access the domain with
> id 8 the kernel driver fails and returns invalid data from doms_3d with
> and invalid offset.
>
> This results in:
> Unable to handle kernel paging request at virtual address
>
> On such a device it is not possible to use the userspace driver at all.
>
> The fix for this off-by-one error is quite simple.
>
> Reported-by: Paul Cercueil
> Tested-by: Paul Cercueil
> Fixes: ed1dd899baa3 ("drm/etnaviv: rework perfmon query infrastructure")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> index e6795bafcbb9..35f7171e779a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> @@ -453,7 +453,7 @@ static const struct etnaviv_pm_domain *pm_domain(const
> struct etnaviv_gpu *gpu,
> if (!(gpu->identity.features & meta->feature))
> continue;
>
> - if (meta->nr_domains < (index - offset)) {
> + if ((meta->nr_domains - 1) < (index - offset)) {
> offset += meta->nr_domains;
> continue;
> }
> --
> 2.26.2
>
ping
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/etnaviv: Fix the pm_domain lookup
Hi Lubomir Am Do., 14. Mai 2020 um 00:02 Uhr schrieb Lubomir Rintel : > > On a GC860 (both 3D and 2D capable) GPU, kmscube crashes: > > # strace -f ~lkundrak/src/kmscube/build/kmscube > ... > ioctl(6, DRM_IOCTL_ETNAVIV_PM_QUERY_DOM, 0xbe92b720) = 0 > ioctl(6, DRM_IOCTL_ETNAVIV_PM_QUERY_SIG ) = ? > +++ killed by SIGSEGV +++ > Segmentation fault (core dumped) > > And triggers an oops: > > 8<--- cut here --- > Unable to handle kernel NULL pointer dereference at virtual address > pgd = 40e2c0f7 > [] *pgd=0df6d831, *pte=, *ppte= > Internal error: Oops: 17 [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 0 PID: 346 Comm: kmscube Not tainted 5.7.0-rc4+ #792 > Hardware name: Marvell MMP2 (Device Tree Support) > PC is at strncpy+0x14/0x30 > LR is at etnaviv_pm_query_sig+0xd0/0x104 > pc : []lr : []psr: 20010013 > sp : c85f5e00 ip : c85f5eb5 fp : beb58748 > r10: 004c r9 : ca6f9100 r8 : c85f5e6c > r7 : 0050 r6 : c85f5e6c r5 : 0001 r4 : c0b69ae8 > r3 : c85f5e75 r2 : 003f r1 : r0 : c85f5e76 > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > Control: 10c5387d Table: 0df70019 DAC: 0051 > Process kmscube (pid: 346, stack limit = 0x816fba31) > Stack: (0xc85f5e00 to 0xc85f6000) > 5e00: d90e6000 0020 c05d5b2c c85f5e6c c059ce90 > c1003f88 > 5e20: c04c644b 004c c0b69610 c04c644b c85f5e6c 004b ca6f9100 > c059d0bc > 5e40: 0001 c0d53ee8 c85f5f18 0001 c85f5f50 c85f5e6c 004c > c8454240 > 5e60: c05d5b2c 0051 0001 > > 5e80: > > 5ea0: 0274 > c011c3fc > 5ec0: 0100 c0290434 5ebc5632 30e03501 5ebc5632 c8526600 0274 > 00100cca > 5ee0: 0831 b64c5000 cdf72d90 c1003f88 c04c644b c8454240 > beb58748 > 5f00: c8454240 0006 c85f4000 d90ecad8 001c01a0 c02d49ac b64c52cc > 8007 > 5f20: da9d6dc0 d9aa4000 d9aa4040 0274 c011818c 0005 > 0e200080 > 5f40: 03e5 0100 > cc78ac40 > 5f60: 0006 0007 c1009a98 b64c52cc c85f5fb0 c0118080 0080 > c1003f88 > 5f80: 0001 beb58748 c04c644b 0036 c0100288 c85f4000 > 0036 > 5fa0: 001c01a0 c0100060 0001 beb58748 0006 c04c644b beb58748 > 004c > 5fc0: 0001 beb58748 c04c644b 0036 beb58748 001bd688 beb58700 > 001c01a0 > 5fe0: b6f41f08 beb586d4 b6f2784c b6e16cec 80010010 0006 > > [] (strncpy) from [] (etnaviv_pm_query_sig+0xd0/0x104) > [] (etnaviv_pm_query_sig) from [] > (drm_ioctl_kernel+0xb4/0xf8) > [] (drm_ioctl_kernel) from [] (drm_ioctl+0x1e8/0x3b8) > [] (drm_ioctl) from [] (ksys_ioctl+0xe0/0xaf0) > [] (ksys_ioctl) from [] (ret_fast_syscall+0x0/0x54) > Exception stack(0xc85f5fa8 to 0xc85f5ff0) > 5fa0: 0001 beb58748 0006 c04c644b beb58748 > 004c > 5fc0: 0001 beb58748 c04c644b 0036 beb58748 001bd688 beb58700 > 001c01a0 > 5fe0: b6f41f08 beb586d4 b6f2784c b6e16cec > Code: 012fff1e e2422001 e2403001 e080c002 (e5d12000) > ---[ end trace 387aad33cd9c15ea ]--- > > Turns out that it's because pm_domain() returns a pointer outside any > any of the etnaviv_pm_domains. Unless I'm mistaken, the algorithm in > pm_domain() is entirely botched when GPU's features match more than one > domain. This tries to remedy it. > > Tested with kmscube with mesa 20 on ome machine with GC860 and another > with GC2000 + GC300 pair. > > Signed-off-by: Lubomir Rintel There is already an older/other fix for this problem: https://patchwork.freedesktop.org/patch/365181/ -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
