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 >> >> > Acked-by: Jani Nikula >> >> &
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/drm/drm_atomic_uapi.c > >> >
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 (property == config->writeback_fb_id_propert
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)
> >
> >
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 */
> + {
