All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability
@ 2022-08-03  7:58 Erling Ljunggren
  2022-08-03  7:58 ` [PATCH v2 1/5] media: videodev2.h: add V4L2_CAP_EDID_MEMORY Erling Ljunggren
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Erling Ljunggren @ 2022-08-03  7:58 UTC (permalink / raw)
  To: linux-media; +Cc: Erling Ljunggren

This series adds support for the standalone cat24c208 EDID EEPROM i2c device.
Usually EDID support is part of an HDMI receiver, but this is a standalone EEPROM.

Note that EEPROMs for EDIDs are not regular EEPROM devices, these are dual port
devices that follow the VESA E-DDC standard.

Since this is a standalone device that does not capture any video a new
V4L2_CAP_EDID_MEMORY capability is introduced to represent such devices.
Note that such a device doesn't have to be an EEPROM, it can also be
implemented using a microcontroller, for example. Hence the use of the generic
word 'MEMORY'.

The new capability uses the free bit 0x00000008. But we are running out of
capability bits: only 0x40000000 and 0x00000008 are free at the moment.

There is one other capability V4L2_CAP_ASYNCIO (0x02000000) that is not used
at all, it was never implemented. Wouldn't it be better to define
V4L2_CAP_ASYNCIO to 0, mark it as obsolete, and instead reuse it for this
V4L2_CAP_EDID_MEMORY capability?

v2:
 - fix dt binding example
 - rename i2c client variables in data struct
 - fix include: of_device.h -> mod_devicetable.h
 - Sorted makefile
 - used define EDID_OFFSET_EXT_FLAG instead of magic number
 - removed of_match_ptr
 - added bus_info
 - remove unneeded headers
 - add depends on OF to Kconfig

Erling Ljunggren (4):
  media: videodev2.h: add V4L2_CAP_EDID_MEMORY
  media: docs: Add V4L2_CAP_EDID_MEMORY
  dt-bindings: media: add cat24c208 bindings
  media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY

Jonathan Selnes (1):
  media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM

 .../bindings/media/i2c/onnn,cat24c208.yaml    |  40 ++
 .../userspace-api/media/v4l/biblio.rst        |  11 +
 .../media/v4l/vidioc-querycap.rst             |   7 +
 .../media/videodev2.h.rst.exceptions          |   1 +
 MAINTAINERS                                   |   7 +
 drivers/media/i2c/Kconfig                     |   9 +
 drivers/media/i2c/Makefile                    |   1 +
 drivers/media/i2c/cat24c208.c                 | 421 ++++++++++++++++++
 drivers/media/v4l2-core/v4l2-dev.c            |   8 +
 include/uapi/linux/videodev2.h                |   1 +
 10 files changed, 506 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
 create mode 100644 drivers/media/i2c/cat24c208.c

-- 
2.37.1


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

* [PATCH v2 1/5] media: videodev2.h: add V4L2_CAP_EDID_MEMORY
  2022-08-03  7:58 [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Erling Ljunggren
@ 2022-08-03  7:58 ` Erling Ljunggren
  2022-08-03  7:58 ` [PATCH v2 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY Erling Ljunggren
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Erling Ljunggren @ 2022-08-03  7:58 UTC (permalink / raw)
  To: linux-media; +Cc: Erling Ljunggren

Add capability flag to indicate that the device is an EDID memory device.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 include/uapi/linux/videodev2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 343b95107fce..1837e89b350d 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -451,6 +451,7 @@ struct v4l2_capability {
 #define V4L2_CAP_VIDEO_CAPTURE		0x00000001  /* Is a video capture device */
 #define V4L2_CAP_VIDEO_OUTPUT		0x00000002  /* Is a video output device */
 #define V4L2_CAP_VIDEO_OVERLAY		0x00000004  /* Can do video overlay */
+#define V4L2_CAP_EDID_MEMORY		0x00000008  /* Is an EDID memory only device */
 #define V4L2_CAP_VBI_CAPTURE		0x00000010  /* Is a raw VBI capture device */
 #define V4L2_CAP_VBI_OUTPUT		0x00000020  /* Is a raw VBI output device */
 #define V4L2_CAP_SLICED_VBI_CAPTURE	0x00000040  /* Is a sliced VBI capture device */
-- 
2.37.1


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

* [PATCH v2 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY
  2022-08-03  7:58 [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Erling Ljunggren
  2022-08-03  7:58 ` [PATCH v2 1/5] media: videodev2.h: add V4L2_CAP_EDID_MEMORY Erling Ljunggren
