| Message ID | 1418689401-22957-2-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/16/2014 02:23 AM, Matt Roper wrote: > Once we integrate our work into the atomic pipeline, plane commit > operations will need to happen with interrupts disabled, due to vblank > evasion. Our commit functions today include sleepable work, so those > operations need to be split out and run either before or after the > atomic register programming. > > The solution here calculates which of those operations will need to be > performed during the 'check' phase and sets flags in an intel_crtc > sub-struct. New intel_begin_crtc_commit() and > intel_finish_crtc_commit() functions are added before and after the > actual register programming; these will eventually be called from the > atomic plane helper's .atomic_begin() and .atomic_end() entrypoints. > > v2: Fix broken sprite code split > > v3: Make the pre/post commit work crtc-based to match how we eventually > want this to be called from the atomic plane helpers. > > v4: Some platforms that haven't had their watermark code reworked were > waiting for vblank, then calling update_sprite_watermarks in their > platform-specific disable code. These also need to be flagged out > of the critical section. > > v5: Sprite plane test for primary show/hide should just set the flag to > wait for pending flips, not actually perform the wait. (Ander) > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 180 ++++++++++++++++++++++------------- > drivers/gpu/drm/i915/intel_drv.h | 31 ++++++ > drivers/gpu/drm/i915/intel_sprite.c | 78 ++++++--------- > 3 files changed, 178 insertions(+), 111 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3044af5..5d90114 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11737,7 +11737,11 @@ static int > intel_check_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > + struct drm_device *dev = plane->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc *crtc = state->base.crtc; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_framebuffer *fb = state->base.fb; > struct drm_rect *dest = &state->dst; > struct drm_rect *src = &state->src; > @@ -11758,6 +11762,40 @@ intel_check_primary_plane(struct drm_plane *plane, > return -EBUSY; > } > > + if (intel_crtc->active) { > + /* > + * FBC does not work on some platforms for rotated > + * planes, so disable it when rotation is not 0 and > + * update it when rotation is set back to 0. > + * > + * FIXME: This is redundant with the fbc update done in > + * the primary plane enable function except that that > + * one is done too late. We eventually need to unify > + * this. > + */ > + if (intel_crtc->primary_enabled && > + INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > + dev_priv->fbc.plane == intel_crtc->plane && > + intel_plane->rotation != BIT(DRM_ROTATE_0)) { > + intel_crtc->atomic.disable_fbc = true; > + } > + > + if (state->visible) { > + /* > + * BDW signals flip done immediately if the plane > + * is disabled, even if the plane enable is already > + * armed to occur at the next vblank :( > + */ > + if (IS_BROADWELL(dev) && !intel_crtc->primary_enabled) > + intel_crtc->atomic.wait_vblank = true; > + } > + > + intel_crtc->atomic.fb_bits |= > + INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe); > + > + intel_crtc->atomic.update_fbc = true; > + } > + > return 0; > } > > @@ -11773,18 +11811,6 @@ intel_commit_primary_plane(struct drm_plane *plane, > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_rect *src = &state->src; > - enum pipe pipe = intel_plane->pipe; > - > - if (!fb) { > - /* > - * 'prepare' is never called when plane is being disabled, so > - * we need to handle frontbuffer tracking here > - */ > - mutex_lock(&dev->struct_mutex); > - i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, > - INTEL_FRONTBUFFER_PRIMARY(pipe)); > - mutex_unlock(&dev->struct_mutex); > - } > > plane->fb = fb; > crtc->x = src->x1 >> 16; > @@ -11801,26 +11827,7 @@ intel_commit_primary_plane(struct drm_plane *plane, > intel_plane->obj = obj; > > if (intel_crtc->active) { > - /* > - * FBC does not work on some platforms for rotated > - * planes, so disable it when rotation is not 0 and > - * update it when rotation is set back to 0. > - * > - * FIXME: This is redundant with the fbc update done in > - * the primary plane enable function except that that > - * one is done too late. We eventually need to unify > - * this. > - */ > - if (intel_crtc->primary_enabled && > - INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && > - dev_priv->fbc.plane == intel_crtc->plane && > - intel_plane->rotation != BIT(DRM_ROTATE_0)) { > - intel_fbc_disable(dev); > - } > - > if (state->visible) { > - bool was_enabled = intel_crtc->primary_enabled; > - > /* FIXME: kill this fastboot hack */ > intel_update_pipe_size(intel_crtc); > > @@ -11828,14 +11835,6 @@ intel_commit_primary_plane(struct drm_plane *plane, > > dev_priv->display.update_primary_plane(crtc, plane->fb, > crtc->x, crtc->y); > - > - /* > - * BDW signals flip done immediately if the plane > - * is disabled, even if the plane enable is already > - * armed to occur at the next vblank :( > - */ > - if (IS_BROADWELL(dev) && !was_enabled) > - intel_wait_for_vblank(dev, intel_crtc->pipe); > } else { > /* > * If clipping results in a non-visible primary plane, > @@ -11846,13 +11845,50 @@ intel_commit_primary_plane(struct drm_plane *plane, > */ > intel_disable_primary_hw_plane(plane, crtc); > } > + } > +} > > - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe)); > +static void intel_begin_crtc_commit(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + > + if (intel_crtc->atomic.disable_fbc) > + intel_fbc_disable(dev); > + > + if (intel_crtc->atomic.pre_disable_primary) > + intel_pre_disable_primary(crtc); > + > + if (intel_crtc->atomic.update_wm) > + intel_update_watermarks(crtc); > +} > > +static void intel_finish_crtc_commit(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_plane *p; > + > + if (intel_crtc->atomic.wait_vblank) > + intel_wait_for_vblank(dev, intel_crtc->pipe); > + > + intel_frontbuffer_flip(dev, intel_crtc->atomic.fb_bits); > + > + if (intel_crtc->atomic.update_fbc) { > mutex_lock(&dev->struct_mutex); > intel_fbc_update(dev); > mutex_unlock(&dev->struct_mutex); > } > + > + if (intel_crtc->atomic.post_enable_primary) > + intel_post_enable_primary(crtc); > + > + drm_for_each_legacy_plane(p, &dev->mode_config.plane_list) > + if (intel_crtc->atomic.update_sprite_watermarks & drm_plane_index(p)) > + intel_update_sprite_watermarks(p, crtc, 0, 0, 0, > + false, false); > + > + memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic)); > } > > int > @@ -11864,7 +11900,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > { > struct drm_device *dev = plane->dev; > struct drm_framebuffer *old_fb = plane->fb; > - struct intel_plane_state state; > + struct intel_plane_state state = {{ 0 }}; > struct intel_plane *intel_plane = to_intel_plane(plane); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int ret; > @@ -11902,7 +11938,33 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > return ret; > } > > + if (!state.base.fb) { > + unsigned fb_bits = 0; > + > + switch (plane->type) { > + case DRM_PLANE_TYPE_PRIMARY: > + fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe); > + break; > + case DRM_PLANE_TYPE_CURSOR: > + fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe); > + break; > + case DRM_PLANE_TYPE_OVERLAY: > + fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe); > + break; > + } > + > + /* > + * 'prepare' is never called when plane is being disabled, so > + * we need to handle frontbuffer tracking here > + */ > + mutex_lock(&dev->struct_mutex); > + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, fb_bits); > + mutex_unlock(&dev->struct_mutex); > + } > + > + intel_begin_crtc_commit(crtc); > intel_plane->commit_plane(plane, &state); > + intel_finish_crtc_commit(crtc); This needs to be rebased on top of the patch that added the intel_runtime_pm_get() and _put() calls here. > if (fb != old_fb && old_fb) { > if (intel_crtc->active) > @@ -12009,6 +12071,7 @@ intel_check_cursor_plane(struct drm_plane *plane, > struct drm_rect *src = &state->src; > const struct drm_rect *clip = &state->clip; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int crtc_w, crtc_h; > unsigned stride; > int ret; > @@ -12024,7 +12087,7 @@ intel_check_cursor_plane(struct drm_plane *plane, > > /* if we want to turn off the cursor ignore width and height */ > if (!obj) > - return 0; > + goto finish; > > /* Check for which cursor types we support */ > crtc_w = drm_rect_width(&state->orig_dst); > @@ -12051,6 +12114,16 @@ intel_check_cursor_plane(struct drm_plane *plane, > } > mutex_unlock(&dev->struct_mutex); > > +finish: > + if (intel_crtc->active) { > + if (intel_crtc->cursor_width != > + drm_rect_width(&state->orig_dst)) > + intel_crtc->atomic.update_wm = true; > + > + intel_crtc->atomic.fb_bits |= > + INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe); > + } > + > return ret; > } > > @@ -12063,9 +12136,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > - enum pipe pipe = intel_crtc->pipe; > - unsigned old_width; > uint32_t addr; > > plane->fb = state->base.fb; > @@ -12085,17 +12155,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, > if (intel_crtc->cursor_bo == obj) > goto update; > > - /* > - * 'prepare' is only called when fb != NULL; we still need to update > - * frontbuffer tracking for the 'disable' case here. > - */ > - if (!obj) { > - mutex_lock(&dev->struct_mutex); > - i915_gem_track_fb(old_obj, NULL, > - INTEL_FRONTBUFFER_CURSOR(pipe)); > - mutex_unlock(&dev->struct_mutex); > - } > - > if (!obj) > addr = 0; > else if (!INTEL_INFO(dev)->cursor_needs_physical) > @@ -12106,18 +12165,11 @@ intel_commit_cursor_plane(struct drm_plane *plane, > intel_crtc->cursor_addr = addr; > intel_crtc->cursor_bo = obj; > update: > - old_width = intel_crtc->cursor_width; > - > intel_crtc->cursor_width = drm_rect_width(&state->orig_dst); > intel_crtc->cursor_height = drm_rect_height(&state->orig_dst); > > - if (intel_crtc->active) { > - if (old_width != intel_crtc->cursor_width) > - intel_update_watermarks(crtc); > + if (intel_crtc->active) > intel_crtc_update_cursor(crtc, state->visible); > - > - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > - } > } > > static const struct drm_plane_funcs intel_cursor_plane_funcs = { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 588b618..a03bd72 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -251,6 +251,12 @@ struct intel_plane_state { > struct drm_rect orig_src; > struct drm_rect orig_dst; > bool visible; > + > + /* > + * used only for sprite planes to determine when to implicitly > + * enable/disable the primary plane > + */ > + bool hides_primary; > }; > > struct intel_plane_config { > @@ -415,6 +421,27 @@ struct skl_pipe_wm { > uint32_t linetime; > }; > > +/* > + * Tracking of operations that need to be performed at the beginning/end of an > + * atomic commit, outside the atomic section where interrupts are disabled. > + * These are generally operations that grab mutexes or might otherwise sleep > + * and thus can't be run with interrupts disabled. > + */ > +struct intel_crtc_atomic_commit { > + /* Sleepable operations to perform before commit */ > + bool wait_for_flips; In intel_begin_crtc_commit(), the three fields below are handled, but not the one above. > + bool disable_fbc; > + bool pre_disable_primary; > + bool update_wm; > + > + /* Sleepable operations to perform after commit */ > + unsigned fb_bits; > + bool wait_vblank; > + bool update_fbc; > + bool post_enable_primary; > + unsigned update_sprite_watermarks; > +}; > + > struct intel_crtc { > struct drm_crtc base; > enum pipe pipe; > @@ -468,6 +495,8 @@ struct intel_crtc { > > int scanline_offset; > struct intel_mmio_flip mmio_flip; > + > + struct intel_crtc_atomic_commit atomic; > }; > > struct intel_plane_wm_parameters { > @@ -1213,6 +1242,8 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > bool intel_pipe_update_start(struct intel_crtc *crtc, > uint32_t *start_vbl_count); > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > +void intel_post_enable_primary(struct drm_crtc *crtc); > +void intel_pre_disable_primary(struct drm_crtc *crtc); > > /* intel_tv.c */ > void intel_tv_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index c18e57d..ff7d6a1 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -771,9 +771,8 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) > * Avoid underruns when disabling the sprite. > * FIXME remove once watermark updates are done properly. > */ > - intel_wait_for_vblank(dev, pipe); > - > - intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false); > + intel_crtc->atomic.wait_vblank = true; > + intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane)); > } > > static int > @@ -976,12 +975,11 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) > * Avoid underruns when disabling the sprite. > * FIXME remove once watermark updates are done properly. > */ > - intel_wait_for_vblank(dev, pipe); > - > - intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false); > + intel_crtc->atomic.wait_vblank = true; > + intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane)); > } > > -static void > +void > intel_post_enable_primary(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > @@ -1008,7 +1006,7 @@ intel_post_enable_primary(struct drm_crtc *crtc) > mutex_unlock(&dev->struct_mutex); > } > > -static void > +void Should this have kernel doc? > intel_pre_disable_primary(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > @@ -1113,7 +1111,7 @@ intel_check_sprite_plane(struct drm_plane *plane, > > if (!fb) { > state->visible = false; > - return 0; > + goto finish; > } > > /* Don't modify another pipe's plane */ > @@ -1260,6 +1258,29 @@ intel_check_sprite_plane(struct drm_plane *plane, > dst->y1 = crtc_y; > dst->y2 = crtc_y + crtc_h; > > +finish: > + /* > + * If the sprite is completely covering the primary plane, > + * we can disable the primary and save power. > + */ > + state->hides_primary = drm_rect_equals(dst, clip) && > + !colorkey_enabled(intel_plane); > + WARN_ON(state->hides_primary && !state->visible && intel_crtc->active); > + > + if (intel_crtc->active) { > + if (intel_crtc->primary_enabled == state->hides_primary) > + intel_crtc->atomic.wait_for_flips = true; > + > + if (intel_crtc->primary_enabled && state->hides_primary) > + intel_crtc->atomic.pre_disable_primary = true; > + > + intel_crtc->atomic.fb_bits |= > + INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe); > + > + if (!intel_crtc->primary_enabled && !state->hides_primary) > + intel_crtc->atomic.post_enable_primary = true; > + } > + > return 0; > } > > @@ -1267,37 +1288,14 @@ static void > intel_commit_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > - struct drm_device *dev = plane->dev; > struct drm_crtc *crtc = state->base.crtc; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > - enum pipe pipe = intel_crtc->pipe; > struct drm_framebuffer *fb = state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > int crtc_x, crtc_y; > unsigned int crtc_w, crtc_h; > uint32_t src_x, src_y, src_w, src_h; > - struct drm_rect *dst = &state->dst; > - const struct drm_rect *clip = &state->clip; > - bool primary_enabled; > - > - /* > - * 'prepare' is never called when plane is being disabled, so we need > - * to handle frontbuffer tracking here > - */ > - if (!fb) { > - mutex_lock(&dev->struct_mutex); > - i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, > - INTEL_FRONTBUFFER_SPRITE(pipe)); > - mutex_unlock(&dev->struct_mutex); > - } > - > - /* > - * If the sprite is completely covering the primary plane, > - * we can disable the primary and save power. > - */ > - primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane); > - WARN_ON(!primary_enabled && !state->visible && intel_crtc->active); > > intel_plane->crtc_x = state->orig_dst.x1; > intel_plane->crtc_y = state->orig_dst.y1; > @@ -1310,15 +1308,7 @@ intel_commit_sprite_plane(struct drm_plane *plane, > intel_plane->obj = obj; > > if (intel_crtc->active) { > - bool primary_was_enabled = intel_crtc->primary_enabled; > - > - intel_crtc->primary_enabled = primary_enabled; > - > - if (primary_was_enabled != primary_enabled) > - intel_crtc_wait_for_pending_flips(crtc); As noted above, it seems that this patch removes the wait for pending flips. Ander > - > - if (primary_was_enabled && !primary_enabled) > - intel_pre_disable_primary(crtc); > + intel_crtc->primary_enabled = !state->hides_primary; > > if (state->visible) { > crtc_x = state->dst.x1; > @@ -1335,12 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, > } else { > intel_plane->disable_plane(plane, crtc); > } > - > - > - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_SPRITE(pipe)); > - > - if (!primary_was_enabled && primary_enabled) > - intel_post_enable_primary(crtc); > } > } > >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3044af5..5d90114 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11737,7 +11737,11 @@ static int intel_check_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) { + struct drm_device *dev = plane->dev; + struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc = state->base.crtc; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_framebuffer *fb = state->base.fb; struct drm_rect *dest = &state->dst; struct drm_rect *src = &state->src; @@ -11758,6 +11762,40 @@ intel_check_primary_plane(struct drm_plane *plane, return -EBUSY; } + if (intel_crtc->active) { + /* + * FBC does not work on some platforms for rotated + * planes, so disable it when rotation is not 0 and + * update it when rotation is set back to 0. + * + * FIXME: This is redundant with the fbc update done in + * the primary plane enable function except that that + * one is done too late. We eventually need to unify + * this. + */ + if (intel_crtc->primary_enabled && + INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && + dev_priv->fbc.plane == intel_crtc->plane && + intel_plane->rotation != BIT(DRM_ROTATE_0)) { + intel_crtc->atomic.disable_fbc = true; + } + + if (state->visible) { + /* + * BDW signals flip done immediately if the plane + * is disabled, even if the plane enable is already + * armed to occur at the next vblank :( + */ + if (IS_BROADWELL(dev) && !intel_crtc->primary_enabled) + intel_crtc->atomic.wait_vblank = true; + } + + intel_crtc->atomic.fb_bits |= + INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe); + + intel_crtc->atomic.update_fbc = true; + } + return 0; } @@ -11773,18 +11811,6 @@ intel_commit_primary_plane(struct drm_plane *plane, struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_rect *src = &state->src; - enum pipe pipe = intel_plane->pipe; - - if (!fb) { - /* - * 'prepare' is never called when plane is being disabled, so - * we need to handle frontbuffer tracking here - */ - mutex_lock(&dev->struct_mutex); - i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(&dev->struct_mutex); - } plane->fb = fb; crtc->x = src->x1 >> 16; @@ -11801,26 +11827,7 @@ intel_commit_primary_plane(struct drm_plane *plane, intel_plane->obj = obj; if (intel_crtc->active) { - /* - * FBC does not work on some platforms for rotated - * planes, so disable it when rotation is not 0 and - * update it when rotation is set back to 0. - * - * FIXME: This is redundant with the fbc update done in - * the primary plane enable function except that that - * one is done too late. We eventually need to unify - * this. - */ - if (intel_crtc->primary_enabled && - INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) && - dev_priv->fbc.plane == intel_crtc->plane && - intel_plane->rotation != BIT(DRM_ROTATE_0)) { - intel_fbc_disable(dev); - } - if (state->visible) { - bool was_enabled = intel_crtc->primary_enabled; - /* FIXME: kill this fastboot hack */ intel_update_pipe_size(intel_crtc); @@ -11828,14 +11835,6 @@ intel_commit_primary_plane(struct drm_plane *plane, dev_priv->display.update_primary_plane(crtc, plane->fb, crtc->x, crtc->y); - - /* - * BDW signals flip done immediately if the plane - * is disabled, even if the plane enable is already - * armed to occur at the next vblank :( - */ - if (IS_BROADWELL(dev) && !was_enabled) - intel_wait_for_vblank(dev, intel_crtc->pipe); } else { /* * If clipping results in a non-visible primary plane, @@ -11846,13 +11845,50 @@ intel_commit_primary_plane(struct drm_plane *plane, */ intel_disable_primary_hw_plane(plane, crtc); } + } +} - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe)); +static void intel_begin_crtc_commit(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + if (intel_crtc->atomic.disable_fbc) + intel_fbc_disable(dev); + + if (intel_crtc->atomic.pre_disable_primary) + intel_pre_disable_primary(crtc); + + if (intel_crtc->atomic.update_wm) + intel_update_watermarks(crtc); +} +static void intel_finish_crtc_commit(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_plane *p; + + if (intel_crtc->atomic.wait_vblank) + intel_wait_for_vblank(dev, intel_crtc->pipe); + + intel_frontbuffer_flip(dev, intel_crtc->atomic.fb_bits); + + if (intel_crtc->atomic.update_fbc) { mutex_lock(&dev->struct_mutex); intel_fbc_update(dev); mutex_unlock(&dev->struct_mutex); } + + if (intel_crtc->atomic.post_enable_primary) + intel_post_enable_primary(crtc); + + drm_for_each_legacy_plane(p, &dev->mode_config.plane_list) + if (intel_crtc->atomic.update_sprite_watermarks & drm_plane_index(p)) + intel_update_sprite_watermarks(p, crtc, 0, 0, 0, + false, false); + + memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic)); } int @@ -11864,7 +11900,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, { struct drm_device *dev = plane->dev; struct drm_framebuffer *old_fb = plane->fb; - struct intel_plane_state state; + struct intel_plane_state state = {{ 0 }}; struct intel_plane *intel_plane = to_intel_plane(plane); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int ret; @@ -11902,7 +11938,33 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, return ret; } + if (!state.base.fb) { + unsigned fb_bits = 0; + + switch (plane->type) { + case DRM_PLANE_TYPE_PRIMARY: + fb_bits = INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe); + break; + case DRM_PLANE_TYPE_CURSOR: + fb_bits = INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe); + break; + case DRM_PLANE_TYPE_OVERLAY: + fb_bits = INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe); + break; + } + + /* + * 'prepare' is never called when plane is being disabled, so + * we need to handle frontbuffer tracking here + */ + mutex_lock(&dev->struct_mutex); + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, fb_bits); + mutex_unlock(&dev->struct_mutex); + } + + intel_begin_crtc_commit(crtc); intel_plane->commit_plane(plane, &state); + intel_finish_crtc_commit(crtc); if (fb != old_fb && old_fb) { if (intel_crtc->active) @@ -12009,6 +12071,7 @@ intel_check_cursor_plane(struct drm_plane *plane, struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip; struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int crtc_w, crtc_h; unsigned stride; int ret; @@ -12024,7 +12087,7 @@ intel_check_cursor_plane(struct drm_plane *plane, /* if we want to turn off the cursor ignore width and height */ if (!obj) - return 0; + goto finish; /* Check for which cursor types we support */ crtc_w = drm_rect_width(&state->orig_dst); @@ -12051,6 +12114,16 @@ intel_check_cursor_plane(struct drm_plane *plane, } mutex_unlock(&dev->struct_mutex); +finish: + if (intel_crtc->active) { + if (intel_crtc->cursor_width != + drm_rect_width(&state->orig_dst)) + intel_crtc->atomic.update_wm = true; + + intel_crtc->atomic.fb_bits |= + INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe); + } + return ret; } @@ -12063,9 +12136,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); - enum pipe pipe = intel_crtc->pipe; - unsigned old_width; uint32_t addr; plane->fb = state->base.fb; @@ -12085,17 +12155,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, if (intel_crtc->cursor_bo == obj) goto update; - /* - * 'prepare' is only called when fb != NULL; we still need to update - * frontbuffer tracking for the 'disable' case here. - */ - if (!obj) { - mutex_lock(&dev->struct_mutex); - i915_gem_track_fb(old_obj, NULL, - INTEL_FRONTBUFFER_CURSOR(pipe)); - mutex_unlock(&dev->struct_mutex); - } - if (!obj) addr = 0; else if (!INTEL_INFO(dev)->cursor_needs_physical) @@ -12106,18 +12165,11 @@ intel_commit_cursor_plane(struct drm_plane *plane, intel_crtc->cursor_addr = addr; intel_crtc->cursor_bo = obj; update: - old_width = intel_crtc->cursor_width; - intel_crtc->cursor_width = drm_rect_width(&state->orig_dst); intel_crtc->cursor_height = drm_rect_height(&state->orig_dst); - if (intel_crtc->active) { - if (old_width != intel_crtc->cursor_width) - intel_update_watermarks(crtc); + if (intel_crtc->active) intel_crtc_update_cursor(crtc, state->visible); - - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); - } } static const struct drm_plane_funcs intel_cursor_plane_funcs = { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 588b618..a03bd72 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -251,6 +251,12 @@ struct intel_plane_state { struct drm_rect orig_src; struct drm_rect orig_dst; bool visible; + + /* + * used only for sprite planes to determine when to implicitly + * enable/disable the primary plane + */ + bool hides_primary; }; struct intel_plane_config { @@ -415,6 +421,27 @@ struct skl_pipe_wm { uint32_t linetime; }; +/* + * Tracking of operations that need to be performed at the beginning/end of an + * atomic commit, outside the atomic section where interrupts are disabled. + * These are generally operations that grab mutexes or might otherwise sleep + * and thus can't be run with interrupts disabled. + */ +struct intel_crtc_atomic_commit { + /* Sleepable operations to perform before commit */ + bool wait_for_flips; + bool disable_fbc; + bool pre_disable_primary; + bool update_wm; + + /* Sleepable operations to perform after commit */ + unsigned fb_bits; + bool wait_vblank; + bool update_fbc; + bool post_enable_primary; + unsigned update_sprite_watermarks; +}; + struct intel_crtc { struct drm_crtc base; enum pipe pipe; @@ -468,6 +495,8 @@ struct intel_crtc { int scanline_offset; struct intel_mmio_flip mmio_flip; + + struct intel_crtc_atomic_commit atomic; }; struct intel_plane_wm_parameters { @@ -1213,6 +1242,8 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, bool intel_pipe_update_start(struct intel_crtc *crtc, uint32_t *start_vbl_count); void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); +void intel_post_enable_primary(struct drm_crtc *crtc); +void intel_pre_disable_primary(struct drm_crtc *crtc); /* intel_tv.c */ void intel_tv_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index c18e57d..ff7d6a1 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -771,9 +771,8 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) * Avoid underruns when disabling the sprite. * FIXME remove once watermark updates are done properly. */ - intel_wait_for_vblank(dev, pipe); - - intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false); + intel_crtc->atomic.wait_vblank = true; + intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane)); } static int @@ -976,12 +975,11 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) * Avoid underruns when disabling the sprite. * FIXME remove once watermark updates are done properly. */ - intel_wait_for_vblank(dev, pipe); - - intel_update_sprite_watermarks(plane, crtc, 0, 0, 0, false, false); + intel_crtc->atomic.wait_vblank = true; + intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane)); } -static void +void intel_post_enable_primary(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -1008,7 +1006,7 @@ intel_post_enable_primary(struct drm_crtc *crtc) mutex_unlock(&dev->struct_mutex); } -static void +void intel_pre_disable_primary(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -1113,7 +1111,7 @@ intel_check_sprite_plane(struct drm_plane *plane, if (!fb) { state->visible = false; - return 0; + goto finish; } /* Don't modify another pipe's plane */ @@ -1260,6 +1258,29 @@ intel_check_sprite_plane(struct drm_plane *plane, dst->y1 = crtc_y; dst->y2 = crtc_y + crtc_h; +finish: + /* + * If the sprite is completely covering the primary plane, + * we can disable the primary and save power. + */ + state->hides_primary = drm_rect_equals(dst, clip) && + !colorkey_enabled(intel_plane); + WARN_ON(state->hides_primary && !state->visible && intel_crtc->active); + + if (intel_crtc->active) { + if (intel_crtc->primary_enabled == state->hides_primary) + intel_crtc->atomic.wait_for_flips = true; + + if (intel_crtc->primary_enabled && state->hides_primary) + intel_crtc->atomic.pre_disable_primary = true; + + intel_crtc->atomic.fb_bits |= + INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe); + + if (!intel_crtc->primary_enabled && !state->hides_primary) + intel_crtc->atomic.post_enable_primary = true; + } + return 0; } @@ -1267,37 +1288,14 @@ static void intel_commit_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct drm_device *dev = plane->dev; struct drm_crtc *crtc = state->base.crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane *intel_plane = to_intel_plane(plane); - enum pipe pipe = intel_crtc->pipe; struct drm_framebuffer *fb = state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); int crtc_x, crtc_y; unsigned int crtc_w, crtc_h; uint32_t src_x, src_y, src_w, src_h; - struct drm_rect *dst = &state->dst; - const struct drm_rect *clip = &state->clip; - bool primary_enabled; - - /* - * 'prepare' is never called when plane is being disabled, so we need - * to handle frontbuffer tracking here - */ - if (!fb) { - mutex_lock(&dev->struct_mutex); - i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, - INTEL_FRONTBUFFER_SPRITE(pipe)); - mutex_unlock(&dev->struct_mutex); - } - - /* - * If the sprite is completely covering the primary plane, - * we can disable the primary and save power. - */ - primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane); - WARN_ON(!primary_enabled && !state->visible && intel_crtc->active); intel_plane->crtc_x = state->orig_dst.x1; intel_plane->crtc_y = state->orig_dst.y1; @@ -1310,15 +1308,7 @@ intel_commit_sprite_plane(struct drm_plane *plane, intel_plane->obj = obj; if (intel_crtc->active) { - bool primary_was_enabled = intel_crtc->primary_enabled; - - intel_crtc->primary_enabled = primary_enabled; - - if (primary_was_enabled != primary_enabled) - intel_crtc_wait_for_pending_flips(crtc); - - if (primary_was_enabled && !primary_enabled) - intel_pre_disable_primary(crtc); + intel_crtc->primary_enabled = !state->hides_primary; if (state->visible) { crtc_x = state->dst.x1; @@ -1335,12 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, } else { intel_plane->disable_plane(plane, crtc); } - - - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_SPRITE(pipe)); - - if (!primary_was_enabled && primary_enabled) - intel_post_enable_primary(crtc); } }
Once we integrate our work into the atomic pipeline, plane commit operations will need to happen with interrupts disabled, due to vblank evasion. Our commit functions today include sleepable work, so those operations need to be split out and run either before or after the atomic register programming. The solution here calculates which of those operations will need to be performed during the 'check' phase and sets flags in an intel_crtc sub-struct. New intel_begin_crtc_commit() and intel_finish_crtc_commit() functions are added before and after the actual register programming; these will eventually be called from the atomic plane helper's .atomic_begin() and .atomic_end() entrypoints. v2: Fix broken sprite code split v3: Make the pre/post commit work crtc-based to match how we eventually want this to be called from the atomic plane helpers. v4: Some platforms that haven't had their watermark code reworked were waiting for vblank, then calling update_sprite_watermarks in their platform-specific disable code. These also need to be flagged out of the critical section. v5: Sprite plane test for primary show/hide should just set the flag to wait for pending flips, not actually perform the wait. (Ander) Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 180 ++++++++++++++++++++++------------- drivers/gpu/drm/i915/intel_drv.h | 31 ++++++ drivers/gpu/drm/i915/intel_sprite.c | 78 ++++++--------- 3 files changed, 178 insertions(+), 111 deletions(-)