Re: [PATCH V2] media: i2c: Add ADV761X support

Hans Verkuil
2013-11-20T15:44:09Z

Hi Valentine,

Did you ever look at this adv7611 driver:

https://github.com/Xilinx/linux-xlnx/commit/610b9d5de22ae7c0047c65a07e4afa42af2daa12

It adds adv761x support to the adv7604 in a pretty clean way.

Thinking it over I prefer to use that code (although you will have to
add the soc-camera hack for the time being) over your driver.

Others need adv7611 support as well, but with all the dv_timings etc. features
that are removed in your driver. So I am thinking that it is easier to merge
the xilinx version and add whatever you need on top of that.

Regards,

Hans

On 11/20/13 13:24, Valentine wrote:
> On 11/20/2013 03:19 PM, Hans Verkuil wrote:
>> Hi Valentine,
>
> Hi Hans,
>
>>
>> On 11/20/13 11:14, Valentine wrote:
>>> On 11/19/2013 01:50 PM, Hans Verkuil wrote:
>>>> Hi Valentine,
>>>
>>> Hi Hans,
>>> thanks for your review.
>>>
>>>>
>>>> I don't entirely understand how you use this driver with soc-camera.
>>>> Since soc-camera doesn't support FMT_CHANGE notifies it can't really
>>>> act on it. Did you hack soc-camera to do this?
>>>
>>> I did not. The format is queried before reading the frame by the user-space.
>>> I'm not sure if there's some kind of generic interface to notify the camera
>>> layer about format change events. Different subdevices use different FMT_CHANGE
>>> defines for that. I've implemented the format change notifier based on the adv7604
>>> in hope that it may be useful later.
>>
>> Yes, I need to generalize the FMT_CHANGE event.
>>
>> But what happens if you are streaming and the HDMI connector is unplugged?
>> Or plugged back in again, possibly with a larger resolution? I'm not sure
>> if the soc_camera driver supports such scenarios.
>
> It doesn't. Currently it's up to the UI to poll the format and do the necessary changes.
> Otherwise the picture will be incorrect.
>
>>
>>>
>>>>
>>>> The way it stands I would prefer to see a version of the driver without
>>>> soc-camera support. I wouldn't have a problem merging that as this driver
>>>> is a good base for further development.
>>>
>>> I've tried to implement the driver base good enough to work with both SoC
>>> and non-SoC cameras since I don't think having 2 separate drivers for
>>> different camera models is a good idea.
>>>
>>> The problem is that I'm using it with R-Car VIN SoC camera driver and don't
>>> have any other h/w. Having a platform data quirk for SoC camera in
>>> the subdevice driver seemed simple and clean enough.
>>
>> I hate it, but it isn't something you can do anything about. So it will have
>> to do for now.
>>
>>> Hacking SoC camera to make it support both generic and SoC cam subdevices
>>> doesn't seem that straightforward to me.
>>
>> Guennadi, what is the status of this? I'm getting really tired of soc-camera
>> infecting sub-devices. Subdev drivers should be independent of any bridge
>> driver using them, but soc-camera keeps breaking that. It's driving me nuts.
>>
>> I'll be honest, it's getting to the point that I want to just NACK any
>> future subdev drivers that depend on soc-camera, just to force a solution.
>> There is no technical reason for this dependency, it just takes some time
>> to fix soc-camera.
>>
>>> Re-implementing R-Car VIN as a non-SoC model seems quite a big task that
>>> involves a lot of regression testing with other R-Car boards that use different
>>> subdevices with VIN.
>>>
>>> What would you suggest?
>>
>> Let's leave it as-is for now :-(
>>
>> I'm not happy, but as I said, it's not your fault.
>
> OK, thanks.
> Once a better solution is available we can remove the quirk.
>
>>
>> Regards,
>>
>> Hans
>
> Thanks,
> Val.
>
>>
>>>
>>>>
>>>> You do however have to add support for the V4L2_CID_DV_RX_POWER_PRESENT
>>>> control. It's easy to implement and that way apps can be notified when
>>>> the hotplug changes value.
>>>
>>> OK, thanks.
>>>
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>
>>> Thanks,
>>> Val.
>>>
>>>>
>>>> On 11/15/13 13:54, Valentine Barshak wrote:
>>>>> This adds ADV7611/ADV7612 Xpressview HDMI Receiver base
>>>>> support. Only one HDMI port is supported on ADV7612.
>>>>>
>>>>> The code is based on the ADV7604 driver, and ADV7612 patch by
>>>>> Shinobu Uehara <shinobu.uehara.xc@renesas.com>
>>>>>
>>>>> Changes in version 2:
>>>>> * Used platform data for I2C addresses setup. The driver
>>>>> should work with both SoC and non-SoC camera models.
>>>>> * Dropped unnecessary code and unsupported callbacks.
>>>>> * Implemented IRQ handling for format change detection.
>>>>>
>>>>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>
>