openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] media: aspeed: add debugfs
@ 2021-09-29  9:00 Jammy Huang
  2021-10-25  7:59 ` Greg KH
  2021-10-25  9:34 ` Paul Menzel
  0 siblings, 2 replies; 5+ messages in thread
From: Jammy Huang @ 2021-09-29  9:00 UTC (permalink / raw)
  To: eajames, mchehab, joel, andrew, linux-media, openbmc,
	linux-arm-kernel, linux-aspeed, linux-kernel
  Cc: BMC-SW

To show video real-time information as below:

Caputre:
  Signal              : Unlock
  Width               : 1920
  Height              : 1080
  FRC                 : 30

Performance:
  Frame#              : 0
  Frame Duration      :
    Now               : 0
    Min               : 0
    Max               : 0
  FPS(ms)             : 0

Change-Id: I483740c4df6db07a9261c18440472a0356512bb7
Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 drivers/media/platform/aspeed-video.c | 101 ++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 8b3939b8052d..c5c413844441 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -21,6 +21,8 @@
 #include <linux/videodev2.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
+#include <linux/debugfs.h>
+#include <linux/ktime.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-dev.h>
 #include <media/v4l2-device.h>
@@ -203,6 +205,14 @@ struct aspeed_video_buffer {
 	struct list_head link;
 };
 
+struct aspeed_video_perf {
+	ktime_t last_sample;
+	u32 totaltime;
+	u32 duration;
+	u32 duration_min;
+	u32 duration_max;
+};
+
 #define to_aspeed_video_buffer(x) \
 	container_of((x), struct aspeed_video_buffer, vb)
 
@@ -241,6 +251,8 @@ struct aspeed_video {
 	unsigned int frame_left;
 	unsigned int frame_right;
 	unsigned int frame_top;
+
+	struct aspeed_video_perf perf;
 };
 
 #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
@@ -444,6 +456,16 @@ static void aspeed_video_write(struct aspeed_video *video, u32 reg, u32 val)
 		readl(video->base + reg));
 }
 
+static void update_perf(struct aspeed_video_perf *p)
+{
+	p->duration =
+		ktime_to_ms(ktime_sub(ktime_get(),  p->last_sample));
+	p->totaltime += p->duration;
+
+	p->duration_max = max(p->duration, p->duration_max);
+	p->duration_min = min(p->duration, p->duration_min);
+}
+
 static int aspeed_video_start_frame(struct aspeed_video *video)
 {
 	dma_addr_t addr;
@@ -482,6 +504,8 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
 	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
 			    VE_INTERRUPT_COMP_COMPLETE);
 
