Re: [PATCH v2 00/12] drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.

From: Sean Paul <seanpaul@chromium.org>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 00/12] drm/atomic: Make drm_atomic_helper_swap_state waiting interruptible.
Date: Fri, 14 Jul 2017 11:01:28 -0400	[thread overview]
Message-ID: <20170714150128.so3evlq3wv2ju5od@art_vandelay> (raw)
In-Reply-To: <20170711143314.2148-1-maarten.lankhorst@linux.intel.com>

On Tue, Jul 11, 2017 at 04:33:02PM +0200, Maarten Lankhorst wrote:
> drm_atomic_helper_swap_state could previously never fail, in order to still
> continue it would set a limit of 10s on waiting for previous updates to complete,
> then it moved forward. This can be very bad when ignoring previous updates,
> because the hw state and sw state may get out of sync when for example a modeset
> is ignored.
> 
> By converting the swap_state to interruptible and handling the error in each driver,
> we fix this issue because if a update takes forever it can be aborted through signals.
> 
> Changes since first version:
> - Split out driver conversions.
> - Fix nouveau error handling first.
> 
> Maarten Lankhorst (12):
>   drm/nouveau: Fix error handling in nv50_disp_atomic_commit
>   drm/atomic: Change drm_atomic_helper_swap_state to return an error.
>   drm/nouveau: Handle drm_atomic_helper_swap_state failure
>   drm/atmel-hlcdc: Handle drm_atomic_helper_swap_state failure
>   drm/i915: Handle drm_atomic_helper_swap_state failure
>   drm/mediatek: Handle drm_atomic_helper_swap_state failure
>   drm/msm: Handle drm_atomic_helper_swap_state failure
>   drm/tegra: Handle drm_atomic_helper_swap_state failure
>   drm/tilcdc: Handle drm_atomic_helper_swap_state failure
>   drm/vc4: Handle drm_atomic_helper_swap_state failure
>   drm/atomic: Add __must_check to drm_atomic_helper_swap_state.
>   drm/atomic: Allow drm_atomic_helper_swap_state to fail

Hi Maarten,
The set looks good to me. The core changes make sense and seem like a good step
forward, and the driver changes seem like they do the right thing.

Normally, I'd suggest waiting a week or so for maintainer acks to trickle in,
but since this is fixing behavior in i915, it makes sense to expedite it.
Perhaps ping individuals on IRC?

For the whole set,
Reviewed-by: Sean Paul <seanpaul@chromium.org>

Sean

> 
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 13 ++++++-----
>  drivers/gpu/drm/drm_atomic_helper.c          | 35 +++++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_display.c         | 10 +++++++-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c       |  7 +++++-
>  drivers/gpu/drm/msm/msm_atomic.c             | 16 ++++++-------
>  drivers/gpu/drm/nouveau/nv50_display.c       | 12 +++++++---
>  drivers/gpu/drm/tegra/drm.c                  |  7 +++++-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c          |  6 ++++-
>  drivers/gpu/drm/vc4/vc4_kms.c                |  2 +-
>  include/drm/drm_atomic_helper.h              |  4 ++--
>  10 files changed, 75 insertions(+), 37 deletions(-)
> 
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx