| Message ID | 1446644611-26532-1-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 04, 2015 at 02:43:31PM +0100, Maarten Lankhorst wrote: > When setcrtc is called and steals the last connector away from a crtc > it's turned off because it can't stay configured without connectors. > > The framebuffer is still preserved however, and that causes troubles > in the IGT stress test kms_flip.flips-vs-fences which tries to use > as many pins as possible and hangs on the third crtc because of > framebuffer pins on the first 2 crtc's. > > Cc: stable@vger.kernel.org #v4.3 > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> drm_atomic_helper_set_config clears the fb for the primary plane if we clear the mode. Where is the leak? Also where exactly does it hang - we should have plenty of fences left for 3x2 pinned framebuffers. If you allow more than 2 pinned fb per plane then that's a bug somewhere with pageflips getting ahead of the unpin worker. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 5e520ae5f42e..d95d8acae51f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13571,15 +13571,17 @@ intel_prepare_plane_fb(struct drm_plane *plane, > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); > + struct drm_crtc_state *crtc_state; > int ret = 0; > > if (!obj && !old_obj) > return 0; > > - if (old_obj) { > - struct drm_crtc_state *crtc_state = > - drm_atomic_get_existing_crtc_state(new_state->state, plane->state->crtc); > + crtc_state = drm_atomic_get_existing_crtc_state(new_state->state, > + new_state->crtc ?: > + plane->state->crtc); > > + if (old_obj) { > /* Big Hammer, we also need to ensure that any pending > * MI_WAIT_FOR_EVENT inside a user batch buffer on the > * current scanout is retired before unpinning the old > @@ -13599,7 +13601,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, > return ret; > } > > - if (!obj) { > + if (!obj || !crtc_state->enable) { > ret = 0; > } else if (plane->type == DRM_PLANE_TYPE_CURSOR && > INTEL_INFO(dev)->cursor_needs_physical) { > @@ -13644,15 +13646,22 @@ intel_cleanup_plane_fb(struct drm_plane *plane, > struct intel_plane_state *old_intel_state; > struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); > struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); > + struct drm_crtc_state *old_crtc_state; > > old_intel_state = to_intel_plane_state(old_state); > > if (!obj && !old_obj) > return; > > - if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || > - !INTEL_INFO(dev)->cursor_needs_physical)) > + old_crtc_state = drm_atomic_get_existing_crtc_state(old_state->state, > + old_state->crtc ?: > + plane->state->crtc); > + > + if (old_obj && old_crtc_state->enable && > + (plane->type != DRM_PLANE_TYPE_CURSOR || > + !INTEL_INFO(dev)->cursor_needs_physical)) { > intel_unpin_fb_obj(old_state->fb, old_state); > + } > > /* prepare_fb aborted? */ > if ((old_obj && (old_obj->frontbuffer_bits & intel_plane->frontbuffer_bit)) || > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 17-11-15 om 19:54 schreef Daniel Vetter: > On Wed, Nov 04, 2015 at 02:43:31PM +0100, Maarten Lankhorst wrote: >> When setcrtc is called and steals the last connector away from a crtc >> it's turned off because it can't stay configured without connectors. >> >> The framebuffer is still preserved however, and that causes troubles >> in the IGT stress test kms_flip.flips-vs-fences which tries to use >> as many pins as possible and hangs on the third crtc because of >> framebuffer pins on the first 2 crtc's. >> >> Cc: stable@vger.kernel.org #v4.3 >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > drm_atomic_helper_set_config clears the fb for the primary plane if we > clear the mode. Where is the leak? Steal the last connector from another crtc. The crtc is deconfigured because it has no connectors any more, its the planes are left untouched. > Also where exactly does it hang - we should have plenty of fences left for > 3x2 pinned framebuffers. If you allow more than 2 pinned fb per plane then > that's a bug somewhere with pageflips getting ahead of the unpin worker. > -Daniel > Just try that stress test, it submits max_fences-a few so you will get -ENOSPC because it ran out out of fences. :) ~Maarten
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5e520ae5f42e..d95d8acae51f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13571,15 +13571,17 @@ intel_prepare_plane_fb(struct drm_plane *plane, struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); + struct drm_crtc_state *crtc_state; int ret = 0; if (!obj && !old_obj) return 0; - if (old_obj) { - struct drm_crtc_state *crtc_state = - drm_atomic_get_existing_crtc_state(new_state->state, plane->state->crtc); + crtc_state = drm_atomic_get_existing_crtc_state(new_state->state, + new_state->crtc ?: + plane->state->crtc); + if (old_obj) { /* Big Hammer, we also need to ensure that any pending * MI_WAIT_FOR_EVENT inside a user batch buffer on the * current scanout is retired before unpinning the old @@ -13599,7 +13601,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, return ret; } - if (!obj) { + if (!obj || !crtc_state->enable) { ret = 0; } else if (plane->type == DRM_PLANE_TYPE_CURSOR && INTEL_INFO(dev)->cursor_needs_physical) { @@ -13644,15 +13646,22 @@ intel_cleanup_plane_fb(struct drm_plane *plane, struct intel_plane_state *old_intel_state; struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); + struct drm_crtc_state *old_crtc_state; old_intel_state = to_intel_plane_state(old_state); if (!obj && !old_obj) return; - if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || - !INTEL_INFO(dev)->cursor_needs_physical)) + old_crtc_state = drm_atomic_get_existing_crtc_state(old_state->state, + old_state->crtc ?: + plane->state->crtc); + + if (old_obj && old_crtc_state->enable && + (plane->type != DRM_PLANE_TYPE_CURSOR || + !INTEL_INFO(dev)->cursor_needs_physical)) { intel_unpin_fb_obj(old_state->fb, old_state); + } /* prepare_fb aborted? */ if ((old_obj && (old_obj->frontbuffer_bits & intel_plane->frontbuffer_bit)) ||
When setcrtc is called and steals the last connector away from a crtc it's turned off because it can't stay configured without connectors. The framebuffer is still preserved however, and that causes troubles in the IGT stress test kms_flip.flips-vs-fences which tries to use as many pins as possible and hangs on the third crtc because of framebuffer pins on the first 2 crtc's. Cc: stable@vger.kernel.org #v4.3 Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)