[Intel-gfx] [PATCH v4 3/7] drm: Add DisplayPort colorspace property

[Ilia Mirkin]

On Tue, Sep 10, 2019 at 9:21 AM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>
> On Tue, Sep 10, 2019 at 3:34 AM Mun, Gwan-gyeong
> <gwan-gyeong.mun at intel.com> wrote:
> >
> > On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote:
> > > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong
> > > <gwan-gyeong.mun at intel.com> wrote:
> > > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote:
> > > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä
> > > > > <ville.syrjala at linux.intel.com> wrote:
> > > > > > On Fri, Sep 06, 2019 at 11:31:55AM +0000, Shankar, Uma wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ilia Mirkin <imirkin at alum.mit.edu>
> > > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM
> > > > > > > > To: Mun, Gwan-gyeong <gwan-gyeong.mun at intel.com>
> > > > > > > > Cc: Intel Graphics Development <
> > > > > > > > intel-gfx at lists.freedesktop.org
> > > > > > > > > ; Shankar, Uma
> > > > > > > > <uma.shankar at intel.com>; dri-devel <
> > > > > > > > dri-devel at lists.freedesktop.org>
> > > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace
> > > > > > > > property
> > > > > > > >
> > > > > > > > So how would this work with a DP++ connector? Should it
> > > > > > > > list
> > > > > > > > the HDMI or DP
> > > > > > > > properties? Or do we need a custom property checker which
> > > > > > > > is
> > > > > > > > aware of what is
> > > > > > > > currently plugged in to validate the values?
> > > > > > >
> > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving
> > > > > > > DP
> > > > > > > or HDMI (with a passive dongle).
> > > > > > > Based on the type of sink detected, we should expose DP or
> > > > > > > HDMI
> > > > > > > colorspaces to userspace.
> > > > > >
> > > > > > For i915 DP connector always drives DP mode, HDMI connector
> > > > > > always
> > > > > > drives
> > > > > > HDMI mode, even when the physical connector is DP++.
> > > > >
> > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and
> > > > > amdgpu
> > > > > create 1 connector (not sure about other drivers) for a single
> > > > > physical DP++ socket. Since we supply the list of valid values at
> > > > > the
> > > > > time of creating the connector, we can't know at that point
> > > > > whether
> > > > > in
> > > > > the future a HDMI or DP will be plugged into it.
> > > > >
> > > > >   -ilia
> > > > Ilia, does it mean that the drm_connector type is
> > > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode?
> > >
> > > That is correct. The connector type is "DisplayPort" in such a case.
> > >
> > > Cheers,
> > >
> > >   -ilia
> >
> > For now drm_mode_create_colorspace_property() is only used for i915.
> > IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for
> > using of drm_mode_create_colorspace_property(),
> > what about do we add a variable which can identify DP++ and DP to
> > drm_connector?
> > And when the drivers (nouveau, radeon, and amdgpu) detect the current
> > protocol, the drivers will set the variable.
>
> I've been working on adding this to nouveau.
>
> Can/should such properties be added/removed at "runtime", rather than
> at connector creation time? Either way, the function
> drm_mode_create_colorspace_property as proposed would not be reusable,
> since it looks at the connector type, which will always be
> "DisplayPort" in such cases.

Summary of conversation Ville and I had on IRC:

 - DP++ connectors to provide a single combined list of options for colorspace
 - set_property hook will check against currently plugged in thing,
and reject incorrect values
 - in the case where someone sets e.g. an HDMI value for an
HDMI-plugged-in thing, and then unplugs, and then plugs in a DP
screen, the modeset should continue to succeed but use a default
colorspace value.

I think there was a bit of contention on that last point. Open to
opinions, but we should try to avoid putting undue burden on esp
non-atomic userspace.

Cheers,

  -ilia