Re: [Intel-gfx] [v6 1/2] drm: Add colorspace connector property
>-Original Message-
>From: Brian Starkey [mailto:brian.star...@arm.com]
>Sent: Tuesday, January 8, 2019 7:43 PM
>To: Shankar, Uma
>Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
>Lankhorst,
>Maarten ; Syrjala, Ville
>;
>Sharma, Shashank ; nd
>Subject: Re: [v6 1/2] drm: Add colorspace connector property
>
>Hi Uma,
>
>On Thu, Dec 27, 2018 at 11:22:37PM +0530, Uma Shankar wrote:
>> This patch adds a colorspace connector 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: 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.
>>
>> Signed-off-by: Uma Shankar
>> Reviewed-by: Shashank Sharma
>> ---
>> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
>> drivers/gpu/drm/drm_connector.c | 79
>+++
>> include/drm/drm_connector.h | 17 +
>> include/uapi/drm/drm_mode.h | 33
>> 4 files changed, 133 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index c408898..5e03292 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..ad1b862 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -826,6 +826,54 @@ 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)
>>
>> +/* List of HDMI Colorspaces supported */ static const struct
>> +drm_prop_enum_list hdmi_colorspaces[] = {
>> +/* For Default case, driver will set the colorspace */
>> +{ COLORIMETRY_DEFAULT, "Default" },
>> +/* Standard Definition Colorimetry based on CEA 861 */
>> +{ COLORIMETRY_ITU_601, "ITU_601" },
>> +{ COLORIMETRY_ITU_709, "ITU_709" },
>> +/* Standard Definition Colorimetry based on IEC 61966-2-4 */
>> +{ COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
>> +/* High Definition Colorimetry based on IEC 61966-2-4 */
>> +{ COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
>> +/* Colorimetry based on IEC 61966-2-1/Amendment 1 */
>> +{ COLORIMETRY_S_YCC_601, "S_YCC_601" },
>> +/* Colorimetry based on IEC 61966-2-5 [33] */
>> +{ COLORIMETRY_OPYCC_601, "opYCC_601" },
>> +/* Colorimetry based on IEC 61966-2-5 */
>> +{ COLORIMETRY_OPRGB, "opRGB" },
>> +/* Colorimetry based on ITU-R BT.2020 */
>> +{ COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
>> +/* Colorimetry based on ITU-R BT.2020 */
>> +{ COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
>> +/* Colorimetry based on ITU-R BT.2020 */
>> +{ COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" }, };
>> +
>> +/* List of DP Colorspaces supported */ static const struct
>> +drm_prop_enum_list dp_colorspaces[] = {
>> +/* For Default case
Re: [Intel-gfx] [v6 1/2] drm: Add colorspace connector property
Hi Uma,
On Thu, Dec 27, 2018 at 11:22:37PM +0530, Uma Shankar wrote:
> This patch adds a colorspace connector 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: 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.
>
> Signed-off-by: Uma Shankar
> Reviewed-by: Shashank Sharma
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
> drivers/gpu/drm/drm_connector.c | 79
> +++
> include/drm/drm_connector.h | 17 +
> include/uapi/drm/drm_mode.h | 33
> 4 files changed, 133 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index c408898..5e03292 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..ad1b862 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -826,6 +826,54 @@ 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)
>
> +/* List of HDMI Colorspaces supported */
> +static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> + /* For Default case, driver will set the colorspace */
> + { COLORIMETRY_DEFAULT, "Default" },
> + /* Standard Definition Colorimetry based on CEA 861 */
> + { COLORIMETRY_ITU_601, "ITU_601" },
> + { COLORIMETRY_ITU_709, "ITU_709" },
> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> + { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
> + /* High Definition Colorimetry based on IEC 61966-2-4 */
> + { COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
> + { COLORIMETRY_S_YCC_601, "S_YCC_601" },
> + /* Colorimetry based on IEC 61966-2-5 [33] */
> + { COLORIMETRY_OPYCC_601, "opYCC_601" },
> + /* Colorimetry based on IEC 61966-2-5 */
> + { COLORIMETRY_OPRGB, "opRGB" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> +};
> +
> +/* List of DP Colorspaces supported */
> +static const struct drm_prop_enum_list dp_colorspaces[] = {
> + /* For Default case, driver will set the colorspace */
> + { COLORIMETRY_DEFAULT, "Default" },
> + /* Standard Definition Colorimetry based on CEA 861 */
> + { COLORIMETRY_ITU_601, "ITU_601" },
> + { COLORIMETRY_ITU_709, "ITU_709" },
> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> + { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
> + /* High Definitio
[Intel-gfx] [v6 1/2] drm: Add colorspace connector property
This patch adds a colorspace connector 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: 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.
Signed-off-by: Uma Shankar
Reviewed-by: Shashank Sharma
---
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
drivers/gpu/drm/drm_connector.c | 79 +++
include/drm/drm_connector.h | 17 +
include/uapi/drm/drm_mode.h | 33
4 files changed, 133 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index c408898..5e03292 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..ad1b862 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -826,6 +826,54 @@ 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)
+/* List of HDMI Colorspaces supported */
+static const struct drm_prop_enum_list hdmi_colorspaces[] = {
+ /* For Default case, driver will set the colorspace */
+ { COLORIMETRY_DEFAULT, "Default" },
+ /* Standard Definition Colorimetry based on CEA 861 */
+ { COLORIMETRY_ITU_601, "ITU_601" },
+ { COLORIMETRY_ITU_709, "ITU_709" },
+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
+ /* High Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
+ /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
+ { COLORIMETRY_S_YCC_601, "S_YCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 [33] */
+ { COLORIMETRY_OPYCC_601, "opYCC_601" },
+ /* Colorimetry based on IEC 61966-2-5 */
+ { COLORIMETRY_OPRGB, "opRGB" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
+};
+
+/* List of DP Colorspaces supported */
+static const struct drm_prop_enum_list dp_colorspaces[] = {
+ /* For Default case, driver will set the colorspace */
+ { COLORIMETRY_DEFAULT, "Default" },
+ /* Standard Definition Colorimetry based on CEA 861 */
+ { COLORIMETRY_ITU_601, "ITU_601" },
+ { COLORIMETRY_ITU_709, "ITU_709" },
+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_601, "XV_YCC_601" },
+ /* High Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_709, "XV_YCC_709" },
+ /* Colorimetry based on IEC 61966-2-5 */
+ { COLORIMETRY_OPRGB, "opRGB" },
+ /* DP
