[Intel-gfx] [PATCH i-g-t 4/7] lib/igt_kms: Only move the in cursor plane for Intel hw.

[Daniel Vetter]

On Thu, Apr 21, 2016 at 01:24:53PM -0400, Robert Foss wrote:
> 
> 
> On 04/21/2016 10:48 AM, Daniel Vetter wrote:
> >On Wed, Apr 20, 2016 at 10:59:46AM -0400, robert.foss at collabora.com wrote:
> >>From: Robert Foss <robert.foss at collabora.com>
> >>
> >>Avoid moving the cursor plane when on non-intel hardware.
> >>Running the move block on hardware with more than IGT_PLANE_CURSOR
> >>number of planes causes planes do be zeroed out.
> >>
> >>Signed-off-by: Robert Foss <robert.foss at collabora.com>
> >>---
> >>  lib/igt_kms.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >>index 3f953ec..522ede5 100644
> >>--- a/lib/igt_kms.c
> >>+++ b/lib/igt_kms.c
> >>@@ -1363,7 +1363,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> >>  			 * only 1 sprite, that's the wrong slot and we need to
> >>  			 * move it down.
> >>  			 */
> >>-			if (p != IGT_PLANE_CURSOR) {
> >>+			if (IS_INTEL(drm_fd) && p != IGT_PLANE_CURSOR) {
> >Reading this again, isn't the problem that on some hw there's simply no
> >cursor plane? With universal planes the cursor plane should alias with
> >some of the real planes, and we simply need to make sure that we get that
> >aliasing right.
> >
> >But if there's no cursor registered, well there's no cursor ...
> >-Daniel
> From my point of view the problem is differentiating between the cases when
> IGT_PLANE_CURSOR is aliasing with IGT_PLANE_4 (IGT_PLANE_3+1).
> 
> IGT_PLANE_CURSOR is assumed to be the last plane in the if statement,
> which isn't the case for platform with more than 4 planes.

That's why you need to add more IGT_PLANE_x defines, to make sure _CURSOR
and the last one don't accidentally alias. Because if they do, we have a
problem.
-Daniel

> 
> So identifying which platforms this assumption is true (IS_INTEL()) and
> only proceeding for those platforms seemed like reasonable fix.
> Maybe a more generic solution is preferable.
> >>  				pipe->planes[p] = pipe->planes[IGT_PLANE_CURSOR];
> >>  				pipe->planes[p].index = p;
> >>  				memset(&pipe->planes[IGT_PLANE_CURSOR], 0,
> >>-- 
> >>2.5.0
> >>
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch