On 07/17/2013 06:55 AM, Arun Kumar K wrote: > On Wed, Jul 17, 2013 at 3:33 AM, Sylwester Nawrocki > <sylvester.nawrocki@xxxxxxxxx> wrote: >> On 07/09/2013 02:04 PM, Arun Kumar K wrote: >>> >>> On Wed, Jun 26, 2013 at 12:57 PM, Hans Verkuil<hverkuil@xxxxxxxxx> wrote: >>>> >>>> On Fri May 31 2013 15:03:23 Arun Kumar K wrote: >>>>> >>>>> FIMC-IS uses certain sensors which are exclusively controlled >>>>> from the IS firmware. This patch adds the sensor subdev for the >>>>> fimc-is sensors. >>>>> >>>>> Signed-off-by: Arun Kumar K<arun.kk@xxxxxxxxxxx> >>>>> Signed-off-by: Kilyeon Im<kilyeon.im@xxxxxxxxxxx> >>>> >>>> >>>> Not surprisingly I really hate the idea of sensor drivers that are tied >>>> to >>>> a specific SoC, since it completely destroys the reusability of such >>>> drivers. >>>> >>> >>> Yes agree to it. >>> >>>> I understand that you have little choice to do something special here, >>>> but >>>> I was wondering whether there is a way of keeping things as generic as >>>> possible. >>>> >>>> I'm just brainstorming here, but as far as I can see this driver is >>>> basically >>>> a partial sensor driver: it handles the clock, the format negotiation and >>>> power management. Any sensor driver needs that. >>>> >>>> What would be nice is if the fmic specific parts are replaced by >>>> callbacks >>>> into the bridge driver using v4l2_subdev_notify(). >>>> >>>> The platform data (or DT) can also state if this sensor is firmware >>>> controlled >>>> or not. If not, then the missing bits can be implemented in the future by >>>> someone who needs that. >>>> >>>> That way the driver itself remains independent from fimc. >>>> >>>> And existing sensor drivers can be adapted to be usable with fimc as well >>>> by >>>> adding support for the notify callback. >>>> >>>> Would a scheme along those lines work? >>>> >>> >>> Yes this should make the implementation very generic. >>> Will check the feasibility of this approach. >> >> >> Is I suggested earlier, you likely could do without this call back to the >> FIMC-IS from within the sensor subdev. Look at your call chain right now: >> >> /dev/video? media-dev-driver sensor-subdev FIMC-IS >> | | | | >> | VIDIOC_STREAMON | | | >> |----------------># s_stream() | | >> | #------------------># pipeline_open() | >> | | # ---------------->| >> | | # pipeline_start() | >> | | # ---------------->| >> | | | | >> >> Couldn't you move pipeline_open(), pipeline_start() to s_stream handler >> of the ISP subdev ? It is currently empty. The media device driver could >> call s_stream on the ISP subdev each time it sees s_stream request on >> the sensor subdev. And you wouldn't need any hacks to get the pipeline >> pointer in the sensor subdev. Then it would be something like: >> >> /dev/video? media-dev-driver sensor-subdev FIMC-IS-ISP-subdev >> | | | | >> | VIDIOC_STREAMON | | | >> |----------------># s_stream() | | >> | #------------------>| | >> | # s_stream() | | >> | #-------------------+------------># pipeline_open() >> | | | # pipeline_start() >> | | | # >> >> I suppose pipeline_open() is better candidate for the s_power callback. >> It just needs to be ensured at the media device level the subdev >> operations sequences are correct. >> > > It can be done this way. But my intention of putting these calls in > the sensor subdev was to use the sensor subdev independent of > isp subdev. This is for the usecase where the pipeline will only contain > > is-sensor --> mipi-csis --> fimc-lite ---> memory > > This way you can capture the bayer rgb data from sensor without using > any isp components at all. > > The second pipeline which is isp --> scc --> scp > can be used for processing the sensor data and can be created and > used if needed. > > In the method you mentioned, the isp subdev has to be used even > when it is not part of the pipeline. Is that allowed? > If its allowed as per media pipeline guidelines, then this definitely > is a better approach. Please suggest on this. Sure, I'm aware of those two relatively separate pipelines. s_power, s_stream callbacks belong to the kernel so I don't think it would be an issue to do it as I described. Please note s_power, s_stream are normally reference counted. Alternatively you could create a separate subdev for the FIMC-IS firmware interface. Such subdev wouldn't be exposing device node and would be used by the media pipeline controller driver to ensure proper hardware configuration sequences. I don't know the Exynos5 FIMC-IS firmware architecture very well so I'm not sure if it is worth to create such a separate subdev as the firmware interface obstruction layer ;) I guess we could do only with subdevs that are exposed to user space. Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
