Re: [v4 0/3] Add Colorspace connector property interface

[Brian Starkey <Brian.Starkey@arm.com>]

Hi,

On Tue, Nov 27, 2018 at 10:10:40PM +0530, Uma Shankar wrote:
> This patch series creates a new connector property to program
> colorspace to sink devices. Modern sink devices support more
> than 1 type of colorspace like 601, 709, BT2020 etc. This helps
> to switch based on content type which is to be displayed. The
> decision lies with compositors as to in which scenarios, a
> particular colorspace will be picked.
> 
> This will be helpful mostly to switch to higher gamut colorspaces
> like BT2020 when the media content is encoded as BT2020. Thereby
> giving a good visual experience to users.
> 
> The expectation from userspace is that it should parse the EDID
> and get supported colorspaces. Use this property and switch to the
> one supported. Kernel will not give the supported colorspaces since
> this is panel dependent and our current property infrastructure is
> not supporting it.

So is the problem here that we've no way to change the supported enum
values at runtime? Conceptually, do you think there's a problem with
the kernel only exposing the colorspaces that the sink supports (if
that were possible)? I'm wondering if changing the current property
infrastructure is better than punting the job to userspace to decode
the EDID.

> 
> Have tested this using xrandr by using below command:
> xrandr --output HDMI2 --set "Colorspace" "BT2020_rgb"
> 

It would also be really great to get some more comprehensive
documentation about how this property is meant to be used. Is the
expectation that userspace does everything? i.e.:

 - Userspace sets up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
   colorspace
 - Userspace sets this new property to let the sink know what it
   converted the CRTC output to.

Or in other words, I think this new property has zero impact on any
pixel processing in the pipeline - it only sets the colorspace in the
infoframe. That seems very valuable to write down explicitly.

BTW, is there already a standard property for converting CRTC output
to YCbCr? And does that interact with picking the YCC colorimetries
with this property?

Cheers,
-Brian

> v2: Addressed Ville and Maarten's review comments. Merged the 2nd
> and 3rd patch into one common logical patch.
> 
> v3: Removed Adobe references from enum definitions as per
> Ville, Hans Verkuil and Jonas Karlman suggestions. Changed
> default to an unset state where driver will assign the colorspace
> when not chosen by user, suggested by Ville and Maarten. Addressed
> other misc review comments from Maarten. Split the changes to
> have separate colorspace property for DP and HDMI.
> 
> v4: Addressed Chris and Ville's review comments, and created a
> common colorspace property for DP and HDMI, filtered the list
> based on the colorspaces supported by the respective protocol
> standard. Handled the default case more efficiently.
> 
> Uma Shankar (3):
>   drm: Add HDMI colorspace property
>   drm: Add DP colorspace property
>   drm/i915: Attach colorspace property and enable modeset
> 
>  drivers/gpu/drm/drm_atomic_uapi.c      |  4 ++
>  drivers/gpu/drm/drm_connector.c        | 92 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_atomic.c    |  1 +
>  drivers/gpu/drm/i915/intel_connector.c |  8 +++
>  drivers/gpu/drm/i915/intel_drv.h       |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c      | 18 +++++++
>  include/drm/drm_connector.h            | 14 ++++++
>  include/uapi/drm/drm_mode.h            | 33 ++++++++++++
>  8 files changed, 171 insertions(+)
> 
> -- 
> 1.9.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel