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@xxxxxxxxx> > 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). I think this will make a bunch of the old platforms blow up. Well, I think they might already blow up since they now look at crtc->state->fb based on the result of intel_crtc_active(). So I believe more fixes are needed all over the wm code at least. In fact looking at the code, I think most (or all?) of the call sites in the in the ilk+ wm code should just be using crtc->active. So for some extra safety it might make sense to do leave intel_crtc_active() alone for now, or in fact we should maybe change it to also look at primary->state->fb instead. That should more or less keep the old platforms in the same state as they were before, while letting new platforms handle each plane properly. > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
