Re: [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled

[Matt Roper <matthew.d.roper@intel.com>]

On Thu, May 07, 2015 at 02:19:07PM -0700, Matt Roper wrote:
> With the recent modeset internal rework, we wind up setting crtc_state->enable
> to false, but leave crtc_state->active as true following a
> drmModeSetCrtc(fb=0), which is incorrect.  This mismatch gets caught by
> drm_atomic_crtc_check() and causes subsequent atomic operations (such as plane
> updates while the CRTC is disabled) to fail.
> 
> Bisect points to
> 
>         commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
>         Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>         Date:   Tue Apr 21 17:13:19 2015 +0300
> 
>             drm/i915: Use atomic helpers for computing changed flags
> 
> as the commit that actually triggers the regression.
> 
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Testcase: igt/kms_universal_plane
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> My original expectation was that we'd need to be updating the in-flight CRTC
> state rather than the committed state as I wound up doing here.  However it
> looks like our legacy modesets aren't yet converted to the point where they do
> state swaps, hence the need to update crtc->state directly.

Actually, it looks like I screwed up my working tree and lost the end of
Ander's patch series.  We actually *do* have proper CRTC state swapping
now (when his whole series is present), so updating in-flight state is
the right way to go; I'll send a v2 shortly that fixes that.


Matt

> 
> Ander/Maarten, does this look like the right place to have 'active' updated for
> the time being, or should it be moved elsewhere in the modeset pipeline?
> 
>  drivers/gpu/drm/i915/intel_display.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c297cdc..be166cf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6128,6 +6128,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
>  
>  	drm_plane_helper_disable(crtc->primary);
>  
> +	crtc->state->active = false;
> +
>  	/* Update computed state. */
>  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>  		if (!connector->encoder || !connector->encoder->crtc)
> -- 
> 1.8.5.1
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx