[Intel-gfx] [PATCH v3 0/3] CRTC background color

Daniel Vetter daniel.vetter at ffwll.ch
Fri Dec 28 17:14:40 UTC 2018


On Fri, Dec 28, 2018 at 5:35 PM Matt Roper <matthew.d.roper at intel.com> wrote:
>
> On Fri, Dec 28, 2018 at 12:53:29PM +0100, Daniel Vetter wrote:
> > Am Fr., 28. Dez. 2018, 02:09 hat Stéphane Marchesin <marcheu at chromium.org>
> > geschrieben:
> > > On Thu, Dec 27, 2018 at 4:45 PM Matt Roper <matthew.d.roper at intel.com>
> > > wrote:
> > > >
> > > > On Thu, Dec 27, 2018 at 04:22:28PM -0800, Stéphane Marchesin wrote:
> > > > > On Thu, Dec 27, 2018 at 3:49 PM Li, Wei C <wei.c.li at intel.com> wrote:
> > > > > >
> > > > > > Matt,
> > > > > >
> > > > > > Is that possible for you to get some time to review
> > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1387366
> > > and confirm the FROMLIST change if it looks good to you so we could move
> > > forward?
> > > > > >
> > > > >
> > > > > To be more precise, I am trying to assess what's needed before this
> > > > > patch goes usptream, and in particular, I'd like to know if we are
> > > > > blocked on any Chrome-side thing.
> > > > >
> > > > > Stéphane
> > > > >
> > > >
> > > > Hi Stéphane.  On the Chrome side of things, I believe we need an Ack
> > > > from the ChromeOS userspace team on the ABI, plus a pointer to where the
> > > > final, reviewed userspace patches are that correspond to it (mailing
> > > > list link or gerrit or whatever).  I have an old gerrit link to some
> > > > in-development chromium/ozone patches for this from November 2nd, but
> > > > I'm not sure if there's a newer one now.
> > >
> > > IMO from user space the ABI is good. I think the question is more
> > > whether other hw would be fine with it. For example if we notice that
> > > other platforms can only do black as a background color, how can we
> > > make this API standard. Hopefully other folks can chime in here about
> > > other hw capabilities.
> >
> > black background is already the default, so "no prop" = "black
> > background". The problem is a bit figuring out whether you can make the
> > primary plane non-fullscreen, which atm is a TEST_ONLY guessing game for
> > userspace. We've discussed some "can_scale" and "can_position" properties
> > for that, but aside from reworking the helpers to check plane updates
> > nothing happened.
> > -Daniel
>
> Based on feedback from Brian and Eric, it sounds like the thing we need
> to get better clarity on is whether all hardware can apply pipe-level
> degamma/csc/gamma to the background color the same way it does to plane
> content (even for platforms where the background color is always a
> non-programmable black).
>
> My initial assumption (which I'll clarify in the kerneldoc) is that
> background color should be treated the same way as a plane filled with
> that solid color; i.e., it should go through the same pipe-level color
> transformations.  Based on Eric's email, it sounds like Raspberry Pi
> matches my assumption.  For Intel hardware, we have register bits that
> specify whether color management should be applied to the background
> color or not (they're not set today for gen9+, which I consider that a
> bug since it seems like inconsistent behavior; the first patch of my
> series addresses that).
>
> If there are any platforms that don't/can't apply degamma/csc/gamma to
> the background color (regardless of whether that background color is
> programmable or always fixed at black), we should probably find some way
> to make that known to userspace (maybe some extra immutable property?).

Nah, I'd say since most hw seems to have the background color before
the CRTC degamma/ctm/gamma stuff we can just require that the odd one
out manually feed the background color through the color stuff before
writing it to hardware. And updating it every time the gamma tables
change again ofc. We could even do a little helper for this and store
the post-color_mgmt background in the crtc_state. Offloading that to
userspace seems silly to me.
-Daniel

>
>
> Matt
>
> > >
> > > Stéphane
> > >
> > >
> > > >
> > > > Aside from satisfying the ABI/userspace requirements, we still need all
> > > > the patches to get reviewed by the upstream kernel devs.  An older
> > > > version of patch #2 has a r-b from Sean, but patches 1 and 3 haven't
> > > > been accepted yet.  Actually it looks like I never sent the latest
> > > > version of my series that incorporates some additional feedback from
> > > > Brian Starkey to the list; I'll do that tomorrow.  I think a lot of
> > > > people are still out on holiday vacation at the moment, so if necessary
> > > > I'll start pinging IRC for reviews in a week or two when people are back
> > > > in office.
> > >
> >
> >
> > >
> > > >
> > > > Matt
> > > >
> > > > >
> > > > > > BTW, I've backported your kernel patch into Chrome OS and verified
> > > it using the TEST=drm-tests/atomictest -t crtc_background_color on a Google
> > > Pixelbook with KBL graphics.
> > > > > >
> > > > > > Thanks,
> > > > > > Wei
> > > > > >
> > > > > > -----Original Message-----
> > > > > > From: Stéphane Marchesin [mailto:marcheu at chromium.org]
> > > > > > Sent: Thursday, December 27, 2018 3:27 PM
> > > > > > To: Roper, Matthew D <matthew.d.roper at intel.com>
> > > > > > Cc: intel-gfx <intel-gfx at lists.freedesktop.org>; Li, Wei C <
> > > wei.c.li at intel.com>; dri-devel <dri-devel at lists.freedesktop.org>
> > > > > > Subject: Re: [Intel-gfx] [PATCH v3 0/3] CRTC background color
> > > > > >
> > > > > > Hey,
> > > > > >
> > > > > > Is there anything missing on the Chrome side to move forward with
> > > this series?
> > > > > >
> > > > > > Stéphane
> > > > > >
> > > > > > On Thu, Nov 15, 2018 at 2:14 PM Matt Roper <
> > > matthew.d.roper at intel.com> wrote:
> > > > > > >
> > > > > > > Third version of the series previously posted here:
> > > > > > >
> > > > > > >
> > > https://lists.freedesktop.org/archives/intel-gfx/2018-November/181777.
> > > > > > > html
> > > > > > >
> > > > > > > This version incorporates review feedback from Ville and Sean Paul.
> > > > > > >
> > > > > > > The first patch here can be merged whenever it receives review
> > > approval.
> > > > > > > The second and third patches still need to wait for opensource
> > > > > > > userspace to be ready before merging (there's ChromeOS work
> > > underway).
> > > > > > >
> > > > > > > Cc: dri-devel at lists.freedesktop.org
> > > > > > > Cc: Wei C Li <wei.c.li at intel.com>
> > > > > > > Cc: Sean Paul <sean at poorly.run>
> > > > > > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > > >
> > > > > > > Matt Roper (3):
> > > > > > >   drm/i915: Force background color to black for gen9+ (v2)
> > > > > > >   drm: Add CRTC background color property (v2)
> > > > > > >   drm/i915/gen9+: Add support for pipe background color (v3)
> > > > > > >
> > > > > > >  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
> > > > > > >  drivers/gpu/drm/drm_atomic_uapi.c         |  5 ++++
> > > > > > >  drivers/gpu/drm/drm_blend.c               | 21 +++++++++++++---
> > > > > > >  drivers/gpu/drm/drm_mode_config.c         |  6 +++++
> > > > > > >  drivers/gpu/drm/i915/i915_debugfs.c       |  9 +++++++
> > > > > > >  drivers/gpu/drm/i915/i915_reg.h           |  6 +++++
> > > > > > >  drivers/gpu/drm/i915/intel_display.c      | 40
> > > +++++++++++++++++++++++++++++++
> > > > > > >  include/drm/drm_blend.h                   |  1 +
> > > > > > >  include/drm/drm_crtc.h                    | 17 +++++++++++++
> > > > > > >  include/drm/drm_mode_config.h             |  5 ++++
> > > > > > >  include/uapi/drm/drm_mode.h               | 28
> > > ++++++++++++++++++++++
> > > > > > >  11 files changed, 136 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > --
> > > > > > > 2.14.4
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > Intel-gfx mailing list
> > > > > > > Intel-gfx at lists.freedesktop.org
> > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > >
> > > > --
> > > > Matt Roper
> > > > Graphics Software Engineer
> > > > IoTG Platform Enabling & Development
> > > > Intel Corporation
> > > > (916) 356-2795
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
>
> >    Am Fr., 28. Dez. 2018, 02:09 hat Stéphane Marchesin
> >    <[1]marcheu at chromium.org> geschrieben:
> >
> >      On Thu, Dec 27, 2018 at 4:45 PM Matt Roper
> >      <[2]matthew.d.roper at intel.com> wrote:
> >      >
> >      > On Thu, Dec 27, 2018 at 04:22:28PM -0800, Stéphane Marchesin wrote:
> >      > > On Thu, Dec 27, 2018 at 3:49 PM Li, Wei C <[3]wei.c.li at intel.com>
> >      wrote:
> >      > > >
> >      > > > Matt,
> >      > > >
> >      > > > Is that possible for you to get some time to review
> >      [4]https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1387366
> >      and confirm the FROMLIST change if it looks good to you so we could move
> >      forward?
> >      > > >
> >      > >
> >      > > To be more precise, I am trying to assess what's needed before this
> >      > > patch goes usptream, and in particular, I'd like to know if we are
> >      > > blocked on any Chrome-side thing.
> >      > >
> >      > > Stéphane
> >      > >
> >      >
> >      > Hi Stéphane.  On the Chrome side of things, I believe we need an Ack
> >      > from the ChromeOS userspace team on the ABI, plus a pointer to where
> >      the
> >      > final, reviewed userspace patches are that correspond to it (mailing
> >      > list link or gerrit or whatever).  I have an old gerrit link to some
> >      > in-development chromium/ozone patches for this from November 2nd, but
> >      > I'm not sure if there's a newer one now.
> >
> >      IMO from user space the ABI is good. I think the question is more
> >      whether other hw would be fine with it. For example if we notice that
> >      other platforms can only do black as a background color, how can we
> >      make this API standard. Hopefully other folks can chime in here about
> >      other hw capabilities.
> >
> >      Stéphane
> >
> >      >
> >      > Aside from satisfying the ABI/userspace requirements, we still need
> >      all
> >      > the patches to get reviewed by the upstream kernel devs.  An older
> >      > version of patch #2 has a r-b from Sean, but patches 1 and 3 haven't
> >      > been accepted yet.  Actually it looks like I never sent the latest
> >      > version of my series that incorporates some additional feedback from
> >      > Brian Starkey to the list; I'll do that tomorrow.  I think a lot of
> >      > people are still out on holiday vacation at the moment, so if
> >      necessary
> >      > I'll start pinging IRC for reviews in a week or two when people are
> >      back
> >      > in office.
> >
> >      >
> >      >
> >      > Matt
> >      >
> >      > >
> >      > > > BTW, I've backported your kernel patch into Chrome OS and verified
> >      it using the TEST=drm-tests/atomictest -t crtc_background_color on a
> >      Google Pixelbook with KBL graphics.
> >      > > >
> >      > > > Thanks,
> >      > > > Wei
> >      > > >
> >      > > > -----Original Message-----
> >      > > > From: Stéphane Marchesin [mailto:[5]marcheu at chromium.org]
> >      > > > Sent: Thursday, December 27, 2018 3:27 PM
> >      > > > To: Roper, Matthew D <[6]matthew.d.roper at intel.com>
> >      > > > Cc: intel-gfx <[7]intel-gfx at lists.freedesktop.org>; Li, Wei C
> >      <[8]wei.c.li at intel.com>; dri-devel <[9]dri-devel at lists.freedesktop.org>
> >      > > > Subject: Re: [Intel-gfx] [PATCH v3 0/3] CRTC background color
> >      > > >
> >      > > > Hey,
> >      > > >
> >      > > > Is there anything missing on the Chrome side to move forward with
> >      this series?
> >      > > >
> >      > > > Stéphane
> >      > > >
> >      > > > On Thu, Nov 15, 2018 at 2:14 PM Matt Roper
> >      <[10]matthew.d.roper at intel.com> wrote:
> >      > > > >
> >      > > > > Third version of the series previously posted here:
> >      > > > >
> >      > > > >
> >      [11]https://lists.freedesktop.org/archives/intel-gfx/2018-November/181777.
> >      > > > > html
> >      > > > >
> >      > > > > This version incorporates review feedback from Ville and Sean
> >      Paul.
> >      > > > >
> >      > > > > The first patch here can be merged whenever it receives review
> >      approval.
> >      > > > > The second and third patches still need to wait for opensource
> >      > > > > userspace to be ready before merging (there's ChromeOS work
> >      underway).
> >      > > > >
> >      > > > > Cc: [12]dri-devel at lists.freedesktop.org
> >      > > > > Cc: Wei C Li <[13]wei.c.li at intel.com>
> >      > > > > Cc: Sean Paul <sean at poorly.run>
> >      > > > > Cc: Ville Syrjälä <[14]ville.syrjala at linux.intel.com>
> >      > > > >
> >      > > > > Matt Roper (3):
> >      > > > >   drm/i915: Force background color to black for gen9+ (v2)
> >      > > > >   drm: Add CRTC background color property (v2)
> >      > > > >   drm/i915/gen9+: Add support for pipe background color (v3)
> >      > > > >
> >      > > > >  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
> >      > > > >  drivers/gpu/drm/drm_atomic_uapi.c         |  5 ++++
> >      > > > >  drivers/gpu/drm/drm_blend.c               | 21 +++++++++++++---
> >      > > > >  drivers/gpu/drm/drm_mode_config.c         |  6 +++++
> >      > > > >  drivers/gpu/drm/i915/i915_debugfs.c       |  9 +++++++
> >      > > > >  drivers/gpu/drm/i915/i915_reg.h           |  6 +++++
> >      > > > >  drivers/gpu/drm/i915/intel_display.c      | 40
> >      +++++++++++++++++++++++++++++++
> >      > > > >  include/drm/drm_blend.h                   |  1 +
> >      > > > >  include/drm/drm_crtc.h                    | 17 +++++++++++++
> >      > > > >  include/drm/drm_mode_config.h             |  5 ++++
> >      > > > >  include/uapi/drm/drm_mode.h               | 28
> >      ++++++++++++++++++++++
> >      > > > >  11 files changed, 136 insertions(+), 3 deletions(-)
> >      > > > >
> >      > > > > --
> >      > > > > 2.14.4
> >      > > > >
> >      > > > > _______________________________________________
> >      > > > > Intel-gfx mailing list
> >      > > > > [15]Intel-gfx at lists.freedesktop.org
> >      > > > > [16]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >      >
> >      > --
> >      > Matt Roper
> >      > Graphics Software Engineer
> >      > IoTG Platform Enabling & Development
> >      > Intel Corporation
> >      > (916) 356-2795
> >      _______________________________________________
> >      dri-devel mailing list
> >      [17]dri-devel at lists.freedesktop.org
> >      [18]https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > References
> >
> >    Visible links
> >    1. mailto:marcheu at chromium.org
> >    2. mailto:matthew.d.roper at intel.com
> >    3. mailto:wei.c.li at intel.com
> >    4. https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1387366
> >    5. mailto:marcheu at chromium.org
> >    6. mailto:matthew.d.roper at intel.com
> >    7. mailto:intel-gfx at lists.freedesktop.org
> >    8. mailto:wei.c.li at intel.com
> >    9. mailto:dri-devel at lists.freedesktop.org
> >   10. mailto:matthew.d.roper at intel.com
> >   11. https://lists.freedesktop.org/archives/intel-gfx/2018-November/181777
> >   12. mailto:dri-devel at lists.freedesktop.org
> >   13. mailto:wei.c.li at intel.com
> >   14. mailto:ville.syrjala at linux.intel.com
> >   15. mailto:Intel-gfx at lists.freedesktop.org
> >   16. https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >   17. mailto:dri-devel at lists.freedesktop.org
> >   18. https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


More information about the Intel-gfx mailing list