>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >Brian Starkey >Sent: Wednesday, November 28, 2018 8:17 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: Syrjala, Ville <ville.syrjala@xxxxxxxxx>; jonas@xxxxxxxxx; intel- >gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; >hansverk@xxxxxxxxx; nd <nd@xxxxxxx>; Lankhorst, Maarten ><maarten.lankhorst@xxxxxxxxx> >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@xxxxxxxxxxxxxxxxxxxxx] On >> >Behalf Of Brian Starkey >> >Sent: Wednesday, November 28, 2018 5:27 PM >> >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >> >Cc: Syrjala, Ville <ville.syrjala@xxxxxxxxx>; jonas@xxxxxxxxx; intel- >> >gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; >> >hansverk@xxxxxxxxx; nd <nd@xxxxxxx>; Lankhorst, Maarten >> ><maarten.lankhorst@xxxxxxxxx> >> >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@xxxxxxxxxxxxxxxxxxxxx >> >https://lists.freedesktop.org/mailman/listinfo/dri-devel >_______________________________________________ >dri-devel mailing list >dri-devel@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
