[PATCH 0/7] media: export InfoFrames to debugfs

[PATCH 0/7] media: export InfoFrames to debugfs

[Hans Verkuil]

Maxime added support for exporting InfoFrames to debugfs for drm,
and this series does the same for the media subsystem.

I used the same names for the InfoFrames as the drm implementation
does, and the format is the same as well. And edid-decode can be
used to parse the InfoFrames and do conformity checking.

The first two patches add helpers for this to the core framework,
and the next 5 patches add support for this to the HDMI drivers.

I tested the three adv drivers, and Dave Stevenson tested the tc358743
driver.

I don't have a tda1997x available for testing, so I might decide
to just drop that patch.

This is very useful for debugging received InfoFrames.

Regards,

	Hans

Hans Verkuil (7):
  media: v4l2-core: add v4l2_debugfs_root()
  media: v4l2-core: add v4l2_debugfs_if_alloc/free()
  media: i2c: adv7511-v4l2: export InfoFrames to debugfs
  media: i2c: adv7604: export InfoFrames to debugfs
  media: i2c: adv7842: export InfoFrames to debugfs
  media: i2c: tc358743: export InfoFrames to debugfs
  media: i2c: tda1997x: export InfoFrames to debugfs

 drivers/media/i2c/adv7511-v4l2.c          |  91 +++++++++++++---
 drivers/media/i2c/adv7604.c               |  90 ++++++++++++----
 drivers/media/i2c/adv7842.c               | 120 ++++++++++++++++------
 drivers/media/i2c/tc358743.c              |  36 ++++++-
 drivers/media/i2c/tda1997x.c              |  50 ++++++++-
 drivers/media/v4l2-core/v4l2-dev.c        |  14 +++
 drivers/media/v4l2-core/v4l2-dv-timings.c |  63 ++++++++++++
 include/media/v4l2-dev.h                  |  15 +++
 include/media/v4l2-dv-timings.h           |  48 +++++++++
 9 files changed, 455 insertions(+), 72 deletions(-)

-- 
2.43.0

[PATCH 1/7] media: v4l2-core: add v4l2_debugfs_root()

[Hans Verkuil]

This new function returns the dentry of the top-level debugfs "v4l2"
directory. If it does not exist yet, then it is created first.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-dev.c | 14 ++++++++++++++
 include/media/v4l2-dev.h           | 15 +++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index be2ba7ca5de2..4bbf279a0c8b 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -93,6 +93,8 @@ static struct attribute *video_device_attrs[] = {
 };
 ATTRIBUTE_GROUPS(video_device);
 
+static struct dentry *v4l2_debugfs_root_dir;
+
 /*
  *	Active devices
  */
@@ -1104,6 +1106,16 @@ void video_unregister_device(struct video_device *vdev)
 }
 EXPORT_SYMBOL(video_unregister_device);
 
+#ifdef CONFIG_DEBUG_FS
+struct dentry *v4l2_debugfs_root(void)
+{
+	if (!v4l2_debugfs_root_dir)
+		v4l2_debugfs_root_dir = debugfs_create_dir("v4l2", NULL);
+	return v4l2_debugfs_root_dir;
+}
+EXPORT_SYMBOL_GPL(v4l2_debugfs_root);
+#endif
+
 #if defined(CONFIG_MEDIA_CONTROLLER)
 
 __must_check int video_device_pipeline_start(struct video_device *vdev,
@@ -1208,6 +1220,8 @@ static void __exit videodev_exit(void)
 
 	class_unregister(&video_class);
 	unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
+	debugfs_remove_recursive(v4l2_debugfs_root_dir);
+	v4l2_debugfs_root_dir = NULL;
 }
 
 subsys_initcall(videodev_init);
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index d82dfdbf6e58..1b6222fab24e 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -62,6 +62,7 @@ struct v4l2_ioctl_callbacks;
 struct video_device;
 struct v4l2_device;
 struct v4l2_ctrl_handler;
