| Message ID | 20190130185122.10322-1-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | CRTC background color | expand |
On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote: > Previous patch series was here: > https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html > > I'm told the ChromeOS userspace code to make use of the background color > has been reviewed and is ready for use: > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 Woops, the second link here should have been to https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 which I believe is some unit tests to go along with the main userspace code. Matt > > So I think ABI-wise we've met the userspace consumer requirements to > upstream this; we just need to get reviews on the two i915-specific > patches and a clean CI report. > > v4.1 is identical to v4 aside from a rebase onto the latest drm-tip. > > > Matt Roper (3): > drm/i915: Force background color to black for gen9+ (v2) > drm: Add CRTC background color property (v4) > drm/i915/gen9+: Add support for pipe background color (v4) > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- > 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 | 43 ++++++++++++++++++++++++++++++++++++ > include/drm/drm_blend.h | 1 + > include/drm/drm_crtc.h | 12 ++++++++++ > include/drm/drm_mode_config.h | 5 +++++ > include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ > 10 files changed, 138 insertions(+), 3 deletions(-) > > -- > 2.14.5 >
On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote: > On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote: > > Previous patch series was here: > > https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html > > > > I'm told the ChromeOS userspace code to make use of the background color > > has been reviewed and is ready for use: > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > Woops, the second link here should have been to > > https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 > > which I believe is some unit tests to go along with the main userspace > code. Do we have this as igts too? -Daniel > > > Matt > > > > > So I think ABI-wise we've met the userspace consumer requirements to > > upstream this; we just need to get reviews on the two i915-specific > > patches and a clean CI report. > > > > v4.1 is identical to v4 aside from a rebase onto the latest drm-tip. > > > > > > Matt Roper (3): > > drm/i915: Force background color to black for gen9+ (v2) > > drm: Add CRTC background color property (v4) > > drm/i915/gen9+: Add support for pipe background color (v4) > > > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > > drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- > > 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 | 43 ++++++++++++++++++++++++++++++++++++ > > include/drm/drm_blend.h | 1 + > > include/drm/drm_crtc.h | 12 ++++++++++ > > include/drm/drm_mode_config.h | 5 +++++ > > include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ > > 10 files changed, 138 insertions(+), 3 deletions(-) > > > > -- > > 2.14.5 > > > > -- > 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
On Wed, Jan 30, 2019 at 09:57:10PM +0100, Daniel Vetter wrote: > On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote: > > On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote: > > > Previous patch series was here: > > > https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html > > > > > > I'm told the ChromeOS userspace code to make use of the background color > > > has been reviewed and is ready for use: > > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > > > Woops, the second link here should have been to > > > > https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 > > > > which I believe is some unit tests to go along with the main userspace > > code. > > Do we have this as igts too? > -Daniel Yeah, I posted it along with some of the earlier revisions of the series, but haven't reposted the latest copy lately. I'll check and see if it needs a rebase and then post it shortly. Unfortunately the IGT isn't as useful as I'd like it to be since the CRC's for a plane filled with a solid color never come up the same as a pure background color (at least on the APL platform I'm using). I can run the IGT in interactive mode and the colors seem identical to visual inspection, but the CRC values never agree, so I've disabled the CRC comparison in the test for now. I'm not sure if this is related to the other blending quirks of gen9; it will be interesting to see if the CRC's match on an Icelake or something. FWIW, I've mmap'd the Cairo-generated plane framebuffer and verified that it contains exactly the pixel values we'd expect (so it's not a matter of bad roundoff in Cairo), and I've tried flipping both 8bpc and 10bpc pixel formats (to match the background color's 10 bits per component), but nothing seems to make the CRC's match. :-( Matt > > > > > > > Matt > > > > > > > > So I think ABI-wise we've met the userspace consumer requirements to > > > upstream this; we just need to get reviews on the two i915-specific > > > patches and a clean CI report. > > > > > > v4.1 is identical to v4 aside from a rebase onto the latest drm-tip. > > > > > > > > > Matt Roper (3): > > > drm/i915: Force background color to black for gen9+ (v2) > > > drm: Add CRTC background color property (v4) > > > drm/i915/gen9+: Add support for pipe background color (v4) > > > > > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > > > drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- > > > 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 | 43 ++++++++++++++++++++++++++++++++++++ > > > include/drm/drm_blend.h | 1 + > > > include/drm/drm_crtc.h | 12 ++++++++++ > > > include/drm/drm_mode_config.h | 5 +++++ > > > include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ > > > 10 files changed, 138 insertions(+), 3 deletions(-) > > > > > > -- > > > 2.14.5 > > > > > > > -- > > 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 > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Wed, Jan 30, 2019 at 03:48:50PM -0800, Matt Roper wrote: > On Wed, Jan 30, 2019 at 09:57:10PM +0100, Daniel Vetter wrote: > > On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote: > > > On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote: > > > > Previous patch series was here: > > > > https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html > > > > > > > > I'm told the ChromeOS userspace code to make use of the background color > > > > has been reviewed and is ready for use: > > > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > > > > > Woops, the second link here should have been to > > > > > > https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 > > > > > > which I believe is some unit tests to go along with the main userspace > > > code. > > > > Do we have this as igts too? > > -Daniel > > Yeah, I posted it along with some of the earlier revisions of the > series, but haven't reposted the latest copy lately. I'll check and see > if it needs a rebase and then post it shortly. > > Unfortunately the IGT isn't as useful as I'd like it to be since the > CRC's for a plane filled with a solid color never come up the same as a > pure background color (at least on the APL platform I'm using). I can > run the IGT in interactive mode and the colors seem identical to visual > inspection, but the CRC values never agree, so I've disabled the CRC > comparison in the test for now. I'm not sure if this is related to the > other blending quirks of gen9; it will be interesting to see if the > CRC's match on an Icelake or something. Please don't do that, we need to know when stuff doesn't work. Also, igt (at least for more generic stuff like this) shouldn't be bent to exactly match intel hw bugs. And yes if the blending is generally broken on gen9 then I'd be surprised if they managed to not screw it up for the background color. Usually backgroun color works as if it's a separate additional plane that you blend the others with. -Daniel > FWIW, I've mmap'd the Cairo-generated plane framebuffer and verified > that it contains exactly the pixel values we'd expect (so it's not a > matter of bad roundoff in Cairo), and I've tried flipping both 8bpc and > 10bpc pixel formats (to match the background color's 10 bits per > component), but nothing seems to make the CRC's match. :-( > > > Matt > > > > > > > > > > > > Matt > > > > > > > > > > > So I think ABI-wise we've met the userspace consumer requirements to > > > > upstream this; we just need to get reviews on the two i915-specific > > > > patches and a clean CI report. > > > > > > > > v4.1 is identical to v4 aside from a rebase onto the latest drm-tip. > > > > > > > > > > > > Matt Roper (3): > > > > drm/i915: Force background color to black for gen9+ (v2) > > > > drm: Add CRTC background color property (v4) > > > > drm/i915/gen9+: Add support for pipe background color (v4) > > > > > > > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > > > > drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- > > > > 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 | 43 ++++++++++++++++++++++++++++++++++++ > > > > include/drm/drm_blend.h | 1 + > > > > include/drm/drm_crtc.h | 12 ++++++++++ > > > > include/drm/drm_mode_config.h | 5 +++++ > > > > include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ > > > > 10 files changed, 138 insertions(+), 3 deletions(-) > > > > > > > > -- > > > > 2.14.5 > > > > > > > > > > -- > > > 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 > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
On Fri, Feb 01, 2019 at 06:13:48PM +0100, Daniel Vetter wrote: > On Wed, Jan 30, 2019 at 03:48:50PM -0800, Matt Roper wrote: > > On Wed, Jan 30, 2019 at 09:57:10PM +0100, Daniel Vetter wrote: > > > On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote: > > > > On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote: > > > > > Previous patch series was here: > > > > > https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html > > > > > > > > > > I'm told the ChromeOS userspace code to make use of the background color > > > > > has been reviewed and is ready for use: > > > > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > > > > * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > > > > > > > Woops, the second link here should have been to > > > > > > > > https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 > > > > > > > > which I believe is some unit tests to go along with the main userspace > > > > code. > > > > > > Do we have this as igts too? > > > -Daniel > > > > Yeah, I posted it along with some of the earlier revisions of the > > series, but haven't reposted the latest copy lately. I'll check and see > > if it needs a rebase and then post it shortly. > > > > Unfortunately the IGT isn't as useful as I'd like it to be since the > > CRC's for a plane filled with a solid color never come up the same as a > > pure background color (at least on the APL platform I'm using). I can > > run the IGT in interactive mode and the colors seem identical to visual > > inspection, but the CRC values never agree, so I've disabled the CRC > > comparison in the test for now. I'm not sure if this is related to the > > other blending quirks of gen9; it will be interesting to see if the > > CRC's match on an Icelake or something. > > Please don't do that, we need to know when stuff doesn't work. Also, igt > (at least for more generic stuff like this) shouldn't be bent to exactly > match intel hw bugs. > > And yes if the blending is generally broken on gen9 then I'd be surprised > if they managed to not screw it up for the background color. Usually > backgroun color works as if it's a separate additional plane that you > blend the others with. > -Daniel +igt-dev list So looking at the bspec closer, it sounds like it might not actually be valid for us to take a DMUX (pipe) CRC of just the background color. The bspec for gen9+ states "The DMUX CRC should only be enabled when the pipe, and one or more planes in the pipe are enabled." which implies that just using the background color (which is always on) isn't sufficient. I'll uncomment the CRC check when we actually push the test so that it can still be used on non-Intel platforms that don't have this restriction. Should we just make the test skip on the relevant i915 platforms, or should we do something else to mark the test as an expected failure due to hardware? Matt > > > FWIW, I've mmap'd the Cairo-generated plane framebuffer and verified > > that it contains exactly the pixel values we'd expect (so it's not a > > matter of bad roundoff in Cairo), and I've tried flipping both 8bpc and > > 10bpc pixel formats (to match the background color's 10 bits per > > component), but nothing seems to make the CRC's match. :-( > > > > > > Matt > > > > > > > > > > > > > > > > > Matt > > > > > > > > > > > > > > So I think ABI-wise we've met the userspace consumer requirements to > > > > > upstream this; we just need to get reviews on the two i915-specific > > > > > patches and a clean CI report. > > > > > > > > > > v4.1 is identical to v4 aside from a rebase onto the latest drm-tip. > > > > > > > > > > > > > > > Matt Roper (3): > > > > > drm/i915: Force background color to black for gen9+ (v2) > > > > > drm: Add CRTC background color property (v4) > > > > > drm/i915/gen9+: Add support for pipe background color (v4) > > > > > > > > > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > > > > > drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- > > > > > 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 | 43 ++++++++++++++++++++++++++++++++++++ > > > > > include/drm/drm_blend.h | 1 + > > > > > include/drm/drm_crtc.h | 12 ++++++++++ > > > > > include/drm/drm_mode_config.h | 5 +++++ > > > > > include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ > > > > > 10 files changed, 138 insertions(+), 3 deletions(-) > > > > > > > > > > -- > > > > > 2.14.5 > > > > > > > > > > > > > -- > > > > 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 > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > > Matt Roper > > Graphics Software Engineer > > IoTG Platform Enabling & Development > > Intel Corporation > > (916) 356-2795 > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch