>-----Original Message-----
>From: dri-devel [mailto:[email protected]] On Behalf Of
>Ville Syrjälä
>Sent: Saturday, February 2, 2019 12:48 AM
>To: Shankar, Uma <[email protected]>
>Cc: [email protected]; [email protected]; Syrjala, Ville
><[email protected]>; Lankhorst, Maarten <[email protected]>;
>[email protected]
>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 <[email protected]>
>> Acked-by: Jani Nikula <[email protected]>
>> Reviewed-by: Maarten Lankhorst <[email protected]>
>> ---
>>  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
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>--
>Ville Syrjälä
>Intel
>_______________________________________________
>dri-devel mailing list
>[email protected]
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to