Hi Alex,
On Thu, Feb 15, 2018 at 07:33:15PM +0000, Alexandru Gheorghe wrote:
> Errors will be reported in /sys/kernel/debug/tracing/trace,
> still not noisy enough, but better than silently ignoring them.
> E.g:
> <idle>-0 [000] d.h1 183.851864: malidp_de_irq: error occurred DE_STATUS is 0x00000001
> surfaceflinger-803 [000] d.h1 197.993003: malidp_de_irq: error occurred DE_STATUS is 0x00000001
> surfaceflinger-803 [000] d.h1 213.595119: malidp_de_irq: error occurred DE_STATUS is 0x00000001
> surfaceflinger-803 [000] d.h. 217.754371: malidp_de_irq: error occurred DE_STATUS is 0x00000001
> surfaceflinger-803 [000] d.h. 217.820848: malidp_de_irq: error occurred DE_STATUS is 0x00000001
> surfaceflinger-803 [000] d.h. 217.854034: malidp_de_irq: error occurred DE_STATUS is 0x00000001
Thanks for the patch, it is quite useful. However, I have some comments:
>
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx>
> ---
> drivers/gpu/drm/arm/malidp_hw.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
> index 2bfb542..8e7d49f 100644
> --- a/drivers/gpu/drm/arm/malidp_hw.c
> +++ b/drivers/gpu/drm/arm/malidp_hw.c
> @@ -797,6 +797,9 @@ static irqreturn_t malidp_de_irq(int irq, void *arg)
> if (status & de->vsync_irq)
> drm_crtc_handle_vblank(&malidp->crtc);
>
> + if (status & ~de->vsync_irq & de->irq_mask)
> + trace_printk("error occurred DE_STATUS is 0x%08X\n", status);
de->irq_mask is already programmed in the MASKIRQ register, so the last
part of the masking is already done. I suggest you replace the 'if' with
an 'else' for the existing check above and it should be equivalent.
> +
> malidp_hw_clear_irq(hwdev, MALIDP_DE_BLOCK, status);
>
> return (ret == IRQ_NONE) ? IRQ_HANDLED : ret;
> @@ -880,6 +883,9 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
> status &= mask;
> /* ToDo: status decoding and firing up of VSYNC and page flip events */
>
> + if (status & ~se->vsync_irq & se->irq_mask)
> + trace_printk("error occurred SE_STATUS is 0x%08X\n", status);
Your diff here doesn't match what I have in my tree. What version are
you using?
Anyway, you can see that the status is already masked before the ToDo
comment, so I guess you can trim the condition a bit.
Best regards,
Liviu
> +
> malidp_hw_clear_irq(hwdev, MALIDP_SE_BLOCK, status);
>
> return IRQ_HANDLED;
> --
> 2.7.4
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
