| Message ID | 1418154823-10819-7-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/09/2014 09:53 PM, Matt Roper wrote: > Now that we have hooks to enable the atomic plane helpers, we can use > the plane helpers for our .update_plane() and .disable_plane() > entrypoints. > > Note that we still need to make a few small behavioral changes to the > driver entrypoints here as we make the transition, due to differences > between how the atomic helpers set things up for us vs how our internal > functions were setting things up. > > v2: Fix commit message typo (Bob) > > v3: Rebased on top of Gustavo Padovan's patches to kill off > intel_crtc_cursor_set_obj() and intel_pipe_set_base(). > > v4: Rebase again on latest display refactor series. Now that previous > patches have smoothed the rough edges of the existing display code, > the transition here is much less painful. > > v5: Plane helpers will re-prepare the same fb for a plane. Drop the > WARN on this condition. I think it would make sense to squash this patch with the previous one. That way it becomes clearer where the intel_update_plane() goes. Also, the atomic.track_fb crtc field is added in the previous patch but only used here. I would also split the change from state.orig_{dst,src} into a separate patch, but I guess that's not really necessary. Cheers, Ander > Testcase: igt/kms_plane > Testcase: igt/kms_universal_plane > Testcase: igt/kms_cursor_crc > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 222 ++++++++++++----------------------- > drivers/gpu/drm/i915/intel_drv.h | 2 - > drivers/gpu/drm/i915/intel_sprite.c | 38 +++--- > 3 files changed, 98 insertions(+), 164 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index ea47923..2f80236 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11678,7 +11678,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, > unsigned frontbuffer_bits = 0; > int ret = 0; > > - if (WARN_ON(fb == plane->fb || !obj)) > + if (!obj) > return 0; > > switch (plane->type) { > @@ -11745,7 +11745,7 @@ intel_check_primary_plane(struct drm_plane *plane, > 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_crtc *intel_crtc; > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_framebuffer *fb = state->base.fb; > struct drm_rect *dest = &state->dst; > @@ -11753,6 +11753,9 @@ intel_check_primary_plane(struct drm_plane *plane, > const struct drm_rect *clip = &state->clip; > int ret; > > + crtc = crtc ? crtc : plane->crtc; > + intel_crtc = to_intel_crtc(crtc); > + > ret = drm_plane_helper_check_update(plane, crtc, fb, > src, dest, clip, > DRM_PLANE_HELPER_NO_SCALING, > @@ -11812,23 +11815,26 @@ intel_commit_primary_plane(struct drm_plane *plane, > struct drm_framebuffer *fb = state->base.fb; > struct drm_device *dev = plane->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc *intel_crtc; > 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; > > + crtc = crtc ? crtc : plane->crtc; > + intel_crtc = to_intel_crtc(crtc); > + > plane->fb = fb; > crtc->x = src->x1 >> 16; > crtc->y = src->y1 >> 16; > > - intel_plane->crtc_x = state->orig_dst.x1; > - intel_plane->crtc_y = state->orig_dst.y1; > - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); > - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); > - intel_plane->src_x = state->orig_src.x1; > - intel_plane->src_y = state->orig_src.y1; > - intel_plane->src_w = drm_rect_width(&state->orig_src); > - intel_plane->src_h = drm_rect_height(&state->orig_src); > + intel_plane->crtc_x = state->base.crtc_x; > + intel_plane->crtc_y = state->base.crtc_y; > + intel_plane->crtc_w = state->base.crtc_w; > + intel_plane->crtc_h = state->base.crtc_h; > + intel_plane->src_x = state->base.src_x; > + intel_plane->src_y = state->base.src_y; > + intel_plane->src_w = state->base.src_w; > + intel_plane->src_h = state->base.src_h; > intel_plane->obj = obj; > > if (intel_crtc->active) { > @@ -11857,6 +11863,32 @@ 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); > + struct intel_plane *intel_plane; > + struct drm_plane *p; > + unsigned fb_bits = 0; > + > + /* Track fb's for any planes being disabled */ > + list_for_each_entry(p, &dev->mode_config.plane_list, head) { > + intel_plane = to_intel_plane(p); > + > + if (intel_crtc->atomic.track_fbs & (1 << drm_plane_index(p))) { > + switch (p->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; > + } > + > + mutex_lock(&dev->struct_mutex); > + i915_gem_track_fb(intel_fb_obj(p->fb), NULL, fb_bits); > + mutex_unlock(&dev->struct_mutex); > + } > + } > > if (intel_crtc->atomic.disable_fbc) > intel_disable_fbc(dev); > @@ -11900,111 +11932,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc) > memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic)); > } > > -int > -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t src_x, uint32_t src_y, > - uint32_t src_w, uint32_t src_h) > -{ > - struct drm_device *dev = plane->dev; > - struct drm_framebuffer *old_fb = plane->fb; > - 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; > - > - state.base.crtc = crtc ? crtc : plane->crtc; > - state.base.fb = fb; > - > - /* sample coordinates in 16.16 fixed point */ > - state.src.x1 = src_x; > - state.src.x2 = src_x + src_w; > - state.src.y1 = src_y; > - state.src.y2 = src_y + src_h; > - > - /* integer pixels */ > - state.dst.x1 = crtc_x; > - state.dst.x2 = crtc_x + crtc_w; > - state.dst.y1 = crtc_y; > - state.dst.y2 = crtc_y + crtc_h; > - > - state.clip.x1 = 0; > - state.clip.y1 = 0; > - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > - > - state.orig_src = state.src; > - state.orig_dst = state.dst; > - > - ret = intel_plane->check_plane(plane, &state); > - if (ret) > - return ret; > - > - if (fb != old_fb && fb) { > - ret = intel_prepare_plane_fb(plane, fb); > - if (ret) > - 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) > - intel_wait_for_vblank(dev, intel_crtc->pipe); > - intel_cleanup_plane_fb(plane, old_fb); > - } > - > - plane->fb = fb; > - > - return 0; > -} > - > -/** > - * intel_disable_plane - disable a plane > - * @plane: plane to disable > - * > - * General disable handler for all plane types. > - */ > -int > -intel_disable_plane(struct drm_plane *plane) > -{ > - if (!plane->fb) > - return 0; > - > - if (WARN_ON(!plane->crtc)) > - return -EINVAL; > - > - return plane->funcs->update_plane(plane, plane->crtc, NULL, > - 0, 0, 0, 0, 0, 0, 0, 0); > -} > - > /* Common destruction function for both primary and cursor planes */ > void intel_plane_destroy(struct drm_plane *plane) > { > @@ -12015,8 +11942,8 @@ void intel_plane_destroy(struct drm_plane *plane) > } > > static const struct drm_plane_funcs intel_primary_plane_funcs = { > - .update_plane = intel_update_plane, > - .disable_plane = intel_disable_plane, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, > @@ -12083,17 +12010,19 @@ intel_check_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > struct drm_crtc *crtc = state->base.crtc; > - struct drm_device *dev = crtc->dev; > + struct drm_device *dev = plane->dev; > struct drm_framebuffer *fb = state->base.fb; > struct drm_rect *dest = &state->dst; > 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; > + struct intel_crtc *intel_crtc; > unsigned stride; > int ret; > > + crtc = crtc ? crtc : plane->crtc; > + intel_crtc = to_intel_crtc(crtc); > + > ret = drm_plane_helper_check_update(plane, crtc, fb, > src, dest, clip, > DRM_PLANE_HELPER_NO_SCALING, > @@ -12108,15 +12037,14 @@ intel_check_cursor_plane(struct drm_plane *plane, > goto finish; > > /* Check for which cursor types we support */ > - crtc_w = drm_rect_width(&state->orig_dst); > - crtc_h = drm_rect_height(&state->orig_dst); > - if (!cursor_size_ok(dev, crtc_w, crtc_h)) { > - DRM_DEBUG("Cursor dimension not supported\n"); > + if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) { > + DRM_DEBUG("Cursor dimension %dx%d not supported\n", > + state->base.crtc_w, state->base.crtc_h); > return -EINVAL; > } > > - stride = roundup_pow_of_two(crtc_w) * 4; > - if (obj->base.size < stride * crtc_h) { > + stride = roundup_pow_of_two(state->base.crtc_w) * 4; > + if (obj->base.size < stride * state->base.crtc_h) { > DRM_DEBUG_KMS("buffer is too small\n"); > return -ENOMEM; > } > @@ -12134,8 +12062,7 @@ intel_check_cursor_plane(struct drm_plane *plane, > > finish: > if (intel_crtc->active) { > - if (intel_crtc->cursor_width != > - drm_rect_width(&state->orig_dst)) > + if (intel_crtc->cursor_width != state->base.crtc_w) > intel_crtc->atomic.update_wm = true; > > intel_crtc->atomic.fb_bits |= > @@ -12150,24 +12077,27 @@ intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > struct drm_crtc *crtc = state->base.crtc; > - struct drm_device *dev = crtc->dev; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_device *dev = plane->dev; > + struct intel_crtc *intel_crtc; > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); > uint32_t addr; > > + crtc = crtc ? crtc : plane->crtc; > + intel_crtc = to_intel_crtc(crtc); > + > plane->fb = state->base.fb; > - crtc->cursor_x = state->orig_dst.x1; > - crtc->cursor_y = state->orig_dst.y1; > - > - intel_plane->crtc_x = state->orig_dst.x1; > - intel_plane->crtc_y = state->orig_dst.y1; > - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); > - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); > - intel_plane->src_x = state->orig_src.x1; > - intel_plane->src_y = state->orig_src.y1; > - intel_plane->src_w = drm_rect_width(&state->orig_src); > - intel_plane->src_h = drm_rect_height(&state->orig_src); > + crtc->cursor_x = state->base.crtc_x; > + crtc->cursor_y = state->base.crtc_y; > + > + intel_plane->crtc_x = state->base.crtc_x; > + intel_plane->crtc_y = state->base.crtc_y; > + intel_plane->crtc_w = state->base.crtc_w; > + intel_plane->crtc_h = state->base.crtc_h; > + intel_plane->src_x = state->base.src_x; > + intel_plane->src_y = state->base.src_y; > + intel_plane->src_w = state->base.src_w; > + intel_plane->src_h = state->base.src_h; > intel_plane->obj = obj; > > if (intel_crtc->cursor_bo == obj) > @@ -12183,16 +12113,16 @@ intel_commit_cursor_plane(struct drm_plane *plane, > intel_crtc->cursor_addr = addr; > intel_crtc->cursor_bo = obj; > update: > - intel_crtc->cursor_width = drm_rect_width(&state->orig_dst); > - intel_crtc->cursor_height = drm_rect_height(&state->orig_dst); > + intel_crtc->cursor_width = state->base.crtc_w; > + intel_crtc->cursor_height = state->base.crtc_h; > > if (intel_crtc->active) > intel_crtc_update_cursor(crtc, state->visible); > } > > static const struct drm_plane_funcs intel_cursor_plane_funcs = { > - .update_plane = intel_update_plane, > - .disable_plane = intel_disable_plane, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index abb80e1..d8a6d6c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -248,8 +248,6 @@ struct intel_plane_state { > struct drm_rect src; > struct drm_rect dst; > struct drm_rect clip; > - struct drm_rect orig_src; > - struct drm_rect orig_dst; > bool visible; > > /* > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 946ff10..7b7acb0 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1064,12 +1064,13 @@ intel_check_sprite_plane(struct drm_plane *plane, > uint32_t src_x, src_y, src_w, src_h; > struct drm_rect *src = &state->src; > struct drm_rect *dst = &state->dst; > - struct drm_rect *orig_src = &state->orig_src; > const struct drm_rect *clip = &state->clip; > int hscale, vscale; > int max_scale, min_scale; > int pixel_size; > > + intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc); > + > if (!fb) { > state->visible = false; > goto finish; > @@ -1150,10 +1151,10 @@ intel_check_sprite_plane(struct drm_plane *plane, > intel_plane->rotation); > > /* sanity check to make sure the src viewport wasn't enlarged */ > - WARN_ON(src->x1 < (int) orig_src->x1 || > - src->y1 < (int) orig_src->y1 || > - src->x2 > (int) orig_src->x2 || > - src->y2 > (int) orig_src->y2); > + WARN_ON(src->x1 < (int) state->base.src_x || > + src->y1 < (int) state->base.src_y || > + src->x2 > (int) state->base.src_x + state->base.src_w || > + src->y2 > (int) state->base.src_y + state->base.src_h); > > /* > * Hardware doesn't handle subpixel coordinates. > @@ -1250,7 +1251,7 @@ intel_commit_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > struct drm_crtc *crtc = state->base.crtc; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc *intel_crtc; > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_framebuffer *fb = state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > @@ -1258,14 +1259,19 @@ intel_commit_sprite_plane(struct drm_plane *plane, > unsigned int crtc_w, crtc_h; > uint32_t src_x, src_y, src_w, src_h; > > - intel_plane->crtc_x = state->orig_dst.x1; > - intel_plane->crtc_y = state->orig_dst.y1; > - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); > - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); > - intel_plane->src_x = state->orig_src.x1; > - intel_plane->src_y = state->orig_src.y1; > - intel_plane->src_w = drm_rect_width(&state->orig_src); > - intel_plane->src_h = drm_rect_height(&state->orig_src); > + intel_plane->crtc_x = state->base.crtc_x; > + intel_plane->crtc_y = state->base.crtc_y; > + intel_plane->crtc_w = state->base.crtc_w; > + intel_plane->crtc_h = state->base.crtc_h; > + intel_plane->src_x = state->base.src_x; > + intel_plane->src_y = state->base.src_y; > + intel_plane->src_w = state->base.src_w; > + intel_plane->src_h = state->base.src_h; > + > + crtc = crtc ? crtc : plane->crtc; > + intel_crtc = to_intel_crtc(crtc); > + > + plane->fb = state->base.fb; > intel_plane->obj = obj; > > if (intel_crtc->active) { > @@ -1389,8 +1395,8 @@ int intel_plane_restore(struct drm_plane *plane) > } > > static const struct drm_plane_funcs intel_plane_funcs = { > - .update_plane = intel_update_plane, > - .disable_plane = intel_disable_plane, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ea47923..2f80236 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11678,7 +11678,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, unsigned frontbuffer_bits = 0; int ret = 0; - if (WARN_ON(fb == plane->fb || !obj)) + if (!obj) return 0; switch (plane->type) { @@ -11745,7 +11745,7 @@ intel_check_primary_plane(struct drm_plane *plane, 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_crtc *intel_crtc; struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_framebuffer *fb = state->base.fb; struct drm_rect *dest = &state->dst; @@ -11753,6 +11753,9 @@ intel_check_primary_plane(struct drm_plane *plane, const struct drm_rect *clip = &state->clip; int ret; + crtc = crtc ? crtc : plane->crtc; + intel_crtc = to_intel_crtc(crtc); + ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, @@ -11812,23 +11815,26 @@ intel_commit_primary_plane(struct drm_plane *plane, struct drm_framebuffer *fb = state->base.fb; struct drm_device *dev = plane->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc *intel_crtc; 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; + crtc = crtc ? crtc : plane->crtc; + intel_crtc = to_intel_crtc(crtc); + plane->fb = fb; crtc->x = src->x1 >> 16; crtc->y = src->y1 >> 16; - intel_plane->crtc_x = state->orig_dst.x1; - intel_plane->crtc_y = state->orig_dst.y1; - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); - intel_plane->src_x = state->orig_src.x1; - intel_plane->src_y = state->orig_src.y1; - intel_plane->src_w = drm_rect_width(&state->orig_src); - intel_plane->src_h = drm_rect_height(&state->orig_src); + intel_plane->crtc_x = state->base.crtc_x; + intel_plane->crtc_y = state->base.crtc_y; + intel_plane->crtc_w = state->base.crtc_w; + intel_plane->crtc_h = state->base.crtc_h; + intel_plane->src_x = state->base.src_x; + intel_plane->src_y = state->base.src_y; + intel_plane->src_w = state->base.src_w; + intel_plane->src_h = state->base.src_h; intel_plane->obj = obj; if (intel_crtc->active) { @@ -11857,6 +11863,32 @@ 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); + struct intel_plane *intel_plane; + struct drm_plane *p; + unsigned fb_bits = 0; + + /* Track fb's for any planes being disabled */ + list_for_each_entry(p, &dev->mode_config.plane_list, head) { + intel_plane = to_intel_plane(p); + + if (intel_crtc->atomic.track_fbs & (1 << drm_plane_index(p))) { + switch (p->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; + } + + mutex_lock(&dev->struct_mutex); + i915_gem_track_fb(intel_fb_obj(p->fb), NULL, fb_bits); + mutex_unlock(&dev->struct_mutex); + } + } if (intel_crtc->atomic.disable_fbc) intel_disable_fbc(dev); @@ -11900,111 +11932,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc) memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic)); } -int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) -{ - struct drm_device *dev = plane->dev; - struct drm_framebuffer *old_fb = plane->fb; - 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; - - state.base.crtc = crtc ? crtc : plane->crtc; - state.base.fb = fb; - - /* sample coordinates in 16.16 fixed point */ - state.src.x1 = src_x; - state.src.x2 = src_x + src_w; - state.src.y1 = src_y; - state.src.y2 = src_y + src_h; - - /* integer pixels */ - state.dst.x1 = crtc_x; - state.dst.x2 = crtc_x + crtc_w; - state.dst.y1 = crtc_y; - state.dst.y2 = crtc_y + crtc_h; - - state.clip.x1 = 0; - state.clip.y1 = 0; - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; - - state.orig_src = state.src; - state.orig_dst = state.dst; - - ret = intel_plane->check_plane(plane, &state); - if (ret) - return ret; - - if (fb != old_fb && fb) { - ret = intel_prepare_plane_fb(plane, fb); - if (ret) - 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) - intel_wait_for_vblank(dev, intel_crtc->pipe); - intel_cleanup_plane_fb(plane, old_fb); - } - - plane->fb = fb; - - return 0; -} - -/** - * intel_disable_plane - disable a plane - * @plane: plane to disable - * - * General disable handler for all plane types. - */ -int -intel_disable_plane(struct drm_plane *plane) -{ - if (!plane->fb) - return 0; - - if (WARN_ON(!plane->crtc)) - return -EINVAL; - - return plane->funcs->update_plane(plane, plane->crtc, NULL, - 0, 0, 0, 0, 0, 0, 0, 0); -} - /* Common destruction function for both primary and cursor planes */ void intel_plane_destroy(struct drm_plane *plane) { @@ -12015,8 +11942,8 @@ void intel_plane_destroy(struct drm_plane *plane) } static const struct drm_plane_funcs intel_primary_plane_funcs = { - .update_plane = intel_update_plane, - .disable_plane = intel_disable_plane, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, @@ -12083,17 +12010,19 @@ intel_check_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_crtc *crtc = state->base.crtc; - struct drm_device *dev = crtc->dev; + struct drm_device *dev = plane->dev; struct drm_framebuffer *fb = state->base.fb; struct drm_rect *dest = &state->dst; 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; + struct intel_crtc *intel_crtc; unsigned stride; int ret; + crtc = crtc ? crtc : plane->crtc; + intel_crtc = to_intel_crtc(crtc); + ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, @@ -12108,15 +12037,14 @@ intel_check_cursor_plane(struct drm_plane *plane, goto finish; /* Check for which cursor types we support */ - crtc_w = drm_rect_width(&state->orig_dst); - crtc_h = drm_rect_height(&state->orig_dst); - if (!cursor_size_ok(dev, crtc_w, crtc_h)) { - DRM_DEBUG("Cursor dimension not supported\n"); + if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) { + DRM_DEBUG("Cursor dimension %dx%d not supported\n", + state->base.crtc_w, state->base.crtc_h); return -EINVAL; } - stride = roundup_pow_of_two(crtc_w) * 4; - if (obj->base.size < stride * crtc_h) { + stride = roundup_pow_of_two(state->base.crtc_w) * 4; + if (obj->base.size < stride * state->base.crtc_h) { DRM_DEBUG_KMS("buffer is too small\n"); return -ENOMEM; } @@ -12134,8 +12062,7 @@ intel_check_cursor_plane(struct drm_plane *plane, finish: if (intel_crtc->active) { - if (intel_crtc->cursor_width != - drm_rect_width(&state->orig_dst)) + if (intel_crtc->cursor_width != state->base.crtc_w) intel_crtc->atomic.update_wm = true; intel_crtc->atomic.fb_bits |= @@ -12150,24 +12077,27 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_crtc *crtc = state->base.crtc; - struct drm_device *dev = crtc->dev; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_device *dev = plane->dev; + struct intel_crtc *intel_crtc; struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); uint32_t addr; + crtc = crtc ? crtc : plane->crtc; + intel_crtc = to_intel_crtc(crtc); + plane->fb = state->base.fb; - crtc->cursor_x = state->orig_dst.x1; - crtc->cursor_y = state->orig_dst.y1; - - intel_plane->crtc_x = state->orig_dst.x1; - intel_plane->crtc_y = state->orig_dst.y1; - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); - intel_plane->src_x = state->orig_src.x1; - intel_plane->src_y = state->orig_src.y1; - intel_plane->src_w = drm_rect_width(&state->orig_src); - intel_plane->src_h = drm_rect_height(&state->orig_src); + crtc->cursor_x = state->base.crtc_x; + crtc->cursor_y = state->base.crtc_y; + + intel_plane->crtc_x = state->base.crtc_x; + intel_plane->crtc_y = state->base.crtc_y; + intel_plane->crtc_w = state->base.crtc_w; + intel_plane->crtc_h = state->base.crtc_h; + intel_plane->src_x = state->base.src_x; + intel_plane->src_y = state->base.src_y; + intel_plane->src_w = state->base.src_w; + intel_plane->src_h = state->base.src_h; intel_plane->obj = obj; if (intel_crtc->cursor_bo == obj) @@ -12183,16 +12113,16 @@ intel_commit_cursor_plane(struct drm_plane *plane, intel_crtc->cursor_addr = addr; intel_crtc->cursor_bo = obj; update: - intel_crtc->cursor_width = drm_rect_width(&state->orig_dst); - intel_crtc->cursor_height = drm_rect_height(&state->orig_dst); + intel_crtc->cursor_width = state->base.crtc_w; + intel_crtc->cursor_height = state->base.crtc_h; if (intel_crtc->active) intel_crtc_update_cursor(crtc, state->visible); } static const struct drm_plane_funcs intel_cursor_plane_funcs = { - .update_plane = intel_update_plane, - .disable_plane = intel_disable_plane, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index abb80e1..d8a6d6c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -248,8 +248,6 @@ struct intel_plane_state { struct drm_rect src; struct drm_rect dst; struct drm_rect clip; - struct drm_rect orig_src; - struct drm_rect orig_dst; bool visible; /* diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 946ff10..7b7acb0 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1064,12 +1064,13 @@ intel_check_sprite_plane(struct drm_plane *plane, uint32_t src_x, src_y, src_w, src_h; struct drm_rect *src = &state->src; struct drm_rect *dst = &state->dst; - struct drm_rect *orig_src = &state->orig_src; const struct drm_rect *clip = &state->clip; int hscale, vscale; int max_scale, min_scale; int pixel_size; + intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc); + if (!fb) { state->visible = false; goto finish; @@ -1150,10 +1151,10 @@ intel_check_sprite_plane(struct drm_plane *plane, intel_plane->rotation); /* sanity check to make sure the src viewport wasn't enlarged */ - WARN_ON(src->x1 < (int) orig_src->x1 || - src->y1 < (int) orig_src->y1 || - src->x2 > (int) orig_src->x2 || - src->y2 > (int) orig_src->y2); + WARN_ON(src->x1 < (int) state->base.src_x || + src->y1 < (int) state->base.src_y || + src->x2 > (int) state->base.src_x + state->base.src_w || + src->y2 > (int) state->base.src_y + state->base.src_h); /* * Hardware doesn't handle subpixel coordinates. @@ -1250,7 +1251,7 @@ intel_commit_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_crtc *crtc = state->base.crtc; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc *intel_crtc; struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_framebuffer *fb = state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); @@ -1258,14 +1259,19 @@ intel_commit_sprite_plane(struct drm_plane *plane, unsigned int crtc_w, crtc_h; uint32_t src_x, src_y, src_w, src_h; - intel_plane->crtc_x = state->orig_dst.x1; - intel_plane->crtc_y = state->orig_dst.y1; - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); - intel_plane->src_x = state->orig_src.x1; - intel_plane->src_y = state->orig_src.y1; - intel_plane->src_w = drm_rect_width(&state->orig_src); - intel_plane->src_h = drm_rect_height(&state->orig_src); + intel_plane->crtc_x = state->base.crtc_x; + intel_plane->crtc_y = state->base.crtc_y; + intel_plane->crtc_w = state->base.crtc_w; + intel_plane->crtc_h = state->base.crtc_h; + intel_plane->src_x = state->base.src_x; + intel_plane->src_y = state->base.src_y; + intel_plane->src_w = state->base.src_w; + intel_plane->src_h = state->base.src_h; + + crtc = crtc ? crtc : plane->crtc; + intel_crtc = to_intel_crtc(crtc); + + plane->fb = state->base.fb; intel_plane->obj = obj; if (intel_crtc->active) { @@ -1389,8 +1395,8 @@ int intel_plane_restore(struct drm_plane *plane) } static const struct drm_plane_funcs intel_plane_funcs = { - .update_plane = intel_update_plane, - .disable_plane = intel_disable_plane, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state,
Now that we have hooks to enable the atomic plane helpers, we can use the plane helpers for our .update_plane() and .disable_plane() entrypoints. Note that we still need to make a few small behavioral changes to the driver entrypoints here as we make the transition, due to differences between how the atomic helpers set things up for us vs how our internal functions were setting things up. v2: Fix commit message typo (Bob) v3: Rebased on top of Gustavo Padovan's patches to kill off intel_crtc_cursor_set_obj() and intel_pipe_set_base(). v4: Rebase again on latest display refactor series. Now that previous patches have smoothed the rough edges of the existing display code, the transition here is much less painful. v5: Plane helpers will re-prepare the same fb for a plane. Drop the WARN on this condition. Testcase: igt/kms_plane Testcase: igt/kms_universal_plane Testcase: igt/kms_cursor_crc Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 222 ++++++++++++----------------------- drivers/gpu/drm/i915/intel_drv.h | 2 - drivers/gpu/drm/i915/intel_sprite.c | 38 +++--- 3 files changed, 98 insertions(+), 164 deletions(-)