| Message ID | 1311115136-31474-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 19 Jul 2011 15:38:56 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > If a mode set fails we may get a message from drm_crtc_helper if we're lucky, > but it won't tell us anything about *why* we failed to set a mode. So > add a few DRM_ERRORs for the cases that shouldn't happen so we can debug > things more easily. Also fix drm not to simply ignore the error codes would also be useful. But how does logging the error without logging the state help? Presumably you are targeting the unreproducible cases? > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_display.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0f1c799..2434441 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1934,7 +1934,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > > /* no fb bound */ > if (!crtc->fb) { > - DRM_DEBUG_KMS("No FB bound\n"); > + DRM_ERROR("No FB bound\n"); > return 0; > } Should this be flagged as en error then? > > @@ -1943,6 +1943,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > case 1: > break; > default: > + DRM_ERROR("no plane for crtc\n"); > return -EINVAL; > } > > @@ -1952,6 +1953,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > NULL); > if (ret != 0) { > mutex_unlock(&dev->struct_mutex); > + DRM_ERROR("pin & fence failed\n"); In this and the following, report the reason/error code for the failure. -Chris
On Wed, 20 Jul 2011 01:14:47 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Tue, 19 Jul 2011 15:38:56 -0700, Jesse Barnes > <jbarnes@virtuousgeek.org> wrote: > > If a mode set fails we may get a message from drm_crtc_helper if > > we're lucky, but it won't tell us anything about *why* we failed to > > set a mode. So add a few DRM_ERRORs for the cases that shouldn't > > happen so we can debug things more easily. > > Also fix drm not to simply ignore the error codes would also be > useful. But how does logging the error without logging the state > help? Presumably you are targeting the unreproducible cases? I added these to catch a mode setting failure Keith was seeing following a GPU hang. The simple debug info was enough to track things down. But I agree, really the DRM hooks should return ints and give us more info about what happened, and try to roll back the config. > > if (!crtc->fb) { > > - DRM_DEBUG_KMS("No FB bound\n"); > > + DRM_ERROR("No FB bound\n"); > > return 0; > > } > > Should this be flagged as en error then? Possibly, unless we shut off displays this way?? > > @@ -1952,6 +1953,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, > > int x, int y, NULL); > > if (ret != 0) { > > mutex_unlock(&dev->struct_mutex); > > + DRM_ERROR("pin & fence failed\n"); > > In this and the following, report the reason/error code for the > failure. -Chris Sure. Jesse
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0f1c799..2434441 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1934,7 +1934,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, /* no fb bound */ if (!crtc->fb) { - DRM_DEBUG_KMS("No FB bound\n"); + DRM_ERROR("No FB bound\n"); return 0; } @@ -1943,6 +1943,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, case 1: break; default: + DRM_ERROR("no plane for crtc\n"); return -EINVAL; } @@ -1952,6 +1953,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, NULL); if (ret != 0) { mutex_unlock(&dev->struct_mutex); + DRM_ERROR("pin & fence failed\n"); return ret; } @@ -1980,6 +1982,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, if (ret) { i915_gem_object_unpin(to_intel_framebuffer(crtc->fb)->obj); mutex_unlock(&dev->struct_mutex); + DRM_ERROR("failed to update base address\n"); return ret; }
If a mode set fails we may get a message from drm_crtc_helper if we're lucky, but it won't tell us anything about *why* we failed to set a mode. So add a few DRM_ERRORs for the cases that shouldn't happen so we can debug things more easily. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_display.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)