[v4 0/3] Add Colorspace connector property interface

Shankar, Uma uma.shankar at intel.com
Thu Nov 29 14:40:55 UTC 2018 


>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf Of
>Brian Starkey
>Sent: Wednesday, November 28, 2018 8:17 PM
>To: Shankar, Uma <uma.shankar at intel.com>
>Cc: Syrjala, Ville <ville.syrjala at intel.com>; jonas at kwiboo.se; intel-
>gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
>hansverk at cisco.com; nd <nd at arm.com>; Lankhorst, Maarten
><maarten.lankhorst at intel.com>
>Subject: Re: [v4 0/3] Add Colorspace connector property interface
>
>On Wed, Nov 28, 2018 at 02:29:53PM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On
>> >Behalf Of Brian Starkey
>> >Sent: Wednesday, November 28, 2018 5:27 PM
>> >To: Shankar, Uma <uma.shankar at intel.com>
>> >Cc: Syrjala, Ville <ville.syrjala at intel.com>; jonas at kwiboo.se; intel-
>> >gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
>> >hansverk at cisco.com; nd <nd at arm.com>; Lankhorst, Maarten
>> ><maarten.lankhorst at intel.com>
>> >Subject: Re: [v4 0/3] Add Colorspace connector property interface
>> >
>> >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.
>>
>> Only problem which I see is that all the connector properties are
>> created at connector initialization including the connector->edid
>> property. The respective sync devices may not be attached also at that
>> moment. There is an option to change the blob id's for case of edid
>> when new sink is plugged in, but the fundamental structure remains
>> same. I mean the enum which is used at creating the colorspace enum property
>can't be changed at runtime. It stays the same throughout the drivers life (this is
>of course based on my understanding :), correct me I am wrong).
>> Hence changing it based on sink is not easy with the current design.
>
>My understanding is the same. Did you look at what would be needed in order to
>change the enum property code to support changing the values at runtime? We
>do already have things which change (e.g. link status), just not enums.

Link Status also has a fixed enum 
static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
        { DRM_MODE_LINK_STATUS_GOOD, "Good" },
        { DRM_MODE_LINK_STATUS_BAD, "Bad" },
};

Not sure if we can change this enum at runtime. 

Regards,
Uma Shankar

>>
>> >>
>> >> 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.
>>
>> Yes, this is what this property does. Userspace decides the blending
>> and colorspace target for blending output from pipe. This helps to
>> pass that info to sink device which was missing till now. I will
>> update it and add documentation on the same to be clear wrt property
>expectation and purpose.
>>
>> >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?
>>
>> AFAIK, there is no dedicated property but the YUV modes are added as
>> part of HDMI 2.0 development work from Shashank. User can set the mode
>> and driver will do the conversion internally if the hardware supports it, else it
>may choose to drop the mode.
>>
>
>Thanks, I'll go take a look at that.
>
>-Brian
>
>> Regards,
>> Uma Shankar
>>
>> >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 at lists.freedesktop.org
>> >https://lists.freedesktop.org/mailman/listinfo/dri-devel
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel

More information about the dri-devel mailing list