On Wed, 2017-03-22 at 10:01 +0100, Maarten Lankhorst wrote:
> Op 14-03-17 om 16:10 schreef ville.syrjala@xxxxxxxxxxxxxxx:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx>
>
> For patch 1 & 2:
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
