| Message ID | 20250311-hdmi-conn-yuv-v2-2-fbdb94f02562@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/connector: hdmi: Allow using the YUV420 output format | expand |
On Tue, Mar 11, 2025 at 12:57:34PM +0200, Cristian Ciocaltea wrote: > Provide the necessary constraints verification in > sink_supports_format_bpc() in order to support handling of YUV420 > output format. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 40 +++++++++++++++++++++++-- > 1 file changed, 37 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > index 6bc96d5d1ab9115989e208d9899e16cd22254fb6..e99d868edc1854eddc5ebf8692ccffb9e2338268 100644 > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > @@ -3,6 +3,7 @@ > #include <drm/drm_atomic.h> > #include <drm/drm_connector.h> > #include <drm/drm_edid.h> > +#include <drm/drm_modes.h> > #include <drm/drm_print.h> > > #include <drm/display/drm_hdmi_audio_helper.h> > @@ -115,6 +116,12 @@ sink_supports_format_bpc(const struct drm_connector *connector, > return false; > } > > + if (drm_mode_is_420_only(info, mode) && format != HDMI_COLORSPACE_YUV420) { > + drm_dbg_kms(dev, "%s format unsupported by the sink for VIC%u.\n", > + drm_hdmi_connector_get_output_format_name(format), vic); We don't necessarily have a VIC for the mode we pass, so it's not super useful to pass it. I'd rather mention that the mode is supposed to be YUV420 only, but the format isn't YUV420. > + return false; > + } > + > switch (format) { > case HDMI_COLORSPACE_RGB: > drm_dbg_kms(dev, "RGB Format, checking the constraints.\n"); > @@ -145,9 +152,36 @@ sink_supports_format_bpc(const struct drm_connector *connector, > return true; > > case HDMI_COLORSPACE_YUV420: > - /* TODO: YUV420 is unsupported at the moment. */ > - drm_dbg_kms(dev, "YUV420 format isn't supported yet.\n"); > - return false; > + drm_dbg_kms(dev, "YUV420 format, checking the constraints.\n"); > + > + if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR420)) { > + drm_dbg_kms(dev, "Sink doesn't support YUV420.\n"); > + return false; > + } > + > + if (!drm_mode_is_420(info, mode)) { > + drm_dbg_kms(dev, "Sink doesn't support YUV420 for VIC%u.\n", vic); Again, we shouldn't print the VIC here. There's a printk format we can use to print drm_display_mode if you want to, but we should keep things consistent. Maxime
On 3/11/25 5:30 PM, Maxime Ripard wrote: > On Tue, Mar 11, 2025 at 12:57:34PM +0200, Cristian Ciocaltea wrote: >> Provide the necessary constraints verification in >> sink_supports_format_bpc() in order to support handling of YUV420 >> output format. >> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> drivers/gpu/drm/display/drm_hdmi_state_helper.c | 40 +++++++++++++++++++++++-- >> 1 file changed, 37 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c >> index 6bc96d5d1ab9115989e208d9899e16cd22254fb6..e99d868edc1854eddc5ebf8692ccffb9e2338268 100644 >> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c >> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c >> @@ -3,6 +3,7 @@ >> #include <drm/drm_atomic.h> >> #include <drm/drm_connector.h> >> #include <drm/drm_edid.h> >> +#include <drm/drm_modes.h> >> #include <drm/drm_print.h> >> >> #include <drm/display/drm_hdmi_audio_helper.h> >> @@ -115,6 +116,12 @@ sink_supports_format_bpc(const struct drm_connector *connector, >> return false; >> } >> >> + if (drm_mode_is_420_only(info, mode) && format != HDMI_COLORSPACE_YUV420) { >> + drm_dbg_kms(dev, "%s format unsupported by the sink for VIC%u.\n", >> + drm_hdmi_connector_get_output_format_name(format), vic); > > We don't necessarily have a VIC for the mode we pass, so it's not super > useful to pass it. I'd rather mention that the mode is supposed to be > YUV420 only, but the format isn't YUV420. Ack, I'll change the message as suggested. >> + return false; >> + } >> + >> switch (format) { >> case HDMI_COLORSPACE_RGB: >> drm_dbg_kms(dev, "RGB Format, checking the constraints.\n"); >> @@ -145,9 +152,36 @@ sink_supports_format_bpc(const struct drm_connector *connector, >> return true; >> >> case HDMI_COLORSPACE_YUV420: >> - /* TODO: YUV420 is unsupported at the moment. */ >> - drm_dbg_kms(dev, "YUV420 format isn't supported yet.\n"); >> - return false; >> + drm_dbg_kms(dev, "YUV420 format, checking the constraints.\n"); >> + >> + if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR420)) { >> + drm_dbg_kms(dev, "Sink doesn't support YUV420.\n"); >> + return false; >> + } >> + >> + if (!drm_mode_is_420(info, mode)) { >> + drm_dbg_kms(dev, "Sink doesn't support YUV420 for VIC%u.\n", vic); > > Again, we shouldn't print the VIC here. There's a printk format we can > use to print drm_display_mode if you want to, but we should keep things > consistent. Agree, will proceed as above. Thanks for the review, Cristian
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c index 6bc96d5d1ab9115989e208d9899e16cd22254fb6..e99d868edc1854eddc5ebf8692ccffb9e2338268 100644 --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c @@ -3,6 +3,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_connector.h> #include <drm/drm_edid.h> +#include <drm/drm_modes.h> #include <drm/drm_print.h> #include <drm/display/drm_hdmi_audio_helper.h> @@ -115,6 +116,12 @@ sink_supports_format_bpc(const struct drm_connector *connector, return false; } + if (drm_mode_is_420_only(info, mode) && format != HDMI_COLORSPACE_YUV420) { + drm_dbg_kms(dev, "%s format unsupported by the sink for VIC%u.\n", + drm_hdmi_connector_get_output_format_name(format), vic); + return false; + } + switch (format) { case HDMI_COLORSPACE_RGB: drm_dbg_kms(dev, "RGB Format, checking the constraints.\n"); @@ -145,9 +152,36 @@ sink_supports_format_bpc(const struct drm_connector *connector, return true; case HDMI_COLORSPACE_YUV420: - /* TODO: YUV420 is unsupported at the moment. */ - drm_dbg_kms(dev, "YUV420 format isn't supported yet.\n"); - return false; + drm_dbg_kms(dev, "YUV420 format, checking the constraints.\n"); + + if (!(info->color_formats & DRM_COLOR_FORMAT_YCBCR420)) { + drm_dbg_kms(dev, "Sink doesn't support YUV420.\n"); + return false; + } + + if (!drm_mode_is_420(info, mode)) { + drm_dbg_kms(dev, "Sink doesn't support YUV420 for VIC%u.\n", vic); + return false; + } + + if (bpc == 10 && !(info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_30)) { + drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n"); + return false; + } + + if (bpc == 12 && !(info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_36)) { + drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n"); + return false; + } + + if (bpc == 16 && !(info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_48)) { + drm_dbg_kms(dev, "16 BPC but sink doesn't support Deep Color 48.\n"); + return false; + } + + drm_dbg_kms(dev, "YUV420 format supported in that configuration.\n"); + + return true; case HDMI_COLORSPACE_YUV422: drm_dbg_kms(dev, "YUV422 format, checking the constraints.\n");
Provide the necessary constraints verification in sink_supports_format_bpc() in order to support handling of YUV420 output format. Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- drivers/gpu/drm/display/drm_hdmi_state_helper.c | 40 +++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-)