| Message ID | 1548852866-32349-3-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Colorspace connector property interface | expand |
On Wed, Jan 30, 2019 at 06:24:25PM +0530, Uma Shankar wrote: > This patch adds a DP colorspace property, enabling > userspace to switch to various supported colorspaces. > This will help enable BT2020 along with other colorspaces. > > 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: 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: Merged the DP handling along with platform colorspace > handling as per Shashank's comments. > > v6: Reverted to old design of exposing all colorspaces to > userspace as per Ville's review comment > > v7: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Acked-by: Jani Nikula <jani.nikula@intel.com> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/drm_connector.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index ed10dd9..b331be8 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -850,6 +850,29 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info, > { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, > }; > > +static const struct drm_prop_enum_list dp_colorspaces[] = { > + /* For Default case, driver will set the colorspace */ > + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, > + /* Standard Definition Colorimetry based on CEA 861 */ > + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, > + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, What's with the duplicated 601/709 values? I think in the HDMI verison you had only these ones here. Maybe we want to actually state explicitly that they are for YCbCr, if only to be consistent with the BT2020 values. > + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ > + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, > + /* High Definition Colorimetry based on IEC 61966-2-4 */ > + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, > + /* Colorimetry based on IEC 61966-2-5 */ > + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, > + /* DP MSA Colorimetry */ > + { DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" }, > + { DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" }, > + { DRM_MODE_DP_COLORIMETRY_SRGB, "sRGB" }, > + { DRM_MODE_DP_COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" }, > + { DRM_MODE_DP_COLORIMETRY_SCRGB, "scRGB" }, > + { DRM_MODE_DP_COLORIMETRY_DCI_P3, "DCI-P3" }, > + { DRM_MODE_DP_COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom Profile" }, I don't think we want this last one since we don't implement anything that could transmit the custom profile. The MSA bits are have "CEA RGB" which we probably want to keep hidden since it seems to be just a poorly specced limited range vs. full range knob, and we already have a mechanism for that. The Y-only and RAW I guess we can skip. Not sure anyone would ever have use for those. > +}; > + > + > /** > * DOC: standard connector properties > * > @@ -1611,6 +1634,14 @@ int drm_mode_create_colorspace_property(struct drm_connector *connector) > ARRAY_SIZE(hdmi_colorspaces)); > if (!prop) > return -ENOMEM; > + } else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP || > + connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) { > + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, > + "Colorspace", dp_colorspaces, > + ARRAY_SIZE(dp_colorspaces)); > + > + if (!prop) > + return -ENOMEM; > } else { > DRM_DEBUG_KMS("Colorspace property not supported\n"); > return 0; > -- > 1.9.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >Ville Syrjälä >Sent: Saturday, February 2, 2019 12:48 AM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: emil.l.velikov@gmail.com; intel-gfx@lists.freedesktop.org; Syrjala, Ville ><ville.syrjala@intel.com>; Lankhorst, Maarten <maarten.lankhorst@intel.com>; >dri-devel@lists.freedesktop.org >Subject: Re: [v10 2/3] drm: Add DP colorspace property > >On Wed, Jan 30, 2019 at 06:24:25PM +0530, Uma Shankar wrote: >> This patch adds a DP colorspace property, enabling userspace to switch >> to various supported colorspaces. >> This will help enable BT2020 along with other colorspaces. >> >> 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: 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: Merged the DP handling along with platform colorspace handling as >> per Shashank's comments. >> >> v6: Reverted to old design of exposing all colorspaces to userspace as >> per Ville's review comment >> >> v7: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> Acked-by: Jani Nikula <jani.nikula@intel.com> >> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/drm_connector.c | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> b/drivers/gpu/drm/drm_connector.c index ed10dd9..b331be8 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -850,6 +850,29 @@ int drm_display_info_set_bus_formats(struct >drm_display_info *info, >> { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, }; >> >> +static const struct drm_prop_enum_list dp_colorspaces[] = { >> + /* For Default case, driver will set the colorspace */ >> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, >> + /* Standard Definition Colorimetry based on CEA 861 */ >> + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, >> + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, > >What's with the duplicated 601/709 values? I think in the HDMI verison you had >only these ones here. Maybe we want to actually state explicitly that they are for >YCbCr, if only to be consistent with the >BT2020 values. Yeah they are for YCbCr, will add that detail to be clear. > >> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ >> + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, >> + /* High Definition Colorimetry based on IEC 61966-2-4 */ >> + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, >> + /* Colorimetry based on IEC 61966-2-5 */ >> + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, >> + /* DP MSA Colorimetry */ >> + { DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" >}, >> + { DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" >}, >> + { DRM_MODE_DP_COLORIMETRY_SRGB, "sRGB" }, >> + { DRM_MODE_DP_COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide >Gamut" }, >> + { DRM_MODE_DP_COLORIMETRY_SCRGB, "scRGB" }, >> + { DRM_MODE_DP_COLORIMETRY_DCI_P3, "DCI-P3" }, >> + { DRM_MODE_DP_COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom >Profile" }, > >I don't think we want this last one since we don't implement anything that could >transmit the custom profile. > >The MSA bits are have "CEA RGB" which we probably want to keep hidden since it >seems to be just a poorly specced limited range vs. full range knob, and we >already have a mechanism for that. The Y-only and RAW I guess we can skip. Not >sure anyone would ever have use for those. Ok Sure, Will drop this. Regards, Uma Shankar > >> +}; >> + >> + >> /** >> * DOC: standard connector properties >> * >> @@ -1611,6 +1634,14 @@ int drm_mode_create_colorspace_property(struct >drm_connector *connector) >> > ARRAY_SIZE(hdmi_colorspaces)); >> if (!prop) >> return -ENOMEM; >> + } else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP >|| >> + connector->connector_type == >DRM_MODE_CONNECTOR_DisplayPort) { >> + prop = drm_property_create_enum(dev, >DRM_MODE_PROP_ENUM, >> + "Colorspace", dp_colorspaces, >> + ARRAY_SIZE(dp_colorspaces)); >> + >> + if (!prop) >> + return -ENOMEM; >> } else { >> DRM_DEBUG_KMS("Colorspace property not supported\n"); >> return 0; >> -- >> 1.9.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >-- >Ville Syrjälä >Intel >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index ed10dd9..b331be8 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -850,6 +850,29 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info, { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, }; +static const struct drm_prop_enum_list dp_colorspaces[] = { + /* For Default case, driver will set the colorspace */ + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, + /* Standard Definition Colorimetry based on CEA 861 */ + { DRM_MODE_COLORIMETRY_ITU_601, "ITU_601" }, + { DRM_MODE_COLORIMETRY_ITU_709, "ITU_709" }, + /* Standard Definition Colorimetry based on IEC 61966-2-4 */ + { DRM_MODE_COLORIMETRY_XV_YCC_601, "XV_YCC_601" }, + /* High Definition Colorimetry based on IEC 61966-2-4 */ + { DRM_MODE_COLORIMETRY_XV_YCC_709, "XV_YCC_709" }, + /* Colorimetry based on IEC 61966-2-5 */ + { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" }, + /* DP MSA Colorimetry */ + { DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" }, + { DRM_MODE_DP_COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" }, + { DRM_MODE_DP_COLORIMETRY_SRGB, "sRGB" }, + { DRM_MODE_DP_COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" }, + { DRM_MODE_DP_COLORIMETRY_SCRGB, "scRGB" }, + { DRM_MODE_DP_COLORIMETRY_DCI_P3, "DCI-P3" }, + { DRM_MODE_DP_COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom Profile" }, +}; + + /** * DOC: standard connector properties * @@ -1611,6 +1634,14 @@ int drm_mode_create_colorspace_property(struct drm_connector *connector) ARRAY_SIZE(hdmi_colorspaces)); if (!prop) return -ENOMEM; + } else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP || + connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) { + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, + "Colorspace", dp_colorspaces, + ARRAY_SIZE(dp_colorspaces)); + + if (!prop) + return -ENOMEM; } else { DRM_DEBUG_KMS("Colorspace property not supported\n"); return 0;
This patch adds a DP colorspace property, enabling userspace to switch to various supported colorspaces. This will help enable BT2020 along with other colorspaces. 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: 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: Merged the DP handling along with platform colorspace handling as per Shashank's comments. v6: Reverted to old design of exposing all colorspaces to userspace as per Ville's review comment v7: Fixed sparse warnings, updated the RB from Maarten and Jani's ack. Signed-off-by: Uma Shankar <uma.shankar@intel.com> Acked-by: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/drm_connector.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)