| Message ID | 1414158695-31605-5-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 24, 2014 at 02:51:35PM +0100, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > Merge it into the plane update_plane() callback and make other > users use the update_plane() functions instead. > > The fb != crtc->cursor->fb was already inside intel_crtc_cursor_set_obj() > so we fold intel_crtc_cursor_set_obj() inside intel_commit_cursor_plane() > and merge both paths into one. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > --- > drivers/gpu/drm/i915/intel_display.c | 215 ++++++++++++++++------------------- > 1 file changed, 100 insertions(+), 115 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 9a913f5..60ec165 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8453,109 +8453,6 @@ static bool cursor_size_ok(struct drm_device *dev, > return true; > } > > -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > - struct drm_i915_gem_object *obj, > - uint32_t width, uint32_t height) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > - unsigned old_width; > - uint32_t addr; > - int ret; > - > - /* if we want to turn off the cursor ignore width and height */ > - if (!obj) { > - DRM_DEBUG_KMS("cursor off\n"); > - addr = 0; > - mutex_lock(&dev->struct_mutex); > - goto finish; > - } > - > - /* we only need to pin inside GTT if cursor is non-phy */ > - mutex_lock(&dev->struct_mutex); > - if (!INTEL_INFO(dev)->cursor_needs_physical) { > - unsigned alignment; > - > - /* > - * Global gtt pte registers are special registers which actually > - * forward writes to a chunk of system memory. Which means that > - * there is no risk that the register values disappear as soon > - * as we call intel_runtime_pm_put(), so it is correct to wrap > - * only the pin/unpin/fence and not more. > - */ > - intel_runtime_pm_get(dev_priv); > - > - /* Note that the w/a also requires 2 PTE of padding following > - * the bo. We currently fill all unused PTE with the shadow > - * page and so we should always have valid PTE following the > - * cursor preventing the VT-d warning. > - */ > - alignment = 0; > - if (need_vtd_wa(dev)) > - alignment = 64*1024; > - > - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > - if (ret) { > - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > - intel_runtime_pm_put(dev_priv); > - goto fail_locked; > - } > - > - ret = i915_gem_object_put_fence(obj); > - if (ret) { > - DRM_DEBUG_KMS("failed to release fence for cursor"); > - intel_runtime_pm_put(dev_priv); > - goto fail_unpin; > - } > - > - addr = i915_gem_obj_ggtt_offset(obj); > - > - intel_runtime_pm_put(dev_priv); > - } else { > - 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"); > - goto fail_locked; > - } > - addr = obj->phys_handle->busaddr; > - } > - > - finish: > - if (intel_crtc->cursor_bo) { > - if (!INTEL_INFO(dev)->cursor_needs_physical) > - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > - } > - > - i915_gem_track_fb(intel_crtc->cursor_bo, obj, > - INTEL_FRONTBUFFER_CURSOR(pipe)); > - mutex_unlock(&dev->struct_mutex); > - > - old_width = intel_crtc->cursor_width; > - > - intel_crtc->cursor_addr = addr; > - intel_crtc->cursor_bo = obj; > - intel_crtc->cursor_width = width; > - intel_crtc->cursor_height = height; > - > - if (intel_crtc->active) { > - if (old_width != width) > - intel_update_watermarks(crtc); > - intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL); > - > - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > - } > - > - return 0; > -fail_unpin: > - i915_gem_object_unpin_from_display_plane(obj); > -fail_locked: > - mutex_unlock(&dev->struct_mutex); > - return ret; > -} > - > static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > u16 *blue, uint32_t start, uint32_t size) > { > @@ -11906,7 +11803,8 @@ intel_cursor_plane_disable(struct drm_plane *plane) > > BUG_ON(!plane->crtc); > > - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); > + return plane->funcs->update_plane(plane, plane->crtc, NULL, > + 0, 0, 0, 0, 0, 0, 0, 0); > } > > static int > @@ -11970,26 +11868,113 @@ intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > struct drm_crtc *crtc = state->crtc; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_framebuffer *fb = state->fb; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > - struct drm_i915_gem_object *obj = intel_fb->obj; > - int crtc_w, crtc_h; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + enum pipe pipe = intel_crtc->pipe; > + unsigned old_width; > + uint32_t addr; > + int ret; > > crtc->cursor_x = state->orig_dst.x1; > crtc->cursor_y = state->orig_dst.y1; > - if (fb != crtc->cursor->fb) { > - crtc_w = drm_rect_width(&state->orig_dst); > - crtc_h = drm_rect_height(&state->orig_dst); > - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); > + > + if (intel_crtc->cursor_bo == obj) > + goto update; > + > + /* if we want to turn off the cursor ignore width and height */ > + if (!obj) { > + DRM_DEBUG_KMS("cursor off\n"); > + addr = 0; > + mutex_lock(&dev->struct_mutex); > + goto finish; > + } > + > + /* we only need to pin inside GTT if cursor is non-phy */ > + mutex_lock(&dev->struct_mutex); > + if (!INTEL_INFO(dev)->cursor_needs_physical) { > + unsigned alignment; > + > + /* > + * Global gtt pte registers are special registers which actually > + * forward writes to a chunk of system memory. Which means that > + * there is no risk that the register values disappear as soon > + * as we call intel_runtime_pm_put(), so it is correct to wrap > + * only the pin/unpin/fence and not more. > + */ > + intel_runtime_pm_get(dev_priv); > + > + /* Note that the w/a also requires 2 PTE of padding following > + * the bo. We currently fill all unused PTE with the shadow > + * page and so we should always have valid PTE following the > + * cursor preventing the VT-d warning. > + */ > + alignment = 0; > + if (need_vtd_wa(dev)) > + alignment = 64*1024; > + > + ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > + if (ret) { > + DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > + intel_runtime_pm_put(dev_priv); > + goto fail_locked; > + } > + > + ret = i915_gem_object_put_fence(obj); > + if (ret) { > + DRM_DEBUG_KMS("failed to release fence for cursor"); > + intel_runtime_pm_put(dev_priv); > + goto fail_unpin; > + } > + > + addr = i915_gem_obj_ggtt_offset(obj); > + > + intel_runtime_pm_put(dev_priv); > } else { > - intel_crtc_update_cursor(crtc, state->visible); > + 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"); > + goto fail_locked; > + } > + addr = obj->phys_handle->busaddr; > + } > > - intel_frontbuffer_flip(crtc->dev, > - INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); > +finish: > + if (intel_crtc->cursor_bo) { > + if (!INTEL_INFO(dev)->cursor_needs_physical) > + i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > + } > > - return 0; > + i915_gem_track_fb(intel_crtc->cursor_bo, obj, > + INTEL_FRONTBUFFER_CURSOR(pipe)); > + mutex_unlock(&dev->struct_mutex); > + > + 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); > + intel_crtc_update_cursor(crtc, state->visible); So we need to make sure state->visible==false when there's no fb. I suppose we should just do that in drm_plane_helper_check_update(). > + > + intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > } > + > + return 0; > +fail_unpin: > + i915_gem_object_unpin_from_display_plane(obj); > +fail_locked: > + mutex_unlock(&dev->struct_mutex); > + drm_gem_object_unreference_unlocked(&obj->base); > + return ret; > } > > static int > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Ville, 2014-10-24 Ville Syrjälä <ville.syrjala@linux.intel.com>: > On Fri, Oct 24, 2014 at 02:51:35PM +0100, Gustavo Padovan wrote: > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > > > Merge it into the plane update_plane() callback and make other > > users use the update_plane() functions instead. > > > > The fb != crtc->cursor->fb was already inside intel_crtc_cursor_set_obj() > > so we fold intel_crtc_cursor_set_obj() inside intel_commit_cursor_plane() > > and merge both paths into one. > > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > --- > > drivers/gpu/drm/i915/intel_display.c | 215 ++++++++++++++++------------------- > > 1 file changed, 100 insertions(+), 115 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 9a913f5..60ec165 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -8453,109 +8453,6 @@ static bool cursor_size_ok(struct drm_device *dev, > > return true; > > } > > > > -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > > - struct drm_i915_gem_object *obj, > > - uint32_t width, uint32_t height) > > -{ > > - struct drm_device *dev = crtc->dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > - enum pipe pipe = intel_crtc->pipe; > > - unsigned old_width; > > - uint32_t addr; > > - int ret; > > - > > - /* if we want to turn off the cursor ignore width and height */ > > - if (!obj) { > > - DRM_DEBUG_KMS("cursor off\n"); > > - addr = 0; > > - mutex_lock(&dev->struct_mutex); > > - goto finish; > > - } > > - > > - /* we only need to pin inside GTT if cursor is non-phy */ > > - mutex_lock(&dev->struct_mutex); > > - if (!INTEL_INFO(dev)->cursor_needs_physical) { > > - unsigned alignment; > > - > > - /* > > - * Global gtt pte registers are special registers which actually > > - * forward writes to a chunk of system memory. Which means that > > - * there is no risk that the register values disappear as soon > > - * as we call intel_runtime_pm_put(), so it is correct to wrap > > - * only the pin/unpin/fence and not more. > > - */ > > - intel_runtime_pm_get(dev_priv); > > - > > - /* Note that the w/a also requires 2 PTE of padding following > > - * the bo. We currently fill all unused PTE with the shadow > > - * page and so we should always have valid PTE following the > > - * cursor preventing the VT-d warning. > > - */ > > - alignment = 0; > > - if (need_vtd_wa(dev)) > > - alignment = 64*1024; > > - > > - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > > - if (ret) { > > - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > > - intel_runtime_pm_put(dev_priv); > > - goto fail_locked; > > - } > > - > > - ret = i915_gem_object_put_fence(obj); > > - if (ret) { > > - DRM_DEBUG_KMS("failed to release fence for cursor"); > > - intel_runtime_pm_put(dev_priv); > > - goto fail_unpin; > > - } > > - > > - addr = i915_gem_obj_ggtt_offset(obj); > > - > > - intel_runtime_pm_put(dev_priv); > > - } else { > > - 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"); > > - goto fail_locked; > > - } > > - addr = obj->phys_handle->busaddr; > > - } > > - > > - finish: > > - if (intel_crtc->cursor_bo) { > > - if (!INTEL_INFO(dev)->cursor_needs_physical) > > - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > > - } > > - > > - i915_gem_track_fb(intel_crtc->cursor_bo, obj, > > - INTEL_FRONTBUFFER_CURSOR(pipe)); > > - mutex_unlock(&dev->struct_mutex); > > - > > - old_width = intel_crtc->cursor_width; > > - > > - intel_crtc->cursor_addr = addr; > > - intel_crtc->cursor_bo = obj; > > - intel_crtc->cursor_width = width; > > - intel_crtc->cursor_height = height; > > - > > - if (intel_crtc->active) { > > - if (old_width != width) > > - intel_update_watermarks(crtc); > > - intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL); > > - > > - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > > - } > > - > > - return 0; > > -fail_unpin: > > - i915_gem_object_unpin_from_display_plane(obj); > > -fail_locked: > > - mutex_unlock(&dev->struct_mutex); > > - return ret; > > -} > > - > > static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > > u16 *blue, uint32_t start, uint32_t size) > > { > > @@ -11906,7 +11803,8 @@ intel_cursor_plane_disable(struct drm_plane *plane) > > > > BUG_ON(!plane->crtc); > > > > - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); > > + return plane->funcs->update_plane(plane, plane->crtc, NULL, > > + 0, 0, 0, 0, 0, 0, 0, 0); > > } > > > > static int > > @@ -11970,26 +11868,113 @@ intel_commit_cursor_plane(struct drm_plane *plane, > > struct intel_plane_state *state) > > { > > struct drm_crtc *crtc = state->crtc; > > + struct drm_device *dev = crtc->dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > struct drm_framebuffer *fb = state->fb; > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > - struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > > - struct drm_i915_gem_object *obj = intel_fb->obj; > > - int crtc_w, crtc_h; > > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > > + enum pipe pipe = intel_crtc->pipe; > > + unsigned old_width; > > + uint32_t addr; > > + int ret; > > > > crtc->cursor_x = state->orig_dst.x1; > > crtc->cursor_y = state->orig_dst.y1; > > - if (fb != crtc->cursor->fb) { > > - crtc_w = drm_rect_width(&state->orig_dst); > > - crtc_h = drm_rect_height(&state->orig_dst); > > - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); > > + > > + if (intel_crtc->cursor_bo == obj) > > + goto update; > > + > > + /* if we want to turn off the cursor ignore width and height */ > > + if (!obj) { > > + DRM_DEBUG_KMS("cursor off\n"); > > + addr = 0; > > + mutex_lock(&dev->struct_mutex); > > + goto finish; > > + } > > + > > + /* we only need to pin inside GTT if cursor is non-phy */ > > + mutex_lock(&dev->struct_mutex); > > + if (!INTEL_INFO(dev)->cursor_needs_physical) { > > + unsigned alignment; > > + > > + /* > > + * Global gtt pte registers are special registers which actually > > + * forward writes to a chunk of system memory. Which means that > > + * there is no risk that the register values disappear as soon > > + * as we call intel_runtime_pm_put(), so it is correct to wrap > > + * only the pin/unpin/fence and not more. > > + */ > > + intel_runtime_pm_get(dev_priv); > > + > > + /* Note that the w/a also requires 2 PTE of padding following > > + * the bo. We currently fill all unused PTE with the shadow > > + * page and so we should always have valid PTE following the > > + * cursor preventing the VT-d warning. > > + */ > > + alignment = 0; > > + if (need_vtd_wa(dev)) > > + alignment = 64*1024; > > + > > + ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > > + if (ret) { > > + DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > > + intel_runtime_pm_put(dev_priv); > > + goto fail_locked; > > + } > > + > > + ret = i915_gem_object_put_fence(obj); > > + if (ret) { > > + DRM_DEBUG_KMS("failed to release fence for cursor"); > > + intel_runtime_pm_put(dev_priv); > > + goto fail_unpin; > > + } > > + > > + addr = i915_gem_obj_ggtt_offset(obj); > > + > > + intel_runtime_pm_put(dev_priv); > > } else { > > - intel_crtc_update_cursor(crtc, state->visible); > > + 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"); > > + goto fail_locked; > > + } > > + addr = obj->phys_handle->busaddr; > > + } > > > > - intel_frontbuffer_flip(crtc->dev, > > - INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); > > +finish: > > + if (intel_crtc->cursor_bo) { > > + if (!INTEL_INFO(dev)->cursor_needs_physical) > > + i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > > + } > > > > - return 0; > > + i915_gem_track_fb(intel_crtc->cursor_bo, obj, > > + INTEL_FRONTBUFFER_CURSOR(pipe)); > > + mutex_unlock(&dev->struct_mutex); > > + > > + 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); > > + intel_crtc_update_cursor(crtc, state->visible); > > So we need to make sure state->visible==false when there's no fb. I > suppose we should just do that in drm_plane_helper_check_update(). The code to check that is merged already, do you have any other comment on this patch? Gustavo
On Fri, Oct 24, 2014 at 02:51:35PM +0100, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > Merge it into the plane update_plane() callback and make other > users use the update_plane() functions instead. > > The fb != crtc->cursor->fb was already inside intel_crtc_cursor_set_obj() > so we fold intel_crtc_cursor_set_obj() inside intel_commit_cursor_plane() > and merge both paths into one. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > --- > drivers/gpu/drm/i915/intel_display.c | 215 ++++++++++++++++------------------- > 1 file changed, 100 insertions(+), 115 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 9a913f5..60ec165 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8453,109 +8453,6 @@ static bool cursor_size_ok(struct drm_device *dev, > return true; > } > > -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > - struct drm_i915_gem_object *obj, > - uint32_t width, uint32_t height) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > - unsigned old_width; > - uint32_t addr; > - int ret; > - > - /* if we want to turn off the cursor ignore width and height */ > - if (!obj) { > - DRM_DEBUG_KMS("cursor off\n"); > - addr = 0; > - mutex_lock(&dev->struct_mutex); > - goto finish; > - } > - > - /* we only need to pin inside GTT if cursor is non-phy */ > - mutex_lock(&dev->struct_mutex); > - if (!INTEL_INFO(dev)->cursor_needs_physical) { > - unsigned alignment; > - > - /* > - * Global gtt pte registers are special registers which actually > - * forward writes to a chunk of system memory. Which means that > - * there is no risk that the register values disappear as soon > - * as we call intel_runtime_pm_put(), so it is correct to wrap > - * only the pin/unpin/fence and not more. > - */ > - intel_runtime_pm_get(dev_priv); > - > - /* Note that the w/a also requires 2 PTE of padding following > - * the bo. We currently fill all unused PTE with the shadow > - * page and so we should always have valid PTE following the > - * cursor preventing the VT-d warning. > - */ > - alignment = 0; > - if (need_vtd_wa(dev)) > - alignment = 64*1024; > - > - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > - if (ret) { > - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > - intel_runtime_pm_put(dev_priv); > - goto fail_locked; > - } > - > - ret = i915_gem_object_put_fence(obj); > - if (ret) { > - DRM_DEBUG_KMS("failed to release fence for cursor"); > - intel_runtime_pm_put(dev_priv); > - goto fail_unpin; > - } > - > - addr = i915_gem_obj_ggtt_offset(obj); > - > - intel_runtime_pm_put(dev_priv); > - } else { > - 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"); > - goto fail_locked; > - } > - addr = obj->phys_handle->busaddr; > - } > - > - finish: > - if (intel_crtc->cursor_bo) { > - if (!INTEL_INFO(dev)->cursor_needs_physical) > - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > - } > - > - i915_gem_track_fb(intel_crtc->cursor_bo, obj, > - INTEL_FRONTBUFFER_CURSOR(pipe)); > - mutex_unlock(&dev->struct_mutex); > - > - old_width = intel_crtc->cursor_width; > - > - intel_crtc->cursor_addr = addr; > - intel_crtc->cursor_bo = obj; > - intel_crtc->cursor_width = width; > - intel_crtc->cursor_height = height; > - > - if (intel_crtc->active) { > - if (old_width != width) > - intel_update_watermarks(crtc); > - intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL); > - > - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > - } > - > - return 0; > -fail_unpin: > - i915_gem_object_unpin_from_display_plane(obj); > -fail_locked: > - mutex_unlock(&dev->struct_mutex); > - return ret; > -} > - > static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > u16 *blue, uint32_t start, uint32_t size) > { > @@ -11906,7 +11803,8 @@ intel_cursor_plane_disable(struct drm_plane *plane) > > BUG_ON(!plane->crtc); > > - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); > + return plane->funcs->update_plane(plane, plane->crtc, NULL, > + 0, 0, 0, 0, 0, 0, 0, 0); > } > > static int > @@ -11970,26 +11868,113 @@ intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > struct drm_crtc *crtc = state->crtc; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_framebuffer *fb = state->fb; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > - struct drm_i915_gem_object *obj = intel_fb->obj; > - int crtc_w, crtc_h; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + enum pipe pipe = intel_crtc->pipe; > + unsigned old_width; > + uint32_t addr; > + int ret; > > crtc->cursor_x = state->orig_dst.x1; > crtc->cursor_y = state->orig_dst.y1; > - if (fb != crtc->cursor->fb) { > - crtc_w = drm_rect_width(&state->orig_dst); > - crtc_h = drm_rect_height(&state->orig_dst); > - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); > + > + if (intel_crtc->cursor_bo == obj) > + goto update; > + > + /* if we want to turn off the cursor ignore width and height */ > + if (!obj) { > + DRM_DEBUG_KMS("cursor off\n"); > + addr = 0; > + mutex_lock(&dev->struct_mutex); > + goto finish; > + } > + > + /* we only need to pin inside GTT if cursor is non-phy */ > + mutex_lock(&dev->struct_mutex); > + if (!INTEL_INFO(dev)->cursor_needs_physical) { > + unsigned alignment; > + > + /* > + * Global gtt pte registers are special registers which actually > + * forward writes to a chunk of system memory. Which means that > + * there is no risk that the register values disappear as soon > + * as we call intel_runtime_pm_put(), so it is correct to wrap > + * only the pin/unpin/fence and not more. > + */ > + intel_runtime_pm_get(dev_priv); > + > + /* Note that the w/a also requires 2 PTE of padding following > + * the bo. We currently fill all unused PTE with the shadow > + * page and so we should always have valid PTE following the > + * cursor preventing the VT-d warning. > + */ > + alignment = 0; > + if (need_vtd_wa(dev)) > + alignment = 64*1024; > + > + ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > + if (ret) { > + DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > + intel_runtime_pm_put(dev_priv); > + goto fail_locked; > + } > + > + ret = i915_gem_object_put_fence(obj); > + if (ret) { > + DRM_DEBUG_KMS("failed to release fence for cursor"); > + intel_runtime_pm_put(dev_priv); > + goto fail_unpin; > + } > + > + addr = i915_gem_obj_ggtt_offset(obj); > + > + intel_runtime_pm_put(dev_priv); > } else { > - intel_crtc_update_cursor(crtc, state->visible); > + 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"); > + goto fail_locked; > + } > + addr = obj->phys_handle->busaddr; > + } > > - intel_frontbuffer_flip(crtc->dev, > - INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); > +finish: > + if (intel_crtc->cursor_bo) { > + if (!INTEL_INFO(dev)->cursor_needs_physical) > + i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > + } > > - return 0; > + i915_gem_track_fb(intel_crtc->cursor_bo, obj, > + INTEL_FRONTBUFFER_CURSOR(pipe)); > + mutex_unlock(&dev->struct_mutex); > + > + 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); > + intel_crtc_update_cursor(crtc, state->visible); > + > + intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > } > + > + return 0; > +fail_unpin: > + i915_gem_object_unpin_from_display_plane(obj); > +fail_locked: > + mutex_unlock(&dev->struct_mutex); > + drm_gem_object_unreference_unlocked(&obj->base); That unref needs to be dropped. With that change the patch is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + return ret; > } > > static int > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9a913f5..60ec165 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8453,109 +8453,6 @@ static bool cursor_size_ok(struct drm_device *dev, return true; } -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, - struct drm_i915_gem_object *obj, - uint32_t width, uint32_t height) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - unsigned old_width; - uint32_t addr; - int ret; - - /* if we want to turn off the cursor ignore width and height */ - if (!obj) { - DRM_DEBUG_KMS("cursor off\n"); - addr = 0; - mutex_lock(&dev->struct_mutex); - goto finish; - } - - /* we only need to pin inside GTT if cursor is non-phy */ - mutex_lock(&dev->struct_mutex); - if (!INTEL_INFO(dev)->cursor_needs_physical) { - unsigned alignment; - - /* - * Global gtt pte registers are special registers which actually - * forward writes to a chunk of system memory. Which means that - * there is no risk that the register values disappear as soon - * as we call intel_runtime_pm_put(), so it is correct to wrap - * only the pin/unpin/fence and not more. - */ - intel_runtime_pm_get(dev_priv); - - /* Note that the w/a also requires 2 PTE of padding following - * the bo. We currently fill all unused PTE with the shadow - * page and so we should always have valid PTE following the - * cursor preventing the VT-d warning. - */ - alignment = 0; - if (need_vtd_wa(dev)) - alignment = 64*1024; - - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); - if (ret) { - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); - intel_runtime_pm_put(dev_priv); - goto fail_locked; - } - - ret = i915_gem_object_put_fence(obj); - if (ret) { - DRM_DEBUG_KMS("failed to release fence for cursor"); - intel_runtime_pm_put(dev_priv); - goto fail_unpin; - } - - addr = i915_gem_obj_ggtt_offset(obj); - - intel_runtime_pm_put(dev_priv); - } else { - 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"); - goto fail_locked; - } - addr = obj->phys_handle->busaddr; - } - - finish: - if (intel_crtc->cursor_bo) { - if (!INTEL_INFO(dev)->cursor_needs_physical) - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); - } - - i915_gem_track_fb(intel_crtc->cursor_bo, obj, - INTEL_FRONTBUFFER_CURSOR(pipe)); - mutex_unlock(&dev->struct_mutex); - - old_width = intel_crtc->cursor_width; - - intel_crtc->cursor_addr = addr; - intel_crtc->cursor_bo = obj; - intel_crtc->cursor_width = width; - intel_crtc->cursor_height = height; - - if (intel_crtc->active) { - if (old_width != width) - intel_update_watermarks(crtc); - intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL); - - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); - } - - return 0; -fail_unpin: - i915_gem_object_unpin_from_display_plane(obj); -fail_locked: - mutex_unlock(&dev->struct_mutex); - return ret; -} - static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t start, uint32_t size) { @@ -11906,7 +11803,8 @@ intel_cursor_plane_disable(struct drm_plane *plane) BUG_ON(!plane->crtc); - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); + return plane->funcs->update_plane(plane, plane->crtc, NULL, + 0, 0, 0, 0, 0, 0, 0, 0); } static int @@ -11970,26 +11868,113 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_crtc *crtc = state->crtc; + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; struct drm_framebuffer *fb = state->fb; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); - struct drm_i915_gem_object *obj = intel_fb->obj; - int crtc_w, crtc_h; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + enum pipe pipe = intel_crtc->pipe; + unsigned old_width; + uint32_t addr; + int ret; crtc->cursor_x = state->orig_dst.x1; crtc->cursor_y = state->orig_dst.y1; - if (fb != crtc->cursor->fb) { - crtc_w = drm_rect_width(&state->orig_dst); - crtc_h = drm_rect_height(&state->orig_dst); - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); + + if (intel_crtc->cursor_bo == obj) + goto update; + + /* if we want to turn off the cursor ignore width and height */ + if (!obj) { + DRM_DEBUG_KMS("cursor off\n"); + addr = 0; + mutex_lock(&dev->struct_mutex); + goto finish; + } + + /* we only need to pin inside GTT if cursor is non-phy */ + mutex_lock(&dev->struct_mutex); + if (!INTEL_INFO(dev)->cursor_needs_physical) { + unsigned alignment; + + /* + * Global gtt pte registers are special registers which actually + * forward writes to a chunk of system memory. Which means that + * there is no risk that the register values disappear as soon + * as we call intel_runtime_pm_put(), so it is correct to wrap + * only the pin/unpin/fence and not more. + */ + intel_runtime_pm_get(dev_priv); + + /* Note that the w/a also requires 2 PTE of padding following + * the bo. We currently fill all unused PTE with the shadow + * page and so we should always have valid PTE following the + * cursor preventing the VT-d warning. + */ + alignment = 0; + if (need_vtd_wa(dev)) + alignment = 64*1024; + + ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); + if (ret) { + DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); + intel_runtime_pm_put(dev_priv); + goto fail_locked; + } + + ret = i915_gem_object_put_fence(obj); + if (ret) { + DRM_DEBUG_KMS("failed to release fence for cursor"); + intel_runtime_pm_put(dev_priv); + goto fail_unpin; + } + + addr = i915_gem_obj_ggtt_offset(obj); + + intel_runtime_pm_put(dev_priv); } else { - intel_crtc_update_cursor(crtc, state->visible); + 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"); + goto fail_locked; + } + addr = obj->phys_handle->busaddr; + } - intel_frontbuffer_flip(crtc->dev, - INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); +finish: + if (intel_crtc->cursor_bo) { + if (!INTEL_INFO(dev)->cursor_needs_physical) + i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); + } - return 0; + i915_gem_track_fb(intel_crtc->cursor_bo, obj, + INTEL_FRONTBUFFER_CURSOR(pipe)); + mutex_unlock(&dev->struct_mutex); + + 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); + intel_crtc_update_cursor(crtc, state->visible); + + intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); } + + return 0; +fail_unpin: + i915_gem_object_unpin_from_display_plane(obj); +fail_locked: + mutex_unlock(&dev->struct_mutex); + drm_gem_object_unreference_unlocked(&obj->base); + return ret; } static int