| Message ID | 20231201140433.2126011-5-paul.elder@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: rkisp1: Misc fixes and debug | expand |
Quoting Paul Elder (2023-12-01 14:04:33) > Consolidate all the debugfs files that were each a single counter into a > single "counters" file. > > While at it, reset the counters at stream on time to make it easier for > to interpret the values in userspace. That gives a better atomicity here I think so that's good. I guess the debug struct could have a lock around it sometime - but I don't think that matters in this context. Resetting at stream on looks to make sense so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > New in v2 > > .../platform/rockchip/rkisp1/rkisp1-capture.c | 2 + > .../platform/rockchip/rkisp1/rkisp1-common.h | 4 ++ > .../platform/rockchip/rkisp1/rkisp1-debug.c | 69 ++++++++++++------- > 3 files changed, 50 insertions(+), 25 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > index c6d7e01c8949..67b2e94dfd67 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > @@ -1030,6 +1030,8 @@ rkisp1_vb2_start_streaming(struct vb2_queue *queue, unsigned int count) > struct media_entity *entity = &cap->vnode.vdev.entity; > int ret; > > + rkisp1_debug_reset_counters(cap->rkisp1); > + > mutex_lock(&cap->rkisp1->stream_lock); > > ret = video_device_pipeline_start(&cap->vnode.vdev, &cap->rkisp1->pipe); > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > index be69173958a4..789259fb304a 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > @@ -599,9 +599,13 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1); > void rkisp1_params_unregister(struct rkisp1_device *rkisp1); > > #if IS_ENABLED(CONFIG_DEBUG_FS) > +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1); > void rkisp1_debug_init(struct rkisp1_device *rkisp1); > void rkisp1_debug_cleanup(struct rkisp1_device *rkisp1); > #else > +static inline void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) > +{ > +} > static inline void rkisp1_debug_init(struct rkisp1_device *rkisp1) > { > } > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > index 79cda589d935..4358ed1367ed 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > @@ -25,6 +25,11 @@ struct rkisp1_debug_register { > const char * const name; > }; > > +struct rkisp1_debug_counter { > + const char * const name; > + unsigned long *value; > +}; > + > #define RKISP1_DEBUG_REG(name) { RKISP1_CIF_##name, 0, #name } > #define RKISP1_DEBUG_SHD_REG(name) { \ > RKISP1_CIF_##name, RKISP1_CIF_##name##_SHD, #name \ > @@ -191,6 +196,43 @@ static int rkisp1_debug_input_status_show(struct seq_file *m, void *p) > } > DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_input_status); > > +static int rkisp1_debug_counters_show(struct seq_file *m, void *p) > +{ > + struct rkisp1_device *rkisp1 = m->private; > + struct rkisp1_debug *debug = &rkisp1->debug; > + > + const struct rkisp1_debug_counter counters[] = { > + { "data_loss", &debug->data_loss }, > + { "outform_size_err", &debug->outform_size_error }, > + { "img_stabilization_size_error", &debug->img_stabilization_size_error }, > + { "inform_size_error", &debug->inform_size_error }, > + { "irq_delay", &debug->irq_delay }, > + { "mipi_error", &debug->mipi_error }, > + { "stats_error", &debug->stats_error }, > + { "mp_stop_timeout", &debug->stop_timeout[RKISP1_MAINPATH] }, > + { "sp_stop_timeout", &debug->stop_timeout[RKISP1_SELFPATH] }, > + { "mp_frame_drop", &debug->frame_drop[RKISP1_MAINPATH] }, > + { "sp_frame_drop", &debug->frame_drop[RKISP1_SELFPATH] }, > + { "complete_frames", &debug->complete_frames }, > + { /* Sentinel */ }, > + }; > + > + const struct rkisp1_debug_counter *counter = counters; > + > + for (; counter->name; ++counter) > + seq_printf(m, "%s: %lu\n", counter->name, *counter->value); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_counters); > + > +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) > +{ > + struct dentry *debugfs_dir = rkisp1->debug.debugfs_dir; > + memset(&rkisp1->debug, 0, sizeof(rkisp1->debug)); > + rkisp1->debug.debugfs_dir = debugfs_dir; > +} > + > void rkisp1_debug_init(struct rkisp1_device *rkisp1) > { > struct rkisp1_debug *debug = &rkisp1->debug; > @@ -198,31 +240,8 @@ void rkisp1_debug_init(struct rkisp1_device *rkisp1) > > debug->debugfs_dir = debugfs_create_dir(dev_name(rkisp1->dev), NULL); > > - debugfs_create_ulong("data_loss", 0444, debug->debugfs_dir, > - &debug->data_loss); > - debugfs_create_ulong("outform_size_err", 0444, debug->debugfs_dir, > - &debug->outform_size_error); > - debugfs_create_ulong("img_stabilization_size_error", 0444, > - debug->debugfs_dir, > - &debug->img_stabilization_size_error); > - debugfs_create_ulong("inform_size_error", 0444, debug->debugfs_dir, > - &debug->inform_size_error); > - debugfs_create_ulong("irq_delay", 0444, debug->debugfs_dir, > - &debug->irq_delay); > - debugfs_create_ulong("mipi_error", 0444, debug->debugfs_dir, > - &debug->mipi_error); > - debugfs_create_ulong("stats_error", 0444, debug->debugfs_dir, > - &debug->stats_error); > - debugfs_create_ulong("mp_stop_timeout", 0444, debug->debugfs_dir, > - &debug->stop_timeout[RKISP1_MAINPATH]); > - debugfs_create_ulong("sp_stop_timeout", 0444, debug->debugfs_dir, > - &debug->stop_timeout[RKISP1_SELFPATH]); > - debugfs_create_ulong("mp_frame_drop", 0444, debug->debugfs_dir, > - &debug->frame_drop[RKISP1_MAINPATH]); > - debugfs_create_ulong("sp_frame_drop", 0444, debug->debugfs_dir, > - &debug->frame_drop[RKISP1_SELFPATH]); > - debugfs_create_ulong("complete_frames", 0444, debug->debugfs_dir, > - &debug->complete_frames); > + debugfs_create_file("counters", 0444, debug->debugfs_dir, rkisp1, > + &rkisp1_debug_counters_fops); > debugfs_create_file("input_status", 0444, debug->debugfs_dir, rkisp1, > &rkisp1_debug_input_status_fops); > > -- > 2.39.2 >
On 01/12/2023 16:04, Paul Elder wrote: > Consolidate all the debugfs files that were each a single counter into a > single "counters" file. > > While at it, reset the counters at stream on time to make it easier for > to interpret the values in userspace. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > New in v2 > > .../platform/rockchip/rkisp1/rkisp1-capture.c | 2 + > .../platform/rockchip/rkisp1/rkisp1-common.h | 4 ++ > .../platform/rockchip/rkisp1/rkisp1-debug.c | 69 ++++++++++++------- > 3 files changed, 50 insertions(+), 25 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > index c6d7e01c8949..67b2e94dfd67 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > @@ -1030,6 +1030,8 @@ rkisp1_vb2_start_streaming(struct vb2_queue *queue, unsigned int count) > struct media_entity *entity = &cap->vnode.vdev.entity; > int ret; > > + rkisp1_debug_reset_counters(cap->rkisp1); > + > mutex_lock(&cap->rkisp1->stream_lock); > > ret = video_device_pipeline_start(&cap->vnode.vdev, &cap->rkisp1->pipe); > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > index be69173958a4..789259fb304a 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > @@ -599,9 +599,13 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1); > void rkisp1_params_unregister(struct rkisp1_device *rkisp1); > > #if IS_ENABLED(CONFIG_DEBUG_FS) > +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1); > void rkisp1_debug_init(struct rkisp1_device *rkisp1); > void rkisp1_debug_cleanup(struct rkisp1_device *rkisp1); > #else > +static inline void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) > +{ > +} > static inline void rkisp1_debug_init(struct rkisp1_device *rkisp1) > { > } > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > index 79cda589d935..4358ed1367ed 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > @@ -25,6 +25,11 @@ struct rkisp1_debug_register { > const char * const name; > }; > > +struct rkisp1_debug_counter { > + const char * const name; > + unsigned long *value; > +}; > + > #define RKISP1_DEBUG_REG(name) { RKISP1_CIF_##name, 0, #name } > #define RKISP1_DEBUG_SHD_REG(name) { \ > RKISP1_CIF_##name, RKISP1_CIF_##name##_SHD, #name \ > @@ -191,6 +196,43 @@ static int rkisp1_debug_input_status_show(struct seq_file *m, void *p) > } > DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_input_status); > > +static int rkisp1_debug_counters_show(struct seq_file *m, void *p) > +{ > + struct rkisp1_device *rkisp1 = m->private; > + struct rkisp1_debug *debug = &rkisp1->debug; > + > + const struct rkisp1_debug_counter counters[] = { > + { "data_loss", &debug->data_loss }, > + { "outform_size_err", &debug->outform_size_error }, > + { "img_stabilization_size_error", &debug->img_stabilization_size_error }, > + { "inform_size_error", &debug->inform_size_error }, > + { "irq_delay", &debug->irq_delay }, > + { "mipi_error", &debug->mipi_error }, > + { "stats_error", &debug->stats_error }, > + { "mp_stop_timeout", &debug->stop_timeout[RKISP1_MAINPATH] }, > + { "sp_stop_timeout", &debug->stop_timeout[RKISP1_SELFPATH] }, > + { "mp_frame_drop", &debug->frame_drop[RKISP1_MAINPATH] }, > + { "sp_frame_drop", &debug->frame_drop[RKISP1_SELFPATH] }, > + { "complete_frames", &debug->complete_frames }, > + { /* Sentinel */ }, > + }; > + > + const struct rkisp1_debug_counter *counter = counters; > + > + for (; counter->name; ++counter) > + seq_printf(m, "%s: %lu\n", counter->name, *counter->value); > + You could also do: const struct { const char *name; unsigned long value; } counters[] = { { "data_loss", debug->data_loss }, { "outform_size_err", debug->outform_size_error }, { "img_stabilization_size_error", debug->img_stabilization_size_error }, { "inform_size_error", debug->inform_size_error }, { "irq_delay", debug->irq_delay }, { "mipi_error", debug->mipi_error }, { "stats_error", debug->stats_error }, { "mp_stop_timeout", debug->stop_timeout[RKISP1_MAINPATH] }, { "sp_stop_timeout", debug->stop_timeout[RKISP1_SELFPATH] }, { "mp_frame_drop", debug->frame_drop[RKISP1_MAINPATH] }, { "sp_frame_drop", debug->frame_drop[RKISP1_SELFPATH] }, { "complete_frames", debug->complete_frames }, }; for (unsigned int i = 0; i < ARRAY_SIZE(counters); ++i) seq_printf(m, "%s: %lu\n", counters[i].name, counters[i].value); Not a big difference, but this doesn't "leak" the struct used only inside the function, doesn't need the sentinel, and I don't see a reason to store pointers to values, instead of just storing the values. Tomi > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_counters); > + > +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) > +{ > + struct dentry *debugfs_dir = rkisp1->debug.debugfs_dir; > + memset(&rkisp1->debug, 0, sizeof(rkisp1->debug)); > + rkisp1->debug.debugfs_dir = debugfs_dir; > +} > + > void rkisp1_debug_init(struct rkisp1_device *rkisp1) > { > struct rkisp1_debug *debug = &rkisp1->debug; > @@ -198,31 +240,8 @@ void rkisp1_debug_init(struct rkisp1_device *rkisp1) > > debug->debugfs_dir = debugfs_create_dir(dev_name(rkisp1->dev), NULL); > > - debugfs_create_ulong("data_loss", 0444, debug->debugfs_dir, > - &debug->data_loss); > - debugfs_create_ulong("outform_size_err", 0444, debug->debugfs_dir, > - &debug->outform_size_error); > - debugfs_create_ulong("img_stabilization_size_error", 0444, > - debug->debugfs_dir, > - &debug->img_stabilization_size_error); > - debugfs_create_ulong("inform_size_error", 0444, debug->debugfs_dir, > - &debug->inform_size_error); > - debugfs_create_ulong("irq_delay", 0444, debug->debugfs_dir, > - &debug->irq_delay); > - debugfs_create_ulong("mipi_error", 0444, debug->debugfs_dir, > - &debug->mipi_error); > - debugfs_create_ulong("stats_error", 0444, debug->debugfs_dir, > - &debug->stats_error); > - debugfs_create_ulong("mp_stop_timeout", 0444, debug->debugfs_dir, > - &debug->stop_timeout[RKISP1_MAINPATH]); > - debugfs_create_ulong("sp_stop_timeout", 0444, debug->debugfs_dir, > - &debug->stop_timeout[RKISP1_SELFPATH]); > - debugfs_create_ulong("mp_frame_drop", 0444, debug->debugfs_dir, > - &debug->frame_drop[RKISP1_MAINPATH]); > - debugfs_create_ulong("sp_frame_drop", 0444, debug->debugfs_dir, > - &debug->frame_drop[RKISP1_SELFPATH]); > - debugfs_create_ulong("complete_frames", 0444, debug->debugfs_dir, > - &debug->complete_frames); > + debugfs_create_file("counters", 0444, debug->debugfs_dir, rkisp1, > + &rkisp1_debug_counters_fops); > debugfs_create_file("input_status", 0444, debug->debugfs_dir, rkisp1, > &rkisp1_debug_input_status_fops); >
On Tue, Dec 05, 2023 at 10:22:47AM +0200, Tomi Valkeinen wrote: > On 01/12/2023 16:04, Paul Elder wrote: > > Consolidate all the debugfs files that were each a single counter into a > > single "counters" file. > > > > While at it, reset the counters at stream on time to make it easier for > > to interpret the values in userspace. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > New in v2 > > > > .../platform/rockchip/rkisp1/rkisp1-capture.c | 2 + > > .../platform/rockchip/rkisp1/rkisp1-common.h | 4 ++ > > .../platform/rockchip/rkisp1/rkisp1-debug.c | 69 ++++++++++++------- > > 3 files changed, 50 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > > index c6d7e01c8949..67b2e94dfd67 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > > @@ -1030,6 +1030,8 @@ rkisp1_vb2_start_streaming(struct vb2_queue *queue, unsigned int count) > > struct media_entity *entity = &cap->vnode.vdev.entity; > > int ret; > > > > + rkisp1_debug_reset_counters(cap->rkisp1); > > + > > mutex_lock(&cap->rkisp1->stream_lock); > > > > ret = video_device_pipeline_start(&cap->vnode.vdev, &cap->rkisp1->pipe); > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > index be69173958a4..789259fb304a 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > @@ -599,9 +599,13 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1); > > void rkisp1_params_unregister(struct rkisp1_device *rkisp1); > > > > #if IS_ENABLED(CONFIG_DEBUG_FS) > > +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1); > > void rkisp1_debug_init(struct rkisp1_device *rkisp1); > > void rkisp1_debug_cleanup(struct rkisp1_device *rkisp1); > > #else > > +static inline void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) > > +{ > > +} > > static inline void rkisp1_debug_init(struct rkisp1_device *rkisp1) > > { > > } > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > > index 79cda589d935..4358ed1367ed 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c > > @@ -25,6 +25,11 @@ struct rkisp1_debug_register { > > const char * const name; > > }; > > > > +struct rkisp1_debug_counter { > > + const char * const name; > > + unsigned long *value; > > +}; > > + > > #define RKISP1_DEBUG_REG(name) { RKISP1_CIF_##name, 0, #name } > > #define RKISP1_DEBUG_SHD_REG(name) { \ > > RKISP1_CIF_##name, RKISP1_CIF_##name##_SHD, #name \ > > @@ -191,6 +196,43 @@ static int rkisp1_debug_input_status_show(struct seq_file *m, void *p) > > } > > DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_input_status); > > > > +static int rkisp1_debug_counters_show(struct seq_file *m, void *p) > > +{ > > + struct rkisp1_device *rkisp1 = m->private; > > + struct rkisp1_debug *debug = &rkisp1->debug; > > + > > + const struct rkisp1_debug_counter counters[] = { > > + { "data_loss", &debug->data_loss }, > > + { "outform_size_err", &debug->outform_size_error }, > > + { "img_stabilization_size_error", &debug->img_stabilization_size_error }, > > + { "inform_size_error", &debug->inform_size_error }, > > + { "irq_delay", &debug->irq_delay }, > > + { "mipi_error", &debug->mipi_error }, > > + { "stats_error", &debug->stats_error }, > > + { "mp_stop_timeout", &debug->stop_timeout[RKISP1_MAINPATH] }, > > + { "sp_stop_timeout", &debug->stop_timeout[RKISP1_SELFPATH] }, > > + { "mp_frame_drop", &debug->frame_drop[RKISP1_MAINPATH] }, > > + { "sp_frame_drop", &debug->frame_drop[RKISP1_SELFPATH] }, > > + { "complete_frames", &debug->complete_frames }, > > + { /* Sentinel */ }, > > + }; > > + > > + const struct rkisp1_debug_counter *counter = counters; > > + > > + for (; counter->name; ++counter) > > + seq_printf(m, "%s: %lu\n", counter->name, *counter->value); > > + > > You could also do: > > const struct { > const char *name; > unsigned long value; > } counters[] = { > { "data_loss", debug->data_loss }, > { "outform_size_err", debug->outform_size_error }, > { "img_stabilization_size_error", debug->img_stabilization_size_error }, > { "inform_size_error", debug->inform_size_error }, > { "irq_delay", debug->irq_delay }, > { "mipi_error", debug->mipi_error }, > { "stats_error", debug->stats_error }, > { "mp_stop_timeout", debug->stop_timeout[RKISP1_MAINPATH] }, > { "sp_stop_timeout", debug->stop_timeout[RKISP1_SELFPATH] }, > { "mp_frame_drop", debug->frame_drop[RKISP1_MAINPATH] }, > { "sp_frame_drop", debug->frame_drop[RKISP1_SELFPATH] }, > { "complete_frames", debug->complete_frames }, > }; > > for (unsigned int i = 0; i < ARRAY_SIZE(counters); ++i) > seq_printf(m, "%s: %lu\n", counters[i].name, counters[i].value); > > Not a big difference, but this doesn't "leak" the struct used only inside the > function, doesn't need the sentinel, and I don't see a reason to store pointers > to values, instead of just storing the values. Wouldn't it be nicer to avoid constructing this on the stack for every call ? We could store the counters in an array: enum rkisp1_debug_counter { RKISP1_DBG_COUNT_DATA_LOSS, ... RKISP1_DBG_COUNT_FRAME_DROP_MP, RKISP1_DBG_COUNT_FRAME_DROP_SP, RKISP1_DBG_COUNT_MAX, }; struct rksisp1_debug { struct dentry *debugfs_dir; unsigned long counters[RKISP1_DBG_COUNT_MAX]; }; The counters[] array above would become a static const char * const counter_names[] = { [RKISP1_DBG_COUNT_DATA_LOSS] = "data_loss", ... }; > > + return 0; > > +} > > +DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_counters); > > + > > +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) > > +{ > > + struct dentry *debugfs_dir = rkisp1->debug.debugfs_dir; > > + memset(&rkisp1->debug, 0, sizeof(rkisp1->debug)); > > + rkisp1->debug.debugfs_dir = debugfs_dir; > > +} > > + > > void rkisp1_debug_init(struct rkisp1_device *rkisp1) > > { > > struct rkisp1_debug *debug = &rkisp1->debug; > > @@ -198,31 +240,8 @@ void rkisp1_debug_init(struct rkisp1_device *rkisp1) > > > > debug->debugfs_dir = debugfs_create_dir(dev_name(rkisp1->dev), NULL); > > > > - debugfs_create_ulong("data_loss", 0444, debug->debugfs_dir, > > - &debug->data_loss); > > - debugfs_create_ulong("outform_size_err", 0444, debug->debugfs_dir, > > - &debug->outform_size_error); > > - debugfs_create_ulong("img_stabilization_size_error", 0444, > > - debug->debugfs_dir, > > - &debug->img_stabilization_size_error); > > - debugfs_create_ulong("inform_size_error", 0444, debug->debugfs_dir, > > - &debug->inform_size_error); > > - debugfs_create_ulong("irq_delay", 0444, debug->debugfs_dir, > > - &debug->irq_delay); > > - debugfs_create_ulong("mipi_error", 0444, debug->debugfs_dir, > > - &debug->mipi_error); > > - debugfs_create_ulong("stats_error", 0444, debug->debugfs_dir, > > - &debug->stats_error); > > - debugfs_create_ulong("mp_stop_timeout", 0444, debug->debugfs_dir, > > - &debug->stop_timeout[RKISP1_MAINPATH]); > > - debugfs_create_ulong("sp_stop_timeout", 0444, debug->debugfs_dir, > > - &debug->stop_timeout[RKISP1_SELFPATH]); > > - debugfs_create_ulong("mp_frame_drop", 0444, debug->debugfs_dir, > > - &debug->frame_drop[RKISP1_MAINPATH]); > > - debugfs_create_ulong("sp_frame_drop", 0444, debug->debugfs_dir, > > - &debug->frame_drop[RKISP1_SELFPATH]); > > - debugfs_create_ulong("complete_frames", 0444, debug->debugfs_dir, > > - &debug->complete_frames); > > + debugfs_create_file("counters", 0444, debug->debugfs_dir, rkisp1, > > + &rkisp1_debug_counters_fops); > > debugfs_create_file("input_status", 0444, debug->debugfs_dir, rkisp1, > > &rkisp1_debug_input_status_fops); > >
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c index c6d7e01c8949..67b2e94dfd67 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c @@ -1030,6 +1030,8 @@ rkisp1_vb2_start_streaming(struct vb2_queue *queue, unsigned int count) struct media_entity *entity = &cap->vnode.vdev.entity; int ret; + rkisp1_debug_reset_counters(cap->rkisp1); + mutex_lock(&cap->rkisp1->stream_lock); ret = video_device_pipeline_start(&cap->vnode.vdev, &cap->rkisp1->pipe); diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h index be69173958a4..789259fb304a 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h @@ -599,9 +599,13 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1); void rkisp1_params_unregister(struct rkisp1_device *rkisp1); #if IS_ENABLED(CONFIG_DEBUG_FS) +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1); void rkisp1_debug_init(struct rkisp1_device *rkisp1); void rkisp1_debug_cleanup(struct rkisp1_device *rkisp1); #else +static inline void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) +{ +} static inline void rkisp1_debug_init(struct rkisp1_device *rkisp1) { } diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c index 79cda589d935..4358ed1367ed 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-debug.c @@ -25,6 +25,11 @@ struct rkisp1_debug_register { const char * const name; }; +struct rkisp1_debug_counter { + const char * const name; + unsigned long *value; +}; + #define RKISP1_DEBUG_REG(name) { RKISP1_CIF_##name, 0, #name } #define RKISP1_DEBUG_SHD_REG(name) { \ RKISP1_CIF_##name, RKISP1_CIF_##name##_SHD, #name \ @@ -191,6 +196,43 @@ static int rkisp1_debug_input_status_show(struct seq_file *m, void *p) } DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_input_status); +static int rkisp1_debug_counters_show(struct seq_file *m, void *p) +{ + struct rkisp1_device *rkisp1 = m->private; + struct rkisp1_debug *debug = &rkisp1->debug; + + const struct rkisp1_debug_counter counters[] = { + { "data_loss", &debug->data_loss }, + { "outform_size_err", &debug->outform_size_error }, + { "img_stabilization_size_error", &debug->img_stabilization_size_error }, + { "inform_size_error", &debug->inform_size_error }, + { "irq_delay", &debug->irq_delay }, + { "mipi_error", &debug->mipi_error }, + { "stats_error", &debug->stats_error }, + { "mp_stop_timeout", &debug->stop_timeout[RKISP1_MAINPATH] }, + { "sp_stop_timeout", &debug->stop_timeout[RKISP1_SELFPATH] }, + { "mp_frame_drop", &debug->frame_drop[RKISP1_MAINPATH] }, + { "sp_frame_drop", &debug->frame_drop[RKISP1_SELFPATH] }, + { "complete_frames", &debug->complete_frames }, + { /* Sentinel */ }, + }; + + const struct rkisp1_debug_counter *counter = counters; + + for (; counter->name; ++counter) + seq_printf(m, "%s: %lu\n", counter->name, *counter->value); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(rkisp1_debug_counters); + +void rkisp1_debug_reset_counters(struct rkisp1_device *rkisp1) +{ + struct dentry *debugfs_dir = rkisp1->debug.debugfs_dir; + memset(&rkisp1->debug, 0, sizeof(rkisp1->debug)); + rkisp1->debug.debugfs_dir = debugfs_dir; +} + void rkisp1_debug_init(struct rkisp1_device *rkisp1) { struct rkisp1_debug *debug = &rkisp1->debug; @@ -198,31 +240,8 @@ void rkisp1_debug_init(struct rkisp1_device *rkisp1) debug->debugfs_dir = debugfs_create_dir(dev_name(rkisp1->dev), NULL); - debugfs_create_ulong("data_loss", 0444, debug->debugfs_dir, - &debug->data_loss); - debugfs_create_ulong("outform_size_err", 0444, debug->debugfs_dir, - &debug->outform_size_error); - debugfs_create_ulong("img_stabilization_size_error", 0444, - debug->debugfs_dir, - &debug->img_stabilization_size_error); - debugfs_create_ulong("inform_size_error", 0444, debug->debugfs_dir, - &debug->inform_size_error); - debugfs_create_ulong("irq_delay", 0444, debug->debugfs_dir, - &debug->irq_delay); - debugfs_create_ulong("mipi_error", 0444, debug->debugfs_dir, - &debug->mipi_error); - debugfs_create_ulong("stats_error", 0444, debug->debugfs_dir, - &debug->stats_error); - debugfs_create_ulong("mp_stop_timeout", 0444, debug->debugfs_dir, - &debug->stop_timeout[RKISP1_MAINPATH]); - debugfs_create_ulong("sp_stop_timeout", 0444, debug->debugfs_dir, - &debug->stop_timeout[RKISP1_SELFPATH]); - debugfs_create_ulong("mp_frame_drop", 0444, debug->debugfs_dir, - &debug->frame_drop[RKISP1_MAINPATH]); - debugfs_create_ulong("sp_frame_drop", 0444, debug->debugfs_dir, - &debug->frame_drop[RKISP1_SELFPATH]); - debugfs_create_ulong("complete_frames", 0444, debug->debugfs_dir, - &debug->complete_frames); + debugfs_create_file("counters", 0444, debug->debugfs_dir, rkisp1, + &rkisp1_debug_counters_fops); debugfs_create_file("input_status", 0444, debug->debugfs_dir, rkisp1, &rkisp1_debug_input_status_fops);
Consolidate all the debugfs files that were each a single counter into a single "counters" file. While at it, reset the counters at stream on time to make it easier for to interpret the values in userspace. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v2 .../platform/rockchip/rkisp1/rkisp1-capture.c | 2 + .../platform/rockchip/rkisp1/rkisp1-common.h | 4 ++ .../platform/rockchip/rkisp1/rkisp1-debug.c | 69 ++++++++++++------- 3 files changed, 50 insertions(+), 25 deletions(-)