| Message ID | 1449673301-6449-2-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 09, 2015 at 05:01:39PM +0200, Mika Kuoppala wrote: > Currently interrupt code is the only place checking > for the unclaimed register access prior to actual register > macros using the same functionality. Rename the function > and make it return bool so that the possible error message > context is clear in the caller side. The motivation is to allow > usage of unclaimed detection on arbitrary places. > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_irq.c | 3 ++- > drivers/gpu/drm/i915/intel_uncore.c | 5 ++--- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2e8c0cbc..30c1aeb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2725,7 +2725,7 @@ extern void intel_uncore_sanitize(struct drm_device *dev); > extern void intel_uncore_early_sanitize(struct drm_device *dev, > bool restore_forcewake); > extern void intel_uncore_init(struct drm_device *dev); > -extern void intel_uncore_check_errors(struct drm_device *dev); > +extern bool intel_uncore_unclaimed_access(struct drm_device *dev); > extern void intel_uncore_fini(struct drm_device *dev); > extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore); > const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id); > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index e88d692..1be3e11 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2167,7 +2167,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg) > > /* We get interrupts on unclaimed registers, so check for this before we > * do any I915_{READ,WRITE}. */ > - intel_uncore_check_errors(dev); > + if (intel_uncore_unclaimed_access(dev)) > + DRM_ERROR("Unclaimed register before interrupt\n"); This reminds me that having this error message is spectacularly unhelpful. Could I persuade you to adopt http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=breadcrumbs&id=357b468e6555cd8a59858aaee1d408f846267896 it's been on the list a few times. With your new framework it may make sense to move it about. For instance, doing the check every interrupt is not great. > /* disable master interrupt before clearing iir */ > de_ier = I915_READ(DEIER); > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 9d672f7..96b04c7 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -1602,8 +1602,7 @@ bool intel_has_gpu_reset(struct drm_device *dev) > return intel_get_gpu_reset(dev) != NULL; > } > > -void intel_uncore_check_errors(struct drm_device *dev) > +bool intel_uncore_unclaimed_access(struct drm_device *dev) > { > - if (unclaimed_reg_access(to_i915(dev))) > - DRM_ERROR("Unclaimed register before interrupt\n"); > + return unclaimed_reg_access(to_i915(dev)); > } Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2e8c0cbc..30c1aeb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2725,7 +2725,7 @@ extern void intel_uncore_sanitize(struct drm_device *dev); extern void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake); extern void intel_uncore_init(struct drm_device *dev); -extern void intel_uncore_check_errors(struct drm_device *dev); +extern bool intel_uncore_unclaimed_access(struct drm_device *dev); extern void intel_uncore_fini(struct drm_device *dev); extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore); const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e88d692..1be3e11 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2167,7 +2167,8 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg) /* We get interrupts on unclaimed registers, so check for this before we * do any I915_{READ,WRITE}. */ - intel_uncore_check_errors(dev); + if (intel_uncore_unclaimed_access(dev)) + DRM_ERROR("Unclaimed register before interrupt\n"); /* disable master interrupt before clearing iir */ de_ier = I915_READ(DEIER); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 9d672f7..96b04c7 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1602,8 +1602,7 @@ bool intel_has_gpu_reset(struct drm_device *dev) return intel_get_gpu_reset(dev) != NULL; } -void intel_uncore_check_errors(struct drm_device *dev) +bool intel_uncore_unclaimed_access(struct drm_device *dev) { - if (unclaimed_reg_access(to_i915(dev))) - DRM_ERROR("Unclaimed register before interrupt\n"); + return unclaimed_reg_access(to_i915(dev)); }
Currently interrupt code is the only place checking for the unclaimed register access prior to actual register macros using the same functionality. Rename the function and make it return bool so that the possible error message context is clear in the caller side. The motivation is to allow usage of unclaimed detection on arbitrary places. Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_irq.c | 3 ++- drivers/gpu/drm/i915/intel_uncore.c | 5 ++--- 3 files changed, 5 insertions(+), 5 deletions(-)