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. 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: [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@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: [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@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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
