[PATCH] drm: mali-dp: Add debugfs file for reporting internal errors
Liviu Dudau
liviu.dudau at arm.com
Mon May 14 09:09:25 UTC 2018
On Fri, May 11, 2018 at 06:56:03PM +0100, Alexandru Gheorghe wrote:
> Status register contains a lot of bits for reporting internal errors
> inside Mali DP. Currently, we just silently ignore all of the erorrs,
> that doesn't help when we are investigating different bugs, especially
> on the FPGA models which have a lot of constrains, so we could easily
> end up in AXI or underrun errors.
>
> Add a new file called debug that contains an agregate of the
> errors reported by the Mali DP hardware.
>
> E.g:
> [root at alarm ~]# cat /sys/kernel/debug/dri/1/debug
> [DE] num_errors : 167
> [DE] last_error_status : 0x00000001
> [DE] last_error_vblank : 385
> [SE] num_errors : 3
> [SE] last_error_status : 0x00e23001
> [SE] last_error_vblank : 201
>
> This a morphosis of the patch presented here [1], where the errors
> where reported continuously via trace_printk.
>
> [1] https://lists.freedesktop.org/archives/dri-devel/2018-February/167042.html
>
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe at arm.com>
Acked-by: Liviu Dudau <liviu.dudau at arm.com>
> ---
> drivers/gpu/drm/arm/malidp_drv.c | 61 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/arm/malidp_drv.h | 15 ++++++++++
> drivers/gpu/drm/arm/malidp_hw.c | 46 ++++++++++++++++++++++++-----
> drivers/gpu/drm/arm/malidp_hw.h | 1 +
> drivers/gpu/drm/arm/malidp_regs.h | 6 ++++
> 5 files changed, 122 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 8d20faa..70ce19a 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -17,6 +17,7 @@
> #include <linux/of_graph.h>
> #include <linux/of_reserved_mem.h>
> #include <linux/pm_runtime.h>
> +#include <linux/debugfs.h>
>
> #include <drm/drmP.h>
> #include <drm/drm_atomic.h>
> @@ -29,6 +30,7 @@
> #include <drm/drm_gem_framebuffer_helper.h>
> #include <drm/drm_modeset_helper.h>
> #include <drm/drm_of.h>
> +#include <drm/drm_debugfs.h>
>
> #include "malidp_drv.h"
> #include "malidp_regs.h"
> @@ -327,6 +329,62 @@ static int malidp_dumb_create(struct drm_file *file_priv,
> return drm_gem_cma_dumb_create_internal(file_priv, drm, args);
> }
>
> +#ifdef CONFIG_DEBUG_FS
> +
> +static void malidp_error_stats_init(struct malidp_error_stats *error_stats)
> +{
> + atomic_set(&error_stats->num_errors, 0);
> + atomic_set(&error_stats->last_error_status, 0);
> + atomic64_set(&error_stats->last_error_vblank, -1);
> +}
> +
> +void malidp_error(struct malidp_error_stats *error_stats, u32 status,
> + u64 vblank)
> +{
> + atomic_set(&error_stats->last_error_status, status);
> + atomic64_set(&error_stats->last_error_vblank, vblank);
> + atomic_inc(&error_stats->num_errors);
> +}
> +
> +void malidp_error_stats_dump(const char *prefix,
> + struct malidp_error_stats *error_stats,
> + struct seq_file *m)
> +{
> + seq_printf(m, "[%s] num_errors : %d\n", prefix,
> + atomic_read(&error_stats->num_errors));
> + seq_printf(m, "[%s] last_error_status : 0x%08x\n", prefix,
> + atomic_read(&error_stats->last_error_status));
> + seq_printf(m, "[%s] last_error_vblank : %ld\n", prefix,
> + atomic64_read(&error_stats->last_error_vblank));
> +}
> +
> +static int malidp_show_stats(struct seq_file *m, void *arg)
> +{
> + struct drm_info_node *node = (struct drm_info_node *)m->private;
> + struct drm_device *drm = node->minor->dev;
> + struct malidp_drm *malidp = drm->dev_private;
> +
> + malidp_error_stats_dump("DE", &malidp->de_errors, m);
> + malidp_error_stats_dump("SE", &malidp->se_errors, m);
> + return 0;
> +}
> +
> +static struct drm_info_list malidp_debugfs_list[] = {
> + { "debug", malidp_show_stats, 0 },
> +};
> +
> +static int malidp_debugfs_init(struct drm_minor *minor)
> +{
> + struct malidp_drm *malidp = minor->dev->dev_private;
> +
> + malidp_error_stats_init(&malidp->de_errors);
> + malidp_error_stats_init(&malidp->se_errors);
> + return drm_debugfs_create_files(malidp_debugfs_list,
> + ARRAY_SIZE(malidp_debugfs_list), minor->debugfs_root, minor);
> +}
> +
> +#endif //CONFIG_DEBUG_FS
> +
> static struct drm_driver malidp_driver = {
> .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
> DRIVER_PRIME,
> @@ -343,6 +401,9 @@ static struct drm_driver malidp_driver = {
> .gem_prime_vmap = drm_gem_cma_prime_vmap,
> .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> .gem_prime_mmap = drm_gem_cma_prime_mmap,
> +#ifdef CONFIG_DEBUG_FS
> + .debugfs_init = malidp_debugfs_init,
> +#endif
> .fops = &fops,
> .name = "mali-dp",
> .desc = "ARM Mali Display Processor driver",
> diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
> index c70989b..c49056c 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.h
> +++ b/drivers/gpu/drm/arm/malidp_drv.h
> @@ -18,6 +18,12 @@
> #include <drm/drmP.h>
> #include "malidp_hw.h"
>
> +struct malidp_error_stats {
> + atomic_t num_errors;
> + atomic_t last_error_status;
> + atomic64_t last_error_vblank;
> +};
> +
> struct malidp_drm {
> struct malidp_hw_device *dev;
> struct drm_crtc crtc;
> @@ -25,6 +31,10 @@ struct malidp_drm {
> struct drm_pending_vblank_event *event;
> atomic_t config_valid;
> u32 core_id;
> +#ifdef CONFIG_DEBUG_FS
> + struct malidp_error_stats de_errors;
> + struct malidp_error_stats se_errors;
> +#endif
> };
>
> #define crtc_to_malidp_device(x) container_of(x, struct malidp_drm, crtc)
> @@ -62,6 +72,11 @@ struct malidp_crtc_state {
> int malidp_de_planes_init(struct drm_device *drm);
> int malidp_crtc_init(struct drm_device *drm);
>
> +#ifdef CONFIG_DEBUG_FS
> +void malidp_error(struct malidp_error_stats *error_stats, u32 status,
> + u64 vblank);
> +#endif
> +
> /* often used combination of rotational bits */
> #define MALIDP_ROTATED_MASK (DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270)
>
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index d789b46..ec40a44 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -632,10 +632,16 @@ const struct malidp_hw malidp_device[MALIDP_MAX_DEVICES] = {
> MALIDP500_DE_IRQ_VSYNC |
> MALIDP500_DE_IRQ_GLOBAL,
> .vsync_irq = MALIDP500_DE_IRQ_VSYNC,
> + .err_mask = MALIDP_DE_IRQ_UNDERRUN |
> + MALIDP500_DE_IRQ_AXI_ERR |
> + MALIDP500_DE_IRQ_SATURATION,
> },
> .se_irq_map = {
> .irq_mask = MALIDP500_SE_IRQ_CONF_MODE,
> .vsync_irq = 0,
> + .err_mask = MALIDP500_SE_IRQ_INIT_BUSY |
> + MALIDP500_SE_IRQ_AXI_ERROR |
> + MALIDP500_SE_IRQ_OVERRUN,
> },
> .dc_irq_map = {
> .irq_mask = MALIDP500_DE_IRQ_CONF_VALID,
> @@ -669,10 +675,15 @@ const struct malidp_hw malidp_device[MALIDP_MAX_DEVICES] = {
> .irq_mask = MALIDP_DE_IRQ_UNDERRUN |
> MALIDP550_DE_IRQ_VSYNC,
> .vsync_irq = MALIDP550_DE_IRQ_VSYNC,
> + .err_mask = MALIDP_DE_IRQ_UNDERRUN |
> + MALIDP550_DE_IRQ_SATURATION |
> + MALIDP550_DE_IRQ_AXI_ERR,
> },
> .se_irq_map = {
> - .irq_mask = MALIDP550_SE_IRQ_EOW |
> - MALIDP550_SE_IRQ_AXI_ERR,
> + .irq_mask = MALIDP550_SE_IRQ_EOW,
> + .err_mask = MALIDP550_SE_IRQ_AXI_ERR |
> + MALIDP550_SE_IRQ_OVR |
> + MALIDP550_SE_IRQ_IBSY,
> },
> .dc_irq_map = {
> .irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
> @@ -707,10 +718,20 @@ const struct malidp_hw malidp_device[MALIDP_MAX_DEVICES] = {
> MALIDP650_DE_IRQ_DRIFT |
> MALIDP550_DE_IRQ_VSYNC,
> .vsync_irq = MALIDP550_DE_IRQ_VSYNC,
> + .err_mask = MALIDP_DE_IRQ_UNDERRUN |
> + MALIDP650_DE_IRQ_DRIFT |
> + MALIDP550_DE_IRQ_SATURATION |
> + MALIDP550_DE_IRQ_AXI_ERR |
> + MALIDP650_DE_IRQ_ACEV1 |
> + MALIDP650_DE_IRQ_ACEV2 |
> + MALIDP650_DE_IRQ_ACEG |
> + MALIDP650_DE_IRQ_AXIEP,
> },
> .se_irq_map = {
> - .irq_mask = MALIDP550_SE_IRQ_EOW |
> - MALIDP550_SE_IRQ_AXI_ERR,
> + .irq_mask = MALIDP550_SE_IRQ_EOW,
> + .err_mask = MALIDP550_SE_IRQ_AXI_ERR |
> + MALIDP550_SE_IRQ_OVR |
> + MALIDP550_SE_IRQ_IBSY,
> },
> .dc_irq_map = {
> .irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
> @@ -799,10 +820,17 @@ static irqreturn_t malidp_de_irq(int irq, void *arg)
> return ret;
>
> mask = malidp_hw_read(hwdev, MALIDP_REG_MASKIRQ);
> - status &= mask;
> + /* keep the status of the enabled interrupts, plus the error bits */
> + status &= (mask | de->err_mask);
> if ((status & de->vsync_irq) && malidp->crtc.enabled)
> drm_crtc_handle_vblank(&malidp->crtc);
>
> +#ifdef CONFIG_DEBUG_FS
> + if (status & de->err_mask) {
> + malidp_error(&malidp->de_errors, status,
> + drm_crtc_vblank_count(&malidp->crtc));
> + }
> +#endif
> malidp_hw_clear_irq(hwdev, MALIDP_DE_BLOCK, status);
>
> return (ret == IRQ_NONE) ? IRQ_HANDLED : ret;
> @@ -878,11 +906,15 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
> return IRQ_NONE;
>
> status = malidp_hw_read(hwdev, hw->map.se_base + MALIDP_REG_STATUS);
> - if (!(status & se->irq_mask))
> + if (!(status & (se->irq_mask | se->err_mask)))
> return IRQ_NONE;
>
> +#ifdef CONFIG_DEBUG_FS
> + if (status & se->err_mask)
> + malidp_error(&malidp->se_errors, status,
> + drm_crtc_vblank_count(&malidp->crtc));
> +#endif
> mask = malidp_hw_read(hwdev, hw->map.se_base + MALIDP_REG_MASKIRQ);
> - status = malidp_hw_read(hwdev, hw->map.se_base + MALIDP_REG_STATUS);
> status &= mask;
> /* ToDo: status decoding and firing up of VSYNC and page flip events */
>
> diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
> index b5dd6c7..b45f99f 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.h
> +++ b/drivers/gpu/drm/arm/malidp_hw.h
> @@ -52,6 +52,7 @@ struct malidp_format_id {
> struct malidp_irq_map {
> u32 irq_mask; /* mask of IRQs that can be enabled in the block */
> u32 vsync_irq; /* IRQ bit used for signaling during VSYNC */
> + u32 err_mask; /* mask of bits that represent errors */
> };
>
> struct malidp_layer {
> diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h
> index 149024f..7c37390 100644
> --- a/drivers/gpu/drm/arm/malidp_regs.h
> +++ b/drivers/gpu/drm/arm/malidp_regs.h
> @@ -53,6 +53,8 @@
> #define MALIDP550_DE_IRQ_AXI_ERR (1 << 16)
> #define MALIDP550_SE_IRQ_EOW (1 << 0)
> #define MALIDP550_SE_IRQ_AXI_ERR (1 << 16)
> +#define MALIDP550_SE_IRQ_OVR (1 << 17)
> +#define MALIDP550_SE_IRQ_IBSY (1 << 18)
> #define MALIDP550_DC_IRQ_CONF_VALID (1 << 0)
> #define MALIDP550_DC_IRQ_CONF_MODE (1 << 4)
> #define MALIDP550_DC_IRQ_CONF_ACTIVE (1 << 16)
> @@ -60,6 +62,10 @@
> #define MALIDP550_DC_IRQ_SE (1 << 24)
>
> #define MALIDP650_DE_IRQ_DRIFT (1 << 4)
> +#define MALIDP650_DE_IRQ_ACEV1 (1 << 17)
> +#define MALIDP650_DE_IRQ_ACEV2 (1 << 18)
> +#define MALIDP650_DE_IRQ_ACEG (1 << 19)
> +#define MALIDP650_DE_IRQ_AXIEP (1 << 28)
>
> /* bit masks that are common between products */
> #define MALIDP_CFG_VALID (1 << 0)
> --
> 2.7.4
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
More information about the dri-devel
mailing list