| Message ID | 1416858786-25376-7-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/24/2014 09:53 PM, Matt Roper wrote: > The 'prepare' step for all types of planes are pretty similar; > consolidate the three 'prepare' functions into a single function. This > paves the way for future integration with the atomic plane handlers. > > Note that we pull the 'wait for pending flips' functionality out of the > primary plane's prepare step and place it directly in the 'setplane' > code. When we move to the atomic plane handlers, this code will be in > the 'atomic begin' step. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 167 +++++++++++++++++++---------------- > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_sprite.c | 44 ++------- > 3 files changed, 99 insertions(+), 114 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 217d1e3..50d4299 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11574,6 +11574,66 @@ disable_unpin: > return 0; > } > > +/** > + * intel_prepare_plane_fb - Prepare fb for usage on plane > + * @plane: drm plane to prepare for > + * @fb: framebuffer to prepare for presentation > + * > + * Prepares a framebuffer for usage on a display plane. Generally this > + * involves pinning the underlying object and updating the frontbuffer tracking > + * bits. Some older platforms need special physical address handling for > + * cursor planes. > + * > + * Returns 0 on success, negative error code on failure. > + */ > +int > +intel_prepare_plane_fb(struct drm_plane *plane, > + struct drm_framebuffer *fb) > +{ > + struct drm_device *dev = plane->dev; > + struct intel_plane *intel_plane = to_intel_plane(plane); > + enum pipe pipe = intel_plane->pipe; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > + unsigned frontbuffer_bits = 0; > + int ret; > + > + if (WARN_ON(fb == plane->fb || !obj)) > + return 0; > + > + switch (plane->type) { > + case DRM_PLANE_TYPE_PRIMARY: > + frontbuffer_bits = INTEL_FRONTBUFFER_PRIMARY(pipe); > + break; > + case DRM_PLANE_TYPE_CURSOR: > + frontbuffer_bits = INTEL_FRONTBUFFER_CURSOR(pipe); > + break; > + case DRM_PLANE_TYPE_OVERLAY: > + frontbuffer_bits = INTEL_FRONTBUFFER_SPRITE(pipe); > + break; > + } > + > + mutex_lock(&dev->struct_mutex); > + > + if (plane->type == DRM_PLANE_TYPE_CURSOR && > + INTEL_INFO(dev)->cursor_needs_physical) { > + int align = IS_I830(dev) ? 16 * 1024 : 256; > + ret = i915_gem_object_attach_phys(obj, align); > + if (ret) > + DRM_DEBUG_KMS("failed to attach phys object\n"); > + } else { > + ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > + if (ret == 0) > + i915_gem_track_fb(old_obj, obj, frontbuffer_bits); > + else > + DRM_DEBUG_KMS("pin & fence failed\n"); > + } As commented on a previous patch, shouldn't i915_gem_track_fb() be called on the INTEL_INFO(dev)->cursor_needs_physical case too? I can't see anything else wrong with the patch. Ander > + > + mutex_unlock(&dev->struct_mutex); > + > + return ret; > +} > + > static int > intel_check_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -11591,42 +11651,6 @@ intel_check_primary_plane(struct drm_plane *plane, > false, true, &state->visible); > } > > -static int > -intel_prepare_primary_plane(struct drm_plane *plane, > - struct intel_plane_state *state) > -{ > - struct drm_crtc *crtc = state->base.crtc; > - struct drm_framebuffer *fb = state->base.fb; > - struct drm_device *dev = crtc->dev; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > - int ret; > - > - intel_crtc_wait_for_pending_flips(crtc); > - > - if (intel_crtc_has_pending_flip(crtc)) { > - DRM_ERROR("pipe is still busy with an old pageflip\n"); > - return -EBUSY; > - } > - > - if (old_obj != obj) { > - mutex_lock(&dev->struct_mutex); > - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(old_obj, obj, > - INTEL_FRONTBUFFER_PRIMARY(pipe)); > - mutex_unlock(&dev->struct_mutex); > - if (ret != 0) { > - DRM_DEBUG_KMS("pin & fence failed\n"); > - return ret; > - } > - } > - > - return 0; > -} > - > static void > intel_commit_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -11728,6 +11752,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > uint32_t src_x, uint32_t src_y, > uint32_t src_w, uint32_t src_h) > { > + struct drm_framebuffer *old_fb = plane->fb; > struct intel_plane_state state; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int ret; > @@ -11759,9 +11784,18 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > if (ret) > return ret; > > - ret = intel_prepare_primary_plane(plane, &state); > - if (ret) > - return ret; > + intel_crtc_wait_for_pending_flips(crtc); > + > + if (intel_crtc_has_pending_flip(crtc)) { > + DRM_ERROR("pipe is still busy with an old pageflip\n"); > + return -EBUSY; > + } > + > + if (fb != old_fb && fb) { > + ret = intel_prepare_plane_fb(plane, fb); > + if (ret) > + return ret; > + } > > intel_commit_primary_plane(plane, &state); > > @@ -11898,40 +11932,6 @@ intel_check_cursor_plane(struct drm_plane *plane, > return ret; > } > > -static int > -intel_prepare_cursor_plane(struct drm_plane *plane, > - struct intel_plane_state *state) > -{ > - struct drm_device *dev = plane->dev; > - struct drm_framebuffer *fb = state->base.fb; > - struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > - enum pipe pipe = intel_crtc->pipe; > - int ret = 0; > - > - if (old_obj != obj) { > - /* we only need to pin inside GTT if cursor is non-phy */ > - mutex_lock(&dev->struct_mutex); > - if (!INTEL_INFO(dev)->cursor_needs_physical) { > - if (obj) > - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(intel_crtc->cursor_bo, obj, > - INTEL_FRONTBUFFER_CURSOR(pipe)); > - } else { > - int align = IS_I830(dev) ? 16 * 1024 : 256; > - if (obj) > - ret = i915_gem_object_attach_phys(obj, align); > - if (ret) > - DRM_DEBUG_KMS("failed to attach phys object\n"); > - } > - mutex_unlock(&dev->struct_mutex); > - } > - > - return ret; > -} > - > static void > intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -11941,6 +11941,7 @@ 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; > @@ -11961,6 +11962,17 @@ 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) > @@ -11997,6 +12009,7 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, > uint32_t src_x, uint32_t src_y, > uint32_t src_w, uint32_t src_h) > { > + struct drm_framebuffer *old_fb = plane->fb; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane_state state; > int ret; > @@ -12028,9 +12041,11 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, > if (ret) > return ret; > > - ret = intel_prepare_cursor_plane(plane, &state); > - if (ret) > - return ret; > + if (fb != old_fb && fb) { > + ret = intel_prepare_plane_fb(plane, fb); > + if (ret) > + return ret; > + } > > intel_commit_cursor_plane(plane, &state); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a6eafb4..e9f4dc1 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -924,6 +924,8 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > void intel_finish_page_flip(struct drm_device *dev, int pipe); > void intel_finish_page_flip_plane(struct drm_device *dev, int plane); > void intel_check_page_flip(struct drm_device *dev, int pipe); > +int intel_prepare_plane_fb(struct drm_plane *plane, > + struct drm_framebuffer *fb); > > /* shared dpll functions */ > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index fc96d13..5d8c2e0 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1257,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane, > return 0; > } > > -static int > -intel_prepare_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); > - struct drm_i915_gem_object *old_obj = intel_plane->obj; > - int ret; > - > - if (old_obj != obj) { > - mutex_lock(&dev->struct_mutex); > - > - /* Note that this will apply the VT-d workaround for scanouts, > - * which is more restrictive than required for sprites. (The > - * primary plane requires 256KiB alignment with 64 PTE padding, > - * the sprite planes only require 128KiB alignment and 32 PTE > - * padding. > - */ > - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(old_obj, obj, > - INTEL_FRONTBUFFER_SPRITE(pipe)); > - mutex_unlock(&dev->struct_mutex); > - if (ret) > - return ret; > - } > - > - return 0; > -} > - > static void > intel_commit_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -1387,6 +1352,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > uint32_t src_x, uint32_t src_y, > uint32_t src_w, uint32_t src_h) > { > + struct drm_framebuffer *old_fb = plane->fb; > struct intel_plane_state state; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int ret; > @@ -1417,9 +1383,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > if (ret) > return ret; > > - ret = intel_prepare_sprite_plane(plane, &state); > - if (ret) > - return ret; > + if (fb != old_fb && fb) { > + ret = intel_prepare_plane_fb(plane, fb); > + if (ret) > + return ret; > + } > > intel_commit_sprite_plane(plane, &state); > return 0; >
On 11/24/2014 09:53 PM, Matt Roper wrote: > The 'prepare' step for all types of planes are pretty similar; > consolidate the three 'prepare' functions into a single function. This > paves the way for future integration with the atomic plane handlers. > > Note that we pull the 'wait for pending flips' functionality out of the > primary plane's prepare step and place it directly in the 'setplane' > code. When we move to the atomic plane handlers, this code will be in > the 'atomic begin' step. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 167 +++++++++++++++++++---------------- > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_sprite.c | 44 ++------- > 3 files changed, 99 insertions(+), 114 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 217d1e3..50d4299 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11574,6 +11574,66 @@ disable_unpin: > return 0; > } > > +/** > + * intel_prepare_plane_fb - Prepare fb for usage on plane > + * @plane: drm plane to prepare for > + * @fb: framebuffer to prepare for presentation > + * > + * Prepares a framebuffer for usage on a display plane. Generally this > + * involves pinning the underlying object and updating the frontbuffer tracking > + * bits. Some older platforms need special physical address handling for > + * cursor planes. > + * > + * Returns 0 on success, negative error code on failure. > + */ > +int > +intel_prepare_plane_fb(struct drm_plane *plane, > + struct drm_framebuffer *fb) > +{ > + struct drm_device *dev = plane->dev; > + struct intel_plane *intel_plane = to_intel_plane(plane); > + enum pipe pipe = intel_plane->pipe; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); [...] > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index fc96d13..5d8c2e0 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1257,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane, > return 0; > } > > -static int > -intel_prepare_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); > - struct drm_i915_gem_object *old_obj = intel_plane->obj; This used to look at intel_plane->obj, but the new unified prepare function uses the value of intel_plane->base.fb, which is not updated in intel_commit_sprite_plane(). Ander
On Mon, Dec 01, 2014 at 10:29:04AM +0200, Ander Conselvan de Oliveira wrote: > On 11/24/2014 09:53 PM, Matt Roper wrote: ... > >-static int > >-intel_prepare_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); > >- struct drm_i915_gem_object *old_obj = intel_plane->obj; > > This used to look at intel_plane->obj, but the new unified prepare > function uses the value of intel_plane->base.fb, which is not > updated in intel_commit_sprite_plane(). > > Ander I think this should be okay (same for the cursor case you noted in a previous patch) because the DRM core updates drm_plane->fb for us in __setplane_internal() after the driver's update handler succeeds. Updating drm_plane->fb inside the driver's commit function should only be necessary if we then use that new value before returning back to the DRM core. In the case you note above, we're looking at the value which was updated by the DRM core following the previous sprite update. Matt
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 217d1e3..50d4299 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11574,6 +11574,66 @@ disable_unpin: return 0; } +/** + * intel_prepare_plane_fb - Prepare fb for usage on plane + * @plane: drm plane to prepare for + * @fb: framebuffer to prepare for presentation + * + * Prepares a framebuffer for usage on a display plane. Generally this + * involves pinning the underlying object and updating the frontbuffer tracking + * bits. Some older platforms need special physical address handling for + * cursor planes. + * + * Returns 0 on success, negative error code on failure. + */ +int +intel_prepare_plane_fb(struct drm_plane *plane, + struct drm_framebuffer *fb) +{ + struct drm_device *dev = plane->dev; + struct intel_plane *intel_plane = to_intel_plane(plane); + enum pipe pipe = intel_plane->pipe; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); + unsigned frontbuffer_bits = 0; + int ret; + + if (WARN_ON(fb == plane->fb || !obj)) + return 0; + + switch (plane->type) { + case DRM_PLANE_TYPE_PRIMARY: + frontbuffer_bits = INTEL_FRONTBUFFER_PRIMARY(pipe); + break; + case DRM_PLANE_TYPE_CURSOR: + frontbuffer_bits = INTEL_FRONTBUFFER_CURSOR(pipe); + break; + case DRM_PLANE_TYPE_OVERLAY: + frontbuffer_bits = INTEL_FRONTBUFFER_SPRITE(pipe); + break; + } + + mutex_lock(&dev->struct_mutex); + + if (plane->type == DRM_PLANE_TYPE_CURSOR && + INTEL_INFO(dev)->cursor_needs_physical) { + int align = IS_I830(dev) ? 16 * 1024 : 256; + ret = i915_gem_object_attach_phys(obj, align); + if (ret) + DRM_DEBUG_KMS("failed to attach phys object\n"); + } else { + ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); + if (ret == 0) + i915_gem_track_fb(old_obj, obj, frontbuffer_bits); + else + DRM_DEBUG_KMS("pin & fence failed\n"); + } + + mutex_unlock(&dev->struct_mutex); + + return ret; +} + static int intel_check_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -11591,42 +11651,6 @@ intel_check_primary_plane(struct drm_plane *plane, false, true, &state->visible); } -static int -intel_prepare_primary_plane(struct drm_plane *plane, - struct intel_plane_state *state) -{ - struct drm_crtc *crtc = state->base.crtc; - struct drm_framebuffer *fb = state->base.fb; - struct drm_device *dev = crtc->dev; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); - int ret; - - intel_crtc_wait_for_pending_flips(crtc); - - if (intel_crtc_has_pending_flip(crtc)) { - DRM_ERROR("pipe is still busy with an old pageflip\n"); - return -EBUSY; - } - - if (old_obj != obj) { - mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret != 0) { - DRM_DEBUG_KMS("pin & fence failed\n"); - return ret; - } - } - - return 0; -} - static void intel_commit_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -11728,6 +11752,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h) { + struct drm_framebuffer *old_fb = plane->fb; struct intel_plane_state state; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int ret; @@ -11759,9 +11784,18 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret; - ret = intel_prepare_primary_plane(plane, &state); - if (ret) - return ret; + intel_crtc_wait_for_pending_flips(crtc); + + if (intel_crtc_has_pending_flip(crtc)) { + DRM_ERROR("pipe is still busy with an old pageflip\n"); + return -EBUSY; + } + + if (fb != old_fb && fb) { + ret = intel_prepare_plane_fb(plane, fb); + if (ret) + return ret; + } intel_commit_primary_plane(plane, &state); @@ -11898,40 +11932,6 @@ intel_check_cursor_plane(struct drm_plane *plane, return ret; } -static int -intel_prepare_cursor_plane(struct drm_plane *plane, - struct intel_plane_state *state) -{ - struct drm_device *dev = plane->dev; - struct drm_framebuffer *fb = state->base.fb; - struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); - enum pipe pipe = intel_crtc->pipe; - int ret = 0; - - if (old_obj != obj) { - /* we only need to pin inside GTT if cursor is non-phy */ - mutex_lock(&dev->struct_mutex); - if (!INTEL_INFO(dev)->cursor_needs_physical) { - if (obj) - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); - if (ret == 0) - i915_gem_track_fb(intel_crtc->cursor_bo, obj, - INTEL_FRONTBUFFER_CURSOR(pipe)); - } else { - int align = IS_I830(dev) ? 16 * 1024 : 256; - if (obj) - ret = i915_gem_object_attach_phys(obj, align); - if (ret) - DRM_DEBUG_KMS("failed to attach phys object\n"); - } - mutex_unlock(&dev->struct_mutex); - } - - return ret; -} - static void intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -11941,6 +11941,7 @@ 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; @@ -11961,6 +11962,17 @@ 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) @@ -11997,6 +12009,7 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h) { + struct drm_framebuffer *old_fb = plane->fb; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane_state state; int ret; @@ -12028,9 +12041,11 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret; - ret = intel_prepare_cursor_plane(plane, &state); - if (ret) - return ret; + if (fb != old_fb && fb) { + ret = intel_prepare_plane_fb(plane, fb); + if (ret) + return ret; + } intel_commit_cursor_plane(plane, &state); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a6eafb4..e9f4dc1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -924,6 +924,8 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); void intel_finish_page_flip(struct drm_device *dev, int pipe); void intel_finish_page_flip_plane(struct drm_device *dev, int plane); void intel_check_page_flip(struct drm_device *dev, int pipe); +int intel_prepare_plane_fb(struct drm_plane *plane, + struct drm_framebuffer *fb); /* shared dpll functions */ struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index fc96d13..5d8c2e0 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1257,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane, return 0; } -static int -intel_prepare_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); - struct drm_i915_gem_object *old_obj = intel_plane->obj; - int ret; - - if (old_obj != obj) { - mutex_lock(&dev->struct_mutex); - - /* Note that this will apply the VT-d workaround for scanouts, - * which is more restrictive than required for sprites. (The - * primary plane requires 256KiB alignment with 64 PTE padding, - * the sprite planes only require 128KiB alignment and 32 PTE - * padding. - */ - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_SPRITE(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret) - return ret; - } - - return 0; -} - static void intel_commit_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -1387,6 +1352,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h) { + struct drm_framebuffer *old_fb = plane->fb; struct intel_plane_state state; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int ret; @@ -1417,9 +1383,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret; - ret = intel_prepare_sprite_plane(plane, &state); - if (ret) - return ret; + if (fb != old_fb && fb) { + ret = intel_prepare_plane_fb(plane, fb); + if (ret) + return ret; + } intel_commit_sprite_plane(plane, &state); return 0;
The 'prepare' step for all types of planes are pretty similar; consolidate the three 'prepare' functions into a single function. This paves the way for future integration with the atomic plane handlers. Note that we pull the 'wait for pending flips' functionality out of the primary plane's prepare step and place it directly in the 'setplane' code. When we move to the atomic plane handlers, this code will be in the 'atomic begin' step. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 167 +++++++++++++++++++---------------- drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_sprite.c | 44 ++------- 3 files changed, 99 insertions(+), 114 deletions(-)