On Mon, Jun 08, 2015 at 06:03:24PM +0100, Tomas Elf wrote:
> Now that i915_wait_request takes per-engine hang recovery into account it is
> more likely to fail and return -EAGAIN or -EIO due to hung engines (unlike
> before when it would only fail if a full GPU reset was imminent). What this
> means is that the display driver might see more frequent failures that are 
> only
> a consequence of ongoing hang recoveries. Therefore, let's not spew a lot of
> warnings in the kernel log every time a flip fails due to an ongoing hang
> recovery, since a) This is to be expected during hang recovery and b) It
> severely degrades performance and makes the hang recovery take even longer to
> complete, which ultimately might cause the userland window compositor to fail
> because the flip is taking too long to complete and it simply gives up, 
> leaving
> the screen in a frozen state.
> 
> Signed-off-by: Tomas Elf <tomas....@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 97922fb..128c58c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10356,9 +10356,21 @@ static void intel_mmio_flip_work_func(struct 
> work_struct *work)
>  
>       mmio_flip = &crtc->mmio_flip;
>       if (mmio_flip->req)
> -             WARN_ON(__i915_wait_request(mmio_flip->req,
> +     {
> +             int ret = __i915_wait_request(mmio_flip->req,
>                                           crtc->reset_counter,
> -                                         false, NULL, NULL) != 0);
> +                                         false, NULL, NULL);
> +
> +             /*
> +              * If a hang has been detected then we expect
> +              * __i915_wait_request to fail since it's probably going to be
> +              * forced to give up the struct_mutex and try to grab it again
> +              * once the TDR is done. Don't produce a warning in that case!
> +              */
> +             if (ret)
> +                     
> WARN_ON(!i915_gem_check_wedge(crtc->base.dev->dev_private,
> +                                     NULL, true));

Now this is plain wrong and should have an alert that your proposed
changes to __i915_wait_request was wrong.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to