Re: [PATCH] drm/komeda: Adds error event print functionality

[Sean Paul]

On Thu, Jul 18, 2019 at 02:17:37PM +0100, Liviu Dudau wrote:
> On Thu, Jun 27, 2019 at 04:10:36AM +0100, Lowry Li (Arm Technology China) wrote:

/snip

> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > index 647bce5..1462bac 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > @@ -47,6 +47,8 @@ static irqreturn_t komeda_kms_irq_handler(int irq, void *data)
> >  	memset(&evts, 0, sizeof(evts));
> >  	status = mdev->funcs->irq_handler(mdev, &evts);
> >  
> > +	komeda_print_events(&evts);
> 
> Calling this function from the IRQ handler is a bad idea. We should use debugfs
> if you really want to have a trace of the events, but I personally don't see
> value in having this functionality in the kernel at all. You can expose the
> value of the evts->global and evts->pipes[] as integers and decode that in
> userspace or as a debugfs entry.

Alternatively, consider using kernel trace events. They allow you to selectively
turn on/off certain events and also allow you to customize which data is
recorded and how it's formatted. Seems like a good fit from the quick scan I've
done.

Sean

> 
> Best regards,
> Liviu
> 
> > +
> >  	/* Notify the crtc to handle the events */
> >  	for (i = 0; i < kms->n_crtcs; i++)
> >  		komeda_crtc_handle_event(&kms->crtcs[i], &evts);
> > -- 
> > 1.9.1
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     Â\_(ã)_/Â

-- 
Sean Paul, Software Engineer, Google / Chromium OS