From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Paul Subject: Re: [PATCH] drm/komeda: Adds error event print functionality Date: Thu, 18 Jul 2019 11:23:50 -0400 Message-ID: <20190718152350.GC31819@art_vandelay> References: <1561604994-26925-1-git-send-email-lowry.li@arm.com> <20190718131737.GD5942@e110455-lin.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Content-Disposition: inline In-Reply-To: <20190718131737.GD5942@e110455-lin.cambridge.arm.com> Sender: linux-kernel-owner@vger.kernel.org To: Liviu Dudau Cc: "Lowry Li (Arm Technology China)" , "james qian wang (Arm Technology China)" , "maarten.lankhorst@linux.intel.com" , "seanpaul@chromium.org" , "airlied@linux.ie" , Brian Starkey , "Julien Yin (Arm Technology China)" , "Jonathan Chai (Arm Technology China)" , Ayan Halder , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , nd List-Id: dri-devel@lists.freedesktop.org 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