@ 2022-08-03  7:58 ` Erling Ljunggren
  2022-08-18  7:41   ` Hans Verkuil
  2022-08-03  7:58 ` [PATCH v2 3/5] dt-bindings: media: add cat24c208 bindings Erling Ljunggren
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Erling Ljunggren @ 2022-08-03  7:58 UTC (permalink / raw)
  To: linux-media; +Cc: Erling Ljunggren

Add documentation for the new edid eeprom capability.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 Documentation/userspace-api/media/v4l/biblio.rst      | 11 +++++++++++
 .../userspace-api/media/v4l/vidioc-querycap.rst       |  7 +++++++
 .../userspace-api/media/videodev2.h.rst.exceptions    |  1 +
 3 files changed, 19 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
index 9cd18c153d19..5cbe41877a63 100644
--- a/Documentation/userspace-api/media/v4l/biblio.rst
+++ b/Documentation/userspace-api/media/v4l/biblio.rst
@@ -334,6 +334,17 @@ VESA DMT
 
 :author:    Video Electronics Standards Association (http://www.vesa.org)
 
+.. _vesaeddc:
+
+E-DDC
+====
+
+
+:title:     VESA Enhanced Display Data Channel (E-DDC) Standard
+:subtitle:  Version 1.3
+
+:author:    Video Electronics Standards Association (http://www.vesa.org)
+
 .. _vesaedid:
 
 EDID
diff --git a/Documentation/userspace-api/media/v4l/vidioc-querycap.rst b/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
index 63e23f6f95ee..bdb530bd6816 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
@@ -173,6 +173,13 @@ specification the ioctl returns an ``EINVAL`` error code.
 	interface. A video overlay device typically stores captured images
 	directly in the video memory of a graphics card, with hardware
 	clipping and scaling.
+    * - ``V4L2_CAP_EDID_MEMORY``
+      - 0x00000008
+      - The device is a standalone EDID memory device. This is typically an eeprom
+        that supports the VESA Enhanced Display Data Channel Standard.
+
+        While an eeprom is the most common implementation, it can be something else
+        as well, such as a microcontroller. Hence the generic name 'memory'.
     * - ``V4L2_CAP_VBI_CAPTURE``
       - 0x00000010
       - The device supports the :ref:`Raw VBI Capture <raw-vbi>`
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 9cbb7a0c354a..12fa290828e7 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -185,6 +185,7 @@ replace define V4L2_CAP_META_OUTPUT device-capabilities
 replace define V4L2_CAP_DEVICE_CAPS device-capabilities
 replace define V4L2_CAP_TOUCH device-capabilities
 replace define V4L2_CAP_IO_MC device-capabilities
+replace define V4L2_CAP_EDID_MEMORY device-capabilities
 
 # V4L2 pix flags
 replace define V4L2_PIX_FMT_PRIV_MAGIC :c:type:`v4l2_pix_format`
-- 
2.37.1


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

* [PATCH v2 3/5] dt-bindings: media: add cat24c208 bindings
  2022-08-03  7:58 [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Erling Ljunggren
  2022-08-03  7:58 ` [PATCH v2 1/5] media: videodev2.h: add V4L2_CAP_EDID_MEMORY Erling Ljunggren
  2022-08-03  7:58 ` [PATCH v2 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY Erling Ljunggren
@ 2022-08-03  7:58 ` Erling Ljunggren
  2022-08-03 23:41   ` Rob Herring
  2022-08-03  7:58 ` [PATCH v2 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM Erling Ljunggren
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Erling Ljunggren @ 2022-08-03  7:58 UTC (permalink / raw)
  To: linux-media; +Cc: Erling Ljunggren, devicetree

Add devicetree bindings for new cat24c208 EDID EEPROM driver.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 .../bindings/media/i2c/onnn,cat24c208.yaml    | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
new file mode 100644
index 000000000000..590cdf0cbdc1
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/onnn,cat24c208.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ON Semiconductor CAT24C208 EDID EEPROM driver
+
+maintainers:
+  - Hans Verkuil <hverkuil-cisco@xs4all.nl>
+
+description: |
+  CAT24C208 is a dual port i2c EEPROM designed for EDID storage.
+
+
+properties:
+  compatible:
+    const: onnn,cat24c208
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cat24c208@31 {
+            compatible = "onnn,cat24c208";
+            reg = <0x31>;
+        };
+    };
+...
-- 
2.37.1


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