+	video->perf.last_sample = ktime_get();
+
 	aspeed_video_update(video, VE_SEQ_CTRL, 0,
 			    VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
 
@@ -600,6 +624,8 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
 		u32 frame_size = aspeed_video_read(video,
 						   VE_JPEG_COMP_SIZE_READ_BACK);
 
+		update_perf(&video->perf);
+
 		spin_lock(&video->lock);
 		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
 		buf = list_first_entry_or_null(&video->buffers,
@@ -760,6 +786,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
 	det->width = MIN_WIDTH;
 	det->height = MIN_HEIGHT;
 	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
+	memset(&video->perf, 0, sizeof(video->perf));
 
 	do {
 		if (tries) {
@@ -1450,6 +1477,8 @@ static int aspeed_video_start_streaming(struct vb2_queue *q,
 	struct aspeed_video *video = vb2_get_drv_priv(q);
 
 	video->sequence = 0;
+	video->perf.duration_max = 0;
+	video->perf.duration_min = 0xffffffff;
 
 	rc = aspeed_video_start_frame(video);
 	if (rc) {
@@ -1517,6 +1546,72 @@ static const struct vb2_ops aspeed_video_vb2_ops = {
 	.buf_queue =  aspeed_video_buf_queue,
 };
 
+#ifdef CONFIG_DEBUG_FS
+static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
+{
+	struct aspeed_video *v = s->private;
+
+	seq_puts(s, "\n");
+
+	seq_printf(s, "  %-20s:\t%s\n", "Signal",
+		   v->v4l2_input_status ? "Unlock" : "Lock");
+	seq_printf(s, "  %-20s:\t%d\n", "Width", v->pix_fmt.width);
+	seq_printf(s, "  %-20s:\t%d\n", "Height", v->pix_fmt.height);
+	seq_printf(s, "  %-20s:\t%d\n", "FRC", v->frame_rate);
+
+	seq_puts(s, "\n");
+
+	seq_puts(s, "Performance:\n");
+	seq_printf(s, "  %-20s:\t%d\n", "Frame#", v->sequence);
+	seq_printf(s, "  %-20s:\n", "Frame Duration");
+	seq_printf(s, "    %-18s:\t%d\n", "Now", v->perf.duration);
+	seq_printf(s, "    %-18s:\t%d\n", "Min", v->perf.duration_min);
+	seq_printf(s, "    %-18s:\t%d\n", "Max", v->perf.duration_max);
+	seq_printf(s, "  %-20s:\t%d\n", "FPS(ms)", 1000/(v->perf.totaltime/v->sequence));
+
+
+	return 0;
+}
+
+int aspeed_video_proc_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, aspeed_video_debugfs_show, inode->i_private);
+}
+
+static struct file_operations aspeed_video_debugfs_ops = {
+	.owner   = THIS_MODULE,
+	.open    = aspeed_video_proc_open,
+	.read    = seq_read,
+	.llseek  = seq_lseek,
+	.release = single_release,
+};
+
+static struct dentry *debugfs_entry;
+
+static void aspeed_video_debugfs_remove(struct aspeed_video *video)
+{
+	debugfs_remove_recursive(debugfs_entry);
+	debugfs_entry = NULL;
+}
+
+static int aspeed_video_debugfs_create(struct aspeed_video *video)
+{
+	debugfs_entry = debugfs_create_file(DEVICE_NAME, 0444, NULL,
+						   video,
+						   &aspeed_video_debugfs_ops);
+	if (!debugfs_entry)
+		aspeed_video_debugfs_remove(video);
+
+	return debugfs_entry == NULL ? -EIO : 0;
+}
+#else
+static void aspeed_video_debugfs_remove(struct aspeed_video *video) { }
+static int aspeed_video_debugfs_create(struct aspeed_video *video)
+{
+	return 0;
+}
+#endif /* CONFIG_DEBUG_FS */
+
 static int aspeed_video_setup_video(struct aspeed_video *video)
 {
 	const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
@@ -1708,6 +1803,10 @@ static int aspeed_video_probe(struct platform_device *pdev)
 		return rc;
 	}
 
+	rc = aspeed_video_debugfs_create(video);
+	if (rc)
+		dev_err(video->dev, "debugfs create failed\n");
+
 	return 0;
 }
 
@@ -1719,6 +1818,8 @@ static int aspeed_video_remove(struct platform_device *pdev)
 
 	aspeed_video_off(video);
 
+	aspeed_video_debugfs_remove(video);
+
 	clk_unprepare(video->vclk);
 	clk_unprepare(video->eclk);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] media: aspeed: add debugfs
  2021-09-29  9:00 [PATCH v2] media: aspeed: add debugfs Jammy Huang
@ 2021-10-25  7:59 ` Greg KH
  2021-10-25 10:06   ` Jammy Huang
  2021-10-25  9:34 ` Paul Menzel
  1 sibling, 1 reply; 5+ messages in thread
From: Greg KH @ 2021-10-25  7:59 UTC (permalink / raw)
  To: Jammy Huang
  Cc: BMC-SW, linux-aspeed, andrew, openbmc, eajames, linux-kernel,
	mchehab, linux-arm-kernel, linux-media

On Wed, Sep 29, 2021 at 05:00:25PM +0800, Jammy Huang wrote:
> To show video real-time information as below:
> 
> Caputre:
>   Signal              : Unlock
>   Width               : 1920
>   Height              : 1080
>   FRC                 : 30
> 
> Performance:
>   Frame#              : 0
>   Frame Duration      :
>     Now               : 0
>     Min               : 0
>     Max               : 0
>   FPS(ms)             : 0
> 
> Change-Id: I483740c4df6db07a9261c18440472a0356512bb7
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>  drivers/media/platform/aspeed-video.c | 101 ++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 8b3939b8052d..c5c413844441 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -21,6 +21,8 @@
>  #include <linux/videodev2.h>
>  #include <linux/wait.h>
>  #include <linux/workqueue.h>
> +#include <linux/debugfs.h>
> +#include <linux/ktime.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-dev.h>
>  #include <media/v4l2-device.h>
> @@ -203,6 +205,14 @@ struct aspeed_video_buffer {
>  	struct list_head link;
>  };
>  
> +struct aspeed_video_perf {
> +	ktime_t last_sample;
> +	u32 totaltime;
> +	u32 duration;
> +	u32 duration_min;
> +	u32 duration_max;
> +};
> +
>  #define to_aspeed_video_buffer(x) \
>  	container_of((x), struct aspeed_video_buffer, vb)
>  
> @@ -241,6 +251,8 @@ struct aspeed_video {
>  	unsigned int frame_left;
>  	unsigned int frame_right;
>  	unsigned int frame_top;
> +
> +	struct aspeed_video_perf perf;
>  };
>  
>  #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
> @@ -444,6 +456,16 @@ static void aspeed_video_write(struct aspeed_video *video, u32 reg, u32 val)
>  		readl(video->base + reg));
>  }
>  
> +static void update_perf(struct aspeed_video_perf *p)
> +{
> +	p->duration =
> +		ktime_to_ms(ktime_sub(ktime_get(),  p->last_sample));
> +	p->totaltime += p->duration;
> +
> +	p->duration_max = max(p->duration, p->duration_max);
> +	p->duration_min = min(p->duration, p->duration_min);
> +}
> +
>  static int aspeed_video_start_frame(struct aspeed_video *video)
>  {
>  	dma_addr_t addr;
> @@ -482,6 +504,8 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>  	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
>  			    VE_INTERRUPT_COMP_COMPLETE);
>  
> +	video->perf.last_sample = ktime_get();
> +
>  	aspeed_video_update(video, VE_SEQ_CTRL, 0,
>  			    VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
>  
> @@ -600,6 +624,8 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>  		u32 frame_size = aspeed_video_read(video,
>  						   VE_JPEG_COMP_SIZE_READ_BACK);
>  
> +		update_perf(&video->perf);
> +
>  		spin_lock(&video->lock);
>  		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
>  		buf = list_first_entry_or_null(&video->buffers,
> @@ -760,6 +786,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>  	det->width = MIN_WIDTH;
>  	det->height = MIN_HEIGHT;
>  	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
> +	memset(&video->perf, 0, sizeof(video->perf));
>  
>  	do {
>  		if (tries) {
> @@ -1450,6 +1477,8 @@ static int aspeed_video_start_streaming(struct vb2_queue *q,
>  	struct aspeed_video *video = vb2_get_drv_priv(q);
>  
>  	video->sequence = 0;
> +	video->perf.duration_max = 0;
> +	video->perf.duration_min = 0xffffffff;
>  
>  	rc = aspeed_video_start_frame(video);
>  	if (rc) {
> @@ -1517,6 +1546,72 @@ static const struct vb2_ops aspeed_video_vb2_ops = {
>  	.buf_queue =  aspeed_video_buf_queue,
>  };
>  
> +#ifdef CONFIG_DEBUG_FS
> +static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
> +{
> +	struct aspeed_video *v = s->private;
> +
> +	seq_puts(s, "\n");
> +
> +	seq_printf(s, "  %-20s:\t%s\n", "Signal",
> +		   v->v4l2_input_status ? "Unlock" : "Lock");
> +	seq_printf(s, "  %-20s:\t%d\n", "Width", v->pix_fmt.width);
> +	seq_printf(s, "  %-20s:\t%d\n", "Height", v->pix_fmt.height);
> +	seq_printf(s, "  %-20s:\t%d\n", "FRC", v->frame_rate);
> +
> +	seq_puts(s, "\n");
> +
> +	seq_puts(s, "Performance:\n");
> +	seq_printf(s, "  %-20s:\t%d\n", "Frame#", v->sequence);
> +	seq_printf(s, "  %-20s:\n", "Frame Duration");
> +	seq_printf(s, "    %-18s:\t%d\n", "Now", v->perf.duration);
> +	seq_printf(s, "    %-18s:\t%d\n", "Min", v->perf.duration_min);
> +	seq_printf(s, "    %-18s:\t%d\n", "Max", v->perf.duration_max);
> +	seq_printf(s, "  %-20s:\t%d\n", "FPS(ms)", 1000/(v->perf.totaltime/v->sequence));
> +
> +
> +	return 0;
> +}
> +
> +int aspeed_video_proc_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, aspeed_video_debugfs_show, inode->i_private);
> +}
> +
> +static struct file_operations aspeed_video_debugfs_ops = {
> +	.owner   = THIS_MODULE,
> +	.open    = aspeed_video_proc_open,
> +	.read    = seq_read,
> +	.llseek  = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static struct dentry *debugfs_entry;
> +
> +static void aspeed_video_debugfs_remove(struct aspeed_video *video)
> +{
> +	debugfs_remove_recursive(debugfs_entry);
> +	debugfs_entry = NULL;
> +}
> +
> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
> +{
> +	debugfs_entry = debugfs_create_file(DEVICE_NAME, 0444, NULL,
> +						   video,
> +						   &aspeed_video_debugfs_ops);
> +	if (!debugfs_entry)
> +		aspeed_video_debugfs_remove(video);

This check will never happen, why are you making it?

There's no need to care if debugfs works or not, just create the file
and move on.  This function should not return anything.

> +
> +	return debugfs_entry == NULL ? -EIO : 0;
> +}
> +#else
> +static void aspeed_video_debugfs_remove(struct aspeed_video *video) { }
> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
>  static int aspeed_video_setup_video(struct aspeed_video *video)
>  {
>  	const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
> @@ -1708,6 +1803,10 @@ static int aspeed_video_probe(struct platform_device *pdev)
>  		return rc;
>  	}
>  
> +	rc = aspeed_video_debugfs_create(video);
> +	if (rc)
> +		dev_err(video->dev, "debugfs create failed\n");

As proof of this, you don't care if this doesn't work, you just print
out a message that was already printed out :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] media: aspeed: add debugfs
  2021-09-29  9:00 [PATCH v2] media: aspeed: add debugfs Jammy Huang
  2021-10-25  7:59 ` Greg KH
@ 2021-10-25  9:34 ` Paul Menzel
  2021-10-25 10:07   ` Jammy Huang
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Menzel @ 2021-10-25  9:34 UTC (permalink / raw)
  To: Jammy Huang
  Cc: BMC-SW, linux-aspeed, andrew, openbmc, eajames, linux-kernel,
	mchehab, linux-arm-kernel, linux-media

Dear Jammy,


On 29.09.21 11:00, Jammy Huang wrote:
> To show video real-time information as below:

Please list the path for that file containing the information below.

> Caputre:

Capture

>    Signal              : Unlock
>    Width               : 1920
>    Height              : 1080
>    FRC                 : 30
> 
> Performance:
>    Frame#              : 0
>    Frame Duration      :
>      Now               : 0
>      Min               : 0
>      Max               : 0
>    FPS(ms)             : 0


Kind regards,

Paul


> Change-Id: I483740c4df6db07a9261c18440472a0356512bb7
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>   drivers/media/platform/aspeed-video.c | 101 ++++++++++++++++++++++++++
>   1 file changed, 101 insertions(+)
> 
> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
> index 8b3939b8052d..c5c413844441 100644
> --- a/drivers/media/platform/aspeed-video.c
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -21,6 +21,8 @@
>   #include <linux/videodev2.h>
>   #include <linux/wait.h>
>   #include <linux/workqueue.h>
> +#include <linux/debugfs.h>
> +#include <linux/ktime.h>
>   #include <media/v4l2-ctrls.h>
>   #include <media/v4l2-dev.h>
>   #include <media/v4l2-device.h>
> @@ -203,6 +205,14 @@ struct aspeed_video_buffer {
>   	struct list_head link;
>   };
>   
> +struct aspeed_video_perf {
> +	ktime_t last_sample;
> +	u32 totaltime;
> +	u32 duration;
> +	u32 duration_min;
> +	u32 duration_max;
> +};
> +
>   #define to_aspeed_video_buffer(x) \
>   	container_of((x), struct aspeed_video_buffer, vb)
>   
> @@ -241,6 +251,8 @@ struct aspeed_video {
>   	unsigned int frame_left;
>   	unsigned int frame_right;
>   	unsigned int frame_top;
> +
> +	struct aspeed_video_perf perf;
>   };
>   
>   #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
> @@ -444,6 +456,16 @@ static void aspeed_video_write(struct aspeed_video *video, u32 reg, u32 val)
>   		readl(video->base + reg));
>   }
>   
> +static void update_perf(struct aspeed_video_perf *p)
> +{
> +	p->duration =
> +		ktime_to_ms(ktime_sub(ktime_get(),  p->last_sample));
> +	p->totaltime += p->duration;
> +
> +	p->duration_max = max(p->duration, p->duration_max);
> +	p->duration_min = min(p->duration, p->duration_min);
> +}
> +
>   static int aspeed_video_start_frame(struct aspeed_video *video)
>   {
>   	dma_addr_t addr;
> @@ -482,6 +504,8 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>   	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
>   			    VE_INTERRUPT_COMP_COMPLETE);
>   
> +	video->perf.last_sample = ktime_get();
> +
>   	aspeed_video_update(video, VE_SEQ_CTRL, 0,
>   			    VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
>   
> @@ -600,6 +624,8 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>   		u32 frame_size = aspeed_video_read(video,
>   						   VE_JPEG_COMP_SIZE_READ_BACK);
>   
> +		update_perf(&video->perf);
> +
>   		spin_lock(&video->lock);
>   		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
>   		buf = list_first_entry_or_null(&video->buffers,
> @@ -760,6 +786,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>   	det->width = MIN_WIDTH;
>   	det->height = MIN_HEIGHT;
>   	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
> +	memset(&video->perf, 0, sizeof(video->perf));
>   
>   	do {
>   		if (tries) {
> @@ -1450,6 +1477,8 @@ static int aspeed_video_start_streaming(struct vb2_queue *q,
>   	struct aspeed_video *video = vb2_get_drv_priv(q);
>   
>   	video->sequence = 0;
> +	video->perf.duration_max = 0;
> +	video->perf.duration_min = 0xffffffff;
>   
>   	rc = aspeed_video_start_frame(video);
>   	if (rc) {
> @@ -1517,6 +1546,72 @@ static const struct vb2_ops aspeed_video_vb2_ops = {
>   	.buf_queue =  aspeed_video_buf_queue,
>   };
>   
> +#ifdef CONFIG_DEBUG_FS
> +static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
> +{
> +	struct aspeed_video *v = s->private;
> +
> +	seq_puts(s, "\n");
> +
> +	seq_printf(s, "  %-20s:\t%s\n", "Signal",
> +		   v->v4l2_input_status ? "Unlock" : "Lock");
> +	seq_printf(s, "  %-20s:\t%d\n", "Width", v->pix_fmt.width);
> +	seq_printf(s, "  %-20s:\t%d\n", "Height", v->pix_fmt.height);
> +	seq_printf(s, "  %-20s:\t%d\n", "FRC", v->frame_rate);
> +
> +	seq_puts(s, "\n");
> +
> +	seq_puts(s, "Performance:\n");
> +	seq_printf(s, "  %-20s:\t%d\n", "Frame#", v->sequence);
> +	seq_printf(s, "  %-20s:\n", "Frame Duration");
> +	seq_printf(s, "    %-18s:\t%d\n", "Now", v->perf.duration);
> +	seq_printf(s, "    %-18s:\t%d\n", "Min", v->perf.duration_min);
> +	seq_printf(s, "    %-18s:\t%d\n", "Max", v->perf.duration_max);
> +	seq_printf(s, "  %-20s:\t%d\n", "FPS(ms)", 1000/(v->perf.totaltime/v->sequence));
> +
> +
> +	return 0;
> +}
> +
> +int aspeed_video_proc_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, aspeed_video_debugfs_show, inode->i_private);
> +}
> +
> +static struct file_operations aspeed_video_debugfs_ops = {
> +	.owner   = THIS_MODULE,
> +	.open    = aspeed_video_proc_open,
> +	.read    = seq_read,
> +	.llseek  = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static struct dentry *debugfs_entry;
> +
> +static void aspeed_video_debugfs_remove(struct aspeed_video *video)
> +{
> +	debugfs_remove_recursive(debugfs_entry);
> +	debugfs_entry = NULL;
> +}
> +
> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
> +{
> +	debugfs_entry = debugfs_create_file(DEVICE_NAME, 0444, NULL,
> +						   video,
> +						   &aspeed_video_debugfs_ops);
> +	if (!debugfs_entry)
> +		aspeed_video_debugfs_remove(video);
> +
> +	return debugfs_entry == NULL ? -EIO : 0;
> +}
> +#else
> +static void aspeed_video_debugfs_remove(struct aspeed_video *video) { }
> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
>   static int aspeed_video_setup_video(struct aspeed_video *video)
>   {
>   	const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
> @@ -1708,6 +1803,10 @@ static int aspeed_video_probe(struct platform_device *pdev)
>   		return rc;
>   	}
>   
> +	rc = aspeed_video_debugfs_create(video);
> +	if (rc)
> +		dev_err(video->dev, "debugfs create failed\n");
> +
>   	return 0;
>   }
>   
> @@ -1719,6 +1818,8 @@ static int aspeed_video_remove(struct platform_device *pdev)
>   
>   	aspeed_video_off(video);
>   
> +	aspeed_video_debugfs_remove(video);
> +
>   	clk_unprepare(video->vclk);
>   	clk_unprepare(video->eclk);
>   
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] media: aspeed: add debugfs
  2021-10-25  7:59 ` Greg KH
@ 2021-10-25 10:06   ` Jammy Huang
  0 siblings, 0 replies; 5+ messages in thread
From: Jammy Huang @ 2021-10-25 10:06 UTC (permalink / raw)
  To: Greg KH
  Cc: BMC-SW, linux-aspeed, andrew, openbmc, eajames, linux-kernel,
	mchehab, linux-arm-kernel, linux-media

Dear Greg,

Yes, you are right. I will update accordingly.


On 2021/10/25 下午 03:59, Greg KH wrote:
> On Wed, Sep 29, 2021 at 05:00:25PM +0800, Jammy Huang wrote:
>> To show video real-time information as below:
>>
>> Caputre:
>>    Signal              : Unlock
>>    Width               : 1920
>>    Height              : 1080
>>    FRC                 : 30
>>
>> Performance:
>>    Frame#              : 0
>>    Frame Duration      :
>>      Now               : 0
>>      Min               : 0
>>      Max               : 0
>>    FPS(ms)             : 0
>>
>> Change-Id: I483740c4df6db07a9261c18440472a0356512bb7
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>>   drivers/media/platform/aspeed-video.c | 101 ++++++++++++++++++++++++++
>>   1 file changed, 101 insertions(+)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index 8b3939b8052d..c5c413844441 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -21,6 +21,8 @@
>>   #include <linux/videodev2.h>
>>   #include <linux/wait.h>
>>   #include <linux/workqueue.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/ktime.h>
>>   #include <media/v4l2-ctrls.h>
>>   #include <media/v4l2-dev.h>
>>   #include <media/v4l2-device.h>
>> @@ -203,6 +205,14 @@ struct aspeed_video_buffer {
>>   	struct list_head link;
>>   };
>>   
>> +struct aspeed_video_perf {
>> +	ktime_t last_sample;
>> +	u32 totaltime;
>> +	u32 duration;
>> +	u32 duration_min;
>> +	u32 duration_max;
>> +};
>> +
>>   #define to_aspeed_video_buffer(x) \
>>   	container_of((x), struct aspeed_video_buffer, vb)
>>   
>> @@ -241,6 +251,8 @@ struct aspeed_video {
>>   	unsigned int frame_left;
>>   	unsigned int frame_right;
>>   	unsigned int frame_top;
>> +
>> +	struct aspeed_video_perf perf;
>>   };
>>   
>>   #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
>> @@ -444,6 +456,16 @@ static void aspeed_video_write(struct aspeed_video *video, u32 reg, u32 val)
>>   		readl(video->base + reg));
>>   }
>>   
>> +static void update_perf(struct aspeed_video_perf *p)
>> +{
>> +	p->duration =
>> +		ktime_to_ms(ktime_sub(ktime_get(),  p->last_sample));
>> +	p->totaltime += p->duration;
>> +
>> +	p->duration_max = max(p->duration, p->duration_max);
>> +	p->duration_min = min(p->duration, p->duration_min);
>> +}
>> +
>>   static int aspeed_video_start_frame(struct aspeed_video *video)
>>   {
>>   	dma_addr_t addr;
>> @@ -482,6 +504,8 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>>   	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
>>   			    VE_INTERRUPT_COMP_COMPLETE);
>>   
>> +	video->perf.last_sample = ktime_get();
>> +
>>   	aspeed_video_update(video, VE_SEQ_CTRL, 0,
>>   			    VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
>>   
>> @@ -600,6 +624,8 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>   		u32 frame_size = aspeed_video_read(video,
>>   						   VE_JPEG_COMP_SIZE_READ_BACK);
>>   
>> +		update_perf(&video->perf);
>> +
>>   		spin_lock(&video->lock);
>>   		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
>>   		buf = list_first_entry_or_null(&video->buffers,
>> @@ -760,6 +786,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>   	det->width = MIN_WIDTH;
>>   	det->height = MIN_HEIGHT;
>>   	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
>> +	memset(&video->perf, 0, sizeof(video->perf));
>>   
>>   	do {
>>   		if (tries) {
>> @@ -1450,6 +1477,8 @@ static int aspeed_video_start_streaming(struct vb2_queue *q,
>>   	struct aspeed_video *video = vb2_get_drv_priv(q);
>>   
>>   	video->sequence = 0;
>> +	video->perf.duration_max = 0;
>> +	video->perf.duration_min = 0xffffffff;
>>   
>>   	rc = aspeed_video_start_frame(video);
>>   	if (rc) {
>> @@ -1517,6 +1546,72 @@ static const struct vb2_ops aspeed_video_vb2_ops = {
>>   	.buf_queue =  aspeed_video_buf_queue,
>>   };
>>   
>> +#ifdef CONFIG_DEBUG_FS
>> +static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
>> +{
>> +	struct aspeed_video *v = s->private;
>> +
>> +	seq_puts(s, "\n");
>> +
>> +	seq_printf(s, "  %-20s:\t%s\n", "Signal",
>> +		   v->v4l2_input_status ? "Unlock" : "Lock");
>> +	seq_printf(s, "  %-20s:\t%d\n", "Width", v->pix_fmt.width);
>> +	seq_printf(s, "  %-20s:\t%d\n", "Height", v->pix_fmt.height);
>> +	seq_printf(s, "  %-20s:\t%d\n", "FRC", v->frame_rate);
>> +
>> +	seq_puts(s, "\n");
>> +
>> +	seq_puts(s, "Performance:\n");
>> +	seq_printf(s, "  %-20s:\t%d\n", "Frame#", v->sequence);
>> +	seq_printf(s, "  %-20s:\n", "Frame Duration");
>> +	seq_printf(s, "    %-18s:\t%d\n", "Now", v->perf.duration);
>> +	seq_printf(s, "    %-18s:\t%d\n", "Min", v->perf.duration_min);
>> +	seq_printf(s, "    %-18s:\t%d\n", "Max", v->perf.duration_max);
>> +	seq_printf(s, "  %-20s:\t%d\n", "FPS(ms)", 1000/(v->perf.totaltime/v->sequence));
>> +
>> +
>> +	return 0;
>> +}
>> +
>> +int aspeed_video_proc_open(struct inode *inode, struct file *file)
>> +{
>> +	return single_open(file, aspeed_video_debugfs_show, inode->i_private);
>> +}
>> +
>> +static struct file_operations aspeed_video_debugfs_ops = {
>> +	.owner   = THIS_MODULE,
>> +	.open    = aspeed_video_proc_open,
>> +	.read    = seq_read,
>> +	.llseek  = seq_lseek,
>> +	.release = single_release,
>> +};
>> +
>> +static struct dentry *debugfs_entry;
>> +
>> +static void aspeed_video_debugfs_remove(struct aspeed_video *video)
>> +{
>> +	debugfs_remove_recursive(debugfs_entry);
>> +	debugfs_entry = NULL;
>> +}
>> +
>> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
>> +{
>> +	debugfs_entry = debugfs_create_file(DEVICE_NAME, 0444, NULL,
>> +						   video,
>> +						   &aspeed_video_debugfs_ops);
>> +	if (!debugfs_entry)
>> +		aspeed_video_debugfs_remove(video);
> This check will never happen, why are you making it?
>
> There's no need to care if debugfs works or not, just create the file
> and move on.  This function should not return anything.
>
>> +
>> +	return debugfs_entry == NULL ? -EIO : 0;
>> +}
>> +#else
>> +static void aspeed_video_debugfs_remove(struct aspeed_video *video) { }
>> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
>> +{
>> +	return 0;
>> +}
>> +#endif /* CONFIG_DEBUG_FS */
>> +
>>   static int aspeed_video_setup_video(struct aspeed_video *video)
>>   {
>>   	const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
>> @@ -1708,6 +1803,10 @@ static int aspeed_video_probe(struct platform_device *pdev)
>>   		return rc;
>>   	}
>>   
>> +	rc = aspeed_video_debugfs_create(video);
>> +	if (rc)
>> +		dev_err(video->dev, "debugfs create failed\n");
> As proof of this, you don't care if this doesn't work, you just print
> out a message that was already printed out :(
>
> thanks,
>
> greg k-h

-- 
--
Best Regards
Jammy


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] media: aspeed: add debugfs
  2021-10-25  9:34 ` Paul Menzel
@ 2021-10-25 10:07   ` Jammy Huang
  0 siblings, 0 replies; 5+ messages in thread
From: Jammy Huang @ 2021-10-25 10:07 UTC (permalink / raw)
  To: Paul Menzel
  Cc: BMC-SW, linux-aspeed, andrew, openbmc, eajames, linux-kernel,
	mchehab, linux-arm-kernel, linux-media

Dear Paul,

Sure, no problem.

On 2021/10/25 下午 05:34, Paul Menzel wrote:
> Dear Jammy,
>
>
> On 29.09.21 11:00, Jammy Huang wrote:
>> To show video real-time information as below:
> Please list the path for that file containing the information below.
>
>> Caputre:
> Capture
>
>>     Signal              : Unlock
>>     Width               : 1920
>>     Height              : 1080
>>     FRC                 : 30
>>
>> Performance:
>>     Frame#              : 0
>>     Frame Duration      :
>>       Now               : 0
>>       Min               : 0
>>       Max               : 0
>>     FPS(ms)             : 0
>
> Kind regards,
>
> Paul
>
>
>> Change-Id: I483740c4df6db07a9261c18440472a0356512bb7
>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>> ---
>>    drivers/media/platform/aspeed-video.c | 101 ++++++++++++++++++++++++++
>>    1 file changed, 101 insertions(+)
>>
>> diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
>> index 8b3939b8052d..c5c413844441 100644
>> --- a/drivers/media/platform/aspeed-video.c
>> +++ b/drivers/media/platform/aspeed-video.c
>> @@ -21,6 +21,8 @@
>>    #include <linux/videodev2.h>
>>    #include <linux/wait.h>
>>    #include <linux/workqueue.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/ktime.h>
>>    #include <media/v4l2-ctrls.h>
>>    #include <media/v4l2-dev.h>
>>    #include <media/v4l2-device.h>
>> @@ -203,6 +205,14 @@ struct aspeed_video_buffer {
>>    	struct list_head link;
>>    };
>>    
>> +struct aspeed_video_perf {
>> +	ktime_t last_sample;
>> +	u32 totaltime;
>> +	u32 duration;
>> +	u32 duration_min;
>> +	u32 duration_max;
>> +};
>> +
>>    #define to_aspeed_video_buffer(x) \
>>    	container_of((x), struct aspeed_video_buffer, vb)
>>    
>> @@ -241,6 +251,8 @@ struct aspeed_video {
>>    	unsigned int frame_left;
>>    	unsigned int frame_right;
>>    	unsigned int frame_top;
>> +
>> +	struct aspeed_video_perf perf;
>>    };
>>    
>>    #define to_aspeed_video(x) container_of((x), struct aspeed_video, v4l2_dev)
>> @@ -444,6 +456,16 @@ static void aspeed_video_write(struct aspeed_video *video, u32 reg, u32 val)
>>    		readl(video->base + reg));
>>    }
>>    
>> +static void update_perf(struct aspeed_video_perf *p)
>> +{
>> +	p->duration =
>> +		ktime_to_ms(ktime_sub(ktime_get(),  p->last_sample));
>> +	p->totaltime += p->duration;
>> +
>> +	p->duration_max = max(p->duration, p->duration_max);
>> +	p->duration_min = min(p->duration, p->duration_min);
>> +}
>> +
>>    static int aspeed_video_start_frame(struct aspeed_video *video)
>>    {
>>    	dma_addr_t addr;
>> @@ -482,6 +504,8 @@ static int aspeed_video_start_frame(struct aspeed_video *video)
>>    	aspeed_video_update(video, VE_INTERRUPT_CTRL, 0,
>>    			    VE_INTERRUPT_COMP_COMPLETE);
>>    
>> +	video->perf.last_sample = ktime_get();
>> +
>>    	aspeed_video_update(video, VE_SEQ_CTRL, 0,
>>    			    VE_SEQ_CTRL_TRIG_CAPTURE | VE_SEQ_CTRL_TRIG_COMP);
>>    
>> @@ -600,6 +624,8 @@ static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>    		u32 frame_size = aspeed_video_read(video,
>>    						   VE_JPEG_COMP_SIZE_READ_BACK);
>>    
>> +		update_perf(&video->perf);
>> +
>>    		spin_lock(&video->lock);
>>    		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
>>    		buf = list_first_entry_or_null(&video->buffers,
>> @@ -760,6 +786,7 @@ static void aspeed_video_get_resolution(struct aspeed_video *video)
>>    	det->width = MIN_WIDTH;
>>    	det->height = MIN_HEIGHT;
>>    	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
>> +	memset(&video->perf, 0, sizeof(video->perf));
>>    
>>    	do {
>>    		if (tries) {
>> @@ -1450,6 +1477,8 @@ static int aspeed_video_start_streaming(struct vb2_queue *q,
>>    	struct aspeed_video *video = vb2_get_drv_priv(q);
>>    
>>    	video->sequence = 0;
>> +	video->perf.duration_max = 0;
>> +	video->perf.duration_min = 0xffffffff;
>>    
>>    	rc = aspeed_video_start_frame(video);
>>    	if (rc) {
>> @@ -1517,6 +1546,72 @@ static const struct vb2_ops aspeed_video_vb2_ops = {
>>    	.buf_queue =  aspeed_video_buf_queue,
>>    };
>>    
>> +#ifdef CONFIG_DEBUG_FS
>> +static int aspeed_video_debugfs_show(struct seq_file *s, void *data)
>> +{
>> +	struct aspeed_video *v = s->private;
>> +
>> +	seq_puts(s, "\n");
>> +
>> +	seq_printf(s, "  %-20s:\t%s\n", "Signal",
>> +		   v->v4l2_input_status ? "Unlock" : "Lock");
>> +	seq_printf(s, "  %-20s:\t%d\n", "Width", v->pix_fmt.width);
>> +	seq_printf(s, "  %-20s:\t%d\n", "Height", v->pix_fmt.height);
>> +	seq_printf(s, "  %-20s:\t%d\n", "FRC", v->frame_rate);
>> +
>> +	seq_puts(s, "\n");
>> +
>> +	seq_puts(s, "Performance:\n");
>> +	seq_printf(s, "  %-20s:\t%d\n", "Frame#", v->sequence);
>> +	seq_printf(s, "  %-20s:\n", "Frame Duration");
>> +	seq_printf(s, "    %-18s:\t%d\n", "Now", v->perf.duration);
>> +	seq_printf(s, "    %-18s:\t%d\n", "Min", v->perf.duration_min);
>> +	seq_printf(s, "    %-18s:\t%d\n", "Max", v->perf.duration_max);
>> +	seq_printf(s, "  %-20s:\t%d\n", "FPS(ms)", 1000/(v->perf.totaltime/v->sequence));
>> +
>> +
>> +	return 0;
>> +}
>> +
>> +int aspeed_video_proc_open(struct inode *inode, struct file *file)
>> +{
>> +	return single_open(file, aspeed_video_debugfs_show, inode->i_private);
>> +}
>> +
>> +static struct file_operations aspeed_video_debugfs_ops = {
>> +	.owner   = THIS_MODULE,
>> +	.open    = aspeed_video_proc_open,
>> +	.read    = seq_read,
>> +	.llseek  = seq_lseek,
>> +	.release = single_release,
>> +};
>> +
>> +static struct dentry *debugfs_entry;
>> +
>> +static void aspeed_video_debugfs_remove(struct aspeed_video *video)
>> +{
>> +	debugfs_remove_recursive(debugfs_entry);
>> +	debugfs_entry = NULL;
>> +}
>> +
>> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
>> +{
>> +	debugfs_entry = debugfs_create_file(DEVICE_NAME, 0444, NULL,
>> +						   video,
>> +						   &aspeed_video_debugfs_ops);
>> +	if (!debugfs_entry)
>> +		aspeed_video_debugfs_remove(video);
>> +
>> +	return debugfs_entry == NULL ? -EIO : 0;
>> +}
>> +#else
>> +static void aspeed_video_debugfs_remove(struct aspeed_video *video) { }
>> +static int aspeed_video_debugfs_create(struct aspeed_video *video)
>> +{
>> +	return 0;
>> +}
>> +#endif /* CONFIG_DEBUG_FS */
>> +
>>    static int aspeed_video_setup_video(struct aspeed_video *video)
>>    {
>>    	const u64 mask = ~(BIT(V4L2_JPEG_CHROMA_SUBSAMPLING_444) |
>> @@ -1708,6 +1803,10 @@ static int aspeed_video_probe(struct platform_device *pdev)
>>    		return rc;
>>    	}
>>    
>> +	rc = aspeed_video_debugfs_create(video);
>> +	if (rc)
>> +		dev_err(video->dev, "debugfs create failed\n");
>> +
>>    	return 0;
>>    }
>>    
>> @@ -1719,6 +1818,8 @@ static int aspeed_video_remove(struct platform_device *pdev)
>>    
>>    	aspeed_video_off(video);
>>    
>> +	aspeed_video_debugfs_remove(video);
>> +
>>    	clk_unprepare(video->vclk);
>>    	clk_unprepare(video->eclk);
>>    
>>
-- 
--
Best Regards
Jammy


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-10-25 10:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  9:00 [PATCH v2] media: aspeed: add debugfs Jammy Huang
2021-10-25  7:59 ` Greg KH
2021-10-25 10:06   ` Jammy Huang
2021-10-25  9:34 ` Paul Menzel
2021-10-25 10:07   ` Jammy Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).