From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:24757 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966936AbcJZMeN (ORCPT ); Wed, 26 Oct 2016 08:34:13 -0400 Date: Wed, 26 Oct 2016 15:34:06 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Chris Wilson , dri-devel@lists.freedesktop.org, "Kirill A . Shutemov" , Carlos Santa , stable@vger.kernel.org Subject: Re: [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors Message-ID: <20161026123406.GB4617@intel.com> References: <1477472755-15288-1-git-send-email-ville.syrjala@linux.intel.com> <1477472755-15288-3-git-send-email-ville.syrjala@linux.intel.com> <20161026095224.GM10167@nuc-i3427.alporthouse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20161026095224.GM10167@nuc-i3427.alporthouse.com> Sender: stable-owner@vger.kernel.org List-ID: 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� > > > > 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 > > Cc: Kirill A. Shutemov > > Tested-by: Carlos Santa > > Tested-by: Kirill A. Shutemov > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666 > > Signed-off-by: Ville Syrj�l� > > --- > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors Date: Wed, 26 Oct 2016 15:34:06 +0300 Message-ID: <20161026123406.GB4617@intel.com> References: <1477472755-15288-1-git-send-email-ville.syrjala@linux.intel.com> <1477472755-15288-3-git-send-email-ville.syrjala@linux.intel.com> <20161026095224.GM10167@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Return-path: Content-Disposition: inline In-Reply-To: <20161026095224.GM10167@nuc-i3427.alporthouse.com> Sender: stable-owner@vger.kernel.org To: Chris Wilson , dri-devel@lists.freedesktop.org, "Kirill A . Shutemov" , Carlos Santa , stable@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org 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ä > > > > 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 > > Cc: Kirill A. Shutemov > > Tested-by: Carlos Santa > > Tested-by: Kirill A. Shutemov > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666 > > Signed-off-by: Ville Syrjälä > > --- > > 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