| Message ID | 20231122043009.2741-12-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: renesas: vsp1: Conversion to subdev active state | expand |
Hi Laurent On Wed, Nov 22, 2023 at 06:30:01AM GMT, Laurent Pinchart wrote: > It is useful for debugging purpose to dump a vsp1_pipeline to the kernel > log. Add a new function to do so, and use it when initializing the video > and DRM pipelines. > > As __vsp1_pipeline_dump() needs to construct the log message > iteratively, it uses pr_cont(...) (exact equivalent to the more verbose > "printk(KERN_CONT ..."). The function thus can't use dev_dbg() to log > the initial part of the message, for two reasons: > > - pr_cont() doesn't seem to work with dev_*(). Even if the format string > passed to dev_*() doesn't end with a '\n', pr_cont() starts a new line > in the log. This behaviour doesn't seem to be clearly documented, and > may or may not be on purpose. > > - Messages printed by dev_dbg() may be omitted if dynamic debugging is > enabled. In that case, the continuation messages will still be > printed, leading to confusing log messages. > > To still benefit from the dynamic debug infrastructure, we declare a > vsp1_pipeline_dump() macro that uses _dynamic_func_call() when dynamic > debugging is enabled. The whole vsp1_pipeline_dump() call can be > selected at runtime. The __vsp1_pipeline_dump() function then uses a > plain "printk(KERN_DEBUG ...)" to print the message header using the > debug log level, and pr_cont() to print the rest of the message on the > same line. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > .../media/platform/renesas/vsp1/vsp1_drm.c | 5 +++++ > .../media/platform/renesas/vsp1/vsp1_pipe.c | 22 +++++++++++++++++++ > .../media/platform/renesas/vsp1/vsp1_pipe.h | 19 ++++++++++++++++ > .../media/platform/renesas/vsp1/vsp1_video.c | 10 ++++++++- > 4 files changed, 55 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > index 3954c138fa7b..1aa59a74672f 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > @@ -733,6 +733,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, > if (ret < 0) > goto unlock; > > + vsp1_pipeline_dump(pipe, "LIF setup"); > + > /* Enable the VSP1. */ > ret = vsp1_device_get(vsp1); > if (ret < 0) > @@ -906,6 +908,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index, > } > > vsp1_du_pipeline_setup_inputs(vsp1, pipe); > + > + vsp1_pipeline_dump(pipe, "atomic update"); > + > vsp1_du_pipeline_configure(pipe); > > done: > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > index 8eba3cda1e3d..edc5e9f3ba65 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > @@ -301,6 +301,28 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe) > pipe->state = VSP1_PIPELINE_STOPPED; > } > > +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, > + const char *msg) > +{ > + struct vsp1_device *vsp1 = pipe->output->entity.vsp1; > + struct vsp1_entity *entity; > + bool first = true; > + > + printk(KERN_DEBUG "%s: %s: pipe: ", dev_name(vsp1->dev), msg); > + > + list_for_each_entry(entity, &pipe->entities, list_pipe) { > + const char *name; > + > + name = strchrnul(entity->subdev.name, ' '); > + name = name ? name + 1 : entity->subdev.name; > + > + pr_cont("%s%s", first ? "" : ", ", name); > + first = false; > + } > + > + pr_cont("\n"); > +} > + > /* Must be called with the pipe irqlock held. */ > void vsp1_pipeline_run(struct vsp1_pipeline *pipe) > { > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > index c1f411227de7..46a82a9f766a 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > @@ -9,6 +9,7 @@ > #ifndef __VSP1_PIPE_H__ > #define __VSP1_PIPE_H__ > > +#include <linux/dynamic_debug.h> > #include <linux/kref.h> > #include <linux/list.h> > #include <linux/spinlock.h> > @@ -142,6 +143,24 @@ struct vsp1_pipeline { > void vsp1_pipeline_reset(struct vsp1_pipeline *pipe); > void vsp1_pipeline_init(struct vsp1_pipeline *pipe); > > +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, > + const char *msg); > + > +#if defined(CONFIG_DYNAMIC_DEBUG) || \ > + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE)) > +#define vsp1_pipeline_dump(pipe, msg) \ > + _dynamic_func_call("vsp1_pipeline_dump()", __vsp1_pipeline_dump, pipe, msg) > +#elif defined(DEBUG) > +#define vsp1_pipeline_dump(pipe, msg) \ > + __vsp1_pipeline_dump(NULL, pipe, msg) > +#else > +#define vsp1_pipeline_dump(pipe, msg) \ > +({ \ > + if (0) \ > + __vsp1_pipeline_dump(NULL, pipe, msg); \ > +)} Why can't this simply be #else #define vsp1_pipeline_dump(pipe, msg) #endif ? > +#endif > + > void vsp1_pipeline_run(struct vsp1_pipeline *pipe); > bool vsp1_pipeline_stopped(struct vsp1_pipeline *pipe); > int vsp1_pipeline_stop(struct vsp1_pipeline *pipe); > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c > index 6a8db541543a..84394994ccee 100644 > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c > @@ -520,11 +520,19 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, > static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe, > struct vsp1_video *video) > { > + int ret; > + > vsp1_pipeline_init(pipe); > > pipe->frame_end = vsp1_video_pipeline_frame_end; > > - return vsp1_video_pipeline_build(pipe, video); > + ret = vsp1_video_pipeline_build(pipe, video); > + if (ret) > + return ret; > + > + vsp1_pipeline_dump(pipe, "video"); > + > + return 0; > } > > static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video) > -- > Regards, > > Laurent Pinchart > >
On Tue, Jun 18, 2024 at 01:34:41PM +0200, Jacopo Mondi wrote: > Hi Laurent > > On Wed, Nov 22, 2023 at 06:30:01AM GMT, Laurent Pinchart wrote: > > It is useful for debugging purpose to dump a vsp1_pipeline to the kernel > > log. Add a new function to do so, and use it when initializing the video > > and DRM pipelines. > > > > As __vsp1_pipeline_dump() needs to construct the log message > > iteratively, it uses pr_cont(...) (exact equivalent to the more verbose > > "printk(KERN_CONT ..."). The function thus can't use dev_dbg() to log > > the initial part of the message, for two reasons: > > > > - pr_cont() doesn't seem to work with dev_*(). Even if the format string > > passed to dev_*() doesn't end with a '\n', pr_cont() starts a new line > > in the log. This behaviour doesn't seem to be clearly documented, and > > may or may not be on purpose. > > > > - Messages printed by dev_dbg() may be omitted if dynamic debugging is > > enabled. In that case, the continuation messages will still be > > printed, leading to confusing log messages. > > > > To still benefit from the dynamic debug infrastructure, we declare a > > vsp1_pipeline_dump() macro that uses _dynamic_func_call() when dynamic > > debugging is enabled. The whole vsp1_pipeline_dump() call can be > > selected at runtime. The __vsp1_pipeline_dump() function then uses a > > plain "printk(KERN_DEBUG ...)" to print the message header using the > > debug log level, and pr_cont() to print the rest of the message on the > > same line. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- > > .../media/platform/renesas/vsp1/vsp1_drm.c | 5 +++++ > > .../media/platform/renesas/vsp1/vsp1_pipe.c | 22 +++++++++++++++++++ > > .../media/platform/renesas/vsp1/vsp1_pipe.h | 19 ++++++++++++++++ > > .../media/platform/renesas/vsp1/vsp1_video.c | 10 ++++++++- > > 4 files changed, 55 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > index 3954c138fa7b..1aa59a74672f 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > @@ -733,6 +733,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, > > if (ret < 0) > > goto unlock; > > > > + vsp1_pipeline_dump(pipe, "LIF setup"); > > + > > /* Enable the VSP1. */ > > ret = vsp1_device_get(vsp1); > > if (ret < 0) > > @@ -906,6 +908,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index, > > } > > > > vsp1_du_pipeline_setup_inputs(vsp1, pipe); > > + > > + vsp1_pipeline_dump(pipe, "atomic update"); > > + > > vsp1_du_pipeline_configure(pipe); > > > > done: > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > > index 8eba3cda1e3d..edc5e9f3ba65 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > > @@ -301,6 +301,28 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe) > > pipe->state = VSP1_PIPELINE_STOPPED; > > } > > > > +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, > > + const char *msg) > > +{ > > + struct vsp1_device *vsp1 = pipe->output->entity.vsp1; > > + struct vsp1_entity *entity; > > + bool first = true; > > + > > + printk(KERN_DEBUG "%s: %s: pipe: ", dev_name(vsp1->dev), msg); > > + > > + list_for_each_entry(entity, &pipe->entities, list_pipe) { > > + const char *name; > > + > > + name = strchrnul(entity->subdev.name, ' '); > > + name = name ? name + 1 : entity->subdev.name; > > + > > + pr_cont("%s%s", first ? "" : ", ", name); > > + first = false; > > + } > > + > > + pr_cont("\n"); > > +} > > + > > /* Must be called with the pipe irqlock held. */ > > void vsp1_pipeline_run(struct vsp1_pipeline *pipe) > > { > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > index c1f411227de7..46a82a9f766a 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > @@ -9,6 +9,7 @@ > > #ifndef __VSP1_PIPE_H__ > > #define __VSP1_PIPE_H__ > > > > +#include <linux/dynamic_debug.h> > > #include <linux/kref.h> > > #include <linux/list.h> > > #include <linux/spinlock.h> > > @@ -142,6 +143,24 @@ struct vsp1_pipeline { > > void vsp1_pipeline_reset(struct vsp1_pipeline *pipe); > > void vsp1_pipeline_init(struct vsp1_pipeline *pipe); > > > > +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, > > + const char *msg); > > + > > +#if defined(CONFIG_DYNAMIC_DEBUG) || \ > > + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE)) > > +#define vsp1_pipeline_dump(pipe, msg) \ > > + _dynamic_func_call("vsp1_pipeline_dump()", __vsp1_pipeline_dump, pipe, msg) > > +#elif defined(DEBUG) > > +#define vsp1_pipeline_dump(pipe, msg) \ > > + __vsp1_pipeline_dump(NULL, pipe, msg) > > +#else > > +#define vsp1_pipeline_dump(pipe, msg) \ > > +({ \ > > + if (0) \ > > + __vsp1_pipeline_dump(NULL, pipe, msg); \ > > +)} > > Why can't this simply be > > #else > #define vsp1_pipeline_dump(pipe, msg) > #endif > > ? To avoid unused local variable warnings. > > +#endif > > + > > void vsp1_pipeline_run(struct vsp1_pipeline *pipe); > > bool vsp1_pipeline_stopped(struct vsp1_pipeline *pipe); > > int vsp1_pipeline_stop(struct vsp1_pipeline *pipe); > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > index 6a8db541543a..84394994ccee 100644 > > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > @@ -520,11 +520,19 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, > > static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe, > > struct vsp1_video *video) > > { > > + int ret; > > + > > vsp1_pipeline_init(pipe); > > > > pipe->frame_end = vsp1_video_pipeline_frame_end; > > > > - return vsp1_video_pipeline_build(pipe, video); > > + ret = vsp1_video_pipeline_build(pipe, video); > > + if (ret) > > + return ret; > > + > > + vsp1_pipeline_dump(pipe, "video"); > > + > > + return 0; > > } > > > > static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video)
Hi Laurent On Tue, Jun 18, 2024 at 07:25:02PM GMT, Laurent Pinchart wrote: > On Tue, Jun 18, 2024 at 01:34:41PM +0200, Jacopo Mondi wrote: > > Hi Laurent > > > > On Wed, Nov 22, 2023 at 06:30:01AM GMT, Laurent Pinchart wrote: > > > It is useful for debugging purpose to dump a vsp1_pipeline to the kernel > > > log. Add a new function to do so, and use it when initializing the video > > > and DRM pipelines. > > > > > > As __vsp1_pipeline_dump() needs to construct the log message > > > iteratively, it uses pr_cont(...) (exact equivalent to the more verbose > > > "printk(KERN_CONT ..."). The function thus can't use dev_dbg() to log > > > the initial part of the message, for two reasons: > > > > > > - pr_cont() doesn't seem to work with dev_*(). Even if the format string > > > passed to dev_*() doesn't end with a '\n', pr_cont() starts a new line > > > in the log. This behaviour doesn't seem to be clearly documented, and > > > may or may not be on purpose. > > > > > > - Messages printed by dev_dbg() may be omitted if dynamic debugging is > > > enabled. In that case, the continuation messages will still be > > > printed, leading to confusing log messages. > > > > > > To still benefit from the dynamic debug infrastructure, we declare a > > > vsp1_pipeline_dump() macro that uses _dynamic_func_call() when dynamic > > > debugging is enabled. The whole vsp1_pipeline_dump() call can be > > > selected at runtime. The __vsp1_pipeline_dump() function then uses a > > > plain "printk(KERN_DEBUG ...)" to print the message header using the > > > debug log level, and pr_cont() to print the rest of the message on the > > > same line. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > --- > > > .../media/platform/renesas/vsp1/vsp1_drm.c | 5 +++++ > > > .../media/platform/renesas/vsp1/vsp1_pipe.c | 22 +++++++++++++++++++ > > > .../media/platform/renesas/vsp1/vsp1_pipe.h | 19 ++++++++++++++++ > > > .../media/platform/renesas/vsp1/vsp1_video.c | 10 ++++++++- > > > 4 files changed, 55 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > index 3954c138fa7b..1aa59a74672f 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c > > > @@ -733,6 +733,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, > > > if (ret < 0) > > > goto unlock; > > > > > > + vsp1_pipeline_dump(pipe, "LIF setup"); > > > + > > > /* Enable the VSP1. */ > > > ret = vsp1_device_get(vsp1); > > > if (ret < 0) > > > @@ -906,6 +908,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index, > > > } > > > > > > vsp1_du_pipeline_setup_inputs(vsp1, pipe); > > > + > > > + vsp1_pipeline_dump(pipe, "atomic update"); > > > + > > > vsp1_du_pipeline_configure(pipe); > > > > > > done: > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > > > index 8eba3cda1e3d..edc5e9f3ba65 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c > > > @@ -301,6 +301,28 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe) > > > pipe->state = VSP1_PIPELINE_STOPPED; > > > } > > > > > > +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, > > > + const char *msg) > > > +{ > > > + struct vsp1_device *vsp1 = pipe->output->entity.vsp1; > > > + struct vsp1_entity *entity; > > > + bool first = true; > > > + > > > + printk(KERN_DEBUG "%s: %s: pipe: ", dev_name(vsp1->dev), msg); > > > + > > > + list_for_each_entry(entity, &pipe->entities, list_pipe) { > > > + const char *name; > > > + > > > + name = strchrnul(entity->subdev.name, ' '); > > > + name = name ? name + 1 : entity->subdev.name; > > > + > > > + pr_cont("%s%s", first ? "" : ", ", name); > > > + first = false; > > > + } > > > + > > > + pr_cont("\n"); > > > +} > > > + > > > /* Must be called with the pipe irqlock held. */ > > > void vsp1_pipeline_run(struct vsp1_pipeline *pipe) > > > { > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > > index c1f411227de7..46a82a9f766a 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h > > > @@ -9,6 +9,7 @@ > > > #ifndef __VSP1_PIPE_H__ > > > #define __VSP1_PIPE_H__ > > > > > > +#include <linux/dynamic_debug.h> > > > #include <linux/kref.h> > > > #include <linux/list.h> > > > #include <linux/spinlock.h> > > > @@ -142,6 +143,24 @@ struct vsp1_pipeline { > > > void vsp1_pipeline_reset(struct vsp1_pipeline *pipe); > > > void vsp1_pipeline_init(struct vsp1_pipeline *pipe); > > > > > > +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, > > > + const char *msg); > > > + > > > +#if defined(CONFIG_DYNAMIC_DEBUG) || \ > > > + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE)) > > > +#define vsp1_pipeline_dump(pipe, msg) \ > > > + _dynamic_func_call("vsp1_pipeline_dump()", __vsp1_pipeline_dump, pipe, msg) > > > +#elif defined(DEBUG) > > > +#define vsp1_pipeline_dump(pipe, msg) \ > > > + __vsp1_pipeline_dump(NULL, pipe, msg) > > > +#else > > > +#define vsp1_pipeline_dump(pipe, msg) \ > > > +({ \ > > > + if (0) \ > > > + __vsp1_pipeline_dump(NULL, pipe, msg); \ > > > +)} > > > > Why can't this simply be > > > > #else > > #define vsp1_pipeline_dump(pipe, msg) > > #endif > > > > ? > > To avoid unused local variable warnings. > Fine indeed! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > +#endif > > > + > > > void vsp1_pipeline_run(struct vsp1_pipeline *pipe); > > > bool vsp1_pipeline_stopped(struct vsp1_pipeline *pipe); > > > int vsp1_pipeline_stop(struct vsp1_pipeline *pipe); > > > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > > index 6a8db541543a..84394994ccee 100644 > > > --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c > > > +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c > > > @@ -520,11 +520,19 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, > > > static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe, > > > struct vsp1_video *video) > > > { > > > + int ret; > > > + > > > vsp1_pipeline_init(pipe); > > > > > > pipe->frame_end = vsp1_video_pipeline_frame_end; > > > > > > - return vsp1_video_pipeline_build(pipe, video); > > > + ret = vsp1_video_pipeline_build(pipe, video); > > > + if (ret) > > > + return ret; > > > + > > > + vsp1_pipeline_dump(pipe, "video"); > > > + > > > + return 0; > > > } > > > > > > static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video) > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c index 3954c138fa7b..1aa59a74672f 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c @@ -733,6 +733,8 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, if (ret < 0) goto unlock; + vsp1_pipeline_dump(pipe, "LIF setup"); + /* Enable the VSP1. */ ret = vsp1_device_get(vsp1); if (ret < 0) @@ -906,6 +908,9 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index, } vsp1_du_pipeline_setup_inputs(vsp1, pipe); + + vsp1_pipeline_dump(pipe, "atomic update"); + vsp1_du_pipeline_configure(pipe); done: diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c index 8eba3cda1e3d..edc5e9f3ba65 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c @@ -301,6 +301,28 @@ void vsp1_pipeline_init(struct vsp1_pipeline *pipe) pipe->state = VSP1_PIPELINE_STOPPED; } +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, + const char *msg) +{ + struct vsp1_device *vsp1 = pipe->output->entity.vsp1; + struct vsp1_entity *entity; + bool first = true; + + printk(KERN_DEBUG "%s: %s: pipe: ", dev_name(vsp1->dev), msg); + + list_for_each_entry(entity, &pipe->entities, list_pipe) { + const char *name; + + name = strchrnul(entity->subdev.name, ' '); + name = name ? name + 1 : entity->subdev.name; + + pr_cont("%s%s", first ? "" : ", ", name); + first = false; + } + + pr_cont("\n"); +} + /* Must be called with the pipe irqlock held. */ void vsp1_pipeline_run(struct vsp1_pipeline *pipe) { diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h index c1f411227de7..46a82a9f766a 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h @@ -9,6 +9,7 @@ #ifndef __VSP1_PIPE_H__ #define __VSP1_PIPE_H__ +#include <linux/dynamic_debug.h> #include <linux/kref.h> #include <linux/list.h> #include <linux/spinlock.h> @@ -142,6 +143,24 @@ struct vsp1_pipeline { void vsp1_pipeline_reset(struct vsp1_pipeline *pipe); void vsp1_pipeline_init(struct vsp1_pipeline *pipe); +void __vsp1_pipeline_dump(struct _ddebug *, struct vsp1_pipeline *pipe, + const char *msg); + +#if defined(CONFIG_DYNAMIC_DEBUG) || \ + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE)) +#define vsp1_pipeline_dump(pipe, msg) \ + _dynamic_func_call("vsp1_pipeline_dump()", __vsp1_pipeline_dump, pipe, msg) +#elif defined(DEBUG) +#define vsp1_pipeline_dump(pipe, msg) \ + __vsp1_pipeline_dump(NULL, pipe, msg) +#else +#define vsp1_pipeline_dump(pipe, msg) \ +({ \ + if (0) \ + __vsp1_pipeline_dump(NULL, pipe, msg); \ +)} +#endif + void vsp1_pipeline_run(struct vsp1_pipeline *pipe); bool vsp1_pipeline_stopped(struct vsp1_pipeline *pipe); int vsp1_pipeline_stop(struct vsp1_pipeline *pipe); diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c index 6a8db541543a..84394994ccee 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c @@ -520,11 +520,19 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, static int vsp1_video_pipeline_init(struct vsp1_pipeline *pipe, struct vsp1_video *video) { + int ret; + vsp1_pipeline_init(pipe); pipe->frame_end = vsp1_video_pipeline_frame_end; - return vsp1_video_pipeline_build(pipe, video); + ret = vsp1_video_pipeline_build(pipe, video); + if (ret) + return ret; + + vsp1_pipeline_dump(pipe, "video"); + + return 0; } static struct vsp1_pipeline *vsp1_video_pipeline_get(struct vsp1_video *video)
It is useful for debugging purpose to dump a vsp1_pipeline to the kernel log. Add a new function to do so, and use it when initializing the video and DRM pipelines. As __vsp1_pipeline_dump() needs to construct the log message iteratively, it uses pr_cont(...) (exact equivalent to the more verbose "printk(KERN_CONT ..."). The function thus can't use dev_dbg() to log the initial part of the message, for two reasons: - pr_cont() doesn't seem to work with dev_*(). Even if the format string passed to dev_*() doesn't end with a '\n', pr_cont() starts a new line in the log. This behaviour doesn't seem to be clearly documented, and may or may not be on purpose. - Messages printed by dev_dbg() may be omitted if dynamic debugging is enabled. In that case, the continuation messages will still be printed, leading to confusing log messages. To still benefit from the dynamic debug infrastructure, we declare a vsp1_pipeline_dump() macro that uses _dynamic_func_call() when dynamic debugging is enabled. The whole vsp1_pipeline_dump() call can be selected at runtime. The __vsp1_pipeline_dump() function then uses a plain "printk(KERN_DEBUG ...)" to print the message header using the debug log level, and pr_cont() to print the rest of the message on the same line. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- .../media/platform/renesas/vsp1/vsp1_drm.c | 5 +++++ .../media/platform/renesas/vsp1/vsp1_pipe.c | 22 +++++++++++++++++++ .../media/platform/renesas/vsp1/vsp1_pipe.h | 19 ++++++++++++++++ .../media/platform/renesas/vsp1/vsp1_video.c | 10 ++++++++- 4 files changed, 55 insertions(+), 1 deletion(-)