| Message ID | 1461722609-32474-2-git-send-email-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 27, 2016 at 12:03:26PM +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > This takes a reference count when fbdev adds the connector, > and drops it when it removes the connector. > > It also drops the now unneeded code to find connectors > and remove the from the modeset as they are reference counted. > > Signed-off-by: Dave Airlie <airlied@redhat.com> Ok, I screwed up part of my first review, and add_all_connectors calls add_one_connector. That looks like we'll leak non-mst connectors when unloading i915. I think we need to add a loop to drm_fb_helper_fini and call drm_fb_helper_remove_one_connector on all the connectors still in the helper list. I think otherwise it looks good. Would be really good to give this is a spin with full debugging enabled when reloading i915.ko. Just to make sure I didn't miss anything. Or cc intel-gfx and CI should be able to pick it up and give it a spin. -Daniel > --- > drivers/gpu/drm/drm_fb_helper.c | 34 ++-------------------------------- > 1 file changed, 2 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 855108e..b2f58fb 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -153,40 +153,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_ > if (!fb_helper_connector) > return -ENOMEM; > > + drm_connector_reference(connector); > fb_helper_connector->connector = connector; > fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector; > return 0; > } > EXPORT_SYMBOL(drm_fb_helper_add_one_connector); > > -static void remove_from_modeset(struct drm_mode_set *set, > - struct drm_connector *connector) > -{ > - int i, j; > - > - for (i = 0; i < set->num_connectors; i++) { > - if (set->connectors[i] == connector) > - break; > - } > - > - if (i == set->num_connectors) > - return; > - > - for (j = i + 1; j < set->num_connectors; j++) { > - set->connectors[j - 1] = set->connectors[j]; > - } > - set->num_connectors--; > - > - /* > - * TODO maybe need to makes sure we set it back to !=NULL somewhere? > - */ > - if (set->num_connectors == 0) { > - set->fb = NULL; > - drm_mode_destroy(connector->dev, set->mode); > - set->mode = NULL; > - } > -} > - > int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, > struct drm_connector *connector) > { > @@ -206,6 +179,7 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, > if (i == fb_helper->connector_count) > return -EINVAL; > fb_helper_connector = fb_helper->connector_info[i]; > + drm_connector_unreference(fb_helper_connector->connector); > > for (j = i + 1; j < fb_helper->connector_count; j++) { > fb_helper->connector_info[j - 1] = fb_helper->connector_info[j]; > @@ -213,10 +187,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, > fb_helper->connector_count--; > kfree(fb_helper_connector); > > - /* also cleanup dangling references to the connector: */ > - for (i = 0; i < fb_helper->crtc_count; i++) > - remove_from_modeset(&fb_helper->crtc_info[i].mode_set, connector); > - > return 0; > } > EXPORT_SYMBOL(drm_fb_helper_remove_one_connector); > -- > 2.5.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..b2f58fb 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -153,40 +153,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_ if (!fb_helper_connector) return -ENOMEM; + drm_connector_reference(connector); fb_helper_connector->connector = connector; fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector; return 0; } EXPORT_SYMBOL(drm_fb_helper_add_one_connector); -static void remove_from_modeset(struct drm_mode_set *set, - struct drm_connector *connector) -{ - int i, j; - - for (i = 0; i < set->num_connectors; i++) { - if (set->connectors[i] == connector) - break; - } - - if (i == set->num_connectors) - return; - - for (j = i + 1; j < set->num_connectors; j++) { - set->connectors[j - 1] = set->connectors[j]; - } - set->num_connectors--; - - /* - * TODO maybe need to makes sure we set it back to !=NULL somewhere? - */ - if (set->num_connectors == 0) { - set->fb = NULL; - drm_mode_destroy(connector->dev, set->mode); - set->mode = NULL; - } -} - int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, struct drm_connector *connector) { @@ -206,6 +179,7 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, if (i == fb_helper->connector_count) return -EINVAL; fb_helper_connector = fb_helper->connector_info[i]; + drm_connector_unreference(fb_helper_connector->connector); for (j = i + 1; j < fb_helper->connector_count; j++) { fb_helper->connector_info[j - 1] = fb_helper->connector_info[j]; @@ -213,10 +187,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper, fb_helper->connector_count--; kfree(fb_helper_connector); - /* also cleanup dangling references to the connector: */ - for (i = 0; i < fb_helper->crtc_count; i++) - remove_from_modeset(&fb_helper->crtc_info[i].mode_set, connector); - return 0; } EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);