Re: [PATCH v3 0/3] CRTC background color

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 28, 2018 at 6:19 PM Matt Roper <matthew.d.roper@xxxxxxxxx> wrote:
>
> On Fri, Dec 28, 2018 at 06:14:40PM +0100, Daniel Vetter wrote:
> > On Fri, Dec 28, 2018 at 5:35 PM Matt Roper <matthew.d.roper@xxxxxxxxx> 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@xxxxxxxxxxxx>
> > > > geschrieben:
> > > > > On Thu, Dec 27, 2018 at 4:45 PM Matt Roper <matthew.d.roper@xxxxxxxxx>
> > > > > 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@xxxxxxxxx> 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
>
> I think the only problem would be if we come across hardware that has a
> non-programmable (always black) background colors that also doesn't pass
> its background black through the gamma/csc pipeline.

That just sounds broken. I guess in that case you don't get to use
both the color mgmt pipeline and have a non-fullscreen primary plane.
Mea culpa, build better hw :-)
-Daniel

>
>
> Matt
>
> >
> > >
> > >
> > > 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@xxxxxxxxxxxx]
> > > > > > > > Sent: Thursday, December 27, 2018 3:27 PM
> > > > > > > > To: Roper, Matthew D <matthew.d.roper@xxxxxxxxx>
> > > > > > > > Cc: intel-gfx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; Li, Wei C <
> > > > > wei.c.li@xxxxxxxxx>; dri-devel <dri-devel@xxxxxxxxxxxxxxxxxxxxx>
> > > > > > > > Subject: Re:  [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@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
> > > > > > > > > Cc: Wei C Li <wei.c.li@xxxxxxxxx>
> > > > > > > > > Cc: Sean Paul <sean@xxxxxxxxxx>
> > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > > > > > > >
> > > > > > > > > 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@xxxxxxxxxxxxxxxxxxxxx
> > > > > > > > > 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@xxxxxxxxxxxxxxxxxxxxx
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > >
> > >
> > > >    Am Fr., 28. Dez. 2018, 02:09 hat Stéphane Marchesin
> > > >    <[1]marcheu@xxxxxxxxxxxx> geschrieben:
> > > >
> > > >      On Thu, Dec 27, 2018 at 4:45 PM Matt Roper
> > > >      <[2]matthew.d.roper@xxxxxxxxx> 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@xxxxxxxxx>
> > > >      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@xxxxxxxxxxxx]
> > > >      > > > Sent: Thursday, December 27, 2018 3:27 PM
> > > >      > > > To: Roper, Matthew D <[6]matthew.d.roper@xxxxxxxxx>
> > > >      > > > Cc: intel-gfx <[7]intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; Li, Wei C
> > > >      <[8]wei.c.li@xxxxxxxxx>; dri-devel <[9]dri-devel@xxxxxxxxxxxxxxxxxxxxx>
> > > >      > > > Subject: Re:  [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@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
> > > >      > > > > Cc: Wei C Li <[13]wei.c.li@xxxxxxxxx>
> > > >      > > > > Cc: Sean Paul <sean@xxxxxxxxxx>
> > > >      > > > > Cc: Ville Syrjälä <[14]ville.syrjala@xxxxxxxxxxxxxxx>
> > > >      > > > >
> > > >      > > > > 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@xxxxxxxxxxxxxxxxxxxxx
> > > >      > > > > [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@xxxxxxxxxxxxxxxxxxxxx
> > > >      [18]https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > >
> > > > References
> > > >
> > > >    Visible links
> > > >    1. mailto:marcheu@xxxxxxxxxxxx
> > > >    2. mailto:matthew.d.roper@xxxxxxxxx
> > > >    3. mailto:wei.c.li@xxxxxxxxx
> > > >    4. https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1387366
> > > >    5. mailto:marcheu@xxxxxxxxxxxx
> > > >    6. mailto:matthew.d.roper@xxxxxxxxx
> > > >    7. mailto:intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > >    8. mailto:wei.c.li@xxxxxxxxx
> > > >    9. mailto:dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > > >   10. mailto:matthew.d.roper@xxxxxxxxx
> > > >   11. https://lists.freedesktop.org/archives/intel-gfx/2018-November/181777
> > > >   12. mailto:dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > > >   13. mailto:wei.c.li@xxxxxxxxx
> > > >   14. mailto:ville.syrjala@xxxxxxxxxxxxxxx
> > > >   15. mailto:Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > >   16. https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > >   17. mailto:dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > > >   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
>
> --
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux