intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Matt Roper <matthew.d.roper@intel.com>
To: Jan Niehusmann <jan@gondor.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: Flicker caused by "drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)"
Date: Tue, 5 Jan 2016 13:58:19 -0800	[thread overview]
Message-ID: <20160105215819.GA21100@intel.com> (raw)
In-Reply-To: <20160105124932.GA3628@x61s.reliablesolutions.de>

On Tue, Jan 05, 2016 at 01:49:33PM +0100, Jan Niehusmann wrote:
> Hi,
> 
> On Thu, Sep 24, 2015 at 03:53:07PM -0700, Matt Roper wrote:
> > Just pull the info out of the plane state structure rather than staging
> > it in an additional structure.
> > 
> > v2: Add 'visible' condition to sprites_scaled so that we don't limit the
> >     WM level when the sprite isn't enabled.  (Ville)
> 
> It looks like this patch - specifically the visible condition in
> ilk_compute_cur_wm - causes screen flicker when moving the cursor from
> one screen to another one in a multi-screen setup.
> 
> My hardware is a Thinkpad x201s
> (http://www.thinkwiki.org/wiki/Category:X201s)
> 00:02.0 VGA compatible controller [0300]: Intel Corporation Core Processor Integrated Graphics Controller [8086:0046] (rev 02),
> CPU is an i7-620LM.
> 
> The screen flickering is the one the coursor is leaving. In most cases,
> parts of the screen just blank for a very short moment. In some rare
> cases, the screen even stays blanked until I move the cursor back to
> that screen. No stability issues were observed, this seems to be a
> purely cosmetic issue.
> 
> I bisected the issue to 43d59eda1f69631c267e06ab6b94ed3c14f1f6d1. As I
> knew the flickering was cursor-related, I tried the following minimal
> patch, which actually fixed the symptom, when applied to 4.4-rc8:
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f091ad1..1ef0c54 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1791,7 +1791,7 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
>  {
>  	int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
>  
> -	if (!cstate->base.active || !pstate->visible)
> +	if (!cstate->base.active)
>  		return 0;
>  
>  	return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
> 
> 
> I have no idea at all if this is actually fixing anything or if it's
> just hiding the real bug. All I can say is that the flickering doesn't
> occur any longer.
> 
> Jan

Hi Jan.  I think the flicker you're seeing is caused by our current lack
of two-stage watermark updates on platforms that use ILK-style
watermarks.  I have a patch series at
http://patchwork.freedesktop.org/bundle/mattrope/atomic_wm_ilk/ that's
supposed to address this, but it's still awaiting review at the moment
so it isn't yet available upstream.  Now that folks are coming back from
holiday vacations and such, hopefully we'll get the review finished soon
and be able to merge the patches.

The change you propose above would cause us to calculate watermarks as
if the cursor plane was on full-time (even when it's actually off
because the cursor is on the other display), so your watermarks wouldn't
need to change when you move your mouse to the other display and you
wouldn't see a flicker at that point.  If the review process for atomic
WM carries on too long, we may want to consider merging a patch like
yours as a short term workaround for the issue you're seeing.  There
would still be plenty of other ways to trigger flickering (changing
cursor size, using sprite planes (e.g., via xvideo), etc., but at least
a cursor getting enabled/disabled wouldn't cause a flicker.

Thanks.


Matt

-- 
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

  reply	other threads:[~2016-01-05 21:58 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-24 22:53 [PATCH 00/15] Atomic watermark updates (v5) Matt Roper
2015-09-24 22:53 ` [PATCH 01/15] drm/i915: Drop redundant watermark programming Matt Roper
2015-09-24 22:53 ` [PATCH 02/15] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2) Matt Roper
2016-01-05 12:49   ` Flicker caused by "drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)" Jan Niehusmann
2016-01-05 21:58     ` Matt Roper [this message]
2016-01-06  0:46       ` Jan Niehusmann
2015-09-24 22:53 ` [PATCH 03/15] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM (v2) Matt Roper
2015-09-24 22:53 ` [PATCH 04/15] drm/i915: Determine I915_MAX_PLANES from plane enum Matt Roper
2015-09-24 22:53 ` [PATCH 05/15] drm/i915/skl: Simplify wm structures slightly (v2) Matt Roper
2015-09-30 15:13   ` Daniel Vetter
2015-09-24 22:53 ` [PATCH 06/15] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v3) Matt Roper
2015-09-24 22:53 ` [PATCH 07/15] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check Matt Roper
2015-09-24 22:53 ` [PATCH 08/15] drm/i915: Drop intel_update_sprite_watermarks Matt Roper
2015-09-24 22:53 ` [PATCH 09/15] drm/i915: Refactor ilk_update_wm (v3) Matt Roper
2015-09-24 22:53 ` [PATCH 10/15] drm/i915: Calculate pipe watermarks into CRTC state (v3) Matt Roper
2015-09-24 22:53 ` [PATCH 11/15] drm/i915: Calculate ILK-style watermarks during atomic check (v3) Matt Roper
2015-09-24 22:53 ` [PATCH 12/15] drm/i915: Don't set plane visible during HW readout if CRTC is off Matt Roper
2015-09-24 22:53 ` [PATCH 13/15] drm/i915: Calculate watermark configuration during atomic check (v2) Matt Roper
2015-09-24 22:53 ` [PATCH 14/15] drm/i915: Sanitize watermarks after hardware state readout Matt Roper
2015-10-01 13:58   ` Jani Nikula
2015-10-01 16:12     ` Daniel Vetter
2015-10-01 16:53     ` [PATCH] drm/i915: Convert hsw_compute_linetime_wm to use in-flight state Matt Roper
2015-10-06 14:34       ` Jani Nikula
2015-09-24 22:53 ` [PATCH 15/15] drm/i915: Add two-stage ILK-style watermark programming (v5) Matt Roper
2015-09-30 15:20 ` [PATCH 00/15] Atomic watermark updates (v5) Daniel Vetter
2015-09-30 22:21   ` Zanoni, Paulo R
2015-10-01 23:03     ` [PATCH] fixup! drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v3) Matt Roper
2015-10-02 18:43       ` Zanoni, Paulo R
2015-10-06 10:13         ` [PATCH] fixup! drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v3) [regression] Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160105215819.GA21100@intel.com \
    --to=matthew.d.roper@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jan@gondor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).