Re: [PATCH 5/7] drm/i915: queue hangcheck on reset

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 03, 2013 at 05:22:10PM +0300, Mika Kuoppala wrote:
> From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> 
> Upon resetting the GPU, we begin processing batches once more, so
> reset the hangcheck timer.
> 
> v2: kicking inside reset instead of hangcheck_elapsed and
>     sane commit message by Chris Wilson
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index b0fec7f..1b0e903 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1452,6 +1452,8 @@ static void i915_error_work_func(struct work_struct *work)
>  
>  			kobject_uevent_env(&dev->primary->kdev.kobj,
>  					   KOBJ_CHANGE, reset_done_event);
> +
> +			i915_queue_hangcheck(dev);

Hm, what exactly is this for? After reset we don't have any batches
running right now (since we reset all batches), so I don't understand why
we need this. And the commit message also doesn't give a reason.

Aside a little maintainer bikeshed: The important thing a commit message
should cover isn't really the "what changed" since the code should be
readable enough to not need additional explanation, but the "why". In some
cases the important part of the "why" is "why not a different approach" or
"why does this work at all" (e.g. for bugfixes).

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch