HDMI and Composite capture on Lager, for kernel 4.1, version 3

HDMI and Composite capture on Lager, for kernel 4.1, version 3

[William Towle]

  Version 3. Obsoletes version 2, as seen at:
	http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/91668

  Key changes in this version: this has some reworking of the adv7604
driver probe and soc_camera initialisation functions. In addition,
we give rcar_vin.c a dependency on CONFIG_MEDIA_CONTROLLER in line with
the drivers used with it.

Cheers,
  Wills.

To follow:
	[PATCH 01/15] ARM: shmobile: lager dts: Add entries for VIN HDMI
	[PATCH 02/15] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888
	[PATCH 03/15] media: adv7180: add of match table
	[PATCH 04/15] media: adv7604: chip info and formats for ADV7612
	[PATCH 05/15] media: adv7604: document support for ADV7612 dual HDMI
	[PATCH 06/15] media: adv7604: ability to read default input port
	[PATCH 07/15] ARM: shmobile: lager dts: specify default-input for
	[PATCH 08/15] v4l: subdev: Add pad config allocator and init
	[PATCH 09/15] media: soc_camera pad-aware driver initialisation
	[PATCH 10/15] media: rcar_vin: Use correct pad number in try_fmt
	[PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad
	[PATCH 12/15] media: soc_camera: Fill std field in enum_input
	[PATCH 13/15] media: soc_camera: Fix error reporting in expbuf
	[PATCH 14/15] media: soc_camera: fill in bus_info field
	[PATCH 15/15] media: rcar_vin: Reject videobufs that are too small

[PATCH 01/15] ARM: shmobile: lager dts: Add entries for VIN HDMI input support

[William Towle]

Add DT entries for vin0, vin0_pins, and adv7612

Signed-off-by: William Towle <william.towle@codethink.co.uk>
Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 arch/arm/boot/dts/r8a7790-lager.dts |   41 ++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index aaa4f25..90c4531 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -370,7 +370,12 @@
 		renesas,function = "usb2";
 	};
 
-	vin1_pins: vin {
+	vin0_pins: vin0 {
+		renesas,groups = "vin0_data24", "vin0_sync", "vin0_field", "vin0_clkenb", "vin0_clk";
+		renesas,function = "vin0";
+	};
+
+	vin1_pins: vin1 {
 		renesas,groups = "vin1_data8", "vin1_clk";
 		renesas,function = "vin1";
 	};
@@ -531,6 +536,18 @@
 		reg = <0x12>;
 	};
 
+	hdmi-in@4c {
+		compatible = "adi,adv7612";
+		reg = <0x4c>;
+		remote = <&vin0>;
+
+		port {
+			hdmi_in_ep: endpoint {
+				remote-endpoint = <&vin0ep0>;
+			};
+		};
+	};
+
 	composite-in@20 {
 		compatible = "adi,adv7180";
 		reg = <0x20>;
@@ -646,6 +663,28 @@
 	status = "okay";
 };
 
