| Message ID | 1476378989-11335-1-git-send-email-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 13, 2016 at 1:16 PM, Rob Clark <robdclark@gmail.com> wrote: > Sometimes we just want to see the atomic state, without getting so many > other debug traces. So add a new drm.debug bit for that. > > Note: it would be nice to put the helpers for printing plane/crtc state > next to plane/crtc state structs.. so someone adding new stuff to the > state structs is more likely to remember to update the corresponding > print_state() fxn. But the header include order for that doesn't really > work out. > > Also, just including the corresponding mdp bits as an example. Dumps > out something like: > > [drm] plane[24]: RGB0 > [drm] crtc=crtc-0 > [drm] fb=35 > [drm] crtc-pos=[0,0, 720x400] > [drm] src-pos=[0,0, 720x400] > [drm] rotation=0 > [drm] premultiplied=0 > [drm] zpos=1 > [drm] alpha=255 > [drm] stage=STAGE0 > [drm] mode_changed=1 > [drm] pending=0 > [drm] crtc[27]: crtc-0 > [drm] enable=1 > [drm] active=0 > [drm] planes_changed=1 > [drm] mode_changed=1 > [drm] active_changed=0 > [drm] connectors_changed=1 > [drm] color_mgmt_changed=0 > [drm] plane_mask=1 > [drm] connector_mask=1 > [drm] encoder_mask=1 > [drm] mode: 0:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x48 0x5 > --- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 3 ++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 28 ++++++++++++++++++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++- > include/drm/drmP.h | 47 ++++++++++++++++++++++++++++++- > 4 files changed, 80 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > index e42f62d..da84179 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, > > DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event); > > + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name); > + drm_print_crtc_state(crtc->state); > + > WARN_ON(mdp5_crtc->event); > > spin_lock_irqsave(&dev->event_lock, flags); > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > index e4b3fb3..466acbc 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > @@ -92,6 +92,22 @@ struct mdp5_plane_state { > #define to_mdp5_plane_state(x) \ > container_of(x, struct mdp5_plane_state, base) > > +static inline const char *stage2name(enum mdp_mixer_stage_id stage); > + > +static inline void > +mdp5_print_plane_state(const struct mdp5_plane_state *state) > +{ > + const struct drm_plane *plane = state->base.plane; > + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name); > + drm_print_plane_state(&state->base); > + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied); > + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos); > + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha); > + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage)); > + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed); > + DRM_DEBUG_STATE("\tpending=%u\n", state->pending); > +} > + > enum mdp5_intf_mode { > MDP5_INTF_MODE_NONE = 0, > > @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 reg) > return msm_readl(mdp5_kms->mmio + reg); > } > > +static inline const char *stage2name(enum mdp_mixer_stage_id stage) > +{ > + static const char *names[] = { > +#define NAME(n) [n] = #n > + NAME(STAGE_UNUSED), NAME(STAGE_BASE), > + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2), > + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6), > +#undef NAME > + }; > + return names[stage]; > +} > + > static inline const char *pipe2name(enum mdp5_pipe pipe) > { > static const char *names[] = { > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > index 432c098..df301df 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, > > DBG("%s: update", mdp5_plane->name); > > + mdp5_print_plane_state(to_mdp5_plane_state(state)); > + I feel like if we add this, we should create a helper drm_atomic_helper_print_state() (or w/e) to dump all of the state at once (on flush), instead of sprinkling calls throughout the driver. So I guess that would require the new helper call + optional helper_funcs for driver-specific crtc/plane fields. Perhaps that's too involved, but IMO it seems like something that'd be easier to gain traction across drivers. Sean > if (!plane_enabled(state)) { > to_mdp5_plane_state(state)->pending = true; > } else if (to_mdp5_plane_state(state)->mode_changed) { > @@ -904,7 +906,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, > type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; > ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, > mdp5_plane->formats, mdp5_plane->nformats, > - type, NULL); > + type, "%s", mdp5_plane->name); > if (ret) > goto fail; > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 28d341a..0ee0aa4 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -133,6 +133,7 @@ struct dma_buf_attachment; > #define DRM_UT_PRIME 0x08 > #define DRM_UT_ATOMIC 0x10 > #define DRM_UT_VBL 0x20 > +#define DRM_UT_STATE 0x40 > > extern __printf(2, 3) > void drm_ut_debug_printk(const char *function_name, > @@ -193,6 +194,8 @@ void drm_err(const char *format, ...); > #define DRM_INFO_ONCE(fmt, ...) \ > printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > > +extern unsigned int drm_debug; > + > /** > * Debug output. > * > @@ -230,6 +233,49 @@ void drm_err(const char *format, ...); > if (unlikely(drm_debug & DRM_UT_VBL)) \ > drm_ut_debug_printk(__func__, fmt, ##args); \ > } while (0) > +#define DRM_DEBUG_STATE(fmt, args...) \ > + do { \ > + if (unlikely(drm_debug & DRM_UT_STATE)) \ > + DRM_INFO(fmt, ##args); \ > + } while (0) > + > + > +static inline void > +drm_print_crtc_state(const struct drm_crtc_state *state) > +{ > + const struct drm_display_mode *mode = &state->mode; > + > + DRM_DEBUG_STATE("\tenable=%d\n", state->enable); > + DRM_DEBUG_STATE("\tactive=%d\n", state->active); > + DRM_DEBUG_STATE("\tplanes_changed=%d\n", state->planes_changed); > + DRM_DEBUG_STATE("\tmode_changed=%d\n", state->mode_changed); > + DRM_DEBUG_STATE("\tactive_changed=%d\n", state->active_changed); > + DRM_DEBUG_STATE("\tconnectors_changed=%d\n", state->connectors_changed); > + DRM_DEBUG_STATE("\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed); > + DRM_DEBUG_STATE("\tplane_mask=%x\n", state->plane_mask); > + DRM_DEBUG_STATE("\tconnector_mask=%x\n", state->connector_mask); > + DRM_DEBUG_STATE("\tencoder_mask=%x\n", state->encoder_mask); > + DRM_DEBUG_STATE("\tmode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n", > + mode->base.id, mode->name, mode->vrefresh, mode->clock, > + mode->hdisplay, mode->hsync_start, > + mode->hsync_end, mode->htotal, > + mode->vdisplay, mode->vsync_start, > + mode->vsync_end, mode->vtotal, mode->type, mode->flags); > +} > + > +static inline void > +drm_print_plane_state(const struct drm_plane_state *state) > +{ > + DRM_DEBUG_STATE("\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); > + DRM_DEBUG_STATE("\tfb=%d\n", state->fb ? state->fb->base.id : -1); > + DRM_DEBUG_STATE("\tcrtc-pos=[%d,%d, %ux%u]\n", > + state->crtc_x, state->crtc_y, > + state->crtc_w, state->crtc_h); > + DRM_DEBUG_STATE("\tsrc-pos=[%u,%u, %ux%u]\n", > + state->src_x >> 16, state->src_y >> 16, > + state->src_w >> 16, state->src_h >> 16); > + DRM_DEBUG_STATE("\trotation=%x\n", state->rotation); > +} > > /*@}*/ > > @@ -988,7 +1034,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc > /* drm_drv.c */ > void drm_put_dev(struct drm_device *dev); > void drm_unplug_dev(struct drm_device *dev); > -extern unsigned int drm_debug; > > /* Debugfs support */ > #if defined(CONFIG_DEBUG_FS) > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Oct 13, 2016 at 1:38 PM, Sean Paul <seanpaul@chromium.org> wrote: > On Thu, Oct 13, 2016 at 1:16 PM, Rob Clark <robdclark@gmail.com> wrote: >> Sometimes we just want to see the atomic state, without getting so many >> other debug traces. So add a new drm.debug bit for that. >> >> Note: it would be nice to put the helpers for printing plane/crtc state >> next to plane/crtc state structs.. so someone adding new stuff to the >> state structs is more likely to remember to update the corresponding >> print_state() fxn. But the header include order for that doesn't really >> work out. >> >> Also, just including the corresponding mdp bits as an example. Dumps >> out something like: >> >> [drm] plane[24]: RGB0 >> [drm] crtc=crtc-0 >> [drm] fb=35 >> [drm] crtc-pos=[0,0, 720x400] >> [drm] src-pos=[0,0, 720x400] >> [drm] rotation=0 >> [drm] premultiplied=0 >> [drm] zpos=1 >> [drm] alpha=255 >> [drm] stage=STAGE0 >> [drm] mode_changed=1 >> [drm] pending=0 >> [drm] crtc[27]: crtc-0 >> [drm] enable=1 >> [drm] active=0 >> [drm] planes_changed=1 >> [drm] mode_changed=1 >> [drm] active_changed=0 >> [drm] connectors_changed=1 >> [drm] color_mgmt_changed=0 >> [drm] plane_mask=1 >> [drm] connector_mask=1 >> [drm] encoder_mask=1 >> [drm] mode: 0:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x48 0x5 >> --- >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 3 ++ >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 28 ++++++++++++++++++ >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++- >> include/drm/drmP.h | 47 ++++++++++++++++++++++++++++++- >> 4 files changed, 80 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c >> index e42f62d..da84179 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c >> @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, >> >> DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event); >> >> + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name); >> + drm_print_crtc_state(crtc->state); >> + >> WARN_ON(mdp5_crtc->event); >> >> spin_lock_irqsave(&dev->event_lock, flags); >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h >> index e4b3fb3..466acbc 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h >> @@ -92,6 +92,22 @@ struct mdp5_plane_state { >> #define to_mdp5_plane_state(x) \ >> container_of(x, struct mdp5_plane_state, base) >> >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage); >> + >> +static inline void >> +mdp5_print_plane_state(const struct mdp5_plane_state *state) >> +{ >> + const struct drm_plane *plane = state->base.plane; >> + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name); >> + drm_print_plane_state(&state->base); >> + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied); >> + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos); >> + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha); >> + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage)); >> + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed); >> + DRM_DEBUG_STATE("\tpending=%u\n", state->pending); >> +} >> + >> enum mdp5_intf_mode { >> MDP5_INTF_MODE_NONE = 0, >> >> @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 reg) >> return msm_readl(mdp5_kms->mmio + reg); >> } >> >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage) >> +{ >> + static const char *names[] = { >> +#define NAME(n) [n] = #n >> + NAME(STAGE_UNUSED), NAME(STAGE_BASE), >> + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2), >> + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6), >> +#undef NAME >> + }; >> + return names[stage]; >> +} >> + >> static inline const char *pipe2name(enum mdp5_pipe pipe) >> { >> static const char *names[] = { >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> index 432c098..df301df 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, >> >> DBG("%s: update", mdp5_plane->name); >> >> + mdp5_print_plane_state(to_mdp5_plane_state(state)); >> + > > I feel like if we add this, we should create a helper > drm_atomic_helper_print_state() (or w/e) to dump all of the state at > once (on flush), instead of sprinkling calls throughout the driver. So > I guess that would require the new helper call + optional helper_funcs > for driver-specific crtc/plane fields. > > Perhaps that's too involved, but IMO it seems like something that'd be > easier to gain traction across drivers. > yeah, sprinkling it in driver (at least for now) was basically just to avoid having to add new print_state() hooks.. I guess hooks could be made optional, and just call the drm_print_xyz_state() helper directly to print core state if driver doesn't override. For now didn't want to make it too fancy in case it turned out to be throw-away debug code for debugging $current_problem ;-) BR, -R > Sean > > >> if (!plane_enabled(state)) { >> to_mdp5_plane_state(state)->pending = true; >> } else if (to_mdp5_plane_state(state)->mode_changed) { >> @@ -904,7 +906,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, >> type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; >> ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, >> mdp5_plane->formats, mdp5_plane->nformats, >> - type, NULL); >> + type, "%s", mdp5_plane->name); >> if (ret) >> goto fail; >> >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >> index 28d341a..0ee0aa4 100644 >> --- a/include/drm/drmP.h >> +++ b/include/drm/drmP.h >> @@ -133,6 +133,7 @@ struct dma_buf_attachment; >> #define DRM_UT_PRIME 0x08 >> #define DRM_UT_ATOMIC 0x10 >> #define DRM_UT_VBL 0x20 >> +#define DRM_UT_STATE 0x40 >> >> extern __printf(2, 3) >> void drm_ut_debug_printk(const char *function_name, >> @@ -193,6 +194,8 @@ void drm_err(const char *format, ...); >> #define DRM_INFO_ONCE(fmt, ...) \ >> printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) >> >> +extern unsigned int drm_debug; >> + >> /** >> * Debug output. >> * >> @@ -230,6 +233,49 @@ void drm_err(const char *format, ...); >> if (unlikely(drm_debug & DRM_UT_VBL)) \ >> drm_ut_debug_printk(__func__, fmt, ##args); \ >> } while (0) >> +#define DRM_DEBUG_STATE(fmt, args...) \ >> + do { \ >> + if (unlikely(drm_debug & DRM_UT_STATE)) \ >> + DRM_INFO(fmt, ##args); \ >> + } while (0) >> + >> + >> +static inline void >> +drm_print_crtc_state(const struct drm_crtc_state *state) >> +{ >> + const struct drm_display_mode *mode = &state->mode; >> + >> + DRM_DEBUG_STATE("\tenable=%d\n", state->enable); >> + DRM_DEBUG_STATE("\tactive=%d\n", state->active); >> + DRM_DEBUG_STATE("\tplanes_changed=%d\n", state->planes_changed); >> + DRM_DEBUG_STATE("\tmode_changed=%d\n", state->mode_changed); >> + DRM_DEBUG_STATE("\tactive_changed=%d\n", state->active_changed); >> + DRM_DEBUG_STATE("\tconnectors_changed=%d\n", state->connectors_changed); >> + DRM_DEBUG_STATE("\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed); >> + DRM_DEBUG_STATE("\tplane_mask=%x\n", state->plane_mask); >> + DRM_DEBUG_STATE("\tconnector_mask=%x\n", state->connector_mask); >> + DRM_DEBUG_STATE("\tencoder_mask=%x\n", state->encoder_mask); >> + DRM_DEBUG_STATE("\tmode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n", >> + mode->base.id, mode->name, mode->vrefresh, mode->clock, >> + mode->hdisplay, mode->hsync_start, >> + mode->hsync_end, mode->htotal, >> + mode->vdisplay, mode->vsync_start, >> + mode->vsync_end, mode->vtotal, mode->type, mode->flags); >> +} >> + >> +static inline void >> +drm_print_plane_state(const struct drm_plane_state *state) >> +{ >> + DRM_DEBUG_STATE("\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); >> + DRM_DEBUG_STATE("\tfb=%d\n", state->fb ? state->fb->base.id : -1); >> + DRM_DEBUG_STATE("\tcrtc-pos=[%d,%d, %ux%u]\n", >> + state->crtc_x, state->crtc_y, >> + state->crtc_w, state->crtc_h); >> + DRM_DEBUG_STATE("\tsrc-pos=[%u,%u, %ux%u]\n", >> + state->src_x >> 16, state->src_y >> 16, >> + state->src_w >> 16, state->src_h >> 16); >> + DRM_DEBUG_STATE("\trotation=%x\n", state->rotation); >> +} >> >> /*@}*/ >> >> @@ -988,7 +1034,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc >> /* drm_drv.c */ >> void drm_put_dev(struct drm_device *dev); >> void drm_unplug_dev(struct drm_device *dev); >> -extern unsigned int drm_debug; >> >> /* Debugfs support */ >> #if defined(CONFIG_DEBUG_FS) >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Oct 13, 2016 at 01:16:29PM -0400, Rob Clark wrote: > Sometimes we just want to see the atomic state, without getting so many > other debug traces. So add a new drm.debug bit for that. > > Note: it would be nice to put the helpers for printing plane/crtc state > next to plane/crtc state structs.. so someone adding new stuff to the > state structs is more likely to remember to update the corresponding > print_state() fxn. But the header include order for that doesn't really > work out. > > Also, just including the corresponding mdp bits as an example. Dumps > out something like: > > [drm] plane[24]: RGB0 > [drm] crtc=crtc-0 > [drm] fb=35 > [drm] crtc-pos=[0,0, 720x400] > [drm] src-pos=[0,0, 720x400] > [drm] rotation=0 > [drm] premultiplied=0 > [drm] zpos=1 > [drm] alpha=255 > [drm] stage=STAGE0 > [drm] mode_changed=1 > [drm] pending=0 > [drm] crtc[27]: crtc-0 > [drm] enable=1 > [drm] active=0 > [drm] planes_changed=1 > [drm] mode_changed=1 > [drm] active_changed=0 > [drm] connectors_changed=1 > [drm] color_mgmt_changed=0 > [drm] plane_mask=1 > [drm] connector_mask=1 > [drm] encoder_mask=1 > [drm] mode: 0:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x48 0x5 > --- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 3 ++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 28 ++++++++++++++++++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++- > include/drm/drmP.h | 47 ++++++++++++++++++++++++++++++- > 4 files changed, 80 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > index e42f62d..da84179 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, > > DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event); > > + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name); > + drm_print_crtc_state(crtc->state); > + > WARN_ON(mdp5_crtc->event); > > spin_lock_irqsave(&dev->event_lock, flags); > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > index e4b3fb3..466acbc 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > @@ -92,6 +92,22 @@ struct mdp5_plane_state { > #define to_mdp5_plane_state(x) \ > container_of(x, struct mdp5_plane_state, base) > > +static inline const char *stage2name(enum mdp_mixer_stage_id stage); > + > +static inline void > +mdp5_print_plane_state(const struct mdp5_plane_state *state) > +{ > + const struct drm_plane *plane = state->base.plane; > + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name); > + drm_print_plane_state(&state->base); > + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied); > + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos); > + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha); > + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage)); > + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed); > + DRM_DEBUG_STATE("\tpending=%u\n", state->pending); > +} > + > enum mdp5_intf_mode { > MDP5_INTF_MODE_NONE = 0, > > @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 reg) > return msm_readl(mdp5_kms->mmio + reg); > } > > +static inline const char *stage2name(enum mdp_mixer_stage_id stage) > +{ > + static const char *names[] = { > +#define NAME(n) [n] = #n > + NAME(STAGE_UNUSED), NAME(STAGE_BASE), > + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2), > + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6), > +#undef NAME > + }; > + return names[stage]; > +} > + > static inline const char *pipe2name(enum mdp5_pipe pipe) > { > static const char *names[] = { > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > index 432c098..df301df 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, > > DBG("%s: update", mdp5_plane->name); > > + mdp5_print_plane_state(to_mdp5_plane_state(state)); > + > if (!plane_enabled(state)) { > to_mdp5_plane_state(state)->pending = true; > } else if (to_mdp5_plane_state(state)->mode_changed) { > @@ -904,7 +906,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, > type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; > ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, > mdp5_plane->formats, mdp5_plane->nformats, > - type, NULL); > + type, "%s", mdp5_plane->name); > if (ret) > goto fail; > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 28d341a..0ee0aa4 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -133,6 +133,7 @@ struct dma_buf_attachment; > #define DRM_UT_PRIME 0x08 > #define DRM_UT_ATOMIC 0x10 > #define DRM_UT_VBL 0x20 > +#define DRM_UT_STATE 0x40 > > extern __printf(2, 3) > void drm_ut_debug_printk(const char *function_name, > @@ -193,6 +194,8 @@ void drm_err(const char *format, ...); > #define DRM_INFO_ONCE(fmt, ...) \ > printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > > +extern unsigned int drm_debug; > + > /** > * Debug output. > * > @@ -230,6 +233,49 @@ void drm_err(const char *format, ...); > if (unlikely(drm_debug & DRM_UT_VBL)) \ > drm_ut_debug_printk(__func__, fmt, ##args); \ > } while (0) > +#define DRM_DEBUG_STATE(fmt, args...) \ > + do { \ > + if (unlikely(drm_debug & DRM_UT_STATE)) \ > + DRM_INFO(fmt, ##args); \ > + } while (0) > + > + > +static inline void > +drm_print_crtc_state(const struct drm_crtc_state *state) > +{ > + const struct drm_display_mode *mode = &state->mode; > + > + DRM_DEBUG_STATE("\tenable=%d\n", state->enable); > + DRM_DEBUG_STATE("\tactive=%d\n", state->active); > + DRM_DEBUG_STATE("\tplanes_changed=%d\n", state->planes_changed); > + DRM_DEBUG_STATE("\tmode_changed=%d\n", state->mode_changed); > + DRM_DEBUG_STATE("\tactive_changed=%d\n", state->active_changed); > + DRM_DEBUG_STATE("\tconnectors_changed=%d\n", state->connectors_changed); > + DRM_DEBUG_STATE("\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed); > + DRM_DEBUG_STATE("\tplane_mask=%x\n", state->plane_mask); > + DRM_DEBUG_STATE("\tconnector_mask=%x\n", state->connector_mask); > + DRM_DEBUG_STATE("\tencoder_mask=%x\n", state->encoder_mask); > + DRM_DEBUG_STATE("\tmode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n", > + mode->base.id, mode->name, mode->vrefresh, mode->clock, > + mode->hdisplay, mode->hsync_start, > + mode->hsync_end, mode->htotal, > + mode->vdisplay, mode->vsync_start, > + mode->vsync_end, mode->vtotal, mode->type, mode->flags); > +} > + > +static inline void > +drm_print_plane_state(const struct drm_plane_state *state) > +{ > + DRM_DEBUG_STATE("\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); > + DRM_DEBUG_STATE("\tfb=%d\n", state->fb ? state->fb->base.id : -1); -1 doesn't look like a legit thing. 0 would mean "not there" I think. I wonder if we want to dump out more about the fb here too? size+fmt perhaps? > + DRM_DEBUG_STATE("\tcrtc-pos=[%d,%d, %ux%u]\n", > + state->crtc_x, state->crtc_y, > + state->crtc_w, state->crtc_h); > + DRM_DEBUG_STATE("\tsrc-pos=[%u,%u, %ux%u]\n", > + state->src_x >> 16, state->src_y >> 16, > + state->src_w >> 16, state->src_h >> 16); These should be printed with the fractional part included. Also your "[...]" thing doesn't match the format I've tried to use elsewhere for these things. Maybe yank the stuff from the drm_rect_print() thing into a few macros that can be used to print them easily anywhere. Something like #define FMT_FIXED_16_16 "...." #define something(x) ... printk("blah " FMT_FIXED_16_16 " bleh\n", something(state->src_x)); maybe? Maybe we should have something like that for printing out modes too? In case we want to mix up mode dumps with other output on the same line. > + DRM_DEBUG_STATE("\trotation=%x\n", state->rotation); Should we dump out all the derived state too? The problem is of course that it might not be up to date yet, so when exactly you print it would affect whether you stale data or not. Maybe dump it only based on some bool function argument? > +} > > /*@}*/ > > @@ -988,7 +1034,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc > /* drm_drv.c */ > void drm_put_dev(struct drm_device *dev); > void drm_unplug_dev(struct drm_device *dev); > -extern unsigned int drm_debug; > > /* Debugfs support */ > #if defined(CONFIG_DEBUG_FS) > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Oct 13, 2016 at 01:38:30PM -0400, Sean Paul wrote: > On Thu, Oct 13, 2016 at 1:16 PM, Rob Clark <robdclark@gmail.com> wrote: > > Sometimes we just want to see the atomic state, without getting so many > > other debug traces. So add a new drm.debug bit for that. > > > > Note: it would be nice to put the helpers for printing plane/crtc state > > next to plane/crtc state structs.. so someone adding new stuff to the > > state structs is more likely to remember to update the corresponding > > print_state() fxn. But the header include order for that doesn't really > > work out. > > > > Also, just including the corresponding mdp bits as an example. Dumps > > out something like: > > > > [drm] plane[24]: RGB0 > > [drm] crtc=crtc-0 > > [drm] fb=35 > > [drm] crtc-pos=[0,0, 720x400] > > [drm] src-pos=[0,0, 720x400] > > [drm] rotation=0 > > [drm] premultiplied=0 > > [drm] zpos=1 > > [drm] alpha=255 > > [drm] stage=STAGE0 > > [drm] mode_changed=1 > > [drm] pending=0 > > [drm] crtc[27]: crtc-0 > > [drm] enable=1 > > [drm] active=0 > > [drm] planes_changed=1 > > [drm] mode_changed=1 > > [drm] active_changed=0 > > [drm] connectors_changed=1 > > [drm] color_mgmt_changed=0 > > [drm] plane_mask=1 > > [drm] connector_mask=1 > > [drm] encoder_mask=1 > > [drm] mode: 0:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x48 0x5 > > --- > > drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 3 ++ > > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 28 ++++++++++++++++++ > > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++- > > include/drm/drmP.h | 47 ++++++++++++++++++++++++++++++- > > 4 files changed, 80 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > > index e42f62d..da84179 100644 > > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > > @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, > > > > DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event); > > > > + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name); > > + drm_print_crtc_state(crtc->state); > > + > > WARN_ON(mdp5_crtc->event); > > > > spin_lock_irqsave(&dev->event_lock, flags); > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > > index e4b3fb3..466acbc 100644 > > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > > @@ -92,6 +92,22 @@ struct mdp5_plane_state { > > #define to_mdp5_plane_state(x) \ > > container_of(x, struct mdp5_plane_state, base) > > > > +static inline const char *stage2name(enum mdp_mixer_stage_id stage); > > + > > +static inline void > > +mdp5_print_plane_state(const struct mdp5_plane_state *state) > > +{ > > + const struct drm_plane *plane = state->base.plane; > > + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name); > > + drm_print_plane_state(&state->base); > > + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied); > > + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos); > > + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha); > > + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage)); > > + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed); > > + DRM_DEBUG_STATE("\tpending=%u\n", state->pending); > > +} > > + > > enum mdp5_intf_mode { > > MDP5_INTF_MODE_NONE = 0, > > > > @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 reg) > > return msm_readl(mdp5_kms->mmio + reg); > > } > > > > +static inline const char *stage2name(enum mdp_mixer_stage_id stage) > > +{ > > + static const char *names[] = { > > +#define NAME(n) [n] = #n > > + NAME(STAGE_UNUSED), NAME(STAGE_BASE), > > + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2), > > + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6), > > +#undef NAME > > + }; > > + return names[stage]; > > +} > > + > > static inline const char *pipe2name(enum mdp5_pipe pipe) > > { > > static const char *names[] = { > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > index 432c098..df301df 100644 > > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > > @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, > > > > DBG("%s: update", mdp5_plane->name); > > > > + mdp5_print_plane_state(to_mdp5_plane_state(state)); > > + > > I feel like if we add this, we should create a helper > drm_atomic_helper_print_state() (or w/e) to dump all of the state at > once (on flush), instead of sprinkling calls throughout the driver. So > I guess that would require the new helper call + optional helper_funcs > for driver-specific crtc/plane fields. > > Perhaps that's too involved, but IMO it seems like something that'd be > easier to gain traction across drivers. Well people might want to print it out from various places during debugging, so exposing the function at least would be a good help so that people don't have to reinvent the same thing every time. > > Sean > > > > if (!plane_enabled(state)) { > > to_mdp5_plane_state(state)->pending = true; > > } else if (to_mdp5_plane_state(state)->mode_changed) { > > @@ -904,7 +906,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, > > type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; > > ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, > > mdp5_plane->formats, mdp5_plane->nformats, > > - type, NULL); > > + type, "%s", mdp5_plane->name); > > if (ret) > > goto fail; > > > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > > index 28d341a..0ee0aa4 100644 > > --- a/include/drm/drmP.h > > +++ b/include/drm/drmP.h > > @@ -133,6 +133,7 @@ struct dma_buf_attachment; > > #define DRM_UT_PRIME 0x08 > > #define DRM_UT_ATOMIC 0x10 > > #define DRM_UT_VBL 0x20 > > +#define DRM_UT_STATE 0x40 > > > > extern __printf(2, 3) > > void drm_ut_debug_printk(const char *function_name, > > @@ -193,6 +194,8 @@ void drm_err(const char *format, ...); > > #define DRM_INFO_ONCE(fmt, ...) \ > > printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > > > > +extern unsigned int drm_debug; > > + > > /** > > * Debug output. > > * > > @@ -230,6 +233,49 @@ void drm_err(const char *format, ...); > > if (unlikely(drm_debug & DRM_UT_VBL)) \ > > drm_ut_debug_printk(__func__, fmt, ##args); \ > > } while (0) > > +#define DRM_DEBUG_STATE(fmt, args...) \ > > + do { \ > > + if (unlikely(drm_debug & DRM_UT_STATE)) \ > > + DRM_INFO(fmt, ##args); \ > > + } while (0) > > + > > + > > +static inline void > > +drm_print_crtc_state(const struct drm_crtc_state *state) > > +{ > > + const struct drm_display_mode *mode = &state->mode; > > + > > + DRM_DEBUG_STATE("\tenable=%d\n", state->enable); > > + DRM_DEBUG_STATE("\tactive=%d\n", state->active); > > + DRM_DEBUG_STATE("\tplanes_changed=%d\n", state->planes_changed); > > + DRM_DEBUG_STATE("\tmode_changed=%d\n", state->mode_changed); > > + DRM_DEBUG_STATE("\tactive_changed=%d\n", state->active_changed); > > + DRM_DEBUG_STATE("\tconnectors_changed=%d\n", state->connectors_changed); > > + DRM_DEBUG_STATE("\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed); > > + DRM_DEBUG_STATE("\tplane_mask=%x\n", state->plane_mask); > > + DRM_DEBUG_STATE("\tconnector_mask=%x\n", state->connector_mask); > > + DRM_DEBUG_STATE("\tencoder_mask=%x\n", state->encoder_mask); > > + DRM_DEBUG_STATE("\tmode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n", > > + mode->base.id, mode->name, mode->vrefresh, mode->clock, > > + mode->hdisplay, mode->hsync_start, > > + mode->hsync_end, mode->htotal, > > + mode->vdisplay, mode->vsync_start, > > + mode->vsync_end, mode->vtotal, mode->type, mode->flags); > > +} > > + > > +static inline void > > +drm_print_plane_state(const struct drm_plane_state *state) > > +{ > > + DRM_DEBUG_STATE("\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); > > + DRM_DEBUG_STATE("\tfb=%d\n", state->fb ? state->fb->base.id : -1); > > + DRM_DEBUG_STATE("\tcrtc-pos=[%d,%d, %ux%u]\n", > > + state->crtc_x, state->crtc_y, > > + state->crtc_w, state->crtc_h); > > + DRM_DEBUG_STATE("\tsrc-pos=[%u,%u, %ux%u]\n", > > + state->src_x >> 16, state->src_y >> 16, > > + state->src_w >> 16, state->src_h >> 16); > > + DRM_DEBUG_STATE("\trotation=%x\n", state->rotation); > > +} > > > > /*@}*/ > > > > @@ -988,7 +1034,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc > > /* drm_drv.c */ > > void drm_put_dev(struct drm_device *dev); > > void drm_unplug_dev(struct drm_device *dev); > > -extern unsigned int drm_debug; > > > > /* Debugfs support */ > > #if defined(CONFIG_DEBUG_FS) > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Oct 13, 2016 at 2:24 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, Oct 13, 2016 at 01:16:29PM -0400, Rob Clark wrote: >> Sometimes we just want to see the atomic state, without getting so many >> other debug traces. So add a new drm.debug bit for that. >> >> Note: it would be nice to put the helpers for printing plane/crtc state >> next to plane/crtc state structs.. so someone adding new stuff to the >> state structs is more likely to remember to update the corresponding >> print_state() fxn. But the header include order for that doesn't really >> work out. >> >> Also, just including the corresponding mdp bits as an example. Dumps >> out something like: >> >> [drm] plane[24]: RGB0 >> [drm] crtc=crtc-0 >> [drm] fb=35 >> [drm] crtc-pos=[0,0, 720x400] >> [drm] src-pos=[0,0, 720x400] >> [drm] rotation=0 >> [drm] premultiplied=0 >> [drm] zpos=1 >> [drm] alpha=255 >> [drm] stage=STAGE0 >> [drm] mode_changed=1 >> [drm] pending=0 >> [drm] crtc[27]: crtc-0 >> [drm] enable=1 >> [drm] active=0 >> [drm] planes_changed=1 >> [drm] mode_changed=1 >> [drm] active_changed=0 >> [drm] connectors_changed=1 >> [drm] color_mgmt_changed=0 >> [drm] plane_mask=1 >> [drm] connector_mask=1 >> [drm] encoder_mask=1 >> [drm] mode: 0:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x48 0x5 >> --- >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 3 ++ >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 28 ++++++++++++++++++ >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++- >> include/drm/drmP.h | 47 ++++++++++++++++++++++++++++++- >> 4 files changed, 80 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c >> index e42f62d..da84179 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c >> @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, >> >> DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event); >> >> + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name); >> + drm_print_crtc_state(crtc->state); >> + >> WARN_ON(mdp5_crtc->event); >> >> spin_lock_irqsave(&dev->event_lock, flags); >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h >> index e4b3fb3..466acbc 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h >> @@ -92,6 +92,22 @@ struct mdp5_plane_state { >> #define to_mdp5_plane_state(x) \ >> container_of(x, struct mdp5_plane_state, base) >> >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage); >> + >> +static inline void >> +mdp5_print_plane_state(const struct mdp5_plane_state *state) >> +{ >> + const struct drm_plane *plane = state->base.plane; >> + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name); >> + drm_print_plane_state(&state->base); >> + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied); >> + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos); >> + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha); >> + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage)); >> + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed); >> + DRM_DEBUG_STATE("\tpending=%u\n", state->pending); >> +} >> + >> enum mdp5_intf_mode { >> MDP5_INTF_MODE_NONE = 0, >> >> @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 reg) >> return msm_readl(mdp5_kms->mmio + reg); >> } >> >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage) >> +{ >> + static const char *names[] = { >> +#define NAME(n) [n] = #n >> + NAME(STAGE_UNUSED), NAME(STAGE_BASE), >> + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2), >> + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6), >> +#undef NAME >> + }; >> + return names[stage]; >> +} >> + >> static inline const char *pipe2name(enum mdp5_pipe pipe) >> { >> static const char *names[] = { >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> index 432c098..df301df 100644 >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c >> @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, >> >> DBG("%s: update", mdp5_plane->name); >> >> + mdp5_print_plane_state(to_mdp5_plane_state(state)); >> + >> if (!plane_enabled(state)) { >> to_mdp5_plane_state(state)->pending = true; >> } else if (to_mdp5_plane_state(state)->mode_changed) { >> @@ -904,7 +906,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, >> type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; >> ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, >> mdp5_plane->formats, mdp5_plane->nformats, >> - type, NULL); >> + type, "%s", mdp5_plane->name); >> if (ret) >> goto fail; >> >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >> index 28d341a..0ee0aa4 100644 >> --- a/include/drm/drmP.h >> +++ b/include/drm/drmP.h >> @@ -133,6 +133,7 @@ struct dma_buf_attachment; >> #define DRM_UT_PRIME 0x08 >> #define DRM_UT_ATOMIC 0x10 >> #define DRM_UT_VBL 0x20 >> +#define DRM_UT_STATE 0x40 >> >> extern __printf(2, 3) >> void drm_ut_debug_printk(const char *function_name, >> @@ -193,6 +194,8 @@ void drm_err(const char *format, ...); >> #define DRM_INFO_ONCE(fmt, ...) \ >> printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) >> >> +extern unsigned int drm_debug; >> + >> /** >> * Debug output. >> * >> @@ -230,6 +233,49 @@ void drm_err(const char *format, ...); >> if (unlikely(drm_debug & DRM_UT_VBL)) \ >> drm_ut_debug_printk(__func__, fmt, ##args); \ >> } while (0) >> +#define DRM_DEBUG_STATE(fmt, args...) \ >> + do { \ >> + if (unlikely(drm_debug & DRM_UT_STATE)) \ >> + DRM_INFO(fmt, ##args); \ >> + } while (0) >> + >> + >> +static inline void >> +drm_print_crtc_state(const struct drm_crtc_state *state) >> +{ >> + const struct drm_display_mode *mode = &state->mode; >> + >> + DRM_DEBUG_STATE("\tenable=%d\n", state->enable); >> + DRM_DEBUG_STATE("\tactive=%d\n", state->active); >> + DRM_DEBUG_STATE("\tplanes_changed=%d\n", state->planes_changed); >> + DRM_DEBUG_STATE("\tmode_changed=%d\n", state->mode_changed); >> + DRM_DEBUG_STATE("\tactive_changed=%d\n", state->active_changed); >> + DRM_DEBUG_STATE("\tconnectors_changed=%d\n", state->connectors_changed); >> + DRM_DEBUG_STATE("\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed); >> + DRM_DEBUG_STATE("\tplane_mask=%x\n", state->plane_mask); >> + DRM_DEBUG_STATE("\tconnector_mask=%x\n", state->connector_mask); >> + DRM_DEBUG_STATE("\tencoder_mask=%x\n", state->encoder_mask); >> + DRM_DEBUG_STATE("\tmode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n", >> + mode->base.id, mode->name, mode->vrefresh, mode->clock, >> + mode->hdisplay, mode->hsync_start, >> + mode->hsync_end, mode->htotal, >> + mode->vdisplay, mode->vsync_start, >> + mode->vsync_end, mode->vtotal, mode->type, mode->flags); >> +} >> + >> +static inline void >> +drm_print_plane_state(const struct drm_plane_state *state) >> +{ >> + DRM_DEBUG_STATE("\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); >> + DRM_DEBUG_STATE("\tfb=%d\n", state->fb ? state->fb->base.id : -1); > > -1 doesn't look like a legit thing. 0 would mean "not there" I think. hmm, yeah, I guess zero isn't a valid mode-object id.. > I wonder if we want to dump out more about the fb here too? size+fmt > perhaps? yeah, sounds reasonable >> + DRM_DEBUG_STATE("\tcrtc-pos=[%d,%d, %ux%u]\n", >> + state->crtc_x, state->crtc_y, >> + state->crtc_w, state->crtc_h); >> + DRM_DEBUG_STATE("\tsrc-pos=[%u,%u, %ux%u]\n", >> + state->src_x >> 16, state->src_y >> 16, >> + state->src_w >> 16, state->src_h >> 16); > > These should be printed with the fractional part included. Also your > "[...]" thing doesn't match the format I've tried to use elsewhere for > these things. Maybe yank the stuff from the drm_rect_print() thing > into a few macros that can be used to print them easily anywhere. > Something like > > #define FMT_FIXED_16_16 "...." > #define something(x) ... > printk("blah " FMT_FIXED_16_16 " bleh\n", something(state->src_x)); > > maybe? > > Maybe we should have something like that for printing out modes too? > In case we want to mix up mode dumps with other output on the same > line. yeah, I basically cut/paste from drm_mode_debug_printmodeline().. so maybe some fmt and "splitter" macros makes sense.. >> + DRM_DEBUG_STATE("\trotation=%x\n", state->rotation); > > Should we dump out all the derived state too? The problem is of course > that it might not be up to date yet, so when exactly you print it would > affect whether you stale data or not. Maybe dump it only based on some > bool function argument? not sure what you meant by derived state? Although I guess we could translate the rotation to strings.. btw, I'm slightly bummed that no one has invented a way to make seq_file that goes to log.. re-using same dump_state() fxns for both debugfs and printk would be nice. (And perhaps be enough of an argument to add (*print_state)() fxn ptrs into drm_{plane,crtc}_funcs..) BR, -R >> +} >> >> /*@}*/ >> >> @@ -988,7 +1034,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc >> /* drm_drv.c */ >> void drm_put_dev(struct drm_device *dev); >> void drm_unplug_dev(struct drm_device *dev); >> -extern unsigned int drm_debug; >> >> /* Debugfs support */ >> #if defined(CONFIG_DEBUG_FS) >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC
On Thu, Oct 13, 2016 at 04:14:04PM -0400, Rob Clark wrote: > On Thu, Oct 13, 2016 at 2:24 PM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > On Thu, Oct 13, 2016 at 01:16:29PM -0400, Rob Clark wrote: > >> Sometimes we just want to see the atomic state, without getting so many > >> other debug traces. So add a new drm.debug bit for that. > >> > >> Note: it would be nice to put the helpers for printing plane/crtc state > >> next to plane/crtc state structs.. so someone adding new stuff to the > >> state structs is more likely to remember to update the corresponding > >> print_state() fxn. But the header include order for that doesn't really > >> work out. > >> > >> Also, just including the corresponding mdp bits as an example. Dumps > >> out something like: > >> > >> [drm] plane[24]: RGB0 > >> [drm] crtc=crtc-0 > >> [drm] fb=35 > >> [drm] crtc-pos=[0,0, 720x400] > >> [drm] src-pos=[0,0, 720x400] > >> [drm] rotation=0 > >> [drm] premultiplied=0 > >> [drm] zpos=1 > >> [drm] alpha=255 > >> [drm] stage=STAGE0 > >> [drm] mode_changed=1 > >> [drm] pending=0 > >> [drm] crtc[27]: crtc-0 > >> [drm] enable=1 > >> [drm] active=0 > >> [drm] planes_changed=1 > >> [drm] mode_changed=1 > >> [drm] active_changed=0 > >> [drm] connectors_changed=1 > >> [drm] color_mgmt_changed=0 > >> [drm] plane_mask=1 > >> [drm] connector_mask=1 > >> [drm] encoder_mask=1 > >> [drm] mode: 0:"1920x1080" 60 148500 1920 2008 2052 2200 1080 1084 1089 1125 0x48 0x5 > >> --- > >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 3 ++ > >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 28 ++++++++++++++++++ > >> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 ++- > >> include/drm/drmP.h | 47 ++++++++++++++++++++++++++++++- > >> 4 files changed, 80 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > >> index e42f62d..da84179 100644 > >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > >> @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, > >> > >> DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event); > >> > >> + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name); > >> + drm_print_crtc_state(crtc->state); > >> + > >> WARN_ON(mdp5_crtc->event); > >> > >> spin_lock_irqsave(&dev->event_lock, flags); > >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > >> index e4b3fb3..466acbc 100644 > >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > >> @@ -92,6 +92,22 @@ struct mdp5_plane_state { > >> #define to_mdp5_plane_state(x) \ > >> container_of(x, struct mdp5_plane_state, base) > >> > >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage); > >> + > >> +static inline void > >> +mdp5_print_plane_state(const struct mdp5_plane_state *state) > >> +{ > >> + const struct drm_plane *plane = state->base.plane; > >> + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name); > >> + drm_print_plane_state(&state->base); > >> + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied); > >> + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos); > >> + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha); > >> + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage)); > >> + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed); > >> + DRM_DEBUG_STATE("\tpending=%u\n", state->pending); > >> +} > >> + > >> enum mdp5_intf_mode { > >> MDP5_INTF_MODE_NONE = 0, > >> > >> @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 reg) > >> return msm_readl(mdp5_kms->mmio + reg); > >> } > >> > >> +static inline const char *stage2name(enum mdp_mixer_stage_id stage) > >> +{ > >> + static const char *names[] = { > >> +#define NAME(n) [n] = #n > >> + NAME(STAGE_UNUSED), NAME(STAGE_BASE), > >> + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2), > >> + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6), > >> +#undef NAME > >> + }; > >> + return names[stage]; > >> +} > >> + > >> static inline const char *pipe2name(enum mdp5_pipe pipe) > >> { > >> static const char *names[] = { > >> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > >> index 432c098..df301df 100644 > >> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > >> @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, > >> > >> DBG("%s: update", mdp5_plane->name); > >> > >> + mdp5_print_plane_state(to_mdp5_plane_state(state)); > >> + > >> if (!plane_enabled(state)) { > >> to_mdp5_plane_state(state)->pending = true; > >> } else if (to_mdp5_plane_state(state)->mode_changed) { > >> @@ -904,7 +906,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, > >> type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; > >> ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, > >> mdp5_plane->formats, mdp5_plane->nformats, > >> - type, NULL); > >> + type, "%s", mdp5_plane->name); > >> if (ret) > >> goto fail; > >> > >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h > >> index 28d341a..0ee0aa4 100644 > >> --- a/include/drm/drmP.h > >> +++ b/include/drm/drmP.h > >> @@ -133,6 +133,7 @@ struct dma_buf_attachment; > >> #define DRM_UT_PRIME 0x08 > >> #define DRM_UT_ATOMIC 0x10 > >> #define DRM_UT_VBL 0x20 > >> +#define DRM_UT_STATE 0x40 > >> > >> extern __printf(2, 3) > >> void drm_ut_debug_printk(const char *function_name, > >> @@ -193,6 +194,8 @@ void drm_err(const char *format, ...); > >> #define DRM_INFO_ONCE(fmt, ...) \ > >> printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > >> > >> +extern unsigned int drm_debug; > >> + > >> /** > >> * Debug output. > >> * > >> @@ -230,6 +233,49 @@ void drm_err(const char *format, ...); > >> if (unlikely(drm_debug & DRM_UT_VBL)) \ > >> drm_ut_debug_printk(__func__, fmt, ##args); \ > >> } while (0) > >> +#define DRM_DEBUG_STATE(fmt, args...) \ > >> + do { \ > >> + if (unlikely(drm_debug & DRM_UT_STATE)) \ > >> + DRM_INFO(fmt, ##args); \ > >> + } while (0) > >> + > >> + > >> +static inline void > >> +drm_print_crtc_state(const struct drm_crtc_state *state) > >> +{ > >> + const struct drm_display_mode *mode = &state->mode; > >> + > >> + DRM_DEBUG_STATE("\tenable=%d\n", state->enable); > >> + DRM_DEBUG_STATE("\tactive=%d\n", state->active); > >> + DRM_DEBUG_STATE("\tplanes_changed=%d\n", state->planes_changed); > >> + DRM_DEBUG_STATE("\tmode_changed=%d\n", state->mode_changed); > >> + DRM_DEBUG_STATE("\tactive_changed=%d\n", state->active_changed); > >> + DRM_DEBUG_STATE("\tconnectors_changed=%d\n", state->connectors_changed); > >> + DRM_DEBUG_STATE("\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed); > >> + DRM_DEBUG_STATE("\tplane_mask=%x\n", state->plane_mask); > >> + DRM_DEBUG_STATE("\tconnector_mask=%x\n", state->connector_mask); > >> + DRM_DEBUG_STATE("\tencoder_mask=%x\n", state->encoder_mask); > >> + DRM_DEBUG_STATE("\tmode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n", > >> + mode->base.id, mode->name, mode->vrefresh, mode->clock, > >> + mode->hdisplay, mode->hsync_start, > >> + mode->hsync_end, mode->htotal, > >> + mode->vdisplay, mode->vsync_start, > >> + mode->vsync_end, mode->vtotal, mode->type, mode->flags); > >> +} > >> + > >> +static inline void > >> +drm_print_plane_state(const struct drm_plane_state *state) > >> +{ > >> + DRM_DEBUG_STATE("\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); > >> + DRM_DEBUG_STATE("\tfb=%d\n", state->fb ? state->fb->base.id : -1); > > > > -1 doesn't look like a legit thing. 0 would mean "not there" I think. > > hmm, yeah, I guess zero isn't a valid mode-object id.. > > > I wonder if we want to dump out more about the fb here too? size+fmt > > perhaps? > > yeah, sounds reasonable > > >> + DRM_DEBUG_STATE("\tcrtc-pos=[%d,%d, %ux%u]\n", > >> + state->crtc_x, state->crtc_y, > >> + state->crtc_w, state->crtc_h); > >> + DRM_DEBUG_STATE("\tsrc-pos=[%u,%u, %ux%u]\n", > >> + state->src_x >> 16, state->src_y >> 16, > >> + state->src_w >> 16, state->src_h >> 16); > > > > These should be printed with the fractional part included. Also your > > "[...]" thing doesn't match the format I've tried to use elsewhere for > > these things. Maybe yank the stuff from the drm_rect_print() thing > > into a few macros that can be used to print them easily anywhere. > > Something like > > > > #define FMT_FIXED_16_16 "...." > > #define something(x) ... > > printk("blah " FMT_FIXED_16_16 " bleh\n", something(state->src_x)); > > > > maybe? > > > > Maybe we should have something like that for printing out modes too? > > In case we want to mix up mode dumps with other output on the same > > line. > > yeah, I basically cut/paste from drm_mode_debug_printmodeline().. so > maybe some fmt and "splitter" macros makes sense.. > > >> + DRM_DEBUG_STATE("\trotation=%x\n", state->rotation); > > > > Should we dump out all the derived state too? The problem is of course > > that it might not be up to date yet, so when exactly you print it would > > affect whether you stale data or not. Maybe dump it only based on some > > bool function argument? > > not sure what you meant by derived state? Everything not directly coming from the user. Eg. src/dst rects, normalized_zpos, etc. > Although I guess we could > translate the rotation to strings.. > > btw, I'm slightly bummed that no one has invented a way to make > seq_file that goes to log.. re-using same dump_state() fxns for both > debugfs and printk would be nice. (And perhaps be enough of an > argument to add (*print_state)() fxn ptrs into > drm_{plane,crtc}_funcs..) > > BR, > -R > > >> +} > >> > >> /*@}*/ > >> > >> @@ -988,7 +1034,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc > >> /* drm_drv.c */ > >> void drm_put_dev(struct drm_device *dev); > >> void drm_unplug_dev(struct drm_device *dev); > >> -extern unsigned int drm_debug; > >> > >> /* Debugfs support */ > >> #if defined(CONFIG_DEBUG_FS) > >> -- > >> 2.7.4 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrjälä > > Intel OTC
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c index e42f62d..da84179 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c @@ -435,6 +435,9 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, DBG("%s: event: %p", mdp5_crtc->name, crtc->state->event); + DRM_DEBUG_STATE("crtc[%u]: %s\n", crtc->base.id, crtc->name); + drm_print_crtc_state(crtc->state); + WARN_ON(mdp5_crtc->event); spin_lock_irqsave(&dev->event_lock, flags); diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h index e4b3fb3..466acbc 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h @@ -92,6 +92,22 @@ struct mdp5_plane_state { #define to_mdp5_plane_state(x) \ container_of(x, struct mdp5_plane_state, base) +static inline const char *stage2name(enum mdp_mixer_stage_id stage); + +static inline void +mdp5_print_plane_state(const struct mdp5_plane_state *state) +{ + const struct drm_plane *plane = state->base.plane; + DRM_DEBUG_STATE("plane[%u]: %s\n", plane->base.id, plane->name); + drm_print_plane_state(&state->base); + DRM_DEBUG_STATE("\tpremultiplied=%u\n", state->premultiplied); + DRM_DEBUG_STATE("\tzpos=%u\n", state->zpos); + DRM_DEBUG_STATE("\talpha=%u\n", state->alpha); + DRM_DEBUG_STATE("\tstage=%s\n", stage2name(state->stage)); + DRM_DEBUG_STATE("\tmode_changed=%u\n", state->mode_changed); + DRM_DEBUG_STATE("\tpending=%u\n", state->pending); +} + enum mdp5_intf_mode { MDP5_INTF_MODE_NONE = 0, @@ -120,6 +136,18 @@ static inline u32 mdp5_read(struct mdp5_kms *mdp5_kms, u32 reg) return msm_readl(mdp5_kms->mmio + reg); } +static inline const char *stage2name(enum mdp_mixer_stage_id stage) +{ + static const char *names[] = { +#define NAME(n) [n] = #n + NAME(STAGE_UNUSED), NAME(STAGE_BASE), + NAME(STAGE0), NAME(STAGE1), NAME(STAGE2), + NAME(STAGE3), NAME(STAGE4), NAME(STAGE6), +#undef NAME + }; + return names[stage]; +} + static inline const char *pipe2name(enum mdp5_pipe pipe) { static const char *names[] = { diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index 432c098..df301df 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -356,6 +356,8 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, DBG("%s: update", mdp5_plane->name); + mdp5_print_plane_state(to_mdp5_plane_state(state)); + if (!plane_enabled(state)) { to_mdp5_plane_state(state)->pending = true; } else if (to_mdp5_plane_state(state)->mode_changed) { @@ -904,7 +906,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, mdp5_plane->formats, mdp5_plane->nformats, - type, NULL); + type, "%s", mdp5_plane->name); if (ret) goto fail; diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 28d341a..0ee0aa4 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -133,6 +133,7 @@ struct dma_buf_attachment; #define DRM_UT_PRIME 0x08 #define DRM_UT_ATOMIC 0x10 #define DRM_UT_VBL 0x20 +#define DRM_UT_STATE 0x40 extern __printf(2, 3) void drm_ut_debug_printk(const char *function_name, @@ -193,6 +194,8 @@ void drm_err(const char *format, ...); #define DRM_INFO_ONCE(fmt, ...) \ printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) +extern unsigned int drm_debug; + /** * Debug output. * @@ -230,6 +233,49 @@ void drm_err(const char *format, ...); if (unlikely(drm_debug & DRM_UT_VBL)) \ drm_ut_debug_printk(__func__, fmt, ##args); \ } while (0) +#define DRM_DEBUG_STATE(fmt, args...) \ + do { \ + if (unlikely(drm_debug & DRM_UT_STATE)) \ + DRM_INFO(fmt, ##args); \ + } while (0) + + +static inline void +drm_print_crtc_state(const struct drm_crtc_state *state) +{ + const struct drm_display_mode *mode = &state->mode; + + DRM_DEBUG_STATE("\tenable=%d\n", state->enable); + DRM_DEBUG_STATE("\tactive=%d\n", state->active); + DRM_DEBUG_STATE("\tplanes_changed=%d\n", state->planes_changed); + DRM_DEBUG_STATE("\tmode_changed=%d\n", state->mode_changed); + DRM_DEBUG_STATE("\tactive_changed=%d\n", state->active_changed); + DRM_DEBUG_STATE("\tconnectors_changed=%d\n", state->connectors_changed); + DRM_DEBUG_STATE("\tcolor_mgmt_changed=%d\n", state->color_mgmt_changed); + DRM_DEBUG_STATE("\tplane_mask=%x\n", state->plane_mask); + DRM_DEBUG_STATE("\tconnector_mask=%x\n", state->connector_mask); + DRM_DEBUG_STATE("\tencoder_mask=%x\n", state->encoder_mask); + DRM_DEBUG_STATE("\tmode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x\n", + mode->base.id, mode->name, mode->vrefresh, mode->clock, + mode->hdisplay, mode->hsync_start, + mode->hsync_end, mode->htotal, + mode->vdisplay, mode->vsync_start, + mode->vsync_end, mode->vtotal, mode->type, mode->flags); +} + +static inline void +drm_print_plane_state(const struct drm_plane_state *state) +{ + DRM_DEBUG_STATE("\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); + DRM_DEBUG_STATE("\tfb=%d\n", state->fb ? state->fb->base.id : -1); + DRM_DEBUG_STATE("\tcrtc-pos=[%d,%d, %ux%u]\n", + state->crtc_x, state->crtc_y, + state->crtc_w, state->crtc_h); + DRM_DEBUG_STATE("\tsrc-pos=[%u,%u, %ux%u]\n", + state->src_x >> 16, state->src_y >> 16, + state->src_w >> 16, state->src_h >> 16); + DRM_DEBUG_STATE("\trotation=%x\n", state->rotation); +} /*@}*/ @@ -988,7 +1034,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc /* drm_drv.c */ void drm_put_dev(struct drm_device *dev); void drm_unplug_dev(struct drm_device *dev); -extern unsigned int drm_debug; /* Debugfs support */ #if defined(CONFIG_DEBUG_FS)