* [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
@ 2017-09-19 12:14 Maarten Lankhorst
2017-09-19 12:14 ` [PATCH 2/2] drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating Maarten Lankhorst
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Maarten Lankhorst @ 2017-09-19 12:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Commit b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.") removed
the call to wait_for_vblanks and replaced it with flip_done.
Unfortunately legacy_cursor_update was unset too late, and the
replacement call drm_atomic_helper_wait_for_flip_done() was
a noop. Make sure that its unset before setup_commit() is
called to fix this issue.
Changes since v1:
- Force vblank wait for watermarks not yet converted to atomic too. (Ville)
- Use for_each_new_intel_crtc_in_state. (Ville)
Changes since v2:
- Move the optimization to a separate commit. (Ville)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
Testcase: kms_cursor_crc
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reported-by: Marta Löfstedt <marta.lofstedt@intel.com>
Cc: Marta Löfstedt <marta.lofstedt@intel.com>
Tested-by: Marta Löfstedt <marta.lofstedt@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8599e425abb1..a85ca19a0c45 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,
struct drm_i915_private *dev_priv = to_i915(dev);
int ret = 0;
- ret = drm_atomic_helper_setup_commit(state, nonblock);
- if (ret)
- return ret;
-
drm_atomic_state_get(state);
i915_sw_fence_init(&intel_state->commit_ready,
intel_atomic_commit_ready);
- ret = intel_atomic_prepare_commit(dev, state);
- if (ret) {
- DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
- i915_sw_fence_commit(&intel_state->commit_ready);
- return ret;
- }
-
/*
* The intel_legacy_cursor_update() fast path takes care
* of avoiding the vblank waits for simple cursor
@@ -12540,11 +12529,11 @@ static int intel_atomic_commit(struct drm_device *dev,
* updates happen during the correct frames. Gen9+ have
* double buffered watermarks and so shouldn't need this.
*
- * Do this after drm_atomic_helper_setup_commit() and
- * intel_atomic_prepare_commit() because we still want
- * to skip the flip and fb cleanup waits. Although that
- * does risk yanking the mapping from under the display
- * engine.
+ * Unset state->legacy_cursor_update before the call to
+ * drm_atomic_helper_setup_commit() because otherwise
+ * drm_atomic_helper_wait_for_flip_done() is a noop and
+ * we get FIFO underruns because we didn't wait
+ * for vblank.
*
* FIXME doing watermarks and fb cleanup from a vblank worker
* (assuming we had any) would solve these problems.
@@ -12552,7 +12541,17 @@ static int intel_atomic_commit(struct drm_device *dev,
if (INTEL_GEN(dev_priv) < 9)
state->legacy_cursor_update = false;
- ret = drm_atomic_helper_swap_state(state, true);
+ ret = intel_atomic_prepare_commit(dev, state);
+ if (ret) {
+ DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
+ i915_sw_fence_commit(&intel_state->commit_ready);
+ return ret;
+ }
+
+ ret = drm_atomic_helper_setup_commit(state, nonblock);
+ if (!ret)
+ ret = drm_atomic_helper_swap_state(state, true);
+
if (ret) {
i915_sw_fence_commit(&intel_state->commit_ready);
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating
2017-09-19 12:14 [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Maarten Lankhorst
@ 2017-09-19 12:14 ` Maarten Lankhorst
2017-09-19 14:03 ` Ville Syrjälä
2017-09-19 13:38 ` [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Ville Syrjälä
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2017-09-19 12:14 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a85ca19a0c45..8d051256da1e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12538,8 +12538,16 @@ static int intel_atomic_commit(struct drm_device *dev,
* FIXME doing watermarks and fb cleanup from a vblank worker
* (assuming we had any) would solve these problems.
*/
- if (INTEL_GEN(dev_priv) < 9)
- state->legacy_cursor_update = false;
+ if (INTEL_GEN(dev_priv) < 9 && state->legacy_cursor_update) {
+ struct intel_crtc_state *new_crtc_state;
+ struct intel_crtc *crtc;
+ int i;
+
+ for_each_new_intel_crtc_in_state(intel_state, crtc, new_crtc_state, i)
+ if (new_crtc_state->wm.need_postvbl_update ||
+ new_crtc_state->update_wm_post)
+ state->legacy_cursor_update = false;
+ }
ret = intel_atomic_prepare_commit(dev, state);
if (ret) {
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
2017-09-19 12:14 [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Maarten Lankhorst
2017-09-19 12:14 ` [PATCH 2/2] drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating Maarten Lankhorst
@ 2017-09-19 13:38 ` Ville Syrjälä
2017-09-19 13:50 ` Maarten Lankhorst
2017-09-19 18:53 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2017-09-19 21:48 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2017-09-19 13:38 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Daniel Vetter, intel-gfx
On Tue, Sep 19, 2017 at 02:14:18PM +0200, Maarten Lankhorst wrote:
> Commit b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.") removed
> the call to wait_for_vblanks and replaced it with flip_done.
>
> Unfortunately legacy_cursor_update was unset too late, and the
> replacement call drm_atomic_helper_wait_for_flip_done() was
> a noop. Make sure that its unset before setup_commit() is
> called to fix this issue.
>
> Changes since v1:
> - Force vblank wait for watermarks not yet converted to atomic too. (Ville)
> - Use for_each_new_intel_crtc_in_state. (Ville)
> Changes since v2:
> - Move the optimization to a separate commit. (Ville)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
> Testcase: kms_cursor_crc
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Reported-by: Marta Löfstedt <marta.lofstedt@intel.com>
> Cc: Marta Löfstedt <marta.lofstedt@intel.com>
> Tested-by: Marta Löfstedt <marta.lofstedt@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8599e425abb1..a85ca19a0c45 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,
> struct drm_i915_private *dev_priv = to_i915(dev);
> int ret = 0;
>
> - ret = drm_atomic_helper_setup_commit(state, nonblock);
> - if (ret)
> - return ret;
> -
> drm_atomic_state_get(state);
> i915_sw_fence_init(&intel_state->commit_ready,
> intel_atomic_commit_ready);
>
> - ret = intel_atomic_prepare_commit(dev, state);
> - if (ret) {
> - DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
> - i915_sw_fence_commit(&intel_state->commit_ready);
> - return ret;
> - }
> -
> /*
> * The intel_legacy_cursor_update() fast path takes care
> * of avoiding the vblank waits for simple cursor
> @@ -12540,11 +12529,11 @@ static int intel_atomic_commit(struct drm_device *dev,
> * updates happen during the correct frames. Gen9+ have
> * double buffered watermarks and so shouldn't need this.
> *
> - * Do this after drm_atomic_helper_setup_commit() and
> - * intel_atomic_prepare_commit() because we still want
> - * to skip the flip and fb cleanup waits. Although that
> - * does risk yanking the mapping from under the display
> - * engine.
> + * Unset state->legacy_cursor_update before the call to
> + * drm_atomic_helper_setup_commit() because otherwise
> + * drm_atomic_helper_wait_for_flip_done() is a noop and
> + * we get FIFO underruns because we didn't wait
> + * for vblank.
> *
> * FIXME doing watermarks and fb cleanup from a vblank worker
> * (assuming we had any) would solve these problems.
> @@ -12552,7 +12541,17 @@ static int intel_atomic_commit(struct drm_device *dev,
> if (INTEL_GEN(dev_priv) < 9)
> state->legacy_cursor_update = false;
>
> - ret = drm_atomic_helper_swap_state(state, true);
> + ret = intel_atomic_prepare_commit(dev, state);
> + if (ret) {
> + DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
> + i915_sw_fence_commit(&intel_state->commit_ready);
> + return ret;
> + }
> +
> + ret = drm_atomic_helper_setup_commit(state, nonblock);
Hmm. One thing I noticed here. Previously if this failed we just called
i915_sw_fence_commit(). Now we will also call
drm_atomic_helper_cleanup_planes(). Is that going to be a problem?
> + if (!ret)
> + ret = drm_atomic_helper_swap_state(state, true);
> +
> if (ret) {
> i915_sw_fence_commit(&intel_state->commit_ready);
>
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
2017-09-19 13:38 ` [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Ville Syrjälä
@ 2017-09-19 13:50 ` Maarten Lankhorst
2017-09-19 14:15 ` Ville Syrjälä
0 siblings, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2017-09-19 13:50 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx
Op 19-09-17 om 15:38 schreef Ville Syrjälä:
> On Tue, Sep 19, 2017 at 02:14:18PM +0200, Maarten Lankhorst wrote:
>> Commit b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.") removed
>> the call to wait_for_vblanks and replaced it with flip_done.
>>
>> Unfortunately legacy_cursor_update was unset too late, and the
>> replacement call drm_atomic_helper_wait_for_flip_done() was
>> a noop. Make sure that its unset before setup_commit() is
>> called to fix this issue.
>>
>> Changes since v1:
>> - Force vblank wait for watermarks not yet converted to atomic too. (Ville)
>> - Use for_each_new_intel_crtc_in_state. (Ville)
>> Changes since v2:
>> - Move the optimization to a separate commit. (Ville)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Fixes: b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
>> Testcase: kms_cursor_crc
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Reported-by: Marta Löfstedt <marta.lofstedt@intel.com>
>> Cc: Marta Löfstedt <marta.lofstedt@intel.com>
>> Tested-by: Marta Löfstedt <marta.lofstedt@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 33 ++++++++++++++++-----------------
>> 1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 8599e425abb1..a85ca19a0c45 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> int ret = 0;
>>
>> - ret = drm_atomic_helper_setup_commit(state, nonblock);
>> - if (ret)
>> - return ret;
>> -
>> drm_atomic_state_get(state);
>> i915_sw_fence_init(&intel_state->commit_ready,
>> intel_atomic_commit_ready);
>>
>> - ret = intel_atomic_prepare_commit(dev, state);
>> - if (ret) {
>> - DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
>> - i915_sw_fence_commit(&intel_state->commit_ready);
>> - return ret;
>> - }
>> -
>> /*
>> * The intel_legacy_cursor_update() fast path takes care
>> * of avoiding the vblank waits for simple cursor
>> @@ -12540,11 +12529,11 @@ static int intel_atomic_commit(struct drm_device *dev,
>> * updates happen during the correct frames. Gen9+ have
>> * double buffered watermarks and so shouldn't need this.
>> *
>> - * Do this after drm_atomic_helper_setup_commit() and
>> - * intel_atomic_prepare_commit() because we still want
>> - * to skip the flip and fb cleanup waits. Although that
>> - * does risk yanking the mapping from under the display
>> - * engine.
>> + * Unset state->legacy_cursor_update before the call to
>> + * drm_atomic_helper_setup_commit() because otherwise
>> + * drm_atomic_helper_wait_for_flip_done() is a noop and
>> + * we get FIFO underruns because we didn't wait
>> + * for vblank.
>> *
>> * FIXME doing watermarks and fb cleanup from a vblank worker
>> * (assuming we had any) would solve these problems.
>> @@ -12552,7 +12541,17 @@ static int intel_atomic_commit(struct drm_device *dev,
>> if (INTEL_GEN(dev_priv) < 9)
>> state->legacy_cursor_update = false;
>>
>> - ret = drm_atomic_helper_swap_state(state, true);
>> + ret = intel_atomic_prepare_commit(dev, state);
>> + if (ret) {
>> + DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
>> + i915_sw_fence_commit(&intel_state->commit_ready);
>> + return ret;
>> + }
>> +
>> + ret = drm_atomic_helper_setup_commit(state, nonblock);
> Hmm. One thing I noticed here. Previously if this failed we just called
> i915_sw_fence_commit(). Now we will also call
> drm_atomic_helper_cleanup_planes(). Is that going to be a problem?
This is intentional, intel_atomic_prepare_commit calls prepare_planes,
if that succeeds we'll need to call cleanup_planes too.
Ordering of setup_commit and prepare_planes doesn't matter.
>> + if (!ret)
>> + ret = drm_atomic_helper_swap_state(state, true);
>> +
>> if (ret) {
>> i915_sw_fence_commit(&intel_state->commit_ready);
>>
>> --
>> 2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating
2017-09-19 12:14 ` [PATCH 2/2] drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating Maarten Lankhorst
@ 2017-09-19 14:03 ` Ville Syrjälä
0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2017-09-19 14:03 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Tue, Sep 19, 2017 at 02:14:19PM +0200, Maarten Lankhorst wrote:
Could use a commit message of some sort.
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a85ca19a0c45..8d051256da1e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12538,8 +12538,16 @@ static int intel_atomic_commit(struct drm_device *dev,
> * FIXME doing watermarks and fb cleanup from a vblank worker
> * (assuming we had any) would solve these problems.
> */
> - if (INTEL_GEN(dev_priv) < 9)
> - state->legacy_cursor_update = false;
> + if (INTEL_GEN(dev_priv) < 9 && state->legacy_cursor_update) {
> + struct intel_crtc_state *new_crtc_state;
> + struct intel_crtc *crtc;
> + int i;
> +
> + for_each_new_intel_crtc_in_state(intel_state, crtc, new_crtc_state, i)
> + if (new_crtc_state->wm.need_postvbl_update ||
> + new_crtc_state->update_wm_post)
> + state->legacy_cursor_update = false;
> + }
Not sure this is going to buy us much actually. I don't recall off hand
whether it's likely/possible that we'll get the same watermark for a
different sized cursors. VLV/CHV are the exception since we currently
use a fixed watermark for cursors on those platforms because the normal
formula didn't actually work there and I was too lazy to come up with
anything better.
Either way I don't think this should make things worse, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ret = intel_atomic_prepare_commit(dev, state);
> if (ret) {
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
2017-09-19 13:50 ` Maarten Lankhorst
@ 2017-09-19 14:15 ` Ville Syrjälä
0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2017-09-19 14:15 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Daniel Vetter, intel-gfx
On Tue, Sep 19, 2017 at 03:50:47PM +0200, Maarten Lankhorst wrote:
> Op 19-09-17 om 15:38 schreef Ville Syrjälä:
> > On Tue, Sep 19, 2017 at 02:14:18PM +0200, Maarten Lankhorst wrote:
> >> Commit b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.") removed
> >> the call to wait_for_vblanks and replaced it with flip_done.
> >>
> >> Unfortunately legacy_cursor_update was unset too late, and the
> >> replacement call drm_atomic_helper_wait_for_flip_done() was
> >> a noop. Make sure that its unset before setup_commit() is
> >> called to fix this issue.
> >>
> >> Changes since v1:
> >> - Force vblank wait for watermarks not yet converted to atomic too. (Ville)
> >> - Use for_each_new_intel_crtc_in_state. (Ville)
> >> Changes since v2:
> >> - Move the optimization to a separate commit. (Ville)
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Fixes: b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
> >> Testcase: kms_cursor_crc
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> >> Reported-by: Marta Löfstedt <marta.lofstedt@intel.com>
> >> Cc: Marta Löfstedt <marta.lofstedt@intel.com>
> >> Tested-by: Marta Löfstedt <marta.lofstedt@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_display.c | 33 ++++++++++++++++-----------------
> >> 1 file changed, 16 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 8599e425abb1..a85ca19a0c45 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,
> >> struct drm_i915_private *dev_priv = to_i915(dev);
> >> int ret = 0;
> >>
> >> - ret = drm_atomic_helper_setup_commit(state, nonblock);
> >> - if (ret)
> >> - return ret;
> >> -
> >> drm_atomic_state_get(state);
> >> i915_sw_fence_init(&intel_state->commit_ready,
> >> intel_atomic_commit_ready);
> >>
> >> - ret = intel_atomic_prepare_commit(dev, state);
> >> - if (ret) {
> >> - DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
> >> - i915_sw_fence_commit(&intel_state->commit_ready);
> >> - return ret;
> >> - }
> >> -
> >> /*
> >> * The intel_legacy_cursor_update() fast path takes care
> >> * of avoiding the vblank waits for simple cursor
> >> @@ -12540,11 +12529,11 @@ static int intel_atomic_commit(struct drm_device *dev,
> >> * updates happen during the correct frames. Gen9+ have
> >> * double buffered watermarks and so shouldn't need this.
> >> *
> >> - * Do this after drm_atomic_helper_setup_commit() and
> >> - * intel_atomic_prepare_commit() because we still want
> >> - * to skip the flip and fb cleanup waits. Although that
> >> - * does risk yanking the mapping from under the display
> >> - * engine.
> >> + * Unset state->legacy_cursor_update before the call to
> >> + * drm_atomic_helper_setup_commit() because otherwise
> >> + * drm_atomic_helper_wait_for_flip_done() is a noop and
> >> + * we get FIFO underruns because we didn't wait
> >> + * for vblank.
> >> *
> >> * FIXME doing watermarks and fb cleanup from a vblank worker
> >> * (assuming we had any) would solve these problems.
> >> @@ -12552,7 +12541,17 @@ static int intel_atomic_commit(struct drm_device *dev,
> >> if (INTEL_GEN(dev_priv) < 9)
> >> state->legacy_cursor_update = false;
> >>
> >> - ret = drm_atomic_helper_swap_state(state, true);
> >> + ret = intel_atomic_prepare_commit(dev, state);
> >> + if (ret) {
> >> + DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
> >> + i915_sw_fence_commit(&intel_state->commit_ready);
> >> + return ret;
> >> + }
> >> +
> >> + ret = drm_atomic_helper_setup_commit(state, nonblock);
> > Hmm. One thing I noticed here. Previously if this failed we just called
> > i915_sw_fence_commit(). Now we will also call
> > drm_atomic_helper_cleanup_planes(). Is that going to be a problem?
> This is intentional, intel_atomic_prepare_commit calls prepare_planes,
> if that succeeds we'll need to call cleanup_planes too.
Doh. Right.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Ordering of setup_commit and prepare_planes doesn't matter.
> >> + if (!ret)
> >> + ret = drm_atomic_helper_swap_state(state, true);
> >> +
> >> if (ret) {
> >> i915_sw_fence_commit(&intel_state->commit_ready);
> >>
> >> --
> >> 2.14.1
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
2017-09-19 12:14 [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Maarten Lankhorst
2017-09-19 12:14 ` [PATCH 2/2] drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating Maarten Lankhorst
2017-09-19 13:38 ` [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Ville Syrjälä
@ 2017-09-19 18:53 ` Patchwork
2017-09-19 21:48 ` ✓ Fi.CI.IGT: " Patchwork
3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-09-19 18:53 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
URL : https://patchwork.freedesktop.org/series/30587/
State : success
== Summary ==
Series 30587v1 series starting with [1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
https://patchwork.freedesktop.org/api/1.0/series/30587/revisions/1/mbox/
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> FAIL (fi-snb-2600) fdo#100215
Test kms_frontbuffer_tracking:
Subgroup basic:
dmesg-warn -> PASS (fi-kbl-7500u)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
incomplete -> DMESG-WARN (fi-cfl-s) fdo#102294
Subgroup suspend-read-crc-pipe-a:
incomplete -> PASS (fi-kbl-7500u)
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:439s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:472s
fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:420s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:515s
fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:280s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:519s
fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:492s
fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:492s
fi-cfl-s total:289 pass:222 dwarn:35 dfail:0 fail:0 skip:32 time:539s
fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:415s
fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:562s
fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:426s
fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:405s
fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:430s
fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:491s
fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:460s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:476s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:575s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:586s
fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:543s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:449s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:751s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:489s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:470s
fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:560s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:412s
bf6ecf6d25c1c45e576643b7d7a65e8b1e6b4f01 drm-tip: 2017y-09m-19d-17h-23m-04s UTC integration manifest
fa47d22ff51e drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating
e318ccc3d8f0 drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5748/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
2017-09-19 12:14 [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Maarten Lankhorst
` (2 preceding siblings ...)
2017-09-19 18:53 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2017-09-19 21:48 ` Patchwork
3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-09-19 21:48 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3.
URL : https://patchwork.freedesktop.org/series/30587/
State : success
== Summary ==
Test perf:
Subgroup polling:
fail -> PASS (shard-hsw) fdo#102252 +1
Test kms_flip:
Subgroup rcs-wf_vblank-vs-dpms:
dmesg-warn -> PASS (shard-hsw)
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
shard-hsw total:2317 pass:1245 dwarn:3 dfail:0 fail:12 skip:1057 time:9699s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5748/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-19 21:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-19 12:14 [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Maarten Lankhorst
2017-09-19 12:14 ` [PATCH 2/2] drm/i915: Skip vblank waits for cursor updates when watermarks dont need updating Maarten Lankhorst
2017-09-19 14:03 ` Ville Syrjälä
2017-09-19 13:38 ` [PATCH 1/2] drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v3 Ville Syrjälä
2017-09-19 13:50 ` Maarten Lankhorst
2017-09-19 14:15 ` Ville Syrjälä
2017-09-19 18:53 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2017-09-19 21:48 ` ✓ Fi.CI.IGT: " Patchwork
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.