Re: [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 26, 2016 at 10:52:24AM +0100, Chris Wilson wrote:
> On Wed, Oct 26, 2016 at 12:05:53PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The fbdev helper code keeps around two lists of connectors. One is the
> > list of all connectors it could use, and that list already holds
> > references for all the connectors. However the other list, or rather
> > lists, is the one actively being used. That list is tracked per-crtc
> > and currently doesn't hold any extra references. Let's grab those
> > extra references to avoid oopsing when the connector vanishes. The
> > list of all possible connectors should get updated when the hpd happens,
> > but the list of actively used connectors would not get updated until
> > the next time the fb-helper picks through the set of possible connectors.
> > And so we need to hang on to the connectors until that time.
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Carlos Santa <carlos.santa@intel.com>
> > Cc: Kirill A. Shutemov <kirill@shutemov.name>
> > Tested-by: Carlos Santa <carlos.santa@intel.com>
> > Tested-by: Kirill A. Shutemov <kirill@shutemov.name>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 25 ++++++++++++++++++-------
> >  1 file changed, 18 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index db469d12d195..9ee1dacf97b8 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2090,7 +2090,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
> >  	struct drm_fb_helper_crtc **crtcs;
> >  	struct drm_display_mode **modes;
> >  	struct drm_fb_offset *offsets;
> > -	struct drm_mode_set *modeset;
> >  	bool *enabled;
> >  	int width, height;
> >  	int i;
> > @@ -2139,7 +2138,14 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
> >  	/* need to set the modesets up here for use later */
> >  	/* fill out the connector<->crtc mappings into the modesets */
> >  	for (i = 0; i < fb_helper->crtc_count; i++) {
> > -		modeset = &fb_helper->crtc_info[i].mode_set;
> > +		struct drm_mode_set *modeset = &fb_helper->crtc_info[i].mode_set;
> > +		int j;
> > +
> > +		for (j = 0; j < modeset->num_connectors; j++) {
> > +			drm_connector_unreference(modeset->connectors[j]);
> > +			modeset->connectors[j] = NULL;
> > +		}
> > +
> 
> Don't we also need a similar cleanup loop in drm_fb_helper_crtc_free()?

Sounds likely. I'll go have a look.

-- 
Ville Syrjälä
Intel OTC