On Tue, 29 Aug 2023, Ville Syrjala <ville.syrj...@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrj...@linux.intel.com> > > Currently drm_sysfs_connector_add() attempts to register > the "ddc" symlink (based one connector->ddc) before the > driver's .early_register() hook has been called. That is > too early for i915 which only fully registers the aux ch > and associated i2c bus from said hook (to prevent half > initialized stuff getting exposed to userspace). This > causes my attempt at using drm_connector_init_with_ddc() > to fail, and the entire connector disappears from sysfs > on account of sysfs_create_link() failing. > > To fix that split the sysfs symlink stuff into separate > functions (drm_sysfs_connector_add_late() and > drm_sysfs_connector_remove_early()) which are called > on the opposite side of the .later_register() and > .early_unregister() hooks. > > Cc: Andrzej Pietrasiewicz <andrze...@collabora.com> > Cc: Daniel Vetter <daniel.vet...@ffwll.ch> > Cc: Andrzej Hajda <a.ha...@samsung.com> > Cc: Emil Velikov <emil.veli...@collabora.com> > Cc: Sam Ravnborg <s...@ravnborg.org> > Cc: Neil Armstrong <narmstr...@baylibre.com> > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nik...@intel.com> > --- > drivers/gpu/drm/drm_connector.c | 9 +++++++++ > drivers/gpu/drm/drm_internal.h | 2 ++ > drivers/gpu/drm/drm_sysfs.c | 22 +++++++++++++++------- > 3 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 988996cf6da5..9d4c7b0c5c05 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -631,6 +631,10 @@ int drm_connector_register(struct drm_connector > *connector) > goto err_debugfs; > } > > + ret = drm_sysfs_connector_add_late(connector); > + if (ret) > + goto err_late_register; > + > drm_mode_object_register(connector->dev, &connector->base); > > connector->registration_state = DRM_CONNECTOR_REGISTERED; > @@ -647,6 +651,9 @@ int drm_connector_register(struct drm_connector > *connector) > mutex_unlock(&connector_list_lock); > goto unlock; > > +err_late_register: > + if (connector->funcs->early_unregister) > + connector->funcs->early_unregister(connector); > err_debugfs: > drm_debugfs_connector_remove(connector); > drm_sysfs_connector_remove(connector); > @@ -681,6 +688,8 @@ void drm_connector_unregister(struct drm_connector > *connector) > connector->privacy_screen, > &connector->privacy_screen_notifier); > > + drm_sysfs_connector_remove_early(connector); > + > if (connector->funcs->early_unregister) > connector->funcs->early_unregister(connector); > > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > index ba12acd55139..4053cf8105ce 100644 > --- a/drivers/gpu/drm/drm_internal.h > +++ b/drivers/gpu/drm/drm_internal.h > @@ -153,6 +153,8 @@ int drm_sysfs_init(void); > void drm_sysfs_destroy(void); > struct device *drm_sysfs_minor_alloc(struct drm_minor *minor); > int drm_sysfs_connector_add(struct drm_connector *connector); > +int drm_sysfs_connector_add_late(struct drm_connector *connector); > +void drm_sysfs_connector_remove_early(struct drm_connector *connector); > void drm_sysfs_connector_remove(struct drm_connector *connector); > > void drm_sysfs_lease_event(struct drm_device *dev); > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index b169b3e44a92..a953f69a34b6 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -400,10 +400,6 @@ int drm_sysfs_connector_add(struct drm_connector > *connector) > drm_err(dev, "failed to add component to create link to > typec connector\n"); > } > > - if (connector->ddc) > - return sysfs_create_link(&connector->kdev->kobj, > - &connector->ddc->dev.kobj, "ddc"); > - > return 0; > > err_free: > @@ -411,14 +407,26 @@ int drm_sysfs_connector_add(struct drm_connector > *connector) > return r; > } > > +int drm_sysfs_connector_add_late(struct drm_connector *connector) > +{ > + if (connector->ddc) > + return sysfs_create_link(&connector->kdev->kobj, > + &connector->ddc->dev.kobj, "ddc"); > + > + return 0; > +} > + > +void drm_sysfs_connector_remove_early(struct drm_connector *connector) > +{ > + if (connector->ddc) > + sysfs_remove_link(&connector->kdev->kobj, "ddc"); > +} > + > void drm_sysfs_connector_remove(struct drm_connector *connector) > { > if (!connector->kdev) > return; > > - if (connector->ddc) > - sysfs_remove_link(&connector->kdev->kobj, "ddc"); > - > if (dev_fwnode(connector->kdev)) > component_del(connector->kdev, &typec_connector_ops); -- Jani Nikula, Intel Open Source Graphics Center