Re: [Intel-gfx] [PATCH v7 0/3] CRTC background color

[Sean Paul <sean@poorly.run>]

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

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel