RE: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property
>-Original Message- >From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of >Ville Syrjälä >Sent: Monday, February 4, 2019 10:54 PM >To: Shankar, Uma >Cc: intel-...@lists.freedesktop.org; Syrjala, Ville ; >dri- >de...@lists.freedesktop.org; Lankhorst, Maarten > >Subject: Re: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property > >On Mon, Feb 04, 2019 at 05:08:40PM +, Shankar, Uma wrote: >> >> >> >-Original Message- >> >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] >> >Sent: Monday, February 4, 2019 8:55 PM >> >To: Shankar, Uma >> >Cc: intel-...@lists.freedesktop.org; Syrjala, Ville >> >; Lankhorst, Maarten >> >; dri- de...@lists.freedesktop.org >> >Subject: Re: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property >> > >> >On Fri, Feb 01, 2019 at 08:50:11PM +0200, Ville Syrjälä wrote: >> >> On Wed, Jan 30, 2019 at 06:24:24PM +0530, Uma Shankar wrote: >> >> > Create 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. Sink supported colorspaces should be retrieved >> >> > by userspace from EDID and driver will not explicitly expose them. >> >> > >> >> > 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. >> >> > >> >> > v2: Addressed Maarten and Ville's review comments. Enhanced the >> >> > colorspace enum to incorporate both HDMI and DP supported >> >> > colorspaces. Also, added a default option for colorspace. >> >> > >> >> > 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 is 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. >> >> > >> >> > v5: Made the property creation helper accept enum list based on >> >> > platform capabilties as suggested by Shashank. Consolidated HDMI >> >> > and DP property creation in the common helper. >> >> > >> >> > v6: Addressed Shashank's review comments. >> >> > >> >> > v7: Added defines instead of enum in uapi as per Brian Starkey's >> >> > suggestion in order to go with string matching at userspace. >> >> > Updated the commit message to add more details as well kernel docs. >> >> > >> >> > v8: Addressed Maarten's review comments. >> >> > >> >> > v9: Removed macro defines from uapi as per Brian Starkey and >> >> > Daniel Stone's comments and moved to drm include file. Moved back >> >> > to older design with exposing all HDMI colorspaces to userspace >> >> > since infoframe capability is there even on legacy platforms, as >> >> > per Ville's review comments. >> >> > >> >> > v10: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. >> >> > >> >> > Signed-off-by: Uma Shankar >> >> > Ac
Re: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property
On Mon, Feb 04, 2019 at 05:08:40PM +, Shankar, Uma wrote: > > > >-Original Message- > >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] > >Sent: Monday, February 4, 2019 8:55 PM > >To: Shankar, Uma > >Cc: intel-...@lists.freedesktop.org; Syrjala, Ville > >; > >Lankhorst, Maarten ; dri- > >de...@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property > > > >On Fri, Feb 01, 2019 at 08:50:11PM +0200, Ville Syrjälä wrote: > >> On Wed, Jan 30, 2019 at 06:24:24PM +0530, Uma Shankar wrote: > >> > Create 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. Sink supported colorspaces should be retrieved by > >> > userspace from EDID and driver will not explicitly expose them. > >> > > >> > 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. > >> > > >> > v2: Addressed Maarten and Ville's review comments. Enhanced the > >> > colorspace enum to incorporate both HDMI and DP supported > >> > colorspaces. Also, added a default option for colorspace. > >> > > >> > 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 is 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. > >> > > >> > v5: Made the property creation helper accept enum list based on > >> > platform capabilties as suggested by Shashank. Consolidated HDMI and > >> > DP property creation in the common helper. > >> > > >> > v6: Addressed Shashank's review comments. > >> > > >> > v7: Added defines instead of enum in uapi as per Brian Starkey's > >> > suggestion in order to go with string matching at userspace. Updated > >> > the commit message to add more details as well kernel docs. > >> > > >> > v8: Addressed Maarten's review comments. > >> > > >> > v9: Removed macro defines from uapi as per Brian Starkey and Daniel > >> > Stone's comments and moved to drm include file. Moved back to older > >> > design with exposing all HDMI colorspaces to userspace since > >> > infoframe capability is there even on legacy platforms, as per > >> > Ville's review comments. > >> > > >> > v10: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. > >> > > >> > Signed-off-by: Uma Shankar > >> > Acked-by: Jani Nikula > >> > Reviewed-by: Shashank Sharma > >> > Reviewed-by: Maarten Lankhorst > >> > --- > >> > drivers/gpu/drm/drm_atomic_uapi.c | 4 +++ > >> > drivers/gpu/drm/drm_connector.c | 75 > >+++ > >> > include/drm/drm_connector.h | 46 > >> > 3 files changed, 125 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > >> > b/drivers/gpu/drm/drm_atomic_uapi.c > >> > index 9a1f41a..9b5d44f 100644 > >> > --- a/drivers/gpu/d
RE: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property
>-Original Message- >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] >Sent: Monday, February 4, 2019 8:55 PM >To: Shankar, Uma >Cc: intel-...@lists.freedesktop.org; Syrjala, Ville ; >Lankhorst, Maarten ; dri- >de...@lists.freedesktop.org >Subject: Re: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property > >On Fri, Feb 01, 2019 at 08:50:11PM +0200, Ville Syrjälä wrote: >> On Wed, Jan 30, 2019 at 06:24:24PM +0530, Uma Shankar wrote: >> > Create 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. Sink supported colorspaces should be retrieved by >> > userspace from EDID and driver will not explicitly expose them. >> > >> > 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. >> > >> > v2: Addressed Maarten and Ville's review comments. Enhanced the >> > colorspace enum to incorporate both HDMI and DP supported >> > colorspaces. Also, added a default option for colorspace. >> > >> > 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 is 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. >> > >> > v5: Made the property creation helper accept enum list based on >> > platform capabilties as suggested by Shashank. Consolidated HDMI and >> > DP property creation in the common helper. >> > >> > v6: Addressed Shashank's review comments. >> > >> > v7: Added defines instead of enum in uapi as per Brian Starkey's >> > suggestion in order to go with string matching at userspace. Updated >> > the commit message to add more details as well kernel docs. >> > >> > v8: Addressed Maarten's review comments. >> > >> > v9: Removed macro defines from uapi as per Brian Starkey and Daniel >> > Stone's comments and moved to drm include file. Moved back to older >> > design with exposing all HDMI colorspaces to userspace since >> > infoframe capability is there even on legacy platforms, as per >> > Ville's review comments. >> > >> > v10: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. >> > >> > Signed-off-by: Uma Shankar >> > Acked-by: Jani Nikula >> > Reviewed-by: Shashank Sharma >> > Reviewed-by: Maarten Lankhorst >> > --- >> > drivers/gpu/drm/drm_atomic_uapi.c | 4 +++ >> > drivers/gpu/drm/drm_connector.c | 75 >+++ >> > include/drm/drm_connector.h | 46 >> > 3 files changed, 125 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c >> > b/drivers/gpu/drm/drm_atomic_uapi.c >> > index 9a1f41a..9b5d44f 100644 >> > --- a/drivers/gpu/drm/drm_atomic_uapi.c >> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c >> > @@ -746,6 +746,8 @@ static int drm_atomic_connector_set_property(struct >drm_connector *connector, >> >return -EINVAL; >> >} >> >state->content_protection = val; >> > + } else if (property == connector->colorspace_property) { >> > + state->colorspace = val; >> >} else if (proper
Re: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property
On Fri, Feb 01, 2019 at 08:50:11PM +0200, Ville Syrjälä wrote: > On Wed, Jan 30, 2019 at 06:24:24PM +0530, Uma Shankar wrote: > > Create 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. Sink supported colorspaces should be retrieved by > > userspace from EDID and driver will not explicitly expose them. > > > > 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. > > > > v2: Addressed Maarten and Ville's review comments. Enhanced > > the colorspace enum to incorporate both HDMI and DP supported > > colorspaces. Also, added a default option for colorspace. > > > > 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 > > is 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. > > > > v5: Made the property creation helper accept enum list based on > > platform capabilties as suggested by Shashank. Consolidated HDMI > > and DP property creation in the common helper. > > > > v6: Addressed Shashank's review comments. > > > > v7: Added defines instead of enum in uapi as per Brian Starkey's > > suggestion in order to go with string matching at userspace. Updated > > the commit message to add more details as well kernel docs. > > > > v8: Addressed Maarten's review comments. > > > > v9: Removed macro defines from uapi as per Brian Starkey and Daniel > > Stone's comments and moved to drm include file. Moved back to older > > design with exposing all HDMI colorspaces to userspace since infoframe > > capability is there even on legacy platforms, as per Ville's review > > comments. > > > > v10: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. > > > > Signed-off-by: Uma Shankar > > Acked-by: Jani Nikula > > Reviewed-by: Shashank Sharma > > Reviewed-by: Maarten Lankhorst > > --- > > drivers/gpu/drm/drm_atomic_uapi.c | 4 +++ > > drivers/gpu/drm/drm_connector.c | 75 > > +++ > > include/drm/drm_connector.h | 46 > > 3 files changed, 125 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > b/drivers/gpu/drm/drm_atomic_uapi.c > > index 9a1f41a..9b5d44f 100644 > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > @@ -746,6 +746,8 @@ static int drm_atomic_connector_set_property(struct > > drm_connector *connector, > > return -EINVAL; > > } > > state->content_protection = val; > > + } else if (property == connector->colorspace_property) { > > + state->colorspace = val; > > } else if (property == config->writeback_fb_id_property) { > > struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, > > val); > > int ret = drm_atomic_set_writeback_fb_for_connector(state, fb); > > @@ -814,6 +816,8 @@ static int drm_atomic_connector_set_property(struct > > drm_connector *connector, > > *val = state->picture_aspect_ratio; > > } else if (property == config->content_type_property) { > > *val = state->content_type; > > + } else if (property == connector->colorspace_property) { > > + *val = state->colorspace; > > } else if (property == connector->scaling_mode_property) { > > *val = state->scaling_mode; > > } else if (property == connector->content_protection_property) { > > diff --git a/drivers/gpu/drm/drm_connector.c > > b/drivers/gpu/drm/drm_connector.c > > index 8475396..ed10dd9 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -826,6 +826,30 @@ int drm_display_info_set_bus_formats(struct > > drm_display_info *info, > > }; > > DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list) > > > > +stat
Re: [Intel-gfx] [v10 1/3] drm: Add HDMI colorspace property
On Wed, Jan 30, 2019 at 06:24:24PM +0530, Uma Shankar wrote: > Create 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. Sink supported colorspaces should be retrieved by > userspace from EDID and driver will not explicitly expose them. > > 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. > > v2: Addressed Maarten and Ville's review comments. Enhanced > the colorspace enum to incorporate both HDMI and DP supported > colorspaces. Also, added a default option for colorspace. > > 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 > is 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. > > v5: Made the property creation helper accept enum list based on > platform capabilties as suggested by Shashank. Consolidated HDMI > and DP property creation in the common helper. > > v6: Addressed Shashank's review comments. > > v7: Added defines instead of enum in uapi as per Brian Starkey's > suggestion in order to go with string matching at userspace. Updated > the commit message to add more details as well kernel docs. > > v8: Addressed Maarten's review comments. > > v9: Removed macro defines from uapi as per Brian Starkey and Daniel > Stone's comments and moved to drm include file. Moved back to older > design with exposing all HDMI colorspaces to userspace since infoframe > capability is there even on legacy platforms, as per Ville's review > comments. > > v10: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. > > Signed-off-by: Uma Shankar > Acked-by: Jani Nikula > Reviewed-by: Shashank Sharma > Reviewed-by: Maarten Lankhorst > --- > drivers/gpu/drm/drm_atomic_uapi.c | 4 +++ > drivers/gpu/drm/drm_connector.c | 75 > +++ > include/drm/drm_connector.h | 46 > 3 files changed, 125 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index 9a1f41a..9b5d44f 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -746,6 +746,8 @@ static int drm_atomic_connector_set_property(struct > drm_connector *connector, > return -EINVAL; > } > state->content_protection = val; > + } else if (property == connector->colorspace_property) { > + state->colorspace = val; > } else if (property == config->writeback_fb_id_property) { > struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, > val); > int ret = drm_atomic_set_writeback_fb_for_connector(state, fb); > @@ -814,6 +816,8 @@ static int drm_atomic_connector_set_property(struct > drm_connector *connector, > *val = state->picture_aspect_ratio; > } else if (property == config->content_type_property) { > *val = state->content_type; > + } else if (property == connector->colorspace_property) { > + *val = state->colorspace; > } else if (property == connector->scaling_mode_property) { > *val = state->scaling_mode; > } else if (property == connector->content_protection_property) { > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 8475396..ed10dd9 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -826,6 +826,30 @@ int drm_display_info_set_bus_formats(struct > drm_display_info *info, > }; > DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list) > > +static const struct drm_prop_enum_list hdmi_colorspaces[] = { > + /* For Default case, driver will set the colorspace */ > + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > + /* Standard Definition Colorimetry based on CEA 861 */ > + { D