* [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb()
@ 2016-11-15 8:58 Chris Wilson
2016-11-15 8:58 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Chris Wilson @ 2016-11-15 8:58 UTC (permalink / raw)
To: intel-gfx
In the next patch, a few rearrangements are made to make these static.
First, we move them so the changes are not lost in the noise.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_display.c | 255 ++++++++++++++++++-----------------
drivers/gpu/drm/i915/intel_drv.h | 2 +
2 files changed, 130 insertions(+), 127 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb9377de456e..8e04f31bf12e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14144,6 +14144,134 @@ static int intel_atomic_check(struct drm_device *dev,
return calc_watermark_data(state);
}
+/**
+ * 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.
+ *
+ * Must be called with struct_mutex held.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int
+intel_prepare_plane_fb(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
+{
+ struct intel_atomic_state *intel_state =
+ to_intel_atomic_state(new_state->state);
+ struct drm_i915_private *dev_priv = to_i915(plane->dev);
+ struct drm_framebuffer *fb = new_state->fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
+ int ret;
+
+ if (!obj && !old_obj)
+ return 0;
+
+ if (old_obj) {
+ struct drm_crtc_state *crtc_state =
+ drm_atomic_get_existing_crtc_state(new_state->state,
+ plane->state->crtc);
+
+ /* Big Hammer, we also need to ensure that any pending
+ * MI_WAIT_FOR_EVENT inside a user batch buffer on the
+ * current scanout is retired before unpinning the old
+ * framebuffer. Note that we rely on userspace rendering
+ * into the buffer attached to the pipe they are waiting
+ * on. If not, userspace generates a GPU hang with IPEHR
+ * point to the MI_WAIT_FOR_EVENT.
+ *
+ * This should only fail upon a hung GPU, in which case we
+ * can safely continue.
+ */
+ if (needs_modeset(crtc_state)) {
+ ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
+ old_obj->resv, NULL,
+ false, 0,
+ GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ if (new_state->fence) { /* explicit fencing */
+ ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready,
+ new_state->fence,
+ I915_FENCE_TIMEOUT,
+ GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (!obj)
+ return 0;
+
+ if (!new_state->fence) { /* implicit fencing */
+ ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
+ obj->resv, NULL,
+ false, I915_FENCE_TIMEOUT,
+ GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+
+ i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
+ }
+
+ if (plane->type == DRM_PLANE_TYPE_CURSOR &&
+ INTEL_INFO(dev_priv)->cursor_needs_physical) {
+ int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
+ ret = i915_gem_object_attach_phys(obj, 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_state->rotation);
+ if (IS_ERR(vma)) {
+ DRM_DEBUG_KMS("failed to pin object\n");
+ return PTR_ERR(vma);
+ }
+ }
+
+ return 0;
+}
+
+/**
+ * intel_cleanup_plane_fb - Cleans up an fb after plane use
+ * @plane: drm plane to clean up for
+ * @fb: old framebuffer that was on plane
+ *
+ * Cleans up a framebuffer that has just been removed from a plane.
+ *
+ * Must be called with struct_mutex held.
+ */
+void
+intel_cleanup_plane_fb(struct drm_plane *plane,
+ struct drm_plane_state *old_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(plane->dev);
+ struct intel_plane_state *old_intel_state;
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
+ struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
+
+ old_intel_state = to_intel_plane_state(old_state);
+
+ if (!obj && !old_obj)
+ return;
+
+ if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
+ !INTEL_INFO(dev_priv)->cursor_needs_physical))
+ intel_unpin_fb_obj(old_state->fb, old_state->rotation);
+}
+
+
static int intel_atomic_prepare_commit(struct drm_device *dev,
struct drm_atomic_state *state)
{
@@ -14716,133 +14844,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
.atomic_destroy_state = intel_crtc_destroy_state,
};
-/**
- * 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.
- *
- * Must be called with struct_mutex held.
- *
- * Returns 0 on success, negative error code on failure.
- */
-int
-intel_prepare_plane_fb(struct drm_plane *plane,
- struct drm_plane_state *new_state)
-{
- struct intel_atomic_state *intel_state =
- to_intel_atomic_state(new_state->state);
- struct drm_i915_private *dev_priv = to_i915(plane->dev);
- struct drm_framebuffer *fb = new_state->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
- int ret;
-
- if (!obj && !old_obj)
- return 0;
-
- if (old_obj) {
- struct drm_crtc_state *crtc_state =
- drm_atomic_get_existing_crtc_state(new_state->state,
- plane->state->crtc);
-
- /* Big Hammer, we also need to ensure that any pending
- * MI_WAIT_FOR_EVENT inside a user batch buffer on the
- * current scanout is retired before unpinning the old
- * framebuffer. Note that we rely on userspace rendering
- * into the buffer attached to the pipe they are waiting
- * on. If not, userspace generates a GPU hang with IPEHR
- * point to the MI_WAIT_FOR_EVENT.
- *
- * This should only fail upon a hung GPU, in which case we
- * can safely continue.
- */
- if (needs_modeset(crtc_state)) {
- ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
- old_obj->resv, NULL,
- false, 0,
- GFP_KERNEL);
- if (ret < 0)
- return ret;
- }
- }
-
- if (new_state->fence) { /* explicit fencing */
- ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready,
- new_state->fence,
- I915_FENCE_TIMEOUT,
- GFP_KERNEL);
- if (ret < 0)
- return ret;
- }
-
- if (!obj)
- return 0;
-
- if (!new_state->fence) { /* implicit fencing */
- ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
- obj->resv, NULL,
- false, I915_FENCE_TIMEOUT,
- GFP_KERNEL);
- if (ret < 0)
- return ret;
-
- i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
- }
-
- if (plane->type == DRM_PLANE_TYPE_CURSOR &&
- INTEL_INFO(dev_priv)->cursor_needs_physical) {
- int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
- ret = i915_gem_object_attach_phys(obj, 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_state->rotation);
- if (IS_ERR(vma)) {
- DRM_DEBUG_KMS("failed to pin object\n");
- return PTR_ERR(vma);
- }
- }
-
- return 0;
-}
-
-/**
- * intel_cleanup_plane_fb - Cleans up an fb after plane use
- * @plane: drm plane to clean up for
- * @fb: old framebuffer that was on plane
- *
- * Cleans up a framebuffer that has just been removed from a plane.
- *
- * Must be called with struct_mutex held.
- */
-void
-intel_cleanup_plane_fb(struct drm_plane *plane,
- struct drm_plane_state *old_state)
-{
- struct drm_i915_private *dev_priv = to_i915(plane->dev);
- struct intel_plane_state *old_intel_state;
- struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
- struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
-
- old_intel_state = to_intel_plane_state(old_state);
-
- if (!obj && !old_obj)
- return;
-
- if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
- !INTEL_INFO(dev_priv)->cursor_needs_physical))
- intel_unpin_fb_obj(old_state->fb, old_state->rotation);
-}
-
int
skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
{
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 75252ecaa613..4cb8254c66dd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -772,6 +772,8 @@ struct intel_plane {
int max_downscale;
uint32_t frontbuffer_bit;
+ struct i915_sw_fence *last_commit;
+
/* Since we need to change the watermarks before/after
* enabling/disabling the planes, we need to store the parameters here
* as the other pieces of the struct may not reflect the values we want
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit 2016-11-15 8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson @ 2016-11-15 8:58 ` Chris Wilson 2016-11-15 9:37 ` Daniel Vetter 2016-11-15 8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson ` (4 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2016-11-15 8:58 UTC (permalink / raw) To: intel-gfx The generic atomic helper likes to skip a prepare_plane_fb() if it decides that the plane->fb is unchanged. This is wrong for us for a couple of reasons: - if the pipe is reconfigured (i.e. a size change) but the framebuffer is untouched, we still have to flush any rendering prior to the reconfiguration to prevent wait-for-scanline GPU hangs - if the framebuffer is rotated, it remains the same but has a different view and a different address. Finally, even if the framebuffer is unchanged the flip/modeset should be ordered with respect to rendering to the frontbuffer. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_atomic_plane.c | 2 -- drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++++++++++++------- drivers/gpu/drm/i915/intel_drv.h | 4 --- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index ff821649486e..89a34350a35d 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -201,8 +201,6 @@ static void intel_plane_atomic_update(struct drm_plane *plane, } const struct drm_plane_helper_funcs intel_plane_helper_funcs = { - .prepare_fb = intel_prepare_plane_fb, - .cleanup_fb = intel_cleanup_plane_fb, .atomic_check = intel_plane_atomic_check, .atomic_update = intel_plane_atomic_update, }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8e04f31bf12e..dca1e0b512e5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14158,7 +14158,7 @@ static int intel_atomic_check(struct drm_device *dev, * * Returns 0 on success, negative error code on failure. */ -int +static int intel_prepare_plane_fb(struct drm_plane *plane, struct drm_plane_state *new_state) { @@ -14252,7 +14252,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, * * Must be called with struct_mutex held. */ -void +static void intel_cleanup_plane_fb(struct drm_plane *plane, struct drm_plane_state *old_state) { @@ -14271,6 +14271,49 @@ intel_cleanup_plane_fb(struct drm_plane *plane, intel_unpin_fb_obj(old_state->fb, old_state->rotation); } +static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state) +{ + struct drm_plane *plane; + struct drm_plane_state *plane_state; + int i, j, ret; + + ret = mutex_lock_interruptible(&state->dev->struct_mutex); + if (ret) + return ret; + + for_each_plane_in_state(state, plane, plane_state, i) { + ret = intel_prepare_plane_fb(plane, plane_state); + if (ret) + break; + } + + if (ret) { + for_each_plane_in_state(state, plane, plane_state, j) { + if (j >= i) + break; + + intel_cleanup_plane_fb(plane, plane_state); + } + } + + mutex_unlock(&state->dev->struct_mutex); + + return ret; +} + +static void intel_atomic_commit_cleanup_planes(struct drm_atomic_state *state) +{ + struct drm_plane *plane; + struct drm_plane_state *plane_state; + int i; + + mutex_lock(&state->dev->struct_mutex); + + for_each_plane_in_state(state, plane, plane_state, i) + intel_cleanup_plane_fb(plane, plane_state); + + mutex_unlock(&state->dev->struct_mutex); +} static int intel_atomic_prepare_commit(struct drm_device *dev, struct drm_atomic_state *state) @@ -14292,14 +14335,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, flush_workqueue(dev_priv->wq); } - ret = mutex_lock_interruptible(&dev->struct_mutex); - if (ret) - return ret; - - ret = drm_atomic_helper_prepare_planes(dev, state); - mutex_unlock(&dev->struct_mutex); - - return ret; + return intel_atomic_commit_prepare_planes(state); } u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) @@ -14622,9 +14658,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) if (intel_state->modeset) intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET); - mutex_lock(&dev->struct_mutex); - drm_atomic_helper_cleanup_planes(dev, state); - mutex_unlock(&dev->struct_mutex); + intel_atomic_commit_cleanup_planes(state); drm_atomic_helper_commit_cleanup_done(state); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4cb8254c66dd..fff55d93ca73 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1281,10 +1281,6 @@ __intel_framebuffer_create(struct drm_device *dev, void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe); void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe); void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe); -int intel_prepare_plane_fb(struct drm_plane *plane, - struct drm_plane_state *new_state); -void intel_cleanup_plane_fb(struct drm_plane *plane, - struct drm_plane_state *old_state); int intel_plane_atomic_get_property(struct drm_plane *plane, const struct drm_plane_state *state, struct drm_property *property, -- 2.10.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit 2016-11-15 8:58 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson @ 2016-11-15 9:37 ` Daniel Vetter 2016-11-15 9:47 ` Tvrtko Ursulin 2016-11-15 9:57 ` Chris Wilson 0 siblings, 2 replies; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 9:37 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Nov 15, 2016 at 08:58:14AM +0000, Chris Wilson wrote: > The generic atomic helper likes to skip a prepare_plane_fb() if it > decides that the plane->fb is unchanged. This is wrong for us for a > couple of reasons: > > - if the pipe is reconfigured (i.e. a size change) but the framebuffer > is untouched, we still have to flush any rendering prior to the > reconfiguration to prevent wait-for-scanline GPU hangs > > - if the framebuffer is rotated, it remains the same but has a > different view and a different address. > > Finally, even if the framebuffer is unchanged the flip/modeset should be > ordered with respect to rendering to the frontbuffer. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> This is directly against commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba Author: Keith Packard <keithp@keithp.com> Date: Sat Jun 4 01:16:22 2016 -0700 drm: Don't prepare or cleanup unchanging frame buffers [v3] and I'm pretty sure that was tested on i915. Do we need to instead revert the core change? Iirc the idea was to make cursors block less, but that didn't really pan out fully. -Daniel > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 -- > drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++++++++++++------- > drivers/gpu/drm/i915/intel_drv.h | 4 --- > 3 files changed, 47 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index ff821649486e..89a34350a35d 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -201,8 +201,6 @@ static void intel_plane_atomic_update(struct drm_plane *plane, > } > > const struct drm_plane_helper_funcs intel_plane_helper_funcs = { > - .prepare_fb = intel_prepare_plane_fb, > - .cleanup_fb = intel_cleanup_plane_fb, > .atomic_check = intel_plane_atomic_check, > .atomic_update = intel_plane_atomic_update, > }; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8e04f31bf12e..dca1e0b512e5 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14158,7 +14158,7 @@ static int intel_atomic_check(struct drm_device *dev, > * > * Returns 0 on success, negative error code on failure. > */ > -int > +static int > intel_prepare_plane_fb(struct drm_plane *plane, > struct drm_plane_state *new_state) > { > @@ -14252,7 +14252,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, > * > * Must be called with struct_mutex held. > */ > -void > +static void > intel_cleanup_plane_fb(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > @@ -14271,6 +14271,49 @@ intel_cleanup_plane_fb(struct drm_plane *plane, > intel_unpin_fb_obj(old_state->fb, old_state->rotation); > } > > +static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state) > +{ > + struct drm_plane *plane; > + struct drm_plane_state *plane_state; > + int i, j, ret; > + > + ret = mutex_lock_interruptible(&state->dev->struct_mutex); > + if (ret) > + return ret; > + > + for_each_plane_in_state(state, plane, plane_state, i) { > + ret = intel_prepare_plane_fb(plane, plane_state); > + if (ret) > + break; > + } > + > + if (ret) { > + for_each_plane_in_state(state, plane, plane_state, j) { > + if (j >= i) > + break; > + > + intel_cleanup_plane_fb(plane, plane_state); > + } > + } > + > + mutex_unlock(&state->dev->struct_mutex); > + > + return ret; > +} > + > +static void intel_atomic_commit_cleanup_planes(struct drm_atomic_state *state) > +{ > + struct drm_plane *plane; > + struct drm_plane_state *plane_state; > + int i; > + > + mutex_lock(&state->dev->struct_mutex); > + > + for_each_plane_in_state(state, plane, plane_state, i) > + intel_cleanup_plane_fb(plane, plane_state); > + > + mutex_unlock(&state->dev->struct_mutex); > +} > > static int intel_atomic_prepare_commit(struct drm_device *dev, > struct drm_atomic_state *state) > @@ -14292,14 +14335,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, > flush_workqueue(dev_priv->wq); > } > > - ret = mutex_lock_interruptible(&dev->struct_mutex); > - if (ret) > - return ret; > - > - ret = drm_atomic_helper_prepare_planes(dev, state); > - mutex_unlock(&dev->struct_mutex); > - > - return ret; > + return intel_atomic_commit_prepare_planes(state); > } > > u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) > @@ -14622,9 +14658,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > if (intel_state->modeset) > intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET); > > - mutex_lock(&dev->struct_mutex); > - drm_atomic_helper_cleanup_planes(dev, state); > - mutex_unlock(&dev->struct_mutex); > + intel_atomic_commit_cleanup_planes(state); > > drm_atomic_helper_commit_cleanup_done(state); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 4cb8254c66dd..fff55d93ca73 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1281,10 +1281,6 @@ __intel_framebuffer_create(struct drm_device *dev, > void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe); > void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe); > void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe); > -int intel_prepare_plane_fb(struct drm_plane *plane, > - struct drm_plane_state *new_state); > -void intel_cleanup_plane_fb(struct drm_plane *plane, > - struct drm_plane_state *old_state); > int intel_plane_atomic_get_property(struct drm_plane *plane, > const struct drm_plane_state *state, > struct drm_property *property, > -- > 2.10.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit 2016-11-15 9:37 ` Daniel Vetter @ 2016-11-15 9:47 ` Tvrtko Ursulin 2016-11-15 9:57 ` Chris Wilson 1 sibling, 0 replies; 19+ messages in thread From: Tvrtko Ursulin @ 2016-11-15 9:47 UTC (permalink / raw) To: Daniel Vetter, Chris Wilson; +Cc: intel-gfx On 15/11/2016 09:37, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 08:58:14AM +0000, Chris Wilson wrote: >> The generic atomic helper likes to skip a prepare_plane_fb() if it >> decides that the plane->fb is unchanged. This is wrong for us for a >> couple of reasons: >> >> - if the pipe is reconfigured (i.e. a size change) but the framebuffer >> is untouched, we still have to flush any rendering prior to the >> reconfiguration to prevent wait-for-scanline GPU hangs >> >> - if the framebuffer is rotated, it remains the same but has a >> different view and a different address. >> >> Finally, even if the framebuffer is unchanged the flip/modeset should be >> ordered with respect to rendering to the frontbuffer. >> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > This is directly against > > commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba > Author: Keith Packard <keithp@keithp.com> > Date: Sat Jun 4 01:16:22 2016 -0700 > > drm: Don't prepare or cleanup unchanging frame buffers [v3] > > and I'm pretty sure that was tested on i915. Do we need to instead revert > the core change? Iirc the idea was to make cursors block less, but that > didn't really pan out fully. As Chris wrote in the commit, and I experienced myself, it indeed blows up when rotation is used since running prepare is crucial to set up the rotated view. In other words, I've been running with the revert of the core change to be able to use rotation. Just my 2c that it is indeed broken, I don't know this way or the other is the way to fix it. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit 2016-11-15 9:37 ` Daniel Vetter 2016-11-15 9:47 ` Tvrtko Ursulin @ 2016-11-15 9:57 ` Chris Wilson 2016-11-15 10:10 ` Daniel Vetter 1 sibling, 1 reply; 19+ messages in thread From: Chris Wilson @ 2016-11-15 9:57 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Tue, Nov 15, 2016 at 10:37:09AM +0100, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 08:58:14AM +0000, Chris Wilson wrote: > > The generic atomic helper likes to skip a prepare_plane_fb() if it > > decides that the plane->fb is unchanged. This is wrong for us for a > > couple of reasons: > > > > - if the pipe is reconfigured (i.e. a size change) but the framebuffer > > is untouched, we still have to flush any rendering prior to the > > reconfiguration to prevent wait-for-scanline GPU hangs > > > > - if the framebuffer is rotated, it remains the same but has a > > different view and a different address. > > > > Finally, even if the framebuffer is unchanged the flip/modeset should be > > ordered with respect to rendering to the frontbuffer. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > This is directly against > > commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba > Author: Keith Packard <keithp@keithp.com> > Date: Sat Jun 4 01:16:22 2016 -0700 > > drm: Don't prepare or cleanup unchanging frame buffers [v3] > > and I'm pretty sure that was tested on i915. That patch is bogus. But if we want plane granularity, the helpers are not sufficient either. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit 2016-11-15 9:57 ` Chris Wilson @ 2016-11-15 10:10 ` Daniel Vetter 2016-11-15 10:18 ` Chris Wilson 0 siblings, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 10:10 UTC (permalink / raw) To: Chris Wilson, Daniel Vetter, intel-gfx On Tue, Nov 15, 2016 at 09:57:37AM +0000, Chris Wilson wrote: > On Tue, Nov 15, 2016 at 10:37:09AM +0100, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 08:58:14AM +0000, Chris Wilson wrote: > > > The generic atomic helper likes to skip a prepare_plane_fb() if it > > > decides that the plane->fb is unchanged. This is wrong for us for a > > > couple of reasons: > > > > > > - if the pipe is reconfigured (i.e. a size change) but the framebuffer > > > is untouched, we still have to flush any rendering prior to the > > > reconfiguration to prevent wait-for-scanline GPU hangs > > > > > > - if the framebuffer is rotated, it remains the same but has a > > > different view and a different address. > > > > > > Finally, even if the framebuffer is unchanged the flip/modeset should be > > > ordered with respect to rendering to the frontbuffer. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > This is directly against > > > > commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba > > Author: Keith Packard <keithp@keithp.com> > > Date: Sat Jun 4 01:16:22 2016 -0700 > > > > drm: Don't prepare or cleanup unchanging frame buffers [v3] > > > > and I'm pretty sure that was tested on i915. > > That patch is bogus. But if we want plane granularity, the helpers are > not sufficient either. Hm, what do you mean with plane granularity? If you just mean async commits, then yes the current helper machinery won't work. But I don't think extending them is a good idea either, per-plane async updates probably need something entirely different on the back-end. We could still use the atomic ioctl just as the transport, but that's about it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit 2016-11-15 10:10 ` Daniel Vetter @ 2016-11-15 10:18 ` Chris Wilson 0 siblings, 0 replies; 19+ messages in thread From: Chris Wilson @ 2016-11-15 10:18 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Tue, Nov 15, 2016 at 11:10:26AM +0100, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 09:57:37AM +0000, Chris Wilson wrote: > > On Tue, Nov 15, 2016 at 10:37:09AM +0100, Daniel Vetter wrote: > > > On Tue, Nov 15, 2016 at 08:58:14AM +0000, Chris Wilson wrote: > > > > The generic atomic helper likes to skip a prepare_plane_fb() if it > > > > decides that the plane->fb is unchanged. This is wrong for us for a > > > > couple of reasons: > > > > > > > > - if the pipe is reconfigured (i.e. a size change) but the framebuffer > > > > is untouched, we still have to flush any rendering prior to the > > > > reconfiguration to prevent wait-for-scanline GPU hangs > > > > > > > > - if the framebuffer is rotated, it remains the same but has a > > > > different view and a different address. > > > > > > > > Finally, even if the framebuffer is unchanged the flip/modeset should be > > > > ordered with respect to rendering to the frontbuffer. > > > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > > > This is directly against > > > > > > commit fcc60b413d14dd06ddbd79ec50e83c4fb2a097ba > > > Author: Keith Packard <keithp@keithp.com> > > > Date: Sat Jun 4 01:16:22 2016 -0700 > > > > > > drm: Don't prepare or cleanup unchanging frame buffers [v3] > > > > > > and I'm pretty sure that was tested on i915. > > > > That patch is bogus. But if we want plane granularity, the helpers are > > not sufficient either. > > Hm, what do you mean with plane granularity? If you just mean async > commits, then yes the current helper machinery won't work. But I don't > think extending them is a good idea either, per-plane async updates > probably need something entirely different on the back-end. We could still > use the atomic ioctl just as the transport, but that's about it. I mean an atomic commit that only modifies the cursor plane (no wm, no other crtc or global state updates) does not impede an atomic commit that only modifies the sprite/primary plane (again no wm, no other crtc or global state updates). crtc granularity (i.e. a single timeline per crtc) simply doesn't work when the existing ABI is plane granularity. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state 2016-11-15 8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson 2016-11-15 8:58 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson @ 2016-11-15 8:58 ` Chris Wilson 2016-11-15 9:52 ` Daniel Vetter 2016-11-15 10:24 ` Daniel Vetter 2016-11-15 8:58 ` [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() Chris Wilson ` (3 subsequent siblings) 5 siblings, 2 replies; 19+ messages in thread From: Chris Wilson @ 2016-11-15 8:58 UTC (permalink / raw) To: intel-gfx With atomic plane states we are able to track an allocation right from preparation, during use and through to the final free after being swapped out for a new plane. We can couple the VMA we pin for the framebuffer (and its rotation) to this lifetime and avoid all the clumsy lookups in between. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.h | 16 ++--- drivers/gpu/drm/i915/intel_atomic_plane.c | 2 + drivers/gpu/drm/i915/intel_display.c | 97 +++++++++++++------------------ drivers/gpu/drm/i915/intel_drv.h | 9 ++- drivers/gpu/drm/i915/intel_fbc.c | 52 +++++++---------- drivers/gpu/drm/i915/intel_fbdev.c | 4 +- drivers/gpu/drm/i915/intel_sprite.c | 8 +-- 7 files changed, 78 insertions(+), 110 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4e7148a3ee8b..2bc285e2ff82 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -989,6 +989,8 @@ struct intel_fbc { struct work_struct underrun_work; struct intel_fbc_state_cache { + struct i915_vma *vma; + struct { unsigned int mode_flags; uint32_t hsw_bdw_pixel_rate; @@ -1002,15 +1004,14 @@ struct intel_fbc { } plane; struct { - u64 ilk_ggtt_offset; uint32_t pixel_format; unsigned int stride; - int fence_reg; - unsigned int tiling_mode; } fb; } state_cache; struct intel_fbc_reg_params { + struct i915_vma *vma; + struct { enum pipe pipe; enum plane plane; @@ -1018,10 +1019,8 @@ struct intel_fbc { } crtc; struct { - u64 ggtt_offset; uint32_t pixel_format; unsigned int stride; - int fence_reg; } fb; int cfb_size; @@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj, return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view); } -static inline unsigned long -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o, - const struct i915_ggtt_view *view) -{ - return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view)); -} - /* i915_gem_fence_reg.c */ int __must_check i915_vma_get_fence(struct i915_vma *vma); int __must_check i915_vma_put_fence(struct i915_vma *vma); diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 89a34350a35d..5a86e0d52409 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane) __drm_atomic_helper_plane_duplicate_state(plane, state); + intel_state->vma = NULL; + return state; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dca1e0b512e5..cb52116f8577 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation) i915_vma_pin_fence(vma); } + i915_vma_get(vma); err: intel_runtime_pm_put(dev_priv); return vma; } -void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation) +void intel_unpin_fb_vma(struct i915_vma *vma) { - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct i915_ggtt_view view; - struct i915_vma *vma; - - WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex)); - - intel_fill_fb_ggtt_view(&view, fb, rotation); - vma = i915_gem_object_to_ggtt(obj, &view); + lockdep_assert_held(&vma->vm->dev->struct_mutex); i915_vma_unpin_fence(vma); i915_gem_object_unpin_from_display_plane(vma); + i915_vma_put(vma); } static int intel_fb_pitch(const struct drm_framebuffer *fb, int plane, @@ -2752,7 +2747,6 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, struct drm_device *dev = intel_crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); struct drm_crtc *c; - struct intel_crtc *i; struct drm_i915_gem_object *obj; struct drm_plane *primary = intel_crtc->base.primary; struct drm_plane_state *plane_state = primary->state; @@ -2777,20 +2771,20 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, * an fb with another CRTC instead */ for_each_crtc(dev, c) { - i = to_intel_crtc(c); + struct intel_plane_state *state; if (c == &intel_crtc->base) continue; - if (!i->active) + if (!to_intel_crtc(c)->active) continue; - fb = c->primary->fb; - if (!fb) + state = to_intel_plane_state(c->primary->state); + if (!state->vma) continue; - obj = intel_fb_obj(fb); - if (i915_gem_object_ggtt_offset(obj, NULL) == plane_config->base) { + if (i915_ggtt_offset(state->vma) == plane_config->base) { + fb = c->primary->fb; drm_framebuffer_reference(fb); goto valid_fb; } @@ -2830,6 +2824,12 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, drm_framebuffer_reference(fb); primary->fb = primary->state->fb = fb; + + mutex_lock(&dev->struct_mutex); + intel_state->vma = + intel_pin_and_fence_fb_obj(fb, primary->state->rotation); + mutex_unlock(&dev->struct_mutex); + primary->crtc = primary->state->crtc = &intel_crtc->base; intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary)); atomic_or(to_intel_plane(primary)->frontbuffer_bit, @@ -3104,13 +3104,13 @@ static void i9xx_update_primary_plane(struct drm_plane *primary, I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); if (INTEL_INFO(dev)->gen >= 4) { I915_WRITE(DSPSURF(plane), - intel_fb_gtt_offset(fb, rotation) + + intel_plane_ggtt_offset(plane_state) + intel_crtc->dspaddr_offset); I915_WRITE(DSPTILEOFF(plane), (y << 16) | x); I915_WRITE(DSPLINOFF(plane), linear_offset); } else { I915_WRITE(DSPADDR(plane), - intel_fb_gtt_offset(fb, rotation) + + intel_plane_ggtt_offset(plane_state) + intel_crtc->dspaddr_offset); } POSTING_READ(reg); @@ -3207,7 +3207,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary, I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); I915_WRITE(DSPSURF(plane), - intel_fb_gtt_offset(fb, rotation) + + intel_plane_ggtt_offset(plane_state) + intel_crtc->dspaddr_offset); if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { I915_WRITE(DSPOFFSET(plane), (y << 16) | x); @@ -3230,23 +3230,6 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv, } } -u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, - unsigned int rotation) -{ - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct i915_ggtt_view view; - struct i915_vma *vma; - - intel_fill_fb_ggtt_view(&view, fb, rotation); - - vma = i915_gem_object_to_ggtt(obj, &view); - if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n", - view.type)) - return -1; - - return i915_ggtt_offset(vma); -} - static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id) { struct drm_device *dev = intel_crtc->base.dev; @@ -3447,7 +3430,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane, } I915_WRITE(PLANE_SURF(pipe, 0), - intel_fb_gtt_offset(fb, rotation) + surf_addr); + intel_plane_ggtt_offset(plane_state) + surf_addr); POSTING_READ(PLANE_SURF(pipe, 0)); } @@ -11576,7 +11559,7 @@ static void intel_unpin_work_fn(struct work_struct *__work) flush_work(&work->mmio_work); mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(work->old_fb, primary->state->rotation); + intel_unpin_fb_vma(work->old_vma); i915_gem_object_put(work->pending_flip_obj); mutex_unlock(&dev->struct_mutex); @@ -12286,8 +12269,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, goto cleanup_pending; } - work->gtt_offset = intel_fb_gtt_offset(fb, primary->state->rotation); - work->gtt_offset += intel_crtc->dspaddr_offset; + work->old_vma = to_intel_plane_state(primary->state)->vma; + to_intel_plane_state(primary->state)->vma = vma; + + work->gtt_offset = i915_ggtt_offset(vma) + intel_crtc->dspaddr_offset; work->rotation = crtc->primary->state->rotation; /* @@ -12340,7 +12325,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, cleanup_request: i915_add_request_no_flush(request); cleanup_unpin: - intel_unpin_fb_obj(fb, crtc->primary->state->rotation); + to_intel_plane_state(primary->state)->vma = work->old_vma; + intel_unpin_fb_vma(vma); cleanup_pending: atomic_dec(&intel_crtc->unpin_work_count); mutex_unlock(&dev->struct_mutex); @@ -14234,10 +14220,10 @@ intel_prepare_plane_fb(struct drm_plane *plane, struct i915_vma *vma; vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation); - if (IS_ERR(vma)) { - DRM_DEBUG_KMS("failed to pin object\n"); - return PTR_ERR(vma); - } + if (IS_ERR(vma)) + ret = PTR_ERR(vma); + + to_intel_plane_state(new_state)->vma = vma; } return 0; @@ -14256,19 +14242,12 @@ static void intel_cleanup_plane_fb(struct drm_plane *plane, struct drm_plane_state *old_state) { - struct drm_i915_private *dev_priv = to_i915(plane->dev); - struct intel_plane_state *old_intel_state; - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); - struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); - - old_intel_state = to_intel_plane_state(old_state); - - if (!obj && !old_obj) - return; + struct i915_vma *vma; - if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || - !INTEL_INFO(dev_priv)->cursor_needs_physical)) - intel_unpin_fb_obj(old_state->fb, old_state->rotation); + /* Should only called after a successful intel_prepare_plane_fb()! */ + vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma); + if (vma) + intel_unpin_fb_vma(vma); } static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state) @@ -15215,7 +15194,7 @@ intel_update_cursor_plane(struct drm_plane *plane, if (!obj) addr = 0; else if (!INTEL_INFO(dev_priv)->cursor_needs_physical) - addr = i915_gem_object_ggtt_offset(obj, NULL); + addr = i915_ggtt_offset(state->vma); else addr = obj->phys_handle->busaddr; @@ -17135,6 +17114,8 @@ void intel_modeset_gem_init(struct drm_device *dev) update_state_fb(c->primary); c->state->plane_mask &= ~(1 << drm_plane_index(c->primary)); } + + to_intel_plane_state(c->primary->state)->vma = vma; } } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index fff55d93ca73..d3d26d69930a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -372,6 +372,7 @@ struct intel_atomic_state { struct intel_plane_state { struct drm_plane_state base; struct drm_rect clip; + struct i915_vma *vma; struct { u32 offset; @@ -1046,6 +1047,7 @@ struct intel_flip_work { struct work_struct mmio_work; struct drm_crtc *crtc; + struct i915_vma *old_vma; struct drm_framebuffer *old_fb; struct drm_i915_gem_object *pending_flip_obj; struct drm_pending_vblank_event *event; @@ -1273,7 +1275,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, struct drm_modeset_acquire_ctx *ctx); struct i915_vma * intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation); -void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation); +void intel_unpin_fb_vma(struct i915_vma *vma); struct drm_framebuffer * __intel_framebuffer_create(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, @@ -1358,7 +1360,10 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state); int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state); -u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation); +static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state) +{ + return i915_ggtt_offset(state->vma); +} u32 skl_plane_ctl_format(uint32_t pixel_format); u32 skl_plane_ctl_tiling(uint64_t fb_modifier); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 62f215b12eb5..f3a1d6a5cabe 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -173,7 +173,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv) if (IS_I945GM(dev_priv)) fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */ fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT; - fbc_ctl |= params->fb.fence_reg; + fbc_ctl |= params->vma->fence->id; I915_WRITE(FBC_CONTROL, fbc_ctl); } @@ -193,8 +193,8 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv) else dpfc_ctl |= DPFC_CTL_LIMIT_1X; - if (params->fb.fence_reg != I915_FENCE_REG_NONE) { - dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fb.fence_reg; + if (params->vma->fence) { + dpfc_ctl |= DPFC_CTL_FENCE_EN | params->vma->fence->id; I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset); } else { I915_WRITE(DPFC_FENCE_YOFF, 0); @@ -251,13 +251,14 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv) break; } - if (params->fb.fence_reg != I915_FENCE_REG_NONE) { + if (params->vma->fence) { dpfc_ctl |= DPFC_CTL_FENCE_EN; if (IS_GEN5(dev_priv)) - dpfc_ctl |= params->fb.fence_reg; + dpfc_ctl |= params->vma->fence->id; if (IS_GEN6(dev_priv)) { I915_WRITE(SNB_DPFC_CTL_SA, - SNB_CPU_FENCE_ENABLE | params->fb.fence_reg); + SNB_CPU_FENCE_ENABLE | + params->vma->fence->id); I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset); } @@ -269,7 +270,8 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv) } I915_WRITE(ILK_DPFC_FENCE_YOFF, params->crtc.fence_y_offset); - I915_WRITE(ILK_FBC_RT_BASE, params->fb.ggtt_offset | ILK_FBC_RT_VALID); + I915_WRITE(ILK_FBC_RT_BASE, + i915_ggtt_offset(params->vma) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); @@ -319,10 +321,11 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv) break; } - if (params->fb.fence_reg != I915_FENCE_REG_NONE) { + if (params->vma->fence) { dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN; I915_WRITE(SNB_DPFC_CTL_SA, - SNB_CPU_FENCE_ENABLE | params->fb.fence_reg); + SNB_CPU_FENCE_ENABLE | + params->vma->fence->id); I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset); } else { I915_WRITE(SNB_DPFC_CTL_SA,0); @@ -727,14 +730,6 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) return effective_w <= max_w && effective_h <= max_h; } -/* XXX replace me when we have VMA tracking for intel_plane_state */ -static int get_fence_id(struct drm_framebuffer *fb) -{ - struct i915_vma *vma = i915_gem_object_to_ggtt(intel_fb_obj(fb), NULL); - - return vma && vma->fence ? vma->fence->id : I915_FENCE_REG_NONE; -} - static void intel_fbc_update_state_cache(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state) @@ -743,7 +738,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc, struct intel_fbc *fbc = &dev_priv->fbc; struct intel_fbc_state_cache *cache = &fbc->state_cache; struct drm_framebuffer *fb = plane_state->base.fb; - struct drm_i915_gem_object *obj; + + cache->vma = NULL; cache->crtc.mode_flags = crtc_state->base.adjusted_mode.flags; if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) @@ -758,16 +754,10 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc, if (!cache->plane.visible) return; - obj = intel_fb_obj(fb); - - /* FIXME: We lack the proper locking here, so only run this on the - * platforms that need. */ - if (IS_GEN(dev_priv, 5, 6)) - cache->fb.ilk_ggtt_offset = i915_gem_object_ggtt_offset(obj, NULL); cache->fb.pixel_format = fb->pixel_format; cache->fb.stride = fb->pitches[0]; - cache->fb.fence_reg = get_fence_id(fb); - cache->fb.tiling_mode = i915_gem_object_get_tiling(obj); + + cache->vma = plane_state->vma; } static bool intel_fbc_can_activate(struct intel_crtc *crtc) @@ -784,7 +774,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) return false; } - if (!cache->plane.visible) { + if (!cache->vma) { fbc->no_fbc_reason = "primary plane not visible"; return false; } @@ -807,8 +797,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) * so have no fence associated with it) due to aperture constaints * at the time of pinning. */ - if (cache->fb.tiling_mode != I915_TILING_X || - cache->fb.fence_reg == I915_FENCE_REG_NONE) { + if (!cache->vma->fence) { fbc->no_fbc_reason = "framebuffer not tiled or fenced"; return false; } @@ -888,17 +877,16 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc, * zero. */ memset(params, 0, sizeof(*params)); + params->vma = cache->vma; + params->crtc.pipe = crtc->pipe; params->crtc.plane = crtc->plane; params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc); params->fb.pixel_format = cache->fb.pixel_format; params->fb.stride = cache->fb.stride; - params->fb.fence_reg = cache->fb.fence_reg; params->cfb_size = intel_fbc_calculate_cfb_size(dev_priv, cache); - - params->fb.ggtt_offset = cache->fb.ilk_ggtt_offset; } static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1, diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index fc958d5ed0dc..ca29a5314f0e 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -284,7 +284,7 @@ static int intelfb_create(struct drm_fb_helper *helper, out_destroy_fbi: drm_fb_helper_release_fbi(helper); out_unpin: - intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0); + intel_unpin_fb_vma(vma); out_unlock: mutex_unlock(&dev->struct_mutex); return ret; @@ -549,7 +549,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) if (ifbdev->fb) { mutex_lock(&ifbdev->helper.dev->struct_mutex); - intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0); + intel_unpin_fb_vma(ifbdev->vma); mutex_unlock(&ifbdev->helper.dev->struct_mutex); drm_framebuffer_remove(&ifbdev->fb->base); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 8b2fc67acbba..ae8ce621e3fc 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -281,7 +281,7 @@ skl_update_plane(struct drm_plane *drm_plane, I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl); I915_WRITE(PLANE_SURF(pipe, plane), - intel_fb_gtt_offset(fb, rotation) + surf_addr); + intel_plane_ggtt_offset(plane_state) + surf_addr); POSTING_READ(PLANE_SURF(pipe, plane)); } @@ -476,7 +476,7 @@ vlv_update_plane(struct drm_plane *dplane, I915_WRITE(SPSIZE(pipe, plane), (crtc_h << 16) | crtc_w); I915_WRITE(SPCNTR(pipe, plane), sprctl); I915_WRITE(SPSURF(pipe, plane), - intel_fb_gtt_offset(fb, rotation) + sprsurf_offset); + intel_plane_ggtt_offset(plane_state) + sprsurf_offset); POSTING_READ(SPSURF(pipe, plane)); } @@ -612,7 +612,7 @@ ivb_update_plane(struct drm_plane *plane, I915_WRITE(SPRSCALE(pipe), sprscale); I915_WRITE(SPRCTL(pipe), sprctl); I915_WRITE(SPRSURF(pipe), - intel_fb_gtt_offset(fb, rotation) + sprsurf_offset); + intel_plane_ggtt_offset(plane_state) + sprsurf_offset); POSTING_READ(SPRSURF(pipe)); } @@ -739,7 +739,7 @@ ilk_update_plane(struct drm_plane *plane, I915_WRITE(DVSSCALE(pipe), dvsscale); I915_WRITE(DVSCNTR(pipe), dvscntr); I915_WRITE(DVSSURF(pipe), - intel_fb_gtt_offset(fb, rotation) + dvssurf_offset); + intel_plane_ggtt_offset(plane_state) + dvssurf_offset); POSTING_READ(DVSSURF(pipe)); } -- 2.10.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state 2016-11-15 8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson @ 2016-11-15 9:52 ` Daniel Vetter 2016-11-15 10:02 ` Chris Wilson 2016-11-15 10:24 ` Daniel Vetter 1 sibling, 1 reply; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 9:52 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote: > With atomic plane states we are able to track an allocation right from > preparation, during use and through to the final free after being > swapped out for a new plane. We can couple the VMA we pin for the > framebuffer (and its rotation) to this lifetime and avoid all the clumsy > lookups in between. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> A few questions and comments below. Of course review assumes some form of patch 2 lands first (but leaning towards just reverting the core one for that). -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 16 ++--- > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 + > drivers/gpu/drm/i915/intel_display.c | 97 +++++++++++++------------------ > drivers/gpu/drm/i915/intel_drv.h | 9 ++- > drivers/gpu/drm/i915/intel_fbc.c | 52 +++++++---------- > drivers/gpu/drm/i915/intel_fbdev.c | 4 +- > drivers/gpu/drm/i915/intel_sprite.c | 8 +-- > 7 files changed, 78 insertions(+), 110 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4e7148a3ee8b..2bc285e2ff82 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -989,6 +989,8 @@ struct intel_fbc { > struct work_struct underrun_work; > > struct intel_fbc_state_cache { > + struct i915_vma *vma; > + > struct { > unsigned int mode_flags; > uint32_t hsw_bdw_pixel_rate; > @@ -1002,15 +1004,14 @@ struct intel_fbc { > } plane; > > struct { > - u64 ilk_ggtt_offset; > uint32_t pixel_format; > unsigned int stride; > - int fence_reg; > - unsigned int tiling_mode; > } fb; > } state_cache; > > struct intel_fbc_reg_params { > + struct i915_vma *vma; > + > struct { > enum pipe pipe; > enum plane plane; > @@ -1018,10 +1019,8 @@ struct intel_fbc { > } crtc; > > struct { > - u64 ggtt_offset; > uint32_t pixel_format; > unsigned int stride; > - int fence_reg; > } fb; > > int cfb_size; > @@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj, > return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view); > } > > -static inline unsigned long > -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o, > - const struct i915_ggtt_view *view) > -{ > - return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view)); > -} > - > /* i915_gem_fence_reg.c */ > int __must_check i915_vma_get_fence(struct i915_vma *vma); > int __must_check i915_vma_put_fence(struct i915_vma *vma); > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 89a34350a35d..5a86e0d52409 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane) > > __drm_atomic_helper_plane_duplicate_state(plane, state); > > + intel_state->vma = NULL; > + > return state; > } > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index dca1e0b512e5..cb52116f8577 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation) > i915_vma_pin_fence(vma); > } > > + i915_vma_get(vma); I'm not entirely clear on why we no need a refcount for this? Vestige code from when you wanted to refcount plane_state->fb like other refcounted state pointers? If we restrict the lifetime to just in between prepare_plane/cleanup_plane like you do I don't think this is needed ... > err: > intel_runtime_pm_put(dev_priv); > return vma; > } > > -void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation) > +void intel_unpin_fb_vma(struct i915_vma *vma) > { > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct i915_ggtt_view view; > - struct i915_vma *vma; > - > - WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex)); > - > - intel_fill_fb_ggtt_view(&view, fb, rotation); > - vma = i915_gem_object_to_ggtt(obj, &view); > + lockdep_assert_held(&vma->vm->dev->struct_mutex); > > i915_vma_unpin_fence(vma); > i915_gem_object_unpin_from_display_plane(vma); > + i915_vma_put(vma); > } > > static int intel_fb_pitch(const struct drm_framebuffer *fb, int plane, > @@ -2752,7 +2747,6 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > struct drm_device *dev = intel_crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct drm_crtc *c; > - struct intel_crtc *i; > struct drm_i915_gem_object *obj; > struct drm_plane *primary = intel_crtc->base.primary; > struct drm_plane_state *plane_state = primary->state; > @@ -2777,20 +2771,20 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > * an fb with another CRTC instead > */ > for_each_crtc(dev, c) { > - i = to_intel_crtc(c); > + struct intel_plane_state *state; > > if (c == &intel_crtc->base) > continue; > > - if (!i->active) > + if (!to_intel_crtc(c)->active) > continue; > > - fb = c->primary->fb; > - if (!fb) > + state = to_intel_plane_state(c->primary->state); > + if (!state->vma) > continue; > > - obj = intel_fb_obj(fb); > - if (i915_gem_object_ggtt_offset(obj, NULL) == plane_config->base) { > + if (i915_ggtt_offset(state->vma) == plane_config->base) { > + fb = c->primary->fb; > drm_framebuffer_reference(fb); > goto valid_fb; > } > @@ -2830,6 +2824,12 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > drm_framebuffer_reference(fb); > primary->fb = primary->state->fb = fb; > + > + mutex_lock(&dev->struct_mutex); > + intel_state->vma = > + intel_pin_and_fence_fb_obj(fb, primary->state->rotation); > + mutex_unlock(&dev->struct_mutex); > + > primary->crtc = primary->state->crtc = &intel_crtc->base; > intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary)); > atomic_or(to_intel_plane(primary)->frontbuffer_bit, > @@ -3104,13 +3104,13 @@ static void i9xx_update_primary_plane(struct drm_plane *primary, > I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); > if (INTEL_INFO(dev)->gen >= 4) { > I915_WRITE(DSPSURF(plane), > - intel_fb_gtt_offset(fb, rotation) + > + intel_plane_ggtt_offset(plane_state) + > intel_crtc->dspaddr_offset); > I915_WRITE(DSPTILEOFF(plane), (y << 16) | x); > I915_WRITE(DSPLINOFF(plane), linear_offset); > } else { > I915_WRITE(DSPADDR(plane), > - intel_fb_gtt_offset(fb, rotation) + > + intel_plane_ggtt_offset(plane_state) + > intel_crtc->dspaddr_offset); > } > POSTING_READ(reg); > @@ -3207,7 +3207,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary, > > I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]); > I915_WRITE(DSPSURF(plane), > - intel_fb_gtt_offset(fb, rotation) + > + intel_plane_ggtt_offset(plane_state) + > intel_crtc->dspaddr_offset); > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > I915_WRITE(DSPOFFSET(plane), (y << 16) | x); > @@ -3230,23 +3230,6 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv, > } > } > > -u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, > - unsigned int rotation) > -{ > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct i915_ggtt_view view; > - struct i915_vma *vma; > - > - intel_fill_fb_ggtt_view(&view, fb, rotation); > - > - vma = i915_gem_object_to_ggtt(obj, &view); > - if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n", > - view.type)) > - return -1; > - > - return i915_ggtt_offset(vma); > -} > - > static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id) > { > struct drm_device *dev = intel_crtc->base.dev; > @@ -3447,7 +3430,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > } > > I915_WRITE(PLANE_SURF(pipe, 0), > - intel_fb_gtt_offset(fb, rotation) + surf_addr); > + intel_plane_ggtt_offset(plane_state) + surf_addr); > > POSTING_READ(PLANE_SURF(pipe, 0)); > } > @@ -11576,7 +11559,7 @@ static void intel_unpin_work_fn(struct work_struct *__work) > flush_work(&work->mmio_work); > > mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(work->old_fb, primary->state->rotation); > + intel_unpin_fb_vma(work->old_vma); > i915_gem_object_put(work->pending_flip_obj); > mutex_unlock(&dev->struct_mutex); > > @@ -12286,8 +12269,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > goto cleanup_pending; > } > > - work->gtt_offset = intel_fb_gtt_offset(fb, primary->state->rotation); > - work->gtt_offset += intel_crtc->dspaddr_offset; > + work->old_vma = to_intel_plane_state(primary->state)->vma; > + to_intel_plane_state(primary->state)->vma = vma; > + > + work->gtt_offset = i915_ggtt_offset(vma) + intel_crtc->dspaddr_offset; > work->rotation = crtc->primary->state->rotation; > > /* > @@ -12340,7 +12325,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > cleanup_request: > i915_add_request_no_flush(request); > cleanup_unpin: > - intel_unpin_fb_obj(fb, crtc->primary->state->rotation); > + to_intel_plane_state(primary->state)->vma = work->old_vma; > + intel_unpin_fb_vma(vma); > cleanup_pending: > atomic_dec(&intel_crtc->unpin_work_count); > mutex_unlock(&dev->struct_mutex); > @@ -14234,10 +14220,10 @@ intel_prepare_plane_fb(struct drm_plane *plane, > struct i915_vma *vma; > > vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation); > - if (IS_ERR(vma)) { > - DRM_DEBUG_KMS("failed to pin object\n"); > - return PTR_ERR(vma); > - } > + if (IS_ERR(vma)) > + ret = PTR_ERR(vma); > + > + to_intel_plane_state(new_state)->vma = vma; > } > > return 0; > @@ -14256,19 +14242,12 @@ static void > intel_cleanup_plane_fb(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > - struct drm_i915_private *dev_priv = to_i915(plane->dev); > - struct intel_plane_state *old_intel_state; > - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); > - struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); > - > - old_intel_state = to_intel_plane_state(old_state); > - > - if (!obj && !old_obj) > - return; > + struct i915_vma *vma; > > - if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || > - !INTEL_INFO(dev_priv)->cursor_needs_physical)) > - intel_unpin_fb_obj(old_state->fb, old_state->rotation); > + /* Should only called after a successful intel_prepare_plane_fb()! */ > + vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma); > + if (vma) WARN_ON(!vma); maybe even instead of the comment? > + intel_unpin_fb_vma(vma); > } > > static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state) > @@ -15215,7 +15194,7 @@ intel_update_cursor_plane(struct drm_plane *plane, > if (!obj) > addr = 0; > else if (!INTEL_INFO(dev_priv)->cursor_needs_physical) > - addr = i915_gem_object_ggtt_offset(obj, NULL); > + addr = i915_ggtt_offset(state->vma); > else > addr = obj->phys_handle->busaddr; > > @@ -17135,6 +17114,8 @@ void intel_modeset_gem_init(struct drm_device *dev) > update_state_fb(c->primary); > c->state->plane_mask &= ~(1 << drm_plane_index(c->primary)); > } > + > + to_intel_plane_state(c->primary->state)->vma = vma; > } > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index fff55d93ca73..d3d26d69930a 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -372,6 +372,7 @@ struct intel_atomic_state { > struct intel_plane_state { > struct drm_plane_state base; > struct drm_rect clip; I think a comment here about the special refcounting rules for this would be good, e.g. "Note that despite that vmas are refcounted using i915_vma_get() and i915_vma_put() we don't refcount them as part of the state like other objects. Instead their lifetime is only between prepare_planes and cleanup_planes, and hence vma must be set to NULL when duplicating the plane state." > + struct i915_vma *vma; > > struct { > u32 offset; > @@ -1046,6 +1047,7 @@ struct intel_flip_work { > struct work_struct mmio_work; > > struct drm_crtc *crtc; > + struct i915_vma *old_vma; > struct drm_framebuffer *old_fb; > struct drm_i915_gem_object *pending_flip_obj; > struct drm_pending_vblank_event *event; > @@ -1273,7 +1275,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, > struct drm_modeset_acquire_ctx *ctx); > struct i915_vma * > intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation); > -void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation); > +void intel_unpin_fb_vma(struct i915_vma *vma); > struct drm_framebuffer * > __intel_framebuffer_create(struct drm_device *dev, > struct drm_mode_fb_cmd2 *mode_cmd, > @@ -1358,7 +1360,10 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, > int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state); > int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state); > > -u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation); > +static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state) > +{ > + return i915_ggtt_offset(state->vma); > +} > > u32 skl_plane_ctl_format(uint32_t pixel_format); > u32 skl_plane_ctl_tiling(uint64_t fb_modifier); > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index 62f215b12eb5..f3a1d6a5cabe 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -173,7 +173,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv) > if (IS_I945GM(dev_priv)) > fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */ > fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT; > - fbc_ctl |= params->fb.fence_reg; > + fbc_ctl |= params->vma->fence->id; > I915_WRITE(FBC_CONTROL, fbc_ctl); > } > > @@ -193,8 +193,8 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv) > else > dpfc_ctl |= DPFC_CTL_LIMIT_1X; > > - if (params->fb.fence_reg != I915_FENCE_REG_NONE) { > - dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fb.fence_reg; > + if (params->vma->fence) { > + dpfc_ctl |= DPFC_CTL_FENCE_EN | params->vma->fence->id; > I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset); > } else { > I915_WRITE(DPFC_FENCE_YOFF, 0); > @@ -251,13 +251,14 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv) > break; > } > > - if (params->fb.fence_reg != I915_FENCE_REG_NONE) { > + if (params->vma->fence) { > dpfc_ctl |= DPFC_CTL_FENCE_EN; > if (IS_GEN5(dev_priv)) > - dpfc_ctl |= params->fb.fence_reg; > + dpfc_ctl |= params->vma->fence->id; > if (IS_GEN6(dev_priv)) { > I915_WRITE(SNB_DPFC_CTL_SA, > - SNB_CPU_FENCE_ENABLE | params->fb.fence_reg); > + SNB_CPU_FENCE_ENABLE | > + params->vma->fence->id); > I915_WRITE(DPFC_CPU_FENCE_OFFSET, > params->crtc.fence_y_offset); > } > @@ -269,7 +270,8 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv) > } > > I915_WRITE(ILK_DPFC_FENCE_YOFF, params->crtc.fence_y_offset); > - I915_WRITE(ILK_FBC_RT_BASE, params->fb.ggtt_offset | ILK_FBC_RT_VALID); > + I915_WRITE(ILK_FBC_RT_BASE, > + i915_ggtt_offset(params->vma) | ILK_FBC_RT_VALID); > /* enable it... */ > I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); > > @@ -319,10 +321,11 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv) > break; > } > > - if (params->fb.fence_reg != I915_FENCE_REG_NONE) { > + if (params->vma->fence) { > dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN; > I915_WRITE(SNB_DPFC_CTL_SA, > - SNB_CPU_FENCE_ENABLE | params->fb.fence_reg); > + SNB_CPU_FENCE_ENABLE | > + params->vma->fence->id); > I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset); > } else { > I915_WRITE(SNB_DPFC_CTL_SA,0); > @@ -727,14 +730,6 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) > return effective_w <= max_w && effective_h <= max_h; > } > > -/* XXX replace me when we have VMA tracking for intel_plane_state */ > -static int get_fence_id(struct drm_framebuffer *fb) > -{ > - struct i915_vma *vma = i915_gem_object_to_ggtt(intel_fb_obj(fb), NULL); > - > - return vma && vma->fence ? vma->fence->id : I915_FENCE_REG_NONE; > -} > - > static void intel_fbc_update_state_cache(struct intel_crtc *crtc, > struct intel_crtc_state *crtc_state, > struct intel_plane_state *plane_state) > @@ -743,7 +738,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc, > struct intel_fbc *fbc = &dev_priv->fbc; > struct intel_fbc_state_cache *cache = &fbc->state_cache; > struct drm_framebuffer *fb = plane_state->base.fb; > - struct drm_i915_gem_object *obj; > + > + cache->vma = NULL; > > cache->crtc.mode_flags = crtc_state->base.adjusted_mode.flags; > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > @@ -758,16 +754,10 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc, > if (!cache->plane.visible) > return; > > - obj = intel_fb_obj(fb); > - > - /* FIXME: We lack the proper locking here, so only run this on the > - * platforms that need. */ > - if (IS_GEN(dev_priv, 5, 6)) > - cache->fb.ilk_ggtt_offset = i915_gem_object_ggtt_offset(obj, NULL); > cache->fb.pixel_format = fb->pixel_format; > cache->fb.stride = fb->pitches[0]; > - cache->fb.fence_reg = get_fence_id(fb); > - cache->fb.tiling_mode = i915_gem_object_get_tiling(obj); > + > + cache->vma = plane_state->vma; > } > > static bool intel_fbc_can_activate(struct intel_crtc *crtc) > @@ -784,7 +774,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) > return false; > } > > - if (!cache->plane.visible) { > + if (!cache->vma) { > fbc->no_fbc_reason = "primary plane not visible"; > return false; > } > @@ -807,8 +797,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) > * so have no fence associated with it) due to aperture constaints > * at the time of pinning. > */ > - if (cache->fb.tiling_mode != I915_TILING_X || > - cache->fb.fence_reg == I915_FENCE_REG_NONE) { > + if (!cache->vma->fence) { > fbc->no_fbc_reason = "framebuffer not tiled or fenced"; > return false; > } > @@ -888,17 +877,16 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc, > * zero. */ > memset(params, 0, sizeof(*params)); > > + params->vma = cache->vma; > + > params->crtc.pipe = crtc->pipe; > params->crtc.plane = crtc->plane; > params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc); > > params->fb.pixel_format = cache->fb.pixel_format; > params->fb.stride = cache->fb.stride; > - params->fb.fence_reg = cache->fb.fence_reg; > > params->cfb_size = intel_fbc_calculate_cfb_size(dev_priv, cache); > - > - params->fb.ggtt_offset = cache->fb.ilk_ggtt_offset; > } > > static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1, > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index fc958d5ed0dc..ca29a5314f0e 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -284,7 +284,7 @@ static int intelfb_create(struct drm_fb_helper *helper, > out_destroy_fbi: > drm_fb_helper_release_fbi(helper); > out_unpin: > - intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0); > + intel_unpin_fb_vma(vma); > out_unlock: > mutex_unlock(&dev->struct_mutex); > return ret; > @@ -549,7 +549,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) > > if (ifbdev->fb) { > mutex_lock(&ifbdev->helper.dev->struct_mutex); > - intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0); > + intel_unpin_fb_vma(ifbdev->vma); > mutex_unlock(&ifbdev->helper.dev->struct_mutex); > > drm_framebuffer_remove(&ifbdev->fb->base); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 8b2fc67acbba..ae8ce621e3fc 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -281,7 +281,7 @@ skl_update_plane(struct drm_plane *drm_plane, > > I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl); > I915_WRITE(PLANE_SURF(pipe, plane), > - intel_fb_gtt_offset(fb, rotation) + surf_addr); > + intel_plane_ggtt_offset(plane_state) + surf_addr); > POSTING_READ(PLANE_SURF(pipe, plane)); > } > > @@ -476,7 +476,7 @@ vlv_update_plane(struct drm_plane *dplane, > I915_WRITE(SPSIZE(pipe, plane), (crtc_h << 16) | crtc_w); > I915_WRITE(SPCNTR(pipe, plane), sprctl); > I915_WRITE(SPSURF(pipe, plane), > - intel_fb_gtt_offset(fb, rotation) + sprsurf_offset); > + intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > POSTING_READ(SPSURF(pipe, plane)); > } > > @@ -612,7 +612,7 @@ ivb_update_plane(struct drm_plane *plane, > I915_WRITE(SPRSCALE(pipe), sprscale); > I915_WRITE(SPRCTL(pipe), sprctl); > I915_WRITE(SPRSURF(pipe), > - intel_fb_gtt_offset(fb, rotation) + sprsurf_offset); > + intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > POSTING_READ(SPRSURF(pipe)); > } > > @@ -739,7 +739,7 @@ ilk_update_plane(struct drm_plane *plane, > I915_WRITE(DVSSCALE(pipe), dvsscale); > I915_WRITE(DVSCNTR(pipe), dvscntr); > I915_WRITE(DVSSURF(pipe), > - intel_fb_gtt_offset(fb, rotation) + dvssurf_offset); > + intel_plane_ggtt_offset(plane_state) + dvssurf_offset); > POSTING_READ(DVSSURF(pipe)); > } > > -- > 2.10.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state 2016-11-15 9:52 ` Daniel Vetter @ 2016-11-15 10:02 ` Chris Wilson 2016-11-15 10:13 ` Daniel Vetter 0 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2016-11-15 10:02 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Tue, Nov 15, 2016 at 10:52:00AM +0100, Daniel Vetter wrote: > On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote: > > With atomic plane states we are able to track an allocation right from > > preparation, during use and through to the final free after being > > swapped out for a new plane. We can couple the VMA we pin for the > > framebuffer (and its rotation) to this lifetime and avoid all the clumsy > > lookups in between. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > A few questions and comments below. Of course review assumes some form of > patch 2 lands first (but leaning towards just reverting the core one for > that). > -Daniel > > > --- > > drivers/gpu/drm/i915/i915_drv.h | 16 ++--- > > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 + > > drivers/gpu/drm/i915/intel_display.c | 97 +++++++++++++------------------ > > drivers/gpu/drm/i915/intel_drv.h | 9 ++- > > drivers/gpu/drm/i915/intel_fbc.c | 52 +++++++---------- > > drivers/gpu/drm/i915/intel_fbdev.c | 4 +- > > drivers/gpu/drm/i915/intel_sprite.c | 8 +-- > > 7 files changed, 78 insertions(+), 110 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 4e7148a3ee8b..2bc285e2ff82 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -989,6 +989,8 @@ struct intel_fbc { > > struct work_struct underrun_work; > > > > struct intel_fbc_state_cache { > > + struct i915_vma *vma; > > + > > struct { > > unsigned int mode_flags; > > uint32_t hsw_bdw_pixel_rate; > > @@ -1002,15 +1004,14 @@ struct intel_fbc { > > } plane; > > > > struct { > > - u64 ilk_ggtt_offset; > > uint32_t pixel_format; > > unsigned int stride; > > - int fence_reg; > > - unsigned int tiling_mode; > > } fb; > > } state_cache; > > > > struct intel_fbc_reg_params { > > + struct i915_vma *vma; > > + > > struct { > > enum pipe pipe; > > enum plane plane; > > @@ -1018,10 +1019,8 @@ struct intel_fbc { > > } crtc; > > > > struct { > > - u64 ggtt_offset; > > uint32_t pixel_format; > > unsigned int stride; > > - int fence_reg; > > } fb; > > > > int cfb_size; > > @@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj, > > return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view); > > } > > > > -static inline unsigned long > > -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o, > > - const struct i915_ggtt_view *view) > > -{ > > - return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view)); > > -} > > - > > /* i915_gem_fence_reg.c */ > > int __must_check i915_vma_get_fence(struct i915_vma *vma); > > int __must_check i915_vma_put_fence(struct i915_vma *vma); > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > > index 89a34350a35d..5a86e0d52409 100644 > > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > > @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane) > > > > __drm_atomic_helper_plane_duplicate_state(plane, state); > > > > + intel_state->vma = NULL; > > + > > return state; > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index dca1e0b512e5..cb52116f8577 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation) > > i915_vma_pin_fence(vma); > > } > > > > + i915_vma_get(vma); > > I'm not entirely clear on why we no need a refcount for this? Vestige code > from when you wanted to refcount plane_state->fb like other refcounted > state pointers? If we restrict the lifetime to just in between > prepare_plane/cleanup_plane like you do I don't think this is needed ... The lifetime is quite broad since we aquire it on takeover and it is not finally released until plane state destruction. It's not as simple as saying they only exist between two atomic commits :| Since the vma is being tracked outside of the parent fb (and it is independent since it is a view of that fb), it is safer to hold a reference. > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index fff55d93ca73..d3d26d69930a 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -372,6 +372,7 @@ struct intel_atomic_state { > > struct intel_plane_state { > > struct drm_plane_state base; > > struct drm_rect clip; > > I think a comment here about the special refcounting rules for this would > be good, e.g. > > "Note that despite that vmas are refcounted using i915_vma_get() and > i915_vma_put() we don't refcount them as part of the state like other > objects. Instead their lifetime is only between prepare_planes and > cleanup_planes, and hence vma must be set to NULL when duplicating the > plane state." That would be false :-p -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state 2016-11-15 10:02 ` Chris Wilson @ 2016-11-15 10:13 ` Daniel Vetter 0 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 10:13 UTC (permalink / raw) To: Chris Wilson, Daniel Vetter, intel-gfx On Tue, Nov 15, 2016 at 10:02:23AM +0000, Chris Wilson wrote: > On Tue, Nov 15, 2016 at 10:52:00AM +0100, Daniel Vetter wrote: > > On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote: > > > With atomic plane states we are able to track an allocation right from > > > preparation, during use and through to the final free after being > > > swapped out for a new plane. We can couple the VMA we pin for the > > > framebuffer (and its rotation) to this lifetime and avoid all the clumsy > > > lookups in between. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > A few questions and comments below. Of course review assumes some form of > > patch 2 lands first (but leaning towards just reverting the core one for > > that). > > -Daniel > > > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 16 ++--- > > > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 + > > > drivers/gpu/drm/i915/intel_display.c | 97 +++++++++++++------------------ > > > drivers/gpu/drm/i915/intel_drv.h | 9 ++- > > > drivers/gpu/drm/i915/intel_fbc.c | 52 +++++++---------- > > > drivers/gpu/drm/i915/intel_fbdev.c | 4 +- > > > drivers/gpu/drm/i915/intel_sprite.c | 8 +-- > > > 7 files changed, 78 insertions(+), 110 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index 4e7148a3ee8b..2bc285e2ff82 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -989,6 +989,8 @@ struct intel_fbc { > > > struct work_struct underrun_work; > > > > > > struct intel_fbc_state_cache { > > > + struct i915_vma *vma; > > > + > > > struct { > > > unsigned int mode_flags; > > > uint32_t hsw_bdw_pixel_rate; > > > @@ -1002,15 +1004,14 @@ struct intel_fbc { > > > } plane; > > > > > > struct { > > > - u64 ilk_ggtt_offset; > > > uint32_t pixel_format; > > > unsigned int stride; > > > - int fence_reg; > > > - unsigned int tiling_mode; > > > } fb; > > > } state_cache; > > > > > > struct intel_fbc_reg_params { > > > + struct i915_vma *vma; > > > + > > > struct { > > > enum pipe pipe; > > > enum plane plane; > > > @@ -1018,10 +1019,8 @@ struct intel_fbc { > > > } crtc; > > > > > > struct { > > > - u64 ggtt_offset; > > > uint32_t pixel_format; > > > unsigned int stride; > > > - int fence_reg; > > > } fb; > > > > > > int cfb_size; > > > @@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj, > > > return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view); > > > } > > > > > > -static inline unsigned long > > > -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o, > > > - const struct i915_ggtt_view *view) > > > -{ > > > - return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view)); > > > -} > > > - > > > /* i915_gem_fence_reg.c */ > > > int __must_check i915_vma_get_fence(struct i915_vma *vma); > > > int __must_check i915_vma_put_fence(struct i915_vma *vma); > > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > > > index 89a34350a35d..5a86e0d52409 100644 > > > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > > > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > > > @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane) > > > > > > __drm_atomic_helper_plane_duplicate_state(plane, state); > > > > > > + intel_state->vma = NULL; > > > + > > > return state; > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index dca1e0b512e5..cb52116f8577 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation) > > > i915_vma_pin_fence(vma); > > > } > > > > > > + i915_vma_get(vma); > > > > I'm not entirely clear on why we no need a refcount for this? Vestige code > > from when you wanted to refcount plane_state->fb like other refcounted > > state pointers? If we restrict the lifetime to just in between > > prepare_plane/cleanup_plane like you do I don't think this is needed ... > > The lifetime is quite broad since we aquire it on takeover and it is not > finally released until plane state destruction. It's not as simple as > saying they only exist between two atomic commits :| Since the vma is > being tracked outside of the parent fb (and it is independent since it > is a view of that fb), it is safer to hold a reference. Takeover = prepare_planes done by someone else. And assuming our current refcounting is correct we shouldn't need this additional refcount either. > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index fff55d93ca73..d3d26d69930a 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -372,6 +372,7 @@ struct intel_atomic_state { > > > struct intel_plane_state { > > > struct drm_plane_state base; > > > struct drm_rect clip; > > > > I think a comment here about the special refcounting rules for this would > > be good, e.g. > > > > "Note that despite that vmas are refcounted using i915_vma_get() and > > i915_vma_put() we don't refcount them as part of the state like other > > objects. Instead their lifetime is only between prepare_planes and > > cleanup_planes, and hence vma must be set to NULL when duplicating the > > plane state." > > That would be false :-p What's the false part? Ignoring the takeover corner case, where we have lots of other cases where we need to adjust refcounts and pointers and make sure it all looks like boot-up never happened, and it matches state that a normal atomic commit could have produced. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state 2016-11-15 8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson 2016-11-15 9:52 ` Daniel Vetter @ 2016-11-15 10:24 ` Daniel Vetter 1 sibling, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 10:24 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote: > @@ -12340,7 +12325,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > cleanup_request: > i915_add_request_no_flush(request); > cleanup_unpin: > - intel_unpin_fb_obj(fb, crtc->primary->state->rotation); > + to_intel_plane_state(primary->state)->vma = work->old_vma; > + intel_unpin_fb_vma(vma); > cleanup_pending: > atomic_dec(&intel_crtc->unpin_work_count); > mutex_unlock(&dev->struct_mutex); > @@ -14234,10 +14220,10 @@ intel_prepare_plane_fb(struct drm_plane *plane, > struct i915_vma *vma; > > vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation); > - if (IS_ERR(vma)) { > - DRM_DEBUG_KMS("failed to pin object\n"); > - return PTR_ERR(vma); > - } > + if (IS_ERR(vma)) > + ret = PTR_ERR(vma); Uncessary change above that you then go ahead and partially undo in the next patch. Please remove. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() 2016-11-15 8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson 2016-11-15 8:58 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson 2016-11-15 8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson @ 2016-11-15 8:58 ` Chris Wilson 2016-11-15 10:23 ` Daniel Vetter 2016-11-15 8:58 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson ` (2 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2016-11-15 8:58 UTC (permalink / raw) To: intel-gfx Just a quick tidy now to make the next patch neater. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cb52116f8577..4d578dc6d23f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14130,6 +14130,17 @@ static int intel_atomic_check(struct drm_device *dev, return calc_watermark_data(state); } +static bool old_plane_needs_modeset(struct drm_plane *plane, + struct drm_plane_state *new_state) +{ + struct drm_crtc_state *crtc_state; + + crtc_state = drm_atomic_get_existing_crtc_state(new_state->state, + plane->state->crtc); + + return needs_modeset(crtc_state); +} + /** * intel_prepare_plane_fb - Prepare fb for usage on plane * @plane: drm plane to prepare for @@ -14153,16 +14164,11 @@ intel_prepare_plane_fb(struct drm_plane *plane, struct drm_i915_private *dev_priv = to_i915(plane->dev); struct drm_framebuffer *fb = new_state->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); int ret; - if (!obj && !old_obj) - return 0; - - if (old_obj) { - struct drm_crtc_state *crtc_state = - drm_atomic_get_existing_crtc_state(new_state->state, - plane->state->crtc); + if (plane->state->fb && old_plane_needs_modeset(plane, new_state)) { + struct drm_i915_gem_object *old_obj = + intel_fb_obj(plane->state->fb); /* Big Hammer, we also need to ensure that any pending * MI_WAIT_FOR_EVENT inside a user batch buffer on the @@ -14175,14 +14181,12 @@ intel_prepare_plane_fb(struct drm_plane *plane, * This should only fail upon a hung GPU, in which case we * can safely continue. */ - if (needs_modeset(crtc_state)) { - ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, - old_obj->resv, NULL, - false, 0, - GFP_KERNEL); - if (ret < 0) - return ret; - } + ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, + old_obj->resv, NULL, + false, 0, + GFP_KERNEL); + if (ret < 0) + return ret; } if (new_state->fence) { /* explicit fencing */ @@ -14221,7 +14225,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation); if (IS_ERR(vma)) - ret = PTR_ERR(vma); + return PTR_ERR(vma); to_intel_plane_state(new_state)->vma = vma; } -- 2.10.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() 2016-11-15 8:58 ` [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() Chris Wilson @ 2016-11-15 10:23 ` Daniel Vetter 0 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 10:23 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Nov 15, 2016 at 08:58:16AM +0000, Chris Wilson wrote: > Just a quick tidy now to make the next patch neater. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++---------------- > 1 file changed, 21 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index cb52116f8577..4d578dc6d23f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14130,6 +14130,17 @@ static int intel_atomic_check(struct drm_device *dev, > return calc_watermark_data(state); > } > > +static bool old_plane_needs_modeset(struct drm_plane *plane, > + struct drm_plane_state *new_state) > +{ > + struct drm_crtc_state *crtc_state; > + > + crtc_state = drm_atomic_get_existing_crtc_state(new_state->state, > + plane->state->crtc); > + > + return needs_modeset(crtc_state); > +} > + > /** > * intel_prepare_plane_fb - Prepare fb for usage on plane > * @plane: drm plane to prepare for > @@ -14153,16 +14164,11 @@ intel_prepare_plane_fb(struct drm_plane *plane, > struct drm_i915_private *dev_priv = to_i915(plane->dev); > struct drm_framebuffer *fb = new_state->fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); > int ret; > > - if (!obj && !old_obj) > - return 0; > - > - if (old_obj) { > - struct drm_crtc_state *crtc_state = > - drm_atomic_get_existing_crtc_state(new_state->state, > - plane->state->crtc); > + if (plane->state->fb && old_plane_needs_modeset(plane, new_state)) { > + struct drm_i915_gem_object *old_obj = > + intel_fb_obj(plane->state->fb); > > /* Big Hammer, we also need to ensure that any pending > * MI_WAIT_FOR_EVENT inside a user batch buffer on the > @@ -14175,14 +14181,12 @@ intel_prepare_plane_fb(struct drm_plane *plane, > * This should only fail upon a hung GPU, in which case we > * can safely continue. > */ > - if (needs_modeset(crtc_state)) { > - ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, > - old_obj->resv, NULL, > - false, 0, > - GFP_KERNEL); > - if (ret < 0) > - return ret; > - } > + ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, > + old_obj->resv, NULL, > + false, 0, > + GFP_KERNEL); > + if (ret < 0) > + return ret; > } > > if (new_state->fence) { /* explicit fencing */ > @@ -14221,7 +14225,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, > > vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation); > if (IS_ERR(vma)) > - ret = PTR_ERR(vma); > + return PTR_ERR(vma); I don't have this one here, I guess you have some different baseline. Current code already has the direct return (but also a debug output on top). Looking closer that's a mistake in the preceeding patch, which also drops the debug output. I didn't spot that, so please fix that patch by dropping the hunk and then you can drop this one here. With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > to_intel_plane_state(new_state)->vma = vma; > } > -- > 2.10.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed 2016-11-15 8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson ` (2 preceding siblings ...) 2016-11-15 8:58 ` [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() Chris Wilson @ 2016-11-15 8:58 ` Chris Wilson 2016-11-15 10:15 ` Daniel Vetter 2016-11-15 9:15 ` ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Patchwork 2016-11-15 9:28 ` [PATCH 1/5] " Daniel Vetter 5 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2016-11-15 8:58 UTC (permalink / raw) To: intel-gfx Since an fb may have multiple VMA (due to rotations etc), we need to wait a vblank and unpin the old VMA not if the fb itself is changed, but if the underlying VMA is changed. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4d578dc6d23f..a92adce033ab 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12476,9 +12476,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, if (!was_visible && !visible) return 0; - if (fb != old_plane_state->base.fb) - pipe_config->fb_changed = true; - turn_off = was_visible && (!visible || mode_changed); turn_on = visible && (!was_visible || mode_changed); @@ -14228,6 +14225,13 @@ intel_prepare_plane_fb(struct drm_plane *plane, return PTR_ERR(vma); to_intel_plane_state(new_state)->vma = vma; + if (to_intel_plane_state(plane->state)->vma != vma) { + struct intel_crtc_state *crtc_state; + + crtc_state = intel_atomic_get_crtc_state(new_state->state, + to_intel_crtc(new_state->crtc)); + crtc_state->fb_changed = true; + } } return 0; -- 2.10.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed 2016-11-15 8:58 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson @ 2016-11-15 10:15 ` Daniel Vetter 0 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 10:15 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Nov 15, 2016 at 08:58:17AM +0000, Chris Wilson wrote: > Since an fb may have multiple VMA (due to rotations etc), we need to > wait a vblank and unpin the old VMA not if the fb itself is changed, but > if the underlying VMA is changed. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Imo we should move this into core state, iirc we're not the only ones needing it. This = fb_changed. Probably also needs to be on the plane_state. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4d578dc6d23f..a92adce033ab 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12476,9 +12476,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, > if (!was_visible && !visible) > return 0; > > - if (fb != old_plane_state->base.fb) > - pipe_config->fb_changed = true; > - > turn_off = was_visible && (!visible || mode_changed); > turn_on = visible && (!was_visible || mode_changed); > > @@ -14228,6 +14225,13 @@ intel_prepare_plane_fb(struct drm_plane *plane, > return PTR_ERR(vma); > > to_intel_plane_state(new_state)->vma = vma; > + if (to_intel_plane_state(plane->state)->vma != vma) { > + struct intel_crtc_state *crtc_state; > + > + crtc_state = intel_atomic_get_crtc_state(new_state->state, > + to_intel_crtc(new_state->crtc)); > + crtc_state->fb_changed = true; > + } > } > > return 0; > -- > 2.10.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() 2016-11-15 8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson ` (3 preceding siblings ...) 2016-11-15 8:58 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson @ 2016-11-15 9:15 ` Patchwork 2016-11-15 9:28 ` [PATCH 1/5] " Daniel Vetter 5 siblings, 0 replies; 19+ messages in thread From: Patchwork @ 2016-11-15 9:15 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx == Series Details == Series: series starting with [1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() URL : https://patchwork.freedesktop.org/series/15325/ State : warning == Summary == Series 15325v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/15325/revisions/1/mbox/ Test drv_module_reload_basic: pass -> DMESG-WARN (fi-hsw-4770r) pass -> DMESG-WARN (fi-skl-6700hq) pass -> DMESG-WARN (fi-bsw-n3050) pass -> DMESG-WARN (fi-skl-6770hq) pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-ivb-3770) pass -> DMESG-WARN (fi-skl-6260u) pass -> DMESG-WARN (fi-ivb-3520m) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (fi-skl-6770hq) Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (fi-skl-6770hq) fi-bdw-5557u total:244 pass:228 dwarn:1 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:244 pass:203 dwarn:1 dfail:0 fail:0 skip:40 fi-byt-j1900 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-n2820 total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32 fi-hsw-4770 total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-4770r total:244 pass:223 dwarn:1 dfail:0 fail:0 skip:20 fi-ilk-650 total:244 pass:191 dwarn:0 dfail:0 fail:0 skip:53 fi-ivb-3520m total:244 pass:221 dwarn:1 dfail:0 fail:0 skip:22 fi-ivb-3770 total:244 pass:221 dwarn:1 dfail:0 fail:0 skip:22 fi-kbl-7200u total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 fi-skl-6260u total:244 pass:229 dwarn:1 dfail:0 fail:0 skip:14 fi-skl-6700hq total:244 pass:222 dwarn:1 dfail:0 fail:0 skip:21 fi-skl-6700k total:244 pass:222 dwarn:1 dfail:0 fail:0 skip:21 fi-skl-6770hq total:244 pass:227 dwarn:3 dfail:0 fail:0 skip:14 fi-snb-2520m total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32 fi-snb-2600 total:244 pass:211 dwarn:0 dfail:0 fail:0 skip:33 6cdc55e3a3822b1a0faff0124828816384bd04e1 drm-intel-nightly: 2016y-11m-15d-08h-13m-16s UTC integration manifest c1e9b34 drm/i915: Set crtc_state->fb_changed whenever a VMA is changed 7a960c5 drm/i915: Quick spring clean of intel_prepare_plane_fb() 144db97 drm/i915: Track pinned vma in intel_plane_state 6e62480 drm/i915: Always prepare planes at the start of an atomic commit 34f16e2 drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2996/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() 2016-11-15 8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson ` (4 preceding siblings ...) 2016-11-15 9:15 ` ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Patchwork @ 2016-11-15 9:28 ` Daniel Vetter 5 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2016-11-15 9:28 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Tue, Nov 15, 2016 at 08:58:13AM +0000, Chris Wilson wrote: > In the next patch, a few rearrangements are made to make these static. > First, we move them so the changes are not lost in the noise. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Assuming you really just moved (didn't spot anything else): Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/intel_display.c | 255 ++++++++++++++++++----------------- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 2 files changed, 130 insertions(+), 127 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index cb9377de456e..8e04f31bf12e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14144,6 +14144,134 @@ static int intel_atomic_check(struct drm_device *dev, > return calc_watermark_data(state); > } > > +/** > + * 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. > + * > + * Must be called with struct_mutex held. > + * > + * Returns 0 on success, negative error code on failure. > + */ > +int > +intel_prepare_plane_fb(struct drm_plane *plane, > + struct drm_plane_state *new_state) > +{ > + struct intel_atomic_state *intel_state = > + to_intel_atomic_state(new_state->state); > + struct drm_i915_private *dev_priv = to_i915(plane->dev); > + struct drm_framebuffer *fb = new_state->fb; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); > + int ret; > + > + if (!obj && !old_obj) > + return 0; > + > + if (old_obj) { > + struct drm_crtc_state *crtc_state = > + drm_atomic_get_existing_crtc_state(new_state->state, > + plane->state->crtc); > + > + /* Big Hammer, we also need to ensure that any pending > + * MI_WAIT_FOR_EVENT inside a user batch buffer on the > + * current scanout is retired before unpinning the old > + * framebuffer. Note that we rely on userspace rendering > + * into the buffer attached to the pipe they are waiting > + * on. If not, userspace generates a GPU hang with IPEHR > + * point to the MI_WAIT_FOR_EVENT. > + * > + * This should only fail upon a hung GPU, in which case we > + * can safely continue. > + */ > + if (needs_modeset(crtc_state)) { > + ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, > + old_obj->resv, NULL, > + false, 0, > + GFP_KERNEL); > + if (ret < 0) > + return ret; > + } > + } > + > + if (new_state->fence) { /* explicit fencing */ > + ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready, > + new_state->fence, > + I915_FENCE_TIMEOUT, > + GFP_KERNEL); > + if (ret < 0) > + return ret; > + } > + > + if (!obj) > + return 0; > + > + if (!new_state->fence) { /* implicit fencing */ > + ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, > + obj->resv, NULL, > + false, I915_FENCE_TIMEOUT, > + GFP_KERNEL); > + if (ret < 0) > + return ret; > + > + i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY); > + } > + > + if (plane->type == DRM_PLANE_TYPE_CURSOR && > + INTEL_INFO(dev_priv)->cursor_needs_physical) { > + int align = IS_I830(dev_priv) ? 16 * 1024 : 256; > + ret = i915_gem_object_attach_phys(obj, 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_state->rotation); > + if (IS_ERR(vma)) { > + DRM_DEBUG_KMS("failed to pin object\n"); > + return PTR_ERR(vma); > + } > + } > + > + return 0; > +} > + > +/** > + * intel_cleanup_plane_fb - Cleans up an fb after plane use > + * @plane: drm plane to clean up for > + * @fb: old framebuffer that was on plane > + * > + * Cleans up a framebuffer that has just been removed from a plane. > + * > + * Must be called with struct_mutex held. > + */ > +void > +intel_cleanup_plane_fb(struct drm_plane *plane, > + struct drm_plane_state *old_state) > +{ > + struct drm_i915_private *dev_priv = to_i915(plane->dev); > + struct intel_plane_state *old_intel_state; > + struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); > + struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); > + > + old_intel_state = to_intel_plane_state(old_state); > + > + if (!obj && !old_obj) > + return; > + > + if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || > + !INTEL_INFO(dev_priv)->cursor_needs_physical)) > + intel_unpin_fb_obj(old_state->fb, old_state->rotation); > +} > + > + > static int intel_atomic_prepare_commit(struct drm_device *dev, > struct drm_atomic_state *state) > { > @@ -14716,133 +14844,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > .atomic_destroy_state = intel_crtc_destroy_state, > }; > > -/** > - * 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. > - * > - * Must be called with struct_mutex held. > - * > - * Returns 0 on success, negative error code on failure. > - */ > -int > -intel_prepare_plane_fb(struct drm_plane *plane, > - struct drm_plane_state *new_state) > -{ > - struct intel_atomic_state *intel_state = > - to_intel_atomic_state(new_state->state); > - struct drm_i915_private *dev_priv = to_i915(plane->dev); > - struct drm_framebuffer *fb = new_state->fb; > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); > - int ret; > - > - if (!obj && !old_obj) > - return 0; > - > - if (old_obj) { > - struct drm_crtc_state *crtc_state = > - drm_atomic_get_existing_crtc_state(new_state->state, > - plane->state->crtc); > - > - /* Big Hammer, we also need to ensure that any pending > - * MI_WAIT_FOR_EVENT inside a user batch buffer on the > - * current scanout is retired before unpinning the old > - * framebuffer. Note that we rely on userspace rendering > - * into the buffer attached to the pipe they are waiting > - * on. If not, userspace generates a GPU hang with IPEHR > - * point to the MI_WAIT_FOR_EVENT. > - * > - * This should only fail upon a hung GPU, in which case we > - * can safely continue. > - */ > - if (needs_modeset(crtc_state)) { > - ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, > - old_obj->resv, NULL, > - false, 0, > - GFP_KERNEL); > - if (ret < 0) > - return ret; > - } > - } > - > - if (new_state->fence) { /* explicit fencing */ > - ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready, > - new_state->fence, > - I915_FENCE_TIMEOUT, > - GFP_KERNEL); > - if (ret < 0) > - return ret; > - } > - > - if (!obj) > - return 0; > - > - if (!new_state->fence) { /* implicit fencing */ > - ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, > - obj->resv, NULL, > - false, I915_FENCE_TIMEOUT, > - GFP_KERNEL); > - if (ret < 0) > - return ret; > - > - i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY); > - } > - > - if (plane->type == DRM_PLANE_TYPE_CURSOR && > - INTEL_INFO(dev_priv)->cursor_needs_physical) { > - int align = IS_I830(dev_priv) ? 16 * 1024 : 256; > - ret = i915_gem_object_attach_phys(obj, 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_state->rotation); > - if (IS_ERR(vma)) { > - DRM_DEBUG_KMS("failed to pin object\n"); > - return PTR_ERR(vma); > - } > - } > - > - return 0; > -} > - > -/** > - * intel_cleanup_plane_fb - Cleans up an fb after plane use > - * @plane: drm plane to clean up for > - * @fb: old framebuffer that was on plane > - * > - * Cleans up a framebuffer that has just been removed from a plane. > - * > - * Must be called with struct_mutex held. > - */ > -void > -intel_cleanup_plane_fb(struct drm_plane *plane, > - struct drm_plane_state *old_state) > -{ > - struct drm_i915_private *dev_priv = to_i915(plane->dev); > - struct intel_plane_state *old_intel_state; > - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); > - struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); > - > - old_intel_state = to_intel_plane_state(old_state); > - > - if (!obj && !old_obj) > - return; > - > - if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || > - !INTEL_INFO(dev_priv)->cursor_needs_physical)) > - intel_unpin_fb_obj(old_state->fb, old_state->rotation); > -} > - > int > skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state) > { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 75252ecaa613..4cb8254c66dd 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -772,6 +772,8 @@ struct intel_plane { > int max_downscale; > uint32_t frontbuffer_bit; > > + struct i915_sw_fence *last_commit; > + > /* Since we need to change the watermarks before/after > * enabling/disabling the planes, we need to store the parameters here > * as the other pieces of the struct may not reflect the values we want > -- > 2.10.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() @ 2016-10-29 12:04 Chris Wilson 2016-10-29 12:04 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson 0 siblings, 1 reply; 19+ messages in thread From: Chris Wilson @ 2016-10-29 12:04 UTC (permalink / raw) To: intel-gfx In the next patch, a few rearrangements are made to make these static. First, we move them so the changes are not lost in the noise. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 256 +++++++++++++++++------------------ drivers/gpu/drm/i915/intel_drv.h | 2 + 2 files changed, 130 insertions(+), 128 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a0de5ce1a3e1..6c817215f61a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14132,6 +14132,134 @@ static int intel_atomic_check(struct drm_device *dev, return calc_watermark_data(state); } +/** + * 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. + * + * Must be called with struct_mutex held. + * + * Returns 0 on success, negative error code on failure. + */ +int +intel_prepare_plane_fb(struct drm_plane *plane, + struct drm_plane_state *new_state) +{ + struct intel_atomic_state *intel_state = + to_intel_atomic_state(new_state->state); + struct drm_device *dev = plane->dev; + struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_framebuffer *fb = new_state->fb; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); + int ret; + + if (!obj && !old_obj) + return 0; + + if (old_obj) { + struct drm_crtc_state *crtc_state = + drm_atomic_get_existing_crtc_state(new_state->state, + plane->state->crtc); + + /* Big Hammer, we also need to ensure that any pending + * MI_WAIT_FOR_EVENT inside a user batch buffer on the + * current scanout is retired before unpinning the old + * framebuffer. Note that we rely on userspace rendering + * into the buffer attached to the pipe they are waiting + * on. If not, userspace generates a GPU hang with IPEHR + * point to the MI_WAIT_FOR_EVENT. + * + * This should only fail upon a hung GPU, in which case we + * can safely continue. + */ + if (needs_modeset(crtc_state)) { + ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, + old_obj->resv, NULL, + false, 0, + GFP_KERNEL); + if (ret < 0) + return ret; + } + } + + if (new_state->fence) { /* explicit fencing */ + ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready, + new_state->fence, + I915_FENCE_TIMEOUT, + GFP_KERNEL); + if (ret < 0) + return ret; + } + + if (!obj) + return 0; + + if (!new_state->fence) { /* implicit fencing */ + ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, + obj->resv, NULL, + false, I915_FENCE_TIMEOUT, + GFP_KERNEL); + if (ret < 0) + return ret; + + i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY); + } + + if (plane->type == DRM_PLANE_TYPE_CURSOR && + INTEL_INFO(dev)->cursor_needs_physical) { + int align = IS_I830(dev_priv) ? 16 * 1024 : 256; + ret = i915_gem_object_attach_phys(obj, 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_state->rotation); + if (IS_ERR(vma)) { + DRM_DEBUG_KMS("failed to pin object\n"); + return PTR_ERR(vma); + } + } + + return 0; +} + +/** + * intel_cleanup_plane_fb - Cleans up an fb after plane use + * @plane: drm plane to clean up for + * @fb: old framebuffer that was on plane + * + * Cleans up a framebuffer that has just been removed from a plane. + * + * Must be called with struct_mutex held. + */ +void +intel_cleanup_plane_fb(struct drm_plane *plane, + struct drm_plane_state *old_state) +{ + struct drm_device *dev = plane->dev; + struct intel_plane_state *old_intel_state; + struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); + struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); + + old_intel_state = to_intel_plane_state(old_state); + + if (!obj && !old_obj) + return; + + if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || + !INTEL_INFO(dev)->cursor_needs_physical)) + intel_unpin_fb_obj(old_state->fb, old_state->rotation); +} + static int intel_atomic_prepare_commit(struct drm_device *dev, struct drm_atomic_state *state) { @@ -14701,134 +14829,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { .atomic_destroy_state = intel_crtc_destroy_state, }; -/** - * 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. - * - * Must be called with struct_mutex held. - * - * Returns 0 on success, negative error code on failure. - */ -int -intel_prepare_plane_fb(struct drm_plane *plane, - struct drm_plane_state *new_state) -{ - struct intel_atomic_state *intel_state = - to_intel_atomic_state(new_state->state); - struct drm_device *dev = plane->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct drm_framebuffer *fb = new_state->fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); - int ret; - - if (!obj && !old_obj) - return 0; - - if (old_obj) { - struct drm_crtc_state *crtc_state = - drm_atomic_get_existing_crtc_state(new_state->state, - plane->state->crtc); - - /* Big Hammer, we also need to ensure that any pending - * MI_WAIT_FOR_EVENT inside a user batch buffer on the - * current scanout is retired before unpinning the old - * framebuffer. Note that we rely on userspace rendering - * into the buffer attached to the pipe they are waiting - * on. If not, userspace generates a GPU hang with IPEHR - * point to the MI_WAIT_FOR_EVENT. - * - * This should only fail upon a hung GPU, in which case we - * can safely continue. - */ - if (needs_modeset(crtc_state)) { - ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, - old_obj->resv, NULL, - false, 0, - GFP_KERNEL); - if (ret < 0) - return ret; - } - } - - if (new_state->fence) { /* explicit fencing */ - ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready, - new_state->fence, - I915_FENCE_TIMEOUT, - GFP_KERNEL); - if (ret < 0) - return ret; - } - - if (!obj) - return 0; - - if (!new_state->fence) { /* implicit fencing */ - ret = i915_sw_fence_await_reservation(&intel_state->commit_ready, - obj->resv, NULL, - false, I915_FENCE_TIMEOUT, - GFP_KERNEL); - if (ret < 0) - return ret; - - i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY); - } - - if (plane->type == DRM_PLANE_TYPE_CURSOR && - INTEL_INFO(dev)->cursor_needs_physical) { - int align = IS_I830(dev_priv) ? 16 * 1024 : 256; - ret = i915_gem_object_attach_phys(obj, 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_state->rotation); - if (IS_ERR(vma)) { - DRM_DEBUG_KMS("failed to pin object\n"); - return PTR_ERR(vma); - } - } - - return 0; -} - -/** - * intel_cleanup_plane_fb - Cleans up an fb after plane use - * @plane: drm plane to clean up for - * @fb: old framebuffer that was on plane - * - * Cleans up a framebuffer that has just been removed from a plane. - * - * Must be called with struct_mutex held. - */ -void -intel_cleanup_plane_fb(struct drm_plane *plane, - struct drm_plane_state *old_state) -{ - struct drm_device *dev = plane->dev; - struct intel_plane_state *old_intel_state; - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); - struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); - - old_intel_state = to_intel_plane_state(old_state); - - if (!obj && !old_obj) - return; - - if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR || - !INTEL_INFO(dev)->cursor_needs_physical)) - intel_unpin_fb_obj(old_state->fb, old_state->rotation); -} - int skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2616d92f9fee..f5975fe4553d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -782,6 +782,8 @@ struct intel_plane { int max_downscale; uint32_t frontbuffer_bit; + struct i915_sw_fence *last_commit; + /* Since we need to change the watermarks before/after * enabling/disabling the planes, we need to store the parameters here * as the other pieces of the struct may not reflect the values we want -- 2.10.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed 2016-10-29 12:04 Chris Wilson @ 2016-10-29 12:04 ` Chris Wilson 0 siblings, 0 replies; 19+ messages in thread From: Chris Wilson @ 2016-10-29 12:04 UTC (permalink / raw) To: intel-gfx Since an fb may have multiple VMA (due to rotations etc), we need to wait a vblank and unpin the old VMA not if the fb itself is changed, but if the underlying VMA is changed. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_display.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 97bd5698003f..2cfc812df04a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12468,9 +12468,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, if (!was_visible && !visible) return 0; - if (fb != old_plane_state->base.fb) - pipe_config->fb_changed = true; - turn_off = was_visible && (!visible || mode_changed); turn_on = visible && (!was_visible || mode_changed); @@ -14218,6 +14215,13 @@ intel_prepare_plane_fb(struct drm_plane *plane, return PTR_ERR(vma); to_intel_plane_state(new_state)->vma = vma; + if (to_intel_plane_state(plane->state)->vma != vma) { + struct intel_crtc_state *crtc_state; + + crtc_state = intel_atomic_get_crtc_state(new_state->state, + to_intel_crtc(new_state->crtc)); + crtc_state->fb_changed = true; + } } return 0; -- 2.10.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-11-15 10:24 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-15 8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson 2016-11-15 8:58 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson 2016-11-15 9:37 ` Daniel Vetter 2016-11-15 9:47 ` Tvrtko Ursulin 2016-11-15 9:57 ` Chris Wilson 2016-11-15 10:10 ` Daniel Vetter 2016-11-15 10:18 ` Chris Wilson 2016-11-15 8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson 2016-11-15 9:52 ` Daniel Vetter 2016-11-15 10:02 ` Chris Wilson 2016-11-15 10:13 ` Daniel Vetter 2016-11-15 10:24 ` Daniel Vetter 2016-11-15 8:58 ` [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() Chris Wilson 2016-11-15 10:23 ` Daniel Vetter 2016-11-15 8:58 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson 2016-11-15 10:15 ` Daniel Vetter 2016-11-15 9:15 ` ✗ Fi.CI.BAT: warning for series starting with [1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Patchwork 2016-11-15 9:28 ` [PATCH 1/5] " Daniel Vetter -- strict thread matches above, loose matches on Subject: below -- 2016-10-29 12:04 Chris Wilson 2016-10-29 12:04 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.