On Fri, 20 Jul 2018 20:03:35 +0100 Daniel Stone <[email protected]> wrote:
> Add a 'drm-debug' scope which prints verbose information about the DRM > backend's repaint cycle, including the decision tree on how views are > assigned (or not) to planes. > > Signed-off-by: Daniel Stone <[email protected]> > --- > libweston/compositor-drm.c | 233 ++++++++++++++++++++++++++++++++----- > 1 file changed, 206 insertions(+), 27 deletions(-) > Hi, lots of output from this one, nice! > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c > index 653d13e0c..2cadf036c 100644 > --- a/libweston/compositor-drm.c > +++ b/libweston/compositor-drm.c > @@ -51,6 +51,7 @@ > > #include "compositor.h" > #include "compositor-drm.h" > +#include "weston-debug.h" > #include "shared/helpers.h" > #include "shared/timespec-util.h" > #include "gl-renderer.h" > @@ -73,6 +74,9 @@ > #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64 > #endif > > +#define drm_debug(b, ...) \ > + weston_debug_scope_printf((b)->debug, __VA_ARGS__) > + > #define MAX_CLONED_CONNECTORS 4 > > /** > @@ -302,6 +306,8 @@ struct drm_backend { > bool shutting_down; > > bool aspect_ratio_supported; > + > + struct weston_debug_scope *debug; > }; > > struct drm_mode { > @@ -2358,6 +2364,9 @@ crtc_add_prop(drmModeAtomicReq *req, struct drm_output > *output, > > ret = drmModeAtomicAddProperty(req, output->crtc_id, info->prop_id, > val); > + drm_debug(output->backend, "\t\t\t[CRTC:%d] %d (%s) -> %lld (0x%llx)\n", > + output->crtc_id, info->prop_id, info->name, > + (unsigned long long) val, (unsigned long long) val); This is using %lld to print (unsigned long long), should it not be %llu instead? All the property ids are unsigned as well, so %d -> %u? > return (ret <= 0) ? -1 : 0; > } > > @@ -2373,6 +2382,9 @@ connector_add_prop(drmModeAtomicReq *req, struct > drm_head *head, > > ret = drmModeAtomicAddProperty(req, head->connector_id, > info->prop_id, val); > + drm_debug(head->backend, "\t\t\t[CONN:%d] %d (%s) -> %lld (0x%llx)\n", > + head->connector_id, info->prop_id, info->name, > + (unsigned long long) val, (unsigned long long) val); %d -> %u %lld -> %llu > return (ret <= 0) ? -1 : 0; > } > > @@ -2388,6 +2400,9 @@ plane_add_prop(drmModeAtomicReq *req, struct drm_plane > *plane, > > ret = drmModeAtomicAddProperty(req, plane->plane_id, info->prop_id, > val); > + drm_debug(plane->backend, "\t\t\t[PLANE:%d] %d (%s) -> %lld (0x%llx)\n", > + plane->plane_id, info->prop_id, info->name, > + (unsigned long long) val, (unsigned long long) val); %d -> %u %lld -> %llu > return (ret <= 0) ? -1 : 0; > } > > @@ -2406,6 +2421,9 @@ drm_mode_ensure_blob(struct drm_backend *backend, > struct drm_mode *mode) > if (ret != 0) > weston_log("failed to create mode property blob: %m\n"); > > + drm_debug(backend, "\t\t\t[atomic] created new mode blob %ld for %s", > + (unsigned long) mode->blob_id, mode->mode_info.name); %ld -> %u without the cast? There are more of these kinds below, should I point them out? > + > return ret; > } ... > @@ -2969,10 +3010,15 @@ drm_repaint_begin(struct weston_compositor > *compositor) > { > struct drm_backend *b = to_drm_backend(compositor); > struct drm_pending_state *ret; > + char *scene_graph = weston_compositor_print_scene_graph(compositor); Maybe you'd want to make the call to weston_compositor_print_scene_graph() conditional on the debug scope being enabled? To not incur the work while not being debugged. > > ret = drm_pending_state_alloc(b); > b->repaint_data = ret; > > + drm_debug(b, "[repaint] Beignning repaint; pending_state %p\n", ret); Typoed "Beginning". > + drm_debug(b, "%s", scene_graph); > + free(scene_graph); > + > return ret; > } > ... > @@ -3531,12 +3691,20 @@ drm_assign_planes(struct weston_output *output_base, > void *repaint_data) > struct weston_view *ev; > struct weston_plane *primary = &output_base->compositor->primary_plane; > > + drm_debug(b, "\t[repaint] preparing state for output %s (%d)\n", > + output_base->name, output_base->id); > + > if (!b->sprites_are_broken) { > state = drm_output_propose_state(output_base, pending_state, > > DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY); > - if (!state) > + if (!state) { > + drm_debug(b, "\t[repaint] could not build planes-only " > + "state, trying mixed\n"); > state = drm_output_propose_state(output_base, > pending_state, > > DRM_OUTPUT_PROPOSE_STATE_MIXED); Should there be a note if mixed mode failed? > + } > + } else { > + drm_debug(b, "\t[state] no overlay plane support\n"); > } > > if (!state) Looks good aside from the minor details. Thanks, pq
pgpKunUvJVsvm.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
