[PATCH v2 1/4] drm/dp: Add a way to init/add a connector in separate steps

[Maxime Ripard]

On Fri, Nov 29, 2024 at 05:58:56PM +0200, Jani Nikula wrote:
> On Fri, 29 Nov 2024, Maxime Ripard <mripard at kernel.org> 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?
> 
> It's an actual problem.
> 
> While in most cases connectors are created at probe, for DP MST they're
> created at runtime via the ->add_connector hook. We want to call
> drm_connector_init() on them soon in that hook, so we can pass the
> connector around and expect it to have connector->dev etc. initialized
> while we continue its initialization.
> 
> But there's a race. As soon as we call drm_connector_init(), the
> connector gets added to dev->mode_config.connector_list, and any drm
> code may discover it. For example connector polling. And we might not be
> done with the initialization yet.

Ack. This should 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 at intel.com>
> >> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com> (v1)
> >> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> >
> > If we do have an actual problem to solve, we'll need kunit tests too.
> 
> That's not an unreasonable request, per se, but what exactly should they
> test? That the new init core didn't add stuff to the list, and the new
> add connector did?

Everything we test with drm_connector_init* already, plus the fact that
we now might have callers that call add without init, so we need to test
that too.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 273 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20241202/ef2b4fb8/attachment.sig>