+/* HDMI video input */
+&vin0 {
+	pinctrl-0 = <&vin0_pins>;
+	pinctrl-names = "default";
+
+	status = "ok";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		vin0ep0: endpoint {
+			remote-endpoint = <&hdmi_in_ep>;
+			bus-width = <24>;
+			hsync-active = <0>;
+			vsync-active = <0>;
+			pclk-sample = <1>;
+			data-active = <1>;
+		};
+	};
+};
+
 /* composite video input */
 &vin1 {
 	pinctrl-0 = <&vin1_pins>;
-- 
1.7.10.4

[PATCH 02/15] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[William Towle]

This adds V4L2_MBUS_FMT_RGB888_1X24 input format support
which is used by the ADV7612 chip.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>

Modified to use MEDIA_BUS_FMT_* constants

Signed-off-by: William Towle <william.towle@codethink.co.uk>
Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 drivers/media/platform/soc_camera/rcar_vin.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index db7700b..16352a8 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -98,6 +98,7 @@
 #define VNMC_INF_YUV10_BT656	(2 << 16)
 #define VNMC_INF_YUV10_BT601	(3 << 16)
 #define VNMC_INF_YUV16		(5 << 16)
+#define VNMC_INF_RGB888		(6 << 16)
 #define VNMC_VUP		(1 << 10)
 #define VNMC_IM_ODD		(0 << 3)
 #define VNMC_IM_ODD_EVEN	(1 << 3)
@@ -589,7 +590,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 	struct soc_camera_device *icd = priv->ici.icd;
 	struct rcar_vin_cam *cam = icd->host_priv;
 	u32 vnmc, dmr, interrupts;
-	bool progressive = false, output_is_yuv = false;
+	bool progressive = false, output_is_yuv = false, input_is_yuv = false;
 
 	switch (priv->field) {
 	case V4L2_FIELD_TOP:
@@ -623,16 +624,22 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 	case MEDIA_BUS_FMT_YUYV8_1X16:
 		/* BT.601/BT.1358 16bit YCbCr422 */
 		vnmc |= VNMC_INF_YUV16;
+		input_is_yuv = true;
 		break;
 	case MEDIA_BUS_FMT_YUYV8_2X8:
 		/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
 		vnmc |= priv->pdata_flags & RCAR_VIN_BT656 ?
 			VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
+		input_is_yuv = true;
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X24:
+		vnmc |= VNMC_INF_RGB888;
 		break;
 	case MEDIA_BUS_FMT_YUYV10_2X10:
 		/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
 		vnmc |= priv->pdata_flags & RCAR_VIN_BT656 ?
 			VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
+		input_is_yuv = true;
 		break;
 	default:
 		break;
@@ -676,7 +683,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
 	vnmc |= VNMC_VUP;
 
 	/* If input and output use the same colorspace, use bypass mode */
-	if (output_is_yuv)
+	if (input_is_yuv == output_is_yuv)
 		vnmc |= VNMC_BPS;
 
 	/* progressive or interlaced mode */
@@ -1423,6 +1430,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx,
 	case MEDIA_BUS_FMT_YUYV8_1X16:
 	case MEDIA_BUS_FMT_YUYV8_2X8:
 	case MEDIA_BUS_FMT_YUYV10_2X10:
+	case MEDIA_BUS_FMT_RGB888_1X24:
 		if (cam->extra_fmt)
 			break;
 
-- 
1.7.10.4

Re: [PATCH 02/15] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888 input support

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
> This adds V4L2_MBUS_FMT_RGB888_1X24 input format support
> which is used by the ADV7612 chip.
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> 
> Modified to use MEDIA_BUS_FMT_* constants
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

> ---
>  drivers/media/platform/soc_camera/rcar_vin.c |   12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index db7700b..16352a8 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -98,6 +98,7 @@
>  #define VNMC_INF_YUV10_BT656	(2 << 16)
>  #define VNMC_INF_YUV10_BT601	(3 << 16)
>  #define VNMC_INF_YUV16		(5 << 16)
> +#define VNMC_INF_RGB888		(6 << 16)
>  #define VNMC_VUP		(1 << 10)
>  #define VNMC_IM_ODD		(0 << 3)
>  #define VNMC_IM_ODD_EVEN	(1 << 3)
> @@ -589,7 +590,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>  	struct soc_camera_device *icd = priv->ici.icd;
>  	struct rcar_vin_cam *cam = icd->host_priv;
>  	u32 vnmc, dmr, interrupts;
> -	bool progressive = false, output_is_yuv = false;
> +	bool progressive = false, output_is_yuv = false, input_is_yuv = false;
>  
>  	switch (priv->field) {
>  	case V4L2_FIELD_TOP:
> @@ -623,16 +624,22 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>  	case MEDIA_BUS_FMT_YUYV8_1X16:
>  		/* BT.601/BT.1358 16bit YCbCr422 */
>  		vnmc |= VNMC_INF_YUV16;
> +		input_is_yuv = true;
>  		break;
>  	case MEDIA_BUS_FMT_YUYV8_2X8:
>  		/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
>  		vnmc |= priv->pdata_flags & RCAR_VIN_BT656 ?
>  			VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
> +		input_is_yuv = true;
> +		break;
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +		vnmc |= VNMC_INF_RGB888;
>  		break;
>  	case MEDIA_BUS_FMT_YUYV10_2X10:
>  		/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
>  		vnmc |= priv->pdata_flags & RCAR_VIN_BT656 ?
>  			VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
> +		input_is_yuv = true;
>  		break;
>  	default:
>  		break;
> @@ -676,7 +683,7 @@ static int rcar_vin_setup(struct rcar_vin_priv *priv)
>  	vnmc |= VNMC_VUP;
>  
>  	/* If input and output use the same colorspace, use bypass mode */
> -	if (output_is_yuv)
> +	if (input_is_yuv == output_is_yuv)
>  		vnmc |= VNMC_BPS;
>  
>  	/* progressive or interlaced mode */
> @@ -1423,6 +1430,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx,
>  	case MEDIA_BUS_FMT_YUYV8_1X16:
>  	case MEDIA_BUS_FMT_YUYV8_2X8:
>  	case MEDIA_BUS_FMT_YUYV10_2X10:
> +	case MEDIA_BUS_FMT_RGB888_1X24:
>  		if (cam->extra_fmt)
>  			break;
>  
> 

[PATCH 03/15] media: adv7180: add of match table

[William Towle]

From: Ben Dooks <ben.dooks@codethink.co.uk>

Add a proper of match id for use when the device is being bound via
device tree, to avoid having to use the i2c old-style binding of the
device.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Signed-off-by: William.Towle <william.towle@codethink.co.uk>
Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 drivers/media/i2c/adv7180.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index a493c0b..09a96df 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -25,6 +25,7 @@
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <media/v4l2-ioctl.h>
 #include <linux/videodev2.h>
 #include <media/v4l2-device.h>
@@ -1324,11 +1325,21 @@ static SIMPLE_DEV_PM_OPS(adv7180_pm_ops, adv7180_suspend, adv7180_resume);
 #define ADV7180_PM_OPS NULL
 #endif
 
+#ifdef CONFIG_OF
+static const struct of_device_id adv7180_of_id[] = {
+	{ .compatible = "adi,adv7180", },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, adv7180_of_id);
+#endif
+
 static struct i2c_driver adv7180_driver = {
 	.driver = {
 		   .owner = THIS_MODULE,
 		   .name = KBUILD_MODNAME,
 		   .pm = ADV7180_PM_OPS,
+		   .of_match_table = of_match_ptr(adv7180_of_id),
 		   },
 	.probe = adv7180_probe,
 	.remove = adv7180_remove,
-- 
1.7.10.4

Re: [PATCH 03/15] media: adv7180: add of match table

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
> From: Ben Dooks <ben.dooks@codethink.co.uk>
> 
> Add a proper of match id for use when the device is being bound via
> device tree, to avoid having to use the i2c old-style binding of the
> device.
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> Signed-off-by: William.Towle <william.towle@codethink.co.uk>
> Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

> ---
>  drivers/media/i2c/adv7180.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index a493c0b..09a96df 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -25,6 +25,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
>  #include <media/v4l2-ioctl.h>
>  #include <linux/videodev2.h>
>  #include <media/v4l2-device.h>
> @@ -1324,11 +1325,21 @@ static SIMPLE_DEV_PM_OPS(adv7180_pm_ops, adv7180_suspend, adv7180_resume);
>  #define ADV7180_PM_OPS NULL
>  #endif
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id adv7180_of_id[] = {
> +	{ .compatible = "adi,adv7180", },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, adv7180_of_id);
> +#endif
> +
>  static struct i2c_driver adv7180_driver = {
>  	.driver = {
>  		   .owner = THIS_MODULE,
>  		   .name = KBUILD_MODNAME,
>  		   .pm = ADV7180_PM_OPS,
> +		   .of_match_table = of_match_ptr(adv7180_of_id),
>  		   },
>  	.probe = adv7180_probe,
>  	.remove = adv7180_remove,
> 

[PATCH 04/15] media: adv7604: chip info and formats for ADV7612

[William Towle]

Add support for the ADV7612 chip as implemented on Renesas' Lager
board to adv7604.c, including lists for formats/colourspace/timing
selection and an IRQ handler.

Signed-off-by: William Towle <william.towle@codethink.co.uk>
---
 drivers/media/i2c/adv7604.c |   91 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index aaa37b0..16646517 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -80,6 +80,7 @@ MODULE_LICENSE("GPL");
 enum adv76xx_type {
 	ADV7604,
 	ADV7611,
+	ADV7612,
 };
 
 struct adv76xx_reg_seq {
@@ -758,6 +759,23 @@ static const struct adv76xx_format_info adv7611_formats[] = {
 	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_12BIT },
 };
 
+static const struct adv76xx_format_info adv7612_formats[] = {
+	{ MEDIA_BUS_FMT_RGB888_1X24, ADV76XX_OP_CH_SEL_RGB, true, false,
+	  ADV76XX_OP_MODE_SEL_SDR_444 | ADV76XX_OP_FORMAT_SEL_8BIT },
+	{ MEDIA_BUS_FMT_YUYV8_2X8, ADV76XX_OP_CH_SEL_RGB, false, false,
+	  ADV76XX_OP_MODE_SEL_SDR_422 | ADV76XX_OP_FORMAT_SEL_8BIT },
+	{ MEDIA_BUS_FMT_YVYU8_2X8, ADV76XX_OP_CH_SEL_RGB, false, true,
+	  ADV76XX_OP_MODE_SEL_SDR_422 | ADV76XX_OP_FORMAT_SEL_8BIT },
+	{ MEDIA_BUS_FMT_UYVY8_1X16, ADV76XX_OP_CH_SEL_RBG, false, false,
+	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
+	{ MEDIA_BUS_FMT_VYUY8_1X16, ADV76XX_OP_CH_SEL_RBG, false, true,
+	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
+	{ MEDIA_BUS_FMT_YUYV8_1X16, ADV76XX_OP_CH_SEL_RGB, false, false,
+	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
+	{ MEDIA_BUS_FMT_YVYU8_1X16, ADV76XX_OP_CH_SEL_RGB, false, true,
+	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
+};
+
 static const struct adv76xx_format_info *
 adv76xx_format_info(struct adv76xx_state *state, u32 code)
 {
@@ -2471,6 +2489,11 @@ static void adv7611_setup_irqs(struct v4l2_subdev *sd)
 	io_write(sd, 0x41, 0xd0); /* STDI irq for any change, disable INT2 */
 }
 
+static void adv7612_setup_irqs(struct v4l2_subdev *sd)
+{
+	io_write(sd, 0x41, 0xd0); /* disable INT2 */
+}
+
 static void adv76xx_unregister_clients(struct adv76xx_state *state)
 {
 	unsigned int i;
@@ -2558,6 +2581,19 @@ static const struct adv76xx_reg_seq adv7611_recommended_settings_hdmi[] = {
 	{ ADV76XX_REG_SEQ_TERM, 0 },
 };
 
+static const struct adv76xx_reg_seq adv7612_recommended_settings_hdmi[] = {
+	{ ADV76XX_REG(ADV76XX_PAGE_CP, 0x6c), 0x00 },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x9b), 0x03 },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x6f), 0x08 },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x85), 0x1f },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x87), 0x70 },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x57), 0xda },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x58), 0x01 },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x03), 0x98 },
+	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x4c), 0x44 },
+	{ ADV76XX_REG_SEQ_TERM, 0 },
+};
+
 static const struct adv76xx_chip_info adv76xx_chip_info[] = {
 	[ADV7604] = {
 		.type = ADV7604,
@@ -2646,17 +2682,59 @@ static const struct adv76xx_chip_info adv76xx_chip_info[] = {
 		.field1_vsync_mask = 0x3fff,
 		.field1_vbackporch_mask = 0x3fff,
 	},
+	[ADV7612] = {
+		.type = ADV7612,
+		.has_afe = false,
+		.max_port = ADV7604_PAD_HDMI_PORT_B,
+		.num_dv_ports = 2,
+		.edid_enable_reg = 0x74,
+		.edid_status_reg = 0x76,
+		.lcf_reg = 0xa3,
+		.tdms_lock_mask = 0x43,
+		.cable_det_mask = 0x01,
+		.fmt_change_digital_mask = 0x03,
+		.formats = adv7612_formats,
+		.nformats = ARRAY_SIZE(adv7612_formats),
+		.set_termination = adv7611_set_termination,
+		.setup_irqs = adv7612_setup_irqs,
+		.read_hdmi_pixelclock = adv7611_read_hdmi_pixelclock,
+		.read_cable_det = adv7611_read_cable_det,
+		.recommended_settings = {
+		    [1] = adv7612_recommended_settings_hdmi,
+		},
+		.num_recommended_settings = {
+		    [1] = ARRAY_SIZE(adv7612_recommended_settings_hdmi),
+		},
+		.page_mask = BIT(ADV76XX_PAGE_IO) | BIT(ADV76XX_PAGE_CEC) |
+			BIT(ADV76XX_PAGE_INFOFRAME) | BIT(ADV76XX_PAGE_AFE) |
+			BIT(ADV76XX_PAGE_REP) |  BIT(ADV76XX_PAGE_EDID) |
+			BIT(ADV76XX_PAGE_HDMI) | BIT(ADV76XX_PAGE_CP),
+		.linewidth_mask = 0x1fff,
+		.field0_height_mask = 0x1fff,
+		.field1_height_mask = 0x1fff,
+		.hfrontporch_mask = 0x1fff,
+		.hsync_mask = 0x1fff,
+		.hbackporch_mask = 0x1fff,
+		.field0_vfrontporch_mask = 0x3fff,
+		.field0_vsync_mask = 0x3fff,
+		.field0_vbackporch_mask = 0x3fff,
+		.field1_vfrontporch_mask = 0x3fff,
+		.field1_vsync_mask = 0x3fff,
+		.field1_vbackporch_mask = 0x3fff,
+	},
 };
 
 static const struct i2c_device_id adv76xx_i2c_id[] = {
 	{ "adv7604", (kernel_ulong_t)&adv76xx_chip_info[ADV7604] },
 	{ "adv7611", (kernel_ulong_t)&adv76xx_chip_info[ADV7611] },
+	{ "adv7612", (kernel_ulong_t)&adv76xx_chip_info[ADV7612] },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, adv76xx_i2c_id);
 
 static const struct of_device_id adv76xx_of_id[] __maybe_unused = {
 	{ .compatible = "adi,adv7611", .data = &adv76xx_chip_info[ADV7611] },
+	{ .compatible = "adi,adv7612", .data = &adv76xx_chip_info[ADV7612] },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, adv76xx_of_id);
@@ -2801,21 +2879,26 @@ static int adv76xx_probe(struct i2c_client *client,
 	 * identifies the revision, while on ADV7611 it identifies the model as
 	 * well. Use the HDMI slave address on ADV7604 and RD_INFO on ADV7611.
 	 */
-	if (state->info->type == ADV7604) {
+	switch (state->info->type) {
+	case ADV7604:
 		val = adv_smbus_read_byte_data_check(client, 0xfb, false);
 		if (val != 0x68) {
 			v4l2_info(sd, "not an adv7604 on address 0x%x\n",
 					client->addr << 1);
 			return -ENODEV;
 		}
-	} else {
+		break;
+	case ADV7611:
+	case ADV7612:
 		val = (adv_smbus_read_byte_data_check(client, 0xea, false) << 8)
 		    | (adv_smbus_read_byte_data_check(client, 0xeb, false) << 0);
-		if (val != 0x2051) {
-			v4l2_info(sd, "not an adv7611 on address 0x%x\n",
+		if ((state->info->type == ADV7611 && val != 0x2051) ||
+			(state->info->type == ADV7612 && val != 0x2041)) {
+			v4l2_info(sd, "not an adv761x on address 0x%x\n",
 					client->addr << 1);
 			return -ENODEV;
 		}
+		break;
 	}
 
 	/* control handlers */
-- 
1.7.10.4

Re: [PATCH 04/15] media: adv7604: chip info and formats for ADV7612

[Hans Verkuil]

Hi William,

Two comments, see below.

On 06/03/2015 03:59 PM, William Towle wrote:
> Add support for the ADV7612 chip as implemented on Renesas' Lager
> board to adv7604.c, including lists for formats/colourspace/timing
> selection and an IRQ handler.
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> ---
>  drivers/media/i2c/adv7604.c |   91 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 87 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index aaa37b0..16646517 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -80,6 +80,7 @@ MODULE_LICENSE("GPL");
>  enum adv76xx_type {
>  	ADV7604,
>  	ADV7611,
> +	ADV7612,
>  };
>  
>  struct adv76xx_reg_seq {
> @@ -758,6 +759,23 @@ static const struct adv76xx_format_info adv7611_formats[] = {
>  	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_12BIT },
>  };
>  
> +static const struct adv76xx_format_info adv7612_formats[] = {
> +	{ MEDIA_BUS_FMT_RGB888_1X24, ADV76XX_OP_CH_SEL_RGB, true, false,
> +	  ADV76XX_OP_MODE_SEL_SDR_444 | ADV76XX_OP_FORMAT_SEL_8BIT },
> +	{ MEDIA_BUS_FMT_YUYV8_2X8, ADV76XX_OP_CH_SEL_RGB, false, false,
> +	  ADV76XX_OP_MODE_SEL_SDR_422 | ADV76XX_OP_FORMAT_SEL_8BIT },
> +	{ MEDIA_BUS_FMT_YVYU8_2X8, ADV76XX_OP_CH_SEL_RGB, false, true,
> +	  ADV76XX_OP_MODE_SEL_SDR_422 | ADV76XX_OP_FORMAT_SEL_8BIT },
> +	{ MEDIA_BUS_FMT_UYVY8_1X16, ADV76XX_OP_CH_SEL_RBG, false, false,
> +	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
> +	{ MEDIA_BUS_FMT_VYUY8_1X16, ADV76XX_OP_CH_SEL_RBG, false, true,
> +	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
> +	{ MEDIA_BUS_FMT_YUYV8_1X16, ADV76XX_OP_CH_SEL_RGB, false, false,
> +	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
> +	{ MEDIA_BUS_FMT_YVYU8_1X16, ADV76XX_OP_CH_SEL_RGB, false, true,
> +	  ADV76XX_OP_MODE_SEL_SDR_422_2X | ADV76XX_OP_FORMAT_SEL_8BIT },
> +};
> +
>  static const struct adv76xx_format_info *
>  adv76xx_format_info(struct adv76xx_state *state, u32 code)
>  {
> @@ -2471,6 +2489,11 @@ static void adv7611_setup_irqs(struct v4l2_subdev *sd)
>  	io_write(sd, 0x41, 0xd0); /* STDI irq for any change, disable INT2 */
>  }
>  
> +static void adv7612_setup_irqs(struct v4l2_subdev *sd)
> +{
> +	io_write(sd, 0x41, 0xd0); /* disable INT2 */
> +}
> +
>  static void adv76xx_unregister_clients(struct adv76xx_state *state)
>  {
>  	unsigned int i;
> @@ -2558,6 +2581,19 @@ static const struct adv76xx_reg_seq adv7611_recommended_settings_hdmi[] = {
>  	{ ADV76XX_REG_SEQ_TERM, 0 },
>  };
>  
> +static const struct adv76xx_reg_seq adv7612_recommended_settings_hdmi[] = {
> +	{ ADV76XX_REG(ADV76XX_PAGE_CP, 0x6c), 0x00 },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x9b), 0x03 },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x6f), 0x08 },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x85), 0x1f },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x87), 0x70 },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x57), 0xda },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x58), 0x01 },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x03), 0x98 },
> +	{ ADV76XX_REG(ADV76XX_PAGE_HDMI, 0x4c), 0x44 },
> +	{ ADV76XX_REG_SEQ_TERM, 0 },
> +};
> +
>  static const struct adv76xx_chip_info adv76xx_chip_info[] = {
>  	[ADV7604] = {
>  		.type = ADV7604,
> @@ -2646,17 +2682,59 @@ static const struct adv76xx_chip_info adv76xx_chip_info[] = {
>  		.field1_vsync_mask = 0x3fff,
>  		.field1_vbackporch_mask = 0x3fff,
>  	},
> +	[ADV7612] = {
> +		.type = ADV7612,
> +		.has_afe = false,
> +		.max_port = ADV7604_PAD_HDMI_PORT_B,
> +		.num_dv_ports = 2,
> +		.edid_enable_reg = 0x74,
> +		.edid_status_reg = 0x76,
> +		.lcf_reg = 0xa3,
> +		.tdms_lock_mask = 0x43,

This looks strange: with two inputs I expect two bits set in the mask, not three.

> +		.cable_det_mask = 0x01,

Same here: I expect to see two bits set, not one.

> +		.fmt_change_digital_mask = 0x03,
> +		.formats = adv7612_formats,
> +		.nformats = ARRAY_SIZE(adv7612_formats),
> +		.set_termination = adv7611_set_termination,
> +		.setup_irqs = adv7612_setup_irqs,
> +		.read_hdmi_pixelclock = adv7611_read_hdmi_pixelclock,
> +		.read_cable_det = adv7611_read_cable_det,
> +		.recommended_settings = {
> +		    [1] = adv7612_recommended_settings_hdmi,
> +		},
> +		.num_recommended_settings = {
> +		    [1] = ARRAY_SIZE(adv7612_recommended_settings_hdmi),
> +		},
> +		.page_mask = BIT(ADV76XX_PAGE_IO) | BIT(ADV76XX_PAGE_CEC) |
> +			BIT(ADV76XX_PAGE_INFOFRAME) | BIT(ADV76XX_PAGE_AFE) |
> +			BIT(ADV76XX_PAGE_REP) |  BIT(ADV76XX_PAGE_EDID) |
> +			BIT(ADV76XX_PAGE_HDMI) | BIT(ADV76XX_PAGE_CP),
> +		.linewidth_mask = 0x1fff,
> +		.field0_height_mask = 0x1fff,
> +		.field1_height_mask = 0x1fff,
> +		.hfrontporch_mask = 0x1fff,
> +		.hsync_mask = 0x1fff,
> +		.hbackporch_mask = 0x1fff,
> +		.field0_vfrontporch_mask = 0x3fff,
> +		.field0_vsync_mask = 0x3fff,
> +		.field0_vbackporch_mask = 0x3fff,
> +		.field1_vfrontporch_mask = 0x3fff,
> +		.field1_vsync_mask = 0x3fff,
> +		.field1_vbackporch_mask = 0x3fff,
> +	},
>  };
>  
>  static const struct i2c_device_id adv76xx_i2c_id[] = {
>  	{ "adv7604", (kernel_ulong_t)&adv76xx_chip_info[ADV7604] },
>  	{ "adv7611", (kernel_ulong_t)&adv76xx_chip_info[ADV7611] },
> +	{ "adv7612", (kernel_ulong_t)&adv76xx_chip_info[ADV7612] },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, adv76xx_i2c_id);
>  
>  static const struct of_device_id adv76xx_of_id[] __maybe_unused = {
>  	{ .compatible = "adi,adv7611", .data = &adv76xx_chip_info[ADV7611] },
> +	{ .compatible = "adi,adv7612", .data = &adv76xx_chip_info[ADV7612] },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, adv76xx_of_id);
> @@ -2801,21 +2879,26 @@ static int adv76xx_probe(struct i2c_client *client,
>  	 * identifies the revision, while on ADV7611 it identifies the model as
>  	 * well. Use the HDMI slave address on ADV7604 and RD_INFO on ADV7611.
>  	 */
> -	if (state->info->type == ADV7604) {
> +	switch (state->info->type) {
> +	case ADV7604:
>  		val = adv_smbus_read_byte_data_check(client, 0xfb, false);
>  		if (val != 0x68) {
>  			v4l2_info(sd, "not an adv7604 on address 0x%x\n",
>  					client->addr << 1);
>  			return -ENODEV;
>  		}
> -	} else {
> +		break;
> +	case ADV7611:
> +	case ADV7612:
>  		val = (adv_smbus_read_byte_data_check(client, 0xea, false) << 8)
>  		    | (adv_smbus_read_byte_data_check(client, 0xeb, false) << 0);
> -		if (val != 0x2051) {
> -			v4l2_info(sd, "not an adv7611 on address 0x%x\n",
> +		if ((state->info->type == ADV7611 && val != 0x2051) ||
> +			(state->info->type == ADV7612 && val != 0x2041)) {
> +			v4l2_info(sd, "not an adv761x on address 0x%x\n",
>  					client->addr << 1);
>  			return -ENODEV;
>  		}
> +		break;
>  	}
>  
>  	/* control handlers */
> 

Regards,

	Hans

[PATCH 05/15] media: adv7604: document support for ADV7612 dual HDMI input decoder

[William Towle]

From: Ian Molton <ian.molton@codethink.co.uk>

This documentation accompanies the patch adding support for the ADV7612
dual HDMI decoder / repeater chip.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
Reviewed-by: William Towle <william.towle@codethink.co.uk>
---
 .../devicetree/bindings/media/i2c/adv7604.txt        |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index c27cede..7eafdbc 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -1,15 +1,17 @@
-* Analog Devices ADV7604/11 video decoder with HDMI receiver
+* Analog Devices ADV7604/11/12 video decoder with HDMI receiver
 
-The ADV7604 and ADV7611 are multiformat video decoders with an integrated HDMI
-receiver. The ADV7604 has four multiplexed HDMI inputs and one analog input,
-and the ADV7611 has one HDMI input and no analog input.
+The ADV7604 and ADV7611/12 are multiformat video decoders with an integrated
+HDMI receiver. The ADV7604 has four multiplexed HDMI inputs and one analog
+input, and the ADV7611 has one HDMI input and no analog input. The 7612 is
+similar to the 7611 but has 2 HDMI inputs.
 
-These device tree bindings support the ADV7611 only at the moment.
+These device tree bindings support the ADV7611/12 only at the moment.
 
 Required Properties:
 
   - compatible: Must contain one of the following
     - "adi,adv7611" for the ADV7611
+    - "adi,adv7612" for the ADV7612
 
   - reg: I2C slave address
 
@@ -22,10 +24,10 @@ port, in accordance with the video interface bindings defined in
 Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
 are numbered as follows.
 
-  Port			ADV7611
+  Port			ADV7611    ADV7612
 ------------------------------------------------------------
-  HDMI			0
-  Digital output	1
+  HDMI			0             0, 1
+  Digital output	1                2
 
 The digital output port node must contain at least one endpoint.
 
-- 
1.7.10.4

Re: [PATCH 05/15] media: adv7604: document support for ADV7612 dual HDMI input decoder

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
> From: Ian Molton <ian.molton@codethink.co.uk>
> 
> This documentation accompanies the patch adding support for the ADV7612
> dual HDMI decoder / repeater chip.
> 
> Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
> Reviewed-by: William Towle <william.towle@codethink.co.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

> ---
>  .../devicetree/bindings/media/i2c/adv7604.txt        |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> index c27cede..7eafdbc 100644
> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> @@ -1,15 +1,17 @@
> -* Analog Devices ADV7604/11 video decoder with HDMI receiver
> +* Analog Devices ADV7604/11/12 video decoder with HDMI receiver
>  
> -The ADV7604 and ADV7611 are multiformat video decoders with an integrated HDMI
> -receiver. The ADV7604 has four multiplexed HDMI inputs and one analog input,
> -and the ADV7611 has one HDMI input and no analog input.
> +The ADV7604 and ADV7611/12 are multiformat video decoders with an integrated
> +HDMI receiver. The ADV7604 has four multiplexed HDMI inputs and one analog
> +input, and the ADV7611 has one HDMI input and no analog input. The 7612 is
> +similar to the 7611 but has 2 HDMI inputs.
>  
> -These device tree bindings support the ADV7611 only at the moment.
> +These device tree bindings support the ADV7611/12 only at the moment.
>  
>  Required Properties:
>  
>    - compatible: Must contain one of the following
>      - "adi,adv7611" for the ADV7611
> +    - "adi,adv7612" for the ADV7612
>  
>    - reg: I2C slave address
>  
> @@ -22,10 +24,10 @@ port, in accordance with the video interface bindings defined in
>  Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
>  are numbered as follows.
>  
> -  Port			ADV7611
> +  Port			ADV7611    ADV7612
>  ------------------------------------------------------------
> -  HDMI			0
> -  Digital output	1
> +  HDMI			0             0, 1
> +  Digital output	1                2
>  
>  The digital output port node must contain at least one endpoint.
>  
> 

[PATCH 06/15] media: adv7604: ability to read default input port from DT

[William Towle]

From: Ian Molton <ian.molton@codethink.co.uk>

Adds support to the adv7604 driver for specifying the default input
port in the Device tree. If no value is provided, the driver will be
unable to select an input without help from userspace.

Tested-by: William Towle <william.towle@codethink.co.uk>
Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
 Documentation/devicetree/bindings/media/i2c/adv7604.txt |    3 +++
 drivers/media/i2c/adv7604.c                             |    8 +++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index 7eafdbc..8337f75 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -47,6 +47,7 @@ Optional Endpoint Properties:
   If none of hsync-active, vsync-active and pclk-sample is specified the
   endpoint will use embedded BT.656 synchronization.
 
+  - default-input: Select which input is selected after reset.
 
 Example:
 
@@ -60,6 +61,8 @@ Example:
 		#address-cells = <1>;
 		#size-cells = <0>;
 
+		default-input = <0>;
+
 		port@0 {
 			reg = <0>;
 		};
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 16646517..5b6ac8e 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2745,6 +2745,7 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
 	struct device_node *endpoint;
 	struct device_node *np;
 	unsigned int flags;
+	u32 v;
 
 	np = state->i2c_clients[ADV76XX_PAGE_IO]->dev.of_node;
 
@@ -2754,6 +2755,12 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
 		return -EINVAL;
 
 	v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+
+	if (!of_property_read_u32(endpoint, "default-input", &v))
+		state->pdata.default_input = v;
+	else
+		state->pdata.default_input = -1;
+
 	of_node_put(endpoint);
 
 	flags = bus_cfg.bus.parallel.flags;
@@ -2792,7 +2799,6 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
 	/* Hardcode the remaining platform data fields. */
 	state->pdata.disable_pwrdnb = 0;
 	state->pdata.disable_cable_det_rst = 0;
-	state->pdata.default_input = -1;
 	state->pdata.blank_data = 1;
 	state->pdata.alt_data_sat = 1;
 	state->pdata.op_format_mode_sel = ADV7604_OP_FORMAT_MODE0;
-- 
1.7.10.4

Re: [PATCH 06/15] media: adv7604: ability to read default input port from DT

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
> From: Ian Molton <ian.molton@codethink.co.uk>
> 
> Adds support to the adv7604 driver for specifying the default input
> port in the Device tree. If no value is provided, the driver will be
> unable to select an input without help from userspace.
> 
> Tested-by: William Towle <william.towle@codethink.co.uk>
> Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

> ---
>  Documentation/devicetree/bindings/media/i2c/adv7604.txt |    3 +++
>  drivers/media/i2c/adv7604.c                             |    8 +++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> index 7eafdbc..8337f75 100644
> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> @@ -47,6 +47,7 @@ Optional Endpoint Properties:
>    If none of hsync-active, vsync-active and pclk-sample is specified the
>    endpoint will use embedded BT.656 synchronization.
>  
> +  - default-input: Select which input is selected after reset.
>  
>  Example:
>  
> @@ -60,6 +61,8 @@ Example:
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> +		default-input = <0>;
> +
>  		port@0 {
>  			reg = <0>;
>  		};
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 16646517..5b6ac8e 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -2745,6 +2745,7 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
>  	struct device_node *endpoint;
>  	struct device_node *np;
>  	unsigned int flags;
> +	u32 v;
>  
>  	np = state->i2c_clients[ADV76XX_PAGE_IO]->dev.of_node;
>  
> @@ -2754,6 +2755,12 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
>  		return -EINVAL;
>  
>  	v4l2_of_parse_endpoint(endpoint, &bus_cfg);
> +
> +	if (!of_property_read_u32(endpoint, "default-input", &v))
> +		state->pdata.default_input = v;
> +	else
> +		state->pdata.default_input = -1;
> +
>  	of_node_put(endpoint);
>  
>  	flags = bus_cfg.bus.parallel.flags;
> @@ -2792,7 +2799,6 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
>  	/* Hardcode the remaining platform data fields. */
>  	state->pdata.disable_pwrdnb = 0;
>  	state->pdata.disable_cable_det_rst = 0;
> -	state->pdata.default_input = -1;
>  	state->pdata.blank_data = 1;
>  	state->pdata.alt_data_sat = 1;
>  	state->pdata.op_format_mode_sel = ADV7604_OP_FORMAT_MODE0;
> 

[PATCH 07/15] ARM: shmobile: lager dts: specify default-input for ADV7612

[William Towle]

Set 'default-input' property for ADV7612. Enables image/video capture
without the need to have userspace specifying routing.

Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
Tested-by: William Towle <william.towle@codethink.co.uk>
---
 arch/arm/boot/dts/r8a7790-lager.dts |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 90c4531..6946e9a 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -544,6 +544,7 @@
 		port {
 			hdmi_in_ep: endpoint {
 				remote-endpoint = <&vin0ep0>;
+				default-input = <0>;
 			};
 		};
 	};
-- 
1.7.10.4

[PATCH 08/15] v4l: subdev: Add pad config allocator and init

[William Towle]

From: Laurent Pinchart <laurent.pinchart@linaro.org>

Add a new subdev operation to initialize a subdev pad config array, and
a helper function to allocate and initialize the array. This can be used
by bridge drivers to implement try format based on subdev pad
operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@linaro.org>
Acked-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

Changes since v1:

- Added v4l2_subdev_free_pad_config
---
 drivers/media/v4l2-core/v4l2-subdev.c |   19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           |   10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 6359606..d594fe5 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -35,7 +35,7 @@
 static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
 {
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-	fh->pad = kzalloc(sizeof(*fh->pad) * sd->entity.num_pads, GFP_KERNEL);
+	fh->pad = v4l2_subdev_alloc_pad_config(sd);
 	if (fh->pad == NULL)
 		return -ENOMEM;
 #endif
@@ -569,6 +569,23 @@ int v4l2_subdev_link_validate(struct media_link *link)
 		sink, link, &source_fmt, &sink_fmt);
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd)
+{
+	struct v4l2_subdev_pad_config *cfg;
+
+	if (!sd->entity.num_pads)
+		return NULL;
+
+	cfg = kcalloc(sd->entity.num_pads, sizeof(*cfg), GFP_KERNEL);
+	if (!cfg)
+		return NULL;
+
+	v4l2_subdev_call(sd, pad, init_cfg, cfg);
+
+	return cfg;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_pad_config);
 #endif /* CONFIG_MEDIA_CONTROLLER */
 
 void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index dc20102..4a609f6 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -485,6 +485,8 @@ struct v4l2_subdev_pad_config {
  *                  may be adjusted by the subdev driver to device capabilities.
  */
 struct v4l2_subdev_pad_ops {
+	void (*init_cfg)(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg);
 	int (*enum_mbus_code)(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_mbus_code_enum *code);
@@ -677,7 +679,15 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_format *source_fmt,
 				      struct v4l2_subdev_format *sink_fmt);
 int v4l2_subdev_link_validate(struct media_link *link);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd);
+
+static inline void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg)
+{
+	kfree(cfg);
+}
 #endif /* CONFIG_MEDIA_CONTROLLER */
+
 void v4l2_subdev_init(struct v4l2_subdev *sd,
 		      const struct v4l2_subdev_ops *ops);
 
-- 
1.7.10.4

[PATCH 09/15] media: soc_camera pad-aware driver initialisation

[William Towle]

Add detection of source pad number for drivers aware of the media
controller API, so that the combination of soc_camera and rcar_vin
can create device nodes to support modern drivers such as adv7604.c
(for HDMI on Lager) and the converted adv7180.c (for composite)
underneath.

Building rcar_vin gains a dependency on CONFIG_MEDIA_CONTROLLER, in
line with requirements for building the drivers associated with it.

Signed-off-by: William Towle <william.towle@codethink.co.uk>
Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 drivers/media/platform/soc_camera/Kconfig      |    1 +
 drivers/media/platform/soc_camera/rcar_vin.c   |    1 +
 drivers/media/platform/soc_camera/soc_camera.c |   46 ++++++++++++++++++++++++
 include/media/soc_camera.h                     |    1 +
 4 files changed, 49 insertions(+)

diff --git a/drivers/media/platform/soc_camera/Kconfig b/drivers/media/platform/soc_camera/Kconfig
index f2776cd..5c45c83 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -38,6 +38,7 @@ config VIDEO_RCAR_VIN
 	depends on VIDEO_DEV && SOC_CAMERA
 	depends on ARCH_SHMOBILE || COMPILE_TEST
 	depends on HAS_DMA
+	depends on MEDIA_CONTROLLER
 	select VIDEOBUF2_DMA_CONTIG
 	select SOC_CAMERA_SCALE_CROP
 	---help---
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 16352a8..00c1034 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1359,6 +1359,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx,
 		struct device *dev = icd->parent;
 		int shift;
 
+		fmt.pad = icd->src_pad_idx;
 		ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
 		if (ret < 0)
 			return ret;
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index d708df4..c4952c8 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -1293,6 +1293,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
 	struct v4l2_mbus_framefmt *mf = &fmt.format;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	struct media_pad pad;
+#endif
 	int ret;
 
 	sd->grp_id = soc_camera_grp_id(icd);
@@ -1310,8 +1313,40 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
 		return ret;
 	}
 
+	icd->src_pad_idx = -1;
+#if defined(CONFIG_MEDIA_CONTROLLER)
 	/* At this point client .probe() should have run already */
+	ret = media_entity_init(&icd->vdev->entity, 1, &pad, 0);
+	if (ret < 0) {
+		goto eusrfmt;
+	} else {
+		int pad_idx;
+
+		for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
+			if (sd->entity.pads[pad_idx].flags
+					== MEDIA_PAD_FL_SOURCE)
+				break;
+		if (pad_idx >= sd->entity.num_pads)
+			goto eusrfmt;
+
+		ret = media_entity_create_link(&icd->vdev->entity, 0,
+						&sd->entity, pad_idx,
+						MEDIA_LNK_FL_IMMUTABLE |
+						MEDIA_LNK_FL_ENABLED);
+		if (ret < 0)
+			goto eusrfmt;
+
+		icd->src_pad_idx = pad_idx;
+		ret = soc_camera_init_user_formats(icd);
+		if (ret < 0) {
+			icd->src_pad_idx = -1;
+			goto eusrfmt;
+		}
+	}
+#else
 	ret = soc_camera_init_user_formats(icd);
+#endif
+
 	if (ret < 0)
 		goto eusrfmt;
 
@@ -1322,6 +1357,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
 		goto evidstart;
 
 	/* Try to improve our guess of a reasonable window format */
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	fmt.pad = icd->src_pad_idx;
+#endif
 	if (!v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt)) {
 		icd->user_width		= mf->width;
 		icd->user_height	= mf->height;
@@ -1335,6 +1373,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
 evidstart:
 	soc_camera_free_user_formats(icd);
 eusrfmt:
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	media_entity_cleanup(&icd->vdev->entity);
+#endif
 	soc_camera_remove_device(icd);
 
 	return ret;
@@ -1856,6 +1897,11 @@ static int soc_camera_remove(struct soc_camera_device *icd)
 	if (icd->num_user_formats)
 		soc_camera_free_user_formats(icd);
 
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	if (icd->vdev->entity.num_pads)
+		media_entity_cleanup(&icd->vdev->entity);
+#endif
+
 	if (icd->clk) {
 		/* For the synchronous case */
 		v4l2_clk_unregister(icd->clk);
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 2f6261f..30193cf 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -42,6 +42,7 @@ struct soc_camera_device {
 	unsigned char devnum;		/* Device number per host */
 	struct soc_camera_sense *sense;	/* See comment in struct definition */
 	struct video_device *vdev;
+	int src_pad_idx;		/* For media-controller drivers */
 	struct v4l2_ctrl_handler ctrl_handler;
 	const struct soc_camera_format_xlate *current_fmt;
 	struct soc_camera_format_xlate *user_formats;
-- 
1.7.10.4

Re: [PATCH 09/15] media: soc_camera pad-aware driver initialisation

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
> Add detection of source pad number for drivers aware of the media
> controller API, so that the combination of soc_camera and rcar_vin
> can create device nodes to support modern drivers such as adv7604.c
> (for HDMI on Lager) and the converted adv7180.c (for composite)
> underneath.
> 
> Building rcar_vin gains a dependency on CONFIG_MEDIA_CONTROLLER, in
> line with requirements for building the drivers associated with it.
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/Kconfig      |    1 +
>  drivers/media/platform/soc_camera/rcar_vin.c   |    1 +
>  drivers/media/platform/soc_camera/soc_camera.c |   46 ++++++++++++++++++++++++
>  include/media/soc_camera.h                     |    1 +
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/media/platform/soc_camera/Kconfig b/drivers/media/platform/soc_camera/Kconfig
> index f2776cd..5c45c83 100644
> --- a/drivers/media/platform/soc_camera/Kconfig
> +++ b/drivers/media/platform/soc_camera/Kconfig
> @@ -38,6 +38,7 @@ config VIDEO_RCAR_VIN
>  	depends on VIDEO_DEV && SOC_CAMERA
>  	depends on ARCH_SHMOBILE || COMPILE_TEST
>  	depends on HAS_DMA
> +	depends on MEDIA_CONTROLLER
>  	select VIDEOBUF2_DMA_CONTIG
>  	select SOC_CAMERA_SCALE_CROP
>  	---help---
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 16352a8..00c1034 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1359,6 +1359,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx,
>  		struct device *dev = icd->parent;
>  		int shift;
>  
> +		fmt.pad = icd->src_pad_idx;
>  		ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>  		if (ret < 0)
>  			return ret;
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
> index d708df4..c4952c8 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -1293,6 +1293,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
>  	};
>  	struct v4l2_mbus_framefmt *mf = &fmt.format;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	struct media_pad pad;
> +#endif
>  	int ret;
>  
>  	sd->grp_id = soc_camera_grp_id(icd);
> @@ -1310,8 +1313,40 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  		return ret;
>  	}
>  
> +	icd->src_pad_idx = -1;

If CONFIG_MEDIA_CONTROLLER is not defined, then I would expect that src_pad_idx should
be 0, not -1.

> +#if defined(CONFIG_MEDIA_CONTROLLER)
>  	/* At this point client .probe() should have run already */
> +	ret = media_entity_init(&icd->vdev->entity, 1, &pad, 0);
> +	if (ret < 0) {
> +		goto eusrfmt;
> +	} else {
> +		int pad_idx;
> +
> +		for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> +			if (sd->entity.pads[pad_idx].flags
> +					== MEDIA_PAD_FL_SOURCE)
> +				break;
> +		if (pad_idx >= sd->entity.num_pads)
> +			goto eusrfmt;
> +
> +		ret = media_entity_create_link(&icd->vdev->entity, 0,
> +						&sd->entity, pad_idx,
> +						MEDIA_LNK_FL_IMMUTABLE |
> +						MEDIA_LNK_FL_ENABLED);

Why would you want to create a link and set up the vdev entity?

All you need to do here is to find the sd source pad and store it in
sd->src_pad_idx.

You are not registering a media device here, so there is no need to
do anything more. I'd drop all that code.

> +		if (ret < 0)
> +			goto eusrfmt;
> +
> +		icd->src_pad_idx = pad_idx;
> +		ret = soc_camera_init_user_formats(icd);
> +		if (ret < 0) {
> +			icd->src_pad_idx = -1;
> +			goto eusrfmt;
> +		}
> +	}
> +#else
>  	ret = soc_camera_init_user_formats(icd);
> +#endif
> +
>  	if (ret < 0)
>  		goto eusrfmt;
>  
> @@ -1322,6 +1357,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  		goto evidstart;
>  
>  	/* Try to improve our guess of a reasonable window format */
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	fmt.pad = icd->src_pad_idx;
> +#endif

If src_pad_idx is 0 if CONFIG_MEDIA_CONTROLLER is undefined, then you don't need the
#if defined() here.

Regards,

	Hans

>  	if (!v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt)) {
>  		icd->user_width		= mf->width;
>  		icd->user_height	= mf->height;
> @@ -1335,6 +1373,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  evidstart:
>  	soc_camera_free_user_formats(icd);
>  eusrfmt:
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	media_entity_cleanup(&icd->vdev->entity);
> +#endif
>  	soc_camera_remove_device(icd);
>  
>  	return ret;
> @@ -1856,6 +1897,11 @@ static int soc_camera_remove(struct soc_camera_device *icd)
>  	if (icd->num_user_formats)
>  		soc_camera_free_user_formats(icd);
>  
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	if (icd->vdev->entity.num_pads)
> +		media_entity_cleanup(&icd->vdev->entity);
> +#endif
> +
>  	if (icd->clk) {
>  		/* For the synchronous case */
>  		v4l2_clk_unregister(icd->clk);
> diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
> index 2f6261f..30193cf 100644
> --- a/include/media/soc_camera.h
> +++ b/include/media/soc_camera.h
> @@ -42,6 +42,7 @@ struct soc_camera_device {
>  	unsigned char devnum;		/* Device number per host */
>  	struct soc_camera_sense *sense;	/* See comment in struct definition */
>  	struct video_device *vdev;
> +	int src_pad_idx;		/* For media-controller drivers */
>  	struct v4l2_ctrl_handler ctrl_handler;
>  	const struct soc_camera_format_xlate *current_fmt;
>  	struct soc_camera_format_xlate *user_formats;
> 

Re: [PATCH 09/15] media: soc_camera pad-aware driver initialisation

[Guennadi Liakhovetski]

Hi William,

On Wed, 3 Jun 2015, William Towle wrote:

> Add detection of source pad number for drivers aware of the media
> controller API, so that the combination of soc_camera and rcar_vin
> can create device nodes to support modern drivers such as adv7604.c
> (for HDMI on Lager) and the converted adv7180.c (for composite)
> underneath.
> 
> Building rcar_vin gains a dependency on CONFIG_MEDIA_CONTROLLER, in
> line with requirements for building the drivers associated with it.
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/Kconfig      |    1 +
>  drivers/media/platform/soc_camera/rcar_vin.c   |    1 +
>  drivers/media/platform/soc_camera/soc_camera.c |   46 ++++++++++++++++++++++++
>  include/media/soc_camera.h                     |    1 +
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/media/platform/soc_camera/Kconfig b/drivers/media/platform/soc_camera/Kconfig
> index f2776cd..5c45c83 100644
> --- a/drivers/media/platform/soc_camera/Kconfig
> +++ b/drivers/media/platform/soc_camera/Kconfig
> @@ -38,6 +38,7 @@ config VIDEO_RCAR_VIN
>  	depends on VIDEO_DEV && SOC_CAMERA
>  	depends on ARCH_SHMOBILE || COMPILE_TEST
>  	depends on HAS_DMA
> +	depends on MEDIA_CONTROLLER
>  	select VIDEOBUF2_DMA_CONTIG
>  	select SOC_CAMERA_SCALE_CROP
>  	---help---
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 16352a8..00c1034 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1359,6 +1359,7 @@ static int rcar_vin_get_formats(struct soc_camera_device *icd, unsigned int idx,
>  		struct device *dev = icd->parent;
>  		int shift;
>  
> +		fmt.pad = icd->src_pad_idx;
>  		ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>  		if (ret < 0)
>  			return ret;
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
> index d708df4..c4952c8 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -1293,6 +1293,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
>  	};
>  	struct v4l2_mbus_framefmt *mf = &fmt.format;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	struct media_pad pad;
> +#endif
>  	int ret;
>  
>  	sd->grp_id = soc_camera_grp_id(icd);
> @@ -1310,8 +1313,40 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  		return ret;
>  	}
>  
> +	icd->src_pad_idx = -1;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
>  	/* At this point client .probe() should have run already */

Please, leave this comment outside of #ifdef.

> +	ret = media_entity_init(&icd->vdev->entity, 1, &pad, 0);
> +	if (ret < 0) {
> +		goto eusrfmt;
> +	} else {

You don't need this "else" here. Just put pad_idx declaration under 
"#ifdef" above.

> +		int pad_idx;
> +
> +		for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> +			if (sd->entity.pads[pad_idx].flags
> +					== MEDIA_PAD_FL_SOURCE)
> +				break;
> +		if (pad_idx >= sd->entity.num_pads)
> +			goto eusrfmt;
> +
> +		ret = media_entity_create_link(&icd->vdev->entity, 0,
> +						&sd->entity, pad_idx,
> +						MEDIA_LNK_FL_IMMUTABLE |
> +						MEDIA_LNK_FL_ENABLED);
> +		if (ret < 0)
> +			goto eusrfmt;
> +
> +		icd->src_pad_idx = pad_idx;
> +		ret = soc_camera_init_user_formats(icd);

I don't think you need soc_camera_init_user_formats() twice - under ifdef 
and outside of it. Just add "icd->src_pad_idx = -1;" in the error case 
below:

> +		if (ret < 0) {
> +			icd->src_pad_idx = -1;
> +			goto eusrfmt;
> +		}
> +	}
> +#else
>  	ret = soc_camera_init_user_formats(icd);
> +#endif
> +
>  	if (ret < 0)
>  		goto eusrfmt;
>  
> @@ -1322,6 +1357,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  		goto evidstart;
>  
>  	/* Try to improve our guess of a reasonable window format */
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	fmt.pad = icd->src_pad_idx;
> +#endif
>  	if (!v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt)) {
>  		icd->user_width		= mf->width;
>  		icd->user_height	= mf->height;
> @@ -1335,6 +1373,9 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
>  evidstart:
>  	soc_camera_free_user_formats(icd);
>  eusrfmt:
> +#if defined(CONFIG_MEDIA_CONTROLLER)

+	icd->src_pad_idx = -1;

> +	media_entity_cleanup(&icd->vdev->entity);
> +#endif
>  	soc_camera_remove_device(icd);
>  
>  	return ret;

Thanks
Guennadi

[PATCH 10/15] media: rcar_vin: Use correct pad number in try_fmt

[William Towle]

Fix rcar_vin_try_fmt to use the correct pad number when calling the
subdev set_fmt. Previously pad number 0 was always used, resulting in
EINVAL if the subdev cares about the pad number (e.g. ADV7612).

Signed-off-by: William Towle <william.towle@codethink.co.uk>
Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 drivers/media/platform/soc_camera/rcar_vin.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 00c1034..cc993bc 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1697,7 +1697,7 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
 	const struct soc_camera_format_xlate *xlate;
 	struct v4l2_pix_format *pix = &f->fmt.pix;
 	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
-	struct v4l2_subdev_pad_config pad_cfg;
+	struct v4l2_subdev_pad_config *pad_cfg;
 	struct v4l2_subdev_format format = {
 		.which = V4L2_SUBDEV_FORMAT_TRY,
 	};
@@ -1706,6 +1706,8 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
 	int width, height;
 	int ret;
 
+	pad_cfg = v4l2_subdev_alloc_pad_config(sd);
+
 	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
 	if (!xlate) {
 		xlate = icd->current_fmt;
@@ -1734,10 +1736,15 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
 	mf->code = xlate->code;
 	mf->colorspace = pix->colorspace;
 
-	ret = v4l2_device_call_until_err(sd->v4l2_dev, soc_camera_grp_id(icd),
-					 pad, set_fmt, &pad_cfg, &format);
-	if (ret < 0)
+	format.pad = icd->src_pad_idx;
+	ret = v4l2_device_call_until_err(sd->v4l2_dev,
+					soc_camera_grp_id(icd), pad,
+					set_fmt, pad_cfg,
+					&format);
+	if (ret < 0) {
+		v4l2_subdev_free_pad_config(pad_cfg);
 		return ret;
+	}
 
 	/* Adjust only if VIN cannot scale */
 	if (pix->width > mf->width * 2)
@@ -1759,13 +1766,15 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
 			 */
 			mf->width = VIN_MAX_WIDTH;
 			mf->height = VIN_MAX_HEIGHT;
+			format.pad = icd->src_pad_idx;
 			ret = v4l2_device_call_until_err(sd->v4l2_dev,
 							 soc_camera_grp_id(icd),
-							 pad, set_fmt, &pad_cfg,
+							 pad, set_fmt, pad_cfg,
 							 &format);
 			if (ret < 0) {
 				dev_err(icd->parent,
 					"client try_fmt() = %d\n", ret);
+				v4l2_subdev_free_pad_config(pad_cfg);
 				return ret;
 			}
 		}
@@ -1776,6 +1785,7 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
 			pix->height = height;
 	}
 
+	v4l2_subdev_free_pad_config(pad_cfg);
 	return ret;
 }
 
-- 
1.7.10.4

Re: [PATCH 10/15] media: rcar_vin: Use correct pad number in try_fmt

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
> Fix rcar_vin_try_fmt to use the correct pad number when calling the
> subdev set_fmt. Previously pad number 0 was always used, resulting in
> EINVAL if the subdev cares about the pad number (e.g. ADV7612).
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/rcar_vin.c |   20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 00c1034..cc993bc 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1697,7 +1697,7 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  	const struct soc_camera_format_xlate *xlate;
>  	struct v4l2_pix_format *pix = &f->fmt.pix;
>  	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> -	struct v4l2_subdev_pad_config pad_cfg;
> +	struct v4l2_subdev_pad_config *pad_cfg;
>  	struct v4l2_subdev_format format = {
>  		.which = V4L2_SUBDEV_FORMAT_TRY,
>  	};
> @@ -1706,6 +1706,8 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  	int width, height;
>  	int ret;
>  
> +	pad_cfg = v4l2_subdev_alloc_pad_config(sd);
> +
>  	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
>  	if (!xlate) {
>  		xlate = icd->current_fmt;
> @@ -1734,10 +1736,15 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  	mf->code = xlate->code;
>  	mf->colorspace = pix->colorspace;
>  
> -	ret = v4l2_device_call_until_err(sd->v4l2_dev, soc_camera_grp_id(icd),
> -					 pad, set_fmt, &pad_cfg, &format);
> -	if (ret < 0)
> +	format.pad = icd->src_pad_idx;
> +	ret = v4l2_device_call_until_err(sd->v4l2_dev,
> +					soc_camera_grp_id(icd), pad,
> +					set_fmt, pad_cfg,
> +					&format);
> +	if (ret < 0) {
> +		v4l2_subdev_free_pad_config(pad_cfg);
>  		return ret;

I would use a goto to the end of the function here.

> +	}
>  
>  	/* Adjust only if VIN cannot scale */
>  	if (pix->width > mf->width * 2)
> @@ -1759,13 +1766,15 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  			 */
>  			mf->width = VIN_MAX_WIDTH;
>  			mf->height = VIN_MAX_HEIGHT;
> +			format.pad = icd->src_pad_idx;
>  			ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  							 soc_camera_grp_id(icd),
> -							 pad, set_fmt, &pad_cfg,
> +							 pad, set_fmt, pad_cfg,
>  							 &format);
>  			if (ret < 0) {
>  				dev_err(icd->parent,
>  					"client try_fmt() = %d\n", ret);
> +				v4l2_subdev_free_pad_config(pad_cfg);
>  				return ret;

Ditto.

>  			}
>  		}
> @@ -1776,6 +1785,7 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  			pix->height = height;
>  	}
>  

