On Wed, Apr 27, 2016 at 12:03:26PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
