* [PATCH 0/4] drm: Fix MST oopses in fbdev restore @ 2016-10-26 9:05 ville.syrjala 2016-10-26 9:05 ` [PATCH 1/4] drm/fb-helper: Fix connector ref leak on error ville.syrjala ` (4 more replies) 0 siblings, 5 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 9:05 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, Carlos Santa From: Ville Syrjälä <ville.syrjala@linux.intel.com> This series would appear to be enough to avoid kernel oopses when trying to restore the fbdev setup after a MST device has been yanked. Apparently we still have some problems left in actually reacting properly to the changed situation, but at least the kernel no longer explodes. Entire series available here: git://github.com/vsyrjala/linux.git dp_mst_fixes_3 Cc: Carlos Santa <carlos.santa@intel.com> Cc: Kirill A. Shutemov <kirill@shutemov.name> Ville Syrjälä (4): drm/fb-helper: Fix connector ref leak on error drm/fb-helper: Keep references for the current set of used connectors drm/dp/mst: Clear port->pdt when tearing down the i2c adapter drm/dp/mst: Check peer device type before attempting EDID read drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++++- drivers/gpu/drm/drm_fb_helper.c | 32 ++++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 9 deletions(-) -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 1/4] drm/fb-helper: Fix connector ref leak on error 2016-10-26 9:05 [PATCH 0/4] drm: Fix MST oopses in fbdev restore ville.syrjala @ 2016-10-26 9:05 ` ville.syrjala 2016-10-26 9:48 ` Chris Wilson 2016-10-26 9:05 ` ville.syrjala ` (3 subsequent siblings) 4 siblings, 1 reply; 43+ messages in thread From: ville.syrjala @ 2016-10-26 9:05 UTC (permalink / raw) To: dri-devel; +Cc: stable, Carlos Santa, Kirill A . Shutemov From: Ville Syrjälä <ville.syrjala@linux.intel.com> We need to drop the connector references already taken when we abort in the middle of drm_fb_helper_single_add_all_connectors() 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> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/drm_fb_helper.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 83dbae0fabcf..db469d12d195 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -131,7 +131,12 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper) return 0; fail: for (i = 0; i < fb_helper->connector_count; i++) { - kfree(fb_helper->connector_info[i]); + struct drm_fb_helper_connector *fb_helper_connector = + fb_helper->connector_info[i]; + + drm_connector_unreference(fb_helper_connector->connector); + + kfree(fb_helper_connector); fb_helper->connector_info[i] = NULL; } fb_helper->connector_count = 0; -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 1/4] drm/fb-helper: Fix connector ref leak on error 2016-10-26 9:05 ` [PATCH 1/4] drm/fb-helper: Fix connector ref leak on error ville.syrjala @ 2016-10-26 9:48 ` Chris Wilson 0 siblings, 0 replies; 43+ messages in thread From: Chris Wilson @ 2016-10-26 9:48 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 12:05:52PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > We need to drop the connector references already taken when we > abort in the middle of drm_fb_helper_single_add_all_connectors() > > 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> > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/4] drm/fb-helper: Fix connector ref leak on error @ 2016-10-26 9:48 ` Chris Wilson 0 siblings, 0 replies; 43+ messages in thread From: Chris Wilson @ 2016-10-26 9:48 UTC (permalink / raw) To: ville.syrjala; +Cc: Kirill A . Shutemov, stable, Carlos Santa, dri-devel On Wed, Oct 26, 2016 at 12:05:52PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We need to drop the connector references already taken when we > abort in the middle of drm_fb_helper_single_add_all_connectors() > > 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> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors 2016-10-26 9:05 [PATCH 0/4] drm: Fix MST oopses in fbdev restore ville.syrjala @ 2016-10-26 9:05 ` ville.syrjala 2016-10-26 9:05 ` ville.syrjala ` (3 subsequent siblings) 4 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 9:05 UTC (permalink / raw) To: dri-devel; +Cc: stable, Carlos Santa, Kirill A . Shutemov 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; + } + modeset->num_connectors = 0; modeset->fb = NULL; } @@ -2148,19 +2154,23 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper) struct drm_display_mode *mode = modes[i]; struct drm_fb_helper_crtc *fb_crtc = crtcs[i]; struct drm_fb_offset *offset = &offsets[i]; - modeset = &fb_crtc->mode_set; + struct drm_mode_set *modeset = &fb_crtc->mode_set; if (mode && fb_crtc) { + struct drm_connector *connector = + fb_helper->connector_info[i]->connector; + DRM_DEBUG_KMS("desired mode %s set on crtc %d (%d,%d)\n", mode->name, fb_crtc->mode_set.crtc->base.id, offset->x, offset->y); + fb_crtc->desired_mode = mode; fb_crtc->x = offset->x; fb_crtc->y = offset->y; if (modeset->mode) drm_mode_destroy(dev, modeset->mode); - modeset->mode = drm_mode_duplicate(dev, - fb_crtc->desired_mode); - modeset->connectors[modeset->num_connectors++] = fb_helper->connector_info[i]->connector; + modeset->mode = drm_mode_duplicate(dev, fb_crtc->desired_mode); + drm_connector_reference(connector); + modeset->connectors[modeset->num_connectors++] = connector; modeset->fb = fb_helper->fb; modeset->x = offset->x; modeset->y = offset->y; @@ -2169,7 +2179,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper) /* Clear out any old modes if there are no more connected outputs. */ 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; + if (modeset->num_connectors == 0) { BUG_ON(modeset->fb); if (modeset->mode) -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors @ 2016-10-26 9:05 ` ville.syrjala 0 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 9:05 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, Carlos Santa, stable 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; + } + modeset->num_connectors = 0; modeset->fb = NULL; } @@ -2148,19 +2154,23 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper) struct drm_display_mode *mode = modes[i]; struct drm_fb_helper_crtc *fb_crtc = crtcs[i]; struct drm_fb_offset *offset = &offsets[i]; - modeset = &fb_crtc->mode_set; + struct drm_mode_set *modeset = &fb_crtc->mode_set; if (mode && fb_crtc) { + struct drm_connector *connector = + fb_helper->connector_info[i]->connector; + DRM_DEBUG_KMS("desired mode %s set on crtc %d (%d,%d)\n", mode->name, fb_crtc->mode_set.crtc->base.id, offset->x, offset->y); + fb_crtc->desired_mode = mode; fb_crtc->x = offset->x; fb_crtc->y = offset->y; if (modeset->mode) drm_mode_destroy(dev, modeset->mode); - modeset->mode = drm_mode_duplicate(dev, - fb_crtc->desired_mode); - modeset->connectors[modeset->num_connectors++] = fb_helper->connector_info[i]->connector; + modeset->mode = drm_mode_duplicate(dev, fb_crtc->desired_mode); + drm_connector_reference(connector); + modeset->connectors[modeset->num_connectors++] = connector; modeset->fb = fb_helper->fb; modeset->x = offset->x; modeset->y = offset->y; @@ -2169,7 +2179,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper) /* Clear out any old modes if there are no more connected outputs. */ 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; + if (modeset->num_connectors == 0) { BUG_ON(modeset->fb); if (modeset->mode) -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors 2016-10-26 9:05 ` ville.syrjala @ 2016-10-26 9:52 ` Chris Wilson -1 siblings, 0 replies; 43+ messages in thread From: Chris Wilson @ 2016-10-26 9:52 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable 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()? -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors @ 2016-10-26 9:52 ` Chris Wilson 0 siblings, 0 replies; 43+ messages in thread From: Chris Wilson @ 2016-10-26 9:52 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable 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()? -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors 2016-10-26 9:52 ` Chris Wilson @ 2016-10-26 12:34 ` Ville Syrjälä -1 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 12:34 UTC (permalink / raw) To: Chris Wilson, dri-devel, Kirill A . Shutemov, Carlos Santa, stable 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors @ 2016-10-26 12:34 ` Ville Syrjälä 0 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 12:34 UTC (permalink / raw) To: Chris Wilson, dri-devel, Kirill A . Shutemov, Carlos Santa, stable 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 2/4] drm/fb-helper: Keep references for the current set of used connectors 2016-10-26 9:05 ` ville.syrjala (?) (?) @ 2016-10-26 13:31 ` ville.syrjala 2016-10-26 13:54 ` Chris Wilson -1 siblings, 1 reply; 43+ messages in thread From: ville.syrjala @ 2016-10-26 13:31 UTC (permalink / raw) To: dri-devel; +Cc: Chris Wilson, stable, Carlos Santa, Kirill A . Shutemov 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. Since we need to clean up in drm_fb_helper_crtc_free() as well, let's pull the code to a common place. And while at it let's pull in up the modeset->mode cleanup in there as well. The case of modeset->fb is a bit less clear. I'm thinking we should probably hold a reference to it, but for now I just slapped on a FIXME. v2: Cleanup things drm_fb_helper_crtc_free() too (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> 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> (v1) Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) 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 | 58 +++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index db469d12d195..83961f1a97d2 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -605,6 +605,24 @@ int drm_fb_helper_blank(int blank, struct fb_info *info) } EXPORT_SYMBOL(drm_fb_helper_blank); +static void drm_fb_helper_modeset_free(struct drm_fb_helper *helper, + struct drm_mode_set *modeset) +{ + int i; + + for (i = 0; i < modeset->num_connectors; i++) { + drm_connector_unreference(modeset->connectors[i]); + modeset->connectors[i] = NULL; + } + modeset->num_connectors = 0; + + drm_mode_destroy(helper->dev, modeset->mode); + modeset->mode = NULL; + + /* FIXME should hold a ref? */ + modeset->fb = NULL; +} + static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) { int i; @@ -614,11 +632,10 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) kfree(helper->connector_info[i]); } kfree(helper->connector_info); - for (i = 0; i < helper->crtc_count; i++) { - kfree(helper->crtc_info[i].mode_set.connectors); - if (helper->crtc_info[i].mode_set.mode) - drm_mode_destroy(helper->dev, helper->crtc_info[i].mode_set.mode); - } + + for (i = 0; i < helper->crtc_count; i++) + drm_fb_helper_modeset_free(helper, + &helper->crtc_info[i].mode_set); kfree(helper->crtc_info); } @@ -2090,7 +2107,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; @@ -2138,45 +2154,35 @@ 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; - modeset->num_connectors = 0; - modeset->fb = NULL; - } + for (i = 0; i < fb_helper->crtc_count; i++) + drm_fb_helper_modeset_free(fb_helper, + &fb_helper->crtc_info[i].mode_set); for (i = 0; i < fb_helper->connector_count; i++) { struct drm_display_mode *mode = modes[i]; struct drm_fb_helper_crtc *fb_crtc = crtcs[i]; struct drm_fb_offset *offset = &offsets[i]; - modeset = &fb_crtc->mode_set; + struct drm_mode_set *modeset = &fb_crtc->mode_set; if (mode && fb_crtc) { + struct drm_connector *connector = + fb_helper->connector_info[i]->connector; + DRM_DEBUG_KMS("desired mode %s set on crtc %d (%d,%d)\n", mode->name, fb_crtc->mode_set.crtc->base.id, offset->x, offset->y); + fb_crtc->desired_mode = mode; fb_crtc->x = offset->x; fb_crtc->y = offset->y; - if (modeset->mode) - drm_mode_destroy(dev, modeset->mode); modeset->mode = drm_mode_duplicate(dev, fb_crtc->desired_mode); - modeset->connectors[modeset->num_connectors++] = fb_helper->connector_info[i]->connector; + drm_connector_reference(connector); + modeset->connectors[modeset->num_connectors++] = connector; modeset->fb = fb_helper->fb; modeset->x = offset->x; modeset->y = offset->y; } } - - /* Clear out any old modes if there are no more connected outputs. */ - for (i = 0; i < fb_helper->crtc_count; i++) { - modeset = &fb_helper->crtc_info[i].mode_set; - if (modeset->num_connectors == 0) { - BUG_ON(modeset->fb); - if (modeset->mode) - drm_mode_destroy(dev, modeset->mode); - modeset->mode = NULL; - } - } out: kfree(crtcs); kfree(modes); -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2 2/4] drm/fb-helper: Keep references for the current set of used connectors 2016-10-26 13:31 ` [PATCH v2 " ville.syrjala @ 2016-10-26 13:54 ` Chris Wilson 0 siblings, 0 replies; 43+ messages in thread From: Chris Wilson @ 2016-10-26 13:54 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, stable, Carlos Santa, Kirill A . Shutemov On Wed, Oct 26, 2016 at 04:31:20PM +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. > > Since we need to clean up in drm_fb_helper_crtc_free() as well, > let's pull the code to a common place. And while at it let's > pull in up the modeset->mode cleanup in there as well. The case > of modeset->fb is a bit less clear. I'm thinking we should probably > hold a reference to it, but for now I just slapped on a FIXME. > > v2: Cleanup things drm_fb_helper_crtc_free() too (Chris) > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > 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> (v1) > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > 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 | 58 +++++++++++++++++++++++------------------ > 1 file changed, 32 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index db469d12d195..83961f1a97d2 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -605,6 +605,24 @@ int drm_fb_helper_blank(int blank, struct fb_info *info) > } > EXPORT_SYMBOL(drm_fb_helper_blank); > > +static void drm_fb_helper_modeset_free(struct drm_fb_helper *helper, > + struct drm_mode_set *modeset) > +{ > + int i; > + > + for (i = 0; i < modeset->num_connectors; i++) { > + drm_connector_unreference(modeset->connectors[i]); > + modeset->connectors[i] = NULL; > + } > + modeset->num_connectors = 0; > + > + drm_mode_destroy(helper->dev, modeset->mode); > + modeset->mode = NULL; > + > + /* FIXME should hold a ref? */ > + modeset->fb = NULL; > +} > + > static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > { > int i; > @@ -614,11 +632,10 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > kfree(helper->connector_info[i]); > } > kfree(helper->connector_info); > - for (i = 0; i < helper->crtc_count; i++) { > - kfree(helper->crtc_info[i].mode_set.connectors); > - if (helper->crtc_info[i].mode_set.mode) > - drm_mode_destroy(helper->dev, helper->crtc_info[i].mode_set.mode); > - } > + > + for (i = 0; i < helper->crtc_count; i++) > + drm_fb_helper_modeset_free(helper, > + &helper->crtc_info[i].mode_set); We lose the kfree(mode_set.connectors) here. So for (i = 0; i < helper->crtc_count; i++) struct drm_mode_set *modeset = &helper->crtc_info[i].mode_set); drm_fb_helper_modeset_release(helper, modeset); kfree(modeset->connectors); } ? Couldn't spot any other missing calls to release the new ref, so with the tiny leak fixed, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 2/4] drm/fb-helper: Keep references for the current set of used connectors @ 2016-10-26 13:54 ` Chris Wilson 0 siblings, 0 replies; 43+ messages in thread From: Chris Wilson @ 2016-10-26 13:54 UTC (permalink / raw) To: ville.syrjala; +Cc: Kirill A . Shutemov, stable, dri-devel, Carlos Santa On Wed, Oct 26, 2016 at 04:31:20PM +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. > > Since we need to clean up in drm_fb_helper_crtc_free() as well, > let's pull the code to a common place. And while at it let's > pull in up the modeset->mode cleanup in there as well. The case > of modeset->fb is a bit less clear. I'm thinking we should probably > hold a reference to it, but for now I just slapped on a FIXME. > > v2: Cleanup things drm_fb_helper_crtc_free() too (Chris) > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > 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> (v1) > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > 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 | 58 +++++++++++++++++++++++------------------ > 1 file changed, 32 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index db469d12d195..83961f1a97d2 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -605,6 +605,24 @@ int drm_fb_helper_blank(int blank, struct fb_info *info) > } > EXPORT_SYMBOL(drm_fb_helper_blank); > > +static void drm_fb_helper_modeset_free(struct drm_fb_helper *helper, > + struct drm_mode_set *modeset) > +{ > + int i; > + > + for (i = 0; i < modeset->num_connectors; i++) { > + drm_connector_unreference(modeset->connectors[i]); > + modeset->connectors[i] = NULL; > + } > + modeset->num_connectors = 0; > + > + drm_mode_destroy(helper->dev, modeset->mode); > + modeset->mode = NULL; > + > + /* FIXME should hold a ref? */ > + modeset->fb = NULL; > +} > + > static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > { > int i; > @@ -614,11 +632,10 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > kfree(helper->connector_info[i]); > } > kfree(helper->connector_info); > - for (i = 0; i < helper->crtc_count; i++) { > - kfree(helper->crtc_info[i].mode_set.connectors); > - if (helper->crtc_info[i].mode_set.mode) > - drm_mode_destroy(helper->dev, helper->crtc_info[i].mode_set.mode); > - } > + > + for (i = 0; i < helper->crtc_count; i++) > + drm_fb_helper_modeset_free(helper, > + &helper->crtc_info[i].mode_set); We lose the kfree(mode_set.connectors) here. So for (i = 0; i < helper->crtc_count; i++) struct drm_mode_set *modeset = &helper->crtc_info[i].mode_set); drm_fb_helper_modeset_release(helper, modeset); kfree(modeset->connectors); } ? Couldn't spot any other missing calls to release the new ref, so with the tiny leak fixed, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 2/4] drm/fb-helper: Keep references for the current set of used connectors 2016-10-26 13:54 ` Chris Wilson @ 2016-10-26 14:11 ` Ville Syrjälä -1 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 14:11 UTC (permalink / raw) To: Chris Wilson, dri-devel, stable, Carlos Santa, Kirill A . Shutemov On Wed, Oct 26, 2016 at 02:54:45PM +0100, Chris Wilson wrote: > On Wed, Oct 26, 2016 at 04:31:20PM +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. > > > > Since we need to clean up in drm_fb_helper_crtc_free() as well, > > let's pull the code to a common place. And while at it let's > > pull in up the modeset->mode cleanup in there as well. The case > > of modeset->fb is a bit less clear. I'm thinking we should probably > > hold a reference to it, but for now I just slapped on a FIXME. > > > > v2: Cleanup things drm_fb_helper_crtc_free() too (Chris) > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > 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> (v1) > > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > > 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 | 58 +++++++++++++++++++++++------------------ > > 1 file changed, 32 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > index db469d12d195..83961f1a97d2 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -605,6 +605,24 @@ int drm_fb_helper_blank(int blank, struct fb_info *info) > > } > > EXPORT_SYMBOL(drm_fb_helper_blank); > > > > +static void drm_fb_helper_modeset_free(struct drm_fb_helper *helper, > > + struct drm_mode_set *modeset) > > +{ > > + int i; > > + > > + for (i = 0; i < modeset->num_connectors; i++) { > > + drm_connector_unreference(modeset->connectors[i]); > > + modeset->connectors[i] = NULL; > > + } > > + modeset->num_connectors = 0; > > + > > + drm_mode_destroy(helper->dev, modeset->mode); > > + modeset->mode = NULL; > > + > > + /* FIXME should hold a ref? */ > > + modeset->fb = NULL; > > +} > > + > > static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > > { > > int i; > > @@ -614,11 +632,10 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > > kfree(helper->connector_info[i]); > > } > > kfree(helper->connector_info); > > - for (i = 0; i < helper->crtc_count; i++) { > > - kfree(helper->crtc_info[i].mode_set.connectors); > > - if (helper->crtc_info[i].mode_set.mode) > > - drm_mode_destroy(helper->dev, helper->crtc_info[i].mode_set.mode); > > - } > > + > > + for (i = 0; i < helper->crtc_count; i++) > > + drm_fb_helper_modeset_free(helper, > > + &helper->crtc_info[i].mode_set); > > We lose the kfree(mode_set.connectors) here. Doh. > > So > for (i = 0; i < helper->crtc_count; i++) > struct drm_mode_set *modeset = &helper->crtc_info[i].mode_set); > > drm_fb_helper_modeset_release(helper, modeset); Hmm. Yeah _release() does seem better than _free() if we don't actually free all of it. > kfree(modeset->connectors); > } > ? > > Couldn't spot any other missing calls to release the new ref, so with > the tiny leak fixed, > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Thanks. I'll respin it once more. -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 2/4] drm/fb-helper: Keep references for the current set of used connectors @ 2016-10-26 14:11 ` Ville Syrjälä 0 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 14:11 UTC (permalink / raw) To: Chris Wilson, dri-devel, stable, Carlos Santa, Kirill A . Shutemov On Wed, Oct 26, 2016 at 02:54:45PM +0100, Chris Wilson wrote: > On Wed, Oct 26, 2016 at 04:31:20PM +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. > > > > Since we need to clean up in drm_fb_helper_crtc_free() as well, > > let's pull the code to a common place. And while at it let's > > pull in up the modeset->mode cleanup in there as well. The case > > of modeset->fb is a bit less clear. I'm thinking we should probably > > hold a reference to it, but for now I just slapped on a FIXME. > > > > v2: Cleanup things drm_fb_helper_crtc_free() too (Chris) > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > 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> (v1) > > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > > 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 | 58 +++++++++++++++++++++++------------------ > > 1 file changed, 32 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > index db469d12d195..83961f1a97d2 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -605,6 +605,24 @@ int drm_fb_helper_blank(int blank, struct fb_info *info) > > } > > EXPORT_SYMBOL(drm_fb_helper_blank); > > > > +static void drm_fb_helper_modeset_free(struct drm_fb_helper *helper, > > + struct drm_mode_set *modeset) > > +{ > > + int i; > > + > > + for (i = 0; i < modeset->num_connectors; i++) { > > + drm_connector_unreference(modeset->connectors[i]); > > + modeset->connectors[i] = NULL; > > + } > > + modeset->num_connectors = 0; > > + > > + drm_mode_destroy(helper->dev, modeset->mode); > > + modeset->mode = NULL; > > + > > + /* FIXME should hold a ref? */ > > + modeset->fb = NULL; > > +} > > + > > static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > > { > > int i; > > @@ -614,11 +632,10 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > > kfree(helper->connector_info[i]); > > } > > kfree(helper->connector_info); > > - for (i = 0; i < helper->crtc_count; i++) { > > - kfree(helper->crtc_info[i].mode_set.connectors); > > - if (helper->crtc_info[i].mode_set.mode) > > - drm_mode_destroy(helper->dev, helper->crtc_info[i].mode_set.mode); > > - } > > + > > + for (i = 0; i < helper->crtc_count; i++) > > + drm_fb_helper_modeset_free(helper, > > + &helper->crtc_info[i].mode_set); > > We lose the kfree(mode_set.connectors) here. Doh. > > So > for (i = 0; i < helper->crtc_count; i++) > struct drm_mode_set *modeset = &helper->crtc_info[i].mode_set); > > drm_fb_helper_modeset_release(helper, modeset); Hmm. Yeah _release() does seem better than _free() if we don't actually free all of it. > kfree(modeset->connectors); > } > ? > > Couldn't spot any other missing calls to release the new ref, so with > the tiny leak fixed, > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Thanks. I'll respin it once more. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3 2/4] drm/fb-helper: Keep references for the current set of used connectors 2016-10-26 9:05 ` ville.syrjala @ 2016-10-26 14:41 ` ville.syrjala -1 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 14:41 UTC (permalink / raw) To: dri-devel; +Cc: Chris Wilson, stable, Carlos Santa, Kirill A . Shutemov 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. Since we need to clean up in drm_fb_helper_crtc_free() as well, let's pull the code to a common place. And while at it let's pull in up the modeset->mode cleanup in there as well. The case of modeset->fb is a bit less clear. I'm thinking we should probably hold a reference to it, but for now I just slapped on a FIXME. v2: Cleanup things drm_fb_helper_crtc_free() too (Chris) v3: Don't leak modeset->connectors (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> 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> (v1) Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/drm_fb_helper.c | 57 ++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index db469d12d195..813280e64311 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -605,6 +605,24 @@ int drm_fb_helper_blank(int blank, struct fb_info *info) } EXPORT_SYMBOL(drm_fb_helper_blank); +static void drm_fb_helper_modeset_release(struct drm_fb_helper *helper, + struct drm_mode_set *modeset) +{ + int i; + + for (i = 0; i < modeset->num_connectors; i++) { + drm_connector_unreference(modeset->connectors[i]); + modeset->connectors[i] = NULL; + } + modeset->num_connectors = 0; + + drm_mode_destroy(helper->dev, modeset->mode); + modeset->mode = NULL; + + /* FIXME should hold a ref? */ + modeset->fb = NULL; +} + static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) { int i; @@ -614,10 +632,12 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) kfree(helper->connector_info[i]); } kfree(helper->connector_info); + for (i = 0; i < helper->crtc_count; i++) { - kfree(helper->crtc_info[i].mode_set.connectors); - if (helper->crtc_info[i].mode_set.mode) - drm_mode_destroy(helper->dev, helper->crtc_info[i].mode_set.mode); + struct drm_mode_set *modeset = &helper->crtc_info[i].mode_set; + + drm_fb_helper_modeset_release(helper, modeset); + kfree(modeset->connectors); } kfree(helper->crtc_info); } @@ -2090,7 +2110,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; @@ -2138,45 +2157,35 @@ 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; - modeset->num_connectors = 0; - modeset->fb = NULL; - } + for (i = 0; i < fb_helper->crtc_count; i++) + drm_fb_helper_modeset_release(fb_helper, + &fb_helper->crtc_info[i].mode_set); for (i = 0; i < fb_helper->connector_count; i++) { struct drm_display_mode *mode = modes[i]; struct drm_fb_helper_crtc *fb_crtc = crtcs[i]; struct drm_fb_offset *offset = &offsets[i]; - modeset = &fb_crtc->mode_set; + struct drm_mode_set *modeset = &fb_crtc->mode_set; if (mode && fb_crtc) { + struct drm_connector *connector = + fb_helper->connector_info[i]->connector; + DRM_DEBUG_KMS("desired mode %s set on crtc %d (%d,%d)\n", mode->name, fb_crtc->mode_set.crtc->base.id, offset->x, offset->y); + fb_crtc->desired_mode = mode; fb_crtc->x = offset->x; fb_crtc->y = offset->y; - if (modeset->mode) - drm_mode_destroy(dev, modeset->mode); modeset->mode = drm_mode_duplicate(dev, fb_crtc->desired_mode); - modeset->connectors[modeset->num_connectors++] = fb_helper->connector_info[i]->connector; + drm_connector_reference(connector); + modeset->connectors[modeset->num_connectors++] = connector; modeset->fb = fb_helper->fb; modeset->x = offset->x; modeset->y = offset->y; } } - - /* Clear out any old modes if there are no more connected outputs. */ - for (i = 0; i < fb_helper->crtc_count; i++) { - modeset = &fb_helper->crtc_info[i].mode_set; - if (modeset->num_connectors == 0) { - BUG_ON(modeset->fb); - if (modeset->mode) - drm_mode_destroy(dev, modeset->mode); - modeset->mode = NULL; - } - } out: kfree(crtcs); kfree(modes); -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v3 2/4] drm/fb-helper: Keep references for the current set of used connectors @ 2016-10-26 14:41 ` ville.syrjala 0 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 14:41 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, Carlos Santa, stable 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. Since we need to clean up in drm_fb_helper_crtc_free() as well, let's pull the code to a common place. And while at it let's pull in up the modeset->mode cleanup in there as well. The case of modeset->fb is a bit less clear. I'm thinking we should probably hold a reference to it, but for now I just slapped on a FIXME. v2: Cleanup things drm_fb_helper_crtc_free() too (Chris) v3: Don't leak modeset->connectors (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> 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> (v1) Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/drm_fb_helper.c | 57 ++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index db469d12d195..813280e64311 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -605,6 +605,24 @@ int drm_fb_helper_blank(int blank, struct fb_info *info) } EXPORT_SYMBOL(drm_fb_helper_blank); +static void drm_fb_helper_modeset_release(struct drm_fb_helper *helper, + struct drm_mode_set *modeset) +{ + int i; + + for (i = 0; i < modeset->num_connectors; i++) { + drm_connector_unreference(modeset->connectors[i]); + modeset->connectors[i] = NULL; + } + modeset->num_connectors = 0; + + drm_mode_destroy(helper->dev, modeset->mode); + modeset->mode = NULL; + + /* FIXME should hold a ref? */ + modeset->fb = NULL; +} + static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) { int i; @@ -614,10 +632,12 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) kfree(helper->connector_info[i]); } kfree(helper->connector_info); + for (i = 0; i < helper->crtc_count; i++) { - kfree(helper->crtc_info[i].mode_set.connectors); - if (helper->crtc_info[i].mode_set.mode) - drm_mode_destroy(helper->dev, helper->crtc_info[i].mode_set.mode); + struct drm_mode_set *modeset = &helper->crtc_info[i].mode_set; + + drm_fb_helper_modeset_release(helper, modeset); + kfree(modeset->connectors); } kfree(helper->crtc_info); } @@ -2090,7 +2110,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; @@ -2138,45 +2157,35 @@ 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; - modeset->num_connectors = 0; - modeset->fb = NULL; - } + for (i = 0; i < fb_helper->crtc_count; i++) + drm_fb_helper_modeset_release(fb_helper, + &fb_helper->crtc_info[i].mode_set); for (i = 0; i < fb_helper->connector_count; i++) { struct drm_display_mode *mode = modes[i]; struct drm_fb_helper_crtc *fb_crtc = crtcs[i]; struct drm_fb_offset *offset = &offsets[i]; - modeset = &fb_crtc->mode_set; + struct drm_mode_set *modeset = &fb_crtc->mode_set; if (mode && fb_crtc) { + struct drm_connector *connector = + fb_helper->connector_info[i]->connector; + DRM_DEBUG_KMS("desired mode %s set on crtc %d (%d,%d)\n", mode->name, fb_crtc->mode_set.crtc->base.id, offset->x, offset->y); + fb_crtc->desired_mode = mode; fb_crtc->x = offset->x; fb_crtc->y = offset->y; - if (modeset->mode) - drm_mode_destroy(dev, modeset->mode); modeset->mode = drm_mode_duplicate(dev, fb_crtc->desired_mode); - modeset->connectors[modeset->num_connectors++] = fb_helper->connector_info[i]->connector; + drm_connector_reference(connector); + modeset->connectors[modeset->num_connectors++] = connector; modeset->fb = fb_helper->fb; modeset->x = offset->x; modeset->y = offset->y; } } - - /* Clear out any old modes if there are no more connected outputs. */ - for (i = 0; i < fb_helper->crtc_count; i++) { - modeset = &fb_helper->crtc_info[i].mode_set; - if (modeset->num_connectors == 0) { - BUG_ON(modeset->fb); - if (modeset->mode) - drm_mode_destroy(dev, modeset->mode); - modeset->mode = NULL; - } - } out: kfree(crtcs); kfree(modes); -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 9:05 [PATCH 0/4] drm: Fix MST oopses in fbdev restore ville.syrjala @ 2016-10-26 9:05 ` ville.syrjala 2016-10-26 9:05 ` ville.syrjala ` (3 subsequent siblings) 4 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 9:05 UTC (permalink / raw) To: dri-devel; +Cc: stable, Carlos Santa, Kirill A . Shutemov From: Ville Syrjälä <ville.syrjala@linux.intel.com> The i2c adapter is only relevant for some peer device types, so let's clear the pdt if it's still the same as the old_pdt when we tear down the i2c adapter. I don't really like this design pattern of updating port->whatever before doing the accompanying changes and passing around old_whatever to figure stuff out. Would make much more sense to me to the pass the new value around and only update the port->whatever when things are consistent. But let's try to work with what we have right now. 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_dp_mst_topology.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..956babc161e5 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -882,6 +882,9 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) drm_dp_put_mst_branch_device(mstb); break; } + + if (port->pdt == old_pdt) + port->pdt = DP_PEER_DEVICE_NONE; } static void drm_dp_destroy_port(struct kref *kref) -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 9:05 ` ville.syrjala 0 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 9:05 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, Carlos Santa, stable From: Ville Syrjälä <ville.syrjala@linux.intel.com> The i2c adapter is only relevant for some peer device types, so let's clear the pdt if it's still the same as the old_pdt when we tear down the i2c adapter. I don't really like this design pattern of updating port->whatever before doing the accompanying changes and passing around old_whatever to figure stuff out. Would make much more sense to me to the pass the new value around and only update the port->whatever when things are consistent. But let's try to work with what we have right now. 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_dp_mst_topology.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..956babc161e5 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -882,6 +882,9 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) drm_dp_put_mst_branch_device(mstb); break; } + + if (port->pdt == old_pdt) + port->pdt = DP_PEER_DEVICE_NONE; } static void drm_dp_destroy_port(struct kref *kref) -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 9:05 ` ville.syrjala @ 2016-10-26 12:53 ` Daniel Vetter -1 siblings, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 12:53 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 12:05:54PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > The i2c adapter is only relevant for some peer device types, so > let's clear the pdt if it's still the same as the old_pdt when we > tear down the i2c adapter. > > I don't really like this design pattern of updating port->whatever > before doing the accompanying changes and passing around old_whatever > to figure stuff out. Would make much more sense to me to the pass the > new value around and only update the port->whatever when things are > consistent. But let's try to work with what we have right now. > > 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_dp_mst_topology.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 04e457117980..956babc161e5 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -882,6 +882,9 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) > drm_dp_put_mst_branch_device(mstb); > break; > } > + > + if (port->pdt == old_pdt) > + port->pdt = DP_PEER_DEVICE_NONE; So from my understanding this is needed for the callsite in drm_dp_destroy_connector_work(). All others are either the final destroy path, or set up the ->pdt to something before calling this function. Only this call site passes port->pdt. I think we should instead change this callsite to set the port->pdt to NONE after the call, i.e. diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..36f47092c703 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2919,6 +2919,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) mgr->cbs->destroy_connector(mgr, port->connector); drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; if (!port->input && port->vcpi.vcpi > 0) { drm_dp_mst_reset_vcpi_slots(mgr, port); I think that would be more consistent than spreading the control flow even more like in your patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 12:53 ` Daniel Vetter 0 siblings, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 12:53 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 12:05:54PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The i2c adapter is only relevant for some peer device types, so > let's clear the pdt if it's still the same as the old_pdt when we > tear down the i2c adapter. > > I don't really like this design pattern of updating port->whatever > before doing the accompanying changes and passing around old_whatever > to figure stuff out. Would make much more sense to me to the pass the > new value around and only update the port->whatever when things are > consistent. But let's try to work with what we have right now. > > 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_dp_mst_topology.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 04e457117980..956babc161e5 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -882,6 +882,9 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) > drm_dp_put_mst_branch_device(mstb); > break; > } > + > + if (port->pdt == old_pdt) > + port->pdt = DP_PEER_DEVICE_NONE; So from my understanding this is needed for the callsite in drm_dp_destroy_connector_work(). All others are either the final destroy path, or set up the ->pdt to something before calling this function. Only this call site passes port->pdt. I think we should instead change this callsite to set the port->pdt to NONE after the call, i.e. diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..36f47092c703 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2919,6 +2919,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) mgr->cbs->destroy_connector(mgr, port->connector); drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; if (!port->input && port->vcpi.vcpi > 0) { drm_dp_mst_reset_vcpi_slots(mgr, port); I think that would be more consistent than spreading the control flow even more like in your patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 12:53 ` Daniel Vetter @ 2016-10-26 13:02 ` Ville Syrjälä -1 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 13:02 UTC (permalink / raw) To: Daniel Vetter; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 02:53:01PM +0200, Daniel Vetter wrote: > On Wed, Oct 26, 2016 at 12:05:54PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > The i2c adapter is only relevant for some peer device types, so > > let's clear the pdt if it's still the same as the old_pdt when we > > tear down the i2c adapter. > > > > I don't really like this design pattern of updating port->whatever > > before doing the accompanying changes and passing around old_whatever > > to figure stuff out. Would make much more sense to me to the pass the > > new value around and only update the port->whatever when things are > > consistent. But let's try to work with what we have right now. > > > > 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_dp_mst_topology.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 04e457117980..956babc161e5 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -882,6 +882,9 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) > > drm_dp_put_mst_branch_device(mstb); > > break; > > } > > + > > + if (port->pdt == old_pdt) > > + port->pdt = DP_PEER_DEVICE_NONE; > > So from my understanding this is needed for the callsite in > drm_dp_destroy_connector_work(). All others are either the final destroy > path, or set up the ->pdt to something before calling this function. Only > this call site passes port->pdt. I think we should instead change this > callsite to set the port->pdt to NONE after the call, i.e. > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 04e457117980..36f47092c703 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -2919,6 +2919,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > mgr->cbs->destroy_connector(mgr, port->connector); > > drm_dp_port_teardown_pdt(port, port->pdt); > + port->pdt = DP_PEER_DEVICE_NONE; > > if (!port->input && port->vcpi.vcpi > 0) { > drm_dp_mst_reset_vcpi_slots(mgr, port); > > > I think that would be more consistent than spreading the control flow even > more like in your patch. Yeah, makes sense. -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 13:02 ` Ville Syrjälä 0 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 13:02 UTC (permalink / raw) To: Daniel Vetter; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 02:53:01PM +0200, Daniel Vetter wrote: > On Wed, Oct 26, 2016 at 12:05:54PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The i2c adapter is only relevant for some peer device types, so > > let's clear the pdt if it's still the same as the old_pdt when we > > tear down the i2c adapter. > > > > I don't really like this design pattern of updating port->whatever > > before doing the accompanying changes and passing around old_whatever > > to figure stuff out. Would make much more sense to me to the pass the > > new value around and only update the port->whatever when things are > > consistent. But let's try to work with what we have right now. > > > > 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_dp_mst_topology.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 04e457117980..956babc161e5 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -882,6 +882,9 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) > > drm_dp_put_mst_branch_device(mstb); > > break; > > } > > + > > + if (port->pdt == old_pdt) > > + port->pdt = DP_PEER_DEVICE_NONE; > > So from my understanding this is needed for the callsite in > drm_dp_destroy_connector_work(). All others are either the final destroy > path, or set up the ->pdt to something before calling this function. Only > this call site passes port->pdt. I think we should instead change this > callsite to set the port->pdt to NONE after the call, i.e. > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 04e457117980..36f47092c703 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -2919,6 +2919,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > mgr->cbs->destroy_connector(mgr, port->connector); > > drm_dp_port_teardown_pdt(port, port->pdt); > + port->pdt = DP_PEER_DEVICE_NONE; > > if (!port->input && port->vcpi.vcpi > 0) { > drm_dp_mst_reset_vcpi_slots(mgr, port); > > > I think that would be more consistent than spreading the control flow even > more like in your patch. Yeah, makes sense. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 9:05 ` ville.syrjala @ 2016-10-26 13:30 ` ville.syrjala -1 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 13:30 UTC (permalink / raw) To: dri-devel; +Cc: Daniel Vetter, stable, Carlos Santa, Kirill A . Shutemov From: Ville Syrjälä <ville.syrjala@linux.intel.com> The i2c adapter is only relevant for some peer device types, so let's clear the pdt if it's still the same as the old_pdt when we tear down the i2c adapter. I don't really like this design pattern of updating port->whatever before doing the accompanying changes and passing around old_whatever to figure stuff out. Would make much more sense to me to the pass the new value around and only update the port->whatever when things are consistent. But let's try to work with what we have right now. v2: Clear port->pdt in the caller, if needed (Daniel) Cc: Daniel Vetter <daniel@ffwll.ch> 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> (v1) Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) 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_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..ba13f9d8720b 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) /* no need to clean up vcpi * as if we have no connector we never setup a vcpi */ drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; } kfree(port); } @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) mgr->cbs->destroy_connector(mgr, port->connector); drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; if (!port->input && port->vcpi.vcpi > 0) { drm_dp_mst_reset_vcpi_slots(mgr, port); -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 13:30 ` ville.syrjala 0 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 13:30 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, stable, Carlos Santa From: Ville Syrjälä <ville.syrjala@linux.intel.com> The i2c adapter is only relevant for some peer device types, so let's clear the pdt if it's still the same as the old_pdt when we tear down the i2c adapter. I don't really like this design pattern of updating port->whatever before doing the accompanying changes and passing around old_whatever to figure stuff out. Would make much more sense to me to the pass the new value around and only update the port->whatever when things are consistent. But let's try to work with what we have right now. v2: Clear port->pdt in the caller, if needed (Daniel) Cc: Daniel Vetter <daniel@ffwll.ch> 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> (v1) Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) 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_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..ba13f9d8720b 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) /* no need to clean up vcpi * as if we have no connector we never setup a vcpi */ drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; } kfree(port); } @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) mgr->cbs->destroy_connector(mgr, port->connector); drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; if (!port->input && port->vcpi.vcpi > 0) { drm_dp_mst_reset_vcpi_slots(mgr, port); -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 13:30 ` ville.syrjala @ 2016-10-26 13:35 ` Ville Syrjälä -1 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 13:35 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, stable, Carlos Santa On Wed, Oct 26, 2016 at 04:30:33PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > The i2c adapter is only relevant for some peer device types, so > let's clear the pdt if it's still the same as the old_pdt when we > tear down the i2c adapter. > > I don't really like this design pattern of updating port->whatever > before doing the accompanying changes and passing around old_whatever > to figure stuff out. Would make much more sense to me to the pass the > new value around and only update the port->whatever when things are > consistent. But let's try to work with what we have right now. > > v2: Clear port->pdt in the caller, if needed (Daniel) > > Cc: Daniel Vetter <daniel@ffwll.ch> > 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> (v1) > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > 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_dp_mst_topology.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 04e457117980..ba13f9d8720b 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) > /* no need to clean up vcpi > * as if we have no connector we never setup a vcpi */ > drm_dp_port_teardown_pdt(port, port->pdt); > + port->pdt = DP_PEER_DEVICE_NONE; And naturally I forgot to amend the commit message w.r.t. this guy. We don't really need to do this here, but I figured I'd try to be a bit more consistent by having it, just to avoid accidental mistakes if/when someone changes this stuff again later. > } > kfree(port); > } > @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > mgr->cbs->destroy_connector(mgr, port->connector); > > drm_dp_port_teardown_pdt(port, port->pdt); > + port->pdt = DP_PEER_DEVICE_NONE; > > if (!port->input && port->vcpi.vcpi > 0) { > drm_dp_mst_reset_vcpi_slots(mgr, port); > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 13:35 ` Ville Syrjälä 0 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 13:35 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 04:30:33PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The i2c adapter is only relevant for some peer device types, so > let's clear the pdt if it's still the same as the old_pdt when we > tear down the i2c adapter. > > I don't really like this design pattern of updating port->whatever > before doing the accompanying changes and passing around old_whatever > to figure stuff out. Would make much more sense to me to the pass the > new value around and only update the port->whatever when things are > consistent. But let's try to work with what we have right now. > > v2: Clear port->pdt in the caller, if needed (Daniel) > > Cc: Daniel Vetter <daniel@ffwll.ch> > 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> (v1) > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > 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_dp_mst_topology.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 04e457117980..ba13f9d8720b 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) > /* no need to clean up vcpi > * as if we have no connector we never setup a vcpi */ > drm_dp_port_teardown_pdt(port, port->pdt); > + port->pdt = DP_PEER_DEVICE_NONE; And naturally I forgot to amend the commit message w.r.t. this guy. We don't really need to do this here, but I figured I'd try to be a bit more consistent by having it, just to avoid accidental mistakes if/when someone changes this stuff again later. > } > kfree(port); > } > @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > mgr->cbs->destroy_connector(mgr, port->connector); > > drm_dp_port_teardown_pdt(port, port->pdt); > + port->pdt = DP_PEER_DEVICE_NONE; > > if (!port->input && port->vcpi.vcpi > 0) { > drm_dp_mst_reset_vcpi_slots(mgr, port); > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 13:35 ` Ville Syrjälä @ 2016-10-26 14:02 ` Daniel Vetter -1 siblings, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 14:02 UTC (permalink / raw) To: Ville Syrjälä Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 04:35:50PM +0300, Ville Syrj�l� wrote: > On Wed, Oct 26, 2016 at 04:30:33PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > The i2c adapter is only relevant for some peer device types, so > > let's clear the pdt if it's still the same as the old_pdt when we > > tear down the i2c adapter. > > > > I don't really like this design pattern of updating port->whatever > > before doing the accompanying changes and passing around old_whatever > > to figure stuff out. Would make much more sense to me to the pass the > > new value around and only update the port->whatever when things are > > consistent. But let's try to work with what we have right now. > > > > v2: Clear port->pdt in the caller, if needed (Daniel) > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > 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> (v1) > > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > > 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_dp_mst_topology.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 04e457117980..ba13f9d8720b 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) > > /* no need to clean up vcpi > > * as if we have no connector we never setup a vcpi */ > > drm_dp_port_teardown_pdt(port, port->pdt); > > + port->pdt = DP_PEER_DEVICE_NONE; > > And naturally I forgot to amend the commit message w.r.t. this guy. > We don't really need to do this here, but I figured I'd try to be a > bit more consistent by having it, just to avoid accidental mistakes > if/when someone changes this stuff again later. With the commit message amended like you say here: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > } > > kfree(port); > > } > > @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > > mgr->cbs->destroy_connector(mgr, port->connector); > > > > drm_dp_port_teardown_pdt(port, port->pdt); > > + port->pdt = DP_PEER_DEVICE_NONE; > > > > if (!port->input && port->vcpi.vcpi > 0) { > > drm_dp_mst_reset_vcpi_slots(mgr, port); > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrj�l� > Intel OTC > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 14:02 ` Daniel Vetter 0 siblings, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 14:02 UTC (permalink / raw) To: Ville Syrjälä Cc: Kirill A . Shutemov, stable, Carlos Santa, dri-devel On Wed, Oct 26, 2016 at 04:35:50PM +0300, Ville Syrjälä wrote: > On Wed, Oct 26, 2016 at 04:30:33PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The i2c adapter is only relevant for some peer device types, so > > let's clear the pdt if it's still the same as the old_pdt when we > > tear down the i2c adapter. > > > > I don't really like this design pattern of updating port->whatever > > before doing the accompanying changes and passing around old_whatever > > to figure stuff out. Would make much more sense to me to the pass the > > new value around and only update the port->whatever when things are > > consistent. But let's try to work with what we have right now. > > > > v2: Clear port->pdt in the caller, if needed (Daniel) > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > 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> (v1) > > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > > 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_dp_mst_topology.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 04e457117980..ba13f9d8720b 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) > > /* no need to clean up vcpi > > * as if we have no connector we never setup a vcpi */ > > drm_dp_port_teardown_pdt(port, port->pdt); > > + port->pdt = DP_PEER_DEVICE_NONE; > > And naturally I forgot to amend the commit message w.r.t. this guy. > We don't really need to do this here, but I figured I'd try to be a > bit more consistent by having it, just to avoid accidental mistakes > if/when someone changes this stuff again later. With the commit message amended like you say here: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > } > > kfree(port); > > } > > @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > > mgr->cbs->destroy_connector(mgr, port->connector); > > > > drm_dp_port_teardown_pdt(port, port->pdt); > > + port->pdt = DP_PEER_DEVICE_NONE; > > > > if (!port->input && port->vcpi.vcpi > 0) { > > drm_dp_mst_reset_vcpi_slots(mgr, port); > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 14:02 ` Daniel Vetter @ 2016-10-26 16:54 ` Daniel Vetter -1 siblings, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 16:54 UTC (permalink / raw) To: Ville Syrjälä Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 04:02:57PM +0200, Daniel Vetter wrote: > On Wed, Oct 26, 2016 at 04:35:50PM +0300, Ville Syrj�l� wrote: > > On Wed, Oct 26, 2016 at 04:30:33PM +0300, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > > > The i2c adapter is only relevant for some peer device types, so > > > let's clear the pdt if it's still the same as the old_pdt when we > > > tear down the i2c adapter. > > > > > > I don't really like this design pattern of updating port->whatever > > > before doing the accompanying changes and passing around old_whatever > > > to figure stuff out. Would make much more sense to me to the pass the > > > new value around and only update the port->whatever when things are > > > consistent. But let's try to work with what we have right now. > > > > > > v2: Clear port->pdt in the caller, if needed (Daniel) > > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > 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> (v1) > > > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > > > 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_dp_mst_topology.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 04e457117980..ba13f9d8720b 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) > > > /* no need to clean up vcpi > > > * as if we have no connector we never setup a vcpi */ > > > drm_dp_port_teardown_pdt(port, port->pdt); > > > + port->pdt = DP_PEER_DEVICE_NONE; > > > > And naturally I forgot to amend the commit message w.r.t. this guy. > > We don't really need to do this here, but I figured I'd try to be a > > bit more consistent by having it, just to avoid accidental mistakes > > if/when someone changes this stuff again later. > > With the commit message amended like you say here: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Bikeshedded while applying and put the bunch into drm-misc-fixes. -Daniel > > > > > > } > > > kfree(port); > > > } > > > @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > > > mgr->cbs->destroy_connector(mgr, port->connector); > > > > > > drm_dp_port_teardown_pdt(port, port->pdt); > > > + port->pdt = DP_PEER_DEVICE_NONE; > > > > > > if (!port->input && port->vcpi.vcpi > 0) { > > > drm_dp_mst_reset_vcpi_slots(mgr, port); > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrj�l� > > Intel OTC > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 16:54 ` Daniel Vetter 0 siblings, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 16:54 UTC (permalink / raw) To: Ville Syrjälä Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 04:02:57PM +0200, Daniel Vetter wrote: > On Wed, Oct 26, 2016 at 04:35:50PM +0300, Ville Syrjälä wrote: > > On Wed, Oct 26, 2016 at 04:30:33PM +0300, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > The i2c adapter is only relevant for some peer device types, so > > > let's clear the pdt if it's still the same as the old_pdt when we > > > tear down the i2c adapter. > > > > > > I don't really like this design pattern of updating port->whatever > > > before doing the accompanying changes and passing around old_whatever > > > to figure stuff out. Would make much more sense to me to the pass the > > > new value around and only update the port->whatever when things are > > > consistent. But let's try to work with what we have right now. > > > > > > v2: Clear port->pdt in the caller, if needed (Daniel) > > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > 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> (v1) > > > Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) > > > 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_dp_mst_topology.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 04e457117980..ba13f9d8720b 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) > > > /* no need to clean up vcpi > > > * as if we have no connector we never setup a vcpi */ > > > drm_dp_port_teardown_pdt(port, port->pdt); > > > + port->pdt = DP_PEER_DEVICE_NONE; > > > > And naturally I forgot to amend the commit message w.r.t. this guy. > > We don't really need to do this here, but I figured I'd try to be a > > bit more consistent by having it, just to avoid accidental mistakes > > if/when someone changes this stuff again later. > > With the commit message amended like you say here: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Bikeshedded while applying and put the bunch into drm-misc-fixes. -Daniel > > > > > > } > > > kfree(port); > > > } > > > @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > > > mgr->cbs->destroy_connector(mgr, port->connector); > > > > > > drm_dp_port_teardown_pdt(port, port->pdt); > > > + port->pdt = DP_PEER_DEVICE_NONE; > > > > > > if (!port->input && port->vcpi.vcpi > 0) { > > > drm_dp_mst_reset_vcpi_slots(mgr, port); > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrjälä > > Intel OTC > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter 2016-10-26 9:05 ` ville.syrjala @ 2016-10-26 14:46 ` ville.syrjala -1 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 14:46 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter, stable, Carlos Santa, Kirill A . Shutemov From: Ville Syrjälä <ville.syrjala@linux.intel.com> The i2c adapter is only relevant for some peer device types, so let's clear the pdt if it's still the same as the old_pdt when we tear down the i2c adapter. I don't really like this design pattern of updating port->whatever before doing the accompanying changes and passing around old_whatever to figure stuff out. Would make much more sense to me to the pass the new value around and only update the port->whatever when things are consistent. But let's try to work with what we have right now. Clearing port->pdt in drm_dp_destroy_connector_work() would be enough, but I chose to do it also in drm_dp_destroy_port(). The reason being extra consistency so that we don't lose this by accident should someone choose to refactor the code later. v2: Cleart port->pdt in the caller, if needed (Daniel) v3: Amend commit message a bit (Daniel) Cc: Daniel Vetter <daniel@ffwll.ch> 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> (v1) Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..ba13f9d8720b 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) /* no need to clean up vcpi * as if we have no connector we never setup a vcpi */ drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; } kfree(port); } @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) mgr->cbs->destroy_connector(mgr, port->connector); drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; if (!port->input && port->vcpi.vcpi > 0) { drm_dp_mst_reset_vcpi_slots(mgr, port); -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v3 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter @ 2016-10-26 14:46 ` ville.syrjala 0 siblings, 0 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 14:46 UTC (permalink / raw) To: intel-gfx; +Cc: Kirill A . Shutemov, stable From: Ville Syrjälä <ville.syrjala@linux.intel.com> The i2c adapter is only relevant for some peer device types, so let's clear the pdt if it's still the same as the old_pdt when we tear down the i2c adapter. I don't really like this design pattern of updating port->whatever before doing the accompanying changes and passing around old_whatever to figure stuff out. Would make much more sense to me to the pass the new value around and only update the port->whatever when things are consistent. But let's try to work with what we have right now. Clearing port->pdt in drm_dp_destroy_connector_work() would be enough, but I chose to do it also in drm_dp_destroy_port(). The reason being extra consistency so that we don't lose this by accident should someone choose to refactor the code later. v2: Cleart port->pdt in the caller, if needed (Daniel) v3: Amend commit message a bit (Daniel) Cc: Daniel Vetter <daniel@ffwll.ch> 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> (v1) Tested-by: Kirill A. Shutemov <kirill@shutemov.name> (v1) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97666 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 04e457117980..ba13f9d8720b 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -914,6 +914,7 @@ static void drm_dp_destroy_port(struct kref *kref) /* no need to clean up vcpi * as if we have no connector we never setup a vcpi */ drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; } kfree(port); } @@ -2919,6 +2920,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) mgr->cbs->destroy_connector(mgr, port->connector); drm_dp_port_teardown_pdt(port, port->pdt); + port->pdt = DP_PEER_DEVICE_NONE; if (!port->input && port->vcpi.vcpi > 0) { drm_dp_mst_reset_vcpi_slots(mgr, port); -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read 2016-10-26 9:05 [PATCH 0/4] drm: Fix MST oopses in fbdev restore ville.syrjala ` (2 preceding siblings ...) 2016-10-26 9:05 ` ville.syrjala @ 2016-10-26 9:05 ` ville.syrjala 2016-10-26 12:55 ` Daniel Vetter 2016-10-26 18:31 ` Carlos Santa 2016-10-26 13:38 ` [PATCH 0/4] drm: Fix MST oopses in fbdev restore Ville Syrjälä 4 siblings, 2 replies; 43+ messages in thread From: ville.syrjala @ 2016-10-26 9:05 UTC (permalink / raw) To: dri-devel; +Cc: stable, Carlos Santa, Kirill A . Shutemov From: Ville Syrjälä <ville.syrjala@linux.intel.com> Only certain types of pdts have the DDC bus registered, so check for that before we attempt the EDID read. Othwewise we risk playing around with an i2c adapter that doesn't actually exist. 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_dp_mst_topology.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 956babc161e5..690d1b407a90 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, drm_dp_put_port(port); goto out; } - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || + port->pdt == DP_PEER_DEVICE_SST_SINK) && + port->port_num >= DP_MST_LOGICAL_PORT_0) { port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_mode_connector_set_tile_property(port->connector); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read 2016-10-26 9:05 ` [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read ville.syrjala @ 2016-10-26 12:55 ` Daniel Vetter 2016-10-26 18:31 ` Carlos Santa 1 sibling, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 12:55 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 12:05:55PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > Only certain types of pdts have the DDC bus registered, so check for > that before we attempt the EDID read. Othwewise we risk playing around > with an i2c adapter that doesn't actually exist. > > 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_dp_mst_topology.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 956babc161e5..690d1b407a90 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, > drm_dp_put_port(port); > goto out; > } > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > + port->pdt == DP_PEER_DEVICE_SST_SINK) && > + port->port_num >= DP_MST_LOGICAL_PORT_0) { Matches what's in drm_dp_port_setup_pdt. Not sure this is the most reliable way to do this though, but I can't come up with anything better. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); > drm_mode_connector_set_tile_property(port->connector); > } > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read @ 2016-10-26 12:55 ` Daniel Vetter 0 siblings, 0 replies; 43+ messages in thread From: Daniel Vetter @ 2016-10-26 12:55 UTC (permalink / raw) To: ville.syrjala; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 12:05:55PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Only certain types of pdts have the DDC bus registered, so check for > that before we attempt the EDID read. Othwewise we risk playing around > with an i2c adapter that doesn't actually exist. > > 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_dp_mst_topology.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 956babc161e5..690d1b407a90 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, > drm_dp_put_port(port); > goto out; > } > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > + port->pdt == DP_PEER_DEVICE_SST_SINK) && > + port->port_num >= DP_MST_LOGICAL_PORT_0) { Matches what's in drm_dp_port_setup_pdt. Not sure this is the most reliable way to do this though, but I can't come up with anything better. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); > drm_mode_connector_set_tile_property(port->connector); > } > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read 2016-10-26 12:55 ` Daniel Vetter @ 2016-10-26 13:05 ` Ville Syrjälä -1 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 13:05 UTC (permalink / raw) To: Daniel Vetter; +Cc: dri-devel, Kirill A . Shutemov, Carlos Santa, stable On Wed, Oct 26, 2016 at 02:55:14PM +0200, Daniel Vetter wrote: > On Wed, Oct 26, 2016 at 12:05:55PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > Only certain types of pdts have the DDC bus registered, so check for > > that before we attempt the EDID read. Othwewise we risk playing around > > with an i2c adapter that doesn't actually exist. > > > > 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_dp_mst_topology.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 956babc161e5..690d1b407a90 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, > > drm_dp_put_port(port); > > goto out; > > } > > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > > + port->pdt == DP_PEER_DEVICE_SST_SINK) && > > + port->port_num >= DP_MST_LOGICAL_PORT_0) { > > Matches what's in drm_dp_port_setup_pdt. Not sure this is the most > reliable way to do this though, but I can't come up with anything better. Yeah. I just went with whatever looks semi-correct based on where the thing is set up. I'd have to go read the actual spec to figure out if this makes any real sense. But at least it avoids the explosion. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); > > drm_mode_connector_set_tile_property(port->connector); > > } > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read @ 2016-10-26 13:05 ` Ville Syrjälä 0 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 13:05 UTC (permalink / raw) To: Daniel Vetter; +Cc: Kirill A . Shutemov, stable, Carlos Santa, dri-devel On Wed, Oct 26, 2016 at 02:55:14PM +0200, Daniel Vetter wrote: > On Wed, Oct 26, 2016 at 12:05:55PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Only certain types of pdts have the DDC bus registered, so check for > > that before we attempt the EDID read. Othwewise we risk playing around > > with an i2c adapter that doesn't actually exist. > > > > 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_dp_mst_topology.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 956babc161e5..690d1b407a90 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, > > drm_dp_put_port(port); > > goto out; > > } > > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > > + port->pdt == DP_PEER_DEVICE_SST_SINK) && > > + port->port_num >= DP_MST_LOGICAL_PORT_0) { > > Matches what's in drm_dp_port_setup_pdt. Not sure this is the most > reliable way to do this though, but I can't come up with anything better. Yeah. I just went with whatever looks semi-correct based on where the thing is set up. I'd have to go read the actual spec to figure out if this makes any real sense. But at least it avoids the explosion. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); > > drm_mode_connector_set_tile_property(port->connector); > > } > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read 2016-10-26 9:05 ` [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read ville.syrjala @ 2016-10-26 18:31 ` Carlos Santa 2016-10-26 18:31 ` Carlos Santa 1 sibling, 0 replies; 43+ messages in thread From: Carlos Santa @ 2016-10-26 18:31 UTC (permalink / raw) To: ville.syrjala, dri-devel; +Cc: stable, Kirill A . Shutemov On Wed, 2016-10-26 at 02:05 -0700, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Only certain types of pdts have the DDC bus registered, so check for > that before we attempt the EDID read. Othwewise we risk playing typo above > around > with an i2c adapter that doesn't actually exist. > Would it be worth pasting the stack trace of the crash?? Carlos > 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_dp_mst_topology.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 956babc161e5..690d1b407a90 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct > drm_dp_mst_branch *mstb, > drm_dp_put_port(port); > goto out; > } > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > + port->pdt == DP_PEER_DEVICE_SST_SINK) && > + port->port_num >= DP_MST_LOGICAL_PORT_0) { > port->cached_edid = drm_get_edid(port- > >connector, &port->aux.ddc); > drm_mode_connector_set_tile_property(port- > >connector); > } > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read @ 2016-10-26 18:31 ` Carlos Santa 0 siblings, 0 replies; 43+ messages in thread From: Carlos Santa @ 2016-10-26 18:31 UTC (permalink / raw) To: ville.syrjala, dri-devel; +Cc: Kirill A . Shutemov, stable On Wed, 2016-10-26 at 02:05 -0700, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Only certain types of pdts have the DDC bus registered, so check for > that before we attempt the EDID read. Othwewise we risk playing typo above > around > with an i2c adapter that doesn't actually exist. > Would it be worth pasting the stack trace of the crash?? Carlos > 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_dp_mst_topology.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 956babc161e5..690d1b407a90 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct > drm_dp_mst_branch *mstb, > drm_dp_put_port(port); > goto out; > } > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > + port->pdt == DP_PEER_DEVICE_SST_SINK) && > + port->port_num >= DP_MST_LOGICAL_PORT_0) { > port->cached_edid = drm_get_edid(port- > >connector, &port->aux.ddc); > drm_mode_connector_set_tile_property(port- > >connector); > } > -- > 2.7.4 > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read 2016-10-26 18:31 ` Carlos Santa @ 2016-10-26 18:45 ` Ville Syrjälä -1 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 18:45 UTC (permalink / raw) To: Carlos Santa; +Cc: dri-devel, stable, Kirill A . Shutemov On Wed, Oct 26, 2016 at 11:31:55AM -0700, Carlos Santa wrote: > On Wed, 2016-10-26 at 02:05 -0700, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > Only certain types of pdts have the DDC bus registered, so check for > > that before we attempt the EDID read. Othwewise we risk playing > > typo above > > > around > > with an i2c adapter that doesn't actually exist. > > > > Would it be worth pasting the stack trace of the crash?? Can't hurt. > > Carlos > > > > 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_dp_mst_topology.c | 4 +++- > > �1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 956babc161e5..690d1b407a90 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct > > drm_dp_mst_branch *mstb, > > � drm_dp_put_port(port); > > � goto out; > > � } > > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > > + �����port->pdt == DP_PEER_DEVICE_SST_SINK) && > > + ����port->port_num >= DP_MST_LOGICAL_PORT_0) { > > � port->cached_edid = drm_get_edid(port- > > >connector, &port->aux.ddc); > > � drm_mode_connector_set_tile_property(port- > > >connector); > > � } > > --� > > 2.7.4 > > -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read @ 2016-10-26 18:45 ` Ville Syrjälä 0 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 18:45 UTC (permalink / raw) To: Carlos Santa; +Cc: dri-devel, stable, Kirill A . Shutemov On Wed, Oct 26, 2016 at 11:31:55AM -0700, Carlos Santa wrote: > On Wed, 2016-10-26 at 02:05 -0700, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Only certain types of pdts have the DDC bus registered, so check for > > that before we attempt the EDID read. Othwewise we risk playing > > typo above > > > around > > with an i2c adapter that doesn't actually exist. > > > > Would it be worth pasting the stack trace of the crash?? Can't hurt. > > Carlos > > > > 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_dp_mst_topology.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 956babc161e5..690d1b407a90 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -1162,7 +1162,9 @@ static void drm_dp_add_port(struct > > drm_dp_mst_branch *mstb, > > drm_dp_put_port(port); > > goto out; > > } > > - if (port->port_num >= DP_MST_LOGICAL_PORT_0) { > > + if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || > > + port->pdt == DP_PEER_DEVICE_SST_SINK) && > > + port->port_num >= DP_MST_LOGICAL_PORT_0) { > > port->cached_edid = drm_get_edid(port- > > >connector, &port->aux.ddc); > > drm_mode_connector_set_tile_property(port- > > >connector); > > } > > -- > > 2.7.4 > > -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 0/4] drm: Fix MST oopses in fbdev restore 2016-10-26 9:05 [PATCH 0/4] drm: Fix MST oopses in fbdev restore ville.syrjala ` (3 preceding siblings ...) 2016-10-26 9:05 ` [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read ville.syrjala @ 2016-10-26 13:38 ` Ville Syrjälä 4 siblings, 0 replies; 43+ messages in thread From: Ville Syrjälä @ 2016-10-26 13:38 UTC (permalink / raw) To: dri-devel; +Cc: Kirill A . Shutemov, Carlos Santa On Wed, Oct 26, 2016 at 12:05:51PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > This series would appear to be enough to avoid kernel oopses when trying > to restore the fbdev setup after a MST device has been yanked. > > Apparently we still have some problems left in actually reacting properly > to the changed situation, but at least the kernel no longer explodes. > > Entire series available here: > git://github.com/vsyrjala/linux.git dp_mst_fixes_3 v2 patches available at: git://github.com/vsyrjala/linux.git dp_mst_fixes_4 in case people want to re-test. > > Cc: Carlos Santa <carlos.santa@intel.com> > Cc: Kirill A. Shutemov <kirill@shutemov.name> > > Ville Syrjälä (4): > drm/fb-helper: Fix connector ref leak on error > drm/fb-helper: Keep references for the current set of used connectors > drm/dp/mst: Clear port->pdt when tearing down the i2c adapter > drm/dp/mst: Check peer device type before attempting EDID read > > drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++++- > drivers/gpu/drm/drm_fb_helper.c | 32 ++++++++++++++++++++++++-------- > 2 files changed, 30 insertions(+), 9 deletions(-) > > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2016-10-26 18:46 UTC | newest] Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-26 9:05 [PATCH 0/4] drm: Fix MST oopses in fbdev restore ville.syrjala 2016-10-26 9:05 ` [PATCH 1/4] drm/fb-helper: Fix connector ref leak on error ville.syrjala 2016-10-26 9:48 ` Chris Wilson 2016-10-26 9:48 ` Chris Wilson 2016-10-26 9:05 ` [PATCH 2/4] drm/fb-helper: Keep references for the current set of used connectors ville.syrjala 2016-10-26 9:05 ` ville.syrjala 2016-10-26 9:52 ` Chris Wilson 2016-10-26 9:52 ` Chris Wilson 2016-10-26 12:34 ` Ville Syrjälä 2016-10-26 12:34 ` Ville Syrjälä 2016-10-26 13:31 ` [PATCH v2 " ville.syrjala 2016-10-26 13:54 ` Chris Wilson 2016-10-26 13:54 ` Chris Wilson 2016-10-26 14:11 ` Ville Syrjälä 2016-10-26 14:11 ` Ville Syrjälä 2016-10-26 14:41 ` [PATCH v3 " ville.syrjala 2016-10-26 14:41 ` ville.syrjala 2016-10-26 9:05 ` [PATCH 3/4] drm/dp/mst: Clear port->pdt when tearing down the i2c adapter ville.syrjala 2016-10-26 9:05 ` ville.syrjala 2016-10-26 12:53 ` Daniel Vetter 2016-10-26 12:53 ` Daniel Vetter 2016-10-26 13:02 ` Ville Syrjälä 2016-10-26 13:02 ` Ville Syrjälä 2016-10-26 13:30 ` [PATCH v2 " ville.syrjala 2016-10-26 13:30 ` ville.syrjala 2016-10-26 13:35 ` Ville Syrjälä 2016-10-26 13:35 ` Ville Syrjälä 2016-10-26 14:02 ` Daniel Vetter 2016-10-26 14:02 ` Daniel Vetter 2016-10-26 16:54 ` Daniel Vetter 2016-10-26 16:54 ` Daniel Vetter 2016-10-26 14:46 ` [PATCH v3 " ville.syrjala 2016-10-26 14:46 ` ville.syrjala 2016-10-26 9:05 ` [PATCH 4/4] drm/dp/mst: Check peer device type before attempting EDID read ville.syrjala 2016-10-26 12:55 ` Daniel Vetter 2016-10-26 12:55 ` Daniel Vetter 2016-10-26 13:05 ` Ville Syrjälä 2016-10-26 13:05 ` Ville Syrjälä 2016-10-26 18:31 ` Carlos Santa 2016-10-26 18:31 ` Carlos Santa 2016-10-26 18:45 ` Ville Syrjälä 2016-10-26 18:45 ` Ville Syrjälä 2016-10-26 13:38 ` [PATCH 0/4] drm: Fix MST oopses in fbdev restore Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.