Re: [PATCH v3 03/11] drm/connector: Add deprication notes for drm_connector_register/unregister

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 12 Dec 2024, Imre Deak <imre.deak@intel.com> wrote:
> Drivers should register/unregister only dynamic (MST) connectors
> manually using drm_connector_dynamic_register()/unregister().
> Static connectors are registered/unregistered by the DRM core
> automatically. Some drivers still call drm_connector_register()/
> unregister() for static connectors, both of which should be a nop
> for them and hence are scheduled to be removed. Update the function
> documentation for these functions accordingly.

Typo *deprecation in subject, nitpick inline,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index a53e5629ba6c1..c322dbf6e3161 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -767,14 +767,17 @@ EXPORT_SYMBOL(drm_connector_cleanup);
>   * drm_connector_register - register a connector
>   * @connector: the connector to register
>   *
> - * Register userspace interfaces for a connector. Only call this for connectors
> - * which can be hotplugged after drm_dev_register() has been called already,
> - * e.g. DP MST connectors. All other connectors will be registered automatically
> - * when calling drm_dev_register().
> + * Register userspace interfaces for a connector. Drivers shouldn't call this
> + * function. Static connectors will be registered automatically by DRM core
> + * from drm_dev_register(), dynamic connectors (MST) should be registered by
> + * drivers calling drm_connector_dynamic_register().
>   *
>   * When the connector is no longer available, callers must call
>   * drm_connector_unregister().
>   *
> + * Note: Existing uses of this function in drivers should be a nop already and
> + * are scheduled to be removed.
> + *
>   * Returns:
>   * Zero on success, error code on failure.
>   */
> @@ -863,9 +866,14 @@ EXPORT_SYMBOL(drm_connector_dynamic_register);
>   * drm_connector_unregister - unregister a connector
>   * @connector: the connector to unregister
>   *
> - * Unregister userspace interfaces for a connector. Only call this for
> - * connectors which have been registered explicitly by calling
> - * drm_connector_register().
> + * Unregister userspace interfaces for a connector. Drivers should call this
> + * for dynamic connectors (MST) only, which were registered explicitly by
> + * calling drm_connector_dynamic_register(). All other - static - connectors
> + * will be unregistered automatically by DRM core and drivers shouldn't call
> + * this function for those.

This kind of supports my point about a single
drm_connector_register(). There's no
drm_connector_dynamic_unregister(). After all the
drm_connector_register() calls have been removed, we're left with the
asymmetric pair:

- drm_connector_dynamic_register()
- drm_connector_unregister()

Then again, all of these should become internal and not for drivers?

> + *
> + * Note: Existing uses of this function in drivers for static connectors
> + * should be a nop already and are scheduled to be removed.
>   */
>  void drm_connector_unregister(struct drm_connector *connector)
>  {

-- 
Jani Nikula, Intel