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....@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.kuopp...@intel.com>
> >>Signed-off-by: Tomas Elf <tomas....@intel.com>
> >>Signed-off-by: Arun Siluvery <arun.siluv...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
