* [PATCH 1/2] drm/i915: Move updating color management to before vblank evasion @ 2017-02-28 14:28 Maarten Lankhorst 2017-02-28 14:28 ` [PATCH 2/2] drm/i915: Complain if we take too long under " Maarten Lankhorst ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Maarten Lankhorst @ 2017-02-28 14:28 UTC (permalink / raw) To: intel-gfx This cannot be done reliably during vblank evasasion since the color management registers are not double buffered. The original commit that moved it always during vblank evasion was wrong, so revert it to before vblank evasion again. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Fixes: 20a34e78f0d7 ("drm/i915: Update color management during vblank evasion.") Cc: stable@vger.kernel.org # v4.7+ --- drivers/gpu/drm/i915/intel_display.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c862d0ab389..a0108041fd4a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13301,17 +13301,19 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, to_intel_atomic_state(old_crtc_state->state); bool modeset = needs_modeset(crtc->state); + if (!modeset && + (intel_cstate->base.color_mgmt_changed || + intel_cstate->update_pipe)) { + intel_color_set_csc(crtc->state); + intel_color_load_luts(crtc->state); + } + /* Perform vblank evasion around commit operation */ intel_pipe_update_start(intel_crtc); if (modeset) goto out; - if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) { - intel_color_set_csc(crtc->state); - intel_color_load_luts(crtc->state); - } - if (intel_cstate->update_pipe) intel_update_pipe_config(intel_crtc, old_intel_cstate); else if (INTEL_GEN(dev_priv) >= 9) -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/i915: Complain if we take too long under vblank evasion. 2017-02-28 14:28 [PATCH 1/2] drm/i915: Move updating color management to before vblank evasion Maarten Lankhorst @ 2017-02-28 14:28 ` Maarten Lankhorst 2017-03-01 15:40 ` Ville Syrjälä 2017-02-28 15:01 ` [PATCH 1/2] drm/i915: Move updating color management to before " Ville Syrjälä 2017-02-28 18:53 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork 2 siblings, 1 reply; 7+ messages in thread From: Maarten Lankhorst @ 2017-02-28 14:28 UTC (permalink / raw) To: intel-gfx Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_sprite.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 27e0752d1578..375ca91b308c 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -65,6 +65,8 @@ int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode, 1000 * adjusted_mode->crtc_htotal); } +#define VBLANK_EVASION_TIME_US 100 + /** * intel_pipe_update_start() - start update of a set of display registers * @crtc: the crtc of which the registers are going to be updated @@ -92,7 +94,8 @@ void intel_pipe_update_start(struct intel_crtc *crtc) vblank_start = DIV_ROUND_UP(vblank_start, 2); /* FIXME needs to be calibrated sensibly */ - min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, 100); + min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, + VBLANK_EVASION_TIME_US); max = vblank_start - 1; local_irq_disable(); @@ -191,7 +194,12 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time), crtc->debug.min_vbl, crtc->debug.max_vbl, crtc->debug.scanline_start, scanline_end); - } + } else if (ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time) > + VBLANK_EVASION_TIME_US) + DRM_WARN("Atomic update on pipe (%c) took %lld us, max time under evasion is %u us\n", + pipe_name(pipe), + ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time), + VBLANK_EVASION_TIME_US); } static void -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/i915: Complain if we take too long under vblank evasion. 2017-02-28 14:28 ` [PATCH 2/2] drm/i915: Complain if we take too long under " Maarten Lankhorst @ 2017-03-01 15:40 ` Ville Syrjälä 2017-03-06 12:06 ` Maarten Lankhorst 0 siblings, 1 reply; 7+ messages in thread From: Ville Syrjälä @ 2017-03-01 15:40 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Tue, Feb 28, 2017 at 03:28:48PM +0100, Maarten Lankhorst wrote: > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 27e0752d1578..375ca91b308c 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -65,6 +65,8 @@ int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode, > 1000 * adjusted_mode->crtc_htotal); > } > > +#define VBLANK_EVASION_TIME_US 100 > + > /** > * intel_pipe_update_start() - start update of a set of display registers > * @crtc: the crtc of which the registers are going to be updated > @@ -92,7 +94,8 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > vblank_start = DIV_ROUND_UP(vblank_start, 2); > > /* FIXME needs to be calibrated sensibly */ > - min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, 100); > + min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, > + VBLANK_EVASION_TIME_US); > max = vblank_start - 1; > > local_irq_disable(); > @@ -191,7 +194,12 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work > ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time), > crtc->debug.min_vbl, crtc->debug.max_vbl, > crtc->debug.scanline_start, scanline_end); > - } > + } else if (ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time) > > + VBLANK_EVASION_TIME_US) > + DRM_WARN("Atomic update on pipe (%c) took %lld us, max time under evasion is %u us\n", > + pipe_name(pipe), > + ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time), > + VBLANK_EVASION_TIME_US); Hmm. Yeah this should make it very likely that we'll catch any potential fails even when they don't happen to hit the actual critical section. Hopefully it won't result in massive amounts of noise. One way to find out ;) Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > } > > static void > -- > 2.7.4 > > _______________________________________________ > 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] 7+ messages in thread
* Re: [PATCH 2/2] drm/i915: Complain if we take too long under vblank evasion. 2017-03-01 15:40 ` Ville Syrjälä @ 2017-03-06 12:06 ` Maarten Lankhorst 0 siblings, 0 replies; 7+ messages in thread From: Maarten Lankhorst @ 2017-03-06 12:06 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx Op 01-03-17 om 16:40 schreef Ville Syrjälä: > On Tue, Feb 28, 2017 at 03:28:48PM +0100, Maarten Lankhorst wrote: >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_sprite.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c >> index 27e0752d1578..375ca91b308c 100644 >> --- a/drivers/gpu/drm/i915/intel_sprite.c >> +++ b/drivers/gpu/drm/i915/intel_sprite.c >> @@ -65,6 +65,8 @@ int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode, >> 1000 * adjusted_mode->crtc_htotal); >> } >> >> +#define VBLANK_EVASION_TIME_US 100 >> + >> /** >> * intel_pipe_update_start() - start update of a set of display registers >> * @crtc: the crtc of which the registers are going to be updated >> @@ -92,7 +94,8 @@ void intel_pipe_update_start(struct intel_crtc *crtc) >> vblank_start = DIV_ROUND_UP(vblank_start, 2); >> >> /* FIXME needs to be calibrated sensibly */ >> - min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, 100); >> + min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, >> + VBLANK_EVASION_TIME_US); >> max = vblank_start - 1; >> >> local_irq_disable(); >> @@ -191,7 +194,12 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work >> ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time), >> crtc->debug.min_vbl, crtc->debug.max_vbl, >> crtc->debug.scanline_start, scanline_end); >> - } >> + } else if (ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time) > >> + VBLANK_EVASION_TIME_US) >> + DRM_WARN("Atomic update on pipe (%c) took %lld us, max time under evasion is %u us\n", >> + pipe_name(pipe), >> + ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time), >> + VBLANK_EVASION_TIME_US); > Hmm. Yeah this should make it very likely that we'll catch any > potential fails even when they don't happen to hit the actual critical > section. Hopefully it won't result in massive amounts of noise. One way > to find out ;) > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks, pushed. Also added a commit message to this commit. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/i915: Move updating color management to before vblank evasion 2017-02-28 14:28 [PATCH 1/2] drm/i915: Move updating color management to before vblank evasion Maarten Lankhorst 2017-02-28 14:28 ` [PATCH 2/2] drm/i915: Complain if we take too long under " Maarten Lankhorst @ 2017-02-28 15:01 ` Ville Syrjälä 2017-03-01 15:38 ` Ville Syrjälä 2017-02-28 18:53 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork 2 siblings, 1 reply; 7+ messages in thread From: Ville Syrjälä @ 2017-02-28 15:01 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Tue, Feb 28, 2017 at 03:28:47PM +0100, Maarten Lankhorst wrote: > This cannot be done reliably during vblank evasasion > since the color management registers are not double buffered. > > The original commit that moved it always during vblank evasion was > wrong, so revert it to before vblank evasion again. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Fixes: 20a34e78f0d7 ("drm/i915: Update color management during vblank evasion.") > Cc: stable@vger.kernel.org # v4.7+ Wasn't there a bugzilla + tested-by ? > --- > drivers/gpu/drm/i915/intel_display.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3c862d0ab389..a0108041fd4a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13301,17 +13301,19 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, > to_intel_atomic_state(old_crtc_state->state); > bool modeset = needs_modeset(crtc->state); > > + if (!modeset && > + (intel_cstate->base.color_mgmt_changed || > + intel_cstate->update_pipe)) { > + intel_color_set_csc(crtc->state); > + intel_color_load_luts(crtc->state); > + } > + > /* Perform vblank evasion around commit operation */ > intel_pipe_update_start(intel_crtc); > > if (modeset) > goto out; > > - if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) { > - intel_color_set_csc(crtc->state); > - intel_color_load_luts(crtc->state); > - } > - > if (intel_cstate->update_pipe) > intel_update_pipe_config(intel_crtc, old_intel_cstate); > else if (INTEL_GEN(dev_priv) >= 9) > -- > 2.7.4 > > _______________________________________________ > 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] 7+ messages in thread
* Re: [PATCH 1/2] drm/i915: Move updating color management to before vblank evasion 2017-02-28 15:01 ` [PATCH 1/2] drm/i915: Move updating color management to before " Ville Syrjälä @ 2017-03-01 15:38 ` Ville Syrjälä 0 siblings, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2017-03-01 15:38 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Tue, Feb 28, 2017 at 05:01:25PM +0200, Ville Syrjälä wrote: > On Tue, Feb 28, 2017 at 03:28:47PM +0100, Maarten Lankhorst wrote: > > This cannot be done reliably during vblank evasasion > > since the color management registers are not double buffered. > > > > The original commit that moved it always during vblank evasion was > > wrong, so revert it to before vblank evasion again. > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Fixes: 20a34e78f0d7 ("drm/i915: Update color management during vblank evasion.") > > Cc: stable@vger.kernel.org # v4.7+ > > Wasn't there a bugzilla + tested-by ? Seems we have two bugzillas now: https://bugs.freedesktop.org/show_bug.cgi?id=91883 https://bugs.freedesktop.org/show_bug.cgi?id=99991 First one is about the atomic update fails, the second about the assert_dsi_pll() mutex vs. irq_disabled() warning. No actual t-bs for this specific variant of the patch (yet) from what I can see. Anyways, this is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > drivers/gpu/drm/i915/intel_display.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 3c862d0ab389..a0108041fd4a 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13301,17 +13301,19 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, > > to_intel_atomic_state(old_crtc_state->state); > > bool modeset = needs_modeset(crtc->state); > > > > + if (!modeset && > > + (intel_cstate->base.color_mgmt_changed || > > + intel_cstate->update_pipe)) { > > + intel_color_set_csc(crtc->state); > > + intel_color_load_luts(crtc->state); > > + } > > + > > /* Perform vblank evasion around commit operation */ > > intel_pipe_update_start(intel_crtc); > > > > if (modeset) > > goto out; > > > > - if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) { > > - intel_color_set_csc(crtc->state); > > - intel_color_load_luts(crtc->state); > > - } > > - > > if (intel_cstate->update_pipe) > > intel_update_pipe_config(intel_crtc, old_intel_cstate); > > else if (INTEL_GEN(dev_priv) >= 9) > > -- > > 2.7.4 > > > > _______________________________________________ > > 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 -- 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] 7+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Move updating color management to before vblank evasion 2017-02-28 14:28 [PATCH 1/2] drm/i915: Move updating color management to before vblank evasion Maarten Lankhorst 2017-02-28 14:28 ` [PATCH 2/2] drm/i915: Complain if we take too long under " Maarten Lankhorst 2017-02-28 15:01 ` [PATCH 1/2] drm/i915: Move updating color management to before " Ville Syrjälä @ 2017-02-28 18:53 ` Patchwork 2 siblings, 0 replies; 7+ messages in thread From: Patchwork @ 2017-02-28 18:53 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915: Move updating color management to before vblank evasion URL : https://patchwork.freedesktop.org/series/20399/ State : success == Summary == Series 20399v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/20399/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#100007 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: skip -> INCOMPLETE (fi-bsw-n3050) fdo#100004 fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007 fdo#100004 https://bugs.freedesktop.org/show_bug.cgi?id=100004 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 fi-bsw-n3050 total:236 pass:201 dwarn:0 dfail:0 fail:0 skip:34 fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 fi-bxt-t5700 total:108 pass:95 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 fi-byt-j1900 failed to collect. IGT log at Patchwork_4006/fi-byt-j1900/igt.log 5d37006b578e38562382215e8782cfced9c992ce drm-tip: 2017y-02m-28d-16h-27m-13s UTC integration manifest 5bced1e drm/i915: Complain if we take too long under vblank evasion. 628d35b drm/i915: Move updating color management to before vblank evasion == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4006/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-06 12:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-28 14:28 [PATCH 1/2] drm/i915: Move updating color management to before vblank evasion Maarten Lankhorst 2017-02-28 14:28 ` [PATCH 2/2] drm/i915: Complain if we take too long under " Maarten Lankhorst 2017-03-01 15:40 ` Ville Syrjälä 2017-03-06 12:06 ` Maarten Lankhorst 2017-02-28 15:01 ` [PATCH 1/2] drm/i915: Move updating color management to before " Ville Syrjälä 2017-03-01 15:38 ` Ville Syrjälä 2017-02-28 18:53 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " 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.