| Message ID | 20230525191809.3524-9-harry.wentland@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable Colorspace connector property in amdgpu | expand |
On Thu, May 25, 2023 at 9:18 PM 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. 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 <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: Simon Ser <contact@emersion.fr> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Melissa Wen <mwen@igalia.com> > Cc: dri-devel@lists.freedesktop.org > Cc: amd-gfx@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(&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) { > -- > 2.40.1 >
On Friday, May 26th, 2023 at 18:40, Sebastian Wick <sebastian.wick@redhat.com> wrote: > On Thu, May 25, 2023 at 9:18 PM 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. > > > 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.
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(&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) {
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: Simon Ser <contact@emersion.fr> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Melissa Wen <mwen@igalia.com> Cc: dri-devel@lists.freedesktop.org Cc: amd-gfx@lists.freedesktop.org --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)