[PATCH v8 02/39] drm/connector: Add caps-based HDMI connector init helper
Dmitry Baryshkov
dmitry.baryshkov at oss.qualcomm.com
Fri Jul 3 07:05:10 PDT 2026
On Thu, Jul 02, 2026 at 05:46:15PM +0300, Cristian Ciocaltea wrote:
> In preparation for adding HDMI 2.x source capabilities, introduce struct
> drm_connector_hdmi_caps and a new drmm_connector_hdmi_init_with_caps()
> helper.
>
> The existing drmm_connector_hdmi_init() helper currently takes
> individual capability arguments such as supported_formats and max_bpc.
> Adding more HDMI-specific arguments to that function would not scale
> well, so move those values into a dedicated capabilities structure and
> implement the existing helper as a wrapper around the new caps-based
> interface.
I think, it was an intention of Maxime: make sure that every driver is
forced to provide some values here. With the struct-based init it is
easy to overlook or to ommit a value.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea at collabora.com>
> ---
> drivers/gpu/drm/drm_connector.c | 202 +++++++++++++++++++++++++++-------------
> include/drm/drm_connector.h | 55 +++++++++++
> 2 files changed, 193 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8b4baed060f3..c7ce6b7bd8b0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -542,6 +542,137 @@ int drmm_connector_init(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drmm_connector_init);
>
> +/**
> + * drmm_connector_hdmi_init_with_caps - Init a preallocated HDMI connector
> + * @dev: DRM device
> + * @connector: A pointer to the HDMI connector to init
> + * @vendor: HDMI Controller Vendor name
> + * @product: HDMI Controller Product name
> + * @funcs: callbacks for this connector
> + * @hdmi_funcs: HDMI-related callbacks for this connector
> + * @connector_type: user visible type of the connector
> + * @ddc: optional pointer to the associated ddc adapter
> + * @caps: optional HDMI connector capabilities
> + *
> + * Initialises a preallocated HDMI connector. Connectors can be
> + * subclassed as part of driver connector objects.
> + *
> + * Cleanup is automatically handled with a call to
> + * drm_connector_cleanup() in a DRM-managed action.
> + *
> + * The connector structure should be allocated with drmm_kzalloc().
> + *
> + * The @drm_connector_funcs.destroy hook must be NULL.
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drmm_connector_hdmi_init_with_caps(struct drm_device *dev,
> + struct drm_connector *connector,
> + const char *vendor, const char *product,
> + const struct drm_connector_funcs *funcs,
> + const struct drm_connector_hdmi_funcs *hdmi_funcs,
> + int connector_type,
> + struct i2c_adapter *ddc,
> + const struct drm_connector_hdmi_caps *caps)
> +{
> + int ret;
> +
> + if (!vendor || !product)
> + return -EINVAL;
> +
> + if ((strlen(vendor) > DRM_CONNECTOR_HDMI_VENDOR_LEN) ||
> + (strlen(product) > DRM_CONNECTOR_HDMI_PRODUCT_LEN))
> + return -EINVAL;
> +
> + if (!(connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> + connector_type == DRM_MODE_CONNECTOR_HDMIB))
> + return -EINVAL;
> +
> + if (!caps)
> + return -EINVAL;
> +
> + if (!caps->supported_formats ||
> + !(caps->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444)))
> + return -EINVAL;
> +
> + if (connector->ycbcr_420_allowed !=
> + !!(caps->supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420)))
> + return -EINVAL;
> +
> + if (!(caps->max_bpc == 8 || caps->max_bpc == 10 || caps->max_bpc == 12))
> + return -EINVAL;
> +
> + if (!hdmi_funcs->avi.clear_infoframe ||
> + !hdmi_funcs->avi.write_infoframe ||
> + !hdmi_funcs->hdmi.clear_infoframe ||
> + !hdmi_funcs->hdmi.write_infoframe)
> + return -EINVAL;
> +
> + ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
> + if (ret)
> + return ret;
> +
> + connector->hdmi.supported_formats = caps->supported_formats;
> +
> + /*
> + * The supported HDMI version can be used to infer the maximum TMDS
> + * character rate allowed by the specification. Some controllers,
> + * however, may support a lower rate than that version would imply.
> + *
> + * A non-zero caps->max_tmds_char_rate lets drivers override the
> + * inferred limit with the actual controller capability. A value of
> + * zero keeps the default limit inferred from supported_hdmi_ver.
> + */
> + if (caps->supported_hdmi_ver >= HDMI_VERSION_2_0)
> + connector->hdmi.max_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
> + else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_3)
> + connector->hdmi.max_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> + else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_0)
> + connector->hdmi.max_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
> +
> + if (caps->max_tmds_char_rate) {
> + if (caps->max_tmds_char_rate > connector->hdmi.max_tmds_char_rate)
> + return -EINVAL;
> + connector->hdmi.max_tmds_char_rate = caps->max_tmds_char_rate;
> + }
Here I'd totally suggest something simpler:
if (!caps->max_tmds_char_rate) {
if (caps->supported_hdmi_ver >= HDMI_VERSION_2_0)
connector->hdmi.max_tmds_char_rate = HDMI_2_0_TMDS_CHAR_RATE_MAX_HZ;
else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_3)
connector->hdmi.max_tmds_char_rate = HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
else if (caps->supported_hdmi_ver >= HDMI_VERSION_1_0)
connector->hdmi.max_tmds_char_rate = HDMI_1_0_TMDS_CHAR_RATE_MAX_HZ;
}
> +
> + strtomem_pad(connector->hdmi.vendor, vendor, 0);
> + strtomem_pad(connector->hdmi.product, product, 0);
> +
> + /*
> + * drm_connector_attach_max_bpc_property() requires the
> + * connector to have a state.
> + */
> + if (connector->funcs->atomic_create_state) {
> + struct drm_connector_state *state;
> +
> + state = connector->funcs->atomic_create_state(connector);
> + if (IS_ERR(state))
> + return PTR_ERR(state);
> +
> + connector->state = state;
> + } else if (connector->funcs->reset) {
> + connector->funcs->reset(connector);
> + }
> +
> + drm_connector_attach_max_bpc_property(connector, 8, caps->max_bpc);
> + connector->max_bpc = caps->max_bpc;
If you don't like the max_bpc argument, let's do it other way:
if (connector->max_bpc < 8)
return -EINVAL;
if (connector->max_bpc > 8)
drm_connector_attach_hdr_output_metadata_property(connector);
> +
> + if (caps->max_bpc > 8)
> + drm_connector_attach_hdr_output_metadata_property(connector);
> +
> + ret = drm_connector_attach_color_format_property(connector,
> + caps->supported_formats);
> + if (ret)
> + return ret;
> +
> + connector->hdmi.funcs = hdmi_funcs;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drmm_connector_hdmi_init_with_caps);
> +
> /**
> * drmm_connector_hdmi_init - Init a preallocated HDMI connector
> * @dev: DRM device
> @@ -578,71 +709,14 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
> unsigned long supported_formats,
> unsigned int max_bpc)
> {
> - int ret;
> + struct drm_connector_hdmi_caps caps = {
> + .supported_formats = supported_formats,
> + .max_bpc = max_bpc,
> + };
>
> - if (!vendor || !product)
> - return -EINVAL;
> -
> - if ((strlen(vendor) > DRM_CONNECTOR_HDMI_VENDOR_LEN) ||
> - (strlen(product) > DRM_CONNECTOR_HDMI_PRODUCT_LEN))
> - return -EINVAL;
> -
> - if (!(connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> - connector_type == DRM_MODE_CONNECTOR_HDMIB))
> - return -EINVAL;
> -
> - if (!supported_formats || !(supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444)))
> - return -EINVAL;
> -
> - if (connector->ycbcr_420_allowed != !!(supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420)))
> - return -EINVAL;
> -
> - if (!(max_bpc == 8 || max_bpc == 10 || max_bpc == 12))
> - return -EINVAL;
> -
> - if (!hdmi_funcs->avi.clear_infoframe ||
> - !hdmi_funcs->avi.write_infoframe ||
> - !hdmi_funcs->hdmi.clear_infoframe ||
> - !hdmi_funcs->hdmi.write_infoframe)
> - return -EINVAL;
> -
> - ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
> - if (ret)
> - return ret;
> -
> - connector->hdmi.supported_formats = supported_formats;
> - strtomem_pad(connector->hdmi.vendor, vendor, 0);
> - strtomem_pad(connector->hdmi.product, product, 0);
> -
> - /*
> - * drm_connector_attach_max_bpc_property() requires the
> - * connector to have a state.
> - */
> - if (connector->funcs->atomic_create_state) {
> - struct drm_connector_state *state;
> -
> - state = connector->funcs->atomic_create_state(connector);
> - if (IS_ERR(state))
> - return PTR_ERR(state);
> -
> - connector->state = state;
> - } else if (connector->funcs->reset) {
> - connector->funcs->reset(connector);
> - }
> -
> - drm_connector_attach_max_bpc_property(connector, 8, max_bpc);
> - connector->max_bpc = max_bpc;
> -
> - if (max_bpc > 8)
> - drm_connector_attach_hdr_output_metadata_property(connector);
> -
> - ret = drm_connector_attach_color_format_property(connector, supported_formats);
> - if (ret)
> - return ret;
> -
> - connector->hdmi.funcs = hdmi_funcs;
> -
> - return 0;
> + return drmm_connector_hdmi_init_with_caps(dev, connector, vendor,
> + product, funcs, hdmi_funcs,
> + connector_type, ddc, &caps);
> }
> EXPORT_SYMBOL(drmm_connector_hdmi_init);
>
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index af075c38f4db..961a729d0869 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2029,6 +2029,47 @@ struct drm_connector_hdmi_audio {
> int dai_port;
> };
>
> +/**
> + * struct drm_connector_hdmi_caps - HDMI capabilities structure
> + *
> + * This structure is required to initialize the connector using
> + * drmm_connector_hdmi_init_with_caps().
> + */
> +struct drm_connector_hdmi_caps {
> + /**
> + * @supported_hdmi_ver:
> + *
> + * Maximum HDMI specification version supported by the controller side
> + * of this connector. This describes the controller capability only;
> + * the effective link capabilities may be further restricted by the
> + * sink, bridge, or mode validation.
> + *
> + * HDMI_VERSION_UNKNOWN means legacy/default behaviour.
What would it actually mean here? Every chip has some version of HDMI
standard it conforms to.
> + */
> + enum hdmi_version supported_hdmi_ver;
> +
> + /**
> + * @supported_formats:
> + *
> + * Bitmask of @drm_output_color_format listing supported output formats.
> + */
> + unsigned long supported_formats;
> +
> + /**
> + * @max_tmds_char_rate:
> + *
> + * Maximum TMDS character rate supported by the source, in Hz.
> + * A value of 0 means the core should use the default limit implied by
> + * @supported_hdmi_ver.
> + */
> + unsigned long long max_tmds_char_rate;
> +
> + /**
> + * @max_bpc: Maximum bits per char the HDMI connector supports.
> + */
> + unsigned int max_bpc;
> +};
> +
> /*
> * struct drm_connector_hdmi - DRM Connector HDMI-related structure
> */
--
With best wishes
Dmitry
More information about the linux-arm-kernel
mailing list