Hey, For the series: Reviewed-by: Maarten Lankhorst <[email protected]>
Would really love it if atomic_sro.[ch] is renamed to atomic_state_readout.[ch], so we have a better idea what it's about. :-) 2 things to note: - Hardware readout may be succesful, but the resulting state may not be valid. Think for example a crtc with a DP-MST connector, but the DP-MST connector is not yet detected at time of readout. crtc_state is invalid because connector_mask = 0. It would be awesome if DP-MST would be detected before this point. I'm not sure this is actually possible but it would make fastboot work on DP-MST as well. - Alternatively some hardware may be connected to the wrong plane. Older i915 cards had a flexible mapping of plane 1 being connected to crtc 1 or 2, but for optimal performance you'd want it to be plane 1 to crtc 2. BIOS chose plane 1 to crtc 1. Kind regards, ~Maarten Lankhorst On 10/15/26 17:09, Maxime Ripard wrote: > drm_bridge_connector allows to create a generic connector from a list of > bridges. > > However, it's a somewhat virtual connector, and relies on the bridges to > implement its various capabilities. > > What we actually want though is for the last bridge implementing > hardware readout to fill the connector state from its own state. > > Thus, implement a new op for bridge_connector to allow just > that. > > Signed-off-by: Maxime Ripard <[email protected]> > --- > drivers/gpu/drm/display/drm_bridge_connector.c | 35 > ++++++++++++++++++++++++++ > include/drm/drm_bridge.h | 27 ++++++++++++++++++++ > 2 files changed, 62 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c > b/drivers/gpu/drm/display/drm_bridge_connector.c > index 81f3e26f6fdf..dfc633bd8823 100644 > --- a/drivers/gpu/drm/display/drm_bridge_connector.c > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c > @@ -8,10 +8,11 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/property.h> > #include <linux/slab.h> > > +#include <drm/drm_atomic_sro_helper.h> > #include <drm/drm_atomic_state_helper.h> > #include <drm/drm_bridge.h> > #include <drm/drm_bridge_connector.h> > #include <drm/drm_connector.h> > #include <drm/drm_device.h> > @@ -65,10 +66,18 @@ struct drm_bridge_connector { > * @encoder: > * > * The encoder at the start of the bridges chain. > */ > struct drm_encoder *encoder; > + /** > + * @bridge_connector_hw_readout: > + * > + * The last bridge in the chain (closest to the connector) that > + * provides hardware state readout support, if any (see > + * &DRM_BRIDGE_OP_CONNECTOR_HW_READOUT). > + */ > + struct drm_bridge *bridge_connector_hw_readout; > /** > * @bridge_edid: > * > * The last bridge in the chain (closest to the connector) that provides > * EDID read support, if any (see &DRM_BRIDGE_OP_EDID). > @@ -242,10 +251,30 @@ drm_bridge_connector_create_state(struct drm_connector > *connector) > conn_state); > > return conn_state; > } > > +static int > +drm_bridge_connector_readout_state(struct drm_connector *connector, > + struct drm_atomic_sro_state *state, > + struct drm_connector_state *conn_state) > +{ > + struct drm_bridge_connector *bridge_connector = > + to_drm_bridge_connector(connector); > + struct drm_bridge *readout = > + bridge_connector->bridge_connector_hw_readout; > + int ret; > + > + if (readout) { > + ret = readout->funcs->atomic_sro_connector_readout(readout, > state, conn_state); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > static enum drm_connector_color_format > drm_bridge_connector_color_format(const struct drm_connector_state > *conn_state) > { > struct drm_bridge_connector *bridge_connector = > to_drm_bridge_connector(conn_state->connector); > @@ -268,10 +297,12 @@ drm_bridge_connector_color_format(const struct > drm_connector_state *conn_state) > } > > static const struct drm_connector_funcs drm_bridge_connector_funcs = { > .fill_modes = drm_helper_probe_single_connector_modes, > .atomic_create_state = drm_bridge_connector_create_state, > + .atomic_sro_readout_state = drm_bridge_connector_readout_state, > + .atomic_sro_compare_state = drm_atomic_helper_connector_compare_state, > .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > .debugfs_init = drm_bridge_connector_debugfs_init, > .oob_hotplug_event = drm_bridge_connector_oob_hotplug_event, > .color_format = drm_bridge_connector_color_format, > @@ -894,10 +925,14 @@ struct drm_connector *drm_bridge_connector_init(struct > drm_device *drm, > if (!bridge->interlace_allowed) > connector->interlace_allowed = false; > if (!bridge->ycbcr_420_allowed) > connector->ycbcr_420_allowed = false; > > + if (bridge->ops & DRM_BRIDGE_OP_CONNECTOR_HW_READOUT) { > + > drm_bridge_put(bridge_connector->bridge_connector_hw_readout); > + bridge_connector->bridge_connector_hw_readout = > drm_bridge_get(bridge); > + } > /* > * Ensure the last bridge declares OP_EDID or OP_MODES or both. > */ > if (bridge->ops & DRM_BRIDGE_OP_EDID || bridge->ops & > DRM_BRIDGE_OP_MODES) { > drm_bridge_put(bridge_connector->bridge_edid); > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index fcccd8f6403d..7d6aaba0b82c 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -935,10 +935,30 @@ struct drm_bridge_funcs { > */ > int (*dp_audio_mute_stream)(struct drm_bridge *bridge, > struct drm_connector *connector, > bool enable, int direction); > > + /** > + * @atomic_sro_connector_readout: > + * > + * This optional hook initializes the &struct drm_connector_state > + * based on hardware state. > + * > + * It is implemented by bridges that set the > + * %DRM_BRIDGE_OP_CONNECTOR_HW_READOUT flag in their > + * &drm_bridge.ops. When using drm_bridge_connector, the last > + * bridge in the chain with this flag set will have its hook > + * called to fill the connector state. > + * > + * RETURNS: > + * > + * 0 on success, a negative error code otherwise. > + */ > + int (*atomic_sro_connector_readout)(struct drm_bridge *bridge, > + struct drm_atomic_sro_state *state, > + struct drm_connector_state > *conn_state); > + > /** > * @debugfs_init: > * > * Allows bridges to create bridge-specific debugfs files. > */ > @@ -1075,10 +1095,17 @@ enum drm_bridge_ops { > * @DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME: The bridge supports > * &drm_bridge_funcs->hdmi_write_spd_infoframe and > * &drm_bridge_funcs->hdmi_clear_spd_infoframe callbacks. > */ > DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME = BIT(10), > + /** > + * @DRM_BRIDGE_OP_CONNECTOR_HW_READOUT: The bridge supports the > + * &drm_bridge_funcs.atomic_sro_connector_readout callback to > + * fill the connector state from the bridge's own hardware state > + * during state readout. > + */ > + DRM_BRIDGE_OP_CONNECTOR_HW_READOUT = BIT(11), > }; > > /** > * struct drm_bridge - central DRM bridge control structure > */ >
