The DRM_UT_STATE controls whether we're calling drm_atomic_print_new_state() whenever a new state is committed. However, that call is made in the drm_mode_atomic_ioctl(), whereas we have multiple users of the drm_atomic_commit() function in the kernel (framebuffer emulation, drm_atomic_helper_dirtyfb, etc.).

This leads to multiple states being committed but never actually displayed even though we asked to have verbose atomic state debugging.

Let's move the call to drm_atomic_print_new_state() to drm_atomic_commit() to make sure we don't miss any. Non-blocking commits were never logged though, and it would create too much churn in the logs to do so, so leave them out for now.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/drm_atomic.c | 4 ++++ drivers/gpu/drm/drm_atomic_uapi.c | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 88cd992df356..637df126c2d9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1423,8 +1423,12 @@ EXPORT_SYMBOL(drm_atomic_check_only); int drm_atomic_commit(struct drm_atomic_state *state) { struct drm_mode_config *config = &state->dev->mode_config; + struct drm_printer p = drm_info_printer(state->dev->dev); int ret;

+ if (drm_debug_enabled(DRM_UT_STATE)) + drm_atomic_print_new_state(state, &p); + ret = drm_atomic_check_only(state); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 9781722519c3..45e6d3c62a9a 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -1326,7 +1326,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_out_fence_state *fence_state; int ret = 0; unsigned int i, j, num_fences; - struct drm_printer p = drm_info_printer(dev->dev);

/* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1458,9 +1457,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { ret = drm_atomic_nonblocking_commit(state); } else { - if (drm_debug_enabled(DRM_UT_STATE)) - drm_atomic_print_new_state(state, &p); - ret = drm_atomic_commit(state); }

A number of drivers (amdgpu, komeda, vc4, etc.) leverage the drm_private_state structure, but we don't have any infrastructure to provide debugging like we do for the other components state. Let's add an atomic_print_state hook to be consistent.

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/drm_atomic.c | 16 ++++++++++++++++ include/drm/drm_atomic.h | 27 +++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 637df126c2d9..58c0283fb6b0 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -789,6 +789,8 @@ drm_atomic_private_obj_init(struct drm_device *dev, obj->state = state; obj->funcs = funcs; list_add_tail(&obj->head, &dev->mode_config.privobj_list); + + state->obj = obj; } EXPORT_SYMBOL(drm_atomic_private_obj_init);

@@ -1636,6 +1638,15 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, } EXPORT_SYMBOL(__drm_atomic_helper_set_config);

+static void drm_atomic_private_obj_print_state(struct drm_printer *p, + const struct drm_private_state *state) +{ + struct drm_private_obj *obj = state->obj; + + if (obj->funcs->atomic_print_state) + obj->funcs->atomic_print_state(p, state); +} + /** * drm_atomic_print_new_state - prints drm atomic state * @state: atomic configuration to check @@ -1656,6 +1667,8 @@ void drm_atomic_print_new_state(const struct drm_atomic_state *state, struct drm_crtc_state *crtc_state; struct drm_connector *connector; struct drm_connector_state *connector_state; + struct drm_private_obj *obj; + struct drm_private_state *obj_state; int i;

if (!p) { @@ -1673,6 +1686,9 @@ void drm_atomic_print_new_state(const struct drm_atomic_state *state,

for_each_new_connector_in_state(state, connector, connector_state, i) drm_atomic_connector_print_state(p, connector_state); + + for_each_new_private_obj_in_state(state, obj, obj_state, i) + drm_atomic_private_obj_print_state(p, obj_state); } EXPORT_SYMBOL(drm_atomic_print_new_state);

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 1701c2128a5c..0777725085df 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -227,6 +227,18 @@ struct drm_private_state_funcs { */ void (*atomic_destroy_state)(struct drm_private_obj *obj, struct drm_private_state *state); + + /** + * @atomic_print_state: + * + * If driver subclasses &struct drm_private_state, it should implement + * this optional hook for printing additional driver specific state. + * + * Do not call this directly, use drm_atomic_private_obj_print_state() + * instead. + */ + void (*atomic_print_state)(struct drm_printer *p, + const struct drm_private_state *state); };

/** @@ -311,14 +323,21 @@ struct drm_private_obj {

/** * struct drm_private_state - base struct for driver private object state - * @state: backpointer to global drm_atomic_state * - * Currently only contains a backpointer to the overall atomic update, but in - * the future also might hold synchronization information similar to e.g. - * &drm_crtc.commit. + * Currently only contains a backpointer to the overall atomic update, + * and the relevant private object but in the future also might hold + * synchronization information similar to e.g. &drm_crtc.commit. */ struct drm_private_state { + /** + * @state: backpointer to global drm_atomic_state + */ struct drm_atomic_state *state; + + /** + * @obj: backpointer to the private object + */ + struct drm_private_obj *obj; };

struct __drm_private_objs_state {

The HVS state configuration is useful when debugging what's going on in the vc4 hardware pipeline. Add an implementation of .atomic_print_state.

Acked-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_kms.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 26b771c919b1..bffd81d4bfcf 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -701,9 +701,26 @@ static void vc4_hvs_channels_destroy_state(struct drm_private_obj *obj, kfree(hvs_state); }

+static void vc4_hvs_channels_print_state(struct drm_printer *p, + const struct drm_private_state *state) +{ + struct vc4_hvs_state *hvs_state = to_vc4_hvs_state(state); + unsigned int i; + + drm_printf(p, "HVS State\n"); + drm_printf(p, "\tCore Clock Rate: %lu\n", hvs_state->core_clock_rate); + + for (i = 0; i < HVS_NUM_CHANNELS; i++) { + drm_printf(p, "\tChannel %d\n", i); + drm_printf(p, "\t\tin use=%d\n", hvs_state->fifo_state[i].in_use); + drm_printf(p, "\t\tload=%lu\n", hvs_state->fifo_state[i].fifo_load); + } +} + static const struct drm_private_state_funcs vc4_hvs_state_funcs = { .atomic_duplicate_state = vc4_hvs_channels_duplicate_state, .atomic_destroy_state = vc4_hvs_channels_destroy_state, + .atomic_print_state = vc4_hvs_channels_print_state, };

static void vc4_hvs_channels_obj_fini(struct drm_device *dev, void *unused)