Re: [PATCH 4/6] drm/atomic: add new drm_debug bit to dump atomic state before commit

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Oct 14, 2016 at 07:55:51PM -0400, Rob Clark wrote:
> Signed-off-by: Rob Clark <robdclark@gmail.com>

General comment: Some drivers (and I think this would be useful in
general) also dump state into debugfs with seq_file. And we have a bunch
of other places where unfortunately we duplicate dumping functions between
dmesg and seq_file (e.g. drm_mm.c).

Fancy idea: Could we create a seq_file that dumps to dmesg and so reuse
one for all? Before we have to add atomic_print_state and
atomic_dump_state for dmesg vs. seq_file variations ...

One kerneldoc nit below.

> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 7ffaa35..642d5da 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,
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 856fdf8..d74d47a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -44,6 +44,7 @@ struct drm_framebuffer;
>  struct drm_object_properties;
>  struct drm_file;
>  struct drm_clip_rect;
> +struct drm_print;
>  struct device_node;
>  struct fence;
>  struct edid;
> @@ -707,6 +708,18 @@ struct drm_crtc_funcs {
>  				   const struct drm_crtc_state *state,
>  				   struct drm_property *property,
>  				   uint64_t *val);
> +
> +	/**
> +	 * @atomic_print_state:
> +	 *
> +	 * If driver subclasses struct &drm_crtc_state, it should implement
> +	 * this optional hook for printing state.

s/printing state/printing additional, driver-private state/ in all of
these for clarification. Same below.
-Daniel

> +	 *
> +	 * Do not call this directly, use drm_atomic_crtc_print_state()
> +	 * instead.
> +	 */
> +	void (*atomic_print_state)(struct drm_print *p,
> +				   const struct drm_crtc_state *state);
>  };
>  
>  /**
> @@ -1132,6 +1145,18 @@ struct drm_connector_funcs {
>  				   const struct drm_connector_state *state,
>  				   struct drm_property *property,
>  				   uint64_t *val);
> +
> +	/**
> +	 * @atomic_print_state:
> +	 *
> +	 * If driver subclasses struct &drm_connector_state, it should
> +	 * implement this optional hook for printing state.
> +	 *
> +	 * Do not call this directly, use drm_atomic_connector_print_state()
> +	 * instead.
> +	 */
> +	void (*atomic_print_state)(struct drm_print *p,
> +				   const struct drm_connector_state *state);
>  };
>  
>  /**
> @@ -1623,6 +1648,18 @@ struct drm_plane_funcs {
>  				   const struct drm_plane_state *state,
>  				   struct drm_property *property,
>  				   uint64_t *val);
> +
> +	/**
> +	 * @atomic_print_state:
> +	 *
> +	 * If driver subclasses struct &drm_plane_state, it should implement
> +	 * this optional hook for printing state.
> +	 *
> +	 * Do not call this directly, use drm_atomic_plane_print_state()
> +	 * instead.
> +	 */
> +	void (*atomic_print_state)(struct drm_print *p,
> +				   const struct drm_plane_state *state);
>  };
>  
>  enum drm_plane_type {
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel