[Intel-gfx] [PATCH 1/3] drm/i915/tdr: Initialize hangcheck struct for each engine

[Chris Wilson]

On Fri, Mar 18, 2016 at 09:22:55PM +0000, Arun Siluvery wrote:
> On 18/03/2016 20:48, Chris Wilson wrote:
> >On Fri, Mar 18, 2016 at 08:07:54PM +0000, Arun Siluvery wrote:
> >>From: Tomas Elf <tomas.elf at intel.com>
> >>
> >>Initialize hangcheck struct during driver load. Since we do the same after
> >>recovering from a reset, this is extracted into a helper function.
> >>
> >>Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> >>Signed-off-by: Tomas Elf <tomas.elf at intel.com>
> >>Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_dma.c         | 12 ++++++++++++
> >>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
> >>  drivers/gpu/drm/i915/intel_lrc.c        |  2 +-
> >>  drivers/gpu/drm/i915/intel_ringbuffer.c |  7 ++++++-
> >>  4 files changed, 20 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> >>index 3f439a0..c5d1673 100644
> >>--- a/drivers/gpu/drm/i915/i915_dma.c
> >>+++ b/drivers/gpu/drm/i915/i915_dma.c
> >>@@ -945,6 +945,16 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv)
> >>  	}
> >>  }
> >>
> >>+static void i915_hangcheck_init(struct drm_device *dev)
> >>+{
> >>+	int i;
> >>+	struct intel_engine_cs *engine;
> >>+	struct drm_i915_private *dev_priv = dev->dev_private;
> >>+
> >>+	for_each_engine(engine, dev_priv, i)
> >>+		intel_engine_init_hangcheck(engine);
> >>+}
> >>+
> >>  static int i915_workqueues_init(struct drm_i915_private *dev_priv)
> >>  {
> >>  	/*
> >>@@ -1233,6 +1243,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> >>
> >>  	i915_gem_load_init_fences(dev_priv);
> >>
> >>+	i915_hangcheck_init(dev);
> >
> >This is tautological. If we are clearing the per-engine hangcheck in
> >ring->init_hw() then we will do later anyway. So why now?
> 
> It was doing slightly more earlier but now it is not required. The
> helper function intel_engine_init_hangcheck() also doesn't add much
> value so this patch can be ignored.

The helper function is ok, it adds a little bit of documentation which
is far more meaningful than memset(). I was just wondering if there was
an actual requirement I missed for i915_driver_init_hw() to do so, or if
there will be.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre