All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Exposing plane type mask and handling 'all' planes
@ 2019-06-19 16:03 Emil Velikov
  2019-06-19 16:33 ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Emil Velikov @ 2019-06-19 16:03 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Miguel Casas-Sanchez, Alexandros Frantzis

Hi all,

Recently I have been looking at i915 and its rather interesting planes.

In particular newer hardware is capable of using 3 universal planes and
a separate cursor-only plane. At the same time only 1 top-most plane can
be enabled - lets calls those plane3 or cursor.

Hence currently the hardware has an extra plane which we do not use.

The current DRM design does not allow us to fully utilise the HW and I
would love to address that. Here are three approaches that come to mind:

1) Extend the DRM plane UAPI to:
 - expose a mask of supported types, and
 - allow userspace to select the type

2) Keep the exposed planes as-is and let the driver orchestrate which
   one gets used.
 - flip between cursor/plane3 based on the use-case/API used, or
 - opt for plane3/cursor for atomic and legacy modeset code path, or
 - other

3) Expose plane3 and cursor, whereby using one of them "marks" the other
   as used.
 - is this allowed by the modeset semantics/policy?
 - does existing user-space have fallback path?


Personally, I would love existing user-space to just work without
modification. At the same time opting for 2 this will cause a serious
amount of complication, and in case of 3 the whole thing will be very
fragile. So my current inclination is towards 1.

Open questions:
 - Do we know of other hardware with this or related design which does
not fit the current DRM design?
 - Vendor KMS specific UAPI, is frowned upon. So if we are to extend the
UAPI, does the current approach sound useful for other HW?
 - Does this approach sound reasonable, can other share their view on a 
better approach, that achieves the goal?

Input and ideas from the Intel team and DRM community are very much
appreciated.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-19 16:03 [RFC] Exposing plane type mask and handling 'all' planes Emil Velikov
@ 2019-06-19 16:33 ` Ville Syrjälä
  2019-06-19 17:49   ` Emil Velikov
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2019-06-19 16:33 UTC (permalink / raw)
  To: Emil Velikov
  Cc: intel-gfx, Miguel Casas-Sanchez, dri-devel, Alexandros Frantzis

On Wed, Jun 19, 2019 at 05:03:53PM +0100, Emil Velikov wrote:
> Hi all,
> 
> Recently I have been looking at i915 and its rather interesting planes.
> 
> In particular newer hardware is capable of using 3 universal planes and
> a separate cursor-only plane. At the same time only 1 top-most plane can
> be enabled - lets calls those plane3 or cursor.
> 
> Hence currently the hardware has an extra plane which we do not use.

Only skl (and derivatives) have that. More modern platforms went back to
the dedicated cursor plane. So IMO no point in exposing this mess at
all.

> 
> The current DRM design does not allow us to fully utilise the HW and I
> would love to address that. Here are three approaches that come to mind:
> 
> 1) Extend the DRM plane UAPI to:
>  - expose a mask of supported types, and
>  - allow userspace to select the type
> 
> 2) Keep the exposed planes as-is and let the driver orchestrate which
>    one gets used.
>  - flip between cursor/plane3 based on the use-case/API used, or
>  - opt for plane3/cursor for atomic and legacy modeset code path, or
>  - other
> 
> 3) Expose plane3 and cursor, whereby using one of them "marks" the other
>    as used.
>  - is this allowed by the modeset semantics/policy?
>  - does existing user-space have fallback path?
> 
> 
> Personally, I would love existing user-space to just work without
> modification. At the same time opting for 2 this will cause a serious
> amount of complication, and in case of 3 the whole thing will be very
> fragile. So my current inclination is towards 1.
> 
> Open questions:
>  - Do we know of other hardware with this or related design which does
> not fit the current DRM design?
>  - Vendor KMS specific UAPI, is frowned upon. So if we are to extend the
> UAPI, does the current approach sound useful for other HW?
>  - Does this approach sound reasonable, can other share their view on a 
> better approach, that achieves the goal?
> 
> Input and ideas from the Intel team and DRM community are very much
> appreciated.
> 
> Thanks
> Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-19 16:33 ` Ville Syrjälä
@ 2019-06-19 17:49   ` Emil Velikov
  2019-06-19 18:24     ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Emil Velikov @ 2019-06-19 17:49 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Intel Graphics Development, Miguel Casas-Sanchez, ML dri-devel,
	Alexandros Frantzis

On Wed, 19 Jun 2019 at 17:33, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Wed, Jun 19, 2019 at 05:03:53PM +0100, Emil Velikov wrote:
> > Hi all,
> >
> > Recently I have been looking at i915 and its rather interesting planes.
> >
> > In particular newer hardware is capable of using 3 universal planes and
> > a separate cursor-only plane. At the same time only 1 top-most plane can
> > be enabled - lets calls those plane3 or cursor.
> >
> > Hence currently the hardware has an extra plane which we do not use.
>
> Only skl (and derivatives) have that. More modern platforms went back to
> the dedicated cursor plane. So IMO no point in exposing this mess at
> all.
>
I suspect you're talking about Ice Lake, correct?

> >
> > The current DRM design does not allow us to fully utilise the HW and I
> > would love to address that. Here are three approaches that come to mind:
> >
> > 1) Extend the DRM plane UAPI to:
> >  - expose a mask of supported types, and
> >  - allow userspace to select the type
> >
> > 2) Keep the exposed planes as-is and let the driver orchestrate which
> >    one gets used.
> >  - flip between cursor/plane3 based on the use-case/API used, or
> >  - opt for plane3/cursor for atomic and legacy modeset code path, or
> >  - other
> >
> > 3) Expose plane3 and cursor, whereby using one of them "marks" the other
> >    as used.
> >  - is this allowed by the modeset semantics/policy?
> >  - does existing user-space have fallback path?
> >
> >
> > Personally, I would love existing user-space to just work without
> > modification. At the same time opting for 2 this will cause a serious
> > amount of complication, and in case of 3 the whole thing will be very
> > fragile. So my current inclination is towards 1.
> >
> > Open questions:
> >  - Do we know of other hardware with this or related design which does
> > not fit the current DRM design?
> >  - Vendor KMS specific UAPI, is frowned upon. So if we are to extend the
> > UAPI, does the current approach sound useful for other HW?
> >  - Does this approach sound reasonable, can other share their view on a
> > better approach, that achieves the goal?
> >
Speaking hypothetically:

If the mutually exclusive design was very common, which of the
proposed solutions you think will be the best fit?
May I ask you for a mini-brain dump how you foresee that being implemented :-)

Thanks
Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-19 17:49   ` Emil Velikov
@ 2019-06-19 18:24     ` Ville Syrjälä
  2019-06-26  0:46       ` Matt Roper
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2019-06-19 18:24 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Intel Graphics Development, Miguel Casas-Sanchez, ML dri-devel,
	Alexandros Frantzis

On Wed, Jun 19, 2019 at 06:49:11PM +0100, Emil Velikov wrote:
> On Wed, 19 Jun 2019 at 17:33, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Wed, Jun 19, 2019 at 05:03:53PM +0100, Emil Velikov wrote:
> > > Hi all,
> > >
> > > Recently I have been looking at i915 and its rather interesting planes.
> > >
> > > In particular newer hardware is capable of using 3 universal planes and
> > > a separate cursor-only plane. At the same time only 1 top-most plane can
> > > be enabled - lets calls those plane3 or cursor.
> > >
> > > Hence currently the hardware has an extra plane which we do not use.
> >
> > Only skl (and derivatives) have that. More modern platforms went back to
> > the dedicated cursor plane. So IMO no point in exposing this mess at
> > all.
> >
> I suspect you're talking about Ice Lake, correct?

And glk. The other relevant platform joined the fight club so we don't
talk about it.

> 
> > >
> > > The current DRM design does not allow us to fully utilise the HW and I
> > > would love to address that. Here are three approaches that come to mind:
> > >
> > > 1) Extend the DRM plane UAPI to:
> > >  - expose a mask of supported types, and
> > >  - allow userspace to select the type
> > >
> > > 2) Keep the exposed planes as-is and let the driver orchestrate which
> > >    one gets used.
> > >  - flip between cursor/plane3 based on the use-case/API used, or
> > >  - opt for plane3/cursor for atomic and legacy modeset code path, or
> > >  - other
> > >
> > > 3) Expose plane3 and cursor, whereby using one of them "marks" the other
> > >    as used.
> > >  - is this allowed by the modeset semantics/policy?
> > >  - does existing user-space have fallback path?
> > >
> > >
> > > Personally, I would love existing user-space to just work without
> > > modification. At the same time opting for 2 this will cause a serious
> > > amount of complication, and in case of 3 the whole thing will be very
> > > fragile. So my current inclination is towards 1.
> > >
> > > Open questions:
> > >  - Do we know of other hardware with this or related design which does
> > > not fit the current DRM design?
> > >  - Vendor KMS specific UAPI, is frowned upon. So if we are to extend the
> > > UAPI, does the current approach sound useful for other HW?
> > >  - Does this approach sound reasonable, can other share their view on a
> > > better approach, that achieves the goal?
> > >
> Speaking hypothetically:
> 
> If the mutually exclusive design was very common, which of the
> proposed solutions you think will be the best fit?
> May I ask you for a mini-brain dump how you foresee that being implemented :-)

I would go for option 2. Option 1 is pretty much the same thing with a
slight hint for userspace that maybe the plane can do a bit more than
just "cursory" things. The problem is that we have no actual definition
of what these "cursory" things are. It's totally hardware/driver
specific. Eg. i915 already allows you to do various things with the
cursor plane that may not work on other hardware. Also I'm sure there's
plenty of hardware out there with no special purpose cursor planes, so
those drivers will probably just designate one regular plane as
"the cursor" anyway.

So I think we've been pretty much doing option 2 all along. So if some
userspace wanted to it could just ignore the cursor type hint and try
to use the plane in any way it sees fit. The usual rule of "try
CHECK_ONLY and see it you get an error" applies here as well.

If we did want to go for option 1 then I think it should take the form
of some better defined plane capabilities. The problem is that there
are alwayas tons of hardware specific exceptions so you wouldn't be
able to blindly trust the caps anyway.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-19 18:24     ` Ville Syrjälä
@ 2019-06-26  0:46       ` Matt Roper
  2019-06-28 16:14         ` Emil Velikov
  0 siblings, 1 reply; 9+ messages in thread
From: Matt Roper @ 2019-06-26  0:46 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Intel Graphics Development, Miguel Casas-Sanchez, ML dri-devel,
	Alexandros Frantzis

On Wed, Jun 19, 2019 at 09:24:56PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 19, 2019 at 06:49:11PM +0100, Emil Velikov wrote:
> > On Wed, 19 Jun 2019 at 17:33, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > On Wed, Jun 19, 2019 at 05:03:53PM +0100, Emil Velikov wrote:
> > > > Hi all,
> > > >
> > > > Recently I have been looking at i915 and its rather interesting planes.
> > > >
> > > > In particular newer hardware is capable of using 3 universal planes and
> > > > a separate cursor-only plane. At the same time only 1 top-most plane can
> > > > be enabled - lets calls those plane3 or cursor.
> > > >
> > > > Hence currently the hardware has an extra plane which we do not use.
> > >
> > > Only skl (and derivatives) have that. More modern platforms went back to
> > > the dedicated cursor plane. So IMO no point in exposing this mess at
> > > all.
> > >
> > I suspect you're talking about Ice Lake, correct?
> 
> And glk. The other relevant platform joined the fight club so we don't
> talk about it.
> 
> > 
> > > >
> > > > The current DRM design does not allow us to fully utilise the HW and I
> > > > would love to address that. Here are three approaches that come to mind:
> > > >
> > > > 1) Extend the DRM plane UAPI to:
> > > >  - expose a mask of supported types, and
> > > >  - allow userspace to select the type
> > > >
> > > > 2) Keep the exposed planes as-is and let the driver orchestrate which
> > > >    one gets used.
> > > >  - flip between cursor/plane3 based on the use-case/API used, or
> > > >  - opt for plane3/cursor for atomic and legacy modeset code path, or
> > > >  - other
> > > >
> > > > 3) Expose plane3 and cursor, whereby using one of them "marks" the other
> > > >    as used.
> > > >  - is this allowed by the modeset semantics/policy?
> > > >  - does existing user-space have fallback path?
> > > >
> > > >
> > > > Personally, I would love existing user-space to just work without
> > > > modification. At the same time opting for 2 this will cause a serious
> > > > amount of complication, and in case of 3 the whole thing will be very
> > > > fragile. So my current inclination is towards 1.
> > > >
> > > > Open questions:
> > > >  - Do we know of other hardware with this or related design which does
> > > > not fit the current DRM design?
> > > >  - Vendor KMS specific UAPI, is frowned upon. So if we are to extend the
> > > > UAPI, does the current approach sound useful for other HW?
> > > >  - Does this approach sound reasonable, can other share their view on a
> > > > better approach, that achieves the goal?
> > > >
> > Speaking hypothetically:
> > 
> > If the mutually exclusive design was very common, which of the
> > proposed solutions you think will be the best fit?
> > May I ask you for a mini-brain dump how you foresee that being implemented :-)
> 
> I would go for option 2. Option 1 is pretty much the same thing with a
> slight hint for userspace that maybe the plane can do a bit more than
> just "cursory" things. The problem is that we have no actual definition
> of what these "cursory" things are. It's totally hardware/driver
> specific. Eg. i915 already allows you to do various things with the
> cursor plane that may not work on other hardware. Also I'm sure there's
> plenty of hardware out there with no special purpose cursor planes, so
> those drivers will probably just designate one regular plane as
> "the cursor" anyway.
> 
> So I think we've been pretty much doing option 2 all along. So if some
> userspace wanted to it could just ignore the cursor type hint and try
> to use the plane in any way it sees fit. The usual rule of "try
> CHECK_ONLY and see it you get an error" applies here as well.
> 
> If we did want to go for option 1 then I think it should take the form
> of some better defined plane capabilities. The problem is that there
> are alwayas tons of hardware specific exceptions so you wouldn't be
> able to blindly trust the caps anyway.

PLANE_CURSOR is basically just an indication that that specific plane is
the one that's also hooked up to the legacy cursor ioctls; like Ville
says, it shouldn't directly indicate that the plane is less
feature-capable than other planes.  You can either detect the true
capabilities of the cursor plane by checking for the presence/absence of
other plane properties and/or experimenting with atomic TEST_ONLY
commits to see what's really possible.

The ideal solution for Intel gen9 hardware would have been to just never
have the driver advertise or program the dedicated hardware cursor at
all, but to instead expose the top-most universal plane to userspace,
describe it as PLANE_CURSOR, and route the legacy cursor ioctl's to that
plane instead.  That would allow legacy cursor behavior to work as
usual, but would also allow atomic userspace to use the plane in a more
full-featured manner.  I wrote patches to do exactly this a couple years
ago, but sadly we discovered that the universal planes on gen9 have a
slight alpha blending defect that the dedicated hardware cursor does not
exhibit.  Thus replacing the hardware cursor with the topmost universal
plane led to a slight regression for existing users and we had to scrap
the whole idea.  :-(

For reference, the relevant patch from a few years ago is here:
        https://patchwork.kernel.org/patch/9398571/


Matt

> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-26  0:46       ` Matt Roper
@ 2019-06-28 16:14         ` Emil Velikov
  2019-06-28 17:37           ` Matt Roper
  0 siblings, 1 reply; 9+ messages in thread
From: Emil Velikov @ 2019-06-28 16:14 UTC (permalink / raw)
  To: Matt Roper
  Cc: Intel Graphics Development, Miguel Casas-Sanchez, ML dri-devel,
	Alexandros Frantzis

Hi Matt,

Thanks for the enlightening input :-)

On 2019/06/25, Matt Roper wrote:

> PLANE_CURSOR is basically just an indication that that specific plane is
> the one that's also hooked up to the legacy cursor ioctls; like Ville
> says, it shouldn't directly indicate that the plane is less
> feature-capable than other planes.  You can either detect the true
> capabilities of the cursor plane by checking for the presence/absence of
> other plane properties and/or experimenting with atomic TEST_ONLY
> commits to see what's really possible.
> 
Interesting, my understanding was the plane type was a hint about the
capabilities. Although yes, userspace must check via TEST_ONLY to ensure
the properties chosen will work.


> The ideal solution for Intel gen9 hardware would have been to just never
> have the driver advertise or program the dedicated hardware cursor at
> all, but to instead expose the top-most universal plane to userspace,
> describe it as PLANE_CURSOR, and route the legacy cursor ioctl's to that
> plane instead.  That would allow legacy cursor behavior to work as
> usual, but would also allow atomic userspace to use the plane in a more
> full-featured manner.  I wrote patches to do exactly this a couple years
> ago, but sadly we discovered that the universal planes on gen9 have a
> slight alpha blending defect that the dedicated hardware cursor does not
> exhibit.  Thus replacing the hardware cursor with the topmost universal
> plane led to a slight regression for existing users and we had to scrap
> the whole idea.  :-(
> 
> For reference, the relevant patch from a few years ago is here:
>         https://patchwork.kernel.org/patch/9398571/
> 
In that thread you mention:

"... 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."

Yet above you mention that universal planes have alpha blending defect.
Did you confirm that with HW/simulation teams or is that based on the
documentation? I would love to read a bit more on the topic.

In particular, but not limited to, if this defect is applicable only for
plane3 or literally all universal planes.

Thanks again,
Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-28 16:14         ` Emil Velikov
@ 2019-06-28 17:37           ` Matt Roper
  2019-06-28 18:54             ` Emil Velikov
  0 siblings, 1 reply; 9+ messages in thread
From: Matt Roper @ 2019-06-28 17:37 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Intel Graphics Development, Miguel Casas-Sanchez, ML dri-devel,
	Alexandros Frantzis

On Fri, Jun 28, 2019 at 05:14:51PM +0100, Emil Velikov wrote:
> Hi Matt,
> 
> Thanks for the enlightening input :-)
> 
> On 2019/06/25, Matt Roper wrote:
> 
> > PLANE_CURSOR is basically just an indication that that specific plane is
> > the one that's also hooked up to the legacy cursor ioctls; like Ville
> > says, it shouldn't directly indicate that the plane is less
> > feature-capable than other planes.  You can either detect the true
> > capabilities of the cursor plane by checking for the presence/absence of
> > other plane properties and/or experimenting with atomic TEST_ONLY
> > commits to see what's really possible.
> > 
> Interesting, my understanding was the plane type was a hint about the
> capabilities. Although yes, userspace must check via TEST_ONLY to ensure
> the properties chosen will work.
> 
> 
> > The ideal solution for Intel gen9 hardware would have been to just never
> > have the driver advertise or program the dedicated hardware cursor at
> > all, but to instead expose the top-most universal plane to userspace,
> > describe it as PLANE_CURSOR, and route the legacy cursor ioctl's to that
> > plane instead.  That would allow legacy cursor behavior to work as
> > usual, but would also allow atomic userspace to use the plane in a more
> > full-featured manner.  I wrote patches to do exactly this a couple years
> > ago, but sadly we discovered that the universal planes on gen9 have a
> > slight alpha blending defect that the dedicated hardware cursor does not
> > exhibit.  Thus replacing the hardware cursor with the topmost universal
> > plane led to a slight regression for existing users and we had to scrap
> > the whole idea.  :-(
> > 
> > For reference, the relevant patch from a few years ago is here:
> >         https://patchwork.kernel.org/patch/9398571/
> > 
> In that thread you mention:
> 
> "... 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."
> 
> Yet above you mention that universal planes have alpha blending defect.
> Did you confirm that with HW/simulation teams or is that based on the
> documentation? I would love to read a bit more on the topic.
> 
> In particular, but not limited to, if this defect is applicable only for
> plane3 or literally all universal planes.

We only figured out exactly what was going on a while after I wrote that
message in the thread, but we did ultimately confirm the problem with
the hardware architects.  Sadly, all of the gen9 universal planes suffer
from the slight blending issue and only the dedicated hardware cursor is
immune because it has some special bypass logic.

Specifically, you'd usually expect blending between two planes' pixels
to give you a final color value of

        bottom * (1.0-alpha) + top * alpha

However due to the way the alpha values are interpreted in the hardware,
there's a problematic case when the top plane's pixel alpha is 0.  You
wind up getting

        bottom * (255/256) + top * 0  = .996 * bottom

meaning pixels with a fully transparent plane above them are very
slightly fainter than pixels that didn't go through blending.  You'll
pretty much only perceive the difference when you have transparent
pixels at the edge of a plane (and even then only if you have a good
monitor and good eyes).  Of course "transparent pixels at the edge of
plane" is pretty common when you're using the plane as a mouse pointer.
You wind up with a faint ghost rectangle around your mouse pointer. :-(


Matt

> 
> Thanks again,
> Emil

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-28 17:37           ` Matt Roper
@ 2019-06-28 18:54             ` Emil Velikov
  2019-06-28 23:20               ` Matt Roper
  0 siblings, 1 reply; 9+ messages in thread
From: Emil Velikov @ 2019-06-28 18:54 UTC (permalink / raw)
  To: Matt Roper
  Cc: Intel Graphics Development, Miguel Casas-Sanchez, ML dri-devel,
	Alexandros Frantzis

On 2019/06/28, Matt Roper wrote:
> On Fri, Jun 28, 2019 at 05:14:51PM +0100, Emil Velikov wrote:
> > Hi Matt,
> > 
> > Thanks for the enlightening input :-)
> > 
> > On 2019/06/25, Matt Roper wrote:
> > 
> > > PLANE_CURSOR is basically just an indication that that specific plane is
> > > the one that's also hooked up to the legacy cursor ioctls; like Ville
> > > says, it shouldn't directly indicate that the plane is less
> > > feature-capable than other planes.  You can either detect the true
> > > capabilities of the cursor plane by checking for the presence/absence of
> > > other plane properties and/or experimenting with atomic TEST_ONLY
> > > commits to see what's really possible.
> > > 
> > Interesting, my understanding was the plane type was a hint about the
> > capabilities. Although yes, userspace must check via TEST_ONLY to ensure
> > the properties chosen will work.
> > 
> > 
> > > The ideal solution for Intel gen9 hardware would have been to just never
> > > have the driver advertise or program the dedicated hardware cursor at
> > > all, but to instead expose the top-most universal plane to userspace,
> > > describe it as PLANE_CURSOR, and route the legacy cursor ioctl's to that
> > > plane instead.  That would allow legacy cursor behavior to work as
> > > usual, but would also allow atomic userspace to use the plane in a more
> > > full-featured manner.  I wrote patches to do exactly this a couple years
> > > ago, but sadly we discovered that the universal planes on gen9 have a
> > > slight alpha blending defect that the dedicated hardware cursor does not
> > > exhibit.  Thus replacing the hardware cursor with the topmost universal
> > > plane led to a slight regression for existing users and we had to scrap
> > > the whole idea.  :-(
> > > 
> > > For reference, the relevant patch from a few years ago is here:
> > >         https://patchwork.kernel.org/patch/9398571/
> > > 
> > In that thread you mention:
> > 
> > "... 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."
> > 
> > Yet above you mention that universal planes have alpha blending defect.
> > Did you confirm that with HW/simulation teams or is that based on the
> > documentation? I would love to read a bit more on the topic.
> > 
> > In particular, but not limited to, if this defect is applicable only for
> > plane3 or literally all universal planes.
> 
> We only figured out exactly what was going on a while after I wrote that
> message in the thread, but we did ultimately confirm the problem with
> the hardware architects.  Sadly, all of the gen9 universal planes suffer
> from the slight blending issue and only the dedicated hardware cursor is
> immune because it has some special bypass logic.
> 
> Specifically, you'd usually expect blending between two planes' pixels
> to give you a final color value of
> 
>         bottom * (1.0-alpha) + top * alpha
> 
> However due to the way the alpha values are interpreted in the hardware,
> there's a problematic case when the top plane's pixel alpha is 0.  You
> wind up getting
> 
>         bottom * (255/256) + top * 0  = .996 * bottom
> 
> meaning pixels with a fully transparent plane above them are very
> slightly fainter than pixels that didn't go through blending.  You'll
> pretty much only perceive the difference when you have transparent
> pixels at the edge of a plane (and even then only if you have a good
> monitor and good eyes).  Of course "transparent pixels at the edge of
> plane" is pretty common when you're using the plane as a mouse pointer.
> You wind up with a faint ghost rectangle around your mouse pointer. :-(
> 
So we have a couple of problems here:
 - general faided bottom image - not much we can do here, also an issue
even w/o using plane3 as cursor.

 - ghosting - w/a: make sure the edge of a plane, is never within the
viewport (scanout region)

Since the fading is a generic problem, we might as well make use of
plane3 regardless.

What I'm proposing is that we pick up dimensions X and Y as cut-off.
Everything larger will use plane3, smaller - cursor.

To determine X/Y I see two approaches:
 - based off the max cursor size exposed by DRM drivers - say Zx larger
 - consider the current resolution +/- delta

We could also extrapolate that with the fact that most drivers expose
and hence user-space handle cursor identical width and height.

Personally, I'm inclined to pick 512x512 as a threshold and re-spin your
patches. Let me know what you think.

Thanks,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC] Exposing plane type mask and handling 'all' planes
  2019-06-28 18:54             ` Emil Velikov
@ 2019-06-28 23:20               ` Matt Roper
  0 siblings, 0 replies; 9+ messages in thread
From: Matt Roper @ 2019-06-28 23:20 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Intel Graphics Development, Miguel Casas-Sanchez, ML dri-devel,
	Alexandros Frantzis

On Fri, Jun 28, 2019 at 07:54:06PM +0100, Emil Velikov wrote:
> On 2019/06/28, Matt Roper wrote:
> > On Fri, Jun 28, 2019 at 05:14:51PM +0100, Emil Velikov wrote:
> > > Hi Matt,
> > > 
> > > Thanks for the enlightening input :-)
> > > 
> > > On 2019/06/25, Matt Roper wrote:
> > > 
> > > > PLANE_CURSOR is basically just an indication that that specific plane is
> > > > the one that's also hooked up to the legacy cursor ioctls; like Ville
> > > > says, it shouldn't directly indicate that the plane is less
> > > > feature-capable than other planes.  You can either detect the true
> > > > capabilities of the cursor plane by checking for the presence/absence of
> > > > other plane properties and/or experimenting with atomic TEST_ONLY
> > > > commits to see what's really possible.
> > > > 
> > > Interesting, my understanding was the plane type was a hint about the
> > > capabilities. Although yes, userspace must check via TEST_ONLY to ensure
> > > the properties chosen will work.
> > > 
> > > 
> > > > The ideal solution for Intel gen9 hardware would have been to just never
> > > > have the driver advertise or program the dedicated hardware cursor at
> > > > all, but to instead expose the top-most universal plane to userspace,
> > > > describe it as PLANE_CURSOR, and route the legacy cursor ioctl's to that
> > > > plane instead.  That would allow legacy cursor behavior to work as
> > > > usual, but would also allow atomic userspace to use the plane in a more
> > > > full-featured manner.  I wrote patches to do exactly this a couple years
> > > > ago, but sadly we discovered that the universal planes on gen9 have a
> > > > slight alpha blending defect that the dedicated hardware cursor does not
> > > > exhibit.  Thus replacing the hardware cursor with the topmost universal
> > > > plane led to a slight regression for existing users and we had to scrap
> > > > the whole idea.  :-(
> > > > 
> > > > For reference, the relevant patch from a few years ago is here:
> > > >         https://patchwork.kernel.org/patch/9398571/
> > > > 
> > > In that thread you mention:
> > > 
> > > "... 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."
> > > 
> > > Yet above you mention that universal planes have alpha blending defect.
> > > Did you confirm that with HW/simulation teams or is that based on the
> > > documentation? I would love to read a bit more on the topic.
> > > 
> > > In particular, but not limited to, if this defect is applicable only for
> > > plane3 or literally all universal planes.
> > 
> > We only figured out exactly what was going on a while after I wrote that
> > message in the thread, but we did ultimately confirm the problem with
> > the hardware architects.  Sadly, all of the gen9 universal planes suffer
> > from the slight blending issue and only the dedicated hardware cursor is
> > immune because it has some special bypass logic.
> > 
> > Specifically, you'd usually expect blending between two planes' pixels
> > to give you a final color value of
> > 
> >         bottom * (1.0-alpha) + top * alpha
> > 
> > However due to the way the alpha values are interpreted in the hardware,
> > there's a problematic case when the top plane's pixel alpha is 0.  You
> > wind up getting
> > 
> >         bottom * (255/256) + top * 0  = .996 * bottom
> > 
> > meaning pixels with a fully transparent plane above them are very
> > slightly fainter than pixels that didn't go through blending.  You'll
> > pretty much only perceive the difference when you have transparent
> > pixels at the edge of a plane (and even then only if you have a good
> > monitor and good eyes).  Of course "transparent pixels at the edge of
> > plane" is pretty common when you're using the plane as a mouse pointer.
> > You wind up with a faint ghost rectangle around your mouse pointer. :-(
> > 
> So we have a couple of problems here:
>  - general faided bottom image - not much we can do here, also an issue
> even w/o using plane3 as cursor.
> 
>  - ghosting - w/a: make sure the edge of a plane, is never within the
> viewport (scanout region)
> 
> Since the fading is a generic problem, we might as well make use of
> plane3 regardless.
> 
> What I'm proposing is that we pick up dimensions X and Y as cut-off.
> Everything larger will use plane3, smaller - cursor.
> 
> To determine X/Y I see two approaches:
>  - based off the max cursor size exposed by DRM drivers - say Zx larger
>  - consider the current resolution +/- delta
> 
> We could also extrapolate that with the fact that most drivers expose
> and hence user-space handle cursor identical width and height.
> 
> Personally, I'm inclined to pick 512x512 as a threshold and re-spin your
> patches. Let me know what you think.

It's definitely possible and something some of us have discussed in the
past.  However it's also a little more complicated code-wise than it
appears at first glance since you'll have to be pretty careful with the
handling of the watermark code.  There's a lot of special case branches
in watermark and DDB handling that would need to be updated to act
differently according to which hardware plane is actually backing your
uapi cursor plane on any given frame.  I.e., some of the skl_* functions
in intel_pm.c are the ones that probably need extra attention.  Changing
the code to never use the hardware cursor was easy (and removed a lot of
special cases), but changing the code to sometimes use the hardware
cursor and sometimes not for the same uapi plane is more complicated.
It's certainly not an insurmountable problem, but would just require
some extra effort and careful code review to make sure no conditions
were missed.


Matt

> 
> Thanks,
> Emil

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-06-28 23:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19 16:03 [RFC] Exposing plane type mask and handling 'all' planes Emil Velikov
2019-06-19 16:33 ` Ville Syrjälä
2019-06-19 17:49   ` Emil Velikov
2019-06-19 18:24     ` Ville Syrjälä
2019-06-26  0:46       ` Matt Roper
2019-06-28 16:14         ` Emil Velikov
2019-06-28 17:37           ` Matt Roper
2019-06-28 18:54             ` Emil Velikov
2019-06-28 23:20               ` Matt Roper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.