[Intel-gfx] [PATCH] drm/i915: Fix ilk watermarks calculation when primary plane is disabled

[Ander Conselvan De Oliveira]

On Wed, 2015-05-20 at 16:53 +0300, Jani Nikula wrote:
> On Wed, 20 May 2015, Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com> wrote:
> > On Fedora 21 or 22, when the transition from the X server to the wayland
> > compositor is done, the CRTC with the login screen is left active with a
> > disabled fb. A cursor ioctl after the transition causes the watermarks
> > to be updated, but due to the logic in intel_crtc_active() checking for
> > the primary plane fb, the update considers all planes to be disabled,
> > untimately setting the wrong watermark values and causing screen
> > flicker. Since the crtc is active, a modeset done later is skipped and
> > replaced with a flip, which doesn't update the watermarks.
> >
> > This regression was introduced somewhere between v3.16 and v3.17.
> > Another issue prevented me from doing a full bisect, but the issue was
> > introduced in one of the following skipped commits:
> >
> >     commit 7707e6535f43328e05e4729ac96eee864b90e8a4
> >     Author: Rob Clark <robdclark at gmail.com>
> >     Date:   Thu Jul 17 23:30:04 2014 -0400
> >
> >         drm/i915: use helpers
> >
> >     commit ca5a1b9ba0fb5291b555a23b76dbe5f6c30bfd7a
> >     Merge: c7dbc6c 3488229
> >     Author: Dave Airlie <airlied at redhat.com>
> >     Date:   Wed Jul 9 10:38:42 2014 +1000
> >
> >         Merge tag 'drm-intel-next-2014-06-20' of git://anongit.freedesktop.org/drm-intel into drm-next
> >
> >     commit c51f71679042a5f388d9580ffbede14c897f1e86
> >     Merge: b957f45 7b3c29f
> >     Author: Dave Airlie <airlied at redhat.com>
> >     Date:   Sat Jul 19 16:43:41 2014 +1000
> >
> >         Merge tag 'drm-intel-next-2014-07-11' of git://anongit.freedesktop.org/drm-intel into drm-next
> >
> > This patch is a simplified version of the following commits:
> >
> >     commit 3dd512fbda0d87d1c3fb44bf878b262baee98fb6
> >     Author: Matt Roper <matthew.d.roper at intel.com>
> >     Date:   Fri Feb 27 10:12:00 2015 -0800
> >
> >         drm/i915: Kill intel_crtc->cursor_{width, height} (v2)
> >
> >     commit 54da691deb123c045259ebf4f5c67381244f58f1
> >     Author: Thomas Gummerer <t.gummerer at gmail.com>
> >     Date:   Thu May 14 09:16:39 2015 +0200
> >
> >         drm/i915: fix screen flickering
> 
> This is expected to land in v4.1-rc5, i.e. these are all upstream
> commits. And I assume none of them apply to stable kernels directly.

A lot of cherry-pick and some amending would be necessary, because of
the changes going on for the atomic conversion. Hence the approach here
was to capture just the important changes into this small patch.

> >
> >     commit 3ef00284e6a48f7deb0784ccca0478ebb7d4bcfc
> >     Author: Matt Roper <matthew.d.roper at intel.com>
> >     Date:   Mon Mar 9 10:19:24 2015 -0700
> >
> >         drm/i915: Use crtc->state->active in ilk/skl watermark calculations (v3)
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90508
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1218688
> > Cc: stable at vger.kernel.org
> 
> I (and most likely the stable team) would like to know which stable
> kernel versions this is targeting.

This is for 4.0.

> Do you have Tested-bys against the stable versions you're targeting?

Only my own testing so far. I guess Ray Strode's doesn't count since he
tested on top of Fedora's kernel?


> Anyway this is
> 
> Acked-by: Jani Nikula <jani.nikula at intel.com>

Thanks,
Ander

> 
> 
> BR,
> Jani.
> 
> > Cc: Dave Airlie <airlied at gmail.com>
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 24d77dd..3d67f8e 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1898,16 +1898,24 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
> >  	enum pipe pipe = intel_crtc->pipe;
> >  	struct drm_plane *plane;
> >  
> > -	if (!intel_crtc_active(crtc))
> > +	if (!intel_crtc->active)
> >  		return;
> >  
> >  	p->active = true;
> >  	p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> >  	p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
> > -	p->pri.bytes_per_pixel = crtc->primary->fb->bits_per_pixel / 8;
> > +
> > +
> > +	if (crtc->primary->fb)
> > +		p->pri.bytes_per_pixel = crtc->primary->fb->bits_per_pixel / 8;
> > +	else
> > +		p->pri.bytes_per_pixel = 4;
> > +
> >  	p->cur.bytes_per_pixel = 4;
> > +
> >  	p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
> > -	p->cur.horiz_pixels = intel_crtc->cursor_width;
> > +	p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
> > +
> >  	/* TODO: for now, assume primary and cursor planes are always enabled. */
> >  	p->pri.enabled = true;
> >  	p->cur.enabled = true;
> > -- 
> > 2.1.0
> >
>