[Intel-gfx] [PATCH 2/2] drm/i915: Fix SKL cursor watermarks

[Ander Conselvan De Oliveira]

On Wed, 2017-03-22 at 10:01 +0100, Maarten Lankhorst wrote:
> Op 14-03-17 om 16:10 schreef ville.syrjala at linux.intel.com:
> > From: Ville Syrjälä <ville.syrjala at 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 at linux.intel.com>
> 
> For patch 1 & 2:
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at 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 at gmail.com>