Re: [PATCH v4 08/13] drm/amd/display: Register Colorspace property for DP and HDMI

2023-05-26 Thread Simon Ser 
On Friday, May 26th, 2023 at 18:40, Sebastian Wick  
wrote:

> On Thu, May 25, 2023 at 9:18 PM Harry Wentland harry.wentl...@amd.com wrote:
> 
> > We want compositors to be able to set the output
> > colorspace on DP and HDMI outputs, based on the
> > caps reported from the receiver via EDID.
> 
> 
> This commit message seems wrong for multiple reasons. The Colorspace
> property documentation explicitly says that user space has to check
> the sink EDID for support:
> 
> > The expectation from userspace is that it should parse the EDID and get 
> > supported colorspaces.
> 
> The code doesn't seem to care about EDID at all. Instead it exposes
> the variants it knows how to support with e.g. the appropriate YCC
> transform when necessary, independent of the sink support.

The commit messages explains that compositors will parse the EDID.
It doesn't state that the kernel does.

Re: [PATCH v4 08/13] drm/amd/display: Register Colorspace property for DP and HDMI

2023-05-26 Thread Sebastian Wick 
On Thu, May 25, 2023 at 9:18 PM Harry Wentland  wrote:
>
> We want compositors to be able to set the output
> colorspace on DP and HDMI outputs, based on the
> caps reported from the receiver via EDID.

This commit message seems wrong for multiple reasons. The Colorspace
property documentation explicitly says that user space has to check
the sink EDID for support:

> The expectation from userspace is that it should parse the EDID and get 
> supported colorspaces.

The code doesn't seem to care about EDID at all. Instead it exposes
the variants it knows how to support with e.g. the appropriate YCC
transform when necessary, independent of the sink support.

> Signed-off-by: Harry Wentland 
> Cc: Pekka Paalanen 
> Cc: Sebastian Wick 
> Cc: vitaly.pros...@amd.com
> Cc: Joshua Ashton 
> Cc: Simon Ser 
> Cc: Ville Syrjälä 
> Cc: Melissa Wen 
> Cc: dri-devel@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index ca093396d1ac..dc99a8ffac70 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7238,6 +7238,12 @@ static int amdgpu_dm_connector_get_modes(struct 
> drm_connector *connector)
> return amdgpu_dm_connector->num_modes;
>  }
>
> +static const u32 supported_colorspaces =
> +   BIT(DRM_MODE_COLORIMETRY_BT709_YCC) |
> +   BIT(DRM_MODE_COLORIMETRY_OPRGB) |
> +   BIT(DRM_MODE_COLORIMETRY_BT2020_RGB) |
> +   BIT(DRM_MODE_COLORIMETRY_BT2020_YCC);
> +
>  void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
>  struct amdgpu_dm_connector *aconnector,
>  int connector_type,
> @@ -7318,6 +7324,15 @@ void amdgpu_dm_connector_init_helper(struct 
> amdgpu_display_manager *dm,
> adev->mode_info.abm_level_property, 0);
> }
>
> +   if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
> +   if 
> (!drm_mode_create_hdmi_colorspace_property(>base, 
> supported_colorspaces))
> +   
> drm_connector_attach_colorspace_property(>base);
> +   } else if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +  connector_type == DRM_MODE_CONNECTOR_eDP) {
> +   if 
> (!drm_mode_create_dp_colorspace_property(>base, 
> supported_colorspaces))
> +   
> drm_connector_attach_colorspace_property(>base);
> +   }
> +
> if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> connector_type == DRM_MODE_CONNECTOR_eDP) {
> --
> 2.40.1
>

[PATCH v4 08/13] drm/amd/display: Register Colorspace property for DP and HDMI

2023-05-25 Thread Harry Wentland 
We want compositors to be able to set the output
colorspace on DP and HDMI outputs, based on the
caps reported from the receiver via EDID.

Signed-off-by: Harry Wentland 
Cc: Pekka Paalanen 
Cc: Sebastian Wick 
Cc: vitaly.pros...@amd.com
Cc: Joshua Ashton 
Cc: Simon Ser 
Cc: Ville Syrjälä 
Cc: Melissa Wen 
Cc: dri-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index ca093396d1ac..dc99a8ffac70 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7238,6 +7238,12 @@ static int amdgpu_dm_connector_get_modes(struct 
drm_connector *connector)
return amdgpu_dm_connector->num_modes;
 }
 
+static const u32 supported_colorspaces =
+   BIT(DRM_MODE_COLORIMETRY_BT709_YCC) |
+   BIT(DRM_MODE_COLORIMETRY_OPRGB) |
+   BIT(DRM_MODE_COLORIMETRY_BT2020_RGB) |
+   BIT(DRM_MODE_COLORIMETRY_BT2020_YCC);
+
 void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 struct amdgpu_dm_connector *aconnector,
 int connector_type,
@@ -7318,6 +7324,15 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
adev->mode_info.abm_level_property, 0);
}
 
+   if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
+   if 
(!drm_mode_create_hdmi_colorspace_property(>base, 
supported_colorspaces))
+   
drm_connector_attach_colorspace_property(>base);
+   } else if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+  connector_type == DRM_MODE_CONNECTOR_eDP) {
+   if (!drm_mode_create_dp_colorspace_property(>base, 
supported_colorspaces))
+   
drm_connector_attach_colorspace_property(>base);
+   }
+
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
-- 
2.40.1