Re: [PATCH v3 09/17] drm/amd/display: Register Colorspace property for DP and HDMI

[Harry Wentland <harry.wentland@amd.com>]

On 3/8/23 04:24, Pekka Paalanen wrote:
> On Tue, 7 Mar 2023 10:10:59 -0500
> Harry Wentland <harry.wentland@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.
>>
>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>> Cc: Pekka Paalanen <ppaalanen@gmail.com>
>> Cc: Sebastian Wick <sebastian.wick@redhat.com>
>> Cc: Vitaly.Prosyak@amd.com
>> Cc: Joshua Ashton <joshua@froggi.es>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: amd-gfx@lists.freedesktop.org
>> Reviewed-By: Joshua Ashton <joshua@froggi.es>
>> ---
>>  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 f91b2ea13d96..2d883c6dae90 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -7184,6 +7184,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) |
>> +	BIT(DRM_MODE_COLORIMETRY_BT2020_DEPRECATED);
> 
> No DEFAULT?

DEFAULT is always added in drm_mode_create_colorspace_property.

> No BT.709 RGB, i.e. sRGB?
> 

No RGB variants for BT.709 or sRGB are explicitly defined in the API
(which is based on the infoframe values). Not trying to change things
up here.

You'll probably want to select "DEFAULT" if you want sRGB.

Harry

> Doesn't DRM core reject enum uint values that are not listed in the enum
> property?
> 
> 
> Thanks,
> pq
> 
>> +
>>  void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
>>  				     struct amdgpu_dm_connector *aconnector,
>>  				     int connector_type,
>> @@ -7264,6 +7270,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(&aconnector->base, supported_colorspaces))
>> +			drm_connector_attach_colorspace_property(&aconnector->base);
>> +	} else if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>> +		   connector_type == DRM_MODE_CONNECTOR_eDP) {
>> +		if (!drm_mode_create_dp_colorspace_property(&aconnector->base, supported_colorspaces))
>> +			drm_connector_attach_colorspace_property(&aconnector->base);
>> +	}
>> +
>>  	if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
>>  	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>>  	    connector_type == DRM_MODE_CONNECTOR_eDP) {
>