* [PATCH v2 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM
  2022-08-03  7:58 [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Erling Ljunggren
                   ` (2 preceding siblings ...)
  2022-08-03  7:58 ` [PATCH v2 3/5] dt-bindings: media: add cat24c208 bindings Erling Ljunggren
@ 2022-08-03  7:58 ` Erling Ljunggren
  2022-08-03  7:58 ` [PATCH v2 5/5] media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY Erling Ljunggren
  2022-08-03  9:14 ` [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Laurent Pinchart
  5 siblings, 0 replies; 12+ messages in thread
From: Erling Ljunggren @ 2022-08-03  7:58 UTC (permalink / raw)
  To: linux-media; +Cc: Jonathan Selnes, Hans Verkuil, Erling Ljunggren

From: Jonathan Selnes <jonathansb1@gmail.com>

Support reading and writing the EDID EEPROM through the
v4l2 API.

Signed-off-by: Jonathan Selnes <jonathansb1@gmail.com>
Co-developed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 MAINTAINERS                   |   7 +
 drivers/media/i2c/Kconfig     |   9 +
 drivers/media/i2c/Makefile    |   1 +
 drivers/media/i2c/cat24c208.c | 421 ++++++++++++++++++++++++++++++++++
 4 files changed, 438 insertions(+)
 create mode 100644 drivers/media/i2c/cat24c208.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7d9490a5c15a..407d76c42bab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14912,6 +14912,13 @@ S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/i2c/ov9734.c
 
+ON SEMICONDUCTOR CAT24C208 EDID EEPROM DRIVER
+M:	Hans Verkuil <hverkuil-cisco@xs4all.nl>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
+F:	drivers/media/i2c/cat24c208*
+
 ONENAND FLASH DRIVER
 M:	Kyungmin Park <kyungmin.park@samsung.com>
 L:	linux-mtd@lists.infradead.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 2b20aa6c37b1..2f5f9f058b48 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1530,6 +1530,15 @@ endmenu
 menu "Miscellaneous helper chips"
 	visible if !MEDIA_HIDE_ANCILLARY_SUBDRV
 
+config VIDEO_CAT24C208
+	tristate "ON Semiconductor cat24c208 EDID EEPROM"
+	depends on VIDEO_DEV && I2C && OF
+	help
+	  Support for the ON Semiconductor CAT24C208 Dual Port EDID EEPROM.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cat24c208.
+
 config VIDEO_I2C
 	tristate "I2C transport video support"
 	depends on VIDEO_DEV && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 3e1696963e7f..70e4360a21ba 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_VIDEO_APTINA_PLL) += aptina-pll.o
 obj-$(CONFIG_VIDEO_BT819) += bt819.o
 obj-$(CONFIG_VIDEO_BT856) += bt856.o
 obj-$(CONFIG_VIDEO_BT866) += bt866.o
+obj-$(CONFIG_VIDEO_CAT24C208) += cat24c208.o
 obj-$(CONFIG_VIDEO_CCS) += ccs/
 obj-$(CONFIG_VIDEO_CCS_PLL) += ccs-pll.o
 obj-$(CONFIG_VIDEO_CS3308) += cs3308.o
diff --git a/drivers/media/i2c/cat24c208.c b/drivers/media/i2c/cat24c208.c
new file mode 100644
index 000000000000..9e4e146cdeff
--- /dev/null
+++ b/drivers/media/i2c/cat24c208.c
@@ -0,0 +1,421 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HDMI i2c controlled EEPROM from ON Semiconductor or Catalyst Semiconductor
+ *
+ * Support for i2c based DDC EEPROM
+ *
+ * Copyright (C) 2021-2022 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+
+/*
+ * REF_01 - ON Semiconductor, cat24c208, Datasheet, URL : https://www.onsemi.com/pdf/datasheet/cat24c208-d.pdf
+ *	Revision 7, May 2018
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/mod_devicetable.h>
+#include <linux/slab.h>
+#include <linux/videodev2.h>
+
+#include <media/v4l2-device.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-ioctl.h>
+
+MODULE_DESCRIPTION("cat24c208 EDID EEPROM driver");
+MODULE_AUTHOR("Jonathan Selnes Bognaes <jonathansb1@gmail.com>");
+MODULE_LICENSE("GPL");
+
+/*
+ * CAT24C208 setup
+ */
+#define BYTES_PER_BLOCK		128
+#define EDID_OFFSET_EXT_FLAG	126
+#define MAX_WRITE_BYTES		16
+#define NUM_BLOCKS		4
+#define NUM_CLIENTS		3
+#define CONFIG_NB_BIT		BIT(0)
+#define CONFIG_AB0_BIT		BIT(1)
+#define CONFIG_AB1_BIT		BIT(2)
+#define CONFIG_WE_BIT		BIT(3)
+
+/*
+ * From the datasheet: REF_01
+ *
+ * The write cycle time is the time from a valid stop condition of a write
+ * sequence to the end of the internal program/erase cycle. During the write
+ * cycle, the bus interface circuits are disabled, SDA is allowed to remain
+ * high, and the device does not respond to its slave address.
+ */
+#define WRITE_CYCLE_TIME_US	5000
+
+/*
+ * CAT24C208 addresses
+ */
+#define CONFIG_I2C_ADDR		0x31
+#define EEPROM_I2C_ADDR		0x50
+#define SEGMENT_I2C_ADDR	0x30
+
+struct cat24c208_state {
+	struct i2c_client *dev_client;
+	struct i2c_client *data_client;
+	struct i2c_client *seg_client;
+	// V4L2 ioctl serialization
+	struct mutex lock;
+
+	struct v4l2_device v4l2_dev;
+	struct video_device vdev;
+
+	u8 edid_blocks;		// edid length can vary, one block = 128 bytes
+	u8 edid[BYTES_PER_BLOCK * NUM_BLOCKS]; // actual active edid data
+};
+
+static const struct v4l2_file_operations cat24c208_fops = {
+	.owner		= THIS_MODULE,
+	.open		= v4l2_fh_open,
+	.release	= v4l2_fh_release,
+	.unlocked_ioctl = video_ioctl2,
+};
+
+static int cat24c208_seg_write(struct cat24c208_state *state, u8 *data, u16 len, u8 seg)
+{
+	struct i2c_msg msg[] = {
+		{
+			.addr = state->seg_client->addr,	// Segment
+			.buf = &seg,
+			.len = 1,
+			.flags = 0,
+		},
+		{
+			.addr = state->data_client->addr,	// write data
+			.buf = data,
+			.len = len,
+			.flags = 0,
+		},
+	};
+	int err;
+
+	if (seg)
+		err = i2c_transfer(state->dev_client->adapter, msg, ARRAY_SIZE(msg));
+	else
+		err = i2c_transfer(state->dev_client->adapter, &msg[1], 1);
+
+	if (err < 0)
+		dev_err(&state->dev_client->dev, "Writing to 0x%x failed (segment %d)\n",
+			state->data_client->addr, seg);
+
+	usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
+	return err < 0 ? err : 0;
+}
+
+static int cat24c208_edid_read(struct cat24c208_state *state, u8 *data, u8 seg, u8 offset, u16 len)
+{
+	int err;
+
+	len *= BYTES_PER_BLOCK;
+	if (seg) {
+		struct i2c_msg msg[] = {
+			{
+				.addr = state->seg_client->addr,	// Segment
+				.buf = &seg,
+				.len = 1,
+				.flags = 0,
+			},
+			{
+				.addr = state->data_client->addr,	// read data
+				.buf = data,
+				.len = len,
+				.flags = I2C_M_RD,
+			},
+		};
+		err = i2c_transfer(state->dev_client->adapter, msg, ARRAY_SIZE(msg));
+	} else {
+		struct i2c_msg msg[] = {
+			{
+				.addr = state->data_client->addr,	// set offset
+				.buf = &offset,
+				.len = 1,
+				.flags = 0,
+			},
+			{
+				.addr = state->data_client->addr,	// read data
+				.buf = data,
+				.len = len,
+				.flags = I2C_M_RD,
+			},
+		};
+		err = i2c_transfer(state->dev_client->adapter, msg, ARRAY_SIZE(msg));
+	}
+
+	if (err < 0)
+		dev_err(&state->dev_client->dev, "Reading of EDID failed\n");
+	return err < 0 ? err : 0;
+}
+
+static int cat24c208_set_config(struct i2c_client *client)
+{
+	u8 buf[2] = { 0, CONFIG_NB_BIT };
+	struct i2c_msg msg = {
+		.addr = client->addr,
+		.buf = buf,
+		.len = sizeof(buf),
+		.flags = 0,
+	};
+	int err;
+
+	err = i2c_transfer(client->adapter, &msg, 1);
+	if (err < 0)
+		dev_err(&client->dev, "Could not set config register\n");
+
+	usleep_range(WRITE_CYCLE_TIME_US, 2 * WRITE_CYCLE_TIME_US);
+	return err < 0 ? err : 0;
+}
+
+static bool cat24c208_is_valid_edid(const u8 *block)
+{
+	static const u8 header_pattern[] = {
+		0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
+	};
+
+	return !memcmp(block, header_pattern, sizeof(header_pattern));
+}
+
+static int cat24c208_set_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+	struct cat24c208_state *state = video_drvdata(file);
+	u8 buf[MAX_WRITE_BYTES + 1];
+	int err;
+	int seg;
+	int i;
+
+	memset(edid->reserved, 0, sizeof(edid->reserved));
+
+	if (edid->pad)
+		return -EINVAL;
+
+	if (edid->blocks > NUM_BLOCKS) {
+		edid->blocks = NUM_BLOCKS;
+		return -E2BIG;
+	}
+
+	if (edid->start_block)
+		return -EINVAL;
+
+	state->edid_blocks = edid->blocks;
+	memcpy(state->edid, edid->edid, state->edid_blocks * BYTES_PER_BLOCK);
+
+	/* Write EDID to EEPROM */
+	for (i = 0; i < edid->blocks * BYTES_PER_BLOCK; i = i + MAX_WRITE_BYTES) {
+		if (i >= 2 * BYTES_PER_BLOCK) {
+			seg = 1;
+			buf[0] = i - BYTES_PER_BLOCK * 2;
+		} else {
+			seg = 0;
+			buf[0] = i;
+		}
+
+		memcpy(buf + 1, &edid->edid[i], MAX_WRITE_BYTES);
+		err = cat24c208_seg_write(state, buf, MAX_WRITE_BYTES + 1, seg);
+		if (err) {
+			dev_err(&state->dev_client->dev,
+				"Could not write EDID to EEPROM, i: %d\n", i);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int cat24c208_get_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+	struct cat24c208_state *state = video_drvdata(file);
+
+	memset(edid->reserved, 0, sizeof(edid->reserved));
+
+	if (edid->pad != 0)
+		return -EINVAL;
+
+	if (edid->start_block == 0 && edid->blocks == 0) {
+		edid->blocks = state->edid_blocks;
+		return 0;
+	}
+
+	if (state->edid_blocks == 0)
+		return -ENODATA;
+
+	if (edid->start_block >= state->edid_blocks)
+		return -EINVAL;
+
+	if (edid->start_block + edid->blocks > state->edid_blocks)
+		edid->blocks = state->edid_blocks - edid->start_block;
+
+	memcpy(edid->edid, state->edid + edid->start_block * BYTES_PER_BLOCK,
+	       edid->blocks * BYTES_PER_BLOCK);
+
+	return 0;
+}
+
+static int cat24c208_get_input(struct file *file, void *priv, unsigned int *i)
+{
+	*i = 0;
+	return 0;
+}
+
+static int cat24c208_set_input(struct file *file, void *priv, unsigned int i)
+{
+	return i > 0 ? -EINVAL : 0;
+}
+
+static int cat24c208_enum_input(struct file *file, void *priv,
+				struct v4l2_input *inp)
+{
+	if (inp->index)
+		return -EINVAL;
+	strscpy(inp->name, "HDMI", sizeof(inp->name));
+	inp->capabilities = 0;
+	inp->type = V4L2_INPUT_TYPE_CAMERA;
+	return 0;
+}
+
+static int cat24c208_querycap(struct file *file,
+			      void *priv, struct v4l2_capability *cap)
+{
+	struct cat24c208_state *state = video_drvdata(file);
+	struct i2c_client *client = state->dev_client;
+
+	strscpy(cap->driver, "cat24c208", sizeof(cap->driver));
+	strscpy(cap->card, "cat24c208 EDID EEPROM", sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info),
+		 "I2C:%d-%04x", client->adapter->nr, client->addr);
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops cat24c208_ioctl_ops = {
+	.vidioc_querycap	= cat24c208_querycap,
+	.vidioc_g_edid		= cat24c208_get_edid,
+	.vidioc_s_edid		= cat24c208_set_edid,
+	.vidioc_g_input		= cat24c208_get_input,
+	.vidioc_s_input		= cat24c208_set_input,
+	.vidioc_enum_input	= cat24c208_enum_input,
+};
+
+static void cat24c208_release(struct video_device *vdev)
+{
+	struct cat24c208_state *state = video_get_drvdata(vdev);
+
+	v4l2_device_unregister(&state->v4l2_dev);
+	mutex_destroy(&state->lock);
+	kfree(state);
+}
+
+static int cat24c208_probe(struct i2c_client *client)
+{
+	struct cat24c208_state *state;
+	struct v4l2_device *v4l2_dev;
+	int blocks;
+	int ret;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	state->dev_client = client;
+	state->data_client = i2c_new_ancillary_device(client, "eeprom", EEPROM_I2C_ADDR);
+	if (IS_ERR(state->data_client)) {
+		ret = PTR_ERR(state->data_client);
+		goto free_state;
+	}
+	state->seg_client = i2c_new_ancillary_device(client, "segment", SEGMENT_I2C_ADDR);
+	if (IS_ERR(state->seg_client)) {
+		ret = PTR_ERR(state->seg_client);
+		goto unreg_i2c_first;
+	}
+
+	ret = cat24c208_set_config(client);
+	if (ret)
+		goto unreg_i2c_all;
+
+	if (cat24c208_edid_read(state, state->edid, 0, 0, 2) >= 0 &&
+	    cat24c208_is_valid_edid(state->edid)) {
+		unsigned int i;
+
+		blocks = 1 + state->edid[EDID_OFFSET_EXT_FLAG];
+		state->edid_blocks = blocks;
+		for (i = 2; i < blocks; i += 2) {
+			if (cat24c208_edid_read(state, state->edid + i * BYTES_PER_BLOCK,
+						i / 2, 0, (i + 1 >= blocks ? 1 : 2))) {
+				state->edid_blocks = i;
+				break;
+			}
+		}
+	}
+
+	v4l2_dev = &state->v4l2_dev;
+	strscpy(v4l2_dev->name, "cat24c208", sizeof(v4l2_dev->name));
+	ret = v4l2_device_register(&client->dev, v4l2_dev);
+	if (ret) {
+		dev_err(&client->dev, "v4l2_device_register failed: %d\n", ret);
+		goto unreg_i2c_all;
+	}
+
+	mutex_init(&state->lock);
+
+	snprintf(state->vdev.name, sizeof(state->vdev.name),
+		 "cat24c208 %d-%d", client->adapter->nr, client->addr);
+
+	state->vdev.v4l2_dev = v4l2_dev;
+	state->vdev.fops = &cat24c208_fops;
+	state->vdev.ioctl_ops = &cat24c208_ioctl_ops;
+	state->vdev.lock = &state->lock;
+	state->vdev.release = cat24c208_release;
+	state->vdev.device_caps = V4L2_CAP_EDID_MEMORY;
+
+	video_set_drvdata(&state->vdev, state);
+	i2c_set_clientdata(client, state);
+	ret = video_register_device(&state->vdev, VFL_TYPE_VIDEO, -1);
+	if (ret != 0) {
+		dev_err(&client->dev, "Video registering failed: %d\n", ret);
+		goto unreg_v4l2_dev;
+	}
+	return 0;
+
+unreg_v4l2_dev:
+	v4l2_device_unregister(&state->v4l2_dev);
+unreg_i2c_all:
+	i2c_unregister_device(state->seg_client);
+unreg_i2c_first:
+	i2c_unregister_device(state->data_client);
+free_state:
+	kfree(state);
+	return ret;
+}
+
+static int cat24c208_remove(struct i2c_client *client)
+{
+	struct cat24c208_state *state = i2c_get_clientdata(client);
+
+	i2c_unregister_device(state->data_client);
+	i2c_unregister_device(state->seg_client);
+	video_unregister_device(&state->vdev);
+
+	return 0;
+}
+
+static const struct of_device_id cat24c208_of_match[] = {
+	{ .compatible = "onnn,cat24c208"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, cat24c208_of_match);
+
+static struct i2c_driver cat24c208_driver = {
+	.driver = {
+		.name	= "cat24c208",
+		.of_match_table = cat24c208_of_match,
+	},
+	.probe_new	= cat24c208_probe,
+	.remove		= cat24c208_remove,
+};
+module_i2c_driver(cat24c208_driver);
-- 
2.37.1


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

* [PATCH v2 5/5] media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY
  2022-08-03  7:58 [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Erling Ljunggren
                   ` (3 preceding siblings ...)
  2022-08-03  7:58 ` [PATCH v2 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM Erling Ljunggren
@ 2022-08-03  7:58 ` Erling Ljunggren
  2022-08-03  9:14 ` [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Laurent Pinchart
  5 siblings, 0 replies; 12+ messages in thread
From: Erling Ljunggren @ 2022-08-03  7:58 UTC (permalink / raw)
  To: linux-media; +Cc: Erling Ljunggren

When the V4L2_CAP_EDID_MEMORY capability flag is set,
ioctls for enum inputs and get/set edid are automatically set.

Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
---
 drivers/media/v4l2-core/v4l2-dev.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index d00237ee4cae..39437911edcc 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -556,6 +556,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
 	bool is_io_mc = vdev->device_caps & V4L2_CAP_IO_MC;
+	bool is_edid_mem =  vdev->device_caps & V4L2_CAP_EDID_MEMORY;
 
 	bitmap_zero(valid_ioctls, BASE_VIDIOC_PRIVATE);
 
@@ -778,6 +779,13 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
 		SET_VALID_IOCTL(ops, VIDIOC_S_HW_FREQ_SEEK, vidioc_s_hw_freq_seek);
 	}
+	if (is_edid_mem) {
+		SET_VALID_IOCTL(ops, VIDIOC_G_EDID, vidioc_g_edid);
+		SET_VALID_IOCTL(ops, VIDIOC_S_EDID, vidioc_s_edid);
+		SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
+		SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
+		SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
+	}
 
 	bitmap_andnot(vdev->valid_ioctls, valid_ioctls, vdev->valid_ioctls,
 			BASE_VIDIOC_PRIVATE);
-- 
2.37.1


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

* Re: [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability
  2022-08-03  7:58 [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Erling Ljunggren
                   ` (4 preceding siblings ...)
  2022-08-03  7:58 ` [PATCH v2 5/5] media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY Erling Ljunggren
@ 2022-08-03  9:14 ` Laurent Pinchart
  2022-08-03  9:19   ` Hans Verkuil
  5 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2022-08-03  9:14 UTC (permalink / raw)
  To: Erling Ljunggren; +Cc: linux-media, linux-i2c, Srinivas Kandagatla

Hi Erling,

(CC'ing the linux-i2c mailing list and Srinivas, the maintainer of the
nvmem framework)

Thank you for the patches.

On Wed, Aug 03, 2022 at 09:58:45AM +0200, Erling Ljunggren wrote:
> This series adds support for the standalone cat24c208 EDID EEPROM i2c device.
> Usually EDID support is part of an HDMI receiver, but this is a standalone EEPROM.
> 
> Note that EEPROMs for EDIDs are not regular EEPROM devices, these are dual port
> devices that follow the VESA E-DDC standard.
> 
> Since this is a standalone device that does not capture any video a new
> V4L2_CAP_EDID_MEMORY capability is introduced to represent such devices.
> Note that such a device doesn't have to be an EEPROM, it can also be
> implemented using a microcontroller, for example. Hence the use of the generic
> word 'MEMORY'.

Why can't this be exposed as a nvmem device, like other types of eeproms
here ?

> The new capability uses the free bit 0x00000008. But we are running out of
> capability bits: only 0x40000000 and 0x00000008 are free at the moment.
> 
> There is one other capability V4L2_CAP_ASYNCIO (0x02000000) that is not used
> at all, it was never implemented. Wouldn't it be better to define
> V4L2_CAP_ASYNCIO to 0, mark it as obsolete, and instead reuse it for this
> V4L2_CAP_EDID_MEMORY capability?
> 
> v2:
>  - fix dt binding example
>  - rename i2c client variables in data struct
>  - fix include: of_device.h -> mod_devicetable.h
>  - Sorted makefile
>  - used define EDID_OFFSET_EXT_FLAG instead of magic number
>  - removed of_match_ptr
>  - added bus_info
>  - remove unneeded headers
>  - add depends on OF to Kconfig
> 
> Erling Ljunggren (4):
>   media: videodev2.h: add V4L2_CAP_EDID_MEMORY
>   media: docs: Add V4L2_CAP_EDID_MEMORY
>   dt-bindings: media: add cat24c208 bindings
>   media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY
> 
> Jonathan Selnes (1):
>   media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM
> 
>  .../bindings/media/i2c/onnn,cat24c208.yaml    |  40 ++
>  .../userspace-api/media/v4l/biblio.rst        |  11 +
>  .../media/v4l/vidioc-querycap.rst             |   7 +
>  .../media/videodev2.h.rst.exceptions          |   1 +
>  MAINTAINERS                                   |   7 +
>  drivers/media/i2c/Kconfig                     |   9 +
>  drivers/media/i2c/Makefile                    |   1 +
>  drivers/media/i2c/cat24c208.c                 | 421 ++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-dev.c            |   8 +
>  include/uapi/linux/videodev2.h                |   1 +
>  10 files changed, 506 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
>  create mode 100644 drivers/media/i2c/cat24c208.c

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability
  2022-08-03  9:14 ` [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Laurent Pinchart
@ 2022-08-03  9:19   ` Hans Verkuil
  2022-08-03  9:25     ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2022-08-03  9:19 UTC (permalink / raw)
  To: Laurent Pinchart, Erling Ljunggren
  Cc: linux-media, linux-i2c, Srinivas Kandagatla



On 03/08/2022 11:14, Laurent Pinchart wrote:
> Hi Erling,
> 
> (CC'ing the linux-i2c mailing list and Srinivas, the maintainer of the
> nvmem framework)
> 
> Thank you for the patches.
> 
> On Wed, Aug 03, 2022 at 09:58:45AM +0200, Erling Ljunggren wrote:
>> This series adds support for the standalone cat24c208 EDID EEPROM i2c device.
>> Usually EDID support is part of an HDMI receiver, but this is a standalone EEPROM.
>>
>> Note that EEPROMs for EDIDs are not regular EEPROM devices, these are dual port
>> devices that follow the VESA E-DDC standard.
>>
>> Since this is a standalone device that does not capture any video a new
>> V4L2_CAP_EDID_MEMORY capability is introduced to represent such devices.
>> Note that such a device doesn't have to be an EEPROM, it can also be
>> implemented using a microcontroller, for example. Hence the use of the generic
>> word 'MEMORY'.
> 
> Why can't this be exposed as a nvmem device, like other types of eeproms
> here ?

Because it isn't. Same discussion I had with Andy: it's not a regular eeprom but
a dedicated eeprom for EDIDs. And we have support for that already in V4L2. It's
specific to receivers as well, so the only subsystem it belongs to is V4L2.

See the discussion I had with Andy on this:

https://patchwork.linuxtv.org/project/linux-media/patch/20220728114050.2400475-5-hljunggr@cisco.com/

Regards,

	Hans

> 
>> The new capability uses the free bit 0x00000008. But we are running out of
>> capability bits: only 0x40000000 and 0x00000008 are free at the moment.
>>
>> There is one other capability V4L2_CAP_ASYNCIO (0x02000000) that is not used
>> at all, it was never implemented. Wouldn't it be better to define
>> V4L2_CAP_ASYNCIO to 0, mark it as obsolete, and instead reuse it for this
>> V4L2_CAP_EDID_MEMORY capability?
>>
>> v2:
>>  - fix dt binding example
>>  - rename i2c client variables in data struct
>>  - fix include: of_device.h -> mod_devicetable.h
>>  - Sorted makefile
>>  - used define EDID_OFFSET_EXT_FLAG instead of magic number
>>  - removed of_match_ptr
>>  - added bus_info
>>  - remove unneeded headers
>>  - add depends on OF to Kconfig
>>
>> Erling Ljunggren (4):
>>   media: videodev2.h: add V4L2_CAP_EDID_MEMORY
>>   media: docs: Add V4L2_CAP_EDID_MEMORY
>>   dt-bindings: media: add cat24c208 bindings
>>   media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY
>>
>> Jonathan Selnes (1):
>>   media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM
>>
>>  .../bindings/media/i2c/onnn,cat24c208.yaml    |  40 ++
>>  .../userspace-api/media/v4l/biblio.rst        |  11 +
>>  .../media/v4l/vidioc-querycap.rst             |   7 +
>>  .../media/videodev2.h.rst.exceptions          |   1 +
>>  MAINTAINERS                                   |   7 +
>>  drivers/media/i2c/Kconfig                     |   9 +
>>  drivers/media/i2c/Makefile                    |   1 +
>>  drivers/media/i2c/cat24c208.c                 | 421 ++++++++++++++++++
>>  drivers/media/v4l2-core/v4l2-dev.c            |   8 +
>>  include/uapi/linux/videodev2.h                |   1 +
>>  10 files changed, 506 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
>>  create mode 100644 drivers/media/i2c/cat24c208.c
> 

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

* Re: [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability
  2022-08-03  9:19   ` Hans Verkuil
@ 2022-08-03  9:25     ` Laurent Pinchart
  2022-08-03 10:37       ` Hans Verkuil
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2022-08-03  9:25 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Erling Ljunggren, linux-media, linux-i2c, Srinivas Kandagatla

Hi Hans,

On Wed, Aug 03, 2022 at 11:19:31AM +0200, Hans Verkuil wrote:
> On 03/08/2022 11:14, Laurent Pinchart wrote:
> > Hi Erling,
> > 
> > (CC'ing the linux-i2c mailing list and Srinivas, the maintainer of the
> > nvmem framework)
> > 
> > Thank you for the patches.
> > 
> > On Wed, Aug 03, 2022 at 09:58:45AM +0200, Erling Ljunggren wrote:
> >> This series adds support for the standalone cat24c208 EDID EEPROM i2c device.
> >> Usually EDID support is part of an HDMI receiver, but this is a standalone EEPROM.
> >>
> >> Note that EEPROMs for EDIDs are not regular EEPROM devices, these are dual port
> >> devices that follow the VESA E-DDC standard.
> >>
> >> Since this is a standalone device that does not capture any video a new
> >> V4L2_CAP_EDID_MEMORY capability is introduced to represent such devices.
> >> Note that such a device doesn't have to be an EEPROM, it can also be
> >> implemented using a microcontroller, for example. Hence the use of the generic
> >> word 'MEMORY'.
> > 
> > Why can't this be exposed as a nvmem device, like other types of eeproms
> > here ?
> 
> Because it isn't. Same discussion I had with Andy: it's not a regular eeprom but
> a dedicated eeprom for EDIDs. And we have support for that already in V4L2. It's
> specific to receivers as well, so the only subsystem it belongs to is V4L2.

Why does that matter ? It's still an NVMEM device that stores 512 bytes
of data, even if the I2C interface to read/write it is different than
other types of standard EEPROMs.

> See the discussion I had with Andy on this:
> 
> https://patchwork.linuxtv.org/project/linux-media/patch/20220728114050.2400475-5-hljunggr@cisco.com/
> 
> >> The new capability uses the free bit 0x00000008. But we are running out of
> >> capability bits: only 0x40000000 and 0x00000008 are free at the moment.
> >>
> >> There is one other capability V4L2_CAP_ASYNCIO (0x02000000) that is not used
> >> at all, it was never implemented. Wouldn't it be better to define
> >> V4L2_CAP_ASYNCIO to 0, mark it as obsolete, and instead reuse it for this
> >> V4L2_CAP_EDID_MEMORY capability?
> >>
> >> v2:
> >>  - fix dt binding example
> >>  - rename i2c client variables in data struct
> >>  - fix include: of_device.h -> mod_devicetable.h
> >>  - Sorted makefile
> >>  - used define EDID_OFFSET_EXT_FLAG instead of magic number
> >>  - removed of_match_ptr
> >>  - added bus_info
> >>  - remove unneeded headers
> >>  - add depends on OF to Kconfig
> >>
> >> Erling Ljunggren (4):
> >>   media: videodev2.h: add V4L2_CAP_EDID_MEMORY
> >>   media: docs: Add V4L2_CAP_EDID_MEMORY
> >>   dt-bindings: media: add cat24c208 bindings
> >>   media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY
> >>
> >> Jonathan Selnes (1):
> >>   media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM
> >>
> >>  .../bindings/media/i2c/onnn,cat24c208.yaml    |  40 ++
> >>  .../userspace-api/media/v4l/biblio.rst        |  11 +
> >>  .../media/v4l/vidioc-querycap.rst             |   7 +
> >>  .../media/videodev2.h.rst.exceptions          |   1 +
> >>  MAINTAINERS                                   |   7 +
> >>  drivers/media/i2c/Kconfig                     |   9 +
> >>  drivers/media/i2c/Makefile                    |   1 +
> >>  drivers/media/i2c/cat24c208.c                 | 421 ++++++++++++++++++
> >>  drivers/media/v4l2-core/v4l2-dev.c            |   8 +
> >>  include/uapi/linux/videodev2.h                |   1 +
> >>  10 files changed, 506 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
> >>  create mode 100644 drivers/media/i2c/cat24c208.c

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability
  2022-08-03  9:25     ` Laurent Pinchart
@ 2022-08-03 10:37       ` Hans Verkuil
  0 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2022-08-03 10:37 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Erling Ljunggren, linux-media, linux-i2c, Srinivas Kandagatla

Hi Laurent,

On 03/08/2022 11:25, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Wed, Aug 03, 2022 at 11:19:31AM +0200, Hans Verkuil wrote:
>> On 03/08/2022 11:14, Laurent Pinchart wrote:
>>> Hi Erling,
>>>
>>> (CC'ing the linux-i2c mailing list and Srinivas, the maintainer of the
>>> nvmem framework)
>>>
>>> Thank you for the patches.
>>>
>>> On Wed, Aug 03, 2022 at 09:58:45AM +0200, Erling Ljunggren wrote:
>>>> This series adds support for the standalone cat24c208 EDID EEPROM i2c device.
>>>> Usually EDID support is part of an HDMI receiver, but this is a standalone EEPROM.
>>>>
>>>> Note that EEPROMs for EDIDs are not regular EEPROM devices, these are dual port
>>>> devices that follow the VESA E-DDC standard.
>>>>
>>>> Since this is a standalone device that does not capture any video a new
>>>> V4L2_CAP_EDID_MEMORY capability is introduced to represent such devices.
>>>> Note that such a device doesn't have to be an EEPROM, it can also be
>>>> implemented using a microcontroller, for example. Hence the use of the generic
>>>> word 'MEMORY'.
>>>
>>> Why can't this be exposed as a nvmem device, like other types of eeproms
>>> here ?
>>
>> Because it isn't. Same discussion I had with Andy: it's not a regular eeprom but
>> a dedicated eeprom for EDIDs. And we have support for that already in V4L2. It's
>> specific to receivers as well, so the only subsystem it belongs to is V4L2.
> 
> Why does that matter ? It's still an NVMEM device that stores 512 bytes
> of data, even if the I2C interface to read/write it is different than
> other types of standard EEPROMs.

EDID support is already present in various HDMI receivers (e.g. adv7604). We have
the API for that (VIDIOC_G/S_EDID). The only difference is that for those HDMI
receivers the EDID is in volatile memory and is integrated into the receiver.

Regardless of the underlying implementation (integrated into an HDMI receiver,
a standalone eeprom, or even implemented with a microcontroller), these devices
are dedicated to storing an EDID and are hooked up to the DDC lines of an HDMI
input (or even to multiple HDMI inputs, as supported by the adv7604).

Changing an EDID typically requires additional work such as toggling the hotplug
detect pin, and (if CEC is supported) updating the physical address of the CEC
adapter. While that is not (yet) supported in this driver, it is something that
we might add later.

None of this is good fit for a nvmem driver. The key feature is that this is for
EDIDs. Whether it is implemented as volatile or non-volatile memory is irrelevant,
the relevant feature is that it stores an EDID and is able (if needed) to take care
of HPD toggling and CEC physical addresses.

Perhaps if we add support for e.g. toggling an HPD pin (even though it is not needed
yet for our use-case), it would become more obvious that this not a nvmem device?

Comparing this driver with adv7604 I see that it would make sense to also validate
the EDID (the physical address in particular). Adding that would also more clearly
tie it to the v4l2 subsystem.

V4L2 has all the infrastructure for EDIDs, both in the kernel and in userspace, so
it really does not make sense to go for nvmem, just because it happens to be an
eeprom.

Regards,

	Hans

> 
>> See the discussion I had with Andy on this:
>>
>> https://patchwork.linuxtv.org/project/linux-media/patch/20220728114050.2400475-5-hljunggr@cisco.com/
>>
>>>> The new capability uses the free bit 0x00000008. But we are running out of
>>>> capability bits: only 0x40000000 and 0x00000008 are free at the moment.
>>>>
>>>> There is one other capability V4L2_CAP_ASYNCIO (0x02000000) that is not used
>>>> at all, it was never implemented. Wouldn't it be better to define
>>>> V4L2_CAP_ASYNCIO to 0, mark it as obsolete, and instead reuse it for this
>>>> V4L2_CAP_EDID_MEMORY capability?
>>>>
>>>> v2:
>>>>  - fix dt binding example
>>>>  - rename i2c client variables in data struct
>>>>  - fix include: of_device.h -> mod_devicetable.h
>>>>  - Sorted makefile
>>>>  - used define EDID_OFFSET_EXT_FLAG instead of magic number
>>>>  - removed of_match_ptr
>>>>  - added bus_info
>>>>  - remove unneeded headers
>>>>  - add depends on OF to Kconfig
>>>>
>>>> Erling Ljunggren (4):
>>>>   media: videodev2.h: add V4L2_CAP_EDID_MEMORY
>>>>   media: docs: Add V4L2_CAP_EDID_MEMORY
>>>>   dt-bindings: media: add cat24c208 bindings
>>>>   media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY
>>>>
>>>> Jonathan Selnes (1):
>>>>   media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM
>>>>
>>>>  .../bindings/media/i2c/onnn,cat24c208.yaml    |  40 ++
>>>>  .../userspace-api/media/v4l/biblio.rst        |  11 +
>>>>  .../media/v4l/vidioc-querycap.rst             |   7 +
>>>>  .../media/videodev2.h.rst.exceptions          |   1 +
>>>>  MAINTAINERS                                   |   7 +
>>>>  drivers/media/i2c/Kconfig                     |   9 +
>>>>  drivers/media/i2c/Makefile                    |   1 +
>>>>  drivers/media/i2c/cat24c208.c                 | 421 ++++++++++++++++++
>>>>  drivers/media/v4l2-core/v4l2-dev.c            |   8 +
>>>>  include/uapi/linux/videodev2.h                |   1 +
>>>>  10 files changed, 506 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
>>>>  create mode 100644 drivers/media/i2c/cat24c208.c
> 

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

* Re: [PATCH v2 3/5] dt-bindings: media: add cat24c208 bindings
  2022-08-03  7:58 ` [PATCH v2 3/5] dt-bindings: media: add cat24c208 bindings Erling Ljunggren
@ 2022-08-03 23:41   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2022-08-03 23:41 UTC (permalink / raw)
  To: Erling Ljunggren; +Cc: devicetree, linux-media

On Wed, 03 Aug 2022 09:58:48 +0200, Erling Ljunggren wrote:
> Add devicetree bindings for new cat24c208 EDID EEPROM driver.
> 
> Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
> ---
>  .../bindings/media/i2c/onnn,cat24c208.yaml    | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/onnn,cat24c208.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v2 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY
  2022-08-03  7:58 ` [PATCH v2 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY Erling Ljunggren
@ 2022-08-18  7:41   ` Hans Verkuil
  0 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2022-08-18  7:41 UTC (permalink / raw)
  To: Erling Ljunggren, linux-media

Hi Erling,

On 03/08/2022 09:58, Erling Ljunggren wrote:
> Add documentation for the new edid eeprom capability.
> 
> Signed-off-by: Erling Ljunggren <hljunggr@cisco.com>
> ---
>  Documentation/userspace-api/media/v4l/biblio.rst      | 11 +++++++++++
>  .../userspace-api/media/v4l/vidioc-querycap.rst       |  7 +++++++
>  .../userspace-api/media/videodev2.h.rst.exceptions    |  1 +
>  3 files changed, 19 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/biblio.rst b/Documentation/userspace-api/media/v4l/biblio.rst
> index 9cd18c153d19..5cbe41877a63 100644
> --- a/Documentation/userspace-api/media/v4l/biblio.rst
> +++ b/Documentation/userspace-api/media/v4l/biblio.rst
> @@ -334,6 +334,17 @@ VESA DMT
>  
>  :author:    Video Electronics Standards Association (http://www.vesa.org)
>  
> +.. _vesaeddc:
> +
> +E-DDC
> +====
> +
> +
> +:title:     VESA Enhanced Display Data Channel (E-DDC) Standard
> +:subtitle:  Version 1.3
> +
> +:author:    Video Electronics Standards Association (http://www.vesa.org)
> +
>  .. _vesaedid:
>  
>  EDID
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-querycap.rst b/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
> index 63e23f6f95ee..bdb530bd6816 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-querycap.rst
> @@ -173,6 +173,13 @@ specification the ioctl returns an ``EINVAL`` error code.
>  	interface. A video overlay device typically stores captured images
>  	directly in the video memory of a graphics card, with hardware
>  	clipping and scaling.
> +    * - ``V4L2_CAP_EDID_MEMORY``
> +      - 0x00000008
> +      - The device is a standalone EDID memory device. This is typically an eeprom
> +        that supports the VESA Enhanced Display Data Channel Standard.
> +
> +        While an eeprom is the most common implementation, it can be something else
> +        as well, such as a microcontroller. Hence the generic name 'memory'.

I realized that this is not just needed for inputs, but also for outputs.

This is the case for an HDMI splitter that is connected to a computer via (typically)
a serial port where you can use that to get/set the EDID for the input port and
get the EDIDs for the output ports (i.e. the EDIDs from the connected displays).

It's the same concept: a standalone EDID device. However, you are not talking
directly to a memory device like an eeprom. So I think to keep things generic
this capability should be renamed to just V4L2_CAP_EDID.

The description should be updated as well since it is no longer just for inputs.

And I think we can drop the references to 'memory'. It's really about hardware
devices that store an EDID for a sink or read an EDID from a source.

Patch 5 will have to be updated as well since for an output device this cap is
associated with ENUM/S/G_OUTPUT. And for output S_EDID is not supported, of
course.

I'm working on a driver for an HDMI Splitter (CEC and EDID support), so this
would come in very handy for that.

Regards,

	Hans

>      * - ``V4L2_CAP_VBI_CAPTURE``
>        - 0x00000010
>        - The device supports the :ref:`Raw VBI Capture <raw-vbi>`
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 9cbb7a0c354a..12fa290828e7 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -185,6 +185,7 @@ replace define V4L2_CAP_META_OUTPUT device-capabilities
>  replace define V4L2_CAP_DEVICE_CAPS device-capabilities
>  replace define V4L2_CAP_TOUCH device-capabilities
>  replace define V4L2_CAP_IO_MC device-capabilities
> +replace define V4L2_CAP_EDID_MEMORY device-capabilities
>  
>  # V4L2 pix flags
>  replace define V4L2_PIX_FMT_PRIV_MAGIC :c:type:`v4l2_pix_format`

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

end of thread, other threads:[~2022-08-18  7:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03  7:58 [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Erling Ljunggren
2022-08-03  7:58 ` [PATCH v2 1/5] media: videodev2.h: add V4L2_CAP_EDID_MEMORY Erling Ljunggren
2022-08-03  7:58 ` [PATCH v2 2/5] media: docs: Add V4L2_CAP_EDID_MEMORY Erling Ljunggren
2022-08-18  7:41   ` Hans Verkuil
2022-08-03  7:58 ` [PATCH v2 3/5] dt-bindings: media: add cat24c208 bindings Erling Ljunggren
2022-08-03 23:41   ` Rob Herring
2022-08-03  7:58 ` [PATCH v2 4/5] media: i2c: cat24c208: driver for the cat24c208 EDID EEPROM Erling Ljunggren
2022-08-03  7:58 ` [PATCH v2 5/5] media: v4l2-dev: handle V4L2_CAP_EDID_MEMORY Erling Ljunggren
2022-08-03  9:14 ` [PATCH v2 0/5] Add the cat24c208 EDID EEPROM driver + new EDID capability Laurent Pinchart
2022-08-03  9:19   ` Hans Verkuil
2022-08-03  9:25     ` Laurent Pinchart
2022-08-03 10:37       ` Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.