Re: [PATCH 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x
int endpoint;
>
> Please do initialise the bus_type field at least.
v4l2_fwnode_endpoint_parse() should do that, right ?
>> +struct device_node *ep;
>> +struct isl7998x *isl7998x;
>> +struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>> +u32 chip_id;
>> +int i, ret;
>> +
>> +ret = i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA);
>> +if (!ret) {
>> +dev_warn(&adapter->dev,
>> + "I2C-Adapter doesn't support I2C_FUNC_SMBUS_WORD\n");
>> +return -EIO;
>> +}
>> +
>> +isl7998x = devm_kzalloc(dev, sizeof(*isl7998x), GFP_KERNEL);
>> +if (!isl7998x)
>> +return -ENOMEM;
>> +
>> +/* Default to all four inputs active unless specified otherwise. */
>> + ret = of_property_read_u32(dev->of_node, "isil,num-inputs",
>> + &isl7998x->nr_inputs);
>> +if (ret)
>> +isl7998x->nr_inputs = 4;
>> +
>> +if (isl7998x->nr_inputs != 1 && isl7998x->nr_inputs != 2 &&
>> +isl7998x->nr_inputs != 4) {
>> +dev_err(dev, "Invalid number of inputs selected in DT\n");
>> +return -EINVAL;
>> +}
>> +
>> +isl7998x->pd_gpio = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
>> +if (IS_ERR(isl7998x->pd_gpio)) {
>> +dev_err(dev, "Failed to retrieve/request PD GPIO: %ld\n",
>> +PTR_ERR(isl7998x->pd_gpio));
>> +return PTR_ERR(isl7998x->pd_gpio);
>> +}
>> +
>> +isl7998x->regmap = devm_regmap_init_i2c(client, &isl7998x_regmap);
>> +if (IS_ERR(isl7998x->regmap)) {
>> +ret = PTR_ERR(isl7998x->regmap);
>> +dev_err(dev, "Failed to allocate register map: %d\n", ret);
>> +return ret;
>> +}
>> +
>> +ep = of_graph_get_next_endpoint(dev->of_node, NULL);
>> +if (!ep) {
>> +dev_err(dev, "Missing endpoint node\n");
>> +return -EINVAL;
>> +}
>> +
>> +ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), &endpoint);
>> +of_node_put(ep);
>> +if (ret) {
>> +dev_err(dev, "Failed to parse endpoint\n");
>> +return ret;
>> +}
>> +
>> +if (endpoint.bus_type != V4L2_MBUS_CSI2_DPHY ||
>> +endpoint.bus.mipi_csi2.num_data_lanes == 0 ||
>> +endpoint.bus.mipi_csi2.num_data_lanes > 2) {
>> +dev_err(dev, "Invalid bus type or number of MIPI lanes\n");
>> +return -EINVAL;
>> +}
>> +
>> +isl7998x->nr_mipi_lanes = endpoint.bus.mipi_csi2.num_data_lanes;
>> +
>> +v4l2_i2c_subdev_init(&isl7998x->subdev, client, &isl7998x_subdev_ops);
>> +isl7998x->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> +#ifdef CONFIG_MEDIA_CONTROLLER
>> +isl7998x->pad.flags = MEDIA_PAD_FL_SOURCE;
>> +isl7998x->subdev.entity.ops = &isl7998x_entity_ops;
>> +isl7998x->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>
> This is definitely not a camera sensor.
>
> MEDIA_ENT_F_VID_IF_BRIDGE perhaps?
>
> How does the driver work without MC?
MC ?
> E.g. how are the formats on the inputs set up?
They are auto-detected by the chip.
[...]
--
Best regards,
Marek Vasut
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
On 7/1/19 10:04 AM, Sakari Ailus wrote:
> Hi Marek,
>
> On Mon, May 20, 2019 at 10:18:11PM +0200, Marek Vasut wrote:
>> Add bindings for the Intersil ISL7998x BT656-to-MIPI-CSI2 decoder.
>>
>> Signed-off-by: Marek Vasut
>> Cc: Sakari Ailus
>> Cc: Mauro Carvalho Chehab
>> Cc: Rob Herring
>> Cc: devicet...@vger.kernel.org
>> To: linux-media@vger.kernel.org
>> ---
>> .../bindings/media/i2c/isl7998x.txt | 37 +++
>> 1 file changed, 37 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> new file mode 100644
>> index ..c21703983360
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> @@ -0,0 +1,37 @@
>> +Intersil ISL7998x BT656-to-MIPI-CSI2 decoder
>> +
>> +The Intersil ISL7998x is a BT656-to-MIPI-CSI decoder which, capable of
>> +receiving up to four analog stream and multiplexing them into up to four
>> +MIPI CSI2 virtual channels, using one MIPI clock lane and 1/2 data lanes.
>> +
>> +Required Properties:
>> +- compatible: value should be "isil,isl79987"
>> +- pd-gpios: a GPIO spec for the Power Down pin (active high)
>> +
>> +Option Properties:
>> +- isil,num-inputs: Number of connected inputs (1, 2 or 4)
>
> The presence of ports describing connected Bt.656 inputs tells this.
>
>> +
>> +For further reading on port node refer to
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> Which endpoint properties are relevant for the endpoint(s) in the CSI-2 port?
clock-lanes, data-lanes and the remote-endpoint .
> How about the ports describing the Bt.656 interfaces? You should have
> those, too...
So who would be connected to these analog "input" endpoint ? What would
be their "remote-endpoint" ?
>> +
>> +Example:
>> +
>> +i2c_master {
>> +isl7998x_mipi@44 {
>> +compatible = "isil,isl79987";
>> +reg = <0x44>;
>> +isil,num-inputs = <4>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&pinctrl_videoadc>;
>> +pd-gpios = <&gpio3 27 GPIO_ACTIVE_HIGH>;
>> +status = "okay";
>> +
>> +port {
>> +isl79987_to_mipi_csi2: endpoint {
>> +remote-endpoint = <&mipi_csi2_in>;
>> +clock-lanes = <0>;
>> +data-lanes = <1 2>;
>> +};
>> +};
>> +};
>> +};
>>
>
--
Best regards,
Marek Vasut
Re: [PATCH 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x
On 7/1/19 9:58 AM, Jacopo Mondi wrote:
[...]
>> +#define ISL7998x_REG_P5_H_LINE_CNT_1ISL7998x_REG(5, 0x3a)
>> +#define ISL7998x_REG_P5_H_LINE_CNT_2ISL7998x_REG(5, 0x3b)
>> +#define ISL7998x_REG_P5_HIST_LINE_CNT_1 ISL7998x_REG(5, 0x3c)
>> +#define ISL7998x_REG_P5_HIST_LINE_CNT_2 ISL7998x_REG(5, 0x3d)
>
> Not all the above definitions are used. While it doesn't hurt to have
> them here, it's a pretty scary list of registers entries.
I guess it's good to have a full list of registers available ?
>> +static const struct reg_sequence isl7998x_init_seq_1[] = {
>> +{ ISL7998x_REG_P0_SHORT_DIAG_IRQ_EN, 0xff },
>> +{ ISL7998x_REG_Px_DEC_SDT(0x1), 0x07 },
>> +{ ISL7998x_REG_Px_DEC_SHORT_DET_CTL_1(0x1), 0x03 },
>> +{ ISL7998x_REG_Px_DEC_SDT(0x2), 0x07 },
>> +{ ISL7998x_REG_Px_DEC_SHORT_DET_CTL_1(0x2), 0x03 },
>> +{ ISL7998x_REG_Px_DEC_SDT(0x3), 0x07 },
>> +{ ISL7998x_REG_Px_DEC_SHORT_DET_CTL_1(0x3), 0x03 },
>> +{ ISL7998x_REG_Px_DEC_SDT(0x4), 0x07 },
>> +{ ISL7998x_REG_Px_DEC_SHORT_DET_CTL_1(0x4), 0x03 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_CTL, 0x00 },
>> +{ ISL7998x_REG_P0_SW_RESET_CTL, 0x1f, 10 },
>> +{ ISL7998x_REG_P0_IO_BUFFER_CTL, 0x00 },
>> +{ ISL7998x_REG_P0_MPP2_SYNC_CTL, 0xc9 },
>> +{ ISL7998x_REG_P0_IRQ_SYNC_CTL, 0xc9 },
>> +{ ISL7998x_REG_P0_CHAN_1_IRQ, 0x03 },
>> +{ ISL7998x_REG_P0_CHAN_2_IRQ, 0x00 },
>> +{ ISL7998x_REG_P0_CHAN_3_IRQ, 0x00 },
>> +{ ISL7998x_REG_P0_CHAN_4_IRQ, 0x00 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_CTL, 0x02 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_LINE_CTL, 0x85 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_PIC_WIDTH, 0xa0 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_SYNC_CTL, 0x18 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_TYPE_CTL, 0x40 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_FIFO_CTL, 0x40 },
>> +{ ISL7998x_REG_P5_MIPI_WCNT_1, 0x05 },
>> +{ ISL7998x_REG_P5_MIPI_WCNT_2, 0xa0 },
>> +{ ISL7998x_REG_P5_TP_GEN_MIPI, 0x00 },
>> +{ ISL7998x_REG_P5_ESC_MODE_TIME_CTL, 0x0c },
>> +{ ISL7998x_REG_P5_MIPI_SP_HS_TRL_CTL, 0x00 },
>> +{ ISL7998x_REG_P5_TP_GEN_RND_SYNC_CTL_1, 0x00 },
>> +{ ISL7998x_REG_P5_TP_GEN_RND_SYNC_CTL_2, 0x19 },
>> +{ ISL7998x_REG_P5_PSF_FIELD_END_CTL_1, 0x18 },
>> +{ ISL7998x_REG_P5_PSF_FIELD_END_CTL_2, 0xf1 },
>> +{ ISL7998x_REG_P5_PSF_FIELD_END_CTL_3, 0x00 },
>> +{ ISL7998x_REG_P5_PSF_FIELD_END_CTL_4, 0xf1 },
>> +{ ISL7998x_REG_P5_MIPI_ANA_DATA_CTL_1, 0x00 },
>> +{ ISL7998x_REG_P5_MIPI_ANA_DATA_CTL_2, 0x00 },
>> +{ ISL7998x_REG_P5_MIPI_ANA_CLK_CTL, 0x00 },
>> +{ ISL7998x_REG_P5_PLL_ANA_STATUS, 0xc0 },
>> +{ ISL7998x_REG_P5_PLL_ANA_MISC_CTL, 0x18 },
>> +{ ISL7998x_REG_P5_PLL_ANA, 0x00 },
>> +{ ISL7998x_REG_P0_SW_RESET_CTL, 0x10, 10 },
>> +/* Page 0xf means write to all of pages 1,2,3,4 */
>> +{ ISL7998x_REG_Px_DEC_VDELAY_LO(0xf), 0x14 },
>> +{ ISL7998x_REG_Px_DEC_MISC3(0xf), 0xe6 },
>> +{ ISL7998x_REG_Px_DEC_CLMD(0xf), 0x85 },
>> +{ ISL7998x_REG_Px_DEC_H_DELAY_II_LOW(0xf), 0x11 },
>> +{ ISL7998x_REG_Px_ACA_XFER_HIST_HOST(0xf), 0x00 },
>> +{ ISL7998x_REG_P0_CLK_CTL_1, 0x1f },
>> +{ ISL7998x_REG_P0_CLK_CTL_2, 0x43 },
>> +{ ISL7998x_REG_P0_CLK_CTL_3, 0x4f },
>> +};
>> +
>> +static const struct reg_sequence isl7998x_init_seq_2[] = {
>
> Is seq_2 alternative to the first?
No, it's second part of the register init sequence .
> Care to comment a bit what these
> do, if we have to live with register writes sequences?
Preinit the chip with sane settings. It's a combination of various
register init sequences from various ISL7998x driver mutations.
It's like many other camera drivers -- preload a blob of register writes
to init the chip into some sane defaults which work well.
>> +{ ISL7998x_REG_P5_LI_ENGINE_SYNC_CTL, 0x10 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_VC_ASSIGNMENT, 0xe4 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_TYPE_CTL, 0x00 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_FIFO_CTL, 0x60 },
>> +{ ISL7998x_REG_P5_MIPI_READ_START_CTL, 0x2b },
>> +{ ISL7998x_REG_P5_PSEUDO_FRM_FIELD_CTL, 0x02 },
>> +{ ISL7998x_REG_P5_ONE_FIELD_MODE_CTL, 0x00 },
>> +{ ISL7998x_REG_P5_MIPI_INT_HW_TST_CTR, 0x62 },
>> +{ ISL7998x_REG_P5_TP_GEN_BAR_PATTERN, 0x02 },
>> +{ ISL7998x_REG_P5_MIPI_PCNT_PSFRM, 0x36 },
>> +{ ISL7998x_REG_P5_LI_ENGINE_TP_GEN_CTL, 0x00 },
>> +{ ISL7998x_REG_P5_MIPI_VBLANK_PSFRM, 0x6c },
>> +{ ISL7998x_REG_P5_LI_ENGINE_CTL_2, 0x00 },
>> +{ ISL7998x_REG_P5_MIPI_WCNT_1, 0x05 },
>> +{ ISL7998x_REG_P5_MIPI_WCNT_2, 0xa0 },
>> +{ ISL7998x_REG_P5_MIPI_DPHY_TIMING_CTL_1, 0x77 },
>> +{ ISL7998x_REG_P5_MIPI_DPHY_TIMING_CTL_2, 0x17 },
>> +{ ISL7998x_REG_P5_MIPI_DPHY_TIMING_CTL_3, 0x08 },
>> +{ ISL7998x_REG_P5_MIPI_DPHY_TIMING_CTL_4, 0x38 },
>> +{ ISL7998x_REG_P5_MIPI_DPHY_TIMING_CTL_5, 0x14 },
>> +{ ISL7998x_REG_P5_MIPI_DPHY_TIMING_CTL_6, 0xf6 },
>> +{ ISL7998x_REG_P5_MIPI_DPHY_PARAMS_1, 0x00 },
>> +
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
On 5/29/19 1:15 PM, Ian Arkver wrote:
> Hi Marek,
>
> On 29/05/2019 12:09, Marek Vasut wrote:
>> On 5/29/19 1:04 PM, Ian Arkver wrote:
>>> Hi,
>>>
>>> On 29/05/2019 11:41, Marek Vasut wrote:
>>>> On 5/29/19 8:28 AM, Jacopo Mondi wrote:
>>>>
>>>> [...]
>>>>
>>>>>>>>> [1]
>>>>>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +- compatible: value should be "isil,isl79987"
>>>>>>>
>>>>>>> And here you might want to have 2 different compatibles for 79987
>>>>>>> and
>>>>>>> 79988.
>>>>>>
>>>>>> The 79988 is not supported yet, do we want to have it in the binding
>>>>>> doc?
>>>>>>
>>>>>
>>>>> I got mislead by the isl7998x naming scheme you used...
>>>>>
>>>>> I would say that's up to you, the two chips seems very similar,
>>>>> and it might make sense to provide bindings that support both. At the
>>>>> same time, as long as the here defined bindings does not prevent
>>>>> future expansions to include the ISL79988, its support could be safely
>>>>> post-poned. In that case please s/isl7998x/isl79987/ in this document
>>>>> and do not mention BT565 in the description.
>>>>
>>>> Right
>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>> I see from the example you only support one output port? How do
>>>>>>>>> you
>>>>>>>>> model the input ones.
>>>>>>>>
>>>>>>>> I don't . Do we model analog inputs now somehow ?
>>>>>>>
>>>>>>> I really think so, please see:
>>>>>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> And as an example of a board device tree using connectors to model
>>>>>>> analog input see how the cvbs input on Salvator-X is described:
>>>>>>>
>>>>>>> cvbs-in {
>>>>>>> compatible = "composite-video-connector";
>>>>>>> label = "CVBS IN";
>>>>>>>
>>>>>>> port {
>>>>>>> cvbs_con: endpoint {
>>>>>>> remote-endpoint = <&adv7482_ain7>;
>>>>>>> };
>>>>>>> };
>>>>>>> };
>>>>>>>
>>>>>>> I think you should provide 4 input ports, where to connect input
>>>>>>> from
>>>>>>> the analog connectors, and derive the number of enabled inputs from
>>>>>>> the number of endpoints connected to an active remote.
>>>>>>
>>>>>> Deriving the number of active physical inputs from some existing
>>>>>> binding
>>>>>> makes sense.
>>>>>>
>>>>>> However unlike the adv7482, the isl79987 does not support
>>>>>> remapping the
>>>>>> physical inputs to ADCs in the chip. It does support some
>>>>>> remapping of
>>>>>> physical inputs to MIPI CSI2 channels, but that's probably not very
>>>>>> useful.
>>>>>>
>>>>>
>>>>> I understand, but I will now use against you the argument you have
>>>>> correctly pointed out here below that DT should describe hardware, and
>>>>> the hardware has indeed 4 input ports..
>>>>
>>>> My question here is whether it makes sense to describe the ports
>>>> even if
>>>> they cannot be muxed to different ADC. Does it ?
>>>
>>> Each input port can be either differential CVBS or single ended with a
>>> 2:1 input select mux. It would be nice to be able to describe this.
>>
>> Where do you see that ?
>
> Bits 0 and 1 of each channel page's Differential Clamping Control 4
> (0x39, ISL7998x_REG_Px_DEC_DIFF_CLMP_CTL_4).
>
> I don't think you change it from the default (single ended on the first
> input).
I don't, since I have no way to test the differential mode of operation.
[...]
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
On 5/29/19 3:43 PM, Jacopo Mondi wrote:
> HI Marek,
Hi,
> On Wed, May 29, 2019 at 01:09:47PM +0200, Marek Vasut wrote:
>> On 5/29/19 1:04 PM, Ian Arkver wrote:
>>> Hi,
>>>
>>> On 29/05/2019 11:41, Marek Vasut wrote:
>>>> On 5/29/19 8:28 AM, Jacopo Mondi wrote:
>>>>
>>>> [...]
>>>>
>>>>>>>>> [1]
>>>>>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +- compatible: value should be "isil,isl79987"
>>>>>>>
>>>>>>> And here you might want to have 2 different compatibles for 79987 and
>>>>>>> 79988.
>>>>>>
>>>>>> The 79988 is not supported yet, do we want to have it in the binding
>>>>>> doc?
>>>>>>
>>>>>
>>>>> I got mislead by the isl7998x naming scheme you used...
>>>>>
>>>>> I would say that's up to you, the two chips seems very similar,
>>>>> and it might make sense to provide bindings that support both. At the
>>>>> same time, as long as the here defined bindings does not prevent
>>>>> future expansions to include the ISL79988, its support could be safely
>>>>> post-poned. In that case please s/isl7998x/isl79987/ in this document
>>>>> and do not mention BT565 in the description.
>>>>
>>>> Right
>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>> I see from the example you only support one output port? How do you
>>>>>>>>> model the input ones.
>>>>>>>>
>>>>>>>> I don't . Do we model analog inputs now somehow ?
>>>>>>>
>>>>>>> I really think so, please see:
>>>>>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>>>>>>
>>>>>>>
>>>>>>> And as an example of a board device tree using connectors to model
>>>>>>> analog input see how the cvbs input on Salvator-X is described:
>>>>>>>
>>>>>>> cvbs-in {
>>>>>>> compatible = "composite-video-connector";
>>>>>>> label = "CVBS IN";
>>>>>>>
>>>>>>> port {
>>>>>>> cvbs_con: endpoint {
>>>>>>> remote-endpoint = <&adv7482_ain7>;
>>>>>>> };
>>>>>>> };
>>>>>>> };
>>>>>>>
>>>>>>> I think you should provide 4 input ports, where to connect input from
>>>>>>> the analog connectors, and derive the number of enabled inputs from
>>>>>>> the number of endpoints connected to an active remote.
>>>>>>
>>>>>> Deriving the number of active physical inputs from some existing
>>>>>> binding
>>>>>> makes sense.
>>>>>>
>>>>>> However unlike the adv7482, the isl79987 does not support remapping the
>>>>>> physical inputs to ADCs in the chip. It does support some remapping of
>>>>>> physical inputs to MIPI CSI2 channels, but that's probably not very
>>>>>> useful.
>>>>>>
>>>>>
>>>>> I understand, but I will now use against you the argument you have
>>>>> correctly pointed out here below that DT should describe hardware, and
>>>>> the hardware has indeed 4 input ports..
>>>>
>>>> My question here is whether it makes sense to describe the ports even if
>>>> they cannot be muxed to different ADC. Does it ?
>>>
>>> Each input port can be either differential CVBS or single ended with a
>>> 2:1 input select mux. It would be nice to be able to describe this.
>>
>> Where do you see that ?
>>
>>> You cannot remap the inputs to different ADCs, but you can remap the
>>> ADCs to different VC IDs using the
>>> ISL7998x_REG_P5_LI_ENGINE_VC_ASSIGNMENT register. Describing each input
>>> would proivde somewhere to specify the vc-id.
>>
&
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
On 5/29/19 1:04 PM, Ian Arkver wrote:
> Hi,
>
> On 29/05/2019 11:41, Marek Vasut wrote:
>> On 5/29/19 8:28 AM, Jacopo Mondi wrote:
>>
>> [...]
>>
>>>>>>> [1]
>>>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>>>>>>>
>>>>>>>
>>>>>>>> +Required Properties:
>>>>>>>> +- compatible: value should be "isil,isl79987"
>>>>>
>>>>> And here you might want to have 2 different compatibles for 79987 and
>>>>> 79988.
>>>>
>>>> The 79988 is not supported yet, do we want to have it in the binding
>>>> doc?
>>>>
>>>
>>> I got mislead by the isl7998x naming scheme you used...
>>>
>>> I would say that's up to you, the two chips seems very similar,
>>> and it might make sense to provide bindings that support both. At the
>>> same time, as long as the here defined bindings does not prevent
>>> future expansions to include the ISL79988, its support could be safely
>>> post-poned. In that case please s/isl7998x/isl79987/ in this document
>>> and do not mention BT565 in the description.
>>
>> Right
>>
>>>> [...]
>>>>
>>>>>>> I see from the example you only support one output port? How do you
>>>>>>> model the input ones.
>>>>>>
>>>>>> I don't . Do we model analog inputs now somehow ?
>>>>>
>>>>> I really think so, please see:
>>>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>>>>
>>>>>
>>>>> And as an example of a board device tree using connectors to model
>>>>> analog input see how the cvbs input on Salvator-X is described:
>>>>>
>>>>> cvbs-in {
>>>>> compatible = "composite-video-connector";
>>>>> label = "CVBS IN";
>>>>>
>>>>> port {
>>>>> cvbs_con: endpoint {
>>>>> remote-endpoint = <&adv7482_ain7>;
>>>>> };
>>>>> };
>>>>> };
>>>>>
>>>>> I think you should provide 4 input ports, where to connect input from
>>>>> the analog connectors, and derive the number of enabled inputs from
>>>>> the number of endpoints connected to an active remote.
>>>>
>>>> Deriving the number of active physical inputs from some existing
>>>> binding
>>>> makes sense.
>>>>
>>>> However unlike the adv7482, the isl79987 does not support remapping the
>>>> physical inputs to ADCs in the chip. It does support some remapping of
>>>> physical inputs to MIPI CSI2 channels, but that's probably not very
>>>> useful.
>>>>
>>>
>>> I understand, but I will now use against you the argument you have
>>> correctly pointed out here below that DT should describe hardware, and
>>> the hardware has indeed 4 input ports..
>>
>> My question here is whether it makes sense to describe the ports even if
>> they cannot be muxed to different ADC. Does it ?
>
> Each input port can be either differential CVBS or single ended with a
> 2:1 input select mux. It would be nice to be able to describe this.
Where do you see that ?
> You cannot remap the inputs to different ADCs, but you can remap the
> ADCs to different VC IDs using the
> ISL7998x_REG_P5_LI_ENGINE_VC_ASSIGNMENT register. Describing each input
> would proivde somewhere to specify the vc-id.
I think Jacopo mentioned above the input muxing and the MIPI CSI2 VC
muxing are two separate things. But I have to wonder, do we have a way
of muxing the VCs in the DT or via the media controller yet ?
--
Best regards,
Marek Vasut
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
On 5/29/19 8:28 AM, Jacopo Mondi wrote:
[...]
>>>>> [1]
>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>>>>>
>>>>>> +Required Properties:
>>>>>> +- compatible: value should be "isil,isl79987"
>>>
>>> And here you might want to have 2 different compatibles for 79987 and
>>> 79988.
>>
>> The 79988 is not supported yet, do we want to have it in the binding doc?
>>
>
> I got mislead by the isl7998x naming scheme you used...
>
> I would say that's up to you, the two chips seems very similar,
> and it might make sense to provide bindings that support both. At the
> same time, as long as the here defined bindings does not prevent
> future expansions to include the ISL79988, its support could be safely
> post-poned. In that case please s/isl7998x/isl79987/ in this document
> and do not mention BT565 in the description.
Right
>> [...]
>>
>>>>> I see from the example you only support one output port? How do you
>>>>> model the input ones.
>>>>
>>>> I don't . Do we model analog inputs now somehow ?
>>>
>>> I really think so, please see:
>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>>
>>> And as an example of a board device tree using connectors to model
>>> analog input see how the cvbs input on Salvator-X is described:
>>>
>>> cvbs-in {
>>> compatible = "composite-video-connector";
>>> label = "CVBS IN";
>>>
>>> port {
>>> cvbs_con: endpoint {
>>> remote-endpoint = <&adv7482_ain7>;
>>> };
>>> };
>>> };
>>>
>>> I think you should provide 4 input ports, where to connect input from
>>> the analog connectors, and derive the number of enabled inputs from
>>> the number of endpoints connected to an active remote.
>>
>> Deriving the number of active physical inputs from some existing binding
>> makes sense.
>>
>> However unlike the adv7482, the isl79987 does not support remapping the
>> physical inputs to ADCs in the chip. It does support some remapping of
>> physical inputs to MIPI CSI2 channels, but that's probably not very useful.
>>
>
> I understand, but I will now use against you the argument you have
> correctly pointed out here below that DT should describe hardware, and
> the hardware has indeed 4 input ports..
My question here is whether it makes sense to describe the ports even if
they cannot be muxed to different ADC. Does it ?
[...]
--
Best regards,
Marek Vasut
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
On 5/28/19 5:10 PM, Jacopo Mondi wrote:
[...]
>>> From my understanding of the product page, both the ISL79987 and
>>> ILS79988 devices support up to 4 analog inputs, and provide a CSI-2
>>> output and a BT656 output respectively.
>>>
>>> What am I reading wrong ?
>>
>> ISL79987 is analog video to mipi csi2 ; I have this chip.
>> ISL79988 is analog video to bt656 ; I don't have this chip.
>>
>
> So please change the description to "Analog to MIPI CSI-2/BT565
> decoder"
Done
>>> [1]
>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>>>
>>>> +Required Properties:
>>>> +- compatible: value should be "isil,isl79987"
>
> And here you might want to have 2 different compatibles for 79987 and
> 79988.
The 79988 is not supported yet, do we want to have it in the binding doc?
[...]
>>> I see from the example you only support one output port? How do you
>>> model the input ones.
>>
>> I don't . Do we model analog inputs now somehow ?
>
> I really think so, please see:
> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>
> And as an example of a board device tree using connectors to model
> analog input see how the cvbs input on Salvator-X is described:
>
> cvbs-in {
> compatible = "composite-video-connector";
> label = "CVBS IN";
>
> port {
> cvbs_con: endpoint {
> remote-endpoint = <&adv7482_ain7>;
> };
> };
> };
>
> I think you should provide 4 input ports, where to connect input from
> the analog connectors, and derive the number of enabled inputs from
> the number of endpoints connected to an active remote.
Deriving the number of active physical inputs from some existing binding
makes sense.
However unlike the adv7482, the isl79987 does not support remapping the
physical inputs to ADCs in the chip. It does support some remapping of
physical inputs to MIPI CSI2 channels, but that's probably not very useful.
> Also, you might want to provide 2 output ports, one CSI-2 and one
> BT565 and parse the right one depending on the compatible string.
The chip only has one physical output port (either CSI2 or parallel) and
DT should model the hardware, so I would expect there to be one output
port per chip ?
> I would also place the input ports last (from port@2 to port@5) so
> that we make easier to support similar chips with more inputs (if
> any).
>
> That said, I'm no expert of analog video, so others might have
> different opinions :)
>
> Thanks
>j
>
>>
>> --
>> Best regards,
>> Marek Vasut
--
Best regards,
Marek Vasut
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
On 5/28/19 1:47 PM, Jacopo Mondi wrote:
> Hi Marek,
>thanks for the patch
Hi,
> On Mon, May 20, 2019 at 10:18:11PM +0200, Marek Vasut wrote:
>> Add bindings for the Intersil ISL7998x BT656-to-MIPI-CSI2 decoder.
>>
>> Signed-off-by: Marek Vasut
>> Cc: Sakari Ailus
>> Cc: Mauro Carvalho Chehab
>> Cc: Rob Herring
>> Cc: devicet...@vger.kernel.org
>> To: linux-media@vger.kernel.org
>> ---
>> .../bindings/media/i2c/isl7998x.txt | 37 +++
>> 1 file changed, 37 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> new file mode 100644
>> index ..c21703983360
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>> @@ -0,0 +1,37 @@
>> +Intersil ISL7998x BT656-to-MIPI-CSI2 decoder
>> +
>> +The Intersil ISL7998x is a BT656-to-MIPI-CSI decoder which, capable of
>> +receiving up to four analog stream and multiplexing them into up to four
>> +MIPI CSI2 virtual channels, using one MIPI clock lane and 1/2 data lanes.
>> +
>
> The documentation is not public, so I can only read what's reported on
> the website and the short public datasheet at [1]
Right
> From my understanding of the product page, both the ISL79987 and
> ILS79988 devices support up to 4 analog inputs, and provide a CSI-2
> output and a BT656 output respectively.
>
> What am I reading wrong ?
ISL79987 is analog video to mipi csi2 ; I have this chip.
ISL79988 is analog video to bt656 ; I don't have this chip.
> [1]
> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html
>
>> +Required Properties:
>> +- compatible: value should be "isil,isl79987"
>> +- pd-gpios: a GPIO spec for the Power Down pin (active high)
>> +
>> +Option Properties:
>> +- isil,num-inputs: Number of connected inputs (1, 2 or 4)
>
> Can't you derive this from the number of connected input endpoints
> instead of providing a custom property?
Input endpoints from where ?
>> +
>> +For further reading on port node refer to
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>
> I think a description of the supported ports and their intended
> usages is required here. You have up to 4 inputs and 1 output port,
> how do you expect them to be numbered? is port@4 the last input or the
> output one?
The only port is the MIPI CSI2 , see the example below.
>> +Example:
>> +
>> +i2c_master {
>> +isl7998x_mipi@44 {
>> +compatible = "isil,isl79987";
>> +reg = <0x44>;
>> +isil,num-inputs = <4>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&pinctrl_videoadc>;
>> +pd-gpios = <&gpio3 27 GPIO_ACTIVE_HIGH>;
>> +status = "okay";
>> +
>> +port {
>> + isl79987_to_mipi_csi2: endpoint {
>> +remote-endpoint = <&mipi_csi2_in>;
>> +clock-lanes = <0>;
>> +data-lanes = <1 2>;
>> +};
>
> I see from the example you only support one output port? How do you
> model the input ones.
I don't . Do we model analog inputs now somehow ?
--
Best regards,
Marek Vasut
[PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
Add bindings for the Intersil ISL7998x BT656-to-MIPI-CSI2 decoder.
Signed-off-by: Marek Vasut
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Cc: Rob Herring
Cc: devicet...@vger.kernel.org
To: linux-media@vger.kernel.org
---
.../bindings/media/i2c/isl7998x.txt | 37 +++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/isl7998x.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
new file mode 100644
index ..c21703983360
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
@@ -0,0 +1,37 @@
+Intersil ISL7998x BT656-to-MIPI-CSI2 decoder
+
+The Intersil ISL7998x is a BT656-to-MIPI-CSI decoder which, capable of
+receiving up to four analog stream and multiplexing them into up to four
+MIPI CSI2 virtual channels, using one MIPI clock lane and 1/2 data lanes.
+
+Required Properties:
+- compatible: value should be "isil,isl79987"
+- pd-gpios: a GPIO spec for the Power Down pin (active high)
+
+Option Properties:
+- isil,num-inputs: Number of connected inputs (1, 2 or 4)
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+ i2c_master {
+ isl7998x_mipi@44 {
+ compatible = "isil,isl79987";
+ reg = <0x44>;
+ isil,num-inputs = <4>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_videoadc>;
+ pd-gpios = <&gpio3 27 GPIO_ACTIVE_HIGH>;
+ status = "okay";
+
+ port {
+ isl79987_to_mipi_csi2: endpoint {
+ remote-endpoint = <&mipi_csi2_in>;
+ clock-lanes = <0>;
+ data-lanes = <1 2>;
+ };
+ };
+ };
+ };
--
2.20.1
[PATCH 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x
Add driver for the Intersil ISL7998x BT656-to-MIPI-CSI2 video decoder. This chip supports 1/2/4 analog video inputs and converts them into 1/2/4 VCs in MIPI CSI2 stream. This driver currently supports ISL79987 and both 720x480 and 720x576 resolutions, however as per specification, all inputs must use the same resolution and standard. The only supported pixel format is now YUYV/YUV422. The chip should support RGB565 on the CSI2 as well, but this is currently unsupported. Signed-off-by: Marek Vasut Cc: Sakari Ailus Cc: Mauro Carvalho Chehab Cc: Rob Herring To: linux-media@vger.kernel.org --- drivers/media/i2c/Kconfig|6 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/isl7998x.c | ++ 3 files changed, 1118 insertions(+) create mode 100644 drivers/media/i2c/isl7998x.c diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 7793358ab8b3..af2ea08d6c59 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -929,6 +929,12 @@ config VIDEO_NOON010PC30 help This driver supports NOON010PC30 CIF camera from Siliconfile +config VIDEO_ISL7998X + tristate "Intersil ISL7998x support" + depends on I2C && VIDEO_V4L2 + help + This driver supports Intersil ISL7998x BT656-to-MIPI-CSI2 bridge. + source "drivers/media/i2c/m5mols/Kconfig" config VIDEO_RJ54N1 diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index d8ad9dad495d..a66f105a7bcc 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -113,6 +113,7 @@ obj-$(CONFIG_VIDEO_IMX258) += imx258.o obj-$(CONFIG_VIDEO_IMX274) += imx274.o obj-$(CONFIG_VIDEO_IMX319) += imx319.o obj-$(CONFIG_VIDEO_IMX355) += imx355.o +obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o obj-$(CONFIG_SDR_MAX2175) += max2175.o diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c new file mode 100644 index ..858e98d8b494 --- /dev/null +++ b/drivers/media/i2c/isl7998x.c @@ -0,0 +1, @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Intersil ISL7998x BT656-to-MIPI-CSI2 driver + * + * Copyright (C) 2018-2019 Marek Vasut + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define ISL7998x_WIDTH 720 +#define ISL7998x_HEIGHT480 +#define ISL7998x_INPUTS4 + +#define V4L2_CID_TEST_PATTERN_COLOR(V4L2_CID_USER_BASE | 0x1001) +#define V4L2_CID_TEST_PATTERN_CHANNELS (V4L2_CID_USER_BASE | 0x1002) +#define V4L2_CID_TEST_PATTERN_BARS (V4L2_CID_USER_BASE | 0x1003) + +#define ISL7998x_REG(page, reg)(((page) << 8) | (reg)) + +#define ISL7998x_REG_Pn_SIZE 256 +#define ISL7998x_REG_Pn_BASE(n)((n) * ISL7998x_REG_Pn_SIZE) + +#define ISL7998x_REG_Px_DEC_PAGE(page) ISL7998x_REG((page), 0xff) +#define ISL7998x_REG_Px_DEC_PAGE_MASK 0xf +#define ISL7998x_REG_P0_PRODUCT_ID_CODEISL7998x_REG(0, 0x00) +#define ISL7998x_REG_P0_PRODUCT_REV_CODE ISL7998x_REG(0, 0x01) +#define ISL7998x_REG_P0_SW_RESET_CTL ISL7998x_REG(0, 0x02) +#define ISL7998x_REG_P0_IO_BUFFER_CTL ISL7998x_REG(0, 0x03) +#define ISL7998x_REG_P0_IO_BUFFER_CTL_1_1 ISL7998x_REG(0, 0x04) +#define ISL7998x_REG_P0_IO_PAD_PULL_EN_CTL ISL7998x_REG(0, 0x05) +#define ISL7998x_REG_P0_IO_BUFFER_CTL_1_2 ISL7998x_REG(0, 0x06) +#define ISL7998x_REG_P0_VIDEO_IN_CHAN_CTL ISL7998x_REG(0, 0x07) +#define ISL7998x_REG_P0_CLK_CTL_1 ISL7998x_REG(0, 0x08) +#define ISL7998x_REG_P0_CLK_CTL_2 ISL7998x_REG(0, 0x09) +#define ISL7998x_REG_P0_CLK_CTL_3 ISL7998x_REG(0, 0x0a) +#define ISL7998x_REG_P0_CLK_CTL_4 ISL7998x_REG(0, 0x0b) +#define ISL7998x_REG_P0_MPP1_SYNC_CTL ISL7998x_REG(0, 0x0c) +#define ISL7998x_REG_P0_MPP2_SYNC_CTL ISL7998x_REG(0, 0x0d) +#define ISL7998x_REG_P0_IRQ_SYNC_CTL ISL7998x_REG(0, 0x0e) +#define ISL7998x_REG_P0_INTERRUPT_STATUS ISL7998x_REG(0, 0x10) +#define ISL7998x_REG_P0_CHAN_1_IRQ ISL7998x_REG(0, 0x11) +#define ISL7998x_REG_P0_CHAN_2_IRQ ISL7998x_REG(0, 0x12) +#define ISL7998x_REG_P0_CHAN_3_IRQ ISL7998x_REG(0, 0x13) +#define ISL7998x_REG_P0_CHAN_4_IRQ ISL7998x_REG(0, 0x14) +#define ISL7998x_REG_P0_SHORT_DIAG_IRQ ISL7998x_REG(0, 0x15) +#define ISL7998x_REG_P0_CHAN_1_IRQ_EN ISL7998x_REG(0, 0x16) +#define ISL7998x_REG_P0_CHAN_2_IRQ_EN ISL7998x_REG(0, 0x17) +#define ISL7998x_REG_P0_CHAN_3_IRQ_EN ISL7998x_REG(0, 0x18) +#define ISL7998x_REG_P0_CHAN_4_IRQ_EN ISL7998x_REG(0, 0x19) +#define ISL7998x_REG_P0_SHORT_DIAG_IRQ_EN ISL7998x_REG(0, 0x1a) +#de
[PATCH] media: imx: Parse MIPI CSI2 link frequency correctly
The current code did not extract the CSI2 link frequency from the menu
items correctly. Fix this.
Signed-off-by: Marek Vasut
Cc: Fabio Estevam
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: Philipp Zabel
Cc: Steve Longerbeam
To: linux-media@vger.kernel.org
---
drivers/staging/media/imx/imx6-mipi-csi2.c | 28 --
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c
b/drivers/staging/media/imx/imx6-mipi-csi2.c
index f29e28df36ed..64d8229336dd 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -193,18 +193,32 @@ static int max_mbps_to_hsfreqrange_sel(u32 max_mbps)
static int csi2_dphy_init(struct csi2_dev *csi2)
{
+ struct v4l2_querymenu qm = { .id = V4L2_CID_LINK_FREQ };
struct v4l2_ctrl *ctrl;
- u32 mbps_per_lane;
- int sel;
+ u32 mbps_per_lane = CSI2_DEFAULT_MAX_MBPS;
+ int ret, sel;
ctrl = v4l2_ctrl_find(csi2->src_sd->ctrl_handler,
V4L2_CID_LINK_FREQ);
- if (!ctrl)
- mbps_per_lane = CSI2_DEFAULT_MAX_MBPS;
- else
- mbps_per_lane = DIV_ROUND_UP_ULL(2 * ctrl->qmenu_int[ctrl->val],
-USEC_PER_SEC);
+ if (ctrl) {
+ qm.index = v4l2_ctrl_g_ctrl(ctrl);
+ ret = v4l2_querymenu(csi2->src_sd->ctrl_handler, &qm);
+ if (ret) {
+ v4l2_err(&csi2->sd,
+"failed to get V4L2_CID_LINK_FREQ menu item,
using default.\n");
+ goto exit;
+ }
+
+ if (!qm.value) {
+ v4l2_err(&csi2->sd,
+"invalid V4L2_CID_LINK_FREQ, using
default.\n");
+ goto exit;
+ }
+
+ mbps_per_lane = DIV_ROUND_UP_ULL(qm.value, USEC_PER_SEC);
+ }
+exit:
sel = max_mbps_to_hsfreqrange_sel(mbps_per_lane);
if (sel < 0)
return sel;
--
2.20.1
[PATCH] media: imx: Handle VIDIOC_ENUMINPUT
Add basic handling for VIDIOC_ENUMINPUT, where the imx capture devices
report they are cameras to userspace. Code like e.g. Qt5 qcamera uses
this information when enumerating camera devices and this fixes it's
operation on iMX6, where it previously didn't detect any cameras.
Signed-off-by: Marek Vasut
Cc: Fabio Estevam
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: Philipp Zabel
Cc: Steve Longerbeam
To: linux-media@vger.kernel.org
---
drivers/staging/media/imx/imx-media-capture.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index 9430c835c434..1e3790479fd6 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -148,6 +148,18 @@ static int capture_enum_frameintervals(struct file *file,
void *fh,
return 0;
}
+static int capture_enum_input(struct file *file, void *priv,
+ struct v4l2_input *inp)
+{
+ if (inp->index > 0)
+ return -EINVAL;
+
+ inp->type = V4L2_INPUT_TYPE_CAMERA;
+ strlcpy(inp->name, "Camera", sizeof(inp->name));
+
+ return 0;
+}
+
static int capture_enum_fmt_vid_cap(struct file *file, void *fh,
struct v4l2_fmtdesc *f)
{
@@ -414,6 +426,7 @@ static const struct v4l2_ioctl_ops capture_ioctl_ops = {
.vidioc_enum_framesizes = capture_enum_framesizes,
.vidioc_enum_frameintervals = capture_enum_frameintervals,
+ .vidioc_enum_input = capture_enum_input,
.vidioc_enum_fmt_vid_cap= capture_enum_fmt_vid_cap,
.vidioc_g_fmt_vid_cap = capture_g_fmt_vid_cap,
--
2.20.1
Re: [PATCH 1/7] dt-bindings: mfd: ds90ux9xx: add description of TI DS90Ux9xx ICs
On 10/09/2018 10:55 PM, Vladimir Zapolskiy wrote: > On 10/09/2018 02:11 PM, Vladimir Zapolskiy wrote: >> Hi Marek, >> >> On 10/09/2018 03:13 AM, Marek Vasut wrote: >>> On 10/08/2018 11:11 PM, Vladimir Zapolskiy wrote: >>>> From: Sandeep Jain >>>> >>>> The change adds device tree binding description of TI DS90Ux9xx >>>> series of serializer and deserializer controllers which support video, >>>> audio and control data transmission over FPD-III Link connection. >>>> > > [snip] > >>>> +Optional properties: >>>> +- reg : Specifies the I2C slave address of a local de-/serializer. >>>> +- power-gpios : GPIO line to control supplied power to the device. >>> >>> Shouldn't this be regulator phandle ? >> >> It could be, right. I'll ponder upon it. >> > > No, it can not. > > The property describes PDB "Power-down Mode Input Pin", it is a control > pin with the predefined voltage, so regulator phandle is not applicable > here. Then the DT binding document needs updating, because this is completely unclear and confusing. -- Best regards, Marek Vasut
Re: [PATCH 1/7] dt-bindings: mfd: ds90ux9xx: add description of TI DS90Ux9xx ICs
On 10/08/2018 11:11 PM, Vladimir Zapolskiy wrote:
> From: Sandeep Jain
>
> The change adds device tree binding description of TI DS90Ux9xx
> series of serializer and deserializer controllers which support video,
> audio and control data transmission over FPD-III Link connection.
>
> Signed-off-by: Sandeep Jain
> [vzapolskiy: various updates and corrections of secondary importance]
> Signed-off-by: Vladimir Zapolskiy
> ---
> .../devicetree/bindings/mfd/ti,ds90ux9xx.txt | 66 +++
> 1 file changed, 66 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt
> b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt
> new file mode 100644
> index ..0733da88f7ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt
> @@ -0,0 +1,66 @@
> +Texas Instruments DS90Ux9xx de-/serializer controllers
> +
> +Required properties:
> +- compatible: Must contain a generic "ti,ds90ux9xx" value and
> + may contain one more specific value from the list:
> + "ti,ds90ub925q",
> + "ti,ds90uh925q",
> + "ti,ds90ub927q",
> + "ti,ds90uh927q",
> + "ti,ds90ub926q",
> + "ti,ds90uh926q",
Keep the list sorted.
> + "ti,ds90ub928q",
> + "ti,ds90uh928q",
> + "ti,ds90ub940q",
> + "ti,ds90uh940q".
> +
> +Optional properties:
> +- reg : Specifies the I2C slave address of a local de-/serializer.
> +- power-gpios : GPIO line to control supplied power to the device.
Shouldn't this be regulator phandle ?
> +- ti,backward-compatible-mode : Overrides backward compatibility mode.
> + Possible values are "<1>" or "<0>".
Make this bool , ie. present or not.
> + If "ti,backward-compatible-mode" is not mentioned, the backward
> + compatibility mode is not touched and given by hardware pin strapping.
> +- ti,low-frequency-mode : Overrides low frequency mode.
> + Possible values are "<1>" or "<0>".
> + If "ti,low-frequency-mode" is not mentioned, the low frequency mode
> + is not touched and given by hardware pin strapping.
> +- ti,video-map-select-msb: Sets video bridge pins to MSB mode, if it is set
> + MAPSEL pin value is ignored.
> +- ti,video-map-select-lsb: Sets video bridge pins to LSB mode, if it is set
> + MAPSEL pin value is ignored.
This needs some additional explanation, what's this about ?
> +- ti,pixel-clock-edge : Selects Pixel Clock Edge.
> + Possible values are "<1>" or "<0>".
> + If "ti,pixel-clock-edge" is High <1>, output data is strobed on the
> + Rising edge of the PCLK. If ti,pixel-clock-edge is Low <0>, data is
> + strobed on the Falling edge of the PCLK.
> + If "ti,pixel-clock-edge" is not mentioned, the pixel clock edge
> + value is not touched and given by hardware pin strapping.
> +- ti,spread-spectrum-clock-generation : Spread Sprectrum Clock Generation.
> + Possible values are from "<0>" to "<7>". The same value will be
> + written to SSC register. If "ti,spread-spectrum-clock-gen" is not
> + found, then SSCG will be disabled.
> +
> +TI DS90Ux9xx serializers and deserializer device nodes may contain a number
> +of children device nodes to describe and enable particular subcomponents
> +found on ICs.
> +
> +Example:
> +
> +serializer: serializer@c {
> + compatible = "ti,ds90ub927q", "ti,ds90ux9xx";
> + reg = <0xc>;
> + power-gpios = <&gpio5 12 GPIO_ACTIVE_HIGH>;
> + ti,backward-compatible-mode = <0>;
> + ti,low-frequency-mode = <0>;
> + ti,pixel-clock-edge = <0>;
> + ...
> +}
> +
> +deserializer: deserializer@3c {
> + compatible = "ti,ds90ub940q", "ti,ds90ux9xx";
> + reg = <0x3c>;
> + power-gpios = <&gpio6 31 GPIO_ACTIVE_HIGH>;
> + ...
> +}
> +
>
--
Best regards,
Marek Vasut
Re: [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes
On 05/22/2018 01:07 PM, Philipp Zabel wrote:
> Hi Marek,
>
> On Fri, 2018-05-18 at 18:21 +0200, Marek Vasut wrote:
>> On 05/18/2018 05:51 PM, Philipp Zabel wrote:
>>> Hi Marek,
>>>
>>> On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote:
>>>> The BT1120 interlaced CCIR codes are the same as BT656 ones
>>>> and different than BT656 progressive CCIR codes, fix this.
>>>
>>> thank you for the patch, and sorry for the delay.
>>>
>>>> Signed-off-by: Marek Vasut
>>>> Cc: Steve Longerbeam
>>>> Cc: Philipp Zabel
>>>> ---
>>>> drivers/gpu/ipu-v3/ipu-csi.c | 8 ++--
>>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
>>>> index caa05b0702e1..301a729581ce 100644
>>>> --- a/drivers/gpu/ipu-v3/ipu-csi.c
>>>> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
>>>> @@ -435,12 +435,16 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>>>>break;
>>>>case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
>>>>case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
>>>> - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR:
>>>> - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR:
>>>>ipu_csi_write(csi, 0x40030 | CSI_CCIR_ERR_DET_EN,
>>>> CSI_CCIR_CODE_1);
>>>>ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
>>>>break;
>>>> + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR:
>>>> + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR:
>>>> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
>>>> CSI_CCIR_CODE_1);
>>>> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
>>>> + ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
>>>
>>> If these are the same as BT656 codes (so this case would be for PAL?),
>>> could this just be moved up into the IPU_CSI_CLK_MODE_CCIR656_INTERLACED
>>> case? Would the NTSC CCIR codes be the same as well?
>>
>> Dunno, I don't have any NTSC device to test. But the above was tested
>> with a PAL device I had.
>>
>> I think the CCIR codes are different from BT656, although I might be wrong.
>
> The driver currently has:
>
> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
> if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
> /*
> * PAL case
> *
> * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> */
> height = 625; /* framelines for PAL */
>
> ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> CSI_CCIR_CODE_1);
> ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
> } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480)
> {
> /*
> * NTSC case
> *
> * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> */
> height = 525; /* framelines for NTSC */
>
> ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> CSI_CCIR_CODE_1);
> ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
> } else {
> dev_err(csi->ipu->dev,
> "Unsupported CCIR656 interlaced video
> mode\n");
> spin_unlock_irqrestore(&csi->lock, flags);
> return -EINVAL;
> }
> break;
>
> The PAL codes are exactly the same as in your patch. That's why I wonder
Re: [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes
On 05/18/2018 05:51 PM, Philipp Zabel wrote: > Hi Marek, > > On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote: >> The BT1120 interlaced CCIR codes are the same as BT656 ones >> and different than BT656 progressive CCIR codes, fix this. > > thank you for the patch, and sorry for the delay. > >> Signed-off-by: Marek Vasut >> Cc: Steve Longerbeam >> Cc: Philipp Zabel >> --- >> drivers/gpu/ipu-v3/ipu-csi.c | 8 ++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c >> index caa05b0702e1..301a729581ce 100644 >> --- a/drivers/gpu/ipu-v3/ipu-csi.c >> +++ b/drivers/gpu/ipu-v3/ipu-csi.c >> @@ -435,12 +435,16 @@ int ipu_csi_init_interface(struct ipu_csi *csi, >> break; >> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR: >> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR: >> -case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: >> -case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: >> ipu_csi_write(csi, 0x40030 | CSI_CCIR_ERR_DET_EN, >> CSI_CCIR_CODE_1); >> ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3); >> break; >> +case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: >> +case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: >> +ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN, >> CSI_CCIR_CODE_1); >> +ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2); >> +ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3); > > If these are the same as BT656 codes (so this case would be for PAL?), > could this just be moved up into the IPU_CSI_CLK_MODE_CCIR656_INTERLACED > case? Would the NTSC CCIR codes be the same as well? Dunno, I don't have any NTSC device to test. But the above was tested with a PAL device I had. I think the CCIR codes are different from BT656, although I might be wrong. -- Best regards, Marek Vasut
Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode
On 04/21/2018 11:29 PM, Steve Longerbeam wrote: > > > On 04/12/2018 03:04 AM, Philipp Zabel wrote: >> On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote: >>> In VDIC direct mode, the VDIC applies combing filter during and >>> doubles the framerate, that is, after the first two half-frames >>> are received and the first frame is emitted by the VDIC, every >>> subsequent half-frame is patched into the result and a full frame >>> is produced. The half-frame order in the full frames is as follows >>> 12 32 34 54 etc. >> Is that true? We are only supporting full motion mode (VDI_MOT_SEL=2), >> so I was under the impression that only data from the current field >> makes it into the full frame. The missing lines should be purely >> estimated from the available field using the di_vfilt 4-tap filter. > > Yes, the reference manual states that the VDI_MOT_SEL field > value 2 is "Full motion, only vertical filter is used". Which is > clearly referring to the di_vfilt 4-tap filter. > > There are still some questions about how full motion mode is > supposed to work. For one thing, the di_vfilt only operates on four > consecutive lines of a single field. It makes no sense that the VDIC > can compensate for motion between fields when it is operating > with only one field. > > Marek, where did you get the information that full motion mode > applies some kind of combing filter? By observing how the HW behaves. The input from NXP forum was mostly useless or actually outright wrong. I have to admit it's been a while since I created the patch, but what I saw was basically the hardware producing frames as a combination of halfframe 1-2 2-3 3-4 etc , thus doubling the framerate. Setting the skip normalized the framerate without any loss of image information. > A combing filter would imply > taking previous and/or future fields back into the result, which is > exactly what the other motion modes do, but as I said the reference > manual is clear that full motion mode uses only the (single field) > vertical filter. > > The manual also mentions regarding "real-time mode" which we are > making use of (in which the VDIC FIFO1 receives field F(n-1) directly > from the CSI rather than from DMA): > > "In addition IDMAC read the field from FIFO1 and store in external memory. > Then stored frames are used as F(n) and F(n+1).". > > It is nowhere explicitly stated, but the assumption is that this is IDMAC > channel 13 that stores the CSI field to memory. But many people have > asked Freescale/NXP how this works in the past, and there has never > been a satisfactory answer. And people have reported no success in > getting this channel to work including myself. > > So the approach this driver takes is to use real-time mode to receive > F(n-1) directly from CSI, in concert with full motion mode, so that > the VDIC operates on F(n-1) only. Thus no DMA is necessary. > > Finally I have to say that the other modes are supported in this driver, > but they require DMA transfer of all three fields, and there is no > output device node written to make use of those modes yet. But > from experience, the de-interlaced result is of much better quality > than the real-time/full-motion output. > > > Steve > > -- Best regards, Marek Vasut
Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode
On 04/12/2018 12:04 PM, Philipp Zabel wrote: > On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote: >> In VDIC direct mode, the VDIC applies combing filter during and >> doubles the framerate, that is, after the first two half-frames >> are received and the first frame is emitted by the VDIC, every >> subsequent half-frame is patched into the result and a full frame >> is produced. The half-frame order in the full frames is as follows >> 12 32 34 54 etc. > > Is that true? We are only supporting full motion mode (VDI_MOT_SEL=2), > so I was under the impression that only data from the current field > makes it into the full frame. The missing lines should be purely > estimated from the available field using the di_vfilt 4-tap filter. Try using the VDIC within a pipeline directly: media-ctl -l "'ipu1_csi0':1->'ipu1_vdic':0[1]" media-ctl -l "'ipu1_vdic':2->'ipu1_ic_prp':0[1]" media-ctl -l "'ipu1_ic_prp':2->'ipu1_ic_prpvf':0[1]" media-ctl -l "'ipu1_ic_prpvf':1->'ipu1_ic_prpvf capture':0[1]" -- Best regards, Marek Vasut
[PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline
In case the PRPVF is not connected directly to CSI, the PRPVF subdev
driver won't find the CSI subdev and will not configure the CSI input
mux. This is not noticable on the IPU1-CSI0 interface with parallel
camera, since the mux is set "correctly" by default and the parallel
camera will work just fine. This is however noticable on IPU2-CSI1,
where the mux is not set to the correct position by default and the
pipeline will fail.
Add similar code to what is in PRPVF to VDIC driver, so that the VDIC
can locate the CSI subdev and configure the mux correctly if the CSI
is connected to the VDIC. Make the PRPVF driver configure the CSI mux
only in case it's connected directly to CSI and not in case it is
connected to VDIC.
Signed-off-by: Marek Vasut
Cc: Philipp Zabel
Cc: Steve Longerbeam
---
drivers/staging/media/imx/imx-ic-prp.c | 6 ++
drivers/staging/media/imx/imx-media-vdic.c | 24
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prp.c
b/drivers/staging/media/imx/imx-ic-prp.c
index 98923fc844ce..84fa66dae21a 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -72,14 +72,12 @@ static inline struct prp_priv *sd_to_priv(struct
v4l2_subdev *sd)
static int prp_start(struct prp_priv *priv)
{
struct imx_ic_priv *ic_priv = priv->ic_priv;
- bool src_is_vdic;
priv->ipu = priv->md->ipu[ic_priv->ipu_id];
/* set IC to receive from CSI or VDI depending on source */
- src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
-
- ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
+ if (!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC))
+ ipu_set_ic_src_mux(priv->ipu, priv->csi_id, false);
return 0;
}
diff --git a/drivers/staging/media/imx/imx-media-vdic.c
b/drivers/staging/media/imx/imx-media-vdic.c
index b538bbebedc5..e660911e7024 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -117,6 +117,9 @@ struct vdic_priv {
bool csi_direct; /* using direct CSI->VDIC->IC pipeline */
+ /* the CSI id at link validate */
+ int csi_id;
+
/* motion select control */
struct v4l2_ctrl_handler ctrl_hdlr;
enum ipu_motion_sel motion;
@@ -388,6 +391,9 @@ static int vdic_start(struct vdic_priv *priv)
if (ret)
return ret;
+ /* set IC to receive from CSI or VDI depending on source */
+ ipu_set_ic_src_mux(priv->ipu, priv->csi_id, true);
+
/*
* init the VDIC.
*
@@ -778,6 +784,7 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
struct v4l2_subdev_format *sink_fmt)
{
struct vdic_priv *priv = v4l2_get_subdevdata(sd);
+ struct imx_media_subdev *csi;
int ret;
ret = v4l2_subdev_link_validate_default(sd, link,
@@ -785,6 +792,23 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
if (ret)
return ret;
+ csi = imx_media_find_upstream_subdev(priv->md, priv->src,
+IMX_MEDIA_GRP_ID_CSI);
+ if (!IS_ERR(csi)) {
+ switch (csi->sd->grp_id) {
+ case IMX_MEDIA_GRP_ID_CSI0:
+ priv->csi_id = 0;
+ break;
+ case IMX_MEDIA_GRP_ID_CSI1:
+ priv->csi_id = 1;
+ break;
+ default:
+ ret = -EINVAL;
+ }
+ } else {
+ priv->csi_id = 0;
+ }
+
mutex_lock(&priv->lock);
if (priv->csi_direct && priv->motion != HIGH_MOTION) {
--
2.16.2
[PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes
The BT1120 interlaced CCIR codes are the same as BT656 ones and different than BT656 progressive CCIR codes, fix this. Signed-off-by: Marek Vasut Cc: Steve Longerbeam Cc: Philipp Zabel --- drivers/gpu/ipu-v3/ipu-csi.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c index caa05b0702e1..301a729581ce 100644 --- a/drivers/gpu/ipu-v3/ipu-csi.c +++ b/drivers/gpu/ipu-v3/ipu-csi.c @@ -435,12 +435,16 @@ int ipu_csi_init_interface(struct ipu_csi *csi, break; case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR: case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR: - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: ipu_csi_write(csi, 0x40030 | CSI_CCIR_ERR_DET_EN, CSI_CCIR_CODE_1); ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3); break; + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN, CSI_CCIR_CODE_1); + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2); + ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3); + break; case IPU_CSI_CLK_MODE_GATED_CLK: case IPU_CSI_CLK_MODE_NONGATED_CLK: ipu_csi_write(csi, 0, CSI_CCIR_CODE_1); -- 2.16.2
[PATCH] media: imx: Skip every second frame in VDIC DIRECT mode
In VDIC direct mode, the VDIC applies combing filter during and doubles the framerate, that is, after the first two half-frames are received and the first frame is emitted by the VDIC, every subsequent half-frame is patched into the result and a full frame is produced. The half-frame order in the full frames is as follows 12 32 34 54 etc. Drop every second frame to trim the framerate back to the original one of the signal and skip the odd patched frames. Signed-off-by: Marek Vasut Cc: Steve Longerbeam Cc: Philipp Zabel --- drivers/staging/media/imx/imx-media-vdic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c index 482250d47e7c..b538bbebedc5 100644 --- a/drivers/staging/media/imx/imx-media-vdic.c +++ b/drivers/staging/media/imx/imx-media-vdic.c @@ -289,6 +289,7 @@ static int vdic_setup_direct(struct vdic_priv *priv) /* set VDIC to receive from CSI for direct path */ ipu_fsu_link(priv->ipu, IPUV3_CHANNEL_CSI_DIRECT, IPUV3_CHANNEL_CSI_VDI_PREV); + ipu_set_vdi_skip(priv->ipu, 0x2); return 0; } @@ -313,6 +314,8 @@ static int vdic_setup_indirect(struct vdic_priv *priv) const struct imx_media_pixfmt *incc; int in_size, ret; + ipu_set_vdi_skip(priv->ipu, 0x0); + infmt = &priv->format_mbus[VDIC_SINK_PAD_IDMAC]; incc = priv->cc[VDIC_SINK_PAD_IDMAC]; -- 2.16.2
Re: [PATCH v5] media: video-i2c: add video-i2c driver
On 01/09/2017 06:17 AM, Matt Ranostay wrote:
> Gentle ping on this! :)
Just some high-level feedback ... You should use regmap instead. Also,
calling a driver which is specific to a particular sensor (amg88x) by
generic name (video_i2c) is probably not a good idea.
> Thanks,
>
> Matt
>
>> On Dec 23, 2016, at 19:04, Matt Ranostay wrote:
>>
>> There are several thermal sensors that only have a low-speed bus
>> interface but output valid video data. This patchset enables support
>> for the AMG88xx "Grid-Eye" sensor family.
>>
>> Cc: Attila Kinali
>> Cc: Marek Vasut
>> Cc: Luca Barbato
>> Cc: Laurent Pinchart
>> Signed-off-by: Matt Ranostay
>> ---
>> Changes from v1:
>> * correct i2c_polling_remove() operations
>> * fixed delay calcuation in buffer_queue()
>> * add include linux/slab.h
>>
>> Changes from v2:
>> * fix build error due to typo in include of slab.h
>>
>> Changes from v3:
>> * switch data transport to a kthread to avoid to .buf_queue that can't sleep
>> * change naming from i2c-polling to video-i2c
>> * make the driver for single chipset under another uses the driver
>>
>> Changes from v4:
>> * fix wraparound issue with jiffies and schedule_timeout_interruptible()
>>
>> drivers/media/i2c/Kconfig | 9 +
>> drivers/media/i2c/Makefile| 1 +
>> drivers/media/i2c/video-i2c.c | 569
>> ++
>> 3 files changed, 579 insertions(+)
>> create mode 100644 drivers/media/i2c/video-i2c.c
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index b31fa6fae009..ef84715293a9 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -768,6 +768,15 @@ config VIDEO_M52790
>>
>> To compile this driver as a module, choose M here: the
>> module will be called m52790.
>> +
>> +config VIDEO_I2C
>> +tristate "I2C transport video support"
>> +depends on VIDEO_V4L2 && I2C
>> +select VIDEOBUF2_VMALLOC
>> +---help---
>> + Enable the I2C transport video support which supports the
>> + following:
>> + * Panasonic AMG88xx Grid-Eye Sensors
>> endmenu
>>
>> menu "Sensors used on soc_camera driver"
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 92773b2e6225..7c8eeb213c3b 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -79,6 +79,7 @@ obj-$(CONFIG_VIDEO_LM3646)+= lm3646.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_I2C)+= video-i2c.o
>> obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>> obj-$(CONFIG_VIDEO_OV2659)+= ov2659.o
>> obj-$(CONFIG_VIDEO_TC358743)+= tc358743.o
>> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
>> new file mode 100644
>> index ..9390560bd117
>> --- /dev/null
>> +++ b/drivers/media/i2c/video-i2c.c
>> @@ -0,0 +1,569 @@
>> +/*
>> + * video-i2c.c - Support for I2C transport video devices
>> + *
>> + * Copyright (C) 2016 Matt Ranostay
>> + *
>> + * 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.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Supported:
>> + * - Panasonic AMG88xx Grid-Eye Sensors
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define VIDEO_I2C_DRIVER"video-i2c"
>> +#define MAX_BUFFER_SIZE128
>> +
>> +struct video_i2c_chip;
>> +
>> +struct video_i2c_buffer {
>> +struct vb2_v4l2_buffer vb;
>> +struct list_head list;
>> +};
>> +
>> +struct video_i2c_data {
>> +struct i
Re: [PATCH v2 00/21] Basic i.MX IPUv3 capture support
On 12/29/2016 09:51 PM, Robert Schwebel wrote: > Hi Jean-Michel, Hi, > On Thu, Dec 29, 2016 at 04:08:33PM +0100, Jean-Michel Hautbois wrote: >> What is the status of this work? > > Philipp's patches have been reworked with the review feedback from the > last round and a new version will be posted when he is back from > holidays. IMO Philipp's patches are better integrated and well structured, so I'd rather like to see his work in at some point. -- Best regards, Marek Vasut -- 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: [RFC] v4l2 support for thermopile devices
On 10/28/2016 10:30 PM, Devin Heitmueller wrote: > Hi Matt, > >> Need some input for the video pixel data types, which the device we >> are using (see datasheet links below) is outputting pixel data in >> little endian 16-bit of which a 12-bits signed value is used. Does it >> make sense to do some basic processing on the data since greyscale is >> going to look weird with temperatures under 0C degrees? Namely a cold >> object is going to be brighter than the hottest object it could read. >> Or should a new V4L2_PIX_FMT_* be defined and processing done in >> software? Another issue is how to report the scaling value of 0.25 C >> for each LSB of the pixels to the respecting recording application. > > Regarding the format for the pixel data: I did some research into > this when doing some driver work for the Seek Thermal (a product > similar to the FLIR Lepton). While it would be nice to be able to use > an existing application like VLC or gStreamer to just take the video > and capture from the V4L2 interface with no additional userland code, > the reality is that how you colorize the data is going to be highly > user specific (e.g. what thermal ranges to show with what colors, > etc). If your goal is really to do a V4L2 driver which returns the > raw data, then you're probably best returning it in the native > greyscale format (whether that be an existing V4L2 PIX_FMT or a new > one needs to be defined), and then in software you can figure out how > to colorize it. All true, I also did my share of poking into SEEK Thermal USB and it is an excellent candidate for a V4L2 driver, that one. But I think this device here is producing much smaller images, something like 8x8 pixels. -- Best regards, Marek Vasut -- 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 08/21] [media] imx: Add i.MX IPUv3 capture driver
0 (2 pads, 1 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "imx-ipuv3-capture.0":0 [ENABLED]
>
> - entity 4: imx-ipuv3-capture.0 (1 pad, 1 link)
> type Node subtype V4L flags 0
> device node name /dev/video0
> pad0: Sink
> <- "IPU0 CSI0":1 [ENABLED]
>
> - entity 10: IPU0 CSI1 (2 pads, 1 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "imx-ipuv3-capture.1":0 [ENABLED]
>
> - entity 13: imx-ipuv3-capture.1 (1 pad, 1 link)
> type Node subtype V4L flags 0
> device node name /dev/video1
> pad0: Sink
> <- "IPU0 CSI1":1 [ENABLED]
>
> - entity 19: IPU1 CSI0 (2 pads, 1 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "imx-ipuv3-capture.0":0 [ENABLED]
>
> - entity 22: imx-ipuv3-capture.0 (1 pad, 1 link)
> type Node subtype V4L flags 0
> device node name /dev/video2
> pad0: Sink
> <- "IPU1 CSI0":1 [ENABLED]
>
> - entity 28: IPU1 CSI1 (2 pads, 1 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "imx-ipuv3-capture.1":0 [ENABLED]
>
> - entity 31: imx-ipuv3-capture.1 (1 pad, 1 link)
> type Node subtype V4L flags 0
> device node name /dev/video3
> pad0: Sink
> <- "IPU1 CSI1":1 [ENABLED]
>
> - entity 37: mipi_ipu1_mux (3 pads, 0 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Sink
> pad2: Source
>
> - entity 41: mipi_ipu2_mux (3 pads, 0 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Sink
> pad2: Source
>
> - entity 45: ar0135 1-0010 (1 pad, 0 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Source
>
>
>
>
> root@imx6:~# media-ctl -v --links '"ar01351-0010":0->"mipi_ipu1_mux":0[1]'
>
> Opening media device /dev/media0
> Enumerating entities
> Found 11 entities
> Enumerating pads and links
> No link between "ar0135 1-0010":0 and "mipi_ipu1_mux":0
> media_parse_setup_link: Unable to parse link
>
> "ar0135 1-0010":0->"mipi_ipu1_mux":0[1]
> ^
> Unable to parse link: Invalid argument (22)
>
> If you have something in the works with a camera example then just tell
> me to be patient and I'll wait for a v3 ;)
Check whether you have something along these lines in your camera driver
in the probe() function:
priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
priv->subdev.dev = &client->dev;
priv->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_init(&priv->subdev.entity, 1, &priv->pad, 0);
if (ret < 0) {
v4l2_clk_put(priv->clk);
return ret;
}
ret = v4l2_async_register_subdev(&priv->subdev);
if (ret < 0) {
v4l2_clk_put(priv->clk);
return ret;
}
--
Best regards,
Marek Vasut
--
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 08/21] [media] imx: Add i.MX IPUv3 capture driver
On 10/17/2016 01:32 PM, Jack Mitchell wrote: > Hi Philipp, Hi, > I'm looking at how I would enable a parallel greyscale camera using this > set of drivers and am a little bit confused. Do you have an example > somewhere of a devicetree with an input node. I also have a further note > below: Which sensor do you use ? [...] -- Best regards, Marek Vasut -- 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 18/22] [media] imx-ipuv3-csi: support downsizing
On 10/14/2016 05:48 PM, Philipp Zabel wrote:
> Am Freitag, den 07.10.2016, 21:01 +0200 schrieb Marek Vasut:
>> On 10/07/2016 06:01 PM, Philipp Zabel wrote:
>>> Add support for the CSI internal horizontal and vertical downsizing.
>>>
>>> Signed-off-by: Philipp Zabel
>>> ---
>>> drivers/media/platform/imx/imx-ipuv3-csi.c | 20 ++--
>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/imx/imx-ipuv3-csi.c
>>> b/drivers/media/platform/imx/imx-ipuv3-csi.c
>>> index 699460e6..e8a6a7b 100644
>>> --- a/drivers/media/platform/imx/imx-ipuv3-csi.c
>>> +++ b/drivers/media/platform/imx/imx-ipuv3-csi.c
>>> @@ -167,8 +167,16 @@ static int ipucsi_subdev_set_format(struct v4l2_subdev
>>> *sd,
>>> width = clamp_t(unsigned int, sdformat->format.width, 16, 8192);
>>> height = clamp_t(unsigned int, sdformat->format.height, 16,
>>> 4096);
>>> } else {
>>> - width = ipucsi->format_mbus[0].width;
>>> - height = ipucsi->format_mbus[0].height;
>>> + if (sdformat->format.width <
>>> + (ipucsi->format_mbus[0].width * 3 / 4))
>>> + width = ipucsi->format_mbus[0].width / 2;
>>> + else
>>> + width = ipucsi->format_mbus[0].width;
>>> + if (sdformat->format.height <
>>> + (ipucsi->format_mbus[0].height * 3 / 4))
>>> + height = ipucsi->format_mbus[0].height / 2;
>>> + else
>>> + height = ipucsi->format_mbus[0].height;
>>> }
>>>
>>> mbusformat = __ipucsi_get_pad_format(sd, cfg, sdformat->pad,
>>> @@ -212,14 +220,14 @@ static int ipucsi_subdev_s_stream(struct v4l2_subdev
>>> *sd, int enable)
>>> window.width = fmt[0].width;
>>> window.height = fmt[0].height;
>>> ipu_csi_set_window(ipucsi->csi, &window);
>>> + ipu_csi_set_downsize(ipucsi->csi,
>>> +fmt[0].width == 2 * fmt[1].width,
>>> +fmt[0].height == 2 * fmt[1].height);
>>>
>>> /* Is CSI data source MCT (MIPI)? */
>>> mux_mct = (mbus_config.type == V4L2_MBUS_CSI2);
>>> -
>>> ipu_set_csi_src_mux(ipucsi->ipu, ipucsi->id, mux_mct);
>>> - if (mux_mct)
>>> - ipu_csi_set_mipi_datatype(ipucsi->csi, /*VC*/ 0,
>>> - &fmt[0]);
>>> + ipu_csi_set_mipi_datatype(ipucsi->csi, /*VC*/ 0, &fmt[0]);
>>
>> This probably needs fixing , so that the correct VC is passed in ?
>
> Absolutely, right now I don't know how though.
> We are still missing API to set the MIPI CSI-2 virtual channel.
Right. And since most cameras use VC0 anyway, it's unlikely anyone will
be severely affected by this, so this shouldn't be considered a blocker
for this patchset. Maybe add a comment, something along the lines of
"FIXME: We are still missing an API for setting VC != 0" .
--
Best regards,
Marek Vasut
--
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 18/22] [media] imx-ipuv3-csi: support downsizing
On 10/07/2016 06:01 PM, Philipp Zabel wrote:
> Add support for the CSI internal horizontal and vertical downsizing.
>
> Signed-off-by: Philipp Zabel
> ---
> drivers/media/platform/imx/imx-ipuv3-csi.c | 20 ++--
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/platform/imx/imx-ipuv3-csi.c
> b/drivers/media/platform/imx/imx-ipuv3-csi.c
> index 699460e6..e8a6a7b 100644
> --- a/drivers/media/platform/imx/imx-ipuv3-csi.c
> +++ b/drivers/media/platform/imx/imx-ipuv3-csi.c
> @@ -167,8 +167,16 @@ static int ipucsi_subdev_set_format(struct v4l2_subdev
> *sd,
> width = clamp_t(unsigned int, sdformat->format.width, 16, 8192);
> height = clamp_t(unsigned int, sdformat->format.height, 16,
> 4096);
> } else {
> - width = ipucsi->format_mbus[0].width;
> - height = ipucsi->format_mbus[0].height;
> + if (sdformat->format.width <
> + (ipucsi->format_mbus[0].width * 3 / 4))
> + width = ipucsi->format_mbus[0].width / 2;
> + else
> + width = ipucsi->format_mbus[0].width;
> + if (sdformat->format.height <
> + (ipucsi->format_mbus[0].height * 3 / 4))
> + height = ipucsi->format_mbus[0].height / 2;
> + else
> + height = ipucsi->format_mbus[0].height;
> }
>
> mbusformat = __ipucsi_get_pad_format(sd, cfg, sdformat->pad,
> @@ -212,14 +220,14 @@ static int ipucsi_subdev_s_stream(struct v4l2_subdev
> *sd, int enable)
> window.width = fmt[0].width;
> window.height = fmt[0].height;
> ipu_csi_set_window(ipucsi->csi, &window);
> + ipu_csi_set_downsize(ipucsi->csi,
> + fmt[0].width == 2 * fmt[1].width,
> + fmt[0].height == 2 * fmt[1].height);
>
> /* Is CSI data source MCT (MIPI)? */
> mux_mct = (mbus_config.type == V4L2_MBUS_CSI2);
> -
> ipu_set_csi_src_mux(ipucsi->ipu, ipucsi->id, mux_mct);
> - if (mux_mct)
> - ipu_csi_set_mipi_datatype(ipucsi->csi, /*VC*/ 0,
> - &fmt[0]);
> + ipu_csi_set_mipi_datatype(ipucsi->csi, /*VC*/ 0, &fmt[0]);
This probably needs fixing , so that the correct VC is passed in ?
> ret = ipu_csi_init_interface(ipucsi->csi, &mbus_config,
>&fmt[0]);
>
--
Best regards,
Marek Vasut
--
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 12/22] [media] tc358743: put lanes in STOP state before starting streaming
On 10/07/2016 06:00 PM, Philipp Zabel wrote:
> Without calling tc358743_set_csi from the new prepare_stream callback
> (or calling tc358743_s_dv_timings or tc358743_set_fmt from userspace
> after stopping the stream), the i.MX6 MIPI CSI2 input fails waiting
> for lanes to enter STOP state when streaming is started again.
What is the impact of that failure ? How does it manifest itself ?
> Signed-off-by: Philipp Zabel
> ---
> drivers/media/i2c/tc358743.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 1e3a0dd2..dfa45d2 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1463,6 +1463,14 @@ static int tc358743_g_mbus_config(struct v4l2_subdev
> *sd,
> return 0;
> }
>
> +static int tc358743_prepare_stream(struct v4l2_subdev *sd)
> +{
> + /* Put all lanes in PL-11 state (STOPSTATE) */
> + tc358743_set_csi(sd);
> +
> + return 0;
> +}
> +
> static int tc358743_s_stream(struct v4l2_subdev *sd, int enable)
> {
> enable_stream(sd, enable);
> @@ -1637,6 +1645,7 @@ static const struct v4l2_subdev_video_ops
> tc358743_video_ops = {
> .g_dv_timings = tc358743_g_dv_timings,
> .query_dv_timings = tc358743_query_dv_timings,
> .g_mbus_config = tc358743_g_mbus_config,
> + .prepare_stream = tc358743_prepare_stream,
> .s_stream = tc358743_s_stream,
> };
>
>
--
Best regards,
Marek Vasut
--
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 03/22] [media] v4l: of: add v4l2_of_subdev_registered
On 10/07/2016 06:00 PM, Philipp Zabel wrote:
> Provide a default registered callback for device tree probed subdevices
> that use OF graph bindings to add still missing source subdevices to
> the async notifier waiting list.
> This is only necessary for subdevices that have input ports to which
> other subdevices are connected that are not initially known to the
> master/bridge device when it sets up the notifier.
>
> Signed-off-by: Philipp Zabel
[...]
> +int v4l2_of_subdev_registered(struct v4l2_subdev *sd)
> +{
> + struct device_node *ep;
> +
> + for_each_endpoint_of_node(sd->of_node, ep) {
> + struct v4l2_of_link link;
> + struct media_entity *entity;
> + unsigned int pad;
> + int ret;
> +
> + ret = v4l2_of_parse_link(ep, &link);
> + if (ret)
> + continue;
> +
> + /*
> + * Assume 1:1 correspondence between OF node and entity,
> + * and between OF port numbers and pad indices.
> + */
> + entity = &sd->entity;
This here will not compile if CONFIG_MEDIA_CONTROLLER is not set,
because ->entity will be missing from struct v4l2_subdev {} .
> + pad = link.local_port;
> +
> + if (pad >= entity->num_pads)
> + return -EINVAL;
> +
> + /* Add source subdevs to async notifier */
> + if (entity->pads[pad].flags & MEDIA_PAD_FL_SINK) {
> + struct v4l2_async_subdev *asd;
> +
> + asd = devm_kzalloc(sd->dev, sizeof(*asd), GFP_KERNEL);
> + if (!asd) {
> + v4l2_of_put_link(&link);
> + return -ENOMEM;
> + }
> +
> + asd->match_type = V4L2_ASYNC_MATCH_OF;
> + asd->match.of.node = link.remote_node;
> +
> + __v4l2_async_notifier_add_subdev(sd->notifier, asd);
> + }
> +
> + v4l2_of_put_link(&link);
> + }
> +
> + return 0;
> +}
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> index 4dc34b2..67d4f8b 100644
> --- a/include/media/v4l2-of.h
> +++ b/include/media/v4l2-of.h
> @@ -22,6 +22,8 @@
> #include
>
> struct device_node;
> +struct v4l2_device;
> +struct v4l2_subdev;
>
> /**
> * struct v4l2_of_bus_mipi_csi2 - MIPI CSI-2 bus data structure
> @@ -95,6 +97,9 @@ void v4l2_of_free_endpoint(struct v4l2_of_endpoint
> *endpoint);
> int v4l2_of_parse_link(const struct device_node *node,
> struct v4l2_of_link *link);
> void v4l2_of_put_link(struct v4l2_of_link *link);
> +int v4l2_of_subdev_registered(struct v4l2_subdev *sd);
> +struct v4l2_subdev *v4l2_find_subdev_by_node(struct v4l2_device *v4l2_dev,
> + struct device_node *node);
> #else /* CONFIG_OF */
>
> static inline int v4l2_of_parse_endpoint(const struct device_node *node,
> @@ -123,6 +128,13 @@ static inline void v4l2_of_put_link(struct v4l2_of_link
> *link)
> {
> }
>
> +#define v4l2_of_subdev_registered NULL
> +
> +struct v4l2_subdev *v4l2_find_subdev_by_node(struct v4l2_device *v4l2_dev,
> + struct device_node *node)
> +{
> + return NULL;
> +}
> #endif /* CONFIG_OF */
>
> #endif /* _V4L2_OF_H */
>
--
Best regards,
Marek Vasut
--
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 01/22] [media] v4l2-async: move code out of v4l2_async_notifier_register into v4l2_async_test_nofity_all
On 10/07/2016 06:00 PM, Philipp Zabel wrote:
> This will be reused in the following patch to catch already registered,
> newly added asynchronous subdevices from v4l2_async_register_subdev.
>
> Signed-off-by: Philipp Zabel
> ---
> drivers/media/v4l2-core/v4l2-async.c | 38
> +---
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c
> index 5bada20..c4f1930 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -134,11 +134,31 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
> sd->dev = NULL;
> }
>
> +static int v4l2_async_test_notify_all(struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd, *tmp;
> +
> + list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
> + struct v4l2_async_subdev *asd;
> + int ret;
> +
> + asd = v4l2_async_belongs(notifier, sd);
> + if (!asd)
> + continue;
> +
> + ret = v4l2_async_test_notify(notifier, sd, asd);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
>struct v4l2_async_notifier *notifier)
> {
> - struct v4l2_subdev *sd, *tmp;
> struct v4l2_async_subdev *asd;
> + int ret;
> int i;
>
> if (!notifier->num_subdevs || notifier->num_subdevs > V4L2_MAX_SUBDEVS)
> @@ -171,23 +191,9 @@ int v4l2_async_notifier_register(struct v4l2_device
> *v4l2_dev,
> /* Keep also completed notifiers on the list */
> list_add(¬ifier->list, ¬ifier_list);
>
> - list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
> - int ret;
> -
> - asd = v4l2_async_belongs(notifier, sd);
> - if (!asd)
> - continue;
> -
> - ret = v4l2_async_test_notify(notifier, sd, asd);
> - if (ret < 0) {
> - mutex_unlock(&list_lock);
> - return ret;
> - }
> - }
Shouldn't you call ret = v4l2_async_test_notify_all() here now instead ?
> mutex_unlock(&list_lock);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL(v4l2_async_notifier_register);
>
>
--
Best regards,
Marek Vasut
--
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 v1 1/5] seq_file: provide an analogue of print_hex_dump()
On Wednesday, July 09, 2014 at 11:21:08 PM, Joe Perches wrote:
> On Wed, 2014-07-09 at 22:39 +0200, Marek Vasut wrote:
> > The above function looks like almost verbatim copy of print_hex_dump().
> > The only difference I can spot is that it's calling seq_printf() instead
> > of printk(). Can you not instead generalize print_hex_dump() and based
> > on it's invocation, make it call either seq_printf() or printk() ?
>
> How do you propose doing that given any seq_ call
> requires a struct seq_file * and print_hex_dump needs
> a KERN_.
I can imagine a rather nasty way, I can't say I would like it myself tho. The
general idea would be to pull out the entire switch {} statement into a
separate
functions , one for printk() and one for seq_printf() cases. Then, have a
generic do_hex_dump() call which would take as an argument a pointer to either
of those functions and a void * to either the seq_file or level . Finally,
there
would have to be a wrapper to call the do_hex_dump() with the correct function
pointer and it's associated arg.
Nasty? Yes ... Ineffective? Most likely.
> Is there an actual value to it?
Reducing the code duplication, but I wonder if there is a smarter solution than
the horrid one above.
Best regards,
Marek Vasut
--
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 v1 1/5] seq_file: provide an analogue of print_hex_dump()
On Wednesday, July 09, 2014 at 05:24:26 PM, Andy Shevchenko wrote:
> The new seq_hex_dump() is a complete analogue of print_hex_dump().
>
> We have few users of this functionality already. It allows to reduce their
> codebase.
>
> Signed-off-by: Andy Shevchenko
> ---
> fs/seq_file.c| 35 +++
> include/linux/seq_file.h | 4
> 2 files changed, 39 insertions(+)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index 3857b72..fec4a6b 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -794,6 +795,40 @@ void seq_pad(struct seq_file *m, char c)
> }
> EXPORT_SYMBOL(seq_pad);
>
> +/* Analogue of print_hex_dump() */
> +void seq_hex_dump(struct seq_file *m, const char *prefix_str, int
> prefix_type, + int rowsize, int groupsize, const void *buf,
size_t len,
> + bool ascii)
> +{
> + const u8 *ptr = buf;
> + int i, linelen, remaining = len;
> + unsigned char linebuf[32 * 3 + 2 + 32 + 1];
> +
> + if (rowsize != 16 && rowsize != 32)
> + rowsize = 16;
> +
> + for (i = 0; i < len; i += rowsize) {
> + linelen = min(remaining, rowsize);
> + remaining -= rowsize;
> +
> + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
> +linebuf, sizeof(linebuf), ascii);
> +
> + switch (prefix_type) {
> + case DUMP_PREFIX_ADDRESS:
> + seq_printf(m, "%s%p: %s\n", prefix_str, ptr + i,
linebuf);
> + break;
> + case DUMP_PREFIX_OFFSET:
> + seq_printf(m, "%s%.8x: %s\n", prefix_str, i, linebuf);
> + break;
> + default:
> + seq_printf(m, "%s%s\n", prefix_str, linebuf);
> + break;
> + }
> + }
> +}
> +EXPORT_SYMBOL(seq_hex_dump);
The above function looks like almost verbatim copy of print_hex_dump(). The
only
difference I can spot is that it's calling seq_printf() instead of printk().
Can
you not instead generalize print_hex_dump() and based on it's invocation, make
it call either seq_printf() or printk() ?
Best regards,
--
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: [PATCHv5][ 3/8] staging: imx-drm: Correct BGR666 and the board's dts that use them.
On Thursday, December 05, 2013 at 07:28:07 PM, Denis Carikli wrote: [...] Can you please explain the correction here ? Why is it needed ? What was the problem ? Thanks! Best regards, Marek Vasut -- 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: [PATCHv16 0/7] of: add display helper
Dear Steffen Trumtrar, > On Tue, Dec 18, 2012 at 06:04:09PM +0100, Steffen Trumtrar wrote: > > Hi! > > > > Finally, right in time before the end of the world on friday, v16 of the > > display helpers. > > So, any more criticism on the series? Any takers for the series as is? > I guess it could be merged via the fbdev-tree if David Airlie can agree > to the DRM patches ?! Does that sound about right? > > I think the series was tested at least with > - imx6q: sabrelite, sabresd > - imx53: tqma53/mba53 > - omap: DA850 EVM, AM335x EVM, EVM-SK > > I don't know what Laurent Pinchart, Marek Vasut and Leela Krishna Amudala > are using. MX53QSB and another custom MX53 board. > Those are the people I know from the top of my head, that use > or at least did use the patches in one of its iterations. If I forgot > anyone, please speak up and possibly add your new HW to the list of tested > devices. > > Thanks, > Steffen Best regards, Marek Vasut -- 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: [PATCHv16 0/7] of: add display helper
Dear Steffen Trumtrar,
> Hi!
>
> On Wed, Jan 09, 2013 at 08:12:01PM +0100, Marek Vasut wrote:
> > Dear Steffen Trumtrar,
> >
> > I tested this on 3.8-rc1 (next 20130103) with the imx drm driver. After
> > adding the following piece of code (quick hack), this works just fine.
> > Thanks!
> >
> > diff --git a/drivers/staging/imx-drm/parallel-display.c
> > b/drivers/staging/imx- drm/parallel-display.c
> > index a8064fc..e45002a 100644
> > --- a/drivers/staging/imx-drm/parallel-display.c
> > +++ b/drivers/staging/imx-drm/parallel-display.c
> > @@ -57,6 +57,7 @@ static void imx_pd_connector_destroy(struct
> > drm_connector *connector)
> >
> > static int imx_pd_connector_get_modes(struct drm_connector *connector)
> > {
> >
> > struct imx_parallel_display *imxpd = con_to_imxpd(connector);
> >
> > + struct device_node *np = imxpd->dev->of_node;
> >
> > int num_modes = 0;
> >
> > if (imxpd->edid) {
> >
> > @@ -72,6 +73,15 @@ static int imx_pd_connector_get_modes(struct
> > drm_connector *connector)
> >
> > num_modes++;
> >
> > }
> >
> > + if (np) {
> > + struct drm_display_mode *mode =
> > drm_mode_create(connector->dev); +
> > of_get_drm_display_mode(np, &imxpd->mode, 0);
> > + drm_mode_copy(mode, &imxpd->mode);
> > + mode->type |= DRM_MODE_TYPE_DRIVER |
> > DRM_MODE_TYPE_PREFERRED, + drm_mode_probed_add(connector,
> > mode);
> > + num_modes++;
> > + }
> > +
> >
> > return num_modes;
> >
> > }
>
> Nice! I haven't tried the parallel display, but I think Philipp Zabel might
> already have a patch for it. If not, I will definitly keep your patch in my
> topic branch.
Works like charm here.
Make sure to adjust the patch and check for the return value of
of_get_drm_display_mode(np, &imxpd->mode, 0); call, that's probably the only
issue that needs fixing in that hack. Checking if np != NULL might not hurt
either. I can roll you a real patch if it helps.
Best regards,
Marek Vasut
--
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: [PATCHv16 0/7] of: add display helper
Dear Steffen Trumtrar,
> Hi!
>
> Finally, right in time before the end of the world on friday, v16 of the
> display helpers.
I tested this on 3.8-rc1 (next 20130103) with the imx drm driver. After adding
the following piece of code (quick hack), this works just fine. Thanks!
diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-
drm/parallel-display.c
index a8064fc..e45002a 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -57,6 +57,7 @@ static void imx_pd_connector_destroy(struct drm_connector
*connector)
static int imx_pd_connector_get_modes(struct drm_connector *connector)
{
struct imx_parallel_display *imxpd = con_to_imxpd(connector);
+ struct device_node *np = imxpd->dev->of_node;
int num_modes = 0;
if (imxpd->edid) {
@@ -72,6 +73,15 @@ static int imx_pd_connector_get_modes(struct drm_connector
*connector)
num_modes++;
}
+ if (np) {
+ struct drm_display_mode *mode = drm_mode_create(connector->dev);
+ of_get_drm_display_mode(np, &imxpd->mode, 0);
+ drm_mode_copy(mode, &imxpd->mode);
+ mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+ drm_mode_probed_add(connector, mode);
+ num_modes++;
+ }
+
return num_modes;
}
Best regards,
Marek Vasut
--
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: Terratec Cinergy S2 USB HD
> Hi Linux Media Team > > Is there a way to get running my Cinergy S2 USB HD under Ubuntu 11.10? > Or should i give it back and buy a supported card? > > Greetz > > franco Does it bind with any driver if you connect it? M -- 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 v3] v4l: Add driver for Micron MT9M032 camera sensor
Dear Martin Hostettler, > The MT9M032 is a parallel 1.6MP sensor from Micron controlled through I2C. > > The driver creates a V4L2 subdevice. It currently supports cropping, gain, > exposure and v/h flipping controls in monochrome mode with an > external pixel clock. > > Signed-off-by: Martin Hostettler > --- > drivers/media/video/Kconfig |7 + > drivers/media/video/Makefile |1 + > drivers/media/video/mt9m032.c | 819 > + include/media/mt9m032.h | > 38 ++ > 4 files changed, 865 insertions(+), 0 deletions(-) > create mode 100644 drivers/media/video/mt9m032.c > create mode 100644 include/media/mt9m032.h > > Changes in V3 > * rebased to current mainline > * removed unneeded includes > * remove all translation code to hide black or active boundry sensor > pixels. Userspace now needs to select crop in original device coordinates. > * remove useless consts, casts and defines > * changed enum_frame_size to return min == max > * removed duplicated input validation > * validate crop rectangle on set_crop > > Changes in V2 > * ported to current mainline > * Moved dispatching for subdev ioctls VIDIOC_DBG_G_REGISTER and > VIDIOC_DBG_S_REGISTER into v4l2-subdev * Removed VIDIOC_DBG_G_CHIP_IDENT > support > * moved header to media/ > * Fixed missing error handling > * lowercase hex constants > * moved v4l2_get_subdevdata to register access helpers > * use div_u64 instead of do_div > * moved all know register values into #define:ed constants > * Fixed error reporting, used clamp instead of open coding. > * lots of style fixes. > * add try_ctrl to make sure user space sees rounded values > * Fixed some problem in control framework usage. > * Fixed set_format to force width and height setup via crop. Simplyfied > code. > > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig > index b303a3f..cf05d9b 100644 > --- a/drivers/media/video/Kconfig > +++ b/drivers/media/video/Kconfig > @@ -820,6 +820,13 @@ config SOC_CAMERA_MT9M001 > This driver supports MT9M001 cameras from Micron, monochrome > and colour models. > > +config VIDEO_MT9M032 > + tristate "MT9M032 camera sensor support" > + depends on I2C && VIDEO_V4L2 > + help > + This driver supports MT9M032 cameras from Micron, monochrome > + models only. > + > config SOC_CAMERA_MT9M111 > tristate "mt9m111, mt9m112 and mt9m131 support" > depends on SOC_CAMERA && I2C > diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile > index 117f9c4..39c7455 100644 > --- a/drivers/media/video/Makefile > +++ b/drivers/media/video/Makefile > @@ -77,6 +77,7 @@ obj-$(CONFIG_VIDEO_ADP1653) += adp1653.o > > obj-$(CONFIG_SOC_CAMERA_IMX074) += imx074.o > obj-$(CONFIG_SOC_CAMERA_MT9M001) += mt9m001.o > +obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o > obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o > obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o > obj-$(CONFIG_SOC_CAMERA_MT9T112) += mt9t112.o > diff --git a/drivers/media/video/mt9m032.c b/drivers/media/video/mt9m032.c > new file mode 100644 > index 000..b4159c7 > --- /dev/null > +++ b/drivers/media/video/mt9m032.c > @@ -0,0 +1,819 @@ > +/* > + * Driver for MT9M032 CMOS Image Sensor from Micron > + * > + * Copyright (C) 2010-2011 Lund Engineering > + * Contact: Gil Lund > + * Author: Martin Hostettler > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA > + * 02110-1301 USA > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#include > + > +#define MT9M032_CHIP_VERSION 0x00 > +#define MT9M032_ROW_START0x01 > +#define MT9M032_COLUMN_START 0x02 > +#define MT9M032_ROW_SIZE 0x03 > +#define MT9M032_COLUMN_SIZE 0x04 > +#define MT9M032_HBLANK 0x05 > +#define MT9M032_VBLANK 0x06 > +#define MT9M032_SHUTTER_WIDTH_HIGH 0x08 > +#define MT9M032_SHUTTER_WIDTH_LOW0x09 > +#define MT9M032_PIX_CLK_CTRL 0x0a > +#define MT9M032_PIX_CLK_CTRL_INV_PIXCLK 0x8000 > +#define MT9M032_RESTART 0x0b > +#define
Re: Problems cloning the git repostories
> On Sun, Sep 25, 2011 at 8:33 AM, Patrick Dickey wrote: > > Hello there, > > > > I tried to follow the steps for cloning both the "media_tree.git" and > > "media_build.git" repositories, and received errors for both. The > > media_tree repository failed on the first line > > > >> git clone > >> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git > >> v4l-dvb > > > > which I'm assuming is because kernel.org is down. > > > > The media_build.git repository fails on the first line also > > > >> git clone git://linuxtv.org/media_build.git > > > > with a fatal: read error: Connection reset by peer. > > > > Is it possible to clone either (or both) repositories at this time, or > > are they down? And in the absence of cloning the kernel for the > > media_tree.git repository, is it possible to simply clone the > > git://linuxtv.org/media_tree.git repository and work from that? > > > > My interest in this is to install some patches created by Devin > > Heitmueller for the Pinnacle PCTV 80e USB tuner (at least the ATSC > > portion of the tuner). Once I'm able to determine exactly what changes > > are made, I would like to either submit the patches to the repository, > > or send them to someone who has more experience in patching the files > > for submission. > > > > One other question (totally unrelated to this post though): When I send > > emails, normally they are GPG signed. Should I disable that for this > > list, or is it acceptable? > > > > Thank you for any information, and have a great day:) > > Patrick. > > Hi Patrick, > > As I said on the blog, the issue isn't getting the driver to work > against current kernels. Merging the driver against the current tree > is a trivial exercise (the patch series should apply trivially against > the current code, with only a few minor conflicts related to board > numbers, etc). > > The bigger issue though is once you do that and have the driver > running, you now have a body of code > 10,000 lines which doesn't meet > the "coding standards". Doing such a refactoring is a relatively > straightforward exercise but very time consuming (you already have a > working driver, so you just have to make sure you don't break > anything). > > The more I think about this, the more it annoys me. I did all the hard > parts: > > * I worked with the product vendor to get the details for the design > * I got Hauppauge/PCTV to compel the chipset vendor to release the > reference code under a GPL compatible license > * I worked out redistribution terms on the firmware > * I ported the driver to Linux > * I integrated the driver and debugged it to achieve signal lock > > And why is it not in the mainline? Because none of the above matters > if I didn't waste a bunch of my time removing a bunch of "#ifdef > WINDOWS" lines and converting whitespace from tabs to spaces. > > It's crap like this that's the reason why some of the best LinuxTV > driver authors still have a bunch of stuff that isn't merged upstream. > We just don't have time for this sort of bullshit that any monkey > could do if he/she was willing to invest the effort. We're just too > busy doing *actual* driver work. > > Five years ago the hard part was finding competent developers, getting > access to datasheets, getting access to reference driver code, and > getting access to the details for a hardware design. Now most of > those problems are not the issue - we have access to all the data but > we want to waste the time of the few competent developers out there > making them do "coding style cleanup" before perfectly good code gets > merged upstream. There has been more than one case where I've > considered doing a driver for a new board and decided against it > because the barrier to getting it upstream is not worth my time. > > Want to see more device support upstream? Optimize the process to > make it easy for the people who know the hardware and how to write the > drivers to get code upstream, and leave it to the "janitors" to work > out the codingstyle issues. Mate, I feel sorry for you :-( -- 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: femon patch for dB
> I added a switch to femon so it displays signal and snr in dB. > > The cx23885 driver for my Hauppauge WinTV-HVR1250 reports signal and snr > in dB. > > http://lockie.ca/test/femon.patch.bz2 Use git send-email to submit patches to mailing lists, thanks -- 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: [RFC PATCH 05/12] ov9640: convert to the control framework.
On Wednesday 12 January 2011 00:06:05 Hans Verkuil wrote:
> Signed-off-by: Hans Verkuil
Hey, I can do a test-run on this one eventually ;-)
Cheers
> ---
> drivers/media/video/ov9640.c | 124
> ++ drivers/media/video/ov9640.h |
> 4 +-
> 2 files changed, 43 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
> index 53d88a2..dbda50c 100644
> --- a/drivers/media/video/ov9640.c
> +++ b/drivers/media/video/ov9640.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "ov9640.h"
> @@ -162,27 +163,6 @@ static enum v4l2_mbus_pixelcode ov9640_codes[] = {
> V4L2_MBUS_FMT_RGB565_2X8_LE,
> };
>
> -static const struct v4l2_queryctrl ov9640_controls[] = {
> - {
> - .id = V4L2_CID_VFLIP,
> - .type = V4L2_CTRL_TYPE_BOOLEAN,
> - .name = "Flip Vertically",
> - .minimum= 0,
> - .maximum= 1,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_HFLIP,
> - .type = V4L2_CTRL_TYPE_BOOLEAN,
> - .name = "Flip Horizontally",
> - .minimum= 0,
> - .maximum= 1,
> - .step = 1,
> - .default_value = 0,
> - },
> -};
> -
> /* read a register */
> static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
> {
> @@ -307,52 +287,25 @@ static unsigned long ov9640_query_bus_param(struct
> soc_camera_device *icd) return soc_camera_apply_sensor_flags(icl, flags);
> }
>
> -/* Get status of additional camera capabilities */
> -static int ov9640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control
> *ctrl) -{
> - struct ov9640_priv *priv = to_ov9640_sensor(sd);
> -
> - switch (ctrl->id) {
> - case V4L2_CID_VFLIP:
> - ctrl->value = priv->flag_vflip;
> - break;
> - case V4L2_CID_HFLIP:
> - ctrl->value = priv->flag_hflip;
> - break;
> - }
> - return 0;
> -}
> -
> /* Set status of additional camera capabilities */
> -static int ov9640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control
> *ctrl) +static int ov9640_s_ctrl(struct v4l2_ctrl *ctrl)
> {
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct ov9640_priv *priv = to_ov9640_sensor(sd);
> -
> - int ret = 0;
> + struct ov9640_priv *priv = container_of(ctrl->handler, struct
> ov9640_priv, hdl); + struct i2c_client *client =
> v4l2_get_subdevdata(&priv->subdev);
>
> switch (ctrl->id) {
> case V4L2_CID_VFLIP:
> - priv->flag_vflip = ctrl->value;
> - if (ctrl->value)
> - ret = ov9640_reg_rmw(client, OV9640_MVFP,
> + if (ctrl->val)
> + return ov9640_reg_rmw(client, OV9640_MVFP,
> OV9640_MVFP_V, 0);
> - else
> - ret = ov9640_reg_rmw(client, OV9640_MVFP,
> - 0, OV9640_MVFP_V);
> - break;
> + return ov9640_reg_rmw(client, OV9640_MVFP, 0, OV9640_MVFP_V);
> case V4L2_CID_HFLIP:
> - priv->flag_hflip = ctrl->value;
> - if (ctrl->value)
> - ret = ov9640_reg_rmw(client, OV9640_MVFP,
> + if (ctrl->val)
> + return ov9640_reg_rmw(client, OV9640_MVFP,
> OV9640_MVFP_H, 0);
> - else
> - ret = ov9640_reg_rmw(client, OV9640_MVFP,
> - 0, OV9640_MVFP_H);
> - break;
> + return ov9640_reg_rmw(client, OV9640_MVFP, 0, OV9640_MVFP_H);
> }
> -
> - return ret;
> + return -EINVAL;
> }
>
> /* Get chip identification */
> @@ -664,8 +617,7 @@ static int ov9640_video_probe(struct soc_camera_device
> *icd, if (!icd->dev.parent ||
> to_soc_camera_host(icd->dev.parent)->nr != icd->iface) {
> dev_err(&client->dev, "Parent missing or invalid!\n");
> - ret = -ENODEV;
> - goto err;
> + return -ENODEV;
> }
>
> /*
> @@ -673,20 +625,14 @@ static int ov9640_video_probe(struct
> soc_camera_device *icd, */
>
> ret = ov9640_reg_read(client, OV9640_PID, &pid);
> + if (!ret)
> + ret = ov9640_reg_read(client, OV9640_VER, &ver);
> + if (!ret)
> + ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
> + if (!ret)
> + ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
> if (ret)
> - goto err;
> -
> - ret = ov9640_reg_read(client, OV9640_VER, &ver);
> - if (ret)
> - goto err;
> -
> - ret = ov9640_reg_re
Re: [RFC] [PATCH 3/6] SoC Camera: add driver for OV6650 sensor
Dne Ne 22. srpna 2010 22:30:10 Guennadi Liakhovetski napsal(a):
> On Sun, 22 Aug 2010, Janusz Krzysztofik wrote:
> > Hi Guennadi,
> > Thanks for your review time.
> >
> > Sunday 22 August 2010 18:40:13 Guennadi Liakhovetski wrote:
> > > On Sun, 18 Jul 2010, Janusz Krzysztofik wrote:
> > > > This patch provides a V4L2 SoC Camera driver for OV6650 camera
> > > > sensor, found on OMAP1 SoC based Amstrad Delta videophone.
> > >
> > > Have you also had a look at drivers/media/video/gspca/sonixb.c - in
> > > also supports ov6650 among other sensors.
> >
> > Yes, I have, but given up for now since:
> > 1. the gspca seems using the sensor in "Bayer 8 BGGR" mode only, which I
> > was
> >
> >not even able to select using mplayer to test my drivers with,
> >
> > 2. not all settings used there are clear for me, so I've decided to
> > revisit
> >
> >them later, when I first get a stable, even if not perfect, working
> >driver version accepted, instead of following them blindly.
>
> But good that you've looked at it.
>
> > > > + unsigned char data[2] = { reg, val };
> > > > + struct i2c_msg msg = {
> > > > + .addr = client->addr,
> > > > + .flags = 0,
> > > > + .len= 2,
> > > > + .buf= data,
> > > > + };
> > > > +
> > > > + ret = i2c_transfer(client->adapter, &msg, 1);
> > > > + if (ret < 0) {
> > > > + dev_err(&client->dev, "Failed writing register 0x%02x!
\n", reg);
> > > > + return ret;
> > > > + }
> > > > + msleep(1);
> > >
> > > Hm, interesting... Is this really needed?
> >
> > If you mean msleep(1) - yes, the sensor didn't work correctly for me
> > without that msleep().
>
> Yes, I meant msleep(1).
Doesn't surprise me at all actually. Check how this is done on ov9640 and also
NOTE I had to use gpio-spi module instead of pxa-spi to get the communication
running at all ... might be your case.
Cheers
>
> > I you mean reading the register back - I've not tried without, will do.
>
> Ok.
>
> > > > + case V4L2_CID_HUE_AUTO:
> > > > + if (ctrl->value) {
> > > > + ret = ov6650_reg_rmw(client, REG_HUE,
> > > > + SET_HUE(DEF_HUE),
SET_HUE(HUE_MASK));
> > > > + if (ret)
> > > > + break;
> > > > + priv->hue = DEF_HUE;
> > > > + } else {
> > > > + ret = ov6650_reg_rmw(client, REG_HUE, HUE_EN,
0);
> > > > + }
> > > > + if (ret)
> > > > + break;
> > > > + priv->hue_auto = ctrl->value;
> > >
> > > Hm, sorry, don't understand. If the user sets auto-hue to ON, you set
> > > the hue enable bit and hue value to default.
> >
> > No, I reset the hue enable bit here, which I understand is used for
> > applying a non-default hue value if set rather than enabling auto hue.
> > Maybe my understanding of this bit function is wrong.
> >
> > > If the user sets auto-hue to OFF,
> > > you just set the hue enable bit on and don't change the value. Does
> > > ov6650 actually support auto-hue?
> >
> > All I was able to find out was the HUE register bits described like this:
> >
> > Bit[7:6]: Reserved
> > Bit[5]: Hue Enable
> > Bit[4:0]: Hue setting
> >
> > and the register default value: 0x10.
> >
> > What do you think the bit[5] meaning is?
>
> Well, from how I interpret, what you say, I think, there is no auto-hue
> implemented by this sensor, at least, not by this register. Maybe drop
> auto-hue support completely? It seems to me just a manual hue value can be
> set.
>
> > > > +/* select nearest higher resolution for capture */
> > > > +static void ov6650_res_roundup(u32 *width, u32 *height)
> > > > +{
> > > > + int i;
> > > > + enum { QCIF, CIF };
> > > > + int res_x[] = { 176, 352 };
> > > > + int res_y[] = { 144, 288 };
> > > > +
> > > > + for (i = 0; i < ARRAY_SIZE(res_x); i++) {
> > > > + if (res_x[i] >= *width && res_y[i] >= *height) {
> > > > + *width = res_x[i];
> > > > + *height = res_y[i];
> > > > + return;
> > > > + }
> > > > + }
> > > > +
> > > > + *width = res_x[CIF];
> > > > + *height = res_y[CIF];
> > > > +}
> > >
> > > This can be replaced by a version of
> > >
> > > http://www.spinics.net/lists/linux-media/msg21893.html
> > >
> > > when it is fixed and accepted;) I'll try to send an updated version of
> > > that patch tomorrow.
> >
> > Fine, I'll use this instead of my dirty workarounds.
>
> /me has to update that patch... Will try to do that asap.
>
> > > > +
> > > > +/* program default register values */
> > > > +static int ov6650_prog_dflt(struct i2c_client *client)
> > > > +{
> > > > + int i, ret;
> > > > +
> > > > + dev_dbg(&client->dev, "rein
Re: V4L2: Add a v4l2-subdev (soc-camera) driver for OmniVision OV9640 sensor
Dne Út 15. září 2009 21:55:32 Guennadi Liakhovetski napsal(a): > On Tue, 15 Sep 2009, Marek Vasut wrote: > > Just briefly skimmed over it. Ok then, that diff seems fine. I assume the > > imagebus will fix the rgb issues anyway. > > Sorry, have to ask to make quite sure - so, you're ok with me pushing that > patch as it was in the mail - not just the diff but also the patch header? > And no, imagebus cannot fix the problem automagically - until we know for > sure what those formats are. So, someone will have to test the driver > again. Yeah, I'll eventually fix it if you broke something. btw. I know what those formats are, I was able to get both YUV and RGB encoded data from it. > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- 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: V4L2: Add a v4l2-subdev (soc-camera) driver for OmniVision OV9640 sensor
Dne Po 14. září 2009 23:21:33 Guennadi Liakhovetski napsal(a): > On Mon, 14 Sep 2009, Marek Vasut wrote: > > Dne Po 14. září 2009 22:30:41 Guennadi Liakhovetski napsal(a): > > > On Mon, 14 Sep 2009, Marek Vasut wrote: > > > > Dne Po 14. září 2009 21:29:26 Guennadi Liakhovetski napsal(a): > > > > > From: Marek Vasut > > > > > > > > > > Signed-off-by: Marek Vasut > > > > > Signed-off-by: Guennadi Liakhovetski > > > > > --- > > > > > > > > > > Marek, please confirm, that this version is ok. I'll push it > > > > > upstream for 2.6.32 then. > > > > > > > > No, it's not OK. You removed the RGB part. Either enclose those parts > > > > into ifdef OV9640_RGB_BUGGY or preserve it in some other way. Someone > > > > will certainly want to re-add RGB parts later and will have to figure > > > > it out all over again. > > > > > > Ok, make a proposal, how you would like to see it. But - I do not want > > > commented out code, including "#ifdef MACRO_THAT_DOESNT_GET_DEFINED." I > > > think, I described it in sufficient detail, so that re-adding that code > > > should not take longer than 10 minutes for anyone sufficiently familiar > > > with the code. Referencing another driver also has an advantage, that > > > if we switch to imagebus or any other API, you don't get stale > > > commented out code, but you look up updated code in a functional > > > driver. But I am open to your ideas / but no commented out code, > > > please. > > > > The RGB is broken only in a way where it swaps colours, the color matrix > > coeficients and register configurations are OK (which is what other > > people who will want to add it will need to figure out again from scratch > > if you remove the code). > > Excuse me, have you looked at my patch? Have you compared it to yours? I > only removed your RGB code entries from the list of supported formats, I > haven't removed any implementation details, thus effectively just > disabling it. Just briefly skimmed over it. Ok then, that diff seems fine. I assume the imagebus will fix the rgb issues anyway. > > > I dont want this merged before this is solved in some way where those > > values are preserved. > > Sure, let's have it fixed and submit it in time for 2.6.33. > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- 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: V4L2: Add a v4l2-subdev (soc-camera) driver for OmniVision OV9640 sensor
Dne Po 14. září 2009 22:30:41 Guennadi Liakhovetski napsal(a): > On Mon, 14 Sep 2009, Marek Vasut wrote: > > Dne Po 14. září 2009 21:29:26 Guennadi Liakhovetski napsal(a): > > > From: Marek Vasut > > > > > > Signed-off-by: Marek Vasut > > > Signed-off-by: Guennadi Liakhovetski > > > --- > > > > > > Marek, please confirm, that this version is ok. I'll push it upstream > > > for 2.6.32 then. > > > > No, it's not OK. You removed the RGB part. Either enclose those parts > > into ifdef OV9640_RGB_BUGGY or preserve it in some other way. Someone > > will certainly want to re-add RGB parts later and will have to figure it > > out all over again. > > Ok, make a proposal, how you would like to see it. But - I do not want > commented out code, including "#ifdef MACRO_THAT_DOESNT_GET_DEFINED." I > think, I described it in sufficient detail, so that re-adding that code > should not take longer than 10 minutes for anyone sufficiently familiar > with the code. Referencing another driver also has an advantage, that if > we switch to imagebus or any other API, you don't get stale commented out > code, but you look up updated code in a functional driver. But I am open > to your ideas / but no commented out code, please. The RGB is broken only in a way where it swaps colours, the color matrix coeficients and register configurations are OK (which is what other people who will want to add it will need to figure out again from scratch if you remove the code). I dont want this merged before this is solved in some way where those values are preserved. > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- 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: V4L2: Add a v4l2-subdev (soc-camera) driver for OmniVision OV9640 sensor
Dne Po 14. září 2009 21:29:26 Guennadi Liakhovetski napsal(a):
> From: Marek Vasut
>
> Signed-off-by: Marek Vasut
> Signed-off-by: Guennadi Liakhovetski
> ---
>
> Marek, please confirm, that this version is ok. I'll push it upstream for
> 2.6.32 then.
No, it's not OK. You removed the RGB part. Either enclose those parts into
ifdef
OV9640_RGB_BUGGY or preserve it in some other way. Someone will certainly want
to re-add RGB parts later and will have to figure it out all over again.
>
> drivers/media/video/Kconfig |6 +
> drivers/media/video/Makefile|1 +
> drivers/media/video/ov9640.c| 805
> +++ drivers/media/video/ov9640.h|
> 209 ++
> include/media/v4l2-chip-ident.h |1 +
> 5 files changed, 1022 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/ov9640.c
> create mode 100644 drivers/media/video/ov9640.h
>
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 14a1b61..498a223 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -843,6 +843,12 @@ config SOC_CAMERA_OV772X
> help
> This is a ov772x camera driver
>
> +config SOC_CAMERA_OV9640
> + tristate "ov9640 camera support"
> + depends on SOC_CAMERA && I2C
> + help
> + This is a ov9640 camera driver
> +
> config MX1_VIDEO
> bool
>
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index 00fb23e..541e7f3 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111)+= mt9m111.o
> obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o
> obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o
> obj-$(CONFIG_SOC_CAMERA_OV772X) += ov772x.o
> +obj-$(CONFIG_SOC_CAMERA_OV9640) += ov9640.o
> obj-$(CONFIG_SOC_CAMERA_TW9910) += tw9910.o
>
> # And now the v4l2 drivers:
> diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
> new file mode 100644
> index 000..f7826b9
> --- /dev/null
> +++ b/drivers/media/video/ov9640.c
> @@ -0,0 +1,805 @@
> +/*
> + * OmniVision OV96xx Camera Driver
> + *
> + * Copyright (C) 2009 Marek Vasut
> + *
> + * Based on ov772x camera driver:
> + *
> + * Copyright (C) 2008 Renesas Solutions Corp.
> + * Kuninori Morimoto
> + *
> + * Based on ov7670 and soc_camera_platform driver,
> + *
> + * Copyright 2006-7 Jonathan Corbet
> + * Copyright (C) 2008 Magnus Damm
> + * Copyright (C) 2008, Guennadi Liakhovetski
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "ov9640.h"
> +
> +/* default register setup */
> +static const struct ov9640_reg ov9640_regs_dflt[] = {
> + { OV9640_COM5, OV9640_COM5_SYSCLK | OV9640_COM5_LONGEXP },
> + { OV9640_COM6, OV9640_COM6_OPT_BLC | OV9640_COM6_ADBLC_BIAS |
> + OV9640_COM6_FMT_RST | OV9640_COM6_ADBLC_OPTEN },
> + { OV9640_PSHFT, OV9640_PSHFT_VAL(0x01) },
> + { OV9640_ACOM, OV9640_ACOM_2X_ANALOG | OV9640_ACOM_RSVD },
> + { OV9640_TSLB, OV9640_TSLB_YUYV_UYVY },
> + { OV9640_COM16, OV9640_COM16_RB_AVG },
> +
> + /* Gamma curve P */
> + { 0x6c, 0x40 }, { 0x6d, 0x30 }, { 0x6e, 0x4b }, { 0x6f, 0x60 },
> + { 0x70, 0x70 }, { 0x71, 0x70 }, { 0x72, 0x70 }, { 0x73, 0x70 },
> + { 0x74, 0x60 }, { 0x75, 0x60 }, { 0x76, 0x50 }, { 0x77, 0x48 },
> + { 0x78, 0x3a }, { 0x79, 0x2e }, { 0x7a, 0x28 }, { 0x7b, 0x22 },
> +
> + /* Gamma curve T */
> + { 0x7c, 0x04 }, { 0x7d, 0x07 }, { 0x7e, 0x10 }, { 0x7f, 0x28 },
> + { 0x80, 0x36 }, { 0x81, 0x44 }, { 0x82, 0x52 }, { 0x83, 0x60 },
> + { 0x84, 0x6c }, { 0x85, 0x78 }, { 0x86, 0x8c }, { 0x87, 0x9e },
> + { 0x88, 0xbb }, { 0x89, 0xd2 }, { 0x8a, 0xe6 },
> +};
> +
> +/* Configurations
> + * NOTE: for YUV, alter the following registers:
> + * COM12 |= OV9640_COM12_YUV_AVG
> + *
> + *for RGB, alter the following registers:
> + * COM7 |= OV9640_COM7_RGB
> + * COM13 |= OV9640_COM13_RGB_AVG
> + * COM15 |= proper RGB color encoding mode
> + */
> +static const struct ov9640_reg ov9640_regs_qqcif[] = {
> + { OV9640_CLKRC, OV9640_CLKRC_DPLL_EN | OV9640_CLKRC_DIV(0x0f) },
> + { OV9640_COM1, OV9640_C
Re: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
Dne Po 14. září 2009 16:45:50 Guennadi Liakhovetski napsal(a): > Ok, you were faster than I:-) If you agree, I can just remove those two > RGB formats myself, changing your comment to a TODO, We can #ifdef them so others dont have to re-add them by hand if they want to try fixing those. > and modify the > comment next to msleep(150) 140 didn't > (if you could tell me what value didn't work, > that would be appreciated) and push it out. > > Thanks > Guennadi Cheers > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- 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 2/3] Add driver for OmniVision OV9640 sensor
Dne Po 14. září 2009 16:35:24 Marek Vasut napsal(a): > Dne Ne 13. září 2009 21:13:32 Guennadi Liakhovetski napsal(a): > > On Sun, 13 Sep 2009, Marek Vasut wrote: > > > Dne Pá 11. záÅà 2009 23:51:44 Guennadi Liakhovetski napsal(a): > > > > Ok, a couple more simple questions / remarks, In principle, there's > > > > just one principle objection - if we agree, that the correct format > > > > is BGR565 and RGB565X, then we should change that. There's no BGR565 > > > > format currently in the kernel, so, we'd have to add it (and the > > > > documentation for the mercurial tree). > > > > > > I dont think I understand you at all. > > > > Your patch used RGB565X, which is the same as RGB565 but with _bytes_ > > swapped, whereas earlier you confirmed, that it's not a byte-swapped > > RGB565 but rather R and B _colours_ are swapped: > > > > http://marc.info/?l=linux-arm-kernel&m=125220918005429&w=2 > > > > , i.e., it is a BGR565. Now it looks like you don't change anything in > > your RGB processing code, but you declare it as plain RGB555 and RGB565 > > codes. So, are these really the normal unswapped formats or am I missing > > something? And you replaced VYUY with UYVY while also modifying register > > configuration, so, I hope, this has settled now and your current > > configuration works properly with the unmodified pxa270 for you, right? > > > > Oh, damn, I see now, I put my signature above the patch, so, you didn't > > look below it, and there were a couple more comments there:-( Sorry! All > > of them should be pretty easy to fix, so, please have a look at them. > > > > > Inlined is a new version of the patch (I did some lookup through the > > > datasheet). I might not need the BGR formats for now. > > > > > > btw. weren't you planning to merge the ov96xx drivers into .31? I > > > havent seen any of them there. > > > > Which ov96xx drivers? Do you mean the ov9655 driver from Stefan > > Herbrechtsmeier? My doubt with the latter one was (and still is), that we > > already have two drivers in the mainline (gspca/sn9c20x.c and > > gspca/ov534.c) that seem to claim support for that chip, so, I wanted to > > see, if we could reuse them. There's also an ov7670 from Jonathan Cameron > > with the same issue. And for that merge we have to come closer to > > v4l2-subdev, which is happening just now. > > > > Thanks > > Guennadi > > --- > > Guennadi Liakhovetski, Ph.D. > > Freelance Open-Source Software Developer > > http://www.open-technology.de/ > > Here's a new patch ... YUV works, RGB might be still broken (but that can > be fixed later when imagebus arrives). Tab fixed... From: Marek Vasut Date: Sat, 22 Aug 2009 05:14:23 +0200 Subject: [PATCH] Add driver for OmniVision OV9640 sensor Signed-off-by: Marek Vasut --- drivers/media/video/Kconfig |6 + drivers/media/video/Makefile|1 + drivers/media/video/ov9640.c| 812 +++ drivers/media/video/ov9640.h| 209 ++ include/media/v4l2-chip-ident.h |1 + 5 files changed, 1029 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov9640.c create mode 100644 drivers/media/video/ov9640.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 84b6fc1..fddd654 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X help This is a ov772x camera driver +config SOC_CAMERA_OV9640 + tristate "ov9640 camera support" + depends on SOC_CAMERA && I2C + help + This is a ov9640 camera driver + config MX1_VIDEO bool diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index 9f2e321..e18efd5 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o obj-$(CONFIG_SOC_CAMERA_OV772X) += ov772x.o +obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o # And now the v4l2 drivers: diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c new file mode 100644 index 000..a651d2b --- /dev/null +++ b/drivers/media/video/ov9640.c @@ -0,0 +1,812 @@ +/* + * OmniVision OV96xx Camera Driver + * + * Copyright (C) 2009 Marek Vasut + * + * Based on ov772x camera driver: + * + * Copyright (C) 2008 Renesas Solutions Corp. + * Kuninori Morimoto + *
Re: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
Dne Ne 13. září 2009 21:13:32 Guennadi Liakhovetski napsal(a): > On Sun, 13 Sep 2009, Marek Vasut wrote: > > Dne Pá 11. záÅà 2009 23:51:44 Guennadi Liakhovetski napsal(a): > > > Ok, a couple more simple questions / remarks, In principle, there's > > > just one principle objection - if we agree, that the correct format is > > > BGR565 and RGB565X, then we should change that. There's no BGR565 > > > format currently in the kernel, so, we'd have to add it (and the > > > documentation for the mercurial tree). > > > > I dont think I understand you at all. > > Your patch used RGB565X, which is the same as RGB565 but with _bytes_ > swapped, whereas earlier you confirmed, that it's not a byte-swapped > RGB565 but rather R and B _colours_ are swapped: > > http://marc.info/?l=linux-arm-kernel&m=125220918005429&w=2 > > , i.e., it is a BGR565. Now it looks like you don't change anything in > your RGB processing code, but you declare it as plain RGB555 and RGB565 > codes. So, are these really the normal unswapped formats or am I missing > something? And you replaced VYUY with UYVY while also modifying register > configuration, so, I hope, this has settled now and your current > configuration works properly with the unmodified pxa270 for you, right? > > Oh, damn, I see now, I put my signature above the patch, so, you didn't > look below it, and there were a couple more comments there:-( Sorry! All > of them should be pretty easy to fix, so, please have a look at them. > > > Inlined is a new version of the patch (I did some lookup through the > > datasheet). I might not need the BGR formats for now. > > > > btw. weren't you planning to merge the ov96xx drivers into .31? I havent > > seen any of them there. > > Which ov96xx drivers? Do you mean the ov9655 driver from Stefan > Herbrechtsmeier? My doubt with the latter one was (and still is), that we > already have two drivers in the mainline (gspca/sn9c20x.c and > gspca/ov534.c) that seem to claim support for that chip, so, I wanted to > see, if we could reuse them. There's also an ov7670 from Jonathan Cameron > with the same issue. And for that merge we have to come closer to > v4l2-subdev, which is happening just now. > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > Here's a new patch ... YUV works, RGB might be still broken (but that can be fixed later when imagebus arrives). From: Marek Vasut Date: Sat, 22 Aug 2009 05:14:23 +0200 Subject: [PATCH] Add driver for OmniVision OV9640 sensor Signed-off-by: Marek Vasut --- drivers/media/video/Kconfig |6 + drivers/media/video/Makefile|1 + drivers/media/video/ov9640.c| 812 +++ drivers/media/video/ov9640.h| 209 ++ include/media/v4l2-chip-ident.h |1 + 5 files changed, 1029 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov9640.c create mode 100644 drivers/media/video/ov9640.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 84b6fc1..fddd654 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X help This is a ov772x camera driver +config SOC_CAMERA_OV9640 + tristate "ov9640 camera support" + depends on SOC_CAMERA && I2C + help + This is a ov9640 camera driver + config MX1_VIDEO bool diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index 9f2e321..e18efd5 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o +obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o # And now the v4l2 drivers: diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c new file mode 100644 index 000..a651d2b --- /dev/null +++ b/drivers/media/video/ov9640.c @@ -0,0 +1,812 @@ +/* + * OmniVision OV96xx Camera Driver + * + * Copyright (C) 2009 Marek Vasut + * + * Based on ov772x camera driver: + * + * Copyright (C) 2008 Renesas Solutions Corp. + * Kuninori Morimoto + * + * Based on ov7670 and soc_camera_platform driver, + * + * Copyright 2006-7 Jonathan Corbet + * Copyright (C) 2008 Magnus Damm + * Copyright (C) 2008, Guennadi Liakhovetski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License
Re: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
Dne Pá 11. září 2009 23:51:44 Guennadi Liakhovetski napsal(a): > On Fri, 4 Sep 2009, Marek Vasut wrote: > > Dne Po 24. srpna 2009 20:06:29 Guennadi Liakhovetski napsal(a): > > > Hi Marek > > > > > > On Sat, 22 Aug 2009, Marek Vasut wrote: > > > > From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00 > > > > 2001 From: Marek Vasut > > > > Date: Sat, 22 Aug 2009 05:14:23 +0200 > > > > Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor > > > > > > > > Signed-off-by: Marek Vasut > > > > > > Ok, I see, you rebased your patches on my soc-camera patch-stack, this > > > is good, thanks. But you missed a couple of my comments - you still > > > have a few static functions in the ov9640.c marked inline: > > > ov9640_set_crop() is not used at all, ov9640_set_bus_param() gets > > > assigned to a struct member, so, cannot be inline. ov9640_alter_regs() > > > is indeed only called at one location, but see Chapter 15 in > > > Documentation/CodingStyle. You left at least one multi-line comment > > > wrongly formatted. You left some broken printk format lines, etc. You > > > moved your header under drivers/... - that's good. But, please, address > > > all my comments that I provided in a private review, or explain why you > > > do not agree. Otherwise I feel like I wasted my time. Besides, your > > > mailer has wrapped lines. Please, read > > > Documentation/email-clients.txt on how to configure your email client > > > to send patches properly. In the worst case, you can inline your > > > patches while reviewing, and then attach them for a final submission. > > > This is, however, discouraged, because it makes review and test of your > > > intermediate patches with wrapped lines more difficult. Also, please > > > check your patches with scripts/checkpatch.pl. > > > > Fixed patch follows, my mailer is fixed as much as possible (working on > > getting git-email to work, but that's still to be done). Please consider > > applying, thanks. > > Ok, a couple more simple questions / remarks, In principle, there's just > one principle objection - if we agree, that the correct format is BGR565 > and RGB565X, then we should change that. There's no BGR565 format > currently in the kernel, so, we'd have to add it (and the documentation > for the mercurial tree). > > Thanks > Guennadi > > > Add driver for OmniVision OV9640 sensor > > > > Signed-off-by: Marek Vasut > > --- > > drivers/media/video/Kconfig |6 + > > drivers/media/video/Makefile|1 + > > drivers/media/video/ov9640.c| 811 > > +++ drivers/media/video/ov9640.h| > > 206 ++ > > include/media/v4l2-chip-ident.h |1 + > > 5 files changed, 1025 insertions(+), 0 deletions(-) > > create mode 100644 drivers/media/video/ov9640.c > > create mode 100644 drivers/media/video/ov9640.h > > > > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig > > index 84b6fc1..fddd654 100644 > > --- a/drivers/media/video/Kconfig > > +++ b/drivers/media/video/Kconfig > > @@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X > > help > > This is a ov772x camera driver > > > > +config SOC_CAMERA_OV9640 > > + tristate "ov9640 camera support" > > + depends on SOC_CAMERA && I2C > > + help > > + This is a ov9640 camera driver > > + > > config MX1_VIDEO > > bool > > > > diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile > > index 9f2e321..e18efd5 100644 > > --- a/drivers/media/video/Makefile > > +++ b/drivers/media/video/Makefile > > @@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o > > obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o > > obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o > > obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o > > +obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o > > obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o > > > > # And now the v4l2 drivers: > > diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c > > new file mode 100644 > > index 000..289e82d > > --- /dev/null > > +++ b/drivers/media/video/ov9640.c > > @@ -0,0 +1,811 @@ > > +/* > > + * OmniVision OV96xx Camera Driver > > + * > > + * Copyright (C) 2009 Marek Vasut > > + * > > + * Based on ov772x camera driv
Re: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
Dne Pá 11. září 2009 23:51:44 Guennadi Liakhovetski napsal(a): > On Fri, 4 Sep 2009, Marek Vasut wrote: > > Dne Po 24. srpna 2009 20:06:29 Guennadi Liakhovetski napsal(a): > > > Hi Marek > > > > > > On Sat, 22 Aug 2009, Marek Vasut wrote: > > > > From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00 > > > > 2001 From: Marek Vasut > > > > Date: Sat, 22 Aug 2009 05:14:23 +0200 > > > > Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor > > > > > > > > Signed-off-by: Marek Vasut > > > > > > Ok, I see, you rebased your patches on my soc-camera patch-stack, this > > > is good, thanks. But you missed a couple of my comments - you still > > > have a few static functions in the ov9640.c marked inline: > > > ov9640_set_crop() is not used at all, ov9640_set_bus_param() gets > > > assigned to a struct member, so, cannot be inline. ov9640_alter_regs() > > > is indeed only called at one location, but see Chapter 15 in > > > Documentation/CodingStyle. You left at least one multi-line comment > > > wrongly formatted. You left some broken printk format lines, etc. You > > > moved your header under drivers/... - that's good. But, please, address > > > all my comments that I provided in a private review, or explain why you > > > do not agree. Otherwise I feel like I wasted my time. Besides, your > > > mailer has wrapped lines. Please, read > > > Documentation/email-clients.txt on how to configure your email client > > > to send patches properly. In the worst case, you can inline your > > > patches while reviewing, and then attach them for a final submission. > > > This is, however, discouraged, because it makes review and test of your > > > intermediate patches with wrapped lines more difficult. Also, please > > > check your patches with scripts/checkpatch.pl. > > > > Fixed patch follows, my mailer is fixed as much as possible (working on > > getting git-email to work, but that's still to be done). Please consider > > applying, thanks. > > Ok, a couple more simple questions / remarks, In principle, there's just > one principle objection - if we agree, that the correct format is BGR565 > and RGB565X, then we should change that. There's no BGR565 format > currently in the kernel, so, we'd have to add it (and the documentation > for the mercurial tree). I dont think I understand you at all. > > Thanks > Guennadi Inlined is a new version of the patch (I did some lookup through the datasheet). I might not need the BGR formats for now. btw. weren't you planning to merge the ov96xx drivers into .31? I havent seen any of them there. Cheers! From: Marek Vasut Date: Sat, 22 Aug 2009 05:14:23 +0200 Subject: [PATCH] Add driver for OmniVision OV9640 sensor Signed-off-by: Marek Vasut --- drivers/media/video/Kconfig |6 + drivers/media/video/Makefile|1 + drivers/media/video/ov9640.c| 818 +++ drivers/media/video/ov9640.h| 209 ++ include/media/v4l2-chip-ident.h |1 + 5 files changed, 1035 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov9640.c create mode 100644 drivers/media/video/ov9640.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 84b6fc1..fddd654 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X help This is a ov772x camera driver +config SOC_CAMERA_OV9640 + tristate "ov9640 camera support" + depends on SOC_CAMERA && I2C + help + This is a ov9640 camera driver + config MX1_VIDEO bool diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index 9f2e321..e18efd5 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o +obj-$(CONFIG_SOC_CAMERA_OV9640) += ov9640.o obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o # And now the v4l2 drivers: diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c new file mode 100644 index 000..0701247 --- /dev/null +++ b/drivers/media/video/ov9640.c @@ -0,0 +1,818 @@ +/* + * OmniVision OV96xx Camera Driver + * + * Copyright (C) 2009 Marek Vasut + * + * Based on ov772x camera driver: + * + * Copyright (C) 2008 Renesas Solutions Corp. + * Kuninori Morimoto + * + * Based on ov7670 and soc_camera_platform driver, + * + * Cop
Re: [PATCH] Add RGB555X and RGB565X formats to pxa-camera
Dne Po 7. září 2009 12:21:50 Robert Jarzmik napsal(a):
> Marek Vasut writes:
> > How's it supposed to get BGR555 if the pxa-camera doesnt support that ?
> > Will the v4l2 layer convert it or something ?
>
> In pxa_camera.c, function pxa_camera_get_formats() :
> > default:
> > /* Generic pass-through */
> > formats++;
> > if (xlate) {
> > xlate->host_fmt = icd->formats + idx;
> > xlate->cam_fmt = icd->formats + idx;
> > xlate->buswidth = icd->formats[idx].depth;
> > xlate++;
> > dev_dbg(ici->dev,
> > "Providing format %s in pass-through
> > mode\n", icd->formats[idx].name);
> > }
> > }
>
> "Pass-through" means that if a sensors provides a cc, ie. BGR555 for
> example, the bridge (pxa_camera) will "forward" to RAM the image in the
> very same cc (ie. BGR555). In that case, the bridge is a dummy "sensor to
> RAM" bus translator if you prefer.
>
> Marek, you should activate debug trace and watch for yourself. You can
> trust Guennadi, when he says it will work, well ... it will work.
>
> If it's out of technical curiousity, check the function above. If you're
> even more curious, there was a thread in linux-media about "RFC: bus
> configuration setup for sub-devices", a very interesting one, especially
> considering the "pass-through" issue.
That one should work for RGB565X ? (the piece of code you posted ?) It's
interesting it didnt work for me ... ok, I'll take it it works then, whatever.
I'll test it again when I have time.
>
> Cheers.
>
> --
> Robert
--
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] Add RGB555X and RGB565X formats to pxa-camera
Dne Po 7. září 2009 08:22:22 Guennadi Liakhovetski napsal(a): > On Mon, 7 Sep 2009, Marek Vasut wrote: > > Dne Ne 6. záÅà 2009 20:15:17 Guennadi Liakhovetski napsal(a): > > > On Sun, 6 Sep 2009, Marek Vasut wrote: > > > > Dne Ne 6. záÅà 2009 18:52:55 Guennadi Liakhovetski napsal(a): > > > > > On Sun, 6 Sep 2009, Marek Vasut wrote: > > > > > > Ah damn, I see what you mean. What the camera does is it swaps > > > > > > the RED and BLUE channel: > > > > > > 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00 > > > > > > B4 B3 B2 B1 B0 G4 G3 G2 G1 G1 R4 R3 R2 R1 R1 -- > > > > > > so it's more a BGR555/565 then. I had to patch fswebcam for this. > > > > > > > > > > Ok, this is, of course, something different. In this case you, > > > > > probably, could deceive the PXA to handle blue as red and the other > > > > > way round, but still, I would prefer not to do that. Hence my > > > > > suggestion remains - pass these formats as raw data. > > > > > > > > Which is bogus from the camera point of view. > > > > > > Not at all. This just means: the subdevice provides a pixel format, > > > that the bridge (PXA) knows nothing specific about, but it can just > > > pass it one-to-one (as raw data) to the user - don't see anything bogus > > > in this. Different bridges have support for different pixel colour > > > formats, but, I think, all bridges can pass data as raw (pass-through). > > > Some bridges can _only_ do this, so, this is actually the default > > > video-capture mode. > > > > But then you'll have to tell your software how to process the raw data > > (in what format they are). If there was this RGB565X passthrough support, > > the software could at least check if you are not forcing it to process > > nonsense. > > There's no difference for user-space software. It requests BGR555 it gets > BGR555 back, because that's the information the pxa driver will find in > this format descriptor: if you take this data and put it in RAM in a > certain way, you get BGR555. How's it supposed to get BGR555 if the pxa-camera doesnt support that ? Will the v4l2 layer convert it or something ? > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ -- 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] Add RGB555X and RGB565X formats to pxa-camera
Dne Ne 6. září 2009 20:15:17 Guennadi Liakhovetski napsal(a): > On Sun, 6 Sep 2009, Marek Vasut wrote: > > Dne Ne 6. září 2009 18:52:55 Guennadi Liakhovetski napsal(a): > > > On Sun, 6 Sep 2009, Marek Vasut wrote: > > > > Ah damn, I see what you mean. What the camera does is it swaps the > > > > RED and BLUE channel: > > > > 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00 > > > > B4 B3 B2 B1 B0 G4 G3 G2 G1 G1 R4 R3 R2 R1 R1 -- > > > > so it's more a BGR555/565 then. I had to patch fswebcam for this. > > > > > > Ok, this is, of course, something different. In this case you, > > > probably, could deceive the PXA to handle blue as red and the other way > > > round, but still, I would prefer not to do that. Hence my suggestion > > > remains - pass these formats as raw data. > > > > Which is bogus from the camera point of view. > > Not at all. This just means: the subdevice provides a pixel format, that > the bridge (PXA) knows nothing specific about, but it can just pass it > one-to-one (as raw data) to the user - don't see anything bogus in this. > Different bridges have support for different pixel colour formats, but, I > think, all bridges can pass data as raw (pass-through). Some bridges can > _only_ do this, so, this is actually the default video-capture mode. But then you'll have to tell your software how to process the raw data (in what format they are). If there was this RGB565X passthrough support, the software could at least check if you are not forcing it to process nonsense. > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ -- 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] Add RGB555X and RGB565X formats to pxa-camera
Dne Ne 6. září 2009 18:52:55 Guennadi Liakhovetski napsal(a): > On Sun, 6 Sep 2009, Marek Vasut wrote: > > Ah damn, I see what you mean. What the camera does is it swaps the RED > > and BLUE channel: > > 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00 > > B4 B3 B2 B1 B0 G4 G3 G2 G1 G1 R4 R3 R2 R1 R1 -- > > so it's more a BGR555/565 then. I had to patch fswebcam for this. > > Ok, this is, of course, something different. In this case you, probably, > could deceive the PXA to handle blue as red and the other way round, but > still, I would prefer not to do that. Hence my suggestion remains - pass > these formats as raw data. > Which is bogus from the camera point of view. > The only case when you might want to put the PXA into RGB555 mode, while > feeding BGR555 to it, is you want to use the QCI to set the transparency > bit for you. But we currently do not support this any way, not in a > configurable way at least. You would need to implement some sort of a > "global (one-bit) alpha" control for pxa_camera to use this. Any need for > this? > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ -- 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] Add RGB555X and RGB565X formats to pxa-camera
Dne Ne 6. září 2009 00:05:14 Guennadi Liakhovetski napsal(a): > On Sat, 5 Sep 2009, Marek Vasut wrote: > > Dne So 5. záÅà 2009 22:19:42 Guennadi Liakhovetski napsal(a): > > > On Sat, 5 Sep 2009, Marek Vasut wrote: > > > > Dne So 5. záÅà 2009 10:55:55 Guennadi Liakhovetski napsal(a): > > > > > Marek, please, look in PXA270 datasheet. To support a specific > > > > > pixel format means, e.g., to be able to process it further, > > > > > according to this format's particular colour component ordering. > > > > > Process further can mean convert to another format, extract various > > > > > information from the data (statistics, etc.)... Now RGB555 looks > > > > > like (from wikipedia) > > > > > > > > > > 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00 > > > > > R4 R3 R2 R1 R0 G4 G3 G2 G1 G1 B4 B3 B2 B1 B1 -- > > > > > > > > > > (Actually, I thought bit 15 was unused, but it doesn't matter for > > > > > this discussion.) Now, imagine what happens if you swap the two > > > > > bytes. I don't think the PXA will still be able to meaningfully > > > > > process that format. > > > > > > > > Not on the pxa side, but on the camera side -- Bs and Rs swapped in > > > > the diagram above. > > > > > > And then? Are you trying to tell me, that the PXA then swaps them > > > back?... > > > > No, the software has to do it then, I'm trying to tell you that it has > > nothing to do with PXA (as PXA really doesnt care if the channel is > > actually blue or red). > > Of course it does. I asked you to swap the above two bytes, you would get > this: > > 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00 > G1 G0 B4 B3 B2 B1 B0 -- R4 R3 R2 R1 R0 G4 G3 G2 > > and PXA would still inerpret this as > > R4 R3 R2 R1 R0 G4 G3 G2 G1 G0 B4 B3 B2 B1 B0 -- > > i.e., it would take bits > > R2 R1 R0 G4 G3 > > for blue, bits > > B1 B0 -- R4 R3 > > for green, and bits > > G1 G0 B4 B3 B2 > > as red. Which, as you see, makes no sense. That's why I'm saying, that it > doesn't support this format, and we can only pass it through as raw data. Ah damn, I see what you mean. What the camera does is it swaps the RED and BLUE channel: 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00 B4 B3 B2 B1 B0 G4 G3 G2 G1 G1 R4 R3 R2 R1 R1 -- so it's more a BGR555/565 then. I had to patch fswebcam for this. Sorry for the confusion. > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ -- 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] Add RGB555X and RGB565X formats to pxa-camera
Dne So 5. září 2009 22:19:42 Guennadi Liakhovetski napsal(a):
> On Sat, 5 Sep 2009, Marek Vasut wrote:
> > Dne So 5. září 2009 10:55:55 Guennadi Liakhovetski napsal(a):
> > > On Sat, 5 Sep 2009, Marek Vasut wrote:
> > > > > > > drivers/media/video/pxa_camera.c |4
> > > > > > > 1 files changed, 4 insertions(+), 0 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/media/video/pxa_camera.c
> > > > > > > b/drivers/media/video/pxa_camera.c
> > > > > > > index 46e0d8a..de0fc8a 100644
> > > > > > > --- a/drivers/media/video/pxa_camera.c
> > > > > > > +++ b/drivers/media/video/pxa_camera.c
> > > > > > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > > > > > soc_camera_data_format *fmt)
> > > > > > > case V4L2_PIX_FMT_YVYU:
> > > > > > > case V4L2_PIX_FMT_RGB565:
> > > > > > > case V4L2_PIX_FMT_RGB555:
> > > > > > > + case V4L2_PIX_FMT_RGB565X:
> > > > > > > + case V4L2_PIX_FMT_RGB555X:
> > > > > > > return 8;
> > > > > > > default:
> > > > > > > return fmt->depth;
> > > > > > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > > > > > soc_camera_device *icd, int idx,
> > > > > > > case V4L2_PIX_FMT_YVYU:
> > > > > > > case V4L2_PIX_FMT_RGB565:
> > > > > > > case V4L2_PIX_FMT_RGB555:
> > > > > > > + case V4L2_PIX_FMT_RGB565X:
> > > > > > > + case V4L2_PIX_FMT_RGB555X:
> > > > > > > formats++;
> > > > > > > if (xlate) {
> > > > > > > xlate->host_fmt = icd->formats + idx;
> > > >
> > > > What should we do with this patch? Any updates? I spoke to Guennadi
> > > > and he thinks it's not a good idea to apply it (as pxaqci doesnt
> > > > support those formats). But to my understanding, those formats are
> > > > endian-swapped versions of the other ones without X at the end so
> > > > there shouldnt be a problem with it.
> > >
> > > Marek, please, look in PXA270 datasheet. To support a specific pixel
> > > format means, e.g., to be able to process it further, according to this
> > > format's particular colour component ordering. Process further can mean
> > > convert to another format, extract various information from the data
> > > (statistics, etc.)... Now RGB555 looks like (from wikipedia)
> > >
> > > 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00
> > > R4 R3 R2 R1 R0 G4 G3 G2 G1 G1 B4 B3 B2 B1 B1 --
> > >
> > > (Actually, I thought bit 15 was unused, but it doesn't matter for this
> > > discussion.) Now, imagine what happens if you swap the two bytes. I
> > > don't think the PXA will still be able to meaningfully process that
> > > format.
> >
> > Not on the pxa side, but on the camera side -- Bs and Rs swapped in the
> > diagram above.
>
> And then? Are you trying to tell me, that the PXA then swaps them back?...
No, the software has to do it then, I'm trying to tell you that it has nothing
to do with PXA (as PXA really doesnt care if the channel is actually blue or
red).
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
--
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] Add RGB555X and RGB565X formats to pxa-camera
Dne So 5. září 2009 10:55:55 Guennadi Liakhovetski napsal(a):
> On Sat, 5 Sep 2009, Marek Vasut wrote:
> > > > > drivers/media/video/pxa_camera.c |4
> > > > > 1 files changed, 4 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/video/pxa_camera.c
> > > > > b/drivers/media/video/pxa_camera.c
> > > > > index 46e0d8a..de0fc8a 100644
> > > > > --- a/drivers/media/video/pxa_camera.c
> > > > > +++ b/drivers/media/video/pxa_camera.c
> > > > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > > > soc_camera_data_format *fmt)
> > > > > case V4L2_PIX_FMT_YVYU:
> > > > > case V4L2_PIX_FMT_RGB565:
> > > > > case V4L2_PIX_FMT_RGB555:
> > > > > + case V4L2_PIX_FMT_RGB565X:
> > > > > + case V4L2_PIX_FMT_RGB555X:
> > > > > return 8;
> > > > > default:
> > > > > return fmt->depth;
> > > > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > > > soc_camera_device *icd, int idx,
> > > > > case V4L2_PIX_FMT_YVYU:
> > > > > case V4L2_PIX_FMT_RGB565:
> > > > > case V4L2_PIX_FMT_RGB555:
> > > > > + case V4L2_PIX_FMT_RGB565X:
> > > > > + case V4L2_PIX_FMT_RGB555X:
> > > > > formats++;
> > > > > if (xlate) {
> > > > > xlate->host_fmt = icd->formats + idx;
> >
> > What should we do with this patch? Any updates? I spoke to Guennadi and
> > he thinks it's not a good idea to apply it (as pxaqci doesnt support
> > those formats). But to my understanding, those formats are endian-swapped
> > versions of the other ones without X at the end so there shouldnt be a
> > problem with it.
>
> Marek, please, look in PXA270 datasheet. To support a specific pixel
> format means, e.g., to be able to process it further, according to this
> format's particular colour component ordering. Process further can mean
> convert to another format, extract various information from the data
> (statistics, etc.)... Now RGB555 looks like (from wikipedia)
>
> 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00
> R4 R3 R2 R1 R0 G4 G3 G2 G1 G1 B4 B3 B2 B1 B1 --
>
> (Actually, I thought bit 15 was unused, but it doesn't matter for this
> discussion.) Now, imagine what happens if you swap the two bytes. I don't
> think the PXA will still be able to meaningfully process that format.
>
Not on the pxa side, but on the camera side -- Bs and Rs swapped in the diagram
above.
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
--
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] Add RGB555X and RGB565X formats to pxa-camera
Dne Po 3. srpna 2009 14:31:12 Guennadi Liakhovetski napsal(a):
> On Mon, 3 Aug 2009, Eric Miao wrote:
> > Marek Vasut wrote:
> > > Hi!
> > >
> > > Eric, would you mind applying ?
> > >
> > > From 4dcbff010e996f4c6e5761b3c19f5d863ab51b39 Mon Sep 17 00:00:00 2001
> > > From: Marek Vasut
> > > Date: Mon, 3 Aug 2009 10:27:57 +0200
> > > Subject: [PATCH] Add RGB555X and RGB565X formats to pxa-camera
> > >
> > > Those formats are requiered on widely used OmniVision OV96xx cameras.
> > > Those formats are nothing more then endian-swapped RGB555 and RGB565.
> > >
> > > Signed-off-by: Marek Vasut
> >
> > Acked-by: Eric Miao
> >
> > Guennadi,
> >
> > Would be better if this gets merged by you, thanks.
>
> Indeed it would, and I do have a couple of questions to this and related
> patches:
>
> 1. Marek, you're saying, you need these formats for the OV96xx camera. Yre
> you using the patch from Stefan Herbrechtsmeier to support ov96xx or some
> other?
>
> 2. Mike, while reviewing this patch I came across code in
> pxa_camera_setup_cicr(), introduced by your earlier patch:
>
> case V4L2_PIX_FMT_RGB555:
> cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
> CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
> break;
>
> Why are you enabling the RGB to RGBT conversion here unconditionally?
> Generally, what are the advantages of configuring CICR1 for a specific RGB
> format compared to using just a raw capture? Do I understand it right,
> that ATM we are not using any of those features?
>
> Thanks
> Guennadi
>
> > > ---
> > > drivers/media/video/pxa_camera.c |4
> > > 1 files changed, 4 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/media/video/pxa_camera.c
> > > b/drivers/media/video/pxa_camera.c
> > > index 46e0d8a..de0fc8a 100644
> > > --- a/drivers/media/video/pxa_camera.c
> > > +++ b/drivers/media/video/pxa_camera.c
> > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > soc_camera_data_format *fmt)
> > > case V4L2_PIX_FMT_YVYU:
> > > case V4L2_PIX_FMT_RGB565:
> > > case V4L2_PIX_FMT_RGB555:
> > > + case V4L2_PIX_FMT_RGB565X:
> > > + case V4L2_PIX_FMT_RGB555X:
> > > return 8;
> > > default:
> > > return fmt->depth;
> > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > soc_camera_device *icd, int idx,
> > > case V4L2_PIX_FMT_YVYU:
> > > case V4L2_PIX_FMT_RGB565:
> > > case V4L2_PIX_FMT_RGB555:
> > > + case V4L2_PIX_FMT_RGB565X:
> > > + case V4L2_PIX_FMT_RGB555X:
> > > formats++;
> > > if (xlate) {
> > > xlate->host_fmt = icd->formats + idx;
>
> ---
> Guennadi Liakhovetski
What should we do with this patch? Any updates? I spoke to Guennadi and he
thinks it's not a good idea to apply it (as pxaqci doesnt support those
formats). But to my understanding, those formats are endian-swapped versions of
the other ones without X at the end so there shouldnt be a problem with it.
--
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 2/3] Add driver for OmniVision OV9640 sensor
Dne Po 24. srpna 2009 20:06:29 Guennadi Liakhovetski napsal(a):
> Hi Marek
>
> On Sat, 22 Aug 2009, Marek Vasut wrote:
> > From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00 2001
> > From: Marek Vasut
> > Date: Sat, 22 Aug 2009 05:14:23 +0200
> > Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
> >
> > Signed-off-by: Marek Vasut
>
> Ok, I see, you rebased your patches on my soc-camera patch-stack, this is
> good, thanks. But you missed a couple of my comments - you still have a
> few static functions in the ov9640.c marked inline: ov9640_set_crop() is
> not used at all, ov9640_set_bus_param() gets assigned to a struct member,
> so, cannot be inline. ov9640_alter_regs() is indeed only called at one
> location, but see Chapter 15 in Documentation/CodingStyle. You left at
> least one multi-line comment wrongly formatted. You left some broken
> printk format lines, etc. You moved your header under drivers/... - that's
> good. But, please, address all my comments that I provided in a private
> review, or explain why you do not agree. Otherwise I feel like I wasted my
> time. Besides, your mailer has wrapped lines. Please, read
> Documentation/email-clients.txt on how to configure your email client to
> send patches properly. In the worst case, you can inline your patches
> while reviewing, and then attach them for a final submission. This is,
> however, discouraged, because it makes review and test of your
> intermediate patches with wrapped lines more difficult. Also, please check
> your patches with scripts/checkpatch.pl.
>
Fixed patch follows, my mailer is fixed as much as possible (working on getting
git-email to work, but that's still to be done). Please consider applying,
thanks.
Add driver for OmniVision OV9640 sensor
Signed-off-by: Marek Vasut
---
drivers/media/video/Kconfig |6 +
drivers/media/video/Makefile|1 +
drivers/media/video/ov9640.c| 811 +++
drivers/media/video/ov9640.h| 206 ++
include/media/v4l2-chip-ident.h |1 +
5 files changed, 1025 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/ov9640.c
create mode 100644 drivers/media/video/ov9640.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 84b6fc1..fddd654 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X
help
This is a ov772x camera driver
+config SOC_CAMERA_OV9640
+ tristate "ov9640 camera support"
+ depends on SOC_CAMERA && I2C
+ help
+ This is a ov9640 camera driver
+
config MX1_VIDEO
bool
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 9f2e321..e18efd5 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o
obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o
obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o
obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o
+obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o
obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o
# And now the v4l2 drivers:
diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
new file mode 100644
index 000..289e82d
--- /dev/null
+++ b/drivers/media/video/ov9640.c
@@ -0,0 +1,811 @@
+/*
+ * OmniVision OV96xx Camera Driver
+ *
+ * Copyright (C) 2009 Marek Vasut
+ *
+ * Based on ov772x camera driver:
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ov9640.h"
+
+/* default register setup */
+static const struct ov9640_reg ov9640_regs_dflt[] = {
+ { OV9640_COM5, OV9640_COM5_SYSCLK | OV9640_COM5_LONGEXP },
+ { OV9640_COM6, OV9640_COM6_OPT_BLC | OV9640_COM6_ADBLC_BIAS |
+ OV9640_COM6_FMT_RST | OV9640_COM6_ADBLC_OPTEN },
+ { OV9640_PSHFT, OV9640_PSHFT_VAL(0x01) },
+ { OV9640_ACOM, OV9640_ACOM_2X_ANALOG | OV9640_ACOM_RSVD },
+ { OV9640_COM16, OV9640_COM16_RB_AVG },
+
+ /* Gamma curve P */
+ { 0x6c, 0x40 }, { 0x6d, 0x30 }, { 0x6e, 0x4b }, { 0x6f, 0x60 },
+ { 0x70, 0x70 }, { 0x71, 0x70 }, { 0x72, 0x70 }, { 0x73, 0x70 },
+ { 0x74, 0x60 }, { 0x75, 0x60 }, { 0x76, 0x50 }, { 0x77, 0x48 },
+ { 0x78
[PATCH 2/3] Add driver for OmniVision OV9640 sensor
>From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00 2001
From: Marek Vasut
Date: Sat, 22 Aug 2009 05:14:23 +0200
Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor
Signed-off-by: Marek Vasut
---
drivers/media/video/Kconfig |6 +
drivers/media/video/Makefile|1 +
drivers/media/video/ov9640.c| 660
+++
drivers/media/video/ov9640.h| 366 ++
include/media/v4l2-chip-ident.h |2 +
5 files changed, 1035 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/video/ov9640.c
create mode 100644 drivers/media/video/ov9640.h
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 84b6fc1..fddd654 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -772,6 +772,12 @@ config SOC_CAMERA_OV772X
help
This is a ov772x camera driver
+config SOC_CAMERA_OV9640
+ tristate "ov9640 camera support"
+ depends on SOC_CAMERA && I2C
+ help
+ This is a ov9640 camera driver
+
config MX1_VIDEO
bool
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 9f2e321..e18efd5 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_SOC_CAMERA_MT9M111) += mt9m111.o
obj-$(CONFIG_SOC_CAMERA_MT9T031) += mt9t031.o
obj-$(CONFIG_SOC_CAMERA_MT9V022) += mt9v022.o
obj-$(CONFIG_SOC_CAMERA_OV772X)+= ov772x.o
+obj-$(CONFIG_SOC_CAMERA_OV9640)+= ov9640.o
obj-$(CONFIG_SOC_CAMERA_TW9910)+= tw9910.o
# And now the v4l2 drivers:
diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
new file mode 100644
index 000..3785c96
--- /dev/null
+++ b/drivers/media/video/ov9640.c
@@ -0,0 +1,660 @@
+/*
+ * OmniVision OV96xx Camera Driver
+ *
+ * Copyright (C) 2009 Marek Vasut
+ *
+ * Based on ov772x camera driver:
+ *
+ * Copyright (C) 2008 Renesas Solutions Corp.
+ * Kuninori Morimoto
+ *
+ * Based on ov7670 and soc_camera_platform driver,
+ *
+ * Copyright 2006-7 Jonathan Corbet
+ * Copyright (C) 2008 Magnus Damm
+ * Copyright (C) 2008, Guennadi Liakhovetski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ov9640.h"
+
+/* read a register */
+static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
+{
+ int ret;
+ u8 data = reg;
+ struct i2c_msg msg = {
+ .addr = client->addr,
+ .flags = 0,
+ .len= 1,
+ .buf= &data,
+ };
+
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ if (ret < 0)
+ goto err;
+
+ msg.flags = I2C_M_RD;
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ if (ret < 0)
+ goto err;
+
+ *val = data;
+ return 0;
+
+err:
+ dev_err(&client->dev, "Failed reading register 0x%02x!\n", reg);
+ return ret;
+}
+
+/* write a register */
+static int ov9640_reg_write(struct i2c_client *client, u8 reg, u8 val)
+{
+ int ret;
+ u8 _val;
+ unsigned char data[2] = { reg, val };
+ struct i2c_msg msg = {
+ .addr = client->addr,
+ .flags = 0,
+ .len= 2,
+ .buf= data,
+ };
+
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed writing register 0x%02x!\n", reg);
+ return ret;
+ }
+
+ /* we have to read the register back ... no idea why, maybe HW bug */
+ ret = ov9640_reg_read(client, reg, &_val);
+ if (ret)
+ dev_err(&client->dev, "Failed reading back register "
+ "0x%02x!\n", reg);
+
+ return 0;
+}
+
+
+/* Read a register, alter it's bits, write it back */
+static int ov9640_reg_rmw(struct i2c_client *client, u8 reg, u8 set, u8
unset)
+{
+ /*s32*/u8 val;
+ int ret;
+
+ ret = ov9640_reg_read(client, reg, &val);
+ if (ret) {
+ dev_err(&client->dev, "[Read]-Modify-Write of register %02x"
+ " failed!\n", reg);
+ return val;
+ }
+
+ val |= set;
+ val &= ~unset;
+
+ ret = ov9640_reg_write(client, reg, val);
+ if (ret)
+ dev_err(&client->dev, "Read-Modify-[Write] of register %02x"
+ "failed!\n", reg);
+
+ return ret;
[PATCH 1/3] Add RGB555X and RGB565X formats to pxa-camera
>From 287b146839e3f96b34336f40e1ab7b154cd58a64 Mon Sep 17 00:00:00 2001
From: Marek Vasut
Date: Sat, 22 Aug 2009 05:13:22 +0200
Subject: [PATCH 1/3] Add RGB555X and RGB565X formats to pxa-camera
Those formats are requiered on widely used OmniVision OV96xx cameras.
Those formats are nothing more then endian-swapped RGB555 and RGB565.
Signed-off-by: Marek Vasut
---
drivers/media/video/pxa_camera.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/pxa_camera.c
b/drivers/media/video/pxa_camera.c
index 7c86ef9..ef5d293 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -1110,10 +1110,12 @@ static void pxa_camera_setup_cicr(struct
soc_camera_device *icd,
cicr1 |= CICR1_COLOR_SP_VAL(2);
break;
case V4L2_PIX_FMT_RGB555:
+ case V4L2_PIX_FMT_RGB555X:
cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
break;
case V4L2_PIX_FMT_RGB565:
+ case V4L2_PIX_FMT_RGB565X:
cicr1 |= CICR1_COLOR_SP_VAL(1) | CICR1_RGB_BPP_VAL(2);
break;
}
@@ -1240,6 +1242,8 @@ static int required_buswidth(const struct
soc_camera_data_format *fmt)
case V4L2_PIX_FMT_YVYU:
case V4L2_PIX_FMT_RGB565:
case V4L2_PIX_FMT_RGB555:
+ case V4L2_PIX_FMT_RGB565X:
+ case V4L2_PIX_FMT_RGB555X:
return 8;
default:
return fmt->depth;
@@ -1289,6 +1293,8 @@ static int pxa_camera_get_formats(struct
soc_camera_device *icd, int idx,
case V4L2_PIX_FMT_YVYU:
case V4L2_PIX_FMT_RGB565:
case V4L2_PIX_FMT_RGB555:
+ case V4L2_PIX_FMT_RGB565X:
+ case V4L2_PIX_FMT_RGB555X:
formats++;
if (xlate) {
xlate->host_fmt = icd->formats + idx;
--
1.6.3.3
--
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] Add RGB555X and RGB565X formats to pxa-camera
Dne Po 3. srpna 2009 14:31:12 Guennadi Liakhovetski napsal(a):
> On Mon, 3 Aug 2009, Eric Miao wrote:
> > Marek Vasut wrote:
> > > Hi!
> > >
> > > Eric, would you mind applying ?
> > >
> > > From 4dcbff010e996f4c6e5761b3c19f5d863ab51b39 Mon Sep 17 00:00:00 2001
> > > From: Marek Vasut
> > > Date: Mon, 3 Aug 2009 10:27:57 +0200
> > > Subject: [PATCH] Add RGB555X and RGB565X formats to pxa-camera
> > >
> > > Those formats are requiered on widely used OmniVision OV96xx cameras.
> > > Those formats are nothing more then endian-swapped RGB555 and RGB565.
> > >
> > > Signed-off-by: Marek Vasut
> >
> > Acked-by: Eric Miao
> >
> > Guennadi,
> >
> > Would be better if this gets merged by you, thanks.
>
> Indeed it would, and I do have a couple of questions to this and related
> patches:
>
> 1. Marek, you're saying, you need these formats for the OV96xx camera. Yre
> you using the patch from Stefan Herbrechtsmeier to support ov96xx or some
> other?
Hi!
I'm writing one mostly (I took some bits from Trilok's earlier attempt) from
scratch, as you can see here:
http://marex-hnd.blogspot.com/2009/08/omnivision-ov9640-hacking.html
http://marex-hnd.blogspot.com/2009/08/omnivision-ov9640-hacking-part.html
>
> 2. Mike, while reviewing this patch I came across code in
> pxa_camera_setup_cicr(), introduced by your earlier patch:
>
> case V4L2_PIX_FMT_RGB555:
> cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
> CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
> break;
>
> Why are you enabling the RGB to RGBT conversion here unconditionally?
> Generally, what are the advantages of configuring CICR1 for a specific RGB
> format compared to using just a raw capture? Do I understand it right,
> that ATM we are not using any of those features?
>
> Thanks
> Guennadi
>
> > > ---
> > > drivers/media/video/pxa_camera.c |4
> > > 1 files changed, 4 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/media/video/pxa_camera.c
> > > b/drivers/media/video/pxa_camera.c
> > > index 46e0d8a..de0fc8a 100644
> > > --- a/drivers/media/video/pxa_camera.c
> > > +++ b/drivers/media/video/pxa_camera.c
> > > @@ -1222,6 +1222,8 @@ static int required_buswidth(const struct
> > > soc_camera_data_format *fmt)
> > > case V4L2_PIX_FMT_YVYU:
> > > case V4L2_PIX_FMT_RGB565:
> > > case V4L2_PIX_FMT_RGB555:
> > > + case V4L2_PIX_FMT_RGB565X:
> > > + case V4L2_PIX_FMT_RGB555X:
> > > return 8;
> > > default:
> > > return fmt->depth;
> > > @@ -1260,6 +1262,8 @@ static int pxa_camera_get_formats(struct
> > > soc_camera_device *icd, int idx,
> > > case V4L2_PIX_FMT_YVYU:
> > > case V4L2_PIX_FMT_RGB565:
> > > case V4L2_PIX_FMT_RGB555:
> > > + case V4L2_PIX_FMT_RGB565X:
> > > + case V4L2_PIX_FMT_RGB555X:
> > > formats++;
> > > if (xlate) {
> > > xlate->host_fmt = icd->formats + idx;
>
> ---
> Guennadi Liakhovetski
--
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
