Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
On Sun April 21 2013 20:40:30 Sergei Shtylyov wrote: From: Vladimir Barinov vladimir.bari...@cogentembedded.com Add OKI Semiconductor ML86V7667 video decoder driver. Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com [Sergei: added v4l2_device_unregister_subdev() call to the error cleanup path of ml86v7667_probe(); some cleanup.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- Changes since the original posting: - fixed ACCC_CHROMA_CB_MASK; - got rid from the autodetection feature; - removed querystd() method calls from other methods; - removed deprecated g_chip_ident() method. drivers/media/i2c/Kconfig |9 drivers/media/i2c/Makefile|1 drivers/media/i2c/ml86v7667.c | 473 ++ 3 files changed, 483 insertions(+) Index: renesas/drivers/media/i2c/Kconfig === --- renesas.orig/drivers/media/i2c/Kconfig +++ renesas/drivers/media/i2c/Kconfig @@ -227,6 +227,15 @@ config VIDEO_KS0127 To compile this driver as a module, choose M here: the module will be called ks0127. +config VIDEO_ML86V7667 + tristate OKI ML86V7667 video decoder + depends on VIDEO_V4L2 I2C + ---help--- + Support for the OKI Semiconductor ML86V7667 video decoder. + + To compile this driver as a module, choose M here: the + module will be called ml86v7667. + config VIDEO_SAA7110 tristate Philips SAA7110 video decoder depends on VIDEO_V4L2 I2C Index: renesas/drivers/media/i2c/Makefile === --- renesas.orig/drivers/media/i2c/Makefile +++ renesas/drivers/media/i2c/Makefile @@ -64,3 +64,4 @@ obj-$(CONFIG_VIDEO_AS3645A) += as3645a.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o obj-$(CONFIG_VIDEO_AK881X) += ak881x.o obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o +obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o Index: renesas/drivers/media/i2c/ml86v7667.c === --- /dev/null +++ renesas/drivers/media/i2c/ml86v7667.c @@ -0,0 +1,473 @@ +/* + * OKI Semiconductor ML86V7667 video decoder driver + * + * Author: Vladimir Barinov sou...@cogentembedded.com + * Copyright (C) 2013 Cogent Embedded, Inc. + * Copyright (C) 2013 Renesas Solutions Corp. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include linux/init.h +#include linux/module.h +#include linux/i2c.h +#include linux/slab.h +#include linux/videodev2.h +#include media/v4l2-chip-ident.h This include should be removed as well. +#include media/v4l2-subdev.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h + +#define DRV_NAME ml86v7667 + +/* Subaddresses */ +#define MRA_REG 0x00 /* Mode Register A */ +#define MRC_REG 0x02 /* Mode Register C */ +#define LUMC_REG 0x0C /* Luminance Control */ +#define CLC_REG 0x10 /* Contrast level control */ +#define SSEPL_REG0x11 /* Sync separation level */ +#define CHRCA_REG0x12 /* Chrominance Control A */ +#define ACCC_REG 0x14 /* ACC Loop filter Chrominance control */ +#define ACCRC_REG0x15 /* ACC Reference level control */ +#define HUE_REG 0x16 /* Hue control */ +#define ADC2_REG 0x1F /* ADC Register 2 */ +#define PLLR1_REG0x20 /* PLL Register 1 */ +#define STATUS_REG 0x2C /* STATUS Register */ + +/* Mode Register A register bits */ +#define MRA_OUTPUT_MODE_MASK (3 6) +#define MRA_ITUR_BT601 (1 6) +#define MRA_ITUR_BT656 (0 6) +#define MRA_INPUT_MODE_MASK (7 3) +#define MRA_PAL_BT601(4 3) +#define MRA_NTSC_BT601 (0 3) +#define MRA_REGISTER_MODE(1 0) + +/* Mode Register C register bits */ +#define MRC_AUTOSELECT (1 7) + +/* Luminance Control register bits */ +#define LUMC_ONOFF_SHIFT 7 +#define LUMC_ONOFF_MASK (1 7) + +/* Contrast level control register bits */ +#define CLC_CONTRAST_ONOFF (1 7) +#define CLC_CONTRAST_MASK0x0F + +/* Sync separation level register bits */ +#define SSEPL_LUMINANCE_ONOFF(1 7) +#define SSEPL_LUMINANCE_MASK 0x7F + +/* Chrominance Control A register bits */ +#define CHRCA_MODE_SHIFT 6 +#define CHRCA_MODE_MASK (1 6) + +/* ACC Loop filter Chrominance control register bits */ +#define ACCC_CHROMA_CR_SHIFT 3 +#define ACCC_CHROMA_CR_MASK (7 3) +#define ACCC_CHROMA_CB_SHIFT 0
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
Hi Hans,
Thank you for the review.
Hans Verkuil wrote:
+#include media/v4l2-chip-ident.h
This include should be removed as well.
ok
+
+static int ml86v7667_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
+{
+ struct ml86v7667_priv *priv = to_ml86v7667(sd);
+
+ *std = priv-std;
That's not right. querystd should attempt to detect the standard, that's
what it is for. It should just return the detected standard, not actually
change it.
Ok.
I've mixed the things up with your review on removing the autoselection
feature and detection.
Thx for pointing on this.
+*/
+ val = i2c_smbus_read_byte_data(client, STATUS_REG);
+ if (val 0)
+ return val;
+
+ priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC;
Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards
and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL.
In the reference manual it is not mentioned about 50/60Hz input format
selection/detection but it mentioned just PAL/NTSC.
The 50hz formats can be ether PAL and NTSC formats variants. The same is
applied to 60Hz.
In the ML86V7667 datasheet the description for STATUS register detection
bit is just PAL/NTSC:
$2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL
If you assure me that I must judge their description as 50 vs 60Hz
formats and not PAL/NTSC then I will make the change.
Regards,
Vladimir
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
On Mon April 22 2013 10:39:42 Vladimir Barinov wrote:
Hi Hans,
Thank you for the review.
Hans Verkuil wrote:
+#include media/v4l2-chip-ident.h
This include should be removed as well.
ok
+
+static int ml86v7667_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
+{
+ struct ml86v7667_priv *priv = to_ml86v7667(sd);
+
+ *std = priv-std;
That's not right. querystd should attempt to detect the standard, that's
what it is for. It should just return the detected standard, not actually
change it.
Ok.
I've mixed the things up with your review on removing the autoselection
feature and detection.
Thx for pointing on this.
+ */
+ val = i2c_smbus_read_byte_data(client, STATUS_REG);
+ if (val 0)
+ return val;
+
+ priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC;
Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards
and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL.
In the reference manual it is not mentioned about 50/60Hz input format
selection/detection but it mentioned just PAL/NTSC.
The 50hz formats can be ether PAL and NTSC formats variants. The same is
applied to 60Hz.
In the ML86V7667 datasheet the description for STATUS register detection
bit is just PAL/NTSC:
$2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL
If you assure me that I must judge their description as 50 vs 60Hz
formats and not PAL/NTSC then I will make the change.
I can't judge that. Are there no status bits anywhere that tell you something
about the number of lines per frame or the framerate?
Are you able to test with a PAL-M or PAL-N(c) input?
Can you ask the manufacturer for more information?
If the answer to all of these is 'no', then stick to STD_PAL/NTSC.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
Hi Hans, Hans Verkuil wrote: +*/ + val = i2c_smbus_read_byte_data(client, STATUS_REG); + if (val 0) + return val; + + priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC; Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL. In the reference manual it is not mentioned about 50/60Hz input format selection/detection but it mentioned just PAL/NTSC. The 50hz formats can be ether PAL and NTSC formats variants. The same is applied to 60Hz. In the ML86V7667 datasheet the description for STATUS register detection bit is just PAL/NTSC: $2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL If you assure me that I must judge their description as 50 vs 60Hz formats and not PAL/NTSC then I will make the change. I can't judge that. Are there no status bits anywhere that tell you something about the number of lines per frame or the framerate? You are right. I've found a relationship table with description of number of total H/V pixels vs Video Modes mentioned in datasheet. It's NTSC has Odd/263 and Even/262 vertical lines. The PAL has Odd/312 and Even/313. So I will change the standard per your suggestion. Are you able to test with a PAL-M or PAL-N(c) input? Unfortunately I cannot. I have a couple of different cameras and all of them mention PAL output with number of lines in the technical manual. All of them are 625 lines. Can you ask the manufacturer for more information? It can take a while for waiting their feedback since OKI was significantly reorganized. Than you very much for your valuable feedback/review. Regards, Vladimir -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
Vladimir Barinov wrote: Hi Hans, Hans Verkuil wrote: + */ +val = i2c_smbus_read_byte_data(client, STATUS_REG); +if (val 0) +return val; + +priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC; Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL. In the reference manual it is not mentioned about 50/60Hz input format selection/detection but it mentioned just PAL/NTSC. The 50hz formats can be ether PAL and NTSC formats variants. The same is applied to 60Hz. In the ML86V7667 datasheet the description for STATUS register detection bit is just PAL/NTSC: $2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL If you assure me that I must judge their description as 50 vs 60Hz formats and not PAL/NTSC then I will make the change. I can't judge that. Are there no status bits anywhere that tell you something about the number of lines per frame or the framerate? You are right. I've found a relationship table with description of number of total H/V pixels vs Video Modes mentioned in datasheet. It's NTSC has Odd/263 and Even/262 vertical lines. The PAL has Odd/312 and Even/313. A little clarification: it's NTSC relates to 485 active lines (after rejection of blank lines) and PAL relates to 578 active lines. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
From: Vladimir Barinov vladimir.bari...@cogentembedded.com Add OKI Semiconductor ML86V7667 video decoder driver. Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com [Sergei: added v4l2_device_unregister_subdev() call to the error cleanup path of ml86v7667_probe(); some cleanup.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- Changes since the original posting: - fixed ACCC_CHROMA_CB_MASK; - got rid from the autodetection feature; - removed querystd() method calls from other methods; - removed deprecated g_chip_ident() method. drivers/media/i2c/Kconfig |9 drivers/media/i2c/Makefile|1 drivers/media/i2c/ml86v7667.c | 473 ++ 3 files changed, 483 insertions(+) Index: renesas/drivers/media/i2c/Kconfig === --- renesas.orig/drivers/media/i2c/Kconfig +++ renesas/drivers/media/i2c/Kconfig @@ -227,6 +227,15 @@ config VIDEO_KS0127 To compile this driver as a module, choose M here: the module will be called ks0127. +config VIDEO_ML86V7667 + tristate OKI ML86V7667 video decoder + depends on VIDEO_V4L2 I2C + ---help--- + Support for the OKI Semiconductor ML86V7667 video decoder. + + To compile this driver as a module, choose M here: the + module will be called ml86v7667. + config VIDEO_SAA7110 tristate Philips SAA7110 video decoder depends on VIDEO_V4L2 I2C Index: renesas/drivers/media/i2c/Makefile === --- renesas.orig/drivers/media/i2c/Makefile +++ renesas/drivers/media/i2c/Makefile @@ -64,3 +64,4 @@ obj-$(CONFIG_VIDEO_AS3645A) += as3645a.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o obj-$(CONFIG_VIDEO_AK881X) += ak881x.o obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o +obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o Index: renesas/drivers/media/i2c/ml86v7667.c === --- /dev/null +++ renesas/drivers/media/i2c/ml86v7667.c @@ -0,0 +1,473 @@ +/* + * OKI Semiconductor ML86V7667 video decoder driver + * + * Author: Vladimir Barinov sou...@cogentembedded.com + * Copyright (C) 2013 Cogent Embedded, Inc. + * Copyright (C) 2013 Renesas Solutions Corp. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include linux/init.h +#include linux/module.h +#include linux/i2c.h +#include linux/slab.h +#include linux/videodev2.h +#include media/v4l2-chip-ident.h +#include media/v4l2-subdev.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h + +#define DRV_NAME ml86v7667 + +/* Subaddresses */ +#define MRA_REG0x00 /* Mode Register A */ +#define MRC_REG0x02 /* Mode Register C */ +#define LUMC_REG 0x0C /* Luminance Control */ +#define CLC_REG0x10 /* Contrast level control */ +#define SSEPL_REG 0x11 /* Sync separation level */ +#define CHRCA_REG 0x12 /* Chrominance Control A */ +#define ACCC_REG 0x14 /* ACC Loop filter Chrominance control */ +#define ACCRC_REG 0x15 /* ACC Reference level control */ +#define HUE_REG0x16 /* Hue control */ +#define ADC2_REG 0x1F /* ADC Register 2 */ +#define PLLR1_REG 0x20 /* PLL Register 1 */ +#define STATUS_REG 0x2C /* STATUS Register */ + +/* Mode Register A register bits */ +#define MRA_OUTPUT_MODE_MASK (3 6) +#define MRA_ITUR_BT601 (1 6) +#define MRA_ITUR_BT656 (0 6) +#define MRA_INPUT_MODE_MASK(7 3) +#define MRA_PAL_BT601 (4 3) +#define MRA_NTSC_BT601 (0 3) +#define MRA_REGISTER_MODE (1 0) + +/* Mode Register C register bits */ +#define MRC_AUTOSELECT (1 7) + +/* Luminance Control register bits */ +#define LUMC_ONOFF_SHIFT 7 +#define LUMC_ONOFF_MASK(1 7) + +/* Contrast level control register bits */ +#define CLC_CONTRAST_ONOFF (1 7) +#define CLC_CONTRAST_MASK 0x0F + +/* Sync separation level register bits */ +#define SSEPL_LUMINANCE_ONOFF (1 7) +#define SSEPL_LUMINANCE_MASK 0x7F + +/* Chrominance Control A register bits */ +#define CHRCA_MODE_SHIFT 6 +#define CHRCA_MODE_MASK(1 6) + +/* ACC Loop filter Chrominance control register bits */ +#define ACCC_CHROMA_CR_SHIFT 3 +#define ACCC_CHROMA_CR_MASK(7 3) +#define ACCC_CHROMA_CB_SHIFT 0 +#define ACCC_CHROMA_CB_MASK(7 0) + +/* ACC Reference level control register bits */ +#define ACCRC_CHROMA_MASK 0xfc +#define ACCRC_CHROMA_SHIFT 2 + +/* ADC Register 2 register bits */
