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

[Josh Boyer]

On Wed, May 20, 2015 at 9:53 AM, Jani Nikula <jani.nikula at intel.com> 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.
>
>>
>>     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.

I would guess 4.0.y for now.  Both Fedora 21 and 22 are using that
stable release, and it is most noticeable on Fedora 22 where we use
Wayland for the GDM session.

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

I've pointed a few people that have machines that hit this issue at
this patch and they're now CC'd.  Hopefully we can get some tests in
this week.

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

Thanks for the Ack.  Hopefully Greg doesn't immediately <formletter>
this one :).

josh

>> 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
>>
>
> --
> Jani Nikula, Intel Open Source Technology Center
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html