[Intel-gfx] [PATCH 1/2] drm/i915: Don't require primary->fb in intel_crtc_active()

[Daniel Vetter]

On Tue, Mar 03, 2015 at 06:15:12PM -0800, Matt Roper wrote:
> Universal planes allow us to have an active CRTC without a primary plane
> framebuffer bound.  Drop the test for primary->fb from
> intel_crtc_active() since we can clearly have active CRTC's without a
> framebuffer, and this check is now interfering with watermark
> calculations when we try to re-enable the primary plane.
> 
> Note that commit
> 
>         commit 0fda65680e92545caea5be7805a7f0a617fb6c20
>         Author: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>         Date:   Fri Feb 27 15:12:35 2015 +0000
> 
>             drm/i915/skl: Update watermarks for Y tiling
> 
> adds a test for primary plane enable/disable to trigger a watermark
> update (previously we ignored updates to primary planes, which wasn't
> really correct, but we got lucky since we always pretended the primary
> plane was on).  Tvrtko's patch tries to update watermarks when we
> re-enable the primary plane, but that watermark computation gets aborted
> early because intel_crtc_active() always returns false when the primary
> plane is disabled; this leads to watermark underruns (at least on
> platforms with ILK-style watermark code).

That pretty much means the igt test coverage isn't exhaustive enough and
doesn't properly check that the right stuff shows up on the screens with
CRCs.

Tvrtko can you please add relevant tests for this scenario?

Thanks, Daniel

> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 589addf..92cb2ff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -893,11 +893,8 @@ bool intel_crtc_active(struct drm_crtc *crtc)
>  	 *
>  	 * We can ditch the adjusted_mode.crtc_clock check as soon
>  	 * as Haswell has gained clock readout/fastboot support.
> -	 *
> -	 * We can ditch the crtc->primary->fb check as soon as we can
> -	 * properly reconstruct framebuffers.
>  	 */
> -	return intel_crtc->active && crtc->primary->fb &&
> +	return intel_crtc->active &&
>  		intel_crtc->config->base.adjusted_mode.crtc_clock;
>  }
>  
> -- 
> 1.8.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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