+struct dentry;
 
 /**
  * enum v4l2_video_device_flags - Flags used by &struct video_device
@@ -539,6 +540,20 @@ static inline int video_is_registered(struct video_device *vdev)
 	return test_bit(V4L2_FL_REGISTERED, &vdev->flags);
 }
 
+/**
+ * v4l2_debugfs_root - returns the dentry of the top-level "v4l2" debugfs dir
+ *
+ * If this directory does not yet exist, then it will be created.
+ */
+#ifdef CONFIG_DEBUG_FS
+struct dentry *v4l2_debugfs_root(void);
+#else
+static inline struct dentry *v4l2_debugfs_root(void)
+{
+	return NULL;
+}
+#endif
+
 #if defined(CONFIG_MEDIA_CONTROLLER)
 
 /**
-- 
2.43.0

[PATCH 2/7] media: v4l2-core: add v4l2_debugfs_if_alloc/free()

[Hans Verkuil]

Add new helpers to export received or transmitted HDMI InfoFrames to
debugfs.

This complements similar code in drm where the transmitted HDMI infoframes
are exported to debugfs.

The same names have been used as in drm, so this is consistent.

The exported infoframes can be parsed with the edid-decode utility.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-dv-timings.c | 63 +++++++++++++++++++++++
 include/media/v4l2-dv-timings.h           | 48 +++++++++++++++++
 2 files changed, 111 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c b/drivers/media/v4l2-core/v4l2-dv-timings.c
index 942d0005c55e..86a8627f4bcc 100644
--- a/drivers/media/v4l2-core/v4l2-dv-timings.c
+++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
@@ -1154,3 +1154,66 @@ int v4l2_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(v4l2_phys_addr_validate);
+
+#ifdef CONFIG_DEBUG_FS
+
+#define DEBUGFS_FOPS(type, flag)					\
+static ssize_t								\
+infoframe_read_##type(struct file *filp,				\
+		      char __user *ubuf, size_t count, loff_t *ppos)	\
+{									\
+	struct v4l2_debugfs_if *infoframes = filp->private_data;	\
+									\
+	return infoframes->if_read((flag), infoframes->priv, filp,	\
+				   ubuf, count, ppos);			\
+}									\
+									\
+static const struct file_operations infoframe_##type##_fops = {		\
+	.owner   = THIS_MODULE,						\
+	.open    = simple_open,						\
+	.read    = infoframe_read_##type,				\
+}
+
+DEBUGFS_FOPS(avi, V4L2_DEBUGFS_IF_AVI);
+DEBUGFS_FOPS(audio, V4L2_DEBUGFS_IF_AUDIO);
+DEBUGFS_FOPS(spd, V4L2_DEBUGFS_IF_SPD);
+DEBUGFS_FOPS(hdmi, V4L2_DEBUGFS_IF_HDMI);
+
+struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
+					      void *priv,
+					      v4l2_debugfs_if_read_t if_read)
+{
+	struct v4l2_debugfs_if *infoframes;
+
+	if (IS_ERR_OR_NULL(root) || !if_types || !if_read)
+		return NULL;
+
+	infoframes = kzalloc(sizeof(*infoframes), GFP_KERNEL);
+	if (!infoframes)
+		return NULL;
+
+	infoframes->if_dir = debugfs_create_dir("infoframes", root);
+	infoframes->priv = priv;
+	infoframes->if_read = if_read;
+	if (if_types & V4L2_DEBUGFS_IF_AVI)
+		debugfs_create_file("avi", 0400, infoframes->if_dir, infoframes, &infoframe_avi_fops);
+	if (if_types & V4L2_DEBUGFS_IF_AUDIO)
+		debugfs_create_file("audio", 0400, infoframes->if_dir, infoframes, &infoframe_audio_fops);
+	if (if_types & V4L2_DEBUGFS_IF_SPD)
+		debugfs_create_file("spd", 0400, infoframes->if_dir, infoframes, &infoframe_spd_fops);
+	if (if_types & V4L2_DEBUGFS_IF_HDMI)
+		debugfs_create_file("hdmi", 0400, infoframes->if_dir, infoframes, &infoframe_hdmi_fops);
+	return infoframes;
+}
+EXPORT_SYMBOL_GPL(v4l2_debugfs_if_alloc);
+
+void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes)
+{
+	if (infoframes) {
+		debugfs_remove_recursive(infoframes->if_dir);
+		kfree(infoframes);
+	}
+}
+EXPORT_SYMBOL_GPL(v4l2_debugfs_if_free);
+
+#endif
diff --git a/include/media/v4l2-dv-timings.h b/include/media/v4l2-dv-timings.h
index 8fa963326bf6..13830411bd6c 100644
--- a/include/media/v4l2-dv-timings.h
+++ b/include/media/v4l2-dv-timings.h
@@ -8,6 +8,7 @@
 #ifndef __V4L2_DV_TIMINGS_H
 #define __V4L2_DV_TIMINGS_H
 
+#include <linux/debugfs.h>
 #include <linux/videodev2.h>
 
 /**
@@ -251,4 +252,51 @@ void v4l2_set_edid_phys_addr(u8 *edid, unsigned int size, u16 phys_addr);
 u16 v4l2_phys_addr_for_input(u16 phys_addr, u8 input);
 int v4l2_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port);
 
+/* Add support for exporting InfoFrames to debugfs */
+
+/*
+ * HDMI InfoFrames start with a 3 byte header, then a checksum,
+ * followed by the actual IF payload.
+ *
+ * The payload length is limited to 30 bytes according to the HDMI spec,
+ * but since the length is encoded in 5 bits, it can be 31 bytes theoretically.
+ * So set the max length as 31 + 3 (header) + 1 (checksum) = 35.
+ */
+#define V4L2_DEBUGFS_IF_MAX_LEN (35)
+
+#define V4L2_DEBUGFS_IF_AVI	BIT(0)
+#define V4L2_DEBUGFS_IF_AUDIO	BIT(1)
+#define V4L2_DEBUGFS_IF_SPD	BIT(2)
+#define V4L2_DEBUGFS_IF_HDMI	BIT(3)
+
+typedef ssize_t (*v4l2_debugfs_if_read_t)(u32 type, void *priv,
+					  struct file *filp, char __user *ubuf,
+					  size_t count, loff_t *ppos);
+
+struct v4l2_debugfs_if {
+	struct dentry *if_dir;
+	void *priv;
+
+	v4l2_debugfs_if_read_t if_read;
+};
+
+#ifdef CONFIG_DEBUG_FS
+struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
+					      void *priv,
+					      v4l2_debugfs_if_read_t if_read);
+void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes);
+#else
+static inline
+struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
+					      void *priv,
+					      v4l2_debugfs_if_read_t if_read)
+{
+	return NULL;
+}
+
+static inline void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes)
+{
+}
+#endif
+
 #endif
-- 
2.43.0

[PATCH 3/7] media: i2c: adv7511-v4l2: export InfoFrames to debugfs

[Hans Verkuil]

Export InfoFrames to debugfs.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/i2c/adv7511-v4l2.c | 91 ++++++++++++++++++++++++++------
 1 file changed, 74 insertions(+), 17 deletions(-)

diff --git a/drivers/media/i2c/adv7511-v4l2.c b/drivers/media/i2c/adv7511-v4l2.c
index e9406d552699..4036972af3a6 100644
--- a/drivers/media/i2c/adv7511-v4l2.c
+++ b/drivers/media/i2c/adv7511-v4l2.c
@@ -116,6 +116,9 @@ struct adv7511_state {
 	unsigned edid_detect_counter;
 	struct workqueue_struct *work_queue;
 	struct delayed_work edid_handler; /* work entry */
+
+	struct dentry *debugfs_dir;
+	struct v4l2_debugfs_if *infoframes;
 };
 
 static void adv7511_check_monitor_present_status(struct v4l2_subdev *sd);
@@ -483,27 +486,25 @@ static u8 hdmi_infoframe_checksum(u8 *ptr, size_t size)
 	return 256 - csum;
 }
 
-static void log_infoframe(struct v4l2_subdev *sd, const struct adv7511_cfg_read_infoframe *cri)
+static int read_infoframe(struct v4l2_subdev *sd,
+			  const struct adv7511_cfg_read_infoframe *cri,
+			  u8 *buffer)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct device *dev = &client->dev;
-	union hdmi_infoframe frame;
-	u8 buffer[32];
 	u8 len;
 	int i;
 
 	if (!(adv7511_rd(sd, cri->present_reg) & cri->present_mask)) {
 		v4l2_info(sd, "%s infoframe not transmitted\n", cri->desc);
-		return;
+		return 0;
 	}
 
 	memcpy(buffer, cri->header, sizeof(cri->header));
 
 	len = buffer[2];
 
-	if (len + 4 > sizeof(buffer)) {
+	if (len + 4 > V4L2_DEBUGFS_IF_MAX_LEN) {
 		v4l2_err(sd, "%s: invalid %s infoframe length %d\n", __func__, cri->desc, len);
-		return;
+		return 0;
 	}
 
 	if (cri->payload_addr >= 0x100) {
@@ -516,21 +517,38 @@ static void log_infoframe(struct v4l2_subdev *sd, const struct adv7511_cfg_read_
 	buffer[3] = 0;
 	buffer[3] = hdmi_infoframe_checksum(buffer, len + 4);
 
-	if (hdmi_infoframe_unpack(&frame, buffer, len + 4) < 0) {
-		v4l2_err(sd, "%s: unpack of %s infoframe failed\n", __func__, cri->desc);
+	return len + 4;
+}
+
+static void log_infoframe(struct v4l2_subdev *sd,
+			  const struct adv7511_cfg_read_infoframe *cri)
+{
+	union hdmi_infoframe frame;
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct device *dev = &client->dev;
+	u8 buffer[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+	int len = read_infoframe(sd, cri, buffer);
+
+	if (len <= 0)
+		return;
+
+	if (hdmi_infoframe_unpack(&frame, buffer, len) < 0) {
+		v4l2_err(sd, "%s: unpack of %s infoframe failed\n",
+			 __func__, cri->desc);
 		return;
 	}
 
 	hdmi_infoframe_log(KERN_INFO, dev, &frame);
 }
 
+static const struct adv7511_cfg_read_infoframe cri[] = {
+	{ "AVI", 0x44, 0x10, { 0x82, 2, 13 }, 0x55 },
+	{ "Audio", 0x44, 0x08, { 0x84, 1, 10 }, 0x73 },
+	{ "SDP", 0x40, 0x40, { 0x83, 1, 25 }, 0x103 },
+};
+
 static void adv7511_log_infoframes(struct v4l2_subdev *sd)
 {
-	static const struct adv7511_cfg_read_infoframe cri[] = {
-		{ "AVI", 0x44, 0x10, { 0x82, 2, 13 }, 0x55 },
-		{ "Audio", 0x44, 0x08, { 0x84, 1, 10 }, 0x73 },
-		{ "SDP", 0x40, 0x40, { 0x83, 1, 25 }, 0x103 },
-	};
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(cri); i++)
@@ -1693,6 +1711,34 @@ static bool adv7511_check_edid_status(struct v4l2_subdev *sd)
 	return false;
 }
 
+static ssize_t
+adv7511_debugfs_if_read(u32 type, void *priv,
+			struct file *filp, char __user *ubuf, size_t count, loff_t *ppos)
+{
+	u8 buf[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+	struct v4l2_subdev *sd = priv;
+	int index;
+	int len;
+
+	switch (type) {
+	case V4L2_DEBUGFS_IF_AVI:
+		index = 0;
+		break;
+	case V4L2_DEBUGFS_IF_AUDIO:
+		index = 1;
+		break;
+	case V4L2_DEBUGFS_IF_SPD:
+		index = 2;
+		break;
+	default:
+		return 0;
+	}
+	len = read_infoframe(sd, &cri[index], buf);
+	if (len > 0)
+		len = simple_read_from_buffer(ubuf, count, ppos, buf, len);
+	return len < 0 ? 0 : len;
+}
+
 static int adv7511_registered(struct v4l2_subdev *sd)
 {
 	struct adv7511_state *state = get_adv7511_state(sd);
@@ -1700,9 +1746,16 @@ static int adv7511_registered(struct v4l2_subdev *sd)
 	int err;
 
 	err = cec_register_adapter(state->cec_adap, &client->dev);
-	if (err)
+	if (err) {
 		cec_delete_adapter(state->cec_adap);
-	return err;
+		return err;
+	}
+
+	state->debugfs_dir = debugfs_create_dir(sd->name, v4l2_debugfs_root());
+	state->infoframes = v4l2_debugfs_if_alloc(state->debugfs_dir,
+		V4L2_DEBUGFS_IF_AVI | V4L2_DEBUGFS_IF_AUDIO |
+		V4L2_DEBUGFS_IF_SPD, sd, adv7511_debugfs_if_read);
+	return 0;
 }
 
 static void adv7511_unregistered(struct v4l2_subdev *sd)
@@ -1710,6 +1763,10 @@ static void adv7511_unregistered(struct v4l2_subdev *sd)
 	struct adv7511_state *state = get_adv7511_state(sd);
 
 	cec_unregister_adapter(state->cec_adap);
+	v4l2_debugfs_if_free(state->infoframes);
+	state->infoframes = NULL;
+	debugfs_remove_recursive(state->debugfs_dir);
+	state->debugfs_dir = NULL;
 }
 
 static const struct v4l2_subdev_internal_ops adv7511_int_ops = {
-- 
2.43.0

[PATCH 4/7] media: i2c: adv7604: export InfoFrames to debugfs

[Hans Verkuil]

Export InfoFrames to debugfs.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/i2c/adv7604.c | 90 ++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 48230d5109f0..3184a2fa1532 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -193,6 +193,9 @@ struct adv76xx_state {
 	struct delayed_work delayed_work_enable_hotplug;
 	bool restart_stdi_once;
 
+	struct dentry *debugfs_dir;
+	struct v4l2_debugfs_if *infoframes;
+
 	/* CEC */
 	struct cec_adapter *cec_adap;
 	u8   cec_addr[ADV76XX_MAX_ADDRS];
@@ -2458,10 +2461,9 @@ static const struct adv76xx_cfg_read_infoframe adv76xx_cri[] = {
 	{ "Vendor", 0x10, 0xec, 0x54 }
 };
 
-static int adv76xx_read_infoframe(struct v4l2_subdev *sd, int index,
-				  union hdmi_infoframe *frame)
+static int adv76xx_read_infoframe_buf(struct v4l2_subdev *sd, int index,
+				      u8 buf[V4L2_DEBUGFS_IF_MAX_LEN])
 {
-	uint8_t buffer[32];
 	u8 len;
 	int i;
 
@@ -2472,27 +2474,20 @@ static int adv76xx_read_infoframe(struct v4l2_subdev *sd, int index,
 	}
 
 	for (i = 0; i < 3; i++)
-		buffer[i] = infoframe_read(sd,
-					   adv76xx_cri[index].head_addr + i);
+		buf[i] = infoframe_read(sd, adv76xx_cri[index].head_addr + i);
 
-	len = buffer[2] + 1;
+	len = buf[2] + 1;
 
-	if (len + 3 > sizeof(buffer)) {
+	if (len + 3 > V4L2_DEBUGFS_IF_MAX_LEN) {
 		v4l2_err(sd, "%s: invalid %s infoframe length %d\n", __func__,
 			 adv76xx_cri[index].desc, len);
 		return -ENOENT;
 	}
 
 	for (i = 0; i < len; i++)
-		buffer[i + 3] = infoframe_read(sd,
-				       adv76xx_cri[index].payload_addr + i);
-
-	if (hdmi_infoframe_unpack(frame, buffer, len + 3) < 0) {
-		v4l2_err(sd, "%s: unpack of %s infoframe failed\n", __func__,
-			 adv76xx_cri[index].desc);
-		return -ENOENT;
-	}
-	return 0;
+		buf[i + 3] = infoframe_read(sd,
+					    adv76xx_cri[index].payload_addr + i);
+	return len + 3;
 }
 
 static void adv76xx_log_infoframes(struct v4l2_subdev *sd)
@@ -2505,10 +2500,19 @@ static void adv76xx_log_infoframes(struct v4l2_subdev *sd)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(adv76xx_cri); i++) {
-		union hdmi_infoframe frame;
 		struct i2c_client *client = v4l2_get_subdevdata(sd);
+		u8 buffer[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+		union hdmi_infoframe frame;
+		int len;
 
-		if (!adv76xx_read_infoframe(sd, i, &frame))
+		len = adv76xx_read_infoframe_buf(sd, i, buffer);
+		if (len < 0)
+			continue;
+
+		if (hdmi_infoframe_unpack(&frame, buffer, len) < 0)
+			v4l2_err(sd, "%s: unpack of %s infoframe failed\n",
+				 __func__, adv76xx_cri[i].desc);
+		else
 			hdmi_infoframe_log(KERN_INFO, &client->dev, &frame);
 	}
 }
@@ -2686,6 +2690,41 @@ static int adv76xx_subscribe_event(struct v4l2_subdev *sd,
 	}
 }
 
+static ssize_t
+adv76xx_debugfs_if_read(u32 type, void *priv, struct file *filp,
+			char __user *ubuf, size_t count, loff_t *ppos)
+{
+	u8 buf[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+	struct v4l2_subdev *sd = priv;
+	int index;
+	int len;
+
+	if (!is_hdmi(sd))
+		return 0;
+
+	switch (type) {
+	case V4L2_DEBUGFS_IF_AVI:
+		index = 0;
+		break;
+	case V4L2_DEBUGFS_IF_AUDIO:
+		index = 1;
+		break;
+	case V4L2_DEBUGFS_IF_SPD:
+		index = 2;
+		break;
+	case V4L2_DEBUGFS_IF_HDMI:
+		index = 3;
+		break;
+	default:
+		return 0;
+	}
+
+	len = adv76xx_read_infoframe_buf(sd, index, buf);
+	if (len > 0)
+		len = simple_read_from_buffer(ubuf, count, ppos, buf, len);
+	return len < 0 ? 0 : len;
+}
+
 static int adv76xx_registered(struct v4l2_subdev *sd)
 {
 	struct adv76xx_state *state = to_state(sd);
@@ -2693,9 +2732,16 @@ static int adv76xx_registered(struct v4l2_subdev *sd)
 	int err;
 
 	err = cec_register_adapter(state->cec_adap, &client->dev);
-	if (err)
+	if (err) {
 		cec_delete_adapter(state->cec_adap);
-	return err;
+		return err;
+	}
+	state->debugfs_dir = debugfs_create_dir(sd->name, v4l2_debugfs_root());
+	state->infoframes = v4l2_debugfs_if_alloc(state->debugfs_dir,
+		V4L2_DEBUGFS_IF_AVI | V4L2_DEBUGFS_IF_AUDIO |
+		V4L2_DEBUGFS_IF_SPD | V4L2_DEBUGFS_IF_HDMI, sd,
+		adv76xx_debugfs_if_read);
+	return 0;
 }
 
 static void adv76xx_unregistered(struct v4l2_subdev *sd)
@@ -2703,6 +2749,10 @@ static void adv76xx_unregistered(struct v4l2_subdev *sd)
 	struct adv76xx_state *state = to_state(sd);
 
 	cec_unregister_adapter(state->cec_adap);
+	v4l2_debugfs_if_free(state->infoframes);
+	state->infoframes = NULL;
+	debugfs_remove_recursive(state->debugfs_dir);
+	state->debugfs_dir = NULL;
 }
 
 /* ----------------------------------------------------------------------- */
-- 
2.43.0

[PATCH 5/7] media: i2c: adv7842: export InfoFrames to debugfs

[Hans Verkuil]

Export InfoFrames to debugfs.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/i2c/adv7842.c | 120 ++++++++++++++++++++++++++----------
 1 file changed, 88 insertions(+), 32 deletions(-)

diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 014fc913225c..e445699da85b 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -114,6 +114,9 @@ struct adv7842_state {
 	bool restart_stdi_once;
 	bool hdmi_port_a;
 
+	struct dentry *debugfs_dir;
+	struct v4l2_debugfs_if *infoframes;
+
 	/* i2c clients */
 	struct i2c_client *i2c_sdp_io;
 	struct i2c_client *i2c_sdp;
@@ -2565,58 +2568,65 @@ struct adv7842_cfg_read_infoframe {
 	u8 payload_addr;
 };
 
-static void log_infoframe(struct v4l2_subdev *sd, const struct adv7842_cfg_read_infoframe *cri)
+static const struct adv7842_cfg_read_infoframe adv7842_cri[] = {
+	{ "AVI", 0x01, 0xe0, 0x00 },
+	{ "Audio", 0x02, 0xe3, 0x1c },
+	{ "SDP", 0x04, 0xe6, 0x2a },
+	{ "Vendor", 0x10, 0xec, 0x54 }
+};
+
+static int adv7842_read_infoframe_buf(struct v4l2_subdev *sd, int index,
+				      u8 buf[V4L2_DEBUGFS_IF_MAX_LEN])
 {
-	int i;
-	u8 buffer[32];
-	union hdmi_infoframe frame;
-	u8 len;
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	struct device *dev = &client->dev;
+	const struct adv7842_cfg_read_infoframe *cri = &adv7842_cri[index];
+	int len, i;
 
 	if (!(io_read(sd, 0x60) & cri->present_mask)) {
-		v4l2_info(sd, "%s infoframe not received\n", cri->desc);
-		return;
+		v4l2_dbg(1, debug, sd,
+			 "%s infoframe not received\n", cri->desc);
+		return -ENOENT;
 	}
 
 	for (i = 0; i < 3; i++)
-		buffer[i] = infoframe_read(sd, cri->head_addr + i);
+		buf[i] = infoframe_read(sd, cri->head_addr + i);
 
-	len = buffer[2] + 1;
+	len = buf[2] + 1;
 
-	if (len + 3 > sizeof(buffer)) {
-		v4l2_err(sd, "%s: invalid %s infoframe length %d\n", __func__, cri->desc, len);
-		return;
+	if (len + 3 > V4L2_DEBUGFS_IF_MAX_LEN) {
+		v4l2_err(sd, "%s: invalid %s infoframe length %d\n",
+			 __func__, cri->desc, len);
+		return -ENOENT;
 	}
 
 	for (i = 0; i < len; i++)
-		buffer[i + 3] = infoframe_read(sd, cri->payload_addr + i);
-
-	if (hdmi_infoframe_unpack(&frame, buffer, len + 3) < 0) {
-		v4l2_err(sd, "%s: unpack of %s infoframe failed\n", __func__, cri->desc);
-		return;
-	}
-
-	hdmi_infoframe_log(KERN_INFO, dev, &frame);
+		buf[i + 3] = infoframe_read(sd, cri->payload_addr + i);
+	return len + 3;
 }
 
 static void adv7842_log_infoframes(struct v4l2_subdev *sd)
 {
-	int i;
-	static const struct adv7842_cfg_read_infoframe cri[] = {
-		{ "AVI", 0x01, 0xe0, 0x00 },
-		{ "Audio", 0x02, 0xe3, 0x1c },
-		{ "SDP", 0x04, 0xe6, 0x2a },
-		{ "Vendor", 0x10, 0xec, 0x54 }
-	};
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct device *dev = &client->dev;
+	union hdmi_infoframe frame;
+	u8 buffer[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+	int len, i;
 
 	if (!(hdmi_read(sd, 0x05) & 0x80)) {
 		v4l2_info(sd, "receive DVI-D signal, no infoframes\n");
 		return;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(cri); i++)
-		log_infoframe(sd, &cri[i]);
+	for (i = 0; i < ARRAY_SIZE(adv7842_cri); i++) {
+		len = adv7842_read_infoframe_buf(sd, i, buffer);
+		if (len < 0)
+			continue;
+
+		if (hdmi_infoframe_unpack(&frame, buffer, len) < 0)
+			v4l2_err(sd, "%s: unpack of %s infoframe failed\n",
+				 __func__, adv7842_cri[i].desc);
+		else
+			hdmi_infoframe_log(KERN_INFO, dev, &frame);
+	}
 }
 
 #if 0
@@ -3263,6 +3273,41 @@ static int adv7842_subscribe_event(struct v4l2_subdev *sd,
 	}
 }
 
+static ssize_t
+adv7842_debugfs_if_read(u32 type, void *priv, struct file *filp,
+			char __user *ubuf, size_t count, loff_t *ppos)
+{
+	u8 buf[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+	struct v4l2_subdev *sd = priv;
+	int index;
+	int len;
+
+	if (!is_hdmi(sd))
+		return 0;
+
+	switch (type) {
+	case V4L2_DEBUGFS_IF_AVI:
+		index = 0;
+		break;
+	case V4L2_DEBUGFS_IF_AUDIO:
+		index = 1;
+		break;
+	case V4L2_DEBUGFS_IF_SPD:
+		index = 2;
+		break;
+	case V4L2_DEBUGFS_IF_HDMI:
+		index = 3;
+		break;
+	default:
+		return 0;
+	}
+
+	len = adv7842_read_infoframe_buf(sd, index, buf);
+	if (len > 0)
+		len = simple_read_from_buffer(ubuf, count, ppos, buf, len);
+	return len < 0 ? 0 : len;
+}
+
 static int adv7842_registered(struct v4l2_subdev *sd)
 {
 	struct adv7842_state *state = to_state(sd);
@@ -3270,8 +3315,15 @@ static int adv7842_registered(struct v4l2_subdev *sd)
 	int err;
 
 	err = cec_register_adapter(state->cec_adap, &client->dev);
-	if (err)
+	if (err) {
 		cec_delete_adapter(state->cec_adap);
+	} else {
+		state->debugfs_dir = debugfs_create_dir(sd->name, v4l2_debugfs_root());
+		state->infoframes = v4l2_debugfs_if_alloc(state->debugfs_dir,
+			V4L2_DEBUGFS_IF_AVI | V4L2_DEBUGFS_IF_AUDIO |
+			V4L2_DEBUGFS_IF_SPD | V4L2_DEBUGFS_IF_HDMI, sd,
+			adv7842_debugfs_if_read);
+	}
 	return err;
 }
 
@@ -3280,6 +3332,10 @@ static void adv7842_unregistered(struct v4l2_subdev *sd)
 	struct adv7842_state *state = to_state(sd);
 
 	cec_unregister_adapter(state->cec_adap);
+	v4l2_debugfs_if_free(state->infoframes);
+	state->infoframes = NULL;
+	debugfs_remove_recursive(state->debugfs_dir);
+	state->debugfs_dir = NULL;
 }
 
 /* ----------------------------------------------------------------------- */
-- 
2.43.0

[PATCH 6/7] media: i2c: tc358743: export InfoFrames to debugfs

[Hans Verkuil]

Export InfoFrames to debugfs.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/tc358743.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 65d58ddf0287..fd49bf824051 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -87,6 +87,10 @@ struct tc358743_state {
 	struct timer_list timer;
 	struct work_struct work_i2c_poll;
 
+	/* debugfs */
+	struct dentry *debugfs_dir;
+	struct v4l2_debugfs_if *infoframes;
+
 	/* edid  */
 	u8 edid_blocks_written;
 
@@ -430,12 +434,35 @@ static void tc358743_erase_bksv(struct v4l2_subdev *sd)
 
 /* --------------- AVI infoframe --------------- */
 
+static ssize_t
+tc358743_debugfs_if_read(u32 type, void *priv, struct file *filp,
+			 char __user *ubuf, size_t count, loff_t *ppos)
+{
+	u8 buf[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+	struct v4l2_subdev *sd = priv;
+	int len;
+
+	if (!is_hdmi(sd))
+		return 0;
+
+	if (type != V4L2_DEBUGFS_IF_AVI)
+		return 0;
+
+	i2c_rd(sd, PK_AVI_0HEAD, buf, PK_AVI_16BYTE - PK_AVI_0HEAD + 1);
+	len = buf[2] + 4;
+	if (len > V4L2_DEBUGFS_IF_MAX_LEN)
+		len = -ENOENT;
+	if (len > 0)
+		len = simple_read_from_buffer(ubuf, count, ppos, buf, len);
+	return len < 0 ? 0 : len;
+}
+
 static void print_avi_infoframe(struct v4l2_subdev *sd)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct device *dev = &client->dev;
 	union hdmi_infoframe frame;
-	u8 buffer[HDMI_INFOFRAME_SIZE(AVI)];
+	u8 buffer[HDMI_INFOFRAME_SIZE(AVI)] = {};
 
 	if (!is_hdmi(sd)) {
 		v4l2_info(sd, "DVI-D signal - AVI infoframe not supported\n");
@@ -2161,6 +2188,11 @@ static int tc358743_probe(struct i2c_client *client)
 	if (err < 0)
 		goto err_work_queues;
 
+	state->debugfs_dir = debugfs_create_dir(sd->name, v4l2_debugfs_root());
+	state->infoframes = v4l2_debugfs_if_alloc(state->debugfs_dir,
+						  V4L2_DEBUGFS_IF_AVI, sd,
+						  tc358743_debugfs_if_read);
+
 	v4l2_info(sd, "%s found @ 0x%x (%s)\n", client->name,
 		  client->addr << 1, client->adapter->name);
 
@@ -2188,6 +2220,8 @@ static void tc358743_remove(struct i2c_client *client)
 		flush_work(&state->work_i2c_poll);
 	}
 	cancel_delayed_work_sync(&state->delayed_work_enable_hotplug);
+	v4l2_debugfs_if_free(state->infoframes);
+	debugfs_remove_recursive(state->debugfs_dir);
 	cec_unregister_adapter(state->cec_adap);
 	v4l2_async_unregister_subdev(sd);
 	v4l2_device_unregister_subdev(sd);
-- 
2.43.0

[PATCH 7/7] media: i2c: tda1997x: export InfoFrames to debugfs

[Hans Verkuil]

Export InfoFrames to debugfs.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/i2c/tda1997x.c | 50 ++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c
index 3b7e5ff5b010..2b33fdecb2d2 100644
--- a/drivers/media/i2c/tda1997x.c
+++ b/drivers/media/i2c/tda1997x.c
@@ -259,6 +259,10 @@ struct tda1997x_state {
 	struct v4l2_ctrl *detect_tx_5v_ctrl;
 	struct v4l2_ctrl *rgb_quantization_range_ctrl;
 
+	/* debugfs */
+	struct dentry *debugfs_dir;
+	struct v4l2_debugfs_if *infoframes;
+
 	/* audio */
 	u8  audio_ch_alloc;
 	int audio_samplerate;
@@ -1263,7 +1267,7 @@ tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr)
 {
 	struct v4l2_subdev *sd = &state->sd;
 	union hdmi_infoframe frame;
-	u8 buffer[40] = { 0 };
+	u8 buffer[V4L2_DEBUGFS_IF_MAX_LEN] = { 0 };
 	u8 reg;
 	int len, err;
 
@@ -1938,11 +1942,44 @@ static const struct v4l2_subdev_pad_ops tda1997x_pad_ops = {
  * v4l2_subdev_core_ops
  */
 
+static ssize_t
+tda1997x_debugfs_if_read(u32 type, void *priv, struct file *filp, char __user *ubuf, size_t count, loff_t *ppos)
+{
+	struct v4l2_subdev *sd = priv;
+	u8 buffer[V4L2_DEBUGFS_IF_MAX_LEN] = {};
+	int addr, len;
+
+	switch (type) {
+	case V4L2_DEBUGFS_IF_AVI:
+		addr = AVI_IF;
+		break;
+	case V4L2_DEBUGFS_IF_AUDIO:
+		addr = AUD_IF;
+		break;
+	case V4L2_DEBUGFS_IF_SPD:
+		addr = SPD_IF;
+		break;
+	default:
+		return 0;
+	}
+
+	/* read data */
+	len = io_readn(sd, addr, sizeof(buffer), buffer);
+	if (len > 0) {
+		len = buffer[2] + 4;
+		if (len > V4L2_DEBUGFS_IF_MAX_LEN)
+			len = -EIO;
+	}
+	if (len > 0)
+		len = simple_read_from_buffer(ubuf, count, ppos, buffer, len);
+	return len < 0 ? 0 : len;
+}
+
 static int tda1997x_log_infoframe(struct v4l2_subdev *sd, int addr)
 {
 	struct tda1997x_state *state = to_state(sd);
 	union hdmi_infoframe frame;
-	u8 buffer[40] = { 0 };
+	u8 buffer[V4L2_DEBUGFS_IF_MAX_LEN] = {};
 	int len, err;
 
 	/* read data */
@@ -2791,6 +2828,12 @@ static int tda1997x_probe(struct i2c_client *client)
 		goto err_free_media;
 	}
 
+	state->debugfs_dir = debugfs_create_dir(sd->name, v4l2_debugfs_root());
+	state->infoframes = v4l2_debugfs_if_alloc(state->debugfs_dir,
+		V4L2_DEBUGFS_IF_AVI | V4L2_DEBUGFS_IF_AUDIO |
+		V4L2_DEBUGFS_IF_SPD, sd,
+		tda1997x_debugfs_if_read);
+
 	return 0;
 
 err_free_media:
@@ -2815,6 +2858,9 @@ static void tda1997x_remove(struct i2c_client *client)
 	struct tda1997x_state *state = to_state(sd);
 	struct tda1997x_platform_data *pdata = &state->pdata;
 
+	v4l2_debugfs_if_free(state->infoframes);
+	debugfs_remove_recursive(state->debugfs_dir);
+
 	if (pdata->audout_format) {
 		mutex_destroy(&state->audio_lock);
 	}
-- 
2.43.0

Re: [PATCH 2/7] media: v4l2-core: add v4l2_debugfs_if_alloc/free()

[Jani Nikula]

On Wed, 28 Aug 2024, Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> Add new helpers to export received or transmitted HDMI InfoFrames to
> debugfs.
>
> This complements similar code in drm where the transmitted HDMI infoframes
> are exported to debugfs.
>
> The same names have been used as in drm, so this is consistent.
>
> The exported infoframes can be parsed with the edid-decode utility.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/v4l2-core/v4l2-dv-timings.c | 63 +++++++++++++++++++++++
>  include/media/v4l2-dv-timings.h           | 48 +++++++++++++++++
>  2 files changed, 111 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c b/drivers/media/v4l2-core/v4l2-dv-timings.c
> index 942d0005c55e..86a8627f4bcc 100644
> --- a/drivers/media/v4l2-core/v4l2-dv-timings.c
> +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
> @@ -1154,3 +1154,66 @@ int v4l2_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_phys_addr_validate);
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +#define DEBUGFS_FOPS(type, flag)					\
> +static ssize_t								\
> +infoframe_read_##type(struct file *filp,				\
> +		      char __user *ubuf, size_t count, loff_t *ppos)	\
> +{									\
> +	struct v4l2_debugfs_if *infoframes = filp->private_data;	\
> +									\
> +	return infoframes->if_read((flag), infoframes->priv, filp,	\
> +				   ubuf, count, ppos);			\
> +}									\
> +									\
> +static const struct file_operations infoframe_##type##_fops = {		\
> +	.owner   = THIS_MODULE,						\
> +	.open    = simple_open,						\
> +	.read    = infoframe_read_##type,				\
> +}
> +
> +DEBUGFS_FOPS(avi, V4L2_DEBUGFS_IF_AVI);
> +DEBUGFS_FOPS(audio, V4L2_DEBUGFS_IF_AUDIO);
> +DEBUGFS_FOPS(spd, V4L2_DEBUGFS_IF_SPD);
> +DEBUGFS_FOPS(hdmi, V4L2_DEBUGFS_IF_HDMI);
> +
> +struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
> +					      void *priv,
> +					      v4l2_debugfs_if_read_t if_read)
> +{
> +	struct v4l2_debugfs_if *infoframes;
> +
> +	if (IS_ERR_OR_NULL(root) || !if_types || !if_read)
> +		return NULL;
> +
> +	infoframes = kzalloc(sizeof(*infoframes), GFP_KERNEL);
> +	if (!infoframes)
> +		return NULL;
> +
> +	infoframes->if_dir = debugfs_create_dir("infoframes", root);
> +	infoframes->priv = priv;
> +	infoframes->if_read = if_read;
> +	if (if_types & V4L2_DEBUGFS_IF_AVI)
> +		debugfs_create_file("avi", 0400, infoframes->if_dir, infoframes, &infoframe_avi_fops);
> +	if (if_types & V4L2_DEBUGFS_IF_AUDIO)
> +		debugfs_create_file("audio", 0400, infoframes->if_dir, infoframes, &infoframe_audio_fops);
> +	if (if_types & V4L2_DEBUGFS_IF_SPD)
> +		debugfs_create_file("spd", 0400, infoframes->if_dir, infoframes, &infoframe_spd_fops);
> +	if (if_types & V4L2_DEBUGFS_IF_HDMI)
> +		debugfs_create_file("hdmi", 0400, infoframes->if_dir, infoframes, &infoframe_hdmi_fops);
> +	return infoframes;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_debugfs_if_alloc);
> +
> +void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes)
> +{
> +	if (infoframes) {
> +		debugfs_remove_recursive(infoframes->if_dir);
> +		kfree(infoframes);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(v4l2_debugfs_if_free);
> +
> +#endif
> diff --git a/include/media/v4l2-dv-timings.h b/include/media/v4l2-dv-timings.h
> index 8fa963326bf6..13830411bd6c 100644
> --- a/include/media/v4l2-dv-timings.h
> +++ b/include/media/v4l2-dv-timings.h
> @@ -8,6 +8,7 @@
>  #ifndef __V4L2_DV_TIMINGS_H
>  #define __V4L2_DV_TIMINGS_H
>  
> +#include <linux/debugfs.h>

Please don't include headers from headers if you can get by with forward
declarations.

I recently discovered a lot of drm depending on getting seq_file.h and
debugfs.h via media/cec.h...

BR,
Jani.


>  #include <linux/videodev2.h>
>  
>  /**
> @@ -251,4 +252,51 @@ void v4l2_set_edid_phys_addr(u8 *edid, unsigned int size, u16 phys_addr);
>  u16 v4l2_phys_addr_for_input(u16 phys_addr, u8 input);
>  int v4l2_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port);
>  
> +/* Add support for exporting InfoFrames to debugfs */
> +
> +/*
> + * HDMI InfoFrames start with a 3 byte header, then a checksum,
> + * followed by the actual IF payload.
> + *
> + * The payload length is limited to 30 bytes according to the HDMI spec,
> + * but since the length is encoded in 5 bits, it can be 31 bytes theoretically.
> + * So set the max length as 31 + 3 (header) + 1 (checksum) = 35.
> + */
> +#define V4L2_DEBUGFS_IF_MAX_LEN (35)
> +
> +#define V4L2_DEBUGFS_IF_AVI	BIT(0)
> +#define V4L2_DEBUGFS_IF_AUDIO	BIT(1)
> +#define V4L2_DEBUGFS_IF_SPD	BIT(2)
> +#define V4L2_DEBUGFS_IF_HDMI	BIT(3)
> +
> +typedef ssize_t (*v4l2_debugfs_if_read_t)(u32 type, void *priv,
> +					  struct file *filp, char __user *ubuf,
> +					  size_t count, loff_t *ppos);
> +
> +struct v4l2_debugfs_if {
> +	struct dentry *if_dir;
> +	void *priv;
> +
> +	v4l2_debugfs_if_read_t if_read;
> +};
> +
> +#ifdef CONFIG_DEBUG_FS
> +struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
> +					      void *priv,
> +					      v4l2_debugfs_if_read_t if_read);
> +void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes);
> +#else
> +static inline
> +struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
> +					      void *priv,
> +					      v4l2_debugfs_if_read_t if_read)
> +{
> +	return NULL;
> +}
> +
> +static inline void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes)
> +{
> +}
> +#endif
> +
>  #endif

-- 
Jani Nikula, Intel

Re: [PATCH 2/7] media: v4l2-core: add v4l2_debugfs_if_alloc/free()

[Hans Verkuil]

On 28/08/2024 17:08, Jani Nikula wrote:
> On Wed, 28 Aug 2024, Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>> Add new helpers to export received or transmitted HDMI InfoFrames to
>> debugfs.
>>
>> This complements similar code in drm where the transmitted HDMI infoframes
>> are exported to debugfs.
>>
>> The same names have been used as in drm, so this is consistent.
>>
>> The exported infoframes can be parsed with the edid-decode utility.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  drivers/media/v4l2-core/v4l2-dv-timings.c | 63 +++++++++++++++++++++++
>>  include/media/v4l2-dv-timings.h           | 48 +++++++++++++++++
>>  2 files changed, 111 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c b/drivers/media/v4l2-core/v4l2-dv-timings.c
>> index 942d0005c55e..86a8627f4bcc 100644
>> --- a/drivers/media/v4l2-core/v4l2-dv-timings.c
>> +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
>> @@ -1154,3 +1154,66 @@ int v4l2_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port)
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL_GPL(v4l2_phys_addr_validate);
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +
>> +#define DEBUGFS_FOPS(type, flag)					\
>> +static ssize_t								\
>> +infoframe_read_##type(struct file *filp,				\
>> +		      char __user *ubuf, size_t count, loff_t *ppos)	\
>> +{									\
>> +	struct v4l2_debugfs_if *infoframes = filp->private_data;	\
>> +									\
>> +	return infoframes->if_read((flag), infoframes->priv, filp,	\
>> +				   ubuf, count, ppos);			\
>> +}									\
>> +									\
>> +static const struct file_operations infoframe_##type##_fops = {		\
>> +	.owner   = THIS_MODULE,						\
>> +	.open    = simple_open,						\
>> +	.read    = infoframe_read_##type,				\
>> +}
>> +
>> +DEBUGFS_FOPS(avi, V4L2_DEBUGFS_IF_AVI);
>> +DEBUGFS_FOPS(audio, V4L2_DEBUGFS_IF_AUDIO);
>> +DEBUGFS_FOPS(spd, V4L2_DEBUGFS_IF_SPD);
>> +DEBUGFS_FOPS(hdmi, V4L2_DEBUGFS_IF_HDMI);
>> +
>> +struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
>> +					      void *priv,
>> +					      v4l2_debugfs_if_read_t if_read)
>> +{
>> +	struct v4l2_debugfs_if *infoframes;
>> +
>> +	if (IS_ERR_OR_NULL(root) || !if_types || !if_read)
>> +		return NULL;
>> +
>> +	infoframes = kzalloc(sizeof(*infoframes), GFP_KERNEL);
>> +	if (!infoframes)
>> +		return NULL;
>> +
>> +	infoframes->if_dir = debugfs_create_dir("infoframes", root);
>> +	infoframes->priv = priv;
>> +	infoframes->if_read = if_read;
>> +	if (if_types & V4L2_DEBUGFS_IF_AVI)
>> +		debugfs_create_file("avi", 0400, infoframes->if_dir, infoframes, &infoframe_avi_fops);
>> +	if (if_types & V4L2_DEBUGFS_IF_AUDIO)
>> +		debugfs_create_file("audio", 0400, infoframes->if_dir, infoframes, &infoframe_audio_fops);
>> +	if (if_types & V4L2_DEBUGFS_IF_SPD)
>> +		debugfs_create_file("spd", 0400, infoframes->if_dir, infoframes, &infoframe_spd_fops);
>> +	if (if_types & V4L2_DEBUGFS_IF_HDMI)
>> +		debugfs_create_file("hdmi", 0400, infoframes->if_dir, infoframes, &infoframe_hdmi_fops);
>> +	return infoframes;
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_debugfs_if_alloc);
>> +
>> +void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes)
>> +{
>> +	if (infoframes) {
>> +		debugfs_remove_recursive(infoframes->if_dir);
>> +		kfree(infoframes);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_debugfs_if_free);
>> +
>> +#endif
>> diff --git a/include/media/v4l2-dv-timings.h b/include/media/v4l2-dv-timings.h
>> index 8fa963326bf6..13830411bd6c 100644
>> --- a/include/media/v4l2-dv-timings.h
>> +++ b/include/media/v4l2-dv-timings.h
>> @@ -8,6 +8,7 @@
>>  #ifndef __V4L2_DV_TIMINGS_H
>>  #define __V4L2_DV_TIMINGS_H
>>  
>> +#include <linux/debugfs.h>
> 
> Please don't include headers from headers if you can get by with forward
> declarations.

Done.

> 
> I recently discovered a lot of drm depending on getting seq_file.h and
> debugfs.h via media/cec.h...

Patches are welcome!

Regards,

	Hans

> 
> BR,
> Jani.
> 
> 
>>  #include <linux/videodev2.h>
>>  
>>  /**
>> @@ -251,4 +252,51 @@ void v4l2_set_edid_phys_addr(u8 *edid, unsigned int size, u16 phys_addr);
>>  u16 v4l2_phys_addr_for_input(u16 phys_addr, u8 input);
>>  int v4l2_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port);
>>  
>> +/* Add support for exporting InfoFrames to debugfs */
>> +
>> +/*
>> + * HDMI InfoFrames start with a 3 byte header, then a checksum,
>> + * followed by the actual IF payload.
>> + *
>> + * The payload length is limited to 30 bytes according to the HDMI spec,
>> + * but since the length is encoded in 5 bits, it can be 31 bytes theoretically.
>> + * So set the max length as 31 + 3 (header) + 1 (checksum) = 35.
>> + */
>> +#define V4L2_DEBUGFS_IF_MAX_LEN (35)
>> +
>> +#define V4L2_DEBUGFS_IF_AVI	BIT(0)
>> +#define V4L2_DEBUGFS_IF_AUDIO	BIT(1)
>> +#define V4L2_DEBUGFS_IF_SPD	BIT(2)
>> +#define V4L2_DEBUGFS_IF_HDMI	BIT(3)
>> +
>> +typedef ssize_t (*v4l2_debugfs_if_read_t)(u32 type, void *priv,
>> +					  struct file *filp, char __user *ubuf,
>> +					  size_t count, loff_t *ppos);
>> +
>> +struct v4l2_debugfs_if {
>> +	struct dentry *if_dir;
>> +	void *priv;
>> +
>> +	v4l2_debugfs_if_read_t if_read;
>> +};
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
>> +					      void *priv,
>> +					      v4l2_debugfs_if_read_t if_read);
>> +void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes);
>> +#else
>> +static inline
>> +struct v4l2_debugfs_if *v4l2_debugfs_if_alloc(struct dentry *root, u32 if_types,
>> +					      void *priv,
>> +					      v4l2_debugfs_if_read_t if_read)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static inline void v4l2_debugfs_if_free(struct v4l2_debugfs_if *infoframes)
>> +{
>> +}
>> +#endif
>> +
>>  #endif
> 

Re: [PATCH 2/7] media: v4l2-core: add v4l2_debugfs_if_alloc/free()

[Jani Nikula]

On Wed, 28 Aug 2024, Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> On 28/08/2024 17:08, Jani Nikula wrote:
>> I recently discovered a lot of drm depending on getting seq_file.h and
>> debugfs.h via media/cec.h...
>
> Patches are welcome!

I've got the patch, just need to get these [1] reviewed and merged
first...

BR,
Jani.


[1] https://lore.kernel.org/r/cover.1724689818.git.jani.nikula@intel.com


-- 
Jani Nikula, Intel