| Message ID | 1441899263-12986-2-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 10, 2015 at 08:34:23AM -0700, Jesse Barnes wrote: > I used these additional fields to track down the issue I saw on HSW. We already have the tracepoints with the scanline information. Not sure what extra this would give. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=91579 > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++++++---- > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 46484e4..90cad50 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -565,6 +565,8 @@ struct intel_crtc { > > unsigned start_vbl_count; > ktime_t start_vbl_time; > + int min_vbl, max_vbl; > + int scanline_start; > > struct intel_crtc_atomic_commit atomic; > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 0c2c62f..ff8c9d5 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -94,6 +94,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > min = vblank_start - usecs_to_scanlines(mode, 100); > max = vblank_start - 1; > > + crtc->min_vbl = min; > + crtc->max_vbl = max; > + > local_irq_disable(); > crtc->start_vbl_count = 0; > > @@ -122,8 +125,10 @@ void intel_pipe_update_start(struct intel_crtc *crtc) > * scanline_offset used to correct the DSL readout.) > */ > scanline = intel_get_crtc_scanline(crtc); > - if (scanline > 2 && (scanline < min || scanline > max)) > + if (scanline > 2 && (scanline < min || scanline > max)) { > + crtc->scanline_start = scanline; > break; > + } > > if (timeout <= 0) { > DRM_ERROR("Potential atomic update failure on pipe %c\n", > @@ -169,10 +174,13 @@ void intel_pipe_update_end(struct intel_crtc *crtc) > > local_irq_enable(); > > - if (crtc->start_vbl_count && crtc->start_vbl_count != end_vbl_count) > - DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us\n", > + if (crtc->start_vbl_count && crtc->start_vbl_count != end_vbl_count) { > + DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n", > pipe_name(pipe), crtc->start_vbl_count, end_vbl_count, > - ktime_us_delta(end_vbl_time, crtc->start_vbl_time)); > + ktime_us_delta(end_vbl_time, crtc->start_vbl_time), > + crtc->min_vbl, crtc->max_vbl, crtc->scanline_start, > + intel_get_crtc_scanline(crtc)); > + } > } > > static void > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 09/10/2015 09:05 AM, Ville Syrjälä wrote: > On Thu, Sep 10, 2015 at 08:34:23AM -0700, Jesse Barnes wrote: >> I used these additional fields to track down the issue I saw on HSW. > > We already have the tracepoints with the scanline information. Not sure > what extra this would give. Saves the trouble of having to dig through tons of tracepoints?
On 09/10/2015 09:05 AM, Ville Syrjälä wrote: > On Thu, Sep 10, 2015 at 08:34:23AM -0700, Jesse Barnes wrote: >> I used these additional fields to track down the issue I saw on HSW. > > We already have the tracepoints with the scanline information. Not sure > what extra this would give. Also when this hits users or test runners, the needed info will be immediately available, rather than having to enable trace points and re-run. I think that's important for error cases like this, as opposed to runtime debugging where tracepoints are fine. Jesse
On Thu, Sep 10, 2015 at 01:23:53PM -0700, Jesse Barnes wrote: > On 09/10/2015 09:05 AM, Ville Syrjälä wrote: > > On Thu, Sep 10, 2015 at 08:34:23AM -0700, Jesse Barnes wrote: > >> I used these additional fields to track down the issue I saw on HSW. > > > > We already have the tracepoints with the scanline information. Not sure > > what extra this would give. > > Also when this hits users or test runners, the needed info will be > immediately available, rather than having to enable trace points and > re-run. I think that's important for error cases like this, as opposed > to runtime debugging where tracepoints are fine. I agree with Jesse here, if things go south we should be able to print useful debug info. And we do have a bunch of rather mysterious pipe update failure backtraces all over already. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 46484e4..90cad50 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -565,6 +565,8 @@ struct intel_crtc { unsigned start_vbl_count; ktime_t start_vbl_time; + int min_vbl, max_vbl; + int scanline_start; struct intel_crtc_atomic_commit atomic; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0c2c62f..ff8c9d5 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -94,6 +94,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc) min = vblank_start - usecs_to_scanlines(mode, 100); max = vblank_start - 1; + crtc->min_vbl = min; + crtc->max_vbl = max; + local_irq_disable(); crtc->start_vbl_count = 0; @@ -122,8 +125,10 @@ void intel_pipe_update_start(struct intel_crtc *crtc) * scanline_offset used to correct the DSL readout.) */ scanline = intel_get_crtc_scanline(crtc); - if (scanline > 2 && (scanline < min || scanline > max)) + if (scanline > 2 && (scanline < min || scanline > max)) { + crtc->scanline_start = scanline; break; + } if (timeout <= 0) { DRM_ERROR("Potential atomic update failure on pipe %c\n", @@ -169,10 +174,13 @@ void intel_pipe_update_end(struct intel_crtc *crtc) local_irq_enable(); - if (crtc->start_vbl_count && crtc->start_vbl_count != end_vbl_count) - DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us\n", + if (crtc->start_vbl_count && crtc->start_vbl_count != end_vbl_count) { + DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n", pipe_name(pipe), crtc->start_vbl_count, end_vbl_count, - ktime_us_delta(end_vbl_time, crtc->start_vbl_time)); + ktime_us_delta(end_vbl_time, crtc->start_vbl_time), + crtc->min_vbl, crtc->max_vbl, crtc->scanline_start, + intel_get_crtc_scanline(crtc)); + } } static void
I used these additional fields to track down the issue I saw on HSW. References: https://bugs.freedesktop.org/show_bug.cgi?id=91579 Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-)