From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp-vbr4.xs4all.nl ([194.109.24.24]:3577 "EHLO smtp-vbr4.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753265Ab3KTPoJ (ORCPT ); Wed, 20 Nov 2013 10:44:09 -0500 Message-ID: <528CD86D.70506@xs4all.nl> Date: Wed, 20 Nov 2013 16:42:37 +0100 From: Hans Verkuil MIME-Version: 1.0 To: Valentine CC: linux-media@vger.kernel.org, Mauro Carvalho Chehab , Hans Verkuil , Laurent Pinchart , Guennadi Liakhovetski , Simon Horman Subject: Re: [PATCH V2] media: i2c: Add ADV761X support References: <1384520071-16463-1-git-send-email-valentine.barshak@cogentembedded.com> <528B347E.2060107@xs4all.nl> <528C8BA1.9070706@cogentembedded.com> <528C9ADB.3050803@xs4all.nl> <528CA9E1.2020401@cogentembedded.com> In-Reply-To: <528CA9E1.2020401@cogentembedded.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: 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 >>>>> >>>>> 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 > >