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 <[email protected]> > --- > 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