With the error label here.

> +	v4l2_subdev_free_pad_config(pad_cfg);
>  	return ret;
>  }
>  
> 

Re: [PATCH 10/15] media: rcar_vin: Use correct pad number in try_fmt

[Guennadi Liakhovetski]

Hi William,

On Wed, 3 Jun 2015, William Towle wrote:

> Fix rcar_vin_try_fmt to use the correct pad number when calling the
> subdev set_fmt. Previously pad number 0 was always used, resulting in
> EINVAL if the subdev cares about the pad number (e.g. ADV7612).
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/rcar_vin.c |   20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index 00c1034..cc993bc 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1697,7 +1697,7 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  	const struct soc_camera_format_xlate *xlate;
>  	struct v4l2_pix_format *pix = &f->fmt.pix;
>  	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> -	struct v4l2_subdev_pad_config pad_cfg;
> +	struct v4l2_subdev_pad_config *pad_cfg;
>  	struct v4l2_subdev_format format = {
>  		.which = V4L2_SUBDEV_FORMAT_TRY,
>  	};
> @@ -1706,6 +1706,8 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  	int width, height;
>  	int ret;
>  
> +	pad_cfg = v4l2_subdev_alloc_pad_config(sd);

Don't you have to check for NULL here? Are all subdevice drivers supposed 
to be able to handle pad_cfg = NULL in their .set_fmt() methods? Doesn't 
look like that to me, looking at the v4l2_subdev_get_try_format() 
implementation.

