On Fri, Nov 29, 2024 at 06:12:01PM +0200, Imre Deak wrote: > On Fri, Nov 29, 2024 at 03:46:28PM +0100, Maxime Ripard wrote: > > On Fri, Nov 29, 2024 at 04:26:01PM +0200, Imre Deak wrote: > > > Adding more people from DRM core domain. Any comment, objection to this > > > change? > > > > > > On Tue, Nov 26, 2024 at 06:18:56PM +0200, Imre Deak wrote: > > > > Atm when the connector is added to the drm_mode_config::connector_list, > > > > the connector may not be fully initialized yet. This is not a problem > > > > for user space, which will see the connector only after it's registered > > > > later, it could be a problem for in-kernel users looking up connectors > > > > via the above list. > > > > It could be, or it actually is a problem? If so, in what situation? > > An actual problem is that after an MST connector is created and added to > the connector list, the connector could be probed without the connector > being fully initialized during a hotplug event handling along with all > the other connectors on the list. The connector's helper functions could > be still unset leading to a NULL deref while trying to call the > connector's detect/detect_ctx callbacks, or if these callbacks are set > already, the detect handler could see a connector which is not yet > initialized fully. Ack. Like I said to Jani, this needs to be in the commit log then. > > > > To resolve the above issue, add a way to separately initialize the DRM > > > > core specific parts of the connector and add it to the above list. This > > > > will move adding the connector to the list after the properties on the > > > > connector have been added, this is ok since these steps don't have a > > > > dependency. > > > > > > > > v2: (Jani) > > > > - Let initing DDC as well via drm_connector_init_core(). > > > > - Rename __drm_connector_init to drm_connector_init_core_and_add(). > > > > > > > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> (v1) > > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > > If we do have an actual problem to solve, we'll need kunit tests too. > > I don't have a good idea for this. The problem is not in a parituclar > function, rather in the order a connector is initialized and added to > the connector list. The above is an actual problem though, so I don't > think fixing that should be blocked by this. It's not about whether we have a problem or not: you introduce new framework functions, you need to have kunit tests to check their behaviour. Maxime
Attachment:
signature.asc
Description: PGP signature
