Re: [PATCH v5 04/13] drm/connector: Use common colorspace_names array

2023-06-07 Thread Simon Ser
On Tuesday, June 6th, 2023 at 22:25, Harry Wentland  
wrote:

> + if (supported_colorspaces != 0 && (colorspaces & BIT(i)) == 0)

This patch actually also introduces a change in behavior: passing no
colorspace will make the function advertise all colorspaces. I have a
hard time understanding how this can be useful: we want to either
advertise all DP colorspaces, or all HDMI colorspaces, but not both?

One way to fix this would be to handle the "zero means everything"
behavior in the specific DP/HDMI callers. But I wonder, is it really
worth the magic if we can expose a simple const variable with all
DP/HDMI colorspaces?


Re: [PATCH v5 04/13] drm/connector: Use common colorspace_names array

2023-06-07 Thread Simon Ser


On Tuesday, June 6th, 2023 at 22:25, Harry Wentland  
wrote:

> We an use bitfields to track the support ones for HDMI

Typo: "We can"


[PATCH v5 04/13] drm/connector: Use common colorspace_names array

2023-06-06 Thread Harry Wentland
We an use bitfields to track the support ones for HDMI
and DP. This allows us to print colorspaces in a consistent
manner without needing to know whether we're dealing with
DP or HDMI.

v4:
- Rename _MAX to _COUNT and leave comment to indicate
  it's not a valid value
- Fix misplaced function doc

Signed-off-by: Harry Wentland 
Reviewed-by: Sebastian Wick 

Cc: Pekka Paalanen 
Cc: Sebastian Wick 
Cc: vitaly.pros...@amd.com
Cc: Uma Shankar 
Cc: Ville Syrjälä 
Cc: Joshua Ashton 
Cc: Jani Nikula 
Cc: Simon Ser 
Cc: Ville Syrjälä 
Cc: Melissa Wen 
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
---
 drivers/gpu/drm/drm_connector.c | 125 ++--
 include/drm/drm_connector.h |   2 +
 2 files changed, 74 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 9c087d6f5691..8d24a5da4076 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1016,64 +1016,70 @@ static const struct drm_prop_enum_list 
drm_dp_subconnector_enum_list[] = {
 DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 drm_dp_subconnector_enum_list)
 
-static const struct drm_prop_enum_list hdmi_colorspaces[] = {
+
+static const char * const colorspace_names[] = {
/* For Default case, driver will set the colorspace */
-   { DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
+   [DRM_MODE_COLORIMETRY_DEFAULT] = "Default",
/* Standard Definition Colorimetry based on CEA 861 */
-   { DRM_MODE_COLORIMETRY_SMPTE_170M_YCC, "SMPTE_170M_YCC" },
-   { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" },
+   [DRM_MODE_COLORIMETRY_SMPTE_170M_YCC] = "SMPTE_170M_YCC",
+   [DRM_MODE_COLORIMETRY_BT709_YCC] = "BT709_YCC",
/* Standard Definition Colorimetry based on IEC 61966-2-4 */
-   { DRM_MODE_COLORIMETRY_XVYCC_601, "XVYCC_601" },
+   [DRM_MODE_COLORIMETRY_XVYCC_601] = "XVYCC_601",
/* High Definition Colorimetry based on IEC 61966-2-4 */
-   { DRM_MODE_COLORIMETRY_XVYCC_709, "XVYCC_709" },
+   [DRM_MODE_COLORIMETRY_XVYCC_709] = "XVYCC_709",
/* Colorimetry based on IEC 61966-2-1/Amendment 1 */
-   { DRM_MODE_COLORIMETRY_SYCC_601, "SYCC_601" },
+   [DRM_MODE_COLORIMETRY_SYCC_601] = "SYCC_601",
/* Colorimetry based on IEC 61966-2-5 [33] */
-   { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
+   [DRM_MODE_COLORIMETRY_OPYCC_601] = "opYCC_601",
/* Colorimetry based on IEC 61966-2-5 */
-   { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
+   [DRM_MODE_COLORIMETRY_OPRGB] = "opRGB",
/* Colorimetry based on ITU-R BT.2020 */
-   { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
+   [DRM_MODE_COLORIMETRY_BT2020_CYCC] = "BT2020_CYCC",
/* Colorimetry based on ITU-R BT.2020 */
-   { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
+   [DRM_MODE_COLORIMETRY_BT2020_RGB] = "BT2020_RGB",
/* Colorimetry based on ITU-R BT.2020 */
-   { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
+   [DRM_MODE_COLORIMETRY_BT2020_YCC] = "BT2020_YCC",
/* Added as part of Additional Colorimetry Extension in 861.G */
-   { DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65, "DCI-P3_RGB_D65" },
-   { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
+   [DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65] = "DCI-P3_RGB_D65",
+   [DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER] = "DCI-P3_RGB_Theater",
+   [DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED] = "RGB_WIDE_FIXED",
+   /* Colorimetry based on scRGB (IEC 61966-2-2) */
+   [DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT] = "RGB_WIDE_FLOAT",
+   [DRM_MODE_COLORIMETRY_BT601_YCC] = "BT601_YCC",
 };
 
+static const u32 hdmi_colorspaces =
+   BIT(DRM_MODE_COLORIMETRY_SMPTE_170M_YCC) |
+   BIT(DRM_MODE_COLORIMETRY_BT709_YCC) |
+   BIT(DRM_MODE_COLORIMETRY_XVYCC_601) |
+   BIT(DRM_MODE_COLORIMETRY_XVYCC_709) |
+   BIT(DRM_MODE_COLORIMETRY_SYCC_601) |
+   BIT(DRM_MODE_COLORIMETRY_OPYCC_601) |
+   BIT(DRM_MODE_COLORIMETRY_OPRGB) |
+   BIT(DRM_MODE_COLORIMETRY_BT2020_CYCC) |
+   BIT(DRM_MODE_COLORIMETRY_BT2020_RGB) |
+   BIT(DRM_MODE_COLORIMETRY_BT2020_YCC) |
+   BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65) |
+   BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER);
+
 /*
  * As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel 
Encoding/Colorimetry
  * Format Table 2-120
  */
-static const struct drm_prop_enum_list dp_colorspaces[] = {
-   /* For Default case, driver will set the colorspace */
-   { DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
-   { DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED, "RGB_Wide_Gamut_Fixed_Point" },
-   /* Colorimetry based on scRGB (IEC 61966-2-2) */
-   { DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT, "RGB_Wide_Gamut_Floating_Point" 
},
-   /* Colorimetry based on IEC 61966-2-5 */
-   { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
-   /* Colorimetry based on SMPTE RP 431-2 */
-