| Message ID | 1545933158-22396-1-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add Colorspace connector property interface | expand |
Hi Uma, On Thu, Dec 27, 2018 at 11:22:36PM +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. > > Basically the expectation from userspace is: > - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink > colorspace > - Set this new property to let the sink know what it > converted the CRTC output to. > - This property is just to inform sink what colorspace > source is trying to drive. All the above info is really important/useful stuff, but it's going to get lost because it's only in the cover letter. This should either find its way into the commit message of patch 1 or even better, into the kerneldoc for the property. Cheers, -Brian > > Have tested this using xrandr by using below command: > xrandr --output HDMI2 --set "Colorspace" "BT2020_rgb" > > 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. > > v5: Modified the colorspace property creation helper to take > platform specific enum list based on the capabilities of the > platform as suggested by Shashank. With this there is no need > for segregation between DP and HDMI. > > v6: Addressed Shashank's review comments. > > Uma Shankar (2): > drm: Add colorspace connector property > drm/i915: Attach colorspace property and enable modeset > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ > drivers/gpu/drm/drm_connector.c | 82 ++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_atomic.c | 1 + > drivers/gpu/drm/i915/intel_connector.c | 63 ++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 18 ++++++++ > include/drm/drm_connector.h | 17 +++++++ > include/uapi/drm/drm_mode.h | 33 ++++++++++++++ > 8 files changed, 219 insertions(+) > > -- > 1.9.1 > > Uma Shankar (2): > drm: Add colorspace connector property > drm/i915: Attach colorspace property and enable modeset > > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ > drivers/gpu/drm/drm_connector.c | 79 ++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_atomic.c | 1 + > drivers/gpu/drm/i915/intel_connector.c | 63 +++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 19 ++++++++ > include/drm/drm_connector.h | 17 ++++++++ > include/uapi/drm/drm_mode.h | 33 ++++++++++++++ > 8 files changed, 217 insertions(+) > > -- > 1.9.1 >
>-----Original Message----- >From: Brian Starkey [mailto:Brian.Starkey@arm.com] >Sent: Tuesday, January 8, 2019 7:37 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Lankhorst, >Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; >Sharma, Shashank <shashank.sharma@intel.com>; nd <nd@arm.com> >Subject: Re: [v6 0/2] Add Colorspace connector property interface > >Hi Uma, > >On Thu, Dec 27, 2018 at 11:22:36PM +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. >> >> Basically the expectation from userspace is: >> - Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink >> colorspace >> - Set this new property to let the sink know what it >> converted the CRTC output to. >> - This property is just to inform sink what colorspace >> source is trying to drive. > >All the above info is really important/useful stuff, but it's going to get lost >because it's only in the cover letter. This should either find its way into the >commit message of patch 1 or even better, into the kerneldoc for the property. Sure Brian, Will add it to kernel doc as well to commit message so that it's not lost. Regards, Uma Shankar >Cheers, >-Brian > >> >> Have tested this using xrandr by using below command: >> xrandr --output HDMI2 --set "Colorspace" "BT2020_rgb" >> >> 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. >> >> v5: Modified the colorspace property creation helper to take platform >> specific enum list based on the capabilities of the platform as >> suggested by Shashank. With this there is no need for segregation >> between DP and HDMI. >> >> v6: Addressed Shashank's review comments. >> >> Uma Shankar (2): >> drm: Add colorspace connector property >> drm/i915: Attach colorspace property and enable modeset >> >> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ >> drivers/gpu/drm/drm_connector.c | 82 >++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_atomic.c | 1 + >> drivers/gpu/drm/i915/intel_connector.c | 63 ++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_hdmi.c | 18 ++++++++ >> include/drm/drm_connector.h | 17 +++++++ >> include/uapi/drm/drm_mode.h | 33 ++++++++++++++ >> 8 files changed, 219 insertions(+) >> >> -- >> 1.9.1 >> >> Uma Shankar (2): >> drm: Add colorspace connector property >> drm/i915: Attach colorspace property and enable modeset >> >> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++ >> drivers/gpu/drm/drm_connector.c | 79 >++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_atomic.c | 1 + >> drivers/gpu/drm/i915/intel_connector.c | 63 +++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_hdmi.c | 19 ++++++++ >> include/drm/drm_connector.h | 17 ++++++++ >> include/uapi/drm/drm_mode.h | 33 ++++++++++++++ >> 8 files changed, 217 insertions(+) >> >> -- >> 1.9.1 >>