From: Ander Conselvan De Oliveira <conselvan2@gmail.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/i915: Fix SKL cursor watermarks
Date: Wed, 22 Mar 2017 11:51:56 +0200 [thread overview]
Message-ID: <1490176316.3234.4.camel@gmail.com> (raw)
In-Reply-To: <dd95a610-6afd-9ac9-5a76-dbaee6901ab9@linux.intel.com>
On Wed, 2017-03-22 at 10:01 +0100, Maarten Lankhorst wrote:
> Op 14-03-17 om 16:10 schreef ville.syrjala@linux.intel.com:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Use intel_wm_plane_visible() to determine cursor visibility for SKL+
> > also. Previously SKL+ would check the actual visibility which now
> > conflicts with the assumptions in intel_legacy_cursor_update().
> >
> > We also change SKL+ to compute the cursor watermarks based on the
> > unclipped cursor size, just as we do on all the other platforms.
> > Using the clipped size could now result in garbage results.
> >
> > Testcase: igt/kms_chv_cursor_fail
> > Fixes: a5509abda48e ("drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100195
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> For patch 1 & 2:
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Should be the right way to fix it. :)
In intel_legacy_cursor_update(), I see the comment below,
/*
* If any parameters change that may affect watermarks,
* take the slowpath. Only changing fb or position should be
* in the fastpath.
*/
followed by a bunch of checks on the plane size and fb. My understanding is that
the bug was caused by those assumptions being out of sync with the actual
watermark code. So IMO, a more proper way to fix this would be to have
intel_legacy_cursor_update() call into watermark code to ask if it can proceed
or not, instead of making assumptions of what can cause watermarks to change.
But since the duplicated assumptions were there before, this fix doesn't make
the overall situation any worse.
Acked-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-03-22 9:52 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-14 15:10 [PATCH 1/2] drm/i915: Extract intel_wm_plane_visible() ville.syrjala
2017-03-14 15:10 ` [PATCH 2/2] drm/i915: Fix SKL cursor watermarks ville.syrjala
2017-03-15 10:46 ` [2/2] " Tahvanainen, Jari
2017-03-22 9:01 ` [PATCH 2/2] " Maarten Lankhorst
2017-03-22 9:51 ` Ander Conselvan De Oliveira [this message]
2017-03-22 20:20 ` Ville Syrjälä
2017-03-14 20:23 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Extract intel_wm_plane_visible() Patchwork
2017-03-21 9:58 ` Dorota Czaplejewicz
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=1490176316.3234.4.camel@gmail.com \
--to=conselvan2@gmail.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=ville.syrjala@linux.intel.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).