Thanks
Guennadi

> +
>  	xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
>  	if (!xlate) {
>  		xlate = icd->current_fmt;
> @@ -1734,10 +1736,15 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  	mf->code = xlate->code;
>  	mf->colorspace = pix->colorspace;
>  
> -	ret = v4l2_device_call_until_err(sd->v4l2_dev, soc_camera_grp_id(icd),
> -					 pad, set_fmt, &pad_cfg, &format);
> -	if (ret < 0)
> +	format.pad = icd->src_pad_idx;
> +	ret = v4l2_device_call_until_err(sd->v4l2_dev,
> +					soc_camera_grp_id(icd), pad,
> +					set_fmt, pad_cfg,
> +					&format);
> +	if (ret < 0) {
> +		v4l2_subdev_free_pad_config(pad_cfg);
>  		return ret;
> +	}
>  
>  	/* Adjust only if VIN cannot scale */
>  	if (pix->width > mf->width * 2)
> @@ -1759,13 +1766,15 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  			 */
>  			mf->width = VIN_MAX_WIDTH;
>  			mf->height = VIN_MAX_HEIGHT;
> +			format.pad = icd->src_pad_idx;
>  			ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  							 soc_camera_grp_id(icd),
> -							 pad, set_fmt, &pad_cfg,
> +							 pad, set_fmt, pad_cfg,
>  							 &format);
>  			if (ret < 0) {
>  				dev_err(icd->parent,
>  					"client try_fmt() = %d\n", ret);
> +				v4l2_subdev_free_pad_config(pad_cfg);
>  				return ret;
>  			}
>  		}
> @@ -1776,6 +1785,7 @@ static int rcar_vin_try_fmt(struct soc_camera_device *icd,
>  			pix->height = height;
>  	}
>  
> +	v4l2_subdev_free_pad_config(pad_cfg);
>  	return ret;
>  }
>  
> -- 
> 1.7.10.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in

[PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad number in try_fmt

[William Towle]

From: Rob Taylor <rob.taylor@codethink.co.uk>

Fix calls to subdev try_fmt to use correct pad. Fixes failures with
subdevs that care about having the right pad number set.

Signed-off-by: William Towle <william.towle@codethink.co.uk>
Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 drivers/media/platform/soc_camera/soc_scale_crop.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/platform/soc_camera/soc_scale_crop.c b/drivers/media/platform/soc_camera/soc_scale_crop.c
index bda29bc..90e2769 100644
--- a/drivers/media/platform/soc_camera/soc_scale_crop.c
+++ b/drivers/media/platform/soc_camera/soc_scale_crop.c
@@ -225,6 +225,10 @@ static int client_set_fmt(struct soc_camera_device *icd,
 	bool host_1to1;
 	int ret;
 
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	format->pad = icd->src_pad_idx;
+#endif
+
 	ret = v4l2_device_call_until_err(sd->v4l2_dev,
 					 soc_camera_grp_id(icd), pad,
 					 set_fmt, NULL, format);
@@ -261,10 +265,16 @@ static int client_set_fmt(struct soc_camera_device *icd,
 	/* width <= max_width && height <= max_height - guaranteed by try_fmt */
 	while ((width > tmp_w || height > tmp_h) &&
 	       tmp_w < max_width && tmp_h < max_height) {
+
 		tmp_w = min(2 * tmp_w, max_width);
 		tmp_h = min(2 * tmp_h, max_height);
 		mf->width = tmp_w;
 		mf->height = tmp_h;
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+		format->pad = icd->src_pad_idx;
+#endif
+
 		ret = v4l2_device_call_until_err(sd->v4l2_dev,
 					soc_camera_grp_id(icd), pad,
 					set_fmt, NULL, format);
-- 
1.7.10.4

Re: [PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad number in try_fmt

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
> From: Rob Taylor <rob.taylor@codethink.co.uk>
> 
> Fix calls to subdev try_fmt to use correct pad. Fixes failures with
> subdevs that care about having the right pad number set.
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/soc_scale_crop.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/media/platform/soc_camera/soc_scale_crop.c b/drivers/media/platform/soc_camera/soc_scale_crop.c
> index bda29bc..90e2769 100644
> --- a/drivers/media/platform/soc_camera/soc_scale_crop.c
> +++ b/drivers/media/platform/soc_camera/soc_scale_crop.c
> @@ -225,6 +225,10 @@ static int client_set_fmt(struct soc_camera_device *icd,
>  	bool host_1to1;
>  	int ret;
>  
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	format->pad = icd->src_pad_idx;
> +#endif

As mentioned in the review of patch 9 it should be possible to drop the
#if defined() here.

> +
>  	ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  					 soc_camera_grp_id(icd), pad,
>  					 set_fmt, NULL, format);
> @@ -261,10 +265,16 @@ static int client_set_fmt(struct soc_camera_device *icd,
>  	/* width <= max_width && height <= max_height - guaranteed by try_fmt */
>  	while ((width > tmp_w || height > tmp_h) &&
>  	       tmp_w < max_width && tmp_h < max_height) {
> +

Spurious whitespace change.

>  		tmp_w = min(2 * tmp_w, max_width);
>  		tmp_h = min(2 * tmp_h, max_height);
>  		mf->width = tmp_w;
>  		mf->height = tmp_h;
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +		format->pad = icd->src_pad_idx;
> +#endif

Same as the first comment.

> +
>  		ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  					soc_camera_grp_id(icd), pad,
>  					set_fmt, NULL, format);
> 

Regards,

	Hans

Re: [PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad number in try_fmt

[Guennadi Liakhovetski]

On Wed, 3 Jun 2015, William Towle wrote:

> From: Rob Taylor <rob.taylor@codethink.co.uk>
> 
> Fix calls to subdev try_fmt to use correct pad. Fixes failures with
> subdevs that care about having the right pad number set.
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/soc_scale_crop.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/media/platform/soc_camera/soc_scale_crop.c b/drivers/media/platform/soc_camera/soc_scale_crop.c
> index bda29bc..90e2769 100644
> --- a/drivers/media/platform/soc_camera/soc_scale_crop.c
> +++ b/drivers/media/platform/soc_camera/soc_scale_crop.c
> @@ -225,6 +225,10 @@ static int client_set_fmt(struct soc_camera_device *icd,
>  	bool host_1to1;
>  	int ret;
>  
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	format->pad = icd->src_pad_idx;
> +#endif
> +
>  	ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  					 soc_camera_grp_id(icd), pad,
>  					 set_fmt, NULL, format);
> @@ -261,10 +265,16 @@ static int client_set_fmt(struct soc_camera_device *icd,
>  	/* width <= max_width && height <= max_height - guaranteed by try_fmt */
>  	while ((width > tmp_w || height > tmp_h) &&
>  	       tmp_w < max_width && tmp_h < max_height) {
> +

I usually don't use an empty line directly after an opening brace. Please, 
drop.

Thanks
Guennadi

>  		tmp_w = min(2 * tmp_w, max_width);
>  		tmp_h = min(2 * tmp_h, max_height);
>  		mf->width = tmp_w;
>  		mf->height = tmp_h;
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +		format->pad = icd->src_pad_idx;
> +#endif
> +
>  		ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  					soc_camera_grp_id(icd), pad,
>  					set_fmt, NULL, format);
> -- 
> 1.7.10.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in

Re: [PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad number in try_fmt

[Guennadi Liakhovetski]

On Wed, 3 Jun 2015, William Towle wrote:

> From: Rob Taylor <rob.taylor@codethink.co.uk>
> 
> Fix calls to subdev try_fmt to use correct pad. Fixes failures with
> subdevs that care about having the right pad number set.
> 
> Signed-off-by: William Towle <william.towle@codethink.co.uk>
> Reviewed-by: Rob Taylor <rob.taylor@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/soc_scale_crop.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/media/platform/soc_camera/soc_scale_crop.c b/drivers/media/platform/soc_camera/soc_scale_crop.c
> index bda29bc..90e2769 100644
> --- a/drivers/media/platform/soc_camera/soc_scale_crop.c
> +++ b/drivers/media/platform/soc_camera/soc_scale_crop.c
> @@ -225,6 +225,10 @@ static int client_set_fmt(struct soc_camera_device *icd,
>  	bool host_1to1;
>  	int ret;
>  
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	format->pad = icd->src_pad_idx;
> +#endif
> +
>  	ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  					 soc_camera_grp_id(icd), pad,
>  					 set_fmt, NULL, format);
> @@ -261,10 +265,16 @@ static int client_set_fmt(struct soc_camera_device *icd,
>  	/* width <= max_width && height <= max_height - guaranteed by try_fmt */
>  	while ((width > tmp_w || height > tmp_h) &&
>  	       tmp_w < max_width && tmp_h < max_height) {
> +
>  		tmp_w = min(2 * tmp_w, max_width);
>  		tmp_h = min(2 * tmp_h, max_height);
>  		mf->width = tmp_w;
>  		mf->height = tmp_h;
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +		format->pad = icd->src_pad_idx;

BTW, can format->pad be changed by subdev's .set_fmt()? Do you really have 
to set it in a loop?

Thanks
Guennadi

> +#endif
> +
>  		ret = v4l2_device_call_until_err(sd->v4l2_dev,
>  					soc_camera_grp_id(icd), pad,
>  					set_fmt, NULL, format);
> -- 
> 1.7.10.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in

[PATCH 12/15] media: soc_camera: Fill std field in enum_input

[William Towle]

From: Hans Verkuil <hans.verkuil@cisco.com>

Fill in the std field from the video_device tvnorms field in
enum_input.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 drivers/media/platform/soc_camera/soc_camera.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index c4952c8..51daeb1 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -309,11 +309,14 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 static int soc_camera_enum_input(struct file *file, void *priv,
 				 struct v4l2_input *inp)
 {
+	struct soc_camera_device *icd = file->private_data;
+
 	if (inp->index != 0)
 		return -EINVAL;
 
 	/* default is camera */
 	inp->type = V4L2_INPUT_TYPE_CAMERA;
+	inp->std = icd->vdev->tvnorms;
 	strcpy(inp->name, "Camera");
 
 	return 0;
-- 
1.7.10.4

[PATCH 13/15] media: soc_camera: Fix error reporting in expbuf

[William Towle]

From: Hans Verkuil <hans.verkuil@cisco.com>

Remove unnecessary check and fix the error code for vb1 drivers.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
---
 drivers/media/platform/soc_camera/soc_camera.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index 51daeb1..4e59833 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -470,14 +470,10 @@ static int soc_camera_expbuf(struct file *file, void *priv,
 	struct soc_camera_device *icd = file->private_data;
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 
-	if (icd->streamer != file)
-		return -EBUSY;
-
 	/* videobuf2 only */
 	if (ici->ops->init_videobuf)
-		return -EINVAL;
-	else
-		return vb2_expbuf(&icd->vb2_vidq, p);
+		return -ENOTTY;
+	return vb2_expbuf(&icd->vb2_vidq, p);
 }
 
 /* Always entered with .host_lock held */
-- 
1.7.10.4

[PATCH 14/15] media: soc_camera: fill in bus_info field

[William Towle]

From: Rob Taylor <rob.taylor@codethink.co.uk>

Adapt soc_camera_querycap() so that cap->bus_info is populated in
addition to cap->driver.

Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
Reviewed-by: William Towle <william.towle@codethink.co.uk>
---
 drivers/media/platform/soc_camera/soc_camera.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index 4e59833..675cfc4 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -954,6 +954,7 @@ static int soc_camera_querycap(struct file *file, void  *priv,
 	WARN_ON(priv != file->private_data);
 
 	strlcpy(cap->driver, ici->drv_name, sizeof(cap->driver));
+	strlcpy(cap->bus_info, "platform:soc_camera", sizeof(cap->bus_info));
 	return ici->ops->querycap(ici, cap);
 }
 
-- 
1.7.10.4

Re: [PATCH 14/15] media: soc_camera: fill in bus_info field

[Hans Verkuil]

On 06/03/2015 04:00 PM, William Towle wrote:
> From: Rob Taylor <rob.taylor@codethink.co.uk>
> 
> Adapt soc_camera_querycap() so that cap->bus_info is populated in
> addition to cap->driver.
> 
> Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
> Reviewed-by: William Towle <william.towle@codethink.co.uk>
> ---
>  drivers/media/platform/soc_camera/soc_camera.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
> index 4e59833..675cfc4 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
> @@ -954,6 +954,7 @@ static int soc_camera_querycap(struct file *file, void  *priv,
>  	WARN_ON(priv != file->private_data);
>  
>  	strlcpy(cap->driver, ici->drv_name, sizeof(cap->driver));
> +	strlcpy(cap->bus_info, "platform:soc_camera", sizeof(cap->bus_info));

Sorry, but this is the wrong place. This should be done in rcar_vin_querycap.

If you have multiple soc_camera instances, then the bus_info should be unique
for each. And that's obviously not the case if it's done here.

The rcar driver will know, however.

Regards,

	Hans

>  	return ici->ops->querycap(ici, cap);
>  }
>  
> 

[PATCH 15/15] media: rcar_vin: Reject videobufs that are too small for current format

[William Towle]

From: Rob Taylor <rob.taylor@codethink.co.uk>

In videobuf_setup reject buffers that are too small for the configured
format. Fixes v4l2-complience issue.

Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
Reviewed-by: William Towle <william.towle@codethink.co.uk>
---
 drivers/media/platform/soc_camera/rcar_vin.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index cc993bc..1531a76 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -541,6 +541,9 @@ static int rcar_vin_videobuf_setup(struct vb2_queue *vq,
 		unsigned int bytes_per_line;
 		int ret;
 
+		if (fmt->fmt.pix.sizeimage < icd->sizeimage)
+			return -EINVAL;
+
 		xlate = soc_camera_xlate_by_fourcc(icd,
 						   fmt->fmt.pix.pixelformat);
 		if (!xlate)
-- 
1.7.10.4

Re: [PATCH 15/15] media: rcar_vin: Reject videobufs that are too small for current format

[Hans Verkuil]

On 06/03/2015 04:00 PM, William Towle wrote:
> From: Rob Taylor <rob.taylor@codethink.co.uk>
> 
> In videobuf_setup reject buffers that are too small for the configured
> format. Fixes v4l2-complience issue.
> 
> Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
> Reviewed-by: William Towle <william.towle@codethink.co.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks,

	Hans

> ---
>  drivers/media/platform/soc_camera/rcar_vin.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
> index cc993bc..1531a76 100644
> --- a/drivers/media/platform/soc_camera/rcar_vin.c
> +++ b/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -541,6 +541,9 @@ static int rcar_vin_videobuf_setup(struct vb2_queue *vq,
>  		unsigned int bytes_per_line;
>  		int ret;
>  
> +		if (fmt->fmt.pix.sizeimage < icd->sizeimage)
> +			return -EINVAL;
> +
>  		xlate = soc_camera_xlate_by_fourcc(icd,
>  						   fmt->fmt.pix.pixelformat);
>  		if (!xlate)
> 

Re: HDMI and Composite capture on Lager, for kernel 4.1, version 3

[Hans Verkuil]

On 06/03/2015 03:59 PM, William Towle wrote:
>   Version 3. Obsoletes version 2, as seen at:
> 	http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/91668
> 
>   Key changes in this version: this has some reworking of the adv7604
> driver probe and soc_camera initialisation functions. In addition,
> we give rcar_vin.c a dependency on CONFIG_MEDIA_CONTROLLER in line with
> the drivers used with it.

I'm not sure if I've asked this before, but shouldn't soc-camera be extended
with support for the DV_TIMINGS ioctls in order to control the adv7604?

It's peculiar that that is not included in this patch series...

Regards,

	Hans

> 
> Cheers,
>   Wills.
> 
> To follow:
> 	[PATCH 01/15] ARM: shmobile: lager dts: Add entries for VIN HDMI
> 	[PATCH 02/15] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888
> 	[PATCH 03/15] media: adv7180: add of match table
> 	[PATCH 04/15] media: adv7604: chip info and formats for ADV7612
> 	[PATCH 05/15] media: adv7604: document support for ADV7612 dual HDMI
> 	[PATCH 06/15] media: adv7604: ability to read default input port
> 	[PATCH 07/15] ARM: shmobile: lager dts: specify default-input for
> 	[PATCH 08/15] v4l: subdev: Add pad config allocator and init
> 	[PATCH 09/15] media: soc_camera pad-aware driver initialisation
> 	[PATCH 10/15] media: rcar_vin: Use correct pad number in try_fmt
> 	[PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad
> 	[PATCH 12/15] media: soc_camera: Fill std field in enum_input
> 	[PATCH 13/15] media: soc_camera: Fix error reporting in expbuf
> 	[PATCH 14/15] media: soc_camera: fill in bus_info field
> 	[PATCH 15/15] media: rcar_vin: Reject videobufs that are too small
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in

Re: HDMI and Composite capture on Lager, for kernel 4.1, version 3

[William Towle]

Hi Hans,
> I'm not sure if I've asked this before, but shouldn't soc-camera be extended
> with support for the DV_TIMINGS ioctls in order to control the adv7604?
>
> It's peculiar that that is not included in this patch series...

   Rob tells me he did some work on this, but we don't have it passing
muster with v4l2-compliance and since gstreamer tries a number of
resolutions of its own accord and subsequently produces images and
video without it we can make do without for our needs. I can include it
in or alongside the next submission if you'd to see it.


   Regarding your other comments, in particular the specification of the
following:
 		.tdms_lock_mask = 0x43,
 		.cable_det_mask = 0x01,
...when testing just the first input, these seem reasonable - in the
case of tdms_lock_mask the two least significant bits represent
V_LOCKED_RAW and DE_REGEN_LCK_RAW, and they are set in line with
the value of TDMSPLL_LCK_A_RAW when queried.

   While an implementation suitable for testing both of the ADV7612's
A and B inputs could just add TDMSPLL_LCK_B_RAW to tdms_lock mask,
the cable detect function isn't so trivial: it would need to query two
separate (and numerically non-adjacent) registers to do its job.

   For simplicity we would like to propose that our next iteration
supports just the first input (which is sufficient for our current
needs) and has commentary in appropriate places regarding its
shortcomings, although we're open to alternative suggestions if there
has already been discussion on the matter here.

Cheers,
   Wills.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in

Re: HDMI and Composite capture on Lager, for kernel 4.1, version 3

[Hans Verkuil]

On 06/22/2015 04:41 PM, William Towle wrote:
> 
> Hi Hans,
>> I'm not sure if I've asked this before, but shouldn't soc-camera be extended
>> with support for the DV_TIMINGS ioctls in order to control the adv7604?
>>
>> It's peculiar that that is not included in this patch series...
> 
>    Rob tells me he did some work on this, but we don't have it passing
> muster with v4l2-compliance and since gstreamer tries a number of
> resolutions of its own accord and subsequently produces images and
> video without it we can make do without for our needs. I can include it
> in or alongside the next submission if you'd to see it.

Another reason not to use soc-camera.

It's really not a very suitable framework for this hardware.

> 
>    Regarding your other comments, in particular the specification of the
> following:
>  		.tdms_lock_mask = 0x43,
>  		.cable_det_mask = 0x01,
> ...when testing just the first input, these seem reasonable - in the
> case of tdms_lock_mask the two least significant bits represent
> V_LOCKED_RAW and DE_REGEN_LCK_RAW, and they are set in line with
> the value of TDMSPLL_LCK_A_RAW when queried.
> 
>    While an implementation suitable for testing both of the ADV7612's
> A and B inputs could just add TDMSPLL_LCK_B_RAW to tdms_lock mask,
> the cable detect function isn't so trivial: it would need to query two
> separate (and numerically non-adjacent) registers to do its job.
> 
>    For simplicity we would like to propose that our next iteration
> supports just the first input (which is sufficient for our current
> needs) and has commentary in appropriate places regarding its
> shortcomings, although we're open to alternative suggestions if there
> has already been discussion on the matter here.

I think supporting just the first input and commenting accordingly is the
best approach for now.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in

HDMI and Composite capture on Lager, for kernel 4.1, version 4

[William Towle]

  Version 4. Obsoletes version 3, as seen at:
	http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/91931

  This version has the bus_info field set in rcar_vin.c, where the
value used can be constructed uniquely. Elsewhere a number of small
optimisations have been made, and some redundant initialisation code
has been removed.

To follow:
	[PATCH 01/15] ARM: shmobile: lager dts: Add entries for VIN HDMI
	[PATCH 02/15] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888
	[PATCH 03/15] media: adv7180: add of match table
	[PATCH 04/15] media: adv7604: chip info and formats for ADV7612
	[PATCH 05/15] media: adv7604: document support for ADV7612 dual HDMI
	[PATCH 06/15] media: adv7604: ability to read default input port
	[PATCH 07/15] ARM: shmobile: lager dts: specify default-input for
	[PATCH 08/15] v4l: subdev: Add pad config allocator and init
	[PATCH 09/15] media: soc_camera pad-aware driver initialisation
	[PATCH 10/15] media: rcar_vin: Use correct pad number in try_fmt
	[PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad
	[PATCH 12/15] media: soc_camera: Fill std field in enum_input
	[PATCH 13/15] media: soc_camera: Fix error reporting in expbuf
	[PATCH 14/15] media: rcar_vin: fill in bus_info field
	[PATCH 15/15] media: rcar_vin: Reject videobufs that are too small

Cheers,
  Wills.

[PATCH 08/15] v4l: subdev: Add pad config allocator and init

[William Towle]

From: Laurent Pinchart <laurent.pinchart@linaro.org>

Add a new subdev operation to initialize a subdev pad config array, and
a helper function to allocate and initialize the array. This can be used
by bridge drivers to implement try format based on subdev pad
operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@linaro.org>
Acked-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

Changes since v1:

- Added v4l2_subdev_free_pad_config
---
 drivers/media/v4l2-core/v4l2-subdev.c |   19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           |   10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 6359606..d594fe5 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -35,7 +35,7 @@
 static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
 {
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-	fh->pad = kzalloc(sizeof(*fh->pad) * sd->entity.num_pads, GFP_KERNEL);
+	fh->pad = v4l2_subdev_alloc_pad_config(sd);
 	if (fh->pad == NULL)
 		return -ENOMEM;
 #endif
@@ -569,6 +569,23 @@ int v4l2_subdev_link_validate(struct media_link *link)
 		sink, link, &source_fmt, &sink_fmt);
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd)
+{
+	struct v4l2_subdev_pad_config *cfg;
+
+	if (!sd->entity.num_pads)
+		return NULL;
+
+	cfg = kcalloc(sd->entity.num_pads, sizeof(*cfg), GFP_KERNEL);
+	if (!cfg)
+		return NULL;
+
+	v4l2_subdev_call(sd, pad, init_cfg, cfg);
+
+	return cfg;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_pad_config);
 #endif /* CONFIG_MEDIA_CONTROLLER */
 
 void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index dc20102..4a609f6 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -485,6 +485,8 @@ struct v4l2_subdev_pad_config {
  *                  may be adjusted by the subdev driver to device capabilities.
  */
 struct v4l2_subdev_pad_ops {
+	void (*init_cfg)(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg);
 	int (*enum_mbus_code)(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_mbus_code_enum *code);
@@ -677,7 +679,15 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_format *source_fmt,
 				      struct v4l2_subdev_format *sink_fmt);
 int v4l2_subdev_link_validate(struct media_link *link);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd);
+
+static inline void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg)
+{
+	kfree(cfg);
+}
 #endif /* CONFIG_MEDIA_CONTROLLER */
+
 void v4l2_subdev_init(struct v4l2_subdev *sd,
 		      const struct v4l2_subdev_ops *ops);
 
-- 
1.7.10.4

HDMI and Composite capture on Lager, for kernel 4.1, version 2

[William Towle]

  Version 2 of the patchset sent last week, ie.
	http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/91423

  In response to comments, this version has: fixes regarding use of
CONFIG_MEDIA_CONTROLLER defines; an additional line for the RGB888
support patch (required by parallel changes); removal of more vestigial
format handling; the set no longer has its debugging changes and now
incorporates the patch at https://patchwork.linuxtv.org/patch/29807/

Cheers,
  Wills.

To follow:
	[PATCH 01/15] ARM: shmobile: lager dts: Add entries for VIN HDMI
	[PATCH 02/15] media: soc_camera: rcar_vin: Add BT.709 24-bit RGB888
	[PATCH 03/15] media: adv7180: add of match table
	[PATCH 04/15] media: adv7604: chip info and formats for ADV7612
	[PATCH 05/15] media: adv7604: document support for ADV7612 dual HDMI
	[PATCH 06/15] media: adv7604: ability to read default input port
	[PATCH 07/15] ARM: shmobile: lager dts: specify default-input for
	[PATCH 08/15] v4l: subdev: Add pad config allocator and init
	[PATCH 09/15] media: soc_camera pad-aware driver initialisation
	[PATCH 10/15] media: rcar_vin: Use correct pad number in try_fmt
	[PATCH 11/15] media: soc_camera: soc_scale_crop: Use correct pad
	[PATCH 12/15] media: soc_camera: Fill std field in enum_input
	[PATCH 13/15] media: soc_camera: Fix error reporting in expbuf
	[PATCH 14/15] media: soc_camera: v4l2-compliance fixes for querycap
	[PATCH 15/15] media: rcar_vin: Reject videobufs that are too small

[PATCH 08/15] v4l: subdev: Add pad config allocator and init

[William Towle]

From: Laurent Pinchart <laurent.pinchart@linaro.org>

Add a new subdev operation to initialize a subdev pad config array, and
a helper function to allocate and initialize the array. This can be used
by bridge drivers to implement try format based on subdev pad
operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@linaro.org>
Acked-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

Changes since v1:

- Added v4l2_subdev_free_pad_config
---
 drivers/media/v4l2-core/v4l2-subdev.c |   19 ++++++++++++++++++-
 include/media/v4l2-subdev.h           |   10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 6359606..d594fe5 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -35,7 +35,7 @@
 static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
 {
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-	fh->pad = kzalloc(sizeof(*fh->pad) * sd->entity.num_pads, GFP_KERNEL);
+	fh->pad = v4l2_subdev_alloc_pad_config(sd);
 	if (fh->pad == NULL)
 		return -ENOMEM;
 #endif
@@ -569,6 +569,23 @@ int v4l2_subdev_link_validate(struct media_link *link)
 		sink, link, &source_fmt, &sink_fmt);
 }
 EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd)
+{
+	struct v4l2_subdev_pad_config *cfg;
+
+	if (!sd->entity.num_pads)
+		return NULL;
+
+	cfg = kcalloc(sd->entity.num_pads, sizeof(*cfg), GFP_KERNEL);
+	if (!cfg)
+		return NULL;
+
+	v4l2_subdev_call(sd, pad, init_cfg, cfg);
+
+	return cfg;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_pad_config);
 #endif /* CONFIG_MEDIA_CONTROLLER */
 
 void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index dc20102..4a609f6 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -485,6 +485,8 @@ struct v4l2_subdev_pad_config {
  *                  may be adjusted by the subdev driver to device capabilities.
  */
 struct v4l2_subdev_pad_ops {
+	void (*init_cfg)(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg);
 	int (*enum_mbus_code)(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_mbus_code_enum *code);
@@ -677,7 +679,15 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd,
 				      struct v4l2_subdev_format *source_fmt,
 				      struct v4l2_subdev_format *sink_fmt);
 int v4l2_subdev_link_validate(struct media_link *link);
+
+struct v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd);
+
+static inline void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg)
+{
+	kfree(cfg);
+}
 #endif /* CONFIG_MEDIA_CONTROLLER */
+
 void v4l2_subdev_init(struct v4l2_subdev *sd,
 		      const struct v4l2_subdev_ops *ops);
 
-- 
1.7.10.4