| Message ID | 20190930224707.14904-1-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | CRTC background color | expand |
On Wed, Oct 9, 2019 at 1:34 PM Matt Roper <matthew.d.roper@intel.com> wrote: > > The previous version of this series was posted in February here: > https://lists.freedesktop.org/archives/dri-devel/2019-February/208068.html > > Right before we merged this in February Maarten noticed that we should > be setting up the initial property state in a CRTC reset function (which > didn't exist yet) instead of when the property was attached. Maarten > landed the CRTC reset functionality a week or two later, but I was busy > with travel and other work at the time, so revisiting and rebasing this > background color series fell through the cracks and I'm just getting > back to it now. > > Userspace consumer is chromeos; these are the links the ChromeOS folks > gave me back in February: > https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 Please note that the current state of the background color on Chrome OS side is that while the property plumbing landed, the property is currently not used by the compositor and no one is working on making that happen. The kernel patches have not landed on the ChromeOS kernel either. > > > IGT is still the same as posted in February: > https://lists.freedesktop.org/archives/igt-dev/2019-February/009637.html > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Matt Roper (3): > drm: Add CRTC background color property > drm/i915/gen9+: Add support for pipe background color > drm/i915: Add background color hardware readout and state check > > drivers/gpu/drm/drm_atomic_state_helper.c | 4 +- > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ > drivers/gpu/drm/drm_blend.c | 35 +++++++++++++-- > drivers/gpu/drm/drm_mode_config.c | 6 +++ > drivers/gpu/drm/i915/display/intel_color.c | 11 +++-- > drivers/gpu/drm/i915/display/intel_display.c | 45 ++++++++++++++++++++ > drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++ > 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 ++++++++++++ > 11 files changed, 153 insertions(+), 7 deletions(-) > > -- > 2.21.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Oct 09, 2019 at 05:01:20PM -0400, Daniele Castagna wrote: > On Wed, Oct 9, 2019 at 1:34 PM Matt Roper <matthew.d.roper@intel.com> wrote: > > > > The previous version of this series was posted in February here: > > https://lists.freedesktop.org/archives/dri-devel/2019-February/208068.html > > > > Right before we merged this in February Maarten noticed that we should > > be setting up the initial property state in a CRTC reset function (which > > didn't exist yet) instead of when the property was attached. Maarten > > landed the CRTC reset functionality a week or two later, but I was busy > > with travel and other work at the time, so revisiting and rebasing this > > background color series fell through the cracks and I'm just getting > > back to it now. > > > > Userspace consumer is chromeos; these are the links the ChromeOS folks > > gave me back in February: > > https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 > > Please note that the current state of the background color on Chrome > OS side is that while the property plumbing landed, the property is > currently not used by the compositor and no one is working on making > that happen. Hmm, in that case it sounds like we probably don't actually have enough userspace support yet to justify merging the kernel changes at this time. I'm not too familiar with Chrome OS' userspace stack; is the rest of the work to actually make use of ozone stuff in the links above a heavy lift? Is it something someone is likely to work on that once they're freed up from other tasks or is there just not enough benefit to justify the effort of utilizing it at the compositor level right now? Matt > The kernel patches have not landed on the ChromeOS kernel either. > > > > > > > > IGT is still the same as posted in February: > > https://lists.freedesktop.org/archives/igt-dev/2019-February/009637.html > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > Matt Roper (3): > > drm: Add CRTC background color property > > drm/i915/gen9+: Add support for pipe background color > > drm/i915: Add background color hardware readout and state check > > > > drivers/gpu/drm/drm_atomic_state_helper.c | 4 +- > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ > > drivers/gpu/drm/drm_blend.c | 35 +++++++++++++-- > > drivers/gpu/drm/drm_mode_config.c | 6 +++ > > drivers/gpu/drm/i915/display/intel_color.c | 11 +++-- > > drivers/gpu/drm/i915/display/intel_display.c | 45 ++++++++++++++++++++ > > drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++ > > 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 ++++++++++++ > > 11 files changed, 153 insertions(+), 7 deletions(-) > > > > -- > > 2.21.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Oct 09, 2019 at 02:27:41PM -0700, Matt Roper wrote: > On Wed, Oct 09, 2019 at 05:01:20PM -0400, Daniele Castagna wrote: > > On Wed, Oct 9, 2019 at 1:34 PM Matt Roper <matthew.d.roper@intel.com> wrote: > > > > > > The previous version of this series was posted in February here: > > > https://lists.freedesktop.org/archives/dri-devel/2019-February/208068.html > > > > > > Right before we merged this in February Maarten noticed that we should > > > be setting up the initial property state in a CRTC reset function (which > > > didn't exist yet) instead of when the property was attached. Maarten > > > landed the CRTC reset functionality a week or two later, but I was busy > > > with travel and other work at the time, so revisiting and rebasing this > > > background color series fell through the cracks and I'm just getting > > > back to it now. > > > > > > Userspace consumer is chromeos; these are the links the ChromeOS folks > > > gave me back in February: > > > https://chromium-review.googlesource.com/c/chromium/src/+/1278858 > > > https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 > > > > Please note that the current state of the background color on Chrome > > OS side is that while the property plumbing landed, the property is > > currently not used by the compositor and no one is working on making > > that happen. > > Hmm, in that case it sounds like we probably don't actually have enough > userspace support yet to justify merging the kernel changes at this > time. I'm not too familiar with Chrome OS' userspace stack; is the rest > of the work to actually make use of ozone stuff in the links above a > heavy lift? Is it something someone is likely to work on that once > they're freed up from other tasks or is there just not enough benefit to > justify the effort of utilizing it at the compositor level right now? > AFAIK, there are no plans for the Chrome team to spend time utilising this feature. If you look at the bug [1], there's no correspondance with CrOS and there is no clear usecase for the feature. The patches are basically just copy/paste of how other properties are surfaced and that's it. A few months later, we get more stub implementations [2]/[3] again with no feedback from the Chrome team on the bugs [4]/[5]. On the first review, Daniele asked if the submitter was going to finish the background work before adding more properties. The answer is that CRTC BG isn't seen on Chrome, so it's not useful on the platform. We should not have landed the crtc-bg stub in the first place, it's just dead code and it's clear from the comments in [2] that it's going to stay that way. So I think the best course of action is to revert this change from Chrome until we have a usecase for the feature which is blessed by the Chrome team and it is implemented fully. Going forward, we're going to keep a closer eye on the HW enablement in Chrome so as to avoid adding dead code to Chrome and to avoid skirting the spirit of the "opensource userspace" rule by just implementing getters/setters. Sean ps: As for the stubs referenced in [2] and [3], that's more of a Chrome matter. There are already other existing userspace implementations for these 2 features, so Chrome is not being used as an upstreaming vehicle for the kernel patches. [1] https://bugs.chromium.org/p/chromium/issues/detail?id=894259 [2] https://polymer2-chromium-review.googlesource.com/c/chromium/src/+/1504300 [3] https://polymer2-chromium-review.googlesource.com/c/chromium/src/+/1572247 [4] https://bugs.chromium.org/p/chromium/issues/detail?id=940683 [5] https://bugs.chromium.org/p/chromium/issues/detail?id=938536 > > Matt > > > The kernel patches have not landed on the ChromeOS kernel either. > > > > > > > > > > > > > IGT is still the same as posted in February: > > > https://lists.freedesktop.org/archives/igt-dev/2019-February/009637.html > > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > Matt Roper (3): > > > drm: Add CRTC background color property > > > drm/i915/gen9+: Add support for pipe background color > > > drm/i915: Add background color hardware readout and state check > > > > > > drivers/gpu/drm/drm_atomic_state_helper.c | 4 +- > > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ > > > drivers/gpu/drm/drm_blend.c | 35 +++++++++++++-- > > > drivers/gpu/drm/drm_mode_config.c | 6 +++ > > > drivers/gpu/drm/i915/display/intel_color.c | 11 +++-- > > > drivers/gpu/drm/i915/display/intel_display.c | 45 ++++++++++++++++++++ > > > drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++ > > > 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 ++++++++++++ > > > 11 files changed, 153 insertions(+), 7 deletions(-) > > > > > > -- > > > 2.21.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Matt Roper > Graphics Software Engineer > VTT-OSGC Platform Enablement > Intel Corporation > (916) 356-2795 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
The previous version of this series was posted in February here: https://lists.freedesktop.org/archives/dri-devel/2019-February/208068.html Right before we merged this in February Maarten noticed that we should be setting up the initial property state in a CRTC reset function (which didn't exist yet) instead of when the property was attached. Maarten landed the CRTC reset functionality a week or two later, but I was busy with travel and other work at the time, so revisiting and rebasing this background color series fell through the cracks and I'm just getting back to it now. Userspace consumer is chromeos; these are the links the ChromeOS folks gave me back in February: https://chromium-review.googlesource.com/c/chromium/src/+/1278858 https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436 IGT is still the same as posted in February: https://lists.freedesktop.org/archives/igt-dev/2019-February/009637.html Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Matt Roper (3): drm: Add CRTC background color property drm/i915/gen9+: Add support for pipe background color drm/i915: Add background color hardware readout and state check drivers/gpu/drm/drm_atomic_state_helper.c | 4 +- drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ drivers/gpu/drm/drm_blend.c | 35 +++++++++++++-- drivers/gpu/drm/drm_mode_config.c | 6 +++ drivers/gpu/drm/i915/display/intel_color.c | 11 +++-- drivers/gpu/drm/i915/display/intel_display.c | 45 ++++++++++++++++++++ drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++ 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 ++++++++++++ 11 files changed, 153 insertions(+), 7 deletions(-)