[Intel-gfx] [PATCH xf86-video-intel] sna: Switch back to hwcursor on the next cursor update

[Chris Wilson]

Quoting Ville Syrjälä (2018-10-19 21:41:05)
> On Fri, Oct 19, 2018 at 09:08:15PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-10-19 17:55:02)
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > Once we've switched to using the swcursor (possibly
> > > due to the cursor ioctl failing) we currently keep
> > > using the swcursor until the modeset.
> > > 
> > > That's not particularly great as the swcursor has several
> > > issues. Apart from the (presumably expected) flicker,
> > > the cursor also tends to leave horrible trails behind
> > > around dri2/3 windows (happens with tearfree at least).
> > > 
> > > To avoid some of that let's try to switch back to the hwcursor
> > > a bit sooner. We can do that neatly via the convenient swcursor
> > > block handler.
> > 
> > Ok, so that forces the sw cursor to be switched off on the very next
> > xf86CursorSetCursor() after being enabled. If the HWCursor fails, the
> > cursor should be invisible until the next block handler. Won't that
> > cause the cursor to flicker even more? Hmm, except that the swcursor
> > should be restored within a frame. So more or less simply doubling the
> > work of using swcursor? (Probably not actually, since swcursor is
> > effectively an undo and paint every time and now we just make those two
> > distinct steps) Certainly achieves the goal of forcing HW cursor back on
> > at the earliest opportunity.
> > 
> > I think I have just argued that this doesn't actually impact on swcursor
> > rendering, just extends the loop.
> 
> Hmm. I didn't fully consider what happens when we try to switch back.
> So first the old swscursor gets cleared out, then we try to turn on
> the hwcursor, and if that still fails we eventually render the new
> swcursor? Right, so I guess this just adds a bit of a delay between
> the two steps.
> 
> My intial thought was to use a timer, but then I realized that I
> can just stick it into the block handler while still avoiding any
> recursion horrors.

Yup, works nicely and even better with your

-       enable_fb_access(scrn, FALSE);
-       enable_fb_access(scrn, TRUE);
+       xf86CursorResetCursor(scrn->pScreen);

improvement!

> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106935
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Tentative
> >       Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> > I think I would like to review the impact on swcursor a bit more,
> > FAIL_CURSOR_IOCTL be my friend.
> 
> Sure. Maybe you can even figure out why we get those dirt trails ;)

Mumble, mumble, mumble (faking copy area around DRI2CopyRegion).
-Chris