* [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line
@ 2022-05-04 22:30 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 01/15] media: uapi: Add IPU3 packed Y10 format Daniel Scally
` (14 more replies)
0 siblings, 15 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Hello all
This series extends the OV7251 driver so it's functional on the
Microsoft Surface line of laptops, where it's used as the IR
camera for face login. The camera sensor is connected to a CIO2
device which packs the 10-bit greyscale data into 25 pixels per 32
bytes similar to the IPU3 formats for Bayer data, so I also added
a new format to describe that and added it to the ipu3-cio2 driver's
list of supported formats.
Series-level changes:
- Added patches 12-15, which extend the driver's controls to fulfil libcamera
requirements.
Thanks
Dan
Daniel Scally (15):
media: uapi: Add IPU3 packed Y10 format
media: ipu3-cio2: Add support for V4L2_PIX_FMT_IPU3_Y10
media: i2c: Add acpi support to ov7251
media: i2c: Provide ov7251_check_hwcfg()
media: i2c: Remove per-mode frequencies from ov7251
media: i2c: Add ov7251_pll_configure()
media: i2c: Add support for new frequencies to ov7251
media: i2c: Add ov7251_detect_chip()
media: i2c: Add pm_runtime support to ov7251
media: i2c: Remove .s_power() from ov7251
media: ipu3-cio2: Add INT347E to cio2-bridge
media: i2c: Extend .get_selection() for ov7251
media: i2c: add ov7251_init_ctrls()
media: i2c: Add hblank control to ov7251
media: i2c: Add vblank control to ov7251 driver
.../media/v4l/pixfmt-yuv-luma.rst | 14 +-
drivers/media/i2c/ov7251.c | 749 +++++++++++++-----
drivers/media/pci/intel/ipu3/cio2-bridge.c | 2 +
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 5 +
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/videodev2.h | 3 +-
6 files changed, 554 insertions(+), 220 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 01/15] media: uapi: Add IPU3 packed Y10 format
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 02/15] media: ipu3-cio2: Add support for V4L2_PIX_FMT_IPU3_Y10 Daniel Scally
` (13 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Some platforms with an Intel IPU3 have an IR sensor producing 10 bit
greyscale format data that is transmitted over a CSI-2 bus to a CIO2
device - this packs the data into 32 bytes per 25 pixels. Add an entry
to the uAPI header defining that format.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- Switched away from using the fourcc in the explanatory note for
pixfmt-yuv-luma.rst (Nicolas)
.../userspace-api/media/v4l/pixfmt-yuv-luma.rst | 14 +++++++++++++-
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/videodev2.h | 3 ++-
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst
index 8ebd58c3588f..6a387f9df3ba 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst
@@ -48,6 +48,17 @@ are often referred to as greyscale formats.
- ...
- ...
+ * .. _V4L2-PIX-FMT-IPU3-Y10:
+
+ - ``V4L2_PIX_FMT_IPU3_Y10``
+ - 'ip3y'
+
+ - Y'\ :sub:`0`\ [7:0]
+ - Y'\ :sub:`1`\ [5:0] Y'\ :sub:`0`\ [9:8]
+ - Y'\ :sub:`2`\ [3:0] Y'\ :sub:`1`\ [9:6]
+ - Y'\ :sub:`3`\ [1:0] Y'\ :sub:`2`\ [9:4]
+ - Y'\ :sub:`3`\ [9:2]
+
* .. _V4L2-PIX-FMT-Y10:
- ``V4L2_PIX_FMT_Y10``
@@ -133,4 +144,5 @@ are often referred to as greyscale formats.
For the Y16 and Y16_BE formats, the actual sampling precision may be lower
than 16 bits. For example, 10 bits per pixel uses values in the range 0 to
- 1023.
+ 1023. For the IPU3_Y10 format 25 pixels are packed into 32 bytes, which
+ leaves the 6 most significant bits of the last byte padded with 0.
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 642cb90f457c..89691bbb372d 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1265,6 +1265,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_Y16_BE: descr = "16-bit Greyscale BE"; break;
case V4L2_PIX_FMT_Y10BPACK: descr = "10-bit Greyscale (Packed)"; break;
case V4L2_PIX_FMT_Y10P: descr = "10-bit Greyscale (MIPI Packed)"; break;
+ case V4L2_PIX_FMT_IPU3_Y10: descr = "10-bit greyscale (IPU3 Packed)"; break;
case V4L2_PIX_FMT_Y8I: descr = "Interleaved 8-bit Greyscale"; break;
case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale"; break;
case V4L2_PIX_FMT_Z16: descr = "16-bit Depth"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index df8b9c486ba1..34329f4655e0 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -569,6 +569,7 @@ struct v4l2_pix_format {
/* Grey bit-packed formats */
#define V4L2_PIX_FMT_Y10BPACK v4l2_fourcc('Y', '1', '0', 'B') /* 10 Greyscale bit-packed */
#define V4L2_PIX_FMT_Y10P v4l2_fourcc('Y', '1', '0', 'P') /* 10 Greyscale, MIPI RAW10 packed */
+#define V4L2_PIX_FMT_IPU3_Y10 v4l2_fourcc('i', 'p', '3', 'y') /* IPU3 packed 10-bit greyscale */
/* Palette formats */
#define V4L2_PIX_FMT_PAL8 v4l2_fourcc('P', 'A', 'L', '8') /* 8 8-bit palette */
@@ -745,7 +746,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */
#define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */
-/* 10bit raw bayer packed, 32 bytes for every 25 pixels, last LSB 6 bits unused */
+/* 10bit raw packed, 32 bytes for every 25 pixels, last LSB 6 bits unused */
#define V4L2_PIX_FMT_IPU3_SBGGR10 v4l2_fourcc('i', 'p', '3', 'b') /* IPU3 packed 10-bit BGGR bayer */
#define V4L2_PIX_FMT_IPU3_SGBRG10 v4l2_fourcc('i', 'p', '3', 'g') /* IPU3 packed 10-bit GBRG bayer */
#define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 02/15] media: ipu3-cio2: Add support for V4L2_PIX_FMT_IPU3_Y10
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
2022-05-04 22:30 ` [PATCH v3 01/15] media: uapi: Add IPU3 packed Y10 format Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 03/15] media: i2c: Add acpi support to ov7251 Daniel Scally
` (12 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
We have platforms where a camera sensor transmits Y10 data to
the CIO2 device - add support for that (packed) format to the
ipu3-cio2 driver.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- None
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
index 0e9b0503b62a..93cc0577b6db 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
@@ -65,6 +65,11 @@ static const struct ipu3_cio2_fmt formats[] = {
.fourcc = V4L2_PIX_FMT_IPU3_SRGGB10,
.mipicode = 0x2b,
.bpp = 10,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_Y10_1X10,
+ .fourcc = V4L2_PIX_FMT_IPU3_Y10,
+ .mipicode = 0x2b,
+ .bpp = 10,
},
};
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 03/15] media: i2c: Add acpi support to ov7251
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
2022-05-04 22:30 ` [PATCH v3 01/15] media: uapi: Add IPU3 packed Y10 format Daniel Scally
2022-05-04 22:30 ` [PATCH v3 02/15] media: ipu3-cio2: Add support for V4L2_PIX_FMT_IPU3_Y10 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 04/15] media: i2c: Provide ov7251_check_hwcfg() Daniel Scally
` (11 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Add support for enumeration through ACPI to the ov7251 driver
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- None
drivers/media/i2c/ov7251.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index ebb299f207e5..d6fe574cb9e0 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -14,6 +14,7 @@
#include <linux/i2c.h>
#include <linux/init.h>
#include <linux/module.h>
+#include <linux/mod_devicetable.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -1490,9 +1491,16 @@ static const struct of_device_id ov7251_of_match[] = {
};
MODULE_DEVICE_TABLE(of, ov7251_of_match);
+static const struct acpi_device_id ov7251_acpi_match[] = {
+ { "INT347E" },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, ov7251_acpi_match);
+
static struct i2c_driver ov7251_i2c_driver = {
.driver = {
.of_match_table = ov7251_of_match,
+ .acpi_match_table = ov7251_acpi_match,
.name = "ov7251",
},
.probe_new = ov7251_probe,
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 04/15] media: i2c: Provide ov7251_check_hwcfg()
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (2 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 03/15] media: i2c: Add acpi support to ov7251 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-05 10:16 ` Andy Shevchenko
2022-05-04 22:30 ` [PATCH v3 05/15] media: i2c: Remove per-mode frequencies from ov7251 Daniel Scally
` (10 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Move the endpoint checking from .probe() to a dedicated function,
and additionally check that the firmware provided link frequencies
are a match for those supported by the driver. Store the index to the
matching link frequency so it can be easily identified later.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- Replaced freq_found variable (Andy)
Changes in v2:
- Switched to use unsigned int (Sakari)
- Dropped the checks for bus_type and number of data lanes (Sakari)
- Fixed the double-loop break (Dave)
- Stored the index to the configured link frequency so it can be used
later on.
drivers/media/i2c/ov7251.c | 77 +++++++++++++++++++++++++++++---------
1 file changed, 59 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index d6fe574cb9e0..f1965c8adbd7 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -60,6 +60,11 @@ struct ov7251_mode_info {
struct v4l2_fract timeperframe;
};
+enum supported_link_freqs {
+ OV7251_LINK_FREQ_240_MHZ,
+ OV7251_NUM_SUPPORTED_LINK_FREQS
+};
+
struct ov7251 {
struct i2c_client *i2c_client;
struct device *dev;
@@ -75,6 +80,7 @@ struct ov7251 {
struct regulator *core_regulator;
struct regulator *analog_regulator;
+ enum supported_link_freqs link_freq_idx;
const struct ov7251_mode_info *current_mode;
struct v4l2_ctrl_handler ctrls;
@@ -1255,10 +1261,60 @@ static const struct v4l2_subdev_ops ov7251_subdev_ops = {
.pad = &ov7251_subdev_pad_ops,
};
+static int ov7251_check_hwcfg(struct ov7251 *ov7251)
+{
+ struct fwnode_handle *fwnode = dev_fwnode(ov7251->dev);
+ struct v4l2_fwnode_endpoint bus_cfg = {
+ .bus_type = V4L2_MBUS_CSI2_DPHY,
+ };
+ struct fwnode_handle *endpoint;
+ unsigned int i, j;
+ int ret;
+
+ endpoint = fwnode_graph_get_next_endpoint(fwnode, NULL);
+ if (!endpoint)
+ return -EPROBE_DEFER; /* could be provided by cio2-bridge */
+
+ ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg);
+ fwnode_handle_put(endpoint);
+ if (ret)
+ return dev_err_probe(ov7251->dev, ret,
+ "parsing endpoint node failed\n");
+
+ if (!bus_cfg.nr_of_link_frequencies) {
+ ret = -EINVAL;
+ dev_err_probe(ov7251->dev, ret,
+ "no link frequencies defined\n");
+ goto out_free_bus_cfg;
+ }
+
+ for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++) {
+ for (j = 0; j < ARRAY_SIZE(link_freq); j++)
+ if (bus_cfg.link_frequencies[i] == link_freq[j])
+ break;
+
+ if (j < ARRAY_SIZE(link_freq))
+ break;
+ }
+
+ if (i == bus_cfg.nr_of_link_frequencies) {
+ ret = -EINVAL;
+ dev_err_probe(ov7251->dev, ret,
+ "no supported link freq found\n");
+ goto out_free_bus_cfg;
+ }
+
+ ov7251->link_freq_idx = i;
+
+out_free_bus_cfg:
+ v4l2_fwnode_endpoint_free(&bus_cfg);
+
+ return ret;
+}
+
static int ov7251_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
- struct fwnode_handle *endpoint;
struct ov7251 *ov7251;
u8 chip_id_high, chip_id_low, chip_rev;
int ret;
@@ -1270,24 +1326,9 @@ static int ov7251_probe(struct i2c_client *client)
ov7251->i2c_client = client;
ov7251->dev = dev;
- endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
- if (!endpoint) {
- dev_err(dev, "endpoint node not found\n");
- return -EINVAL;
- }
-
- ret = v4l2_fwnode_endpoint_parse(endpoint, &ov7251->ep);
- fwnode_handle_put(endpoint);
- if (ret < 0) {
- dev_err(dev, "parsing endpoint node failed\n");
+ ret = ov7251_check_hwcfg(ov7251);
+ if (ret)
return ret;
- }
-
- if (ov7251->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
- dev_err(dev, "invalid bus type (%u), must be CSI2 (%u)\n",
- ov7251->ep.bus_type, V4L2_MBUS_CSI2_DPHY);
- return -EINVAL;
- }
/* get system clock (xclk) */
ov7251->xclk = devm_clk_get(dev, "xclk");
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 05/15] media: i2c: Remove per-mode frequencies from ov7251
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (3 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 04/15] media: i2c: Provide ov7251_check_hwcfg() Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 06/15] media: i2c: Add ov7251_pll_configure() Daniel Scally
` (9 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Each of the defined modes in the ov7251 driver uses the same link
frequency and pixel rate; just drop those members of the modes and
set the controls to read only during initialisation. Add a new
table defining the supported pixel rates to substitue for the values
hardcoded in the modes.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- New patch
drivers/media/i2c/ov7251.c | 43 +++++++++++++-------------------------
1 file changed, 14 insertions(+), 29 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index f1965c8adbd7..f21119064b2d 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -526,7 +526,11 @@ static const struct reg_value ov7251_setting_vga_90fps[] = {
};
static const s64 link_freq[] = {
- 240000000,
+ [OV7251_LINK_FREQ_240_MHZ] = 240000000,
+};
+
+static const s64 pixel_rates[] = {
+ [OV7251_LINK_FREQ_240_MHZ] = 48000000,
};
static const struct ov7251_mode_info ov7251_mode_info_data[] = {
@@ -535,8 +539,6 @@ static const struct ov7251_mode_info ov7251_mode_info_data[] = {
.height = 480,
.data = ov7251_setting_vga_30fps,
.data_size = ARRAY_SIZE(ov7251_setting_vga_30fps),
- .pixel_clock = 48000000,
- .link_freq = 0, /* an index in link_freq[] */
.exposure_max = 1704,
.exposure_def = 504,
.timeperframe = {
@@ -549,8 +551,6 @@ static const struct ov7251_mode_info ov7251_mode_info_data[] = {
.height = 480,
.data = ov7251_setting_vga_60fps,
.data_size = ARRAY_SIZE(ov7251_setting_vga_60fps),
- .pixel_clock = 48000000,
- .link_freq = 0, /* an index in link_freq[] */
.exposure_max = 840,
.exposure_def = 504,
.timeperframe = {
@@ -563,8 +563,6 @@ static const struct ov7251_mode_info ov7251_mode_info_data[] = {
.height = 480,
.data = ov7251_setting_vga_90fps,
.data_size = ARRAY_SIZE(ov7251_setting_vga_90fps),
- .pixel_clock = 48000000,
- .link_freq = 0, /* an index in link_freq[] */
.exposure_max = 552,
.exposure_def = 504,
.timeperframe = {
@@ -1059,16 +1057,6 @@ static int ov7251_set_format(struct v4l2_subdev *sd,
__crop->height = new_mode->height;
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
- ret = __v4l2_ctrl_s_ctrl_int64(ov7251->pixel_clock,
- new_mode->pixel_clock);
- if (ret < 0)
- goto exit;
-
- ret = __v4l2_ctrl_s_ctrl(ov7251->link_freq,
- new_mode->link_freq);
- if (ret < 0)
- goto exit;
-
ret = __v4l2_ctrl_modify_range(ov7251->exposure,
1, new_mode->exposure_max,
1, new_mode->exposure_def);
@@ -1199,16 +1187,6 @@ static int ov7251_set_frame_interval(struct v4l2_subdev *subdev,
new_mode = ov7251_find_mode_by_ival(ov7251, &fi->interval);
if (new_mode != ov7251->current_mode) {
- ret = __v4l2_ctrl_s_ctrl_int64(ov7251->pixel_clock,
- new_mode->pixel_clock);
- if (ret < 0)
- goto exit;
-
- ret = __v4l2_ctrl_s_ctrl(ov7251->link_freq,
- new_mode->link_freq);
- if (ret < 0)
- goto exit;
-
ret = __v4l2_ctrl_modify_range(ov7251->exposure,
1, new_mode->exposure_max,
1, new_mode->exposure_def);
@@ -1317,6 +1295,7 @@ static int ov7251_probe(struct i2c_client *client)
struct device *dev = &client->dev;
struct ov7251 *ov7251;
u8 chip_id_high, chip_id_low, chip_rev;
+ s64 pixel_rate;
int ret;
ov7251 = devm_kzalloc(dev, sizeof(struct ov7251), GFP_KERNEL);
@@ -1398,17 +1377,23 @@ static int ov7251_probe(struct i2c_client *client)
V4L2_CID_TEST_PATTERN,
ARRAY_SIZE(ov7251_test_pattern_menu) - 1,
0, 0, ov7251_test_pattern_menu);
+
+ pixel_rate = pixel_rates[ov7251->link_freq_idx];
ov7251->pixel_clock = v4l2_ctrl_new_std(&ov7251->ctrls,
&ov7251_ctrl_ops,
V4L2_CID_PIXEL_RATE,
- 1, INT_MAX, 1, 1);
+ pixel_rate, INT_MAX,
+ pixel_rate, pixel_rate);
ov7251->link_freq = v4l2_ctrl_new_int_menu(&ov7251->ctrls,
&ov7251_ctrl_ops,
V4L2_CID_LINK_FREQ,
ARRAY_SIZE(link_freq) - 1,
- 0, link_freq);
+ ov7251->link_freq_idx,
+ link_freq);
if (ov7251->link_freq)
ov7251->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ if (ov7251->pixel_clock)
+ ov7251->pixel_clock->flags |= V4L2_CTRL_FLAG_READ_ONLY;
ov7251->sd.ctrl_handler = &ov7251->ctrls;
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 06/15] media: i2c: Add ov7251_pll_configure()
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (4 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 05/15] media: i2c: Remove per-mode frequencies from ov7251 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 07/15] media: i2c: Add support for new frequencies to ov7251 Daniel Scally
` (8 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Rather than having the pll settings hidden inside mode blobs, define
them in structs and use a dedicated function to set them. This makes
it simpler to extend the driver to support other frequencies for both
the external clock and desired link frequency.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- Added commas to last items in arrays (Andy)
Changes in v2:
- Updated to support different link-frequencies in addition to different
external clock frequencies.
drivers/media/i2c/ov7251.c | 175 ++++++++++++++++++++++++++++++-------
1 file changed, 145 insertions(+), 30 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index f21119064b2d..3440077e8ba9 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -42,6 +42,16 @@
#define OV7251_TIMING_FORMAT2_MIRROR BIT(2)
#define OV7251_PRE_ISP_00 0x5e00
#define OV7251_PRE_ISP_00_TEST_PATTERN BIT(7)
+#define OV7251_PLL1_PRE_DIV_REG 0x30b4
+#define OV7251_PLL1_MULT_REG 0x30b3
+#define OV7251_PLL1_DIVIDER_REG 0x30b1
+#define OV7251_PLL1_PIX_DIV_REG 0x30b0
+#define OV7251_PLL1_MIPI_DIV_REG 0x30b5
+#define OV7251_PLL2_PRE_DIV_REG 0x3098
+#define OV7251_PLL2_MULT_REG 0x3099
+#define OV7251_PLL2_DIVIDER_REG 0x309d
+#define OV7251_PLL2_SYS_DIV_REG 0x309a
+#define OV7251_PLL2_ADC_DIV_REG 0x309b
struct reg_value {
u16 reg;
@@ -60,6 +70,36 @@ struct ov7251_mode_info {
struct v4l2_fract timeperframe;
};
+struct ov7251_pll1_cfg {
+ unsigned int pre_div;
+ unsigned int mult;
+ unsigned int div;
+ unsigned int pix_div;
+ unsigned int mipi_div;
+};
+
+struct ov7251_pll2_cfg {
+ unsigned int pre_div;
+ unsigned int mult;
+ unsigned int div;
+ unsigned int sys_div;
+ unsigned int adc_div;
+};
+
+/*
+ * Rubbish ordering, but only PLL1 needs to have a separate configuration per
+ * link frequency and the array member needs to be last.
+ */
+struct ov7251_pll_cfgs {
+ const struct ov7251_pll2_cfg *pll2;
+ const struct ov7251_pll1_cfg *pll1[];
+};
+
+enum xclk_rate {
+ OV7251_24_MHZ,
+ OV7251_NUM_SUPPORTED_RATES
+};
+
enum supported_link_freqs {
OV7251_LINK_FREQ_240_MHZ,
OV7251_NUM_SUPPORTED_LINK_FREQS
@@ -80,6 +120,7 @@ struct ov7251 {
struct regulator *core_regulator;
struct regulator *analog_regulator;
+ const struct ov7251_pll_cfgs *pll_cfgs;
enum supported_link_freqs link_freq_idx;
const struct ov7251_mode_info *current_mode;
@@ -106,6 +147,33 @@ static inline struct ov7251 *to_ov7251(struct v4l2_subdev *sd)
return container_of(sd, struct ov7251, sd);
}
+static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_240_mhz = {
+ .pre_div = 0x03,
+ .mult = 0x64,
+ .div = 0x01,
+ .pix_div = 0x0a,
+ .mipi_div = 0x05,
+};
+
+static const struct ov7251_pll2_cfg ov7251_pll2_cfg_24_mhz = {
+ .pre_div = 0x04,
+ .mult = 0x28,
+ .div = 0x00,
+ .sys_div = 0x05,
+ .adc_div = 0x04,
+};
+
+static const struct ov7251_pll_cfgs ov7251_pll_cfgs_24_mhz = {
+ .pll2 = &ov7251_pll2_cfg_24_mhz,
+ .pll1 = {
+ [OV7251_LINK_FREQ_240_MHZ] = &ov7251_pll1_cfg_24_mhz_240_mhz,
+ },
+};
+
+static const struct ov7251_pll_cfgs *ov7251_pll_cfgs[] = {
+ [OV7251_24_MHZ] = &ov7251_pll_cfgs_24_mhz,
+};
+
static const struct reg_value ov7251_global_init_setting[] = {
{ 0x0103, 0x01 },
{ 0x303b, 0x02 },
@@ -124,16 +192,6 @@ static const struct reg_value ov7251_setting_vga_30fps[] = {
{ 0x301c, 0xf0 },
{ 0x3023, 0x05 },
{ 0x3037, 0xf0 },
- { 0x3098, 0x04 }, /* pll2 pre divider */
- { 0x3099, 0x28 }, /* pll2 multiplier */
- { 0x309a, 0x05 }, /* pll2 sys divider */
- { 0x309b, 0x04 }, /* pll2 adc divider */
- { 0x309d, 0x00 }, /* pll2 divider */
- { 0x30b0, 0x0a }, /* pll1 pix divider */
- { 0x30b1, 0x01 }, /* pll1 divider */
- { 0x30b3, 0x64 }, /* pll1 multiplier */
- { 0x30b4, 0x03 }, /* pll1 pre divider */
- { 0x30b5, 0x05 }, /* pll1 mipi divider */
{ 0x3106, 0xda },
{ 0x3503, 0x07 },
{ 0x3509, 0x10 },
@@ -262,16 +320,6 @@ static const struct reg_value ov7251_setting_vga_60fps[] = {
{ 0x301c, 0x00 },
{ 0x3023, 0x05 },
{ 0x3037, 0xf0 },
- { 0x3098, 0x04 }, /* pll2 pre divider */
- { 0x3099, 0x28 }, /* pll2 multiplier */
- { 0x309a, 0x05 }, /* pll2 sys divider */
- { 0x309b, 0x04 }, /* pll2 adc divider */
- { 0x309d, 0x00 }, /* pll2 divider */
- { 0x30b0, 0x0a }, /* pll1 pix divider */
- { 0x30b1, 0x01 }, /* pll1 divider */
- { 0x30b3, 0x64 }, /* pll1 multiplier */
- { 0x30b4, 0x03 }, /* pll1 pre divider */
- { 0x30b5, 0x05 }, /* pll1 mipi divider */
{ 0x3106, 0xda },
{ 0x3503, 0x07 },
{ 0x3509, 0x10 },
@@ -400,16 +448,6 @@ static const struct reg_value ov7251_setting_vga_90fps[] = {
{ 0x301c, 0x00 },
{ 0x3023, 0x05 },
{ 0x3037, 0xf0 },
- { 0x3098, 0x04 }, /* pll2 pre divider */
- { 0x3099, 0x28 }, /* pll2 multiplier */
- { 0x309a, 0x05 }, /* pll2 sys divider */
- { 0x309b, 0x04 }, /* pll2 adc divider */
- { 0x309d, 0x00 }, /* pll2 divider */
- { 0x30b0, 0x0a }, /* pll1 pix divider */
- { 0x30b1, 0x01 }, /* pll1 divider */
- { 0x30b3, 0x64 }, /* pll1 multiplier */
- { 0x30b4, 0x03 }, /* pll1 pre divider */
- { 0x30b5, 0x05 }, /* pll1 mipi divider */
{ 0x3106, 0xda },
{ 0x3503, 0x07 },
{ 0x3509, 0x10 },
@@ -525,6 +563,10 @@ static const struct reg_value ov7251_setting_vga_90fps[] = {
{ 0x5001, 0x80 },
};
+static const unsigned long supported_xclk_rates[] = {
+ [OV7251_24_MHZ] = 24000000,
+};
+
static const s64 link_freq[] = {
[OV7251_LINK_FREQ_240_MHZ] = 240000000,
};
@@ -696,6 +738,63 @@ static int ov7251_read_reg(struct ov7251 *ov7251, u16 reg, u8 *val)
return 0;
}
+static int ov7251_pll_configure(struct ov7251 *ov7251)
+{
+ const struct ov7251_pll_cfgs *configs;
+ int ret;
+
+ configs = ov7251->pll_cfgs;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL1_PRE_DIV_REG,
+ configs->pll1[ov7251->link_freq_idx]->pre_div);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL1_MULT_REG,
+ configs->pll1[ov7251->link_freq_idx]->mult);
+ if (ret < 0)
+ return ret;
+ ret = ov7251_write_reg(ov7251, OV7251_PLL1_DIVIDER_REG,
+ configs->pll1[ov7251->link_freq_idx]->div);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL1_PIX_DIV_REG,
+ configs->pll1[ov7251->link_freq_idx]->pix_div);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL1_MIPI_DIV_REG,
+ configs->pll1[ov7251->link_freq_idx]->mipi_div);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL2_PRE_DIV_REG,
+ configs->pll2->pre_div);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL2_MULT_REG,
+ configs->pll2->mult);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL2_DIVIDER_REG,
+ configs->pll2->div);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL2_SYS_DIV_REG,
+ configs->pll2->sys_div);
+ if (ret < 0)
+ return ret;
+
+ ret = ov7251_write_reg(ov7251, OV7251_PLL2_ADC_DIV_REG,
+ configs->pll2->adc_div);
+
+ return ret;
+}
+
static int ov7251_set_exposure(struct ov7251 *ov7251, s32 exposure)
{
u16 reg;
@@ -1137,6 +1236,11 @@ static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
mutex_lock(&ov7251->lock);
if (enable) {
+ ret = ov7251_pll_configure(ov7251);
+ if (ret)
+ return dev_err_probe(ov7251->dev, ret,
+ "error configuring PLLs\n");
+
ret = ov7251_set_register_array(ov7251,
ov7251->current_mode->data,
ov7251->current_mode->data_size);
@@ -1297,6 +1401,7 @@ static int ov7251_probe(struct i2c_client *client)
u8 chip_id_high, chip_id_low, chip_rev;
s64 pixel_rate;
int ret;
+ int i;
ov7251 = devm_kzalloc(dev, sizeof(struct ov7251), GFP_KERNEL);
if (!ov7251)
@@ -1335,6 +1440,16 @@ static int ov7251_probe(struct i2c_client *client)
dev_err(dev, "could not set xclk frequency\n");
return ret;
}
+ for (i = 0; i < ARRAY_SIZE(supported_xclk_rates); i++)
+ if (ov7251->xclk_freq == supported_xclk_rates[i])
+ break;
+
+ if (i == ARRAY_SIZE(supported_xclk_rates))
+ return dev_err_probe(dev, -EINVAL,
+ "clock rate %u Hz is unsupported\n",
+ ov7251->xclk_freq);
+
+ ov7251->pll_cfgs = ov7251_pll_cfgs[i];
ov7251->io_regulator = devm_regulator_get(dev, "vdddo");
if (IS_ERR(ov7251->io_regulator)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 07/15] media: i2c: Add support for new frequencies to ov7251
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (5 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 06/15] media: i2c: Add ov7251_pll_configure() Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-05 12:49 ` kernel test robot
2022-05-04 22:30 ` [PATCH v3 08/15] media: i2c: Add ov7251_detect_chip() Daniel Scally
` (7 subsequent siblings)
14 siblings, 1 reply; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
The OV7251 sensor is used as the IR camera sensor on the Microsoft
Surface line of tablets; this provides a 19.2MHz external clock, and
the Windows driver for this sensor configures a 319.2MHz link freq to
the CSI-2 receiver. Add the ability to support those rate to the
driver by defining a new set of PLL configs.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- Added commas to last items in arrays (Andy)
Changes in v2:
- Added support for 319.2MHz link frequency
drivers/media/i2c/ov7251.c | 93 +++++++++++++++++++++++++++++---------
1 file changed, 72 insertions(+), 21 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index 3440077e8ba9..9a026354a1bd 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -96,12 +96,14 @@ struct ov7251_pll_cfgs {
};
enum xclk_rate {
+ OV7251_19_2_MHZ,
OV7251_24_MHZ,
OV7251_NUM_SUPPORTED_RATES
};
enum supported_link_freqs {
OV7251_LINK_FREQ_240_MHZ,
+ OV7251_LINK_FREQ_319_2_MHZ,
OV7251_NUM_SUPPORTED_LINK_FREQS
};
@@ -147,6 +149,22 @@ static inline struct ov7251 *to_ov7251(struct v4l2_subdev *sd)
return container_of(sd, struct ov7251, sd);
}
+static const struct ov7251_pll1_cfg ov7251_pll1_cfg_19_2_mhz_240_mhz = {
+ .pre_div = 0x03,
+ .mult = 0x4b,
+ .div = 0x01,
+ .pix_div = 0x0a,
+ .mipi_div = 0x05,
+};
+
+static const struct ov7251_pll1_cfg ov7251_pll1_cfg_19_2_mhz_319_2_mhz = {
+ .pre_div = 0x01,
+ .mult = 0x85,
+ .div = 0x04,
+ .pix_div = 0x0a,
+ .mipi_div = 0x05,
+};
+
static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_240_mhz = {
.pre_div = 0x03,
.mult = 0x64,
@@ -155,6 +173,22 @@ static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_240_mhz = {
.mipi_div = 0x05,
};
+static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_319_2_mhz = {
+ .pre_div = 0x05,
+ .mult = 0x85,
+ .div = 0x02,
+ .pix_div = 0x0a,
+ .mipi_div = 0x05,
+};
+
+static const struct ov7251_pll2_cfg ov7251_pll2_cfg_19_2_mhz = {
+ .pre_div = 0x04,
+ .mult = 0x32,
+ .div = 0x00,
+ .sys_div = 0x05,
+ .adc_div = 0x04,
+};
+
static const struct ov7251_pll2_cfg ov7251_pll2_cfg_24_mhz = {
.pre_div = 0x04,
.mult = 0x28,
@@ -163,14 +197,24 @@ static const struct ov7251_pll2_cfg ov7251_pll2_cfg_24_mhz = {
.adc_div = 0x04,
};
+static const struct ov7251_pll_cfgs ov7251_pll_cfgs_19_2_mhz = {
+ .pll2 = &ov7251_pll2_cfg_19_2_mhz,
+ .pll1 = {
+ [OV7251_LINK_FREQ_240_MHZ] = &ov7251_pll1_cfg_19_2_mhz_240_mhz,
+ [OV7251_LINK_FREQ_319_2_MHZ] = &ov7251_pll1_cfg_19_2_mhz_319_2_mhz,
+ },
+};
+
static const struct ov7251_pll_cfgs ov7251_pll_cfgs_24_mhz = {
.pll2 = &ov7251_pll2_cfg_24_mhz,
.pll1 = {
[OV7251_LINK_FREQ_240_MHZ] = &ov7251_pll1_cfg_24_mhz_240_mhz,
+ [OV7251_LINK_FREQ_319_2_MHZ] = &ov7251_pll1_cfg_19_2_mhz_319_2_mhz,
},
};
static const struct ov7251_pll_cfgs *ov7251_pll_cfgs[] = {
+ [OV7251_19_2_MHZ] = &ov7251_pll_cfgs_19_2_mhz,
[OV7251_24_MHZ] = &ov7251_pll_cfgs_24_mhz,
};
@@ -564,15 +608,18 @@ static const struct reg_value ov7251_setting_vga_90fps[] = {
};
static const unsigned long supported_xclk_rates[] = {
+ [OV7251_19_2_MHZ] = 19200000,
[OV7251_24_MHZ] = 24000000,
};
static const s64 link_freq[] = {
[OV7251_LINK_FREQ_240_MHZ] = 240000000,
+ [OV7251_LINK_FREQ_319_2_MHZ] = 319200000,
};
static const s64 pixel_rates[] = {
[OV7251_LINK_FREQ_240_MHZ] = 48000000,
+ [OV7251_LINK_FREQ_319_2_MHZ] = 63840000,
};
static const struct ov7251_mode_info ov7251_mode_info_data[] = {
@@ -1399,6 +1446,7 @@ static int ov7251_probe(struct i2c_client *client)
struct device *dev = &client->dev;
struct ov7251 *ov7251;
u8 chip_id_high, chip_id_low, chip_rev;
+ unsigned int rate = 0, clk_rate = 0;
s64 pixel_rate;
int ret;
int i;
@@ -1415,31 +1463,34 @@ static int ov7251_probe(struct i2c_client *client)
return ret;
/* get system clock (xclk) */
- ov7251->xclk = devm_clk_get(dev, "xclk");
- if (IS_ERR(ov7251->xclk)) {
- dev_err(dev, "could not get xclk");
- return PTR_ERR(ov7251->xclk);
- }
-
+ ov7251->xclk = devm_clk_get_optional(dev, NULL);
+ if (IS_ERR(ov7251->xclk))
+ return dev_err_probe(dev, PTR_ERR(ov7251->xclk),
+ "could not get xclk");
+
+ /*
+ * We could have either a 24MHz or 19.2MHz clock rate from either DT or
+ * ACPI. We also need to support the IPU3 case which will have both an
+ * external clock AND a clock-frequency property.
+ */
ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
- &ov7251->xclk_freq);
- if (ret) {
- dev_err(dev, "could not get xclk frequency\n");
- return ret;
- }
+ &rate);
+ if (ret && !ov7251->xclk)
+ return dev_err_probe(dev, ret, "invalid clock config\n");
- /* external clock must be 24MHz, allow 1% tolerance */
- if (ov7251->xclk_freq < 23760000 || ov7251->xclk_freq > 24240000) {
- dev_err(dev, "external clock frequency %u is not supported\n",
- ov7251->xclk_freq);
- return -EINVAL;
- }
+ clk_rate = clk_get_rate(ov7251->xclk);
+ ov7251->xclk_freq = clk_rate ? clk_rate : rate;
- ret = clk_set_rate(ov7251->xclk, ov7251->xclk_freq);
- if (ret) {
- dev_err(dev, "could not set xclk frequency\n");
- return ret;
+ if (ov7251->xclk_freq == 0)
+ return dev_err_probe(dev, -EINVAL, "invalid clock frequency\n");
+
+ if (!ret && ov7251->xclk) {
+ ret = clk_set_rate(ov7251->xclk, rate);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to set clock rate\n");
}
+
for (i = 0; i < ARRAY_SIZE(supported_xclk_rates); i++)
if (ov7251->xclk_freq == supported_xclk_rates[i])
break;
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 08/15] media: i2c: Add ov7251_detect_chip()
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (6 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 07/15] media: i2c: Add support for new frequencies to ov7251 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 09/15] media: i2c: Add pm_runtime support to ov7251 Daniel Scally
` (6 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
.probe() is quite busy for this driver; make it cleaner by moving the
chip verification to a dedicated function.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- None
drivers/media/i2c/ov7251.c | 62 +++++++++++++++++++++-----------------
1 file changed, 35 insertions(+), 27 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index 9a026354a1bd..ff31629839f9 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -1441,11 +1441,43 @@ static int ov7251_check_hwcfg(struct ov7251 *ov7251)
return ret;
}
+static int ov7251_detect_chip(struct ov7251 *ov7251)
+{
+ u8 chip_id_high, chip_id_low, chip_rev;
+ int ret;
+
+ ret = ov7251_read_reg(ov7251, OV7251_CHIP_ID_HIGH, &chip_id_high);
+ if (ret < 0 || chip_id_high != OV7251_CHIP_ID_HIGH_BYTE)
+ return dev_err_probe(ov7251->dev, -ENODEV,
+ "could not read ID high\n");
+
+ ret = ov7251_read_reg(ov7251, OV7251_CHIP_ID_LOW, &chip_id_low);
+ if (ret < 0 || chip_id_low != OV7251_CHIP_ID_LOW_BYTE)
+ return dev_err_probe(ov7251->dev, -ENODEV,
+ "could not read ID low\n");
+
+ ret = ov7251_read_reg(ov7251, OV7251_SC_GP_IO_IN1, &chip_rev);
+ if (ret < 0)
+ return dev_err_probe(ov7251->dev, -ENODEV,
+ "could not read revision\n");
+ chip_rev >>= 4;
+
+ dev_info(ov7251->dev,
+ "OV7251 revision %x (%s) detected at address 0x%02x\n",
+ chip_rev,
+ chip_rev == 0x4 ? "1A / 1B" :
+ chip_rev == 0x5 ? "1C / 1D" :
+ chip_rev == 0x6 ? "1E" :
+ chip_rev == 0x7 ? "1F" : "unknown",
+ ov7251->i2c_client->addr);
+
+ return 0;
+}
+
static int ov7251_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct ov7251 *ov7251;
- u8 chip_id_high, chip_id_low, chip_rev;
unsigned int rate = 0, clk_rate = 0;
s64 pixel_rate;
int ret;
@@ -1588,34 +1620,10 @@ static int ov7251_probe(struct i2c_client *client)
goto free_entity;
}
- ret = ov7251_read_reg(ov7251, OV7251_CHIP_ID_HIGH, &chip_id_high);
- if (ret < 0 || chip_id_high != OV7251_CHIP_ID_HIGH_BYTE) {
- dev_err(dev, "could not read ID high\n");
- ret = -ENODEV;
- goto power_down;
- }
- ret = ov7251_read_reg(ov7251, OV7251_CHIP_ID_LOW, &chip_id_low);
- if (ret < 0 || chip_id_low != OV7251_CHIP_ID_LOW_BYTE) {
- dev_err(dev, "could not read ID low\n");
- ret = -ENODEV;
- goto power_down;
- }
-
- ret = ov7251_read_reg(ov7251, OV7251_SC_GP_IO_IN1, &chip_rev);
- if (ret < 0) {
- dev_err(dev, "could not read revision\n");
- ret = -ENODEV;
+ ret = ov7251_detect_chip(ov7251);
+ if (ret)
goto power_down;
- }
- chip_rev >>= 4;
- dev_info(dev, "OV7251 revision %x (%s) detected at address 0x%02x\n",
- chip_rev,
- chip_rev == 0x4 ? "1A / 1B" :
- chip_rev == 0x5 ? "1C / 1D" :
- chip_rev == 0x6 ? "1E" :
- chip_rev == 0x7 ? "1F" : "unknown",
- client->addr);
ret = ov7251_read_reg(ov7251, OV7251_PRE_ISP_00,
&ov7251->pre_isp_00);
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 09/15] media: i2c: Add pm_runtime support to ov7251
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (7 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 08/15] media: i2c: Add ov7251_detect_chip() Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 10/15] media: i2c: Remove .s_power() from ov7251 Daniel Scally
` (5 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Add pm_runtime support to the ov7251 driver.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- Switched ov7251_set_power_[on|off]() to take a struct device * as the
input parameter and used those functions for pm_runtime instead of
adding wrappers (Sakari)
drivers/media/i2c/ov7251.c | 81 ++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index ff31629839f9..a9e890181200 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -15,6 +15,7 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
+#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -883,8 +884,11 @@ static int ov7251_set_register_array(struct ov7251 *ov7251,
return 0;
}
-static int ov7251_set_power_on(struct ov7251 *ov7251)
+static int ov7251_set_power_on(struct device *dev)
{
+ struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct ov7251 *ov7251 = to_ov7251(sd);
int ret;
u32 wait_us;
@@ -909,11 +913,17 @@ static int ov7251_set_power_on(struct ov7251 *ov7251)
return 0;
}
-static void ov7251_set_power_off(struct ov7251 *ov7251)
+static int ov7251_set_power_off(struct device *dev)
{
+ struct i2c_client *client = container_of(dev, struct i2c_client, dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct ov7251 *ov7251 = to_ov7251(sd);
+
clk_disable_unprepare(ov7251->xclk);
gpiod_set_value_cansleep(ov7251->enable_gpio, 0);
ov7251_regulators_disable(ov7251);
+
+ return 0;
}
static int ov7251_s_power(struct v4l2_subdev *sd, int on)
@@ -928,7 +938,7 @@ static int ov7251_s_power(struct v4l2_subdev *sd, int on)
goto exit;
if (on) {
- ret = ov7251_set_power_on(ov7251);
+ ret = ov7251_set_power_on(ov7251->dev);
if (ret < 0)
goto exit;
@@ -937,13 +947,13 @@ static int ov7251_s_power(struct v4l2_subdev *sd, int on)
ARRAY_SIZE(ov7251_global_init_setting));
if (ret < 0) {
dev_err(ov7251->dev, "could not set init registers\n");
- ov7251_set_power_off(ov7251);
+ ov7251_set_power_off(ov7251->dev);
goto exit;
}
ov7251->power_on = true;
} else {
- ov7251_set_power_off(ov7251);
+ ov7251_set_power_off(ov7251->dev);
ov7251->power_on = false;
}
@@ -1017,7 +1027,7 @@ static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
/* v4l2_ctrl_lock() locks our mutex */
- if (!ov7251->power_on)
+ if (!pm_runtime_get_if_in_use(ov7251->dev))
return 0;
switch (ctrl->id) {
@@ -1041,6 +1051,8 @@ static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
break;
}
+ pm_runtime_put(ov7251->dev);
+
return ret;
}
@@ -1283,10 +1295,15 @@ static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
mutex_lock(&ov7251->lock);
if (enable) {
+ ret = pm_runtime_get_sync(ov7251->dev);
+ if (ret < 0)
+ goto unlock_out;
+
ret = ov7251_pll_configure(ov7251);
- if (ret)
- return dev_err_probe(ov7251->dev, ret,
- "error configuring PLLs\n");
+ if (ret) {
+ dev_err(ov7251->dev, "error configuring PLLs\n");
+ goto err_power_down;
+ }
ret = ov7251_set_register_array(ov7251,
ov7251->current_mode->data,
@@ -1295,23 +1312,29 @@ static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
dev_err(ov7251->dev, "could not set mode %dx%d\n",
ov7251->current_mode->width,
ov7251->current_mode->height);
- goto exit;
+ goto err_power_down;
}
ret = __v4l2_ctrl_handler_setup(&ov7251->ctrls);
if (ret < 0) {
dev_err(ov7251->dev, "could not sync v4l2 controls\n");
- goto exit;
+ goto err_power_down;
}
ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
OV7251_SC_MODE_SELECT_STREAMING);
+ if (ret)
+ goto err_power_down;
} else {
ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
OV7251_SC_MODE_SELECT_SW_STANDBY);
+ pm_runtime_put(ov7251->dev);
}
-exit:
+unlock_out:
mutex_unlock(&ov7251->lock);
+ return ret;
+err_power_down:
+ pm_runtime_put_noidle(ov7251->dev);
return ret;
}
@@ -1614,23 +1637,24 @@ static int ov7251_probe(struct i2c_client *client)
goto free_ctrl;
}
- ret = ov7251_s_power(&ov7251->sd, true);
- if (ret < 0) {
- dev_err(dev, "could not power up OV7251\n");
+ ret = ov7251_set_power_on(ov7251->dev);
+ if (ret)
goto free_entity;
- }
ret = ov7251_detect_chip(ov7251);
if (ret)
goto power_down;
+ pm_runtime_set_active(&client->dev);
+ pm_runtime_get_noresume(&client->dev);
+ pm_runtime_enable(&client->dev);
ret = ov7251_read_reg(ov7251, OV7251_PRE_ISP_00,
&ov7251->pre_isp_00);
if (ret < 0) {
dev_err(dev, "could not read test pattern value\n");
ret = -ENODEV;
- goto power_down;
+ goto err_pm_runtime;
}
ret = ov7251_read_reg(ov7251, OV7251_TIMING_FORMAT1,
@@ -1638,7 +1662,7 @@ static int ov7251_probe(struct i2c_client *client)
if (ret < 0) {
dev_err(dev, "could not read vflip value\n");
ret = -ENODEV;
- goto power_down;
+ goto err_pm_runtime;
}
ret = ov7251_read_reg(ov7251, OV7251_TIMING_FORMAT2,
@@ -1646,10 +1670,12 @@ static int ov7251_probe(struct i2c_client *client)
if (ret < 0) {
dev_err(dev, "could not read hflip value\n");
ret = -ENODEV;
- goto power_down;
+ goto err_pm_runtime;
}
- ov7251_s_power(&ov7251->sd, false);
+ pm_runtime_set_autosuspend_delay(&client->dev, 1000);
+ pm_runtime_use_autosuspend(&client->dev);
+ pm_runtime_put_autosuspend(&client->dev);
ret = v4l2_async_register_subdev(&ov7251->sd);
if (ret < 0) {
@@ -1661,8 +1687,11 @@ static int ov7251_probe(struct i2c_client *client)
return 0;
+err_pm_runtime:
+ pm_runtime_disable(ov7251->dev);
+ pm_runtime_put_noidle(ov7251->dev);
power_down:
- ov7251_s_power(&ov7251->sd, false);
+ ov7251_set_power_off(ov7251->dev);
free_entity:
media_entity_cleanup(&ov7251->sd.entity);
free_ctrl:
@@ -1682,9 +1711,18 @@ static int ov7251_remove(struct i2c_client *client)
v4l2_ctrl_handler_free(&ov7251->ctrls);
mutex_destroy(&ov7251->lock);
+ pm_runtime_disable(ov7251->dev);
+ if (!pm_runtime_status_suspended(ov7251->dev))
+ ov7251_set_power_off(ov7251->dev);
+ pm_runtime_set_suspended(ov7251->dev);
+
return 0;
}
+static const struct dev_pm_ops ov7251_pm_ops = {
+ SET_RUNTIME_PM_OPS(ov7251_set_power_off, ov7251_set_power_on, NULL)
+};
+
static const struct of_device_id ov7251_of_match[] = {
{ .compatible = "ovti,ov7251" },
{ /* sentinel */ }
@@ -1702,6 +1740,7 @@ static struct i2c_driver ov7251_i2c_driver = {
.of_match_table = ov7251_of_match,
.acpi_match_table = ov7251_acpi_match,
.name = "ov7251",
+ .pm = &ov7251_pm_ops,
},
.probe_new = ov7251_probe,
.remove = ov7251_remove,
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 10/15] media: i2c: Remove .s_power() from ov7251
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (8 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 09/15] media: i2c: Add pm_runtime support to ov7251 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 11/15] media: ipu3-cio2: Add INT347E to cio2-bridge Daniel Scally
` (4 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
The .s_power() callback is deprecated, and now that we have pm_runtime
functionality in the driver there's no further use for it. Delete the
function.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- Set the global init registers as part of ov7251_set_power_on() (Sakari)
drivers/media/i2c/ov7251.c | 53 +++++++-------------------------------
1 file changed, 10 insertions(+), 43 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index a9e890181200..4f8c797839f6 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -910,7 +910,16 @@ static int ov7251_set_power_on(struct device *dev)
DIV_ROUND_UP(ov7251->xclk_freq, 1000));
usleep_range(wait_us, wait_us + 1000);
- return 0;
+ ret = ov7251_set_register_array(ov7251,
+ ov7251_global_init_setting,
+ ARRAY_SIZE(ov7251_global_init_setting));
+ if (ret < 0) {
+ dev_err(ov7251->dev, "error during global init\n");
+ ov7251_regulators_disable(ov7251);
+ return ret;
+ }
+
+ return ret;
}
static int ov7251_set_power_off(struct device *dev)
@@ -926,43 +935,6 @@ static int ov7251_set_power_off(struct device *dev)
return 0;
}
-static int ov7251_s_power(struct v4l2_subdev *sd, int on)
-{
- struct ov7251 *ov7251 = to_ov7251(sd);
- int ret = 0;
-
- mutex_lock(&ov7251->lock);
-
- /* If the power state is not modified - no work to do. */
- if (ov7251->power_on == !!on)
- goto exit;
-
- if (on) {
- ret = ov7251_set_power_on(ov7251->dev);
- if (ret < 0)
- goto exit;
-
- ret = ov7251_set_register_array(ov7251,
- ov7251_global_init_setting,
- ARRAY_SIZE(ov7251_global_init_setting));
- if (ret < 0) {
- dev_err(ov7251->dev, "could not set init registers\n");
- ov7251_set_power_off(ov7251->dev);
- goto exit;
- }
-
- ov7251->power_on = true;
- } else {
- ov7251_set_power_off(ov7251->dev);
- ov7251->power_on = false;
- }
-
-exit:
- mutex_unlock(&ov7251->lock);
-
- return ret;
-}
-
static int ov7251_set_hflip(struct ov7251 *ov7251, s32 value)
{
u8 val = ov7251->timing_format2;
@@ -1387,10 +1359,6 @@ static int ov7251_set_frame_interval(struct v4l2_subdev *subdev,
return ret;
}
-static const struct v4l2_subdev_core_ops ov7251_core_ops = {
- .s_power = ov7251_s_power,
-};
-
static const struct v4l2_subdev_video_ops ov7251_video_ops = {
.s_stream = ov7251_s_stream,
.g_frame_interval = ov7251_get_frame_interval,
@@ -1408,7 +1376,6 @@ static const struct v4l2_subdev_pad_ops ov7251_subdev_pad_ops = {
};
static const struct v4l2_subdev_ops ov7251_subdev_ops = {
- .core = &ov7251_core_ops,
.video = &ov7251_video_ops,
.pad = &ov7251_subdev_pad_ops,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 11/15] media: ipu3-cio2: Add INT347E to cio2-bridge
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (9 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 10/15] media: i2c: Remove .s_power() from ov7251 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 12/15] media: i2c: Extend .get_selection() for ov7251 Daniel Scally
` (3 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
The OVTI7251 sensor can be found on x86 laptops with an IPU3, and so
needs to be supported by the cio2-bridge. Add it to the table of
supported sensors.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- None
Changes in v2:
- Switched to 319.2MHz link frequency
drivers/media/pci/intel/ipu3/cio2-bridge.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
index 7ccb7b6eaa82..df6c94da2f6a 100644
--- a/drivers/media/pci/intel/ipu3/cio2-bridge.c
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
@@ -25,6 +25,8 @@ static const struct cio2_sensor_config cio2_supported_sensors[] = {
CIO2_SENSOR_CONFIG("INT33BE", 1, 419200000),
/* Omnivision OV8865 */
CIO2_SENSOR_CONFIG("INT347A", 1, 360000000),
+ /* Omnivision OV7251 */
+ CIO2_SENSOR_CONFIG("INT347E", 1, 319200000),
/* Omnivision OV2680 */
CIO2_SENSOR_CONFIG("OVTI2680", 0),
};
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 12/15] media: i2c: Extend .get_selection() for ov7251
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (10 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 11/15] media: ipu3-cio2: Add INT347E to cio2-bridge Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 13/15] media: i2c: add ov7251_init_ctrls() Daniel Scally
` (2 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Extend the .get_selection() callback to support other values for
sel->target, primarily to satisfy libcamera's requirements.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- New patch
drivers/media/i2c/ov7251.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index 4f8c797839f6..40e42d19cddd 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -54,6 +54,13 @@
#define OV7251_PLL2_SYS_DIV_REG 0x309a
#define OV7251_PLL2_ADC_DIV_REG 0x309b
+#define OV7251_NATIVE_WIDTH 656
+#define OV7251_NATIVE_HEIGHT 496
+#define OV7251_ACTIVE_START_LEFT 4
+#define OV7251_ACTIVE_START_TOP 4
+#define OV7251_ACTIVE_WIDTH 648
+#define OV7251_ACTIVE_HEIGHT 488
+
struct reg_value {
u16 reg;
u8 val;
@@ -1248,13 +1255,29 @@ static int ov7251_get_selection(struct v4l2_subdev *sd,
{
struct ov7251 *ov7251 = to_ov7251(sd);
- if (sel->target != V4L2_SEL_TGT_CROP)
- return -EINVAL;
-
+ switch (sel->target) {
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP:
mutex_lock(&ov7251->lock);
- sel->r = *__ov7251_get_pad_crop(ov7251, sd_state, sel->pad,
- sel->which);
- mutex_unlock(&ov7251->lock);
+ sel->r = *__ov7251_get_pad_crop(ov7251, sd_state, sel->pad,
+ sel->which);
+ mutex_unlock(&ov7251->lock);
+ break;
+ case V4L2_SEL_TGT_NATIVE_SIZE:
+ sel->r.top = 0;
+ sel->r.left = 0;
+ sel->r.width = OV7251_NATIVE_WIDTH;
+ sel->r.height = OV7251_NATIVE_HEIGHT;
+ break;
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ sel->r.top = OV7251_ACTIVE_START_TOP;
+ sel->r.left = OV7251_ACTIVE_START_LEFT;
+ sel->r.width = OV7251_ACTIVE_WIDTH;
+ sel->r.height = OV7251_ACTIVE_HEIGHT;
+ break;
+ default:
+ return -EINVAL;
+ }
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 13/15] media: i2c: add ov7251_init_ctrls()
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (11 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 12/15] media: i2c: Extend .get_selection() for ov7251 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 14/15] media: i2c: Add hblank control to ov7251 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver Daniel Scally
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
V4L2 controls initialisation takes up a sizeable portion of the
driver's .probe() function. To keep things neat, move it to a
dedicated function.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- New patch
drivers/media/i2c/ov7251.c | 93 +++++++++++++++++++++-----------------
1 file changed, 52 insertions(+), 41 deletions(-)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index 40e42d19cddd..b7d89ad49887 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -1487,12 +1487,58 @@ static int ov7251_detect_chip(struct ov7251 *ov7251)
return 0;
}
+static int ov7251_init_ctrls(struct ov7251 *ov7251)
+{
+ s64 pixel_rate;
+
+ v4l2_ctrl_handler_init(&ov7251->ctrls, 7);
+ ov7251->ctrls.lock = &ov7251->lock;
+
+ v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ ov7251->exposure = v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
+ V4L2_CID_EXPOSURE, 1, 32, 1, 32);
+ ov7251->gain = v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
+ V4L2_CID_GAIN, 16, 1023, 1, 16);
+ v4l2_ctrl_new_std_menu_items(&ov7251->ctrls, &ov7251_ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(ov7251_test_pattern_menu) - 1,
+ 0, 0, ov7251_test_pattern_menu);
+
+ pixel_rate = pixel_rates[ov7251->link_freq_idx];
+ ov7251->pixel_clock = v4l2_ctrl_new_std(&ov7251->ctrls,
+ &ov7251_ctrl_ops,
+ V4L2_CID_PIXEL_RATE,
+ pixel_rate, INT_MAX,
+ pixel_rate, pixel_rate);
+ ov7251->link_freq = v4l2_ctrl_new_int_menu(&ov7251->ctrls,
+ &ov7251_ctrl_ops,
+ V4L2_CID_LINK_FREQ,
+ ARRAY_SIZE(link_freq) - 1,
+ ov7251->link_freq_idx,
+ link_freq);
+ if (ov7251->link_freq)
+ ov7251->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ if (ov7251->pixel_clock)
+ ov7251->pixel_clock->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+ ov7251->sd.ctrl_handler = &ov7251->ctrls;
+
+ if (ov7251->ctrls.error) {
+ v4l2_ctrl_handler_free(&ov7251->ctrls);
+ return ov7251->ctrls.error;
+ }
+
+ return 0;
+}
+
static int ov7251_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct ov7251 *ov7251;
unsigned int rate = 0, clk_rate = 0;
- s64 pixel_rate;
int ret;
int i;
@@ -1573,46 +1619,10 @@ static int ov7251_probe(struct i2c_client *client)
mutex_init(&ov7251->lock);
- v4l2_ctrl_handler_init(&ov7251->ctrls, 7);
- ov7251->ctrls.lock = &ov7251->lock;
-
- v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
- V4L2_CID_HFLIP, 0, 1, 1, 0);
- v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
- V4L2_CID_VFLIP, 0, 1, 1, 0);
- ov7251->exposure = v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
- V4L2_CID_EXPOSURE, 1, 32, 1, 32);
- ov7251->gain = v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
- V4L2_CID_GAIN, 16, 1023, 1, 16);
- v4l2_ctrl_new_std_menu_items(&ov7251->ctrls, &ov7251_ctrl_ops,
- V4L2_CID_TEST_PATTERN,
- ARRAY_SIZE(ov7251_test_pattern_menu) - 1,
- 0, 0, ov7251_test_pattern_menu);
-
- pixel_rate = pixel_rates[ov7251->link_freq_idx];
- ov7251->pixel_clock = v4l2_ctrl_new_std(&ov7251->ctrls,
- &ov7251_ctrl_ops,
- V4L2_CID_PIXEL_RATE,
- pixel_rate, INT_MAX,
- pixel_rate, pixel_rate);
- ov7251->link_freq = v4l2_ctrl_new_int_menu(&ov7251->ctrls,
- &ov7251_ctrl_ops,
- V4L2_CID_LINK_FREQ,
- ARRAY_SIZE(link_freq) - 1,
- ov7251->link_freq_idx,
- link_freq);
- if (ov7251->link_freq)
- ov7251->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
- if (ov7251->pixel_clock)
- ov7251->pixel_clock->flags |= V4L2_CTRL_FLAG_READ_ONLY;
-
- ov7251->sd.ctrl_handler = &ov7251->ctrls;
-
- if (ov7251->ctrls.error) {
- dev_err(dev, "%s: control initialization error %d\n",
- __func__, ov7251->ctrls.error);
- ret = ov7251->ctrls.error;
- goto free_ctrl;
+ ret = ov7251_init_ctrls(ov7251);
+ if (ret) {
+ dev_err_probe(dev, ret, "error during v4l2 ctrl init\n");
+ goto destroy_mutex;
}
v4l2_i2c_subdev_init(&ov7251->sd, client, &ov7251_subdev_ops);
@@ -1686,6 +1696,7 @@ static int ov7251_probe(struct i2c_client *client)
media_entity_cleanup(&ov7251->sd.entity);
free_ctrl:
v4l2_ctrl_handler_free(&ov7251->ctrls);
+destroy_mutex:
mutex_destroy(&ov7251->lock);
return ret;
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 14/15] media: i2c: Add hblank control to ov7251
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (12 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 13/15] media: i2c: add ov7251_init_ctrls() Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver Daniel Scally
14 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Add a hblank control to the ov7251 driver. This necessitates setting
a default mode, for which I am simply picking the first available.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- New patch
drivers/media/i2c/ov7251.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index b7d89ad49887..003a7a5ae038 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -61,6 +61,8 @@
#define OV7251_ACTIVE_WIDTH 648
#define OV7251_ACTIVE_HEIGHT 488
+#define OV7251_FIXED_PPL 928
+
struct reg_value {
u16 reg;
u8 val;
@@ -139,6 +141,7 @@ struct ov7251 {
struct v4l2_ctrl *link_freq;
struct v4l2_ctrl *exposure;
struct v4l2_ctrl *gain;
+ struct v4l2_ctrl *hblank;
/* Cached register values */
u8 aec_pk_manual;
@@ -1490,6 +1493,7 @@ static int ov7251_detect_chip(struct ov7251 *ov7251)
static int ov7251_init_ctrls(struct ov7251 *ov7251)
{
s64 pixel_rate;
+ int hblank;
v4l2_ctrl_handler_init(&ov7251->ctrls, 7);
ov7251->ctrls.lock = &ov7251->lock;
@@ -1524,6 +1528,13 @@ static int ov7251_init_ctrls(struct ov7251 *ov7251)
if (ov7251->pixel_clock)
ov7251->pixel_clock->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ hblank = OV7251_FIXED_PPL - ov7251->current_mode->width;
+ ov7251->hblank = v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
+ V4L2_CID_HBLANK, hblank, hblank, 1,
+ hblank);
+ if (ov7251->hblank)
+ ov7251->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
ov7251->sd.ctrl_handler = &ov7251->ctrls;
if (ov7251->ctrls.error) {
@@ -1619,6 +1630,7 @@ static int ov7251_probe(struct i2c_client *client)
mutex_init(&ov7251->lock);
+ ov7251->current_mode = &ov7251_mode_info_data[0];
ret = ov7251_init_ctrls(ov7251);
if (ret) {
dev_err_probe(dev, ret, "error during v4l2 ctrl init\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
` (13 preceding siblings ...)
2022-05-04 22:30 ` [PATCH v3 14/15] media: i2c: Add hblank control to ov7251 Daniel Scally
@ 2022-05-04 22:30 ` Daniel Scally
2022-05-05 1:02 ` kernel test robot
` (2 more replies)
14 siblings, 3 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-04 22:30 UTC (permalink / raw)
To: linux-media
Cc: yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Add a vblank control to the ov7251 driver.
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:
- New patch
drivers/media/i2c/ov7251.c | 53 ++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index 003a7a5ae038..dc9d4e08efae 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -62,6 +62,10 @@
#define OV7251_ACTIVE_HEIGHT 488
#define OV7251_FIXED_PPL 928
+#define OV7251_TIMING_VTS_REG 0x380e
+#define OV7251_TIMING_MIN_VTS 1
+#define OV7251_TIMING_MAX_VTS 0xffff
+#define OV7251_INTEGRATION_MARGIN 20
struct reg_value {
u16 reg;
@@ -71,6 +75,7 @@ struct reg_value {
struct ov7251_mode_info {
u32 width;
u32 height;
+ u32 vts;
const struct reg_value *data;
u32 data_size;
u32 pixel_clock;
@@ -142,6 +147,7 @@ struct ov7251 {
struct v4l2_ctrl *exposure;
struct v4l2_ctrl *gain;
struct v4l2_ctrl *hblank;
+ struct v4l2_ctrl *vblank;
/* Cached register values */
u8 aec_pk_manual;
@@ -637,6 +643,7 @@ static const struct ov7251_mode_info ov7251_mode_info_data[] = {
{
.width = 640,
.height = 480,
+ .vts = 1724,
.data = ov7251_setting_vga_30fps,
.data_size = ARRAY_SIZE(ov7251_setting_vga_30fps),
.exposure_max = 1704,
@@ -649,6 +656,7 @@ static const struct ov7251_mode_info ov7251_mode_info_data[] = {
{
.width = 640,
.height = 480,
+ .vts = 860,
.data = ov7251_setting_vga_60fps,
.data_size = ARRAY_SIZE(ov7251_setting_vga_60fps),
.exposure_max = 840,
@@ -661,6 +669,7 @@ static const struct ov7251_mode_info ov7251_mode_info_data[] = {
{
.width = 640,
.height = 480,
+ .vts = 572,
.data = ov7251_setting_vga_90fps,
.data_size = ARRAY_SIZE(ov7251_setting_vga_90fps),
.exposure_max = 552,
@@ -1001,12 +1010,36 @@ static const char * const ov7251_test_pattern_menu[] = {
"Vertical Pattern Bars",
};
+static int ov7251_vts_configure(struct ov7251 *ov7251, s32 vblank)
+{
+ u8 vts[2];
+
+ vts[0] = ((ov7251->current_mode->height + vblank) & 0xff00) >> 8;
+ vts[1] = ((ov7251->current_mode->height + vblank) & 0x00ff);
+
+ return ov7251_write_seq_regs(ov7251, OV7251_TIMING_VTS_REG, vts, 2);
+}
+
static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct ov7251 *ov7251 = container_of(ctrl->handler,
struct ov7251, ctrls);
int ret;
+ /* If VBLANK is altered we need to update exposure to compensate */
+ if (ctrl->id == V4L2_CID_VBLANK) {
+ int exposure_max;
+
+ exposure_max = ov7251->current_mode->height + ctrl->val -
+ OV7251_INTEGRATION_MARGIN;
+ __v4l2_ctrl_modify_range(ov7251->exposure,
+ ov7251->exposure->minimum,
+ exposure_max,
+ ov7251->exposure->step,
+ min(ov7251->exposure->val,
+ exposure_max));
+ }
+
/* v4l2_ctrl_lock() locks our mutex */
if (!pm_runtime_get_if_in_use(ov7251->dev))
@@ -1028,6 +1061,9 @@ static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_VFLIP:
ret = ov7251_set_vflip(ov7251, ctrl->val);
break;
+ case V4L2_CID_VBLANK:
+ ret = ov7251_vts_configure(ov7251, ctrl->val);
+ break;
default:
ret = -EINVAL;
break;
@@ -1179,6 +1215,7 @@ static int ov7251_set_format(struct v4l2_subdev *sd,
{
struct ov7251 *ov7251 = to_ov7251(sd);
struct v4l2_mbus_framefmt *__format;
+ int vblank_max, vblank_def;
struct v4l2_rect *__crop;
const struct ov7251_mode_info *new_mode;
int ret = 0;
@@ -1212,6 +1249,14 @@ static int ov7251_set_format(struct v4l2_subdev *sd,
if (ret < 0)
goto exit;
+ vblank_max = OV7251_TIMING_MAX_VTS - new_mode->height;
+ vblank_def = new_mode->vts - new_mode->height;
+ ret = __v4l2_ctrl_modify_range(ov7251->vblank,
+ OV7251_TIMING_MIN_VTS,
+ vblank_max, 1, vblank_max);
+ if (ret < 0)
+ goto exit;
+
ov7251->current_mode = new_mode;
}
@@ -1492,6 +1537,7 @@ static int ov7251_detect_chip(struct ov7251 *ov7251)
static int ov7251_init_ctrls(struct ov7251 *ov7251)
{
+ int vblank_max, vblank_def;
s64 pixel_rate;
int hblank;
@@ -1535,6 +1581,13 @@ static int ov7251_init_ctrls(struct ov7251 *ov7251)
if (ov7251->hblank)
ov7251->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+ vblank_max = OV7251_TIMING_MAX_VTS - ov7251->current_mode->height;
+ vblank_def = ov7251->current_mode->vts - ov7251->current_mode->height;
+ ov7251->vblank = v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
+ V4L2_CID_VBLANK,
+ OV7251_TIMING_MIN_VTS, vblank_max, 1,
+ vblank_def);
+
ov7251->sd.ctrl_handler = &ov7251->ctrls;
if (ov7251->ctrls.error) {
--
2.25.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-04 22:30 ` [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver Daniel Scally
@ 2022-05-05 1:02 ` kernel test robot
2022-05-05 8:04 ` Daniel Scally
2022-05-05 10:31 ` Andy Shevchenko
2022-05-05 17:11 ` kernel test robot
2 siblings, 1 reply; 31+ messages in thread
From: kernel test robot @ 2022-05-05 1:02 UTC (permalink / raw)
To: Daniel Scally, linux-media
Cc: kbuild-all, yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc5 next-20220504]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
base: git://linuxtv.org/media_tree.git master
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220505/202205050844.k1CPWqtV-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/132a5a799bbe214b679bc8e242193c5c1ff1d967
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
git checkout 132a5a799bbe214b679bc8e242193c5c1ff1d967
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/media/i2c/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/media/i2c/ov7251.c: In function 'ov7251_set_format':
>> drivers/media/i2c/ov7251.c:1218:25: warning: variable 'vblank_def' set but not used [-Wunused-but-set-variable]
1218 | int vblank_max, vblank_def;
| ^~~~~~~~~~
At top level:
drivers/media/i2c/ov7251.c:193:37: warning: 'ov7251_pll1_cfg_24_mhz_319_2_mhz' defined but not used [-Wunused-const-variable=]
193 | static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_319_2_mhz = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/vblank_def +1218 drivers/media/i2c/ov7251.c
1211
1212 static int ov7251_set_format(struct v4l2_subdev *sd,
1213 struct v4l2_subdev_state *sd_state,
1214 struct v4l2_subdev_format *format)
1215 {
1216 struct ov7251 *ov7251 = to_ov7251(sd);
1217 struct v4l2_mbus_framefmt *__format;
> 1218 int vblank_max, vblank_def;
1219 struct v4l2_rect *__crop;
1220 const struct ov7251_mode_info *new_mode;
1221 int ret = 0;
1222
1223 mutex_lock(&ov7251->lock);
1224
1225 __crop = __ov7251_get_pad_crop(ov7251, sd_state, format->pad,
1226 format->which);
1227
1228 new_mode = v4l2_find_nearest_size(ov7251_mode_info_data,
1229 ARRAY_SIZE(ov7251_mode_info_data),
1230 width, height,
1231 format->format.width, format->format.height);
1232
1233 __crop->width = new_mode->width;
1234 __crop->height = new_mode->height;
1235
1236 if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
1237 ret = __v4l2_ctrl_modify_range(ov7251->exposure,
1238 1, new_mode->exposure_max,
1239 1, new_mode->exposure_def);
1240 if (ret < 0)
1241 goto exit;
1242
1243 ret = __v4l2_ctrl_s_ctrl(ov7251->exposure,
1244 new_mode->exposure_def);
1245 if (ret < 0)
1246 goto exit;
1247
1248 ret = __v4l2_ctrl_s_ctrl(ov7251->gain, 16);
1249 if (ret < 0)
1250 goto exit;
1251
1252 vblank_max = OV7251_TIMING_MAX_VTS - new_mode->height;
1253 vblank_def = new_mode->vts - new_mode->height;
1254 ret = __v4l2_ctrl_modify_range(ov7251->vblank,
1255 OV7251_TIMING_MIN_VTS,
1256 vblank_max, 1, vblank_max);
1257 if (ret < 0)
1258 goto exit;
1259
1260 ov7251->current_mode = new_mode;
1261 }
1262
1263 __format = __ov7251_get_pad_format(ov7251, sd_state, format->pad,
1264 format->which);
1265 __format->width = __crop->width;
1266 __format->height = __crop->height;
1267 __format->code = MEDIA_BUS_FMT_Y10_1X10;
1268 __format->field = V4L2_FIELD_NONE;
1269 __format->colorspace = V4L2_COLORSPACE_SRGB;
1270 __format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(__format->colorspace);
1271 __format->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
1272 __format->colorspace, __format->ycbcr_enc);
1273 __format->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(__format->colorspace);
1274
1275 format->format = *__format;
1276
1277 exit:
1278 mutex_unlock(&ov7251->lock);
1279
1280 return ret;
1281 }
1282
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-05 1:02 ` kernel test robot
@ 2022-05-05 8:04 ` Daniel Scally
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-05 8:04 UTC (permalink / raw)
To: kernel test robot, linux-media
Cc: kbuild-all, yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
andriy.shevchenko, hverkuil-cisco
Argh - I screwed up a rebase here, not sure how I missed that. Sorry
all, let me resend
On 05/05/2022 02:02, kernel test robot wrote:
> Hi Daniel,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on media-tree/master]
> [also build test WARNING on v5.18-rc5 next-20220504]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
> base: git://linuxtv.org/media_tree.git master
> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220505/202205050844.k1CPWqtV-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 11.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/132a5a799bbe214b679bc8e242193c5c1ff1d967
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
> git checkout 132a5a799bbe214b679bc8e242193c5c1ff1d967
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/media/i2c/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/media/i2c/ov7251.c: In function 'ov7251_set_format':
>>> drivers/media/i2c/ov7251.c:1218:25: warning: variable 'vblank_def' set but not used [-Wunused-but-set-variable]
> 1218 | int vblank_max, vblank_def;
> | ^~~~~~~~~~
> At top level:
> drivers/media/i2c/ov7251.c:193:37: warning: 'ov7251_pll1_cfg_24_mhz_319_2_mhz' defined but not used [-Wunused-const-variable=]
> 193 | static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_319_2_mhz = {
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> vim +/vblank_def +1218 drivers/media/i2c/ov7251.c
>
> 1211
> 1212 static int ov7251_set_format(struct v4l2_subdev *sd,
> 1213 struct v4l2_subdev_state *sd_state,
> 1214 struct v4l2_subdev_format *format)
> 1215 {
> 1216 struct ov7251 *ov7251 = to_ov7251(sd);
> 1217 struct v4l2_mbus_framefmt *__format;
>> 1218 int vblank_max, vblank_def;
> 1219 struct v4l2_rect *__crop;
> 1220 const struct ov7251_mode_info *new_mode;
> 1221 int ret = 0;
> 1222
> 1223 mutex_lock(&ov7251->lock);
> 1224
> 1225 __crop = __ov7251_get_pad_crop(ov7251, sd_state, format->pad,
> 1226 format->which);
> 1227
> 1228 new_mode = v4l2_find_nearest_size(ov7251_mode_info_data,
> 1229 ARRAY_SIZE(ov7251_mode_info_data),
> 1230 width, height,
> 1231 format->format.width, format->format.height);
> 1232
> 1233 __crop->width = new_mode->width;
> 1234 __crop->height = new_mode->height;
> 1235
> 1236 if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> 1237 ret = __v4l2_ctrl_modify_range(ov7251->exposure,
> 1238 1, new_mode->exposure_max,
> 1239 1, new_mode->exposure_def);
> 1240 if (ret < 0)
> 1241 goto exit;
> 1242
> 1243 ret = __v4l2_ctrl_s_ctrl(ov7251->exposure,
> 1244 new_mode->exposure_def);
> 1245 if (ret < 0)
> 1246 goto exit;
> 1247
> 1248 ret = __v4l2_ctrl_s_ctrl(ov7251->gain, 16);
> 1249 if (ret < 0)
> 1250 goto exit;
> 1251
> 1252 vblank_max = OV7251_TIMING_MAX_VTS - new_mode->height;
> 1253 vblank_def = new_mode->vts - new_mode->height;
> 1254 ret = __v4l2_ctrl_modify_range(ov7251->vblank,
> 1255 OV7251_TIMING_MIN_VTS,
> 1256 vblank_max, 1, vblank_max);
> 1257 if (ret < 0)
> 1258 goto exit;
> 1259
> 1260 ov7251->current_mode = new_mode;
> 1261 }
> 1262
> 1263 __format = __ov7251_get_pad_format(ov7251, sd_state, format->pad,
> 1264 format->which);
> 1265 __format->width = __crop->width;
> 1266 __format->height = __crop->height;
> 1267 __format->code = MEDIA_BUS_FMT_Y10_1X10;
> 1268 __format->field = V4L2_FIELD_NONE;
> 1269 __format->colorspace = V4L2_COLORSPACE_SRGB;
> 1270 __format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(__format->colorspace);
> 1271 __format->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
> 1272 __format->colorspace, __format->ycbcr_enc);
> 1273 __format->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(__format->colorspace);
> 1274
> 1275 format->format = *__format;
> 1276
> 1277 exit:
> 1278 mutex_unlock(&ov7251->lock);
> 1279
> 1280 return ret;
> 1281 }
> 1282
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
@ 2022-05-05 8:04 ` Daniel Scally
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-05 8:04 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5197 bytes --]
Argh - I screwed up a rebase here, not sure how I missed that. Sorry
all, let me resend
On 05/05/2022 02:02, kernel test robot wrote:
> Hi Daniel,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on media-tree/master]
> [also build test WARNING on v5.18-rc5 next-20220504]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
> base: git://linuxtv.org/media_tree.git master
> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220505/202205050844.k1CPWqtV-lkp(a)intel.com/config)
> compiler: m68k-linux-gcc (GCC) 11.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/132a5a799bbe214b679bc8e242193c5c1ff1d967
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
> git checkout 132a5a799bbe214b679bc8e242193c5c1ff1d967
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/media/i2c/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/media/i2c/ov7251.c: In function 'ov7251_set_format':
>>> drivers/media/i2c/ov7251.c:1218:25: warning: variable 'vblank_def' set but not used [-Wunused-but-set-variable]
> 1218 | int vblank_max, vblank_def;
> | ^~~~~~~~~~
> At top level:
> drivers/media/i2c/ov7251.c:193:37: warning: 'ov7251_pll1_cfg_24_mhz_319_2_mhz' defined but not used [-Wunused-const-variable=]
> 193 | static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_319_2_mhz = {
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> vim +/vblank_def +1218 drivers/media/i2c/ov7251.c
>
> 1211
> 1212 static int ov7251_set_format(struct v4l2_subdev *sd,
> 1213 struct v4l2_subdev_state *sd_state,
> 1214 struct v4l2_subdev_format *format)
> 1215 {
> 1216 struct ov7251 *ov7251 = to_ov7251(sd);
> 1217 struct v4l2_mbus_framefmt *__format;
>> 1218 int vblank_max, vblank_def;
> 1219 struct v4l2_rect *__crop;
> 1220 const struct ov7251_mode_info *new_mode;
> 1221 int ret = 0;
> 1222
> 1223 mutex_lock(&ov7251->lock);
> 1224
> 1225 __crop = __ov7251_get_pad_crop(ov7251, sd_state, format->pad,
> 1226 format->which);
> 1227
> 1228 new_mode = v4l2_find_nearest_size(ov7251_mode_info_data,
> 1229 ARRAY_SIZE(ov7251_mode_info_data),
> 1230 width, height,
> 1231 format->format.width, format->format.height);
> 1232
> 1233 __crop->width = new_mode->width;
> 1234 __crop->height = new_mode->height;
> 1235
> 1236 if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> 1237 ret = __v4l2_ctrl_modify_range(ov7251->exposure,
> 1238 1, new_mode->exposure_max,
> 1239 1, new_mode->exposure_def);
> 1240 if (ret < 0)
> 1241 goto exit;
> 1242
> 1243 ret = __v4l2_ctrl_s_ctrl(ov7251->exposure,
> 1244 new_mode->exposure_def);
> 1245 if (ret < 0)
> 1246 goto exit;
> 1247
> 1248 ret = __v4l2_ctrl_s_ctrl(ov7251->gain, 16);
> 1249 if (ret < 0)
> 1250 goto exit;
> 1251
> 1252 vblank_max = OV7251_TIMING_MAX_VTS - new_mode->height;
> 1253 vblank_def = new_mode->vts - new_mode->height;
> 1254 ret = __v4l2_ctrl_modify_range(ov7251->vblank,
> 1255 OV7251_TIMING_MIN_VTS,
> 1256 vblank_max, 1, vblank_max);
> 1257 if (ret < 0)
> 1258 goto exit;
> 1259
> 1260 ov7251->current_mode = new_mode;
> 1261 }
> 1262
> 1263 __format = __ov7251_get_pad_format(ov7251, sd_state, format->pad,
> 1264 format->which);
> 1265 __format->width = __crop->width;
> 1266 __format->height = __crop->height;
> 1267 __format->code = MEDIA_BUS_FMT_Y10_1X10;
> 1268 __format->field = V4L2_FIELD_NONE;
> 1269 __format->colorspace = V4L2_COLORSPACE_SRGB;
> 1270 __format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(__format->colorspace);
> 1271 __format->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
> 1272 __format->colorspace, __format->ycbcr_enc);
> 1273 __format->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(__format->colorspace);
> 1274
> 1275 format->format = *__format;
> 1276
> 1277 exit:
> 1278 mutex_unlock(&ov7251->lock);
> 1279
> 1280 return ret;
> 1281 }
> 1282
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-05 8:04 ` Daniel Scally
@ 2022-05-05 8:32 ` Sakari Ailus
-1 siblings, 0 replies; 31+ messages in thread
From: Sakari Ailus @ 2022-05-05 8:32 UTC (permalink / raw)
To: Daniel Scally
Cc: kernel test robot, linux-media, kbuild-all, yong.zhi,
sakari.ailus, bingbu.cao, tian.shu.qiu, andriy.shevchenko,
hverkuil-cisco
On Thu, May 05, 2022 at 09:04:32AM +0100, Daniel Scally wrote:
> Argh - I screwed up a rebase here, not sure how I missed that. Sorry
> all, let me resend
If it's just that, I can fix it. No need to resend.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
@ 2022-05-05 8:32 ` Sakari Ailus
0 siblings, 0 replies; 31+ messages in thread
From: Sakari Ailus @ 2022-05-05 8:32 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 232 bytes --]
On Thu, May 05, 2022 at 09:04:32AM +0100, Daniel Scally wrote:
> Argh - I screwed up a rebase here, not sure how I missed that. Sorry
> all, let me resend
If it's just that, I can fix it. No need to resend.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-05 8:32 ` Sakari Ailus
@ 2022-05-05 9:10 ` Daniel Scally
-1 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-05 9:10 UTC (permalink / raw)
To: Sakari Ailus
Cc: kernel test robot, linux-media, kbuild-all, yong.zhi,
sakari.ailus, bingbu.cao, tian.shu.qiu, andriy.shevchenko,
hverkuil-cisco
On 05/05/2022 09:32, Sakari Ailus wrote:
> On Thu, May 05, 2022 at 09:04:32AM +0100, Daniel Scally wrote:
>> Argh - I screwed up a rebase here, not sure how I missed that. Sorry
>> all, let me resend
> If it's just that, I can fix it. No need to resend.
>
Thanks...there's two problems shown though actually, I used 1 instead of
vblank_def in the __v4l2_ctrl_modify_range() call in
ov7251_set_format(), and the other problem
(ov7251_pll1_cfg_24_mhz_319_2_mhz defined but not used) was actually
introduced in patch #7 (media: i2c: Add support for new frequencies to
ov7251). This change:
|static const struct ov7251_pll_cfgs ov7251_pll_cfgs_24_mhz = { .pll2 =
&ov7251_pll2_cfg_24_mhz, .pll1 = { [OV7251_LINK_FREQ_240_MHZ] =
&ov7251_pll1_cfg_24_mhz_240_mhz, + [OV7251_LINK_FREQ_319_2_MHZ] =
&ov7251_pll1_cfg_19_2_mhz_319_2_mhz, }, }; Should have referred to |ov7251_pll1_cfg_24_mhz_319_2_mhz. I can fix them, but it wouldn't be until tonight (sorry, I expected to have this ready much earlier by the end of the weekend!)
||
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
@ 2022-05-05 9:10 ` Daniel Scally
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-05 9:10 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]
On 05/05/2022 09:32, Sakari Ailus wrote:
> On Thu, May 05, 2022 at 09:04:32AM +0100, Daniel Scally wrote:
>> Argh - I screwed up a rebase here, not sure how I missed that. Sorry
>> all, let me resend
> If it's just that, I can fix it. No need to resend.
>
Thanks...there's two problems shown though actually, I used 1 instead of
vblank_def in the __v4l2_ctrl_modify_range() call in
ov7251_set_format(), and the other problem
(ov7251_pll1_cfg_24_mhz_319_2_mhz defined but not used) was actually
introduced in patch #7 (media: i2c: Add support for new frequencies to
ov7251). This change:
|static const struct ov7251_pll_cfgs ov7251_pll_cfgs_24_mhz = { .pll2 =
&ov7251_pll2_cfg_24_mhz, .pll1 = { [OV7251_LINK_FREQ_240_MHZ] =
&ov7251_pll1_cfg_24_mhz_240_mhz, + [OV7251_LINK_FREQ_319_2_MHZ] =
&ov7251_pll1_cfg_19_2_mhz_319_2_mhz, }, }; Should have referred to |ov7251_pll1_cfg_24_mhz_319_2_mhz. I can fix them, but it wouldn't be until tonight (sorry, I expected to have this ready much earlier by the end of the weekend!)
||
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 04/15] media: i2c: Provide ov7251_check_hwcfg()
2022-05-04 22:30 ` [PATCH v3 04/15] media: i2c: Provide ov7251_check_hwcfg() Daniel Scally
@ 2022-05-05 10:16 ` Andy Shevchenko
2022-05-05 10:41 ` Daniel Scally
0 siblings, 1 reply; 31+ messages in thread
From: Andy Shevchenko @ 2022-05-05 10:16 UTC (permalink / raw)
To: Daniel Scally
Cc: linux-media, yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
hverkuil-cisco
On Wed, May 04, 2022 at 11:30:16PM +0100, Daniel Scally wrote:
> Move the endpoint checking from .probe() to a dedicated function,
> and additionally check that the firmware provided link frequencies
> are a match for those supported by the driver. Store the index to the
> matching link frequency so it can be easily identified later.
...
> + if (!bus_cfg.nr_of_link_frequencies) {
> + ret = -EINVAL;
> + dev_err_probe(ov7251->dev, ret,
> + "no link frequencies defined\n");
ret = dev_err_probe(ov7251->dev, -EINVAL,
"no link frequencies defined\n");
?
> + goto out_free_bus_cfg;
> + }
...
> + if (i == bus_cfg.nr_of_link_frequencies) {
> + ret = -EINVAL;
> + dev_err_probe(ov7251->dev, ret,
> + "no supported link freq found\n");
Ditto.
> + goto out_free_bus_cfg;
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-04 22:30 ` [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver Daniel Scally
2022-05-05 1:02 ` kernel test robot
@ 2022-05-05 10:31 ` Andy Shevchenko
2022-05-05 22:04 ` Daniel Scally
2022-05-05 17:11 ` kernel test robot
2 siblings, 1 reply; 31+ messages in thread
From: Andy Shevchenko @ 2022-05-05 10:31 UTC (permalink / raw)
To: Daniel Scally
Cc: linux-media, yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
hverkuil-cisco
On Wed, May 04, 2022 at 11:30:27PM +0100, Daniel Scally wrote:
> Add a vblank control to the ov7251 driver.
> +static int ov7251_vts_configure(struct ov7251 *ov7251, s32 vblank)
> +{
> + u8 vts[2];
> +
> + vts[0] = ((ov7251->current_mode->height + vblank) & 0xff00) >> 8;
> + vts[1] = ((ov7251->current_mode->height + vblank) & 0x00ff);
__be16 vts;
cpu_to_be16();
> + return ov7251_write_seq_regs(ov7251, OV7251_TIMING_VTS_REG, vts, 2);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 04/15] media: i2c: Provide ov7251_check_hwcfg()
2022-05-05 10:16 ` Andy Shevchenko
@ 2022-05-05 10:41 ` Daniel Scally
0 siblings, 0 replies; 31+ messages in thread
From: Daniel Scally @ 2022-05-05 10:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-media, yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
hverkuil-cisco
Hi Andy
On 05/05/2022 11:16, Andy Shevchenko wrote:
> On Wed, May 04, 2022 at 11:30:16PM +0100, Daniel Scally wrote:
>> Move the endpoint checking from .probe() to a dedicated function,
>> and additionally check that the firmware provided link frequencies
>> are a match for those supported by the driver. Store the index to the
>> matching link frequency so it can be easily identified later.
> ...
>
>> + if (!bus_cfg.nr_of_link_frequencies) {
>> + ret = -EINVAL;
>> + dev_err_probe(ov7251->dev, ret,
>> + "no link frequencies defined\n");
> ret = dev_err_probe(ov7251->dev, -EINVAL,
> "no link frequencies defined\n");
>
> ?
Derp - of course, thanks!
>> + goto out_free_bus_cfg;
>> + }
> ...
>
>> + if (i == bus_cfg.nr_of_link_frequencies) {
>> + ret = -EINVAL;
>> + dev_err_probe(ov7251->dev, ret,
>> + "no supported link freq found\n");
> Ditto.
>
>> + goto out_free_bus_cfg;
>> + }
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/15] media: i2c: Add support for new frequencies to ov7251
2022-05-04 22:30 ` [PATCH v3 07/15] media: i2c: Add support for new frequencies to ov7251 Daniel Scally
@ 2022-05-05 12:49 ` kernel test robot
0 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2022-05-05 12:49 UTC (permalink / raw)
To: Daniel Scally, linux-media
Cc: llvm, kbuild-all, yong.zhi, sakari.ailus, bingbu.cao,
tian.shu.qiu, andriy.shevchenko, hverkuil-cisco
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
base: git://linuxtv.org/media_tree.git master
config: hexagon-buildonly-randconfig-r002-20220501 (https://download.01.org/0day-ci/archive/20220505/202205052045.9ovlO4pA-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5e004fb787698440a387750db7f8028e7cb14cfc)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/4f69999ece8bcd0a4e1e616cac23441d2606348d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
git checkout 4f69999ece8bcd0a4e1e616cac23441d2606348d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/i2c/ov7251.c:176:37: warning: unused variable 'ov7251_pll1_cfg_24_mhz_319_2_mhz' [-Wunused-const-variable]
static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_319_2_mhz = {
^
1 warning generated.
vim +/ov7251_pll1_cfg_24_mhz_319_2_mhz +176 drivers/media/i2c/ov7251.c
175
> 176 static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_319_2_mhz = {
177 .pre_div = 0x05,
178 .mult = 0x85,
179 .div = 0x02,
180 .pix_div = 0x0a,
181 .mipi_div = 0x05,
182 };
183
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-04 22:30 ` [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver Daniel Scally
2022-05-05 1:02 ` kernel test robot
2022-05-05 10:31 ` Andy Shevchenko
@ 2022-05-05 17:11 ` kernel test robot
2 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2022-05-05 17:11 UTC (permalink / raw)
To: Daniel Scally, linux-media
Cc: llvm, kbuild-all, yong.zhi, sakari.ailus, bingbu.cao,
tian.shu.qiu, andriy.shevchenko, hverkuil-cisco
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc5 next-20220505]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-a014 (https://download.01.org/0day-ci/archive/20220506/202205060133.HrXVpuXG-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5e004fb787698440a387750db7f8028e7cb14cfc)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/132a5a799bbe214b679bc8e242193c5c1ff1d967
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
git checkout 132a5a799bbe214b679bc8e242193c5c1ff1d967
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/crypto/ drivers/media/i2c/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/i2c/ov7251.c:1218:18: warning: variable 'vblank_def' set but not used [-Wunused-but-set-variable]
int vblank_max, vblank_def;
^
drivers/media/i2c/ov7251.c:193:37: warning: unused variable 'ov7251_pll1_cfg_24_mhz_319_2_mhz' [-Wunused-const-variable]
static const struct ov7251_pll1_cfg ov7251_pll1_cfg_24_mhz_319_2_mhz = {
^
2 warnings generated.
vim +/vblank_def +1218 drivers/media/i2c/ov7251.c
1211
1212 static int ov7251_set_format(struct v4l2_subdev *sd,
1213 struct v4l2_subdev_state *sd_state,
1214 struct v4l2_subdev_format *format)
1215 {
1216 struct ov7251 *ov7251 = to_ov7251(sd);
1217 struct v4l2_mbus_framefmt *__format;
> 1218 int vblank_max, vblank_def;
1219 struct v4l2_rect *__crop;
1220 const struct ov7251_mode_info *new_mode;
1221 int ret = 0;
1222
1223 mutex_lock(&ov7251->lock);
1224
1225 __crop = __ov7251_get_pad_crop(ov7251, sd_state, format->pad,
1226 format->which);
1227
1228 new_mode = v4l2_find_nearest_size(ov7251_mode_info_data,
1229 ARRAY_SIZE(ov7251_mode_info_data),
1230 width, height,
1231 format->format.width, format->format.height);
1232
1233 __crop->width = new_mode->width;
1234 __crop->height = new_mode->height;
1235
1236 if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
1237 ret = __v4l2_ctrl_modify_range(ov7251->exposure,
1238 1, new_mode->exposure_max,
1239 1, new_mode->exposure_def);
1240 if (ret < 0)
1241 goto exit;
1242
1243 ret = __v4l2_ctrl_s_ctrl(ov7251->exposure,
1244 new_mode->exposure_def);
1245 if (ret < 0)
1246 goto exit;
1247
1248 ret = __v4l2_ctrl_s_ctrl(ov7251->gain, 16);
1249 if (ret < 0)
1250 goto exit;
1251
1252 vblank_max = OV7251_TIMING_MAX_VTS - new_mode->height;
1253 vblank_def = new_mode->vts - new_mode->height;
1254 ret = __v4l2_ctrl_modify_range(ov7251->vblank,
1255 OV7251_TIMING_MIN_VTS,
1256 vblank_max, 1, vblank_max);
1257 if (ret < 0)
1258 goto exit;
1259
1260 ov7251->current_mode = new_mode;
1261 }
1262
1263 __format = __ov7251_get_pad_format(ov7251, sd_state, format->pad,
1264 format->which);
1265 __format->width = __crop->width;
1266 __format->height = __crop->height;
1267 __format->code = MEDIA_BUS_FMT_Y10_1X10;
1268 __format->field = V4L2_FIELD_NONE;
1269 __format->colorspace = V4L2_COLORSPACE_SRGB;
1270 __format->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(__format->colorspace);
1271 __format->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(true,
1272 __format->colorspace, __format->ycbcr_enc);
1273 __format->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(__format->colorspace);
1274
1275 format->format = *__format;
1276
1277 exit:
1278 mutex_unlock(&ov7251->lock);
1279
1280 return ret;
1281 }
1282
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-05 10:31 ` Andy Shevchenko
@ 2022-05-05 22:04 ` Daniel Scally
2022-05-06 21:22 ` Andy Shevchenko
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Scally @ 2022-05-05 22:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-media, yong.zhi, sakari.ailus, bingbu.cao, tian.shu.qiu,
hverkuil-cisco
Hi Andy
On 05/05/2022 11:31, Andy Shevchenko wrote:
> On Wed, May 04, 2022 at 11:30:27PM +0100, Daniel Scally wrote:
>> Add a vblank control to the ov7251 driver.
>> +static int ov7251_vts_configure(struct ov7251 *ov7251, s32 vblank)
>> +{
>> + u8 vts[2];
>> +
>> + vts[0] = ((ov7251->current_mode->height + vblank) & 0xff00) >> 8;
>> + vts[1] = ((ov7251->current_mode->height + vblank) & 0x00ff);
> __be16 vts;
>
> cpu_to_be16();
Most places that do this seem to do the conversion in the i2c read/write
functions, so in this case within ov7251_write_seq_regs(). Can I do it
there, as an extra patch? I actually have more changes to make on this
driver but they're not remotely read yet so there'll be another series
in the future
>> + return ov7251_write_seq_regs(ov7251, OV7251_TIMING_VTS_REG, vts, 2);
>> +}
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver
2022-05-05 22:04 ` Daniel Scally
@ 2022-05-06 21:22 ` Andy Shevchenko
0 siblings, 0 replies; 31+ messages in thread
From: Andy Shevchenko @ 2022-05-06 21:22 UTC (permalink / raw)
To: Daniel Scally
Cc: Andy Shevchenko, linux-media, yong.zhi, sakari.ailus, bingbu.cao,
tian.shu.qiu, hverkuil-cisco
On Fri, May 6, 2022 at 1:46 PM Daniel Scally <djrscally@gmail.com> wrote:
> On 05/05/2022 11:31, Andy Shevchenko wrote:
> > On Wed, May 04, 2022 at 11:30:27PM +0100, Daniel Scally wrote:
...
> >> + u8 vts[2];
> >> +
> >> + vts[0] = ((ov7251->current_mode->height + vblank) & 0xff00) >> 8;
> >> + vts[1] = ((ov7251->current_mode->height + vblank) & 0x00ff);
> > __be16 vts;
> >
> > cpu_to_be16();
> Most places that do this seem to do the conversion in the i2c read/write
> functions, so in this case within ov7251_write_seq_regs(). Can I do it
> there, as an extra patch? I actually have more changes to make on this
> driver but they're not remotely read yet so there'll be another series
> in the future
Ideally you should first convert them and then add your patch with
this change in mind.
> >> + return ov7251_write_seq_regs(ov7251, OV7251_TIMING_VTS_REG, vts, 2);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 06/15] media: i2c: Add ov7251_pll_configure()
@ 2022-05-05 12:29 kernel test robot
0 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2022-05-05 12:29 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4550 bytes --]
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220504223027.3480287-7-djrscally@gmail.com>
References: <20220504223027.3480287-7-djrscally@gmail.com>
TO: Daniel Scally <djrscally@gmail.com>
TO: linux-media(a)vger.kernel.org
CC: yong.zhi(a)intel.com
CC: sakari.ailus(a)linux.intel.com
CC: bingbu.cao(a)intel.com
CC: tian.shu.qiu(a)intel.com
CC: andriy.shevchenko(a)linux.intel.com
CC: hverkuil-cisco(a)xs4all.nl
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc5 next-20220504]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220505-063608
base: git://linuxtv.org/media_tree.git master
:::::: branch date: 14 hours ago
:::::: commit date: 14 hours ago
config: i386-randconfig-c001 (https://download.01.org/0day-ci/archive/20220505/202205052030.TLrDYuNF-lkp(a)intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
cocci warnings: (new ones prefixed by >>)
>> drivers/media/i2c/ov7251.c:1241:3-9: preceding lock on line 1236
vim +1241 drivers/media/i2c/ov7251.c
d30bb512da3d8e Todor Tomov 2018-04-25 1230
d30bb512da3d8e Todor Tomov 2018-04-25 1231 static int ov7251_s_stream(struct v4l2_subdev *subdev, int enable)
d30bb512da3d8e Todor Tomov 2018-04-25 1232 {
d30bb512da3d8e Todor Tomov 2018-04-25 1233 struct ov7251 *ov7251 = to_ov7251(subdev);
d30bb512da3d8e Todor Tomov 2018-04-25 1234 int ret;
d30bb512da3d8e Todor Tomov 2018-04-25 1235
d30bb512da3d8e Todor Tomov 2018-04-25 @1236 mutex_lock(&ov7251->lock);
d30bb512da3d8e Todor Tomov 2018-04-25 1237
d30bb512da3d8e Todor Tomov 2018-04-25 1238 if (enable) {
575c4ea63dc4cb Daniel Scally 2022-05-04 1239 ret = ov7251_pll_configure(ov7251);
575c4ea63dc4cb Daniel Scally 2022-05-04 1240 if (ret)
575c4ea63dc4cb Daniel Scally 2022-05-04 @1241 return dev_err_probe(ov7251->dev, ret,
575c4ea63dc4cb Daniel Scally 2022-05-04 1242 "error configuring PLLs\n");
575c4ea63dc4cb Daniel Scally 2022-05-04 1243
d30bb512da3d8e Todor Tomov 2018-04-25 1244 ret = ov7251_set_register_array(ov7251,
d30bb512da3d8e Todor Tomov 2018-04-25 1245 ov7251->current_mode->data,
d30bb512da3d8e Todor Tomov 2018-04-25 1246 ov7251->current_mode->data_size);
d30bb512da3d8e Todor Tomov 2018-04-25 1247 if (ret < 0) {
d30bb512da3d8e Todor Tomov 2018-04-25 1248 dev_err(ov7251->dev, "could not set mode %dx%d\n",
d30bb512da3d8e Todor Tomov 2018-04-25 1249 ov7251->current_mode->width,
d30bb512da3d8e Todor Tomov 2018-04-25 1250 ov7251->current_mode->height);
d30bb512da3d8e Todor Tomov 2018-04-25 1251 goto exit;
d30bb512da3d8e Todor Tomov 2018-04-25 1252 }
d30bb512da3d8e Todor Tomov 2018-04-25 1253 ret = __v4l2_ctrl_handler_setup(&ov7251->ctrls);
d30bb512da3d8e Todor Tomov 2018-04-25 1254 if (ret < 0) {
d30bb512da3d8e Todor Tomov 2018-04-25 1255 dev_err(ov7251->dev, "could not sync v4l2 controls\n");
d30bb512da3d8e Todor Tomov 2018-04-25 1256 goto exit;
d30bb512da3d8e Todor Tomov 2018-04-25 1257 }
d30bb512da3d8e Todor Tomov 2018-04-25 1258 ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
d30bb512da3d8e Todor Tomov 2018-04-25 1259 OV7251_SC_MODE_SELECT_STREAMING);
d30bb512da3d8e Todor Tomov 2018-04-25 1260 } else {
d30bb512da3d8e Todor Tomov 2018-04-25 1261 ret = ov7251_write_reg(ov7251, OV7251_SC_MODE_SELECT,
d30bb512da3d8e Todor Tomov 2018-04-25 1262 OV7251_SC_MODE_SELECT_SW_STANDBY);
d30bb512da3d8e Todor Tomov 2018-04-25 1263 }
d30bb512da3d8e Todor Tomov 2018-04-25 1264
d30bb512da3d8e Todor Tomov 2018-04-25 1265 exit:
d30bb512da3d8e Todor Tomov 2018-04-25 1266 mutex_unlock(&ov7251->lock);
d30bb512da3d8e Todor Tomov 2018-04-25 1267
d30bb512da3d8e Todor Tomov 2018-04-25 1268 return ret;
d30bb512da3d8e Todor Tomov 2018-04-25 1269 }
d30bb512da3d8e Todor Tomov 2018-04-25 1270
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2022-05-06 21:22 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04 22:30 [PATCH v3 00/15] Support OVTI7251 on Microsoft Surface line Daniel Scally
2022-05-04 22:30 ` [PATCH v3 01/15] media: uapi: Add IPU3 packed Y10 format Daniel Scally
2022-05-04 22:30 ` [PATCH v3 02/15] media: ipu3-cio2: Add support for V4L2_PIX_FMT_IPU3_Y10 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 03/15] media: i2c: Add acpi support to ov7251 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 04/15] media: i2c: Provide ov7251_check_hwcfg() Daniel Scally
2022-05-05 10:16 ` Andy Shevchenko
2022-05-05 10:41 ` Daniel Scally
2022-05-04 22:30 ` [PATCH v3 05/15] media: i2c: Remove per-mode frequencies from ov7251 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 06/15] media: i2c: Add ov7251_pll_configure() Daniel Scally
2022-05-04 22:30 ` [PATCH v3 07/15] media: i2c: Add support for new frequencies to ov7251 Daniel Scally
2022-05-05 12:49 ` kernel test robot
2022-05-04 22:30 ` [PATCH v3 08/15] media: i2c: Add ov7251_detect_chip() Daniel Scally
2022-05-04 22:30 ` [PATCH v3 09/15] media: i2c: Add pm_runtime support to ov7251 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 10/15] media: i2c: Remove .s_power() from ov7251 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 11/15] media: ipu3-cio2: Add INT347E to cio2-bridge Daniel Scally
2022-05-04 22:30 ` [PATCH v3 12/15] media: i2c: Extend .get_selection() for ov7251 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 13/15] media: i2c: add ov7251_init_ctrls() Daniel Scally
2022-05-04 22:30 ` [PATCH v3 14/15] media: i2c: Add hblank control to ov7251 Daniel Scally
2022-05-04 22:30 ` [PATCH v3 15/15] media: i2c: Add vblank control to ov7251 driver Daniel Scally
2022-05-05 1:02 ` kernel test robot
2022-05-05 8:04 ` Daniel Scally
2022-05-05 8:04 ` Daniel Scally
2022-05-05 8:32 ` Sakari Ailus
2022-05-05 8:32 ` Sakari Ailus
2022-05-05 9:10 ` Daniel Scally
2022-05-05 9:10 ` Daniel Scally
2022-05-05 10:31 ` Andy Shevchenko
2022-05-05 22:04 ` Daniel Scally
2022-05-06 21:22 ` Andy Shevchenko
2022-05-05 17:11 ` kernel test robot
2022-05-05 12:29 [PATCH v3 06/15] media: i2c: Add ov7251_pll_configure() kernel test robot
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.