| Message ID | 1461702945-14185-10-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26/04/16 23:35, Laurent Pinchart wrote: > Instead of going through a complicated registration mechanism, just > call the OCP error IRQ handler directly from the main IRQ handler. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/omap_drv.h | 1 - > drivers/gpu/drm/omapdrm/omap_irq.c | 29 +++++++++++------------------ > 2 files changed, 11 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h > index 2d8fbdb2d39f..17dd3b98fc1a 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.h > +++ b/drivers/gpu/drm/omapdrm/omap_drv.h > @@ -104,7 +104,6 @@ struct omap_drm_private { > /* irq handling: */ > struct list_head irq_list; /* list of omap_drm_irq */ > uint32_t irq_mask; /* enabled irqs in addition to irq_list */ > - struct omap_drm_irq error_handler; > > /* atomic commit */ > struct { > diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c > index a90e093f5f42..499da6e2c5a4 100644 > --- a/drivers/gpu/drm/omapdrm/omap_irq.c > +++ b/drivers/gpu/drm/omapdrm/omap_irq.c > @@ -21,12 +21,6 @@ > > static DEFINE_SPINLOCK(list_lock); > > -static void omap_irq_error_handler(struct omap_drm_irq *irq, > - uint32_t irqstatus) > -{ > - DRM_ERROR("errors: %08x\n", irqstatus); > -} > - > /* call with list_lock and dispc runtime held */ > static void omap_irq_update(struct drm_device *dev) > { > @@ -219,6 +213,14 @@ static void omap_irq_fifo_underflow(uint32_t irqstatus) > pr_cont("(0x%08x)\n", irqstatus); > } > > +static void omap_irq_error_handler(uint32_t irqstatus) I think the function should mention "ocp_error". > +{ > + if (!(irqstatus & DISPC_IRQ_OCP_ERR)) > + return; > + > + DRM_ERROR("errors: %08x\n", irqstatus); Now we have a separate print for OCP error, so we could instead of printing hex numbers, print "OCP error". > +} > + > static irqreturn_t omap_irq_handler(int irq, void *arg) > { > struct drm_device *dev = (struct drm_device *) arg; > @@ -245,6 +247,7 @@ static irqreturn_t omap_irq_handler(int irq, void *arg) > omap_crtc_error_irq(crtc, irqstatus); > } > > + omap_irq_error_handler(irqstatus); > omap_irq_fifo_underflow(irqstatus); > > spin_lock_irqsave(&list_lock, flags); > @@ -270,14 +273,14 @@ static irqreturn_t omap_irq_handler(int irq, void *arg) > int omap_drm_irq_install(struct drm_device *dev) > { > struct omap_drm_private *priv = dev->dev_private; > - struct omap_drm_irq *error_handler = &priv->error_handler; > unsigned int num_mgrs = dss_feat_get_num_mgrs(); > unsigned int i; > int ret; > > INIT_LIST_HEAD(&priv->irq_list); > > - priv->irq_mask = DISPC_IRQ_GFX_FIFO_UNDERFLOW > + priv->irq_mask = DISPC_IRQ_OCP_ERR > + | DISPC_IRQ_GFX_FIFO_UNDERFLOW > | DISPC_IRQ_VID1_FIFO_UNDERFLOW > | DISPC_IRQ_VID2_FIFO_UNDERFLOW > | DISPC_IRQ_VID3_FIFO_UNDERFLOW; > @@ -293,16 +296,6 @@ int omap_drm_irq_install(struct drm_device *dev) > if (ret < 0) > return ret; > > - error_handler->irq = omap_irq_error_handler; > - error_handler->irqmask = DISPC_IRQ_OCP_ERR; > - > - /* for now ignore DISPC_IRQ_SYNC_LOST_DIGIT.. really I think > - * we just need to ignore it while enabling tv-out > - */ > - error_handler->irqmask &= ~DISPC_IRQ_SYNC_LOST_DIGIT; > - > - omap_irq_register(dev, error_handler); This makes me wonder is the previous patch correct... It doesn't ignore the SYNC_LOST_DIGIT. Oh, but is this ignore only for the error handler that only prints. Ah, confusing =). Tomi
Hi Tomi, On Tuesday 10 May 2016 16:10:59 Tomi Valkeinen wrote: > On 26/04/16 23:35, Laurent Pinchart wrote: > > Instead of going through a complicated registration mechanism, just > > call the OCP error IRQ handler directly from the main IRQ handler. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > > > drivers/gpu/drm/omapdrm/omap_drv.h | 1 - > > drivers/gpu/drm/omapdrm/omap_irq.c | 29 +++++++++++------------------ > > 2 files changed, 11 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h > > b/drivers/gpu/drm/omapdrm/omap_drv.h index 2d8fbdb2d39f..17dd3b98fc1a > > 100644 > > --- a/drivers/gpu/drm/omapdrm/omap_drv.h > > +++ b/drivers/gpu/drm/omapdrm/omap_drv.h [snip] > > diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c > > b/drivers/gpu/drm/omapdrm/omap_irq.c index a90e093f5f42..499da6e2c5a4 > > 100644 > > --- a/drivers/gpu/drm/omapdrm/omap_irq.c > > +++ b/drivers/gpu/drm/omapdrm/omap_irq.c [snip] > > @@ -219,6 +213,14 @@ static void omap_irq_fifo_underflow(uint32_t > > irqstatus)> > > pr_cont("(0x%08x)\n", irqstatus); > > } > > > > +static void omap_irq_error_handler(uint32_t irqstatus) > > I think the function should mention "ocp_error". > > > +{ > > + if (!(irqstatus & DISPC_IRQ_OCP_ERR)) > > + return; > > + > > + DRM_ERROR("errors: %08x\n", irqstatus); > > Now we have a separate print for OCP error, so we could instead of > printing hex numbers, print "OCP error". I'll fix those two issues in v2. > > +} [snip] > > @@ -293,16 +296,6 @@ int omap_drm_irq_install(struct drm_device *dev) > > > > if (ret < 0) > > > > return ret; > > > > - error_handler->irq = omap_irq_error_handler; > > - error_handler->irqmask = DISPC_IRQ_OCP_ERR; > > - > > - /* for now ignore DISPC_IRQ_SYNC_LOST_DIGIT.. really I think > > - * we just need to ignore it while enabling tv-out > > - */ > > - error_handler->irqmask &= ~DISPC_IRQ_SYNC_LOST_DIGIT; > > - > > - omap_irq_register(dev, error_handler); > > This makes me wonder is the previous patch correct... It doesn't ignore > the SYNC_LOST_DIGIT. Oh, but is this ignore only for the error handler > that only prints. Ah, confusing =). The good news is that this patch removes the confusing code :-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h index 2d8fbdb2d39f..17dd3b98fc1a 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.h +++ b/drivers/gpu/drm/omapdrm/omap_drv.h @@ -104,7 +104,6 @@ struct omap_drm_private { /* irq handling: */ struct list_head irq_list; /* list of omap_drm_irq */ uint32_t irq_mask; /* enabled irqs in addition to irq_list */ - struct omap_drm_irq error_handler; /* atomic commit */ struct { diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c index a90e093f5f42..499da6e2c5a4 100644 --- a/drivers/gpu/drm/omapdrm/omap_irq.c +++ b/drivers/gpu/drm/omapdrm/omap_irq.c @@ -21,12 +21,6 @@ static DEFINE_SPINLOCK(list_lock); -static void omap_irq_error_handler(struct omap_drm_irq *irq, - uint32_t irqstatus) -{ - DRM_ERROR("errors: %08x\n", irqstatus); -} - /* call with list_lock and dispc runtime held */ static void omap_irq_update(struct drm_device *dev) { @@ -219,6 +213,14 @@ static void omap_irq_fifo_underflow(uint32_t irqstatus) pr_cont("(0x%08x)\n", irqstatus); } +static void omap_irq_error_handler(uint32_t irqstatus) +{ + if (!(irqstatus & DISPC_IRQ_OCP_ERR)) + return; + + DRM_ERROR("errors: %08x\n", irqstatus); +} + static irqreturn_t omap_irq_handler(int irq, void *arg) { struct drm_device *dev = (struct drm_device *) arg; @@ -245,6 +247,7 @@ static irqreturn_t omap_irq_handler(int irq, void *arg) omap_crtc_error_irq(crtc, irqstatus); } + omap_irq_error_handler(irqstatus); omap_irq_fifo_underflow(irqstatus); spin_lock_irqsave(&list_lock, flags); @@ -270,14 +273,14 @@ static irqreturn_t omap_irq_handler(int irq, void *arg) int omap_drm_irq_install(struct drm_device *dev) { struct omap_drm_private *priv = dev->dev_private; - struct omap_drm_irq *error_handler = &priv->error_handler; unsigned int num_mgrs = dss_feat_get_num_mgrs(); unsigned int i; int ret; INIT_LIST_HEAD(&priv->irq_list); - priv->irq_mask = DISPC_IRQ_GFX_FIFO_UNDERFLOW + priv->irq_mask = DISPC_IRQ_OCP_ERR + | DISPC_IRQ_GFX_FIFO_UNDERFLOW | DISPC_IRQ_VID1_FIFO_UNDERFLOW | DISPC_IRQ_VID2_FIFO_UNDERFLOW | DISPC_IRQ_VID3_FIFO_UNDERFLOW; @@ -293,16 +296,6 @@ int omap_drm_irq_install(struct drm_device *dev) if (ret < 0) return ret; - error_handler->irq = omap_irq_error_handler; - error_handler->irqmask = DISPC_IRQ_OCP_ERR; - - /* for now ignore DISPC_IRQ_SYNC_LOST_DIGIT.. really I think - * we just need to ignore it while enabling tv-out - */ - error_handler->irqmask &= ~DISPC_IRQ_SYNC_LOST_DIGIT; - - omap_irq_register(dev, error_handler); - dev->irq_enabled = true; return 0;
Instead of going through a complicated registration mechanism, just call the OCP error IRQ handler directly from the main IRQ handler. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/omapdrm/omap_drv.h | 1 - drivers/gpu/drm/omapdrm/omap_irq.c | 29 +++++++++++------------------ 2 files changed, 11 insertions(+), 19 deletions(-)