Re: [Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor

[Lyude Paul]

On Thu, 2016-10-27 at 15:35 -0700, Matt Roper wrote:
> On Thu, Oct 27, 2016 at 06:15:32PM -0400, Lyude Paul wrote:
> > 
> > On Wed, 2016-10-26 at 15:51 -0700, Matt Roper wrote:
> > > 
> > > Gen9 has a traditional cursor plane that is mutually exclusive
> > > with
> > > the
> > > system's top-most "universal" plane; it seems likely that two
> > > planes
> > > are
> > > really a single shared hardware unit with two different register
> > > interfaces.  Thus far i915 has exposed a cursor plane to
> > > userspace
> > > that's hooked up to the old-style cursor registers; we just
> > > pretended
> > > that the top-most universal plane didn't exist and reported one
> > > fewer
> > > "sprite/overlay" planes for each pipe than the platform
> > > technically
> > > has.
> > > Let's switch this around so that the cursor exposed to userspace
> > > is
> > > actually wired up to top-most universal plane registers.  We'll
> > > continue
> > > to present the same cursor ABI to userspace that we always have,
> > > but
> > > internally we'll just be programming a different set of registers
> > > and
> > > doing so in a way that's more consistent with how all the other
> > > gen9
> > > planes work --- less cursor-specific special cases throughout the
> > > code.
> > > 
> > > Aside from making the code a bit simpler (fewer cursor-specific
> > > special
> > > cases), this will also pave the way to eventually exposing the
> > > top-
> > > most
> > > plane in a more full-featured manner to userspace clients that
> > > don't
> > > need a traditional cursor and wish to opt into having access to
> > > an
> > > additional sprite/overlay-style plane instead.
> > > 
> > > It's worth noting that a lot of the special-cased cursor-specific
> > > code
> > > was in the gen9 watermark programming.  It's good to simplify
> > > that
> > > code,
> > > but we should keep an eye out for any unwanted side effects of
> > > this
> > > patch; since sprites/overlays tend to get used less than cursors,
> > > it's
> > > possible that this could help us uncover additional underruns
> > > that
> > > nobody had run across yet.  Or it could have the opposite effect
> > > and
> > > eliminate some of the underruns that we haven't been able to get
> > > rid
> > > of
> > > yet.
> > 
> > Alright, so I had to sit on this patch for a while. I think
> > exposing
> > this as a normal plane is a good idea: the rest of the world just
> > uses
> > planes, so we should too. We need to be *really* careful with this
> > though. Like Paulo said watermarks are still fragile and honestly I
> > wouldn't be surprised if we found more underruns hiding somewhere.
> 
> Agreed.  We're programming a different subset of the internal
> hardware
> so even if we do it 100% properly according to the spec, we can
> easily
> uncover previously hidden painpoints in the hardware that need new
> workarounds.
> 
> > 
> > First off, although this was mentioned in an e-mail down the chain,
> > I'm
> > pretty sure having a 0 block allocation is definitely not what we
> > want.
> > The spec itself says a minimum of 8 blocks assuming it's a normal
> > cursor, so if we're going below that we definitely need to at least
> > make sure that behavior isn't wrong.
> 
> DDB allocation should only be necessary if a plane is being used
> (otherwise you have no data to fill the buffer with anyway).  If a
> plane

Ah whoops, I thought you were implying that we had 0 block ddb
allocation on active planes.

> is off, then it shouldn't need allocation (and this patch guarantees
> that the cursor will _never_ be turned on) I'm pretty sure we've
> actually had some threads where Art (one of our hardware architects)
> mentioned that the cursor fixed allocation is just an optimization
> and
> isn't in any way required even when you do use the cursor hardware
> plane.  The only one I can find searching now is this one:
> 
> https://lists.freedesktop.org/archives/intel-gfx/2016-June/09
> 9256.html
> 
> but I feel like there was another thread (or maybe an IRC
> conversation)
> where he talked about it a bit more.  I'll see if I can find the one
> I'm
> thinking of.
> 
> > 
> > The second thing is that unfortunately this regresses in a rather
> > strange way. On this X1 yoga w/ a 4K screen (SKL chipset of
> > course), if
> > I move the cursor in front of a gray background I can see a faintly
> > visible box starting from the beginning of the cursor and going to
> > the
> > end of the plane. Reverting the patch fixes the problem.
> 
> Yeah, I mentioned that in the cover letter...I believe the color
> correction settings are different for the universal plane vs the
> cursor
> plane (which causes IGT CRC mismatches at the moment and may be
> visually
> noticeable if you have good eyes); that shouldn't be hard to track 
Ah whoops, must have missed that part. Regardless though, we should
probably get those out of the way first before applying any patches
like this. Otherwise I think the patch looks good, so long 

Re: [Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor

[Matt Roper]

On Thu, Oct 27, 2016 at 06:15:32PM -0400, Lyude Paul wrote:
> On Wed, 2016-10-26 at 15:51 -0700, Matt Roper wrote:
> > Gen9 has a traditional cursor plane that is mutually exclusive with
> > the
> > system's top-most "universal" plane; it seems likely that two planes
> > are
> > really a single shared hardware unit with two different register
> > interfaces.  Thus far i915 has exposed a cursor plane to userspace
> > that's hooked up to the old-style cursor registers; we just pretended
> > that the top-most universal plane didn't exist and reported one fewer
> > "sprite/overlay" planes for each pipe than the platform technically
> > has.
> > Let's switch this around so that the cursor exposed to userspace is
> > actually wired up to top-most universal plane registers.  We'll
> > continue
> > to present the same cursor ABI to userspace that we always have, but
> > internally we'll just be programming a different set of registers and
> > doing so in a way that's more consistent with how all the other gen9
> > planes work --- less cursor-specific special cases throughout the
> > code.
> > 
> > Aside from making the code a bit simpler (fewer cursor-specific
> > special
> > cases), this will also pave the way to eventually exposing the top-
> > most
> > plane in a more full-featured manner to userspace clients that don't
> > need a traditional cursor and wish to opt into having access to an
> > additional sprite/overlay-style plane instead.
> > 
> > It's worth noting that a lot of the special-cased cursor-specific
> > code
> > was in the gen9 watermark programming.  It's good to simplify that
> > code,
> > but we should keep an eye out for any unwanted side effects of this
> > patch; since sprites/overlays tend to get used less than cursors,
> > it's
> > possible that this could help us uncover additional underruns that
> > nobody had run across yet.  Or it could have the opposite effect and
> > eliminate some of the underruns that we haven't been able to get rid
> > of
> > yet.
> 
> Alright, so I had to sit on this patch for a while. I think exposing
> this as a normal plane is a good idea: the rest of the world just uses
> planes, so we should too. We need to be *really* careful with this
> though. Like Paulo said watermarks are still fragile and honestly I
> wouldn't be surprised if we found more underruns hiding somewhere.

Agreed.  We're programming a different subset of the internal hardware
so even if we do it 100% properly according to the spec, we can easily
uncover previously hidden painpoints in the hardware that need new
workarounds.

> First off, although this was mentioned in an e-mail down the chain, I'm
> pretty sure having a 0 block allocation is definitely not what we want.
> The spec itself says a minimum of 8 blocks assuming it's a normal
> cursor, so if we're going below that we definitely need to at least
> make sure that behavior isn't wrong.

DDB allocation should only be necessary if a plane is being used
(otherwise you have no data to fill the buffer with anyway).  If a plane
is off, then it shouldn't need allocation (and this patch guarantees
that the cursor will _never_ be turned on) I'm pretty sure we've
actually had some threads where Art (one of our hardware architects)
mentioned that the cursor fixed allocation is just an optimization and
isn't in any way required even when you do use the cursor hardware
plane.  The only one I can find searching now is this one:

https://lists.freedesktop.org/archives/intel-gfx/2016-June/099256.html

but I feel like there was another thread (or maybe an IRC conversation)
where he talked about it a bit more.  I'll see if I can find the one I'm
thinking of.

> The second thing is that unfortunately this regresses in a rather
> strange way. On this X1 yoga w/ a 4K screen (SKL chipset of course), if
> I move the cursor in front of a gray background I can see a faintly
> visible box starting from the beginning of the cursor and going to the
> end of the plane. Reverting the patch fixes the problem.

Yeah, I mentioned that in the cover letter...I believe the color
correction settings are different for the universal plane vs the cursor
plane (which causes IGT CRC mismatches at the moment and may be visually
noticeable if you have good eyes); that shouldn't be hard to track down
and fix.


Matt

> 
> > 
> > Cc: Bob Paauwe 
> > Cc: Maarten Lankhorst 
> > Cc: Paulo Zanoni 
> > Cc: Lyude Paul 
> > Signed-off-by: Matt Roper 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c  |  4 --
> >  drivers/gpu/drm/i915/i915_drv.h  | 11 +++-
> >  drivers/gpu/drm/i915/intel_device_info.c | 38 +
> >  drivers/gpu/drm/i915/intel_display.c | 97 
> > 
> >  drivers/gpu/drm/i915/intel_drv.h |  7 +++
> >  drivers/gpu/drm/i915/intel_pm.c  | 85 
> > 

Re: [Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor

[Lyude Paul]

On Wed, 2016-10-26 at 15:51 -0700, Matt Roper wrote:
> Gen9 has a traditional cursor plane that is mutually exclusive with
> the
> system's top-most "universal" plane; it seems likely that two planes
> are
> really a single shared hardware unit with two different register
> interfaces.  Thus far i915 has exposed a cursor plane to userspace
> that's hooked up to the old-style cursor registers; we just pretended
> that the top-most universal plane didn't exist and reported one fewer
> "sprite/overlay" planes for each pipe than the platform technically
> has.
> Let's switch this around so that the cursor exposed to userspace is
> actually wired up to top-most universal plane registers.  We'll
> continue
> to present the same cursor ABI to userspace that we always have, but
> internally we'll just be programming a different set of registers and
> doing so in a way that's more consistent with how all the other gen9
> planes work --- less cursor-specific special cases throughout the
> code.
> 
> Aside from making the code a bit simpler (fewer cursor-specific
> special
> cases), this will also pave the way to eventually exposing the top-
> most
> plane in a more full-featured manner to userspace clients that don't
> need a traditional cursor and wish to opt into having access to an
> additional sprite/overlay-style plane instead.
> 
> It's worth noting that a lot of the special-cased cursor-specific
> code
> was in the gen9 watermark programming.  It's good to simplify that
> code,
> but we should keep an eye out for any unwanted side effects of this
> patch; since sprites/overlays tend to get used less than cursors,
> it's
> possible that this could help us uncover additional underruns that
> nobody had run across yet.  Or it could have the opposite effect and
> eliminate some of the underruns that we haven't been able to get rid
> of
> yet.

Alright, so I had to sit on this patch for a while. I think exposing
this as a normal plane is a good idea: the rest of the world just uses
planes, so we should too. We need to be *really* careful with this
though. Like Paulo said watermarks are still fragile and honestly I
wouldn't be surprised if we found more underruns hiding somewhere.

First off, although this was mentioned in an e-mail down the chain, I'm
pretty sure having a 0 block allocation is definitely not what we want.
The spec itself says a minimum of 8 blocks assuming it's a normal
cursor, so if we're going below that we definitely need to at least
make sure that behavior isn't wrong.

The second thing is that unfortunately this regresses in a rather
strange way. On this X1 yoga w/ a 4K screen (SKL chipset of course), if
I move the cursor in front of a gray background I can see a faintly
visible box starting from the beginning of the cursor and going to the
end of the plane. Reverting the patch fixes the problem.

> 
> Cc: Bob Paauwe 
> Cc: Maarten Lankhorst 
> Cc: Paulo Zanoni 
> Cc: Lyude Paul 
> Signed-off-by: Matt Roper 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  4 --
>  drivers/gpu/drm/i915/i915_drv.h  | 11 +++-
>  drivers/gpu/drm/i915/intel_device_info.c | 38 +
>  drivers/gpu/drm/i915/intel_display.c | 97 
> 
>  drivers/gpu/drm/i915/intel_drv.h |  7 +++
>  drivers/gpu/drm/i915/intel_pm.c  | 85 
> 
>  drivers/gpu/drm/i915/intel_sprite.c  |  6 +-
>  7 files changed, 93 insertions(+), 155 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9f5a392..0bba472 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3469,10 +3469,6 @@ static int i915_ddb_info(struct seq_file *m,
> void *unused)
>      entry->start, entry->end,
>      skl_ddb_entry_size(entry));
>   }
> -
> - entry = >plane[pipe][PLANE_CURSOR];
> - seq_printf(m, "  %-13s%8u%8u%8u\n", "Cursor", entry-
> >start,
> -    entry->end, skl_ddb_entry_size(entry));
>   }
>  
>   drm_modeset_unlock_all(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 4714051..83aaed2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -178,7 +178,7 @@ enum plane {
>   PLANE_A = 0,
>   PLANE_B,
>   PLANE_C,
> - PLANE_CURSOR,
> + PLANE_D,
>   I915_MAX_PLANES,
>  };
>  #define plane_name(p) ((p) + 'A')
> @@ -316,9 +316,15 @@ struct i915_hotplug {
>   for ((__p) = 0; 
>   \
>    (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] +
> 1;\
>    (__p)++)
> +
> +/*
> + * Only iterate over sprites exposed as sprites; omit sprites that
> + 

Re: [Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor

[Matt Roper]

On Thu, Oct 27, 2016 at 06:03:54PM -0200, Paulo Zanoni wrote:
> Em Qua, 2016-10-26 às 15:51 -0700, Matt Roper escreveu:
> > Gen9 has a traditional cursor plane that is mutually exclusive with
> > the
> > system's top-most "universal" plane; it seems likely that two planes
> > are
> > really a single shared hardware unit with two different register
> > interfaces.  Thus far i915 has exposed a cursor plane to userspace
> > that's hooked up to the old-style cursor registers; we just pretended
> > that the top-most universal plane didn't exist and reported one fewer
> > "sprite/overlay" planes for each pipe than the platform technically
> > has.
> > Let's switch this around so that the cursor exposed to userspace is
> > actually wired up to top-most universal plane registers.  We'll
> > continue
> > to present the same cursor ABI to userspace that we always have, but
> > internally we'll just be programming a different set of registers and
> > doing so in a way that's more consistent with how all the other gen9
> > planes work --- less cursor-specific special cases throughout the
> > code.
> > 
> > Aside from making the code a bit simpler (fewer cursor-specific
> > special
> > cases), this will also pave the way to eventually exposing the top-
> > most
> > plane in a more full-featured manner to userspace clients that don't
> > need a traditional cursor and wish to opt into having access to an
> > additional sprite/overlay-style plane instead.
> > 
> > It's worth noting that a lot of the special-cased cursor-specific
> > code
> > was in the gen9 watermark programming.  It's good to simplify that
> > code,
> > but we should keep an eye out for any unwanted side effects of this
> > patch; since sprites/overlays tend to get used less than cursors,
> > it's
> > possible that this could help us uncover additional underruns that
> > nobody had run across yet.  Or it could have the opposite effect and
> > eliminate some of the underruns that we haven't been able to get rid
> > of
> > yet.
> > 
> > Cc: Bob Paauwe 
> > Cc: Maarten Lankhorst 
> > Cc: Paulo Zanoni 
> > Cc: Lyude Paul 
> > Signed-off-by: Matt Roper 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c  |  4 --
> >  drivers/gpu/drm/i915/i915_drv.h  | 11 +++-
> >  drivers/gpu/drm/i915/intel_device_info.c | 38 +
> >  drivers/gpu/drm/i915/intel_display.c | 97 
> > 
> >  drivers/gpu/drm/i915/intel_drv.h |  7 +++
> >  drivers/gpu/drm/i915/intel_pm.c  | 85 
> > 
> >  drivers/gpu/drm/i915/intel_sprite.c  |  6 +-
> >  7 files changed, 93 insertions(+), 155 deletions(-)
> > 
...
> >  
> > -   cursor_blocks = skl_cursor_allocation(num_active);
> > -   ddb->plane[pipe][PLANE_CURSOR].start = alloc->end -
> > cursor_blocks;
> > -   ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
> > -
> > -   alloc_size -= cursor_blocks;
> 
> So this means that now in step 2 we're going to try to increase the
> cursor DDBs beyond the current fixed sizes of 32 or 8? Aren't we going
> to leave the cursor DDB way-too-big and taking space of the other DDBs
> when it's just being used as a cursor?

You're right that we're not using the fixed size allocation anymore.
However the extra blocks that we give the pseudo-cursor should still be
proportional to the size/data rate of the plane.  So if you're using a
64x64 "cursor," that's going to be very small compared to your other
planes (the primary plane is often fullscreen and other universal planes
are a good portion of full size).  In practice it seems to work out that
the extra blocks calculation winds up rounding down to zero or near zero
since the "cursor" is so dwarfed by the primary plane --- on my
1920x1200 display it works out as:

Primary data rate:  8294400
Cursor data rate: 16384
Total data rate:8310784
Remaining DDB blocks:   492

Primary extra blocks = 8294400 * 492 / 8310784
 = (int)491.03006
 = 491
Cursor extra blocks = 16384 * 492 / 8310784
 = (int)0.969935
 = 0

Due to rounding we don't actually add any cursor blocks at all.  In fact
we're using _fewer_ blocks for cursor than the 32 fixed allocation we
were previously using for single display setups.

> Also, we're not going to leave the cursor DDB at the end of the buffer,
> which may cause some extra DDB reallocations that we may not want.
> Also, whenever someone disables the cursor we'll do a DDB reallocation,
> and as far as I understand we really don't want this specific case
> since it's so common.

True, but the reallocation in this case is an intra-pipe reallocation
rather than the horrific "lock the whole world" inter-pipe reallocation.
It means we crunch a few more 

Re: [Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor

[Paulo Zanoni]

Em Qua, 2016-10-26 às 15:51 -0700, Matt Roper escreveu:
> Gen9 has a traditional cursor plane that is mutually exclusive with
> the
> system's top-most "universal" plane; it seems likely that two planes
> are
> really a single shared hardware unit with two different register
> interfaces.  Thus far i915 has exposed a cursor plane to userspace
> that's hooked up to the old-style cursor registers; we just pretended
> that the top-most universal plane didn't exist and reported one fewer
> "sprite/overlay" planes for each pipe than the platform technically
> has.
> Let's switch this around so that the cursor exposed to userspace is
> actually wired up to top-most universal plane registers.  We'll
> continue
> to present the same cursor ABI to userspace that we always have, but
> internally we'll just be programming a different set of registers and
> doing so in a way that's more consistent with how all the other gen9
> planes work --- less cursor-specific special cases throughout the
> code.
> 
> Aside from making the code a bit simpler (fewer cursor-specific
> special
> cases), this will also pave the way to eventually exposing the top-
> most
> plane in a more full-featured manner to userspace clients that don't
> need a traditional cursor and wish to opt into having access to an
> additional sprite/overlay-style plane instead.
> 
> It's worth noting that a lot of the special-cased cursor-specific
> code
> was in the gen9 watermark programming.  It's good to simplify that
> code,
> but we should keep an eye out for any unwanted side effects of this
> patch; since sprites/overlays tend to get used less than cursors,
> it's
> possible that this could help us uncover additional underruns that
> nobody had run across yet.  Or it could have the opposite effect and
> eliminate some of the underruns that we haven't been able to get rid
> of
> yet.
> 
> Cc: Bob Paauwe 
> Cc: Maarten Lankhorst 
> Cc: Paulo Zanoni 
> Cc: Lyude Paul 
> Signed-off-by: Matt Roper 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  4 --
>  drivers/gpu/drm/i915/i915_drv.h  | 11 +++-
>  drivers/gpu/drm/i915/intel_device_info.c | 38 +
>  drivers/gpu/drm/i915/intel_display.c | 97 
> 
>  drivers/gpu/drm/i915/intel_drv.h |  7 +++
>  drivers/gpu/drm/i915/intel_pm.c  | 85 
> 
>  drivers/gpu/drm/i915/intel_sprite.c  |  6 +-
>  7 files changed, 93 insertions(+), 155 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9f5a392..0bba472 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3469,10 +3469,6 @@ static int i915_ddb_info(struct seq_file *m,
> void *unused)
>      entry->start, entry->end,
>      skl_ddb_entry_size(entry));
>   }
> -
> - entry = >plane[pipe][PLANE_CURSOR];
> - seq_printf(m, "  %-13s%8u%8u%8u\n", "Cursor", entry-
> >start,
> -    entry->end, skl_ddb_entry_size(entry));
>   }
>  
>   drm_modeset_unlock_all(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 4714051..83aaed2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -178,7 +178,7 @@ enum plane {
>   PLANE_A = 0,
>   PLANE_B,
>   PLANE_C,
> - PLANE_CURSOR,
> + PLANE_D,
>   I915_MAX_PLANES,
>  };
>  #define plane_name(p) ((p) + 'A')
> @@ -316,9 +316,15 @@ struct i915_hotplug {
>   for ((__p) = 0; 
>   \
>    (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] +
> 1;\
>    (__p)++)
> +
> +/*
> + * Only iterate over sprites exposed as sprites; omit sprites that
> + * are being repurposed to simulate a cursor.
> + */
>  #define for_each_sprite(__dev_priv, __p, __s)
>   \
>   for ((__s) = 0; 
>   \
> -  (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];
> \
> +  (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)] -   
> \
> +  (INTEL_INFO(__dev_priv)->has_real_cursor ? 0 :
> 1);   \
>    (__s)++)
>  
>  #define for_each_port_masked(__port, __ports_mask) \
> @@ -687,6 +693,7 @@ struct intel_csr {
>   func(has_psr); \
>   func(has_rc6); \
>   func(has_rc6p); \
> + func(has_real_cursor); \
>   func(has_resource_streamer); \
>   func(has_runtime_pm); \
>   func(has_snoop); \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index d6a8f11..a464e0e 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ 

[Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor

[Matt Roper]

Gen9 has a traditional cursor plane that is mutually exclusive with the
system's top-most "universal" plane; it seems likely that two planes are
really a single shared hardware unit with two different register
interfaces.  Thus far i915 has exposed a cursor plane to userspace
that's hooked up to the old-style cursor registers; we just pretended
that the top-most universal plane didn't exist and reported one fewer
"sprite/overlay" planes for each pipe than the platform technically has.
Let's switch this around so that the cursor exposed to userspace is
actually wired up to top-most universal plane registers.  We'll continue
to present the same cursor ABI to userspace that we always have, but
internally we'll just be programming a different set of registers and
doing so in a way that's more consistent with how all the other gen9
planes work --- less cursor-specific special cases throughout the code.

Aside from making the code a bit simpler (fewer cursor-specific special
cases), this will also pave the way to eventually exposing the top-most
plane in a more full-featured manner to userspace clients that don't
need a traditional cursor and wish to opt into having access to an
additional sprite/overlay-style plane instead.

It's worth noting that a lot of the special-cased cursor-specific code
was in the gen9 watermark programming.  It's good to simplify that code,
but we should keep an eye out for any unwanted side effects of this
patch; since sprites/overlays tend to get used less than cursors, it's
possible that this could help us uncover additional underruns that
nobody had run across yet.  Or it could have the opposite effect and
eliminate some of the underruns that we haven't been able to get rid of
yet.

Cc: Bob Paauwe 
Cc: Maarten Lankhorst 
Cc: Paulo Zanoni 
Cc: Lyude Paul 
Signed-off-by: Matt Roper 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  4 --
 drivers/gpu/drm/i915/i915_drv.h  | 11 +++-
 drivers/gpu/drm/i915/intel_device_info.c | 38 +
 drivers/gpu/drm/i915/intel_display.c | 97 
 drivers/gpu/drm/i915/intel_drv.h |  7 +++
 drivers/gpu/drm/i915/intel_pm.c  | 85 
 drivers/gpu/drm/i915/intel_sprite.c  |  6 +-
 7 files changed, 93 insertions(+), 155 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 9f5a392..0bba472 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3469,10 +3469,6 @@ static int i915_ddb_info(struct seq_file *m, void 
*unused)
   entry->start, entry->end,
   skl_ddb_entry_size(entry));
}
-
-   entry = >plane[pipe][PLANE_CURSOR];
-   seq_printf(m, "  %-13s%8u%8u%8u\n", "Cursor", entry->start,
-  entry->end, skl_ddb_entry_size(entry));
}
 
drm_modeset_unlock_all(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4714051..83aaed2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -178,7 +178,7 @@ enum plane {
PLANE_A = 0,
PLANE_B,
PLANE_C,
-   PLANE_CURSOR,
+   PLANE_D,
I915_MAX_PLANES,
 };
 #define plane_name(p) ((p) + 'A')
@@ -316,9 +316,15 @@ struct i915_hotplug {
for ((__p) = 0; \
 (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] + 1; \
 (__p)++)
+
+/*
+ * Only iterate over sprites exposed as sprites; omit sprites that
+ * are being repurposed to simulate a cursor.
+ */
 #define for_each_sprite(__dev_priv, __p, __s)  \
for ((__s) = 0; \
-(__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];\
+(__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)] -   \
+(INTEL_INFO(__dev_priv)->has_real_cursor ? 0 : 1); \
 (__s)++)
 
 #define for_each_port_masked(__port, __ports_mask) \
@@ -687,6 +693,7 @@ struct intel_csr {
func(has_psr); \
func(has_rc6); \
func(has_rc6p); \
+   func(has_real_cursor); \
func(has_resource_streamer); \
func(has_runtime_pm); \
func(has_snoop); \
diff --git a/drivers/gpu/drm/i915/intel_device_info.c 
b/drivers/gpu/drm/i915/intel_device_info.c
index d6a8f11..a464e0e 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -271,23 +271,39 @@ void intel_device_info_runtime_init(struct 
drm_i915_private *dev_priv)
enum pipe pipe;
 
/*
-* Skylake and Broxton currently don't expose the topmost plane as its
-* use is exclusive with the