| Message ID | 1481204729-9058-7-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8 December 2016 at 13:45, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > Do something similar to vc4, only allow updating the cursor state > in-place through a fastpath when the watermarks are unaffected. This > will allow cursor movement to be smooth, but changing cursor size or > showing/hiding cursor will still fall back so watermarks can be updated. > > Only moving and changing fb is allowed. > > Changes since v1: > - Set page flip to always_unused for trybot. > - Copy fence correctly, ignore plane_state->state, should be NULL. > - Check crtc_state for !active and modeset, go to slowpath if the case. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 47 +++++++----- > drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 3 files changed, 153 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index dbe9fb41ae53..60d75ce8a989 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -103,36 +103,24 @@ intel_plane_destroy_state(struct drm_plane *plane, > drm_atomic_helper_plane_destroy_state(plane, state); > } > > -static int intel_plane_atomic_check(struct drm_plane *plane, > - struct drm_plane_state *state) > +int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state, > + struct intel_plane_state *intel_state) > { > + struct drm_plane *plane = intel_state->base.plane; > struct drm_i915_private *dev_priv = to_i915(plane->dev); > - struct drm_crtc *crtc = state->crtc; > - struct intel_crtc *intel_crtc; > - struct intel_crtc_state *crtc_state; > + struct drm_plane_state *state = &intel_state->base; > struct intel_plane *intel_plane = to_intel_plane(plane); > - struct intel_plane_state *intel_state = to_intel_plane_state(state); > - struct drm_crtc_state *drm_crtc_state; > int ret; > > - crtc = crtc ? crtc : plane->state->crtc; > - intel_crtc = to_intel_crtc(crtc); > - > /* > * Both crtc and plane->crtc could be NULL if we're updating a > * property while the plane is disabled. We don't actually have > * anything driver-specific we need to test in that case, so > * just return success. > */ > - if (!crtc) > + if (!intel_state->base.crtc && !plane->state->crtc) > return 0; > > - drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); > - if (WARN_ON(!drm_crtc_state)) > - return -EINVAL; > - > - crtc_state = to_intel_crtc_state(drm_crtc_state); > - > /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ > intel_state->clip.x1 = 0; > intel_state->clip.y1 = 0; > @@ -184,6 +172,31 @@ static int intel_plane_atomic_check(struct drm_plane *plane, > return intel_plane_atomic_calc_changes(&crtc_state->base, state); > } > > +static int intel_plane_atomic_check(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + struct drm_crtc *crtc = state->crtc; > + struct drm_crtc_state *drm_crtc_state; > + > + crtc = crtc ? crtc : plane->state->crtc; > + > + /* > + * Both crtc and plane->crtc could be NULL if we're updating a > + * property while the plane is disabled. We don't actually have > + * anything driver-specific we need to test in that case, so > + * just return success. > + */ > + if (!crtc) > + return 0; > + > + drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); > + if (WARN_ON(!drm_crtc_state)) > + return -EINVAL; > + > + return intel_plane_atomic_check_with_state(to_intel_crtc_state(drm_crtc_state), > + to_intel_plane_state(state)); > +} > + > static void intel_plane_atomic_update(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 9eaf1e5bdae9..a8b8663ecda7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15028,6 +15029,124 @@ const struct drm_plane_funcs intel_plane_funcs = { > .atomic_destroy_state = intel_plane_destroy_state, > }; > > +static int > +intel_legacy_cursor_update(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_i915_private *dev_priv = to_i915(crtc->dev); > + int ret; > + struct drm_plane_state *old_plane_state, *new_plane_state; > + struct intel_plane *intel_plane = to_intel_plane(plane); > + struct drm_framebuffer *old_fb; > + struct drm_crtc_state *crtc_state = crtc->state; > + > + if (!crtc_state->active || needs_modeset(crtc_state) || > + to_intel_crtc_state(crtc_state)->update_pipe) > + goto slow; > + > + old_plane_state = plane->state; > + > + if (old_plane_state->crtc != crtc || > + old_plane_state->src_w != src_w || > + old_plane_state->src_h != src_h || > + old_plane_state->crtc_w != crtc_w || > + old_plane_state->crtc_h != crtc_h || > + !old_plane_state->visible || > + old_plane_state->fb->modifier != fb->modifier) > + goto slow; > + > + new_plane_state = intel_plane_duplicate_state(plane); > + if (!new_plane_state) > + return -ENOMEM; > + > + drm_atomic_set_fb_for_plane(new_plane_state, fb); > + > + new_plane_state->src_x = src_x; > + new_plane_state->src_y = src_y; > + new_plane_state->src_w = src_w; > + new_plane_state->src_h = src_h; > + new_plane_state->crtc_x = crtc_x; > + new_plane_state->crtc_y = crtc_y; > + new_plane_state->crtc_w = crtc_w; > + new_plane_state->crtc_h = crtc_h; > + > + ret = intel_plane_atomic_check_with_state(to_intel_crtc_state(crtc->state), > + to_intel_plane_state(new_plane_state)); > + if (ret) > + return ret; > + > + if (!new_plane_state->visible) { > + intel_plane_destroy_state(plane, new_plane_state); > + goto slow; > + } > + > + ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex); > + if (ret) > + return ret; > + > + if (INTEL_INFO(dev_priv)->cursor_needs_physical) { > + int align = IS_I830(dev_priv) ? 16 * 1024 : 256; > + > + ret = i915_gem_object_attach_phys(intel_fb_obj(fb), align); > + if (ret) { > + DRM_DEBUG_KMS("failed to attach phys object\n"); > + return ret; Drive-by-comment. Shouldn't there be some clean up in both error paths, like mutex_unlock ? > + } > + } else { > + struct i915_vma *vma; > + > + vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation); > + if (IS_ERR(vma)) { > + DRM_DEBUG_KMS("failed to pin object\n"); > + > + return PTR_ERR(vma); > + } > + } > + > + old_fb = old_plane_state->fb; > + > + i915_gem_track_fb(intel_fb_obj(old_fb), intel_fb_obj(fb), > + intel_plane->frontbuffer_bit); > + > + /* Swap plane state */ > + new_plane_state->fence = old_plane_state->fence; > + *to_intel_plane_state(old_plane_state) = *to_intel_plane_state(new_plane_state); > + new_plane_state->fence = NULL; > + new_plane_state->fb = old_fb; > + > + intel_plane->update_plane(plane, > + to_intel_crtc_state(crtc->state), > + to_intel_plane_state(plane->state)); > + > + intel_cleanup_plane_fb(plane, new_plane_state); > + mutex_unlock(&dev_priv->drm.struct_mutex); > + > + intel_plane_destroy_state(plane, new_plane_state); > + > + return 0; > + > +slow: > + return drm_atomic_helper_update_plane(plane, crtc, fb, > + crtc_x, crtc_y, crtc_w, crtc_h, > + src_x, src_y, src_w, src_h); > +} > + > +static const struct drm_plane_funcs intel_cursor_plane_funcs = { > + .update_plane = intel_legacy_cursor_update, > + .disable_plane = drm_atomic_helper_disable_plane, > + .destroy = intel_plane_destroy, > + .set_property = drm_atomic_helper_plane_set_property, > + .atomic_get_property = intel_plane_atomic_get_property, > + .atomic_set_property = intel_plane_atomic_set_property, > + .atomic_duplicate_state = intel_plane_duplicate_state, > + .atomic_destroy_state = intel_plane_destroy_state, > +}; > + > static struct intel_plane * > intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) > { > @@ -15274,7 +15393,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) > cursor->disable_plane = intel_disable_cursor_plane; > > ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base, > - 0, &intel_plane_funcs, > + 0, &intel_cursor_plane_funcs, > intel_cursor_formats, > ARRAY_SIZE(intel_cursor_formats), > DRM_PLANE_TYPE_CURSOR, > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 8f4ddca0f521..8b6f39506d45 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1844,6 +1844,8 @@ struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); > void intel_plane_destroy_state(struct drm_plane *plane, > struct drm_plane_state *state); > extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; > +int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state, > + struct intel_plane_state *intel_state); > > /* intel_color.c */ > void intel_color_init(struct drm_crtc *crtc); > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index dbe9fb41ae53..60d75ce8a989 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -103,36 +103,24 @@ intel_plane_destroy_state(struct drm_plane *plane, drm_atomic_helper_plane_destroy_state(plane, state); } -static int intel_plane_atomic_check(struct drm_plane *plane, - struct drm_plane_state *state) +int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state, + struct intel_plane_state *intel_state) { + struct drm_plane *plane = intel_state->base.plane; struct drm_i915_private *dev_priv = to_i915(plane->dev); - struct drm_crtc *crtc = state->crtc; - struct intel_crtc *intel_crtc; - struct intel_crtc_state *crtc_state; + struct drm_plane_state *state = &intel_state->base; struct intel_plane *intel_plane = to_intel_plane(plane); - struct intel_plane_state *intel_state = to_intel_plane_state(state); - struct drm_crtc_state *drm_crtc_state; int ret; - crtc = crtc ? crtc : plane->state->crtc; - intel_crtc = to_intel_crtc(crtc); - /* * Both crtc and plane->crtc could be NULL if we're updating a * property while the plane is disabled. We don't actually have * anything driver-specific we need to test in that case, so * just return success. */ - if (!crtc) + if (!intel_state->base.crtc && !plane->state->crtc) return 0; - drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); - if (WARN_ON(!drm_crtc_state)) - return -EINVAL; - - crtc_state = to_intel_crtc_state(drm_crtc_state); - /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ intel_state->clip.x1 = 0; intel_state->clip.y1 = 0; @@ -184,6 +172,31 @@ static int intel_plane_atomic_check(struct drm_plane *plane, return intel_plane_atomic_calc_changes(&crtc_state->base, state); } +static int intel_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct drm_crtc *crtc = state->crtc; + struct drm_crtc_state *drm_crtc_state; + + crtc = crtc ? crtc : plane->state->crtc; + + /* + * Both crtc and plane->crtc could be NULL if we're updating a + * property while the plane is disabled. We don't actually have + * anything driver-specific we need to test in that case, so + * just return success. + */ + if (!crtc) + return 0; + + drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); + if (WARN_ON(!drm_crtc_state)) + return -EINVAL; + + return intel_plane_atomic_check_with_state(to_intel_crtc_state(drm_crtc_state), + to_intel_plane_state(state)); +} + static void intel_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9eaf1e5bdae9..a8b8663ecda7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15028,6 +15029,124 @@ const struct drm_plane_funcs intel_plane_funcs = { .atomic_destroy_state = intel_plane_destroy_state, }; +static int +intel_legacy_cursor_update(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_i915_private *dev_priv = to_i915(crtc->dev); + int ret; + struct drm_plane_state *old_plane_state, *new_plane_state; + struct intel_plane *intel_plane = to_intel_plane(plane); + struct drm_framebuffer *old_fb; + struct drm_crtc_state *crtc_state = crtc->state; + + if (!crtc_state->active || needs_modeset(crtc_state) || + to_intel_crtc_state(crtc_state)->update_pipe) + goto slow; + + old_plane_state = plane->state; + + if (old_plane_state->crtc != crtc || + old_plane_state->src_w != src_w || + old_plane_state->src_h != src_h || + old_plane_state->crtc_w != crtc_w || + old_plane_state->crtc_h != crtc_h || + !old_plane_state->visible || + old_plane_state->fb->modifier != fb->modifier) + goto slow; + + new_plane_state = intel_plane_duplicate_state(plane); + if (!new_plane_state) + return -ENOMEM; + + drm_atomic_set_fb_for_plane(new_plane_state, fb); + + new_plane_state->src_x = src_x; + new_plane_state->src_y = src_y; + new_plane_state->src_w = src_w; + new_plane_state->src_h = src_h; + new_plane_state->crtc_x = crtc_x; + new_plane_state->crtc_y = crtc_y; + new_plane_state->crtc_w = crtc_w; + new_plane_state->crtc_h = crtc_h; + + ret = intel_plane_atomic_check_with_state(to_intel_crtc_state(crtc->state), + to_intel_plane_state(new_plane_state)); + if (ret) + return ret; + + if (!new_plane_state->visible) { + intel_plane_destroy_state(plane, new_plane_state); + goto slow; + } + + ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex); + if (ret) + return ret; + + if (INTEL_INFO(dev_priv)->cursor_needs_physical) { + int align = IS_I830(dev_priv) ? 16 * 1024 : 256; + + ret = i915_gem_object_attach_phys(intel_fb_obj(fb), align); + if (ret) { + DRM_DEBUG_KMS("failed to attach phys object\n"); + return ret; + } + } else { + struct i915_vma *vma; + + vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation); + if (IS_ERR(vma)) { + DRM_DEBUG_KMS("failed to pin object\n"); + + return PTR_ERR(vma); + } + } + + old_fb = old_plane_state->fb; + + i915_gem_track_fb(intel_fb_obj(old_fb), intel_fb_obj(fb), + intel_plane->frontbuffer_bit); + + /* Swap plane state */ + new_plane_state->fence = old_plane_state->fence; + *to_intel_plane_state(old_plane_state) = *to_intel_plane_state(new_plane_state); + new_plane_state->fence = NULL; + new_plane_state->fb = old_fb; + + intel_plane->update_plane(plane, + to_intel_crtc_state(crtc->state), + to_intel_plane_state(plane->state)); + + intel_cleanup_plane_fb(plane, new_plane_state); + mutex_unlock(&dev_priv->drm.struct_mutex); + + intel_plane_destroy_state(plane, new_plane_state); + + return 0; + +slow: + return drm_atomic_helper_update_plane(plane, crtc, fb, + crtc_x, crtc_y, crtc_w, crtc_h, + src_x, src_y, src_w, src_h); +} + +static const struct drm_plane_funcs intel_cursor_plane_funcs = { + .update_plane = intel_legacy_cursor_update, + .disable_plane = drm_atomic_helper_disable_plane, + .destroy = intel_plane_destroy, + .set_property = drm_atomic_helper_plane_set_property, + .atomic_get_property = intel_plane_atomic_get_property, + .atomic_set_property = intel_plane_atomic_set_property, + .atomic_duplicate_state = intel_plane_duplicate_state, + .atomic_destroy_state = intel_plane_destroy_state, +}; + static struct intel_plane * intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) { @@ -15274,7 +15393,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) cursor->disable_plane = intel_disable_cursor_plane; ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base, - 0, &intel_plane_funcs, + 0, &intel_cursor_plane_funcs, intel_cursor_formats, ARRAY_SIZE(intel_cursor_formats), DRM_PLANE_TYPE_CURSOR, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8f4ddca0f521..8b6f39506d45 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1844,6 +1844,8 @@ struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); void intel_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; +int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state, + struct intel_plane_state *intel_state); /* intel_color.c */ void intel_color_init(struct drm_crtc *crtc);