Re: [PATCH] drm/i915: add missing condition for committing planes on crtc

Ville Syrjälä
2016-04-18T11:28:16Z

On Fri, Apr 08, 2016 at 05:30:40PM +0100, Lionel Landwerlin wrote:
> We are currently missing the color management update condition to
> commit planes on crtc.

On a related note, could you move the LUT stuff out from the critical
section? It has no business being there since the registers aren't
double buffered. I'm thinking this is the cause for the sporadic
atomic update fails that I've seen.

> Fixes: 20a34e78f0d7 (drm/i915: Update color management during vblank evasion.)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index feb7028..670b2ad 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13480,6 +13480,13 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
> return false;
> }
>
> +static bool needs_commit_planes_on_crtc(struct drm_crtc_state *crtc_state)
> +{
> + return (crtc_state->planes_changed ||
> + crtc_state->color_mgmt_changed ||
> + to_intel_crtc_state(crtc_state)->update_pipe);
> +}
> +
> /**
> * intel_atomic_commit - commit validated state object
> * @dev: DRM device
> @@ -13583,7 +13590,6 @@ static int intel_atomic_commit(struct drm_device *dev,
> bool modeset = needs_modeset(crtc->state);
> struct intel_crtc_state *pipe_config =
> to_intel_crtc_state(crtc->state);
> - bool update_pipe = !modeset && pipe_config->update_pipe;
>
> if (modeset && crtc->state->active) {
> update_scanline_offset(to_intel_crtc(crtc));
> @@ -13597,8 +13603,8 @@ static int intel_atomic_commit(struct drm_device *dev,
> drm_atomic_get_existing_plane_state(state, crtc->primary))
> intel_fbc_enable(intel_crtc);
>
> - if (crtc->state->active &&
> - (crtc->state->planes_changed || update_pipe))
> + if (crtc->state->active && !modeset &&
> + needs_commit_planes_on_crtc(crtc->state))
> drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
>
> if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> --
> 2.8.0.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC