| Message ID | 1362754765-2651-2-git-send-email-arun.kk@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/08/2013 03:59 PM, Arun Kumar K wrote: > Add the fimc-is node and the required pinctrl nodes for > fimc-is driver for Exynos5. Also adds the DT binding documentation > for the new fimc-is node. > > Signed-off-by: Arun Kumar K<arun.kk@samsung.com> > Signed-off-by: Kilyeon Im<kilyeon.im@samsung.com> > --- > .../devicetree/bindings/media/soc/exynos5-is.txt | 81 ++++++++++++++++++++ > arch/arm/boot/dts/exynos5250-pinctrl.dtsi | 60 +++++++++++++++ > arch/arm/boot/dts/exynos5250-smdk5250.dts | 54 ++++++++++++- > arch/arm/boot/dts/exynos5250.dtsi | 8 ++ > 4 files changed, 201 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/soc/exynos5-is.txt > mode change 100644 => 100755 arch/arm/boot/dts/exynos5250-smdk5250.dts > mode change 100644 => 100755 arch/arm/boot/dts/exynos5250.dtsi I suspect this mode change wasn't intentional. > diff --git a/Documentation/devicetree/bindings/media/soc/exynos5-is.txt b/Documentation/devicetree/bindings/media/soc/exynos5-is.txt > new file mode 100644 > index 0000000..e0fdf02 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/soc/exynos5-is.txt > @@ -0,0 +1,81 @@ > +Samsung EXYNOS SoC Camera Subsystem > +----------------------------------- > + > +The camera subsystem on Samsung Exynos5 SoC has some changes relative > +to previous SoC versions. Exynos5 has almost similar MIPI-CSIS and > +FIMC-LITE IPs but removed the FIMC-CAPTURE. Instead it has an improved s/FIMC-CAPTURE/FIMC IPs (IP blocks) ? > +FIMC-IS which can provide imate data DMA output. s/imate/image Not sure if this is true. FIMC (camera host interface and video post-processor) IPs have been removed, but I think they were replaced with GSCALER. There is FIMC-IS on both Exynos4 and Exynos5 series. > + > +The device tree binding remain similar to the Exynos4 bindings which can > +be seen at samsung-fimc.txt with the addition of fimc-is sub-node which will > +be explained here. > + > +fimc-is subnode of camera node > +------------------------------ > + > +Required properties: > + > +- compatible : must be "samsung,exynos5250-fimc-is" > +- reg : physical base address and size of the memory mapped > + registers > +- interrupt-parent : Parent interrupt controller > +- interrupts : fimc-is interrupt to the parent combiner > + > +Board specific properties: > + > +- pinctrl-names : pinctrl names for camera port pinmux control, at least > + "default" needs to be specified. > +- pinctrl-0...N : pinctrl properties corresponding to pinctrl-names > + > +Sensor sub-nodes: > + > +FIMC-IS IP supports custom built sensors to be controlled exclusively by > +the FIMC-IS firmware. These sensor properties are to be defined here. What this means is that the software architecture defines how we describe the hardware. I have serious doubts about this approach. I do recall this concept was used in an early version of code I shared with you. But then I decided to describe the I2C bus controllers (other bus controllers could be probably added later when required) in usual way, as fimc-is node child nodes or standalone, at root level. The reason is that all these peripheral bus controllers are accessible in same way by the FIMC-IS and the host CPU. Their interrupts are routed to both, and registers regions are visible to both CPUs as well. AFAIK ISP I2C bus controllers are directly compatible with s3c2440, so on some systems that do not use the IS these I2C controllers should in theory work without problems with current i2c-s3c2410 I2C bus driver. Defining image sensor nodes in a standard way as ISP I2C bus controller nodes has an disadvantage that we need dummy I2C bus controller driver, at least this is how I have written the driver for Exynos4x12. In some version of it I had sensor nodes put in a isp-i2c fimc-is sub-node, but then there was an issue that this was not a fully specified I2C bus controller node. You can refer to my exynos4 fimc-is patch series for details on how this is now implemented. Handling the image sensor in a standard way, as regular I2C client devices has an advantage that we can put pinctrl properties in relevant device nodes, where available, which more closely describes the hardware structure. I'm not really sure in 100% if all this complication is required. It would allow to use same DT blob for different Imaging Subsystem SW architecture. For example some parts of functionality handled currently by FIMC-IS (ARM Cortex-A5) could be moved to host CPU, without any change in the device tree structure. The kernel could decide e.g. if it uses image sensor driver implemented in the ISP firmware, or a driver run on the host CPU. What do you think ? Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, Thank you for the review. >> +Sensor sub-nodes: >> + >> +FIMC-IS IP supports custom built sensors to be controlled exclusively by >> +the FIMC-IS firmware. These sensor properties are to be defined here. > [snip] > > Defining image sensor nodes in a standard way as ISP I2C bus controller > nodes has an disadvantage that we need dummy I2C bus controller driver, > at least this is how I have written the driver for Exynos4x12. In some > version of it I had sensor nodes put in a isp-i2c fimc-is sub-node, but > then there was an issue that this was not a fully specified I2C bus > controller node. > > You can refer to my exynos4 fimc-is patch series for details on how this > is now implemented. > > Handling the image sensor in a standard way, as regular I2C client devices > has an advantage that we can put pinctrl properties in relevant device > nodes, > where available, which more closely describes the hardware structure. > > I'm not really sure in 100% if all this complication is required. It would > allow to use same DT blob for different Imaging Subsystem SW architecture. > For example some parts of functionality handled currently by FIMC-IS (ARM > Cortex-A5) could be moved to host CPU, without any change in the device > tree structure. The kernel could decide e.g. if it uses image sensor driver > implemented in the ISP firmware, or a driver run on the host CPU. > > What do you think ? > I have seen your Exynos4 FIMC-IS patchset and you have made a dummy I2C sensor driver there. That mode would work fine in Exynos4 since the sensor and ISP will be used by the same media controller pipeline. So the ISP component in the pipeline will ensure that the HW is initialized and sensor is working. But in Exynos5, we are using sensor in pipeline0 and ISP in pipeline1. So there is a possibility of using sensor subdev independently without using pipeline1 ISP components. So with the driver I sent, the pipeline0 can still work like this --> ISP sensor ---> MIPI-CSIS ---> FIMC-LITE ---> Memory This cannot be done if a dummy I2C driver is made for ISP sensor. What is your suggestion on this? Regards Arun -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/26/2013 01:17 PM, Arun Kumar K wrote: >>> +Sensor sub-nodes: >>> + >>> +FIMC-IS IP supports custom built sensors to be controlled exclusively by >>> +the FIMC-IS firmware. These sensor properties are to be defined here. > > [snip] > >> Defining image sensor nodes in a standard way as ISP I2C bus controller >> nodes has an disadvantage that we need dummy I2C bus controller driver, >> at least this is how I have written the driver for Exynos4x12. In some >> version of it I had sensor nodes put in a isp-i2c fimc-is sub-node, but >> then there was an issue that this was not a fully specified I2C bus >> controller node. >> >> You can refer to my exynos4 fimc-is patch series for details on how this >> is now implemented. >> >> Handling the image sensor in a standard way, as regular I2C client devices >> has an advantage that we can put pinctrl properties in relevant device >> nodes, >> where available, which more closely describes the hardware structure. >> >> I'm not really sure in 100% if all this complication is required. It would >> allow to use same DT blob for different Imaging Subsystem SW architecture. >> For example some parts of functionality handled currently by FIMC-IS (ARM >> Cortex-A5) could be moved to host CPU, without any change in the device >> tree structure. The kernel could decide e.g. if it uses image sensor driver >> implemented in the ISP firmware, or a driver run on the host CPU. >> >> What do you think ? > > I have seen your Exynos4 FIMC-IS patchset and you have made a dummy > I2C sensor driver there. > That mode would work fine in Exynos4 since the sensor and ISP will be used > by the same media controller pipeline. So the ISP component in the pipeline > will ensure that the HW is initialized and sensor is working. > > But in Exynos5, we are using sensor in pipeline0 and ISP in pipeline1. > So there is a possibility of using sensor subdev independently > without using pipeline1 ISP components. > > So with the driver I sent, the pipeline0 can still work like this --> > > ISP sensor ---> MIPI-CSIS ---> FIMC-LITE ---> Memory > > This cannot be done if a dummy I2C driver is made for ISP sensor. Why not ? I'm not sure what the problem is here. I realize that describing image sensors in DT as standard I2C slave devices is not helpful with current firmware architecture. It adds some unnecessary complication, OTOH it could simplify the sensors registration and media graph initialization code, by unifying it for the firmware based ISP specific sensors and the external ones with a built-in ISP. Also we could avoid having the bindings defined by current architecture of the driver. Nevertheless, coming back to your question, the I2C controller driver would be in same module as the FIMC-IS driver. From user space perspective nothing changes when you add I2C bus driver and register the sensor in a standard way. What exactly couldn't be done in the kernel ? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 27, 2013 at 4:21 AM, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote: > On 03/26/2013 01:17 PM, Arun Kumar K wrote: >>>> >>>> +Sensor sub-nodes: >>>> + >>>> +FIMC-IS IP supports custom built sensors to be controlled exclusively >>>> by >>>> +the FIMC-IS firmware. These sensor properties are to be defined here. >> >> >> [snip] >> >>> Defining image sensor nodes in a standard way as ISP I2C bus controller >>> nodes has an disadvantage that we need dummy I2C bus controller driver, >>> at least this is how I have written the driver for Exynos4x12. In some >>> version of it I had sensor nodes put in a isp-i2c fimc-is sub-node, but >>> then there was an issue that this was not a fully specified I2C bus >>> controller node. >>> >>> You can refer to my exynos4 fimc-is patch series for details on how this >>> is now implemented. >>> >>> Handling the image sensor in a standard way, as regular I2C client >>> devices >>> has an advantage that we can put pinctrl properties in relevant device >>> nodes, >>> where available, which more closely describes the hardware structure. >>> >>> I'm not really sure in 100% if all this complication is required. It >>> would >>> allow to use same DT blob for different Imaging Subsystem SW >>> architecture. >>> For example some parts of functionality handled currently by FIMC-IS (ARM >>> Cortex-A5) could be moved to host CPU, without any change in the device >>> tree structure. The kernel could decide e.g. if it uses image sensor >>> driver >>> implemented in the ISP firmware, or a driver run on the host CPU. >>> >>> What do you think ? >> >> >> I have seen your Exynos4 FIMC-IS patchset and you have made a dummy >> I2C sensor driver there. >> That mode would work fine in Exynos4 since the sensor and ISP will be used >> by the same media controller pipeline. So the ISP component in the >> pipeline >> will ensure that the HW is initialized and sensor is working. >> >> But in Exynos5, we are using sensor in pipeline0 and ISP in pipeline1. >> So there is a possibility of using sensor subdev independently >> without using pipeline1 ISP components. >> >> So with the driver I sent, the pipeline0 can still work like this --> >> >> ISP sensor ---> MIPI-CSIS ---> FIMC-LITE ---> Memory >> >> This cannot be done if a dummy I2C driver is made for ISP sensor. > > > Why not ? I'm not sure what the problem is here. > > I realize that describing image sensors in DT as standard I2C slave devices > is not helpful with current firmware architecture. It adds some unnecessary > complication, OTOH it could simplify the sensors registration and media > graph > initialization code, by unifying it for the firmware based ISP specific > sensors and the external ones with a built-in ISP. Also we could avoid > having > the bindings defined by current architecture of the driver. > > Nevertheless, coming back to your question, the I2C controller driver would > be in same module as the FIMC-IS driver. From user space perspective nothing > changes when you add I2C bus driver and register the sensor in a standard > way. > What exactly couldn't be done in the kernel ? Only issue is with the context sharing. Right now you can see that the fimc-is context is shared between all the subdevs. As all of them are part of the same driver, it is possible. If sensor is made as an independent i2c device, a separate probe will be called for it. For ISP sensor subdev to work independently, it needs to call the fimc_is_pipeline_* calls for FW initialization and other configurations for which it needs the fimc-is main context. Now there is a workaround here by calling a get_context() macro in sensor's probe to get the fimc-is context. This will cause the same context to be shared and updated by 2 drivers though both are part of fimc-is. Is this acceptable? Or am I missing some other simple solution of implementing it in a better way? Regards Arun -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/27/2013 05:31 AM, Arun Kumar K wrote: > On Wed, Mar 27, 2013 at 4:21 AM, Sylwester Nawrocki > <sylvester.nawrocki@gmail.com> wrote: >> On 03/26/2013 01:17 PM, Arun Kumar K wrote: [...] > Only issue is with the context sharing. > Right now you can see that the fimc-is context is shared between all > the subdevs. > As all of them are part of the same driver, it is possible. > If sensor is made as an independent i2c device, a separate probe will > be called for it. > For ISP sensor subdev to work independently, it needs to call the > fimc_is_pipeline_* calls > for FW initialization and other configurations for which it needs the > fimc-is main context. > > Now there is a workaround here by calling a get_context() macro in > sensor's probe > to get the fimc-is context. This will cause the same context to be > shared and updated by > 2 drivers though both are part of fimc-is. > Is this acceptable? Or am I missing some other simple solution of implementing > it in a better way? OK, thanks for the explanation. I can think of at least one possible way to get hold of the fimc-is context in the subdev. For instance, in subdev's .registered callback you get a pointer to struct v4l2_device, which is normally embedded in a top level driver's private data. Then with container_of() you could get hold of required data at the fimc-is driver. But... to make the subdev drivers reuse possible subdevs should normally not be required to know the internals of a host driver they are registered to. And it looks a bit unusual to have fimc_pipeline_* calls in the sensor's operation handlers. I thought that the subdevs could provide basic methods and it would be the top level media driver that would resolve any dependencies in calling required subdev ops, according to media graph configuration done by the user on /dev/media?. The media driver has a list of media entities (subdevices and video nodes) and I though it could coordinate any requests involving whole video/image processing pipeline originating from /dev/video ioctls/fops. So for instance if /dev/video in this pipeline is opened sensor (sd) -> mipi-csis (sd) -> fimc-lite (sd) -> memory (/dev/video) it would call s_power operation on the above subdevs and additionally on e.g. the isp subdev (or any other we choose as a main subdev implementing the FIMC-IS slave interface). Then couldn't it be done that video node ioctls invoke pipeline operations, and the media device resolves any dependencies/calls order, as in case of the exynos4 driver ? As a side note, I'm working on adding a generic method to get any v4l2_subdev/video_device from a struct media_entity instance, so it is easier to handle link_notify events, power/streaming enable/ disable sequences, etc. Currently I have a data structure like: struct exynos_iss_entity { struct video_device vdev; struct v4l2_subdev subdev; struct exynos_iss_pipeline pipe; }; Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 27, 2013 at 7:17 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 03/27/2013 05:31 AM, Arun Kumar K wrote: >> On Wed, Mar 27, 2013 at 4:21 AM, Sylwester Nawrocki >> <sylvester.nawrocki@gmail.com> wrote: >>> On 03/26/2013 01:17 PM, Arun Kumar K wrote: > [...] >> Only issue is with the context sharing. >> Right now you can see that the fimc-is context is shared between all >> the subdevs. >> As all of them are part of the same driver, it is possible. >> If sensor is made as an independent i2c device, a separate probe will >> be called for it. >> For ISP sensor subdev to work independently, it needs to call the >> fimc_is_pipeline_* calls >> for FW initialization and other configurations for which it needs the >> fimc-is main context. >> >> Now there is a workaround here by calling a get_context() macro in >> sensor's probe >> to get the fimc-is context. This will cause the same context to be >> shared and updated by >> 2 drivers though both are part of fimc-is. >> Is this acceptable? Or am I missing some other simple solution of implementing >> it in a better way? > > OK, thanks for the explanation. > > I can think of at least one possible way to get hold of the fimc-is > context in the subdev. For instance, in subdev's .registered callback > you get a pointer to struct v4l2_device, which is normally embedded > in a top level driver's private data. Then with container_of() > you could get hold of required data at the fimc-is driver. But as per current implementation, it is not the fimc-is driver that is registering the ISP subdevs. It will be registered from the media controller driver. So fimc-is context cannot be obtained by just using container_of(). > > But... to make the subdev drivers reuse possible subdevs should > normally not be required to know the internals of a host driver they > are registered to. And it looks a bit unusual to have fimc_pipeline_* > calls in the sensor's operation handlers. fimc_pipeline_* I mentioned is not the media controller pipeline. In the fimc-is driver, all the subdevs just implement the interface part. All the core functionalities happen in fimc-is-pipeline.c and fimc-is-interface.c. Since these ISP subdevs (sensor, isp, scc, scp) are not independent devices, all are controlled by the ISP firmware whose configuration and interface is done from the fimc_is_pipeline_* and fimc_is_itf_* functions. So all the ISP subdevs including sensor need to call these functions. > > I thought that the subdevs could provide basic methods and it would > be the top level media driver that would resolve any dependencies > in calling required subdev ops, according to media graph configuration > done by the user on /dev/media?. > In case of ISP subdevs (isp, scc and scp), there is not much configuration that the media device can do. Only control possible is to turn on/off specific scaler DMA outputs which can be done via the video node ioctls. The role of media device here is mostly to convey the pipeline structure to the user. For eg. it is not possible to directly connect isp (sd) --> scp (sd). In the media controller pipeline1 implementation, we were planning to put immutable links between these subdevs. Is that acceptable? > The media driver has a list of media entities (subdevices and video > nodes) and I though it could coordinate any requests involving whole > video/image processing pipeline originating from /dev/video ioctls/fops. > > So for instance if /dev/video in this pipeline is opened > > sensor (sd) -> mipi-csis (sd) -> fimc-lite (sd) -> memory (/dev/video) > > it would call s_power operation on the above subdevs and additionally > on e.g. the isp subdev (or any other we choose as a main subdev > implementing the FIMC-IS slave interface). > > Then couldn't it be done that video node ioctls invoke pipeline > operations, and the media device resolves any dependencies/calls > order, as in case of the exynos4 driver ? On Exynos4 subdevs, it is well and good since all the subdevs are independent IPs. Here in ISP since the same IP can take one input and provide multiple outputs, we designed them as separate subdevs. So here we cannot make the subdevs independent of each other where only the sequence / dependencies is controlled from the media device. Regards Arun -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arun, On 03/28/2013 06:10 AM, Arun Kumar K wrote: > On Wed, Mar 27, 2013 at 7:17 PM, Sylwester Nawrocki > <s.nawrocki@samsung.com> wrote: >> On 03/27/2013 05:31 AM, Arun Kumar K wrote: >>> On Wed, Mar 27, 2013 at 4:21 AM, Sylwester Nawrocki >>> <sylvester.nawrocki@gmail.com> wrote: >>>> On 03/26/2013 01:17 PM, Arun Kumar K wrote: >> [...] >>> Only issue is with the context sharing. >>> Right now you can see that the fimc-is context is shared between all >>> the subdevs. >>> As all of them are part of the same driver, it is possible. >>> If sensor is made as an independent i2c device, a separate probe will >>> be called for it. >>> For ISP sensor subdev to work independently, it needs to call the >>> fimc_is_pipeline_* calls >>> for FW initialization and other configurations for which it needs the >>> fimc-is main context. >>> >>> Now there is a workaround here by calling a get_context() macro in >>> sensor's probe >>> to get the fimc-is context. This will cause the same context to be >>> shared and updated by >>> 2 drivers though both are part of fimc-is. >>> Is this acceptable? Or am I missing some other simple solution of implementing >>> it in a better way? >> >> OK, thanks for the explanation. >> >> I can think of at least one possible way to get hold of the fimc-is >> context in the subdev. For instance, in subdev's .registered callback >> you get a pointer to struct v4l2_device, which is normally embedded >> in a top level driver's private data. Then with container_of() >> you could get hold of required data at the fimc-is driver. > > But as per current implementation, it is not the fimc-is driver that is > registering the ISP subdevs. It will be registered from the > media controller driver. So fimc-is context cannot be obtained by > just using container_of(). I guess best option would be to have a function to get the IS slave interface driver context at the sensor subdev exported by the IS driver module, as you suggested previously. You still could obtain the fimc-is object from the media device private data structure, since the media device has normally a list of its all entities in one form or the other. But the sensor would need to know details of the media device, which makes it a bit pointless. Nevertheless, my main concern is the DT binding. Sharing the sensor subdev driver might not be that important at the moment, we are talking about 300..500 lines of code per ISP driver currently anyway. More important is to have the hardware described in a standard way, so when the firmware changes there is no need to change the DT bindings. >> But... to make the subdev drivers reuse possible subdevs should >> normally not be required to know the internals of a host driver they >> are registered to. And it looks a bit unusual to have fimc_pipeline_* >> calls in the sensor's operation handlers. > > fimc_pipeline_* I mentioned is not the media controller pipeline. Ok, I admit I got confused a bit, since the word "pipeline" refers in the code to both: the internal ISP chain, and the data processing chain containing the FIMC-IS and other devices like CSI-2 receiver or GScaler. > In the fimc-is driver, all the subdevs just implement the interface part. > All the core functionalities happen in fimc-is-pipeline.c and > fimc-is-interface.c. > Since these ISP subdevs (sensor, isp, scc, scp) are not independent > devices, all are controlled by the ISP firmware whose configuration and > interface is done from the fimc_is_pipeline_* and fimc_is_itf_* functions. > So all the ISP subdevs including sensor need to call these functions. > >> >> I thought that the subdevs could provide basic methods and it would >> be the top level media driver that would resolve any dependencies >> in calling required subdev ops, according to media graph configuration >> done by the user on /dev/media?. >> > > In case of ISP subdevs (isp, scc and scp), there is not much configuration > that the media device can do. Only control possible is to turn on/off > specific scaler DMA outputs which can be done via the video node ioctls. > The role of media device here is mostly to convey the pipeline structure > to the user. For eg. it is not possible to directly connect isp (sd) > --> scp (sd). > In the media controller pipeline1 implementation, we were planning to > put immutable links between these subdevs. Is that acceptable? Not sure I understand which links you mean exactly. Could you post the media graph generated by media-ctl (--print-dot) ? If you're talking about the on-the-fly (FIFO) links, then it probably makes sense. The media device driver should respond to the link_notify events and not to allow data links unsupported in the hardware. If you create immutable OTF links, then how would you switch between DMA and OTF interfaces ? Or can all processing blocks of the ISP chain work simultaneously with the DMA and OTF ? The FD block, for instance, can fed data from memory _or_ from previous processing block in the chain, right ? You will need a user interface to control which input is used and the links configuration seems most natural here. >> The media driver has a list of media entities (subdevices and video >> nodes) and I though it could coordinate any requests involving whole >> video/image processing pipeline originating from /dev/video ioctls/fops. >> >> So for instance if /dev/video in this pipeline is opened >> >> sensor (sd) -> mipi-csis (sd) -> fimc-lite (sd) -> memory (/dev/video) >> >> it would call s_power operation on the above subdevs and additionally >> on e.g. the isp subdev (or any other we choose as a main subdev >> implementing the FIMC-IS slave interface). >> >> Then couldn't it be done that video node ioctls invoke pipeline >> operations, and the media device resolves any dependencies/calls >> order, as in case of the exynos4 driver ? > > On Exynos4 subdevs, it is well and good since all the subdevs are > independent IPs. Here in ISP since the same IP can take one input and Not really, there are going to be 2 subdevs exposed by the fimc-is: ISP and FD. However FD is still not supported in my last patch series. I was planning this for a subsequent kernel release. > provide multiple outputs, we designed them as separate subdevs. So > here we cannot make the subdevs independent of each other where only > the sequence / dependencies is controlled from the media device. I'm not asking you to make the FIMC-IS subdevs more self-contained, it's of course perfectly fine to have multiple (logical) subdevs exposed by a complex device like that. I have been thinking only about the sensor driver, since the sensors are normally shared across ISPs from various chip manufacturers. But let us leave this topic for now. BTW, in my interpretation FIMC-IS is a collection of IPs/peripheral devices, not a single black box, including an image sensor. And the firmware should not be the most significant factor how we expose the whole subsystem to the user. All elements of the ISP chain, i.e. an IP control registers are visible to both, the main CPU and the FIMC-IS (Cortex-A5) MCU. Still, it would be possible to create v4l2 subdevs dynamically, depending on the firmware architecture. --- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, Sorry for the late reply. >>> >>> OK, thanks for the explanation. >>> >>> I can think of at least one possible way to get hold of the fimc-is >>> context in the subdev. For instance, in subdev's .registered callback >>> you get a pointer to struct v4l2_device, which is normally embedded >>> in a top level driver's private data. Then with container_of() >>> you could get hold of required data at the fimc-is driver. >> >> >> But as per current implementation, it is not the fimc-is driver that is >> registering the ISP subdevs. It will be registered from the >> media controller driver. So fimc-is context cannot be obtained by >> just using container_of(). > > > I guess best option would be to have a function to get the IS slave > interface driver context at the sensor subdev exported by the IS driver > module, as you suggested previously. Ok I will make the sensor as i2c device and check what is the best way to get the IS context in sensor subdev. > > You still could obtain the fimc-is object from the media device private > data structure, since the media device has normally a list of its all > entities in one form or the other. But the sensor would need to know > details of the media device, which makes it a bit pointless. > > Nevertheless, my main concern is the DT binding. Sharing the sensor > subdev driver might not be that important at the moment, we are talking > about 300..500 lines of code per ISP driver currently anyway. > > More important is to have the hardware described in a standard way, so > when the firmware changes there is no need to change the DT bindings. > Yes that's right. Need to define the hardware in a generic way regardless of what the firmware is doing. >>> >> >> In case of ISP subdevs (isp, scc and scp), there is not much configuration >> that the media device can do. Only control possible is to turn on/off >> specific scaler DMA outputs which can be done via the video node ioctls. >> The role of media device here is mostly to convey the pipeline structure >> to the user. For eg. it is not possible to directly connect isp (sd) >> --> scp (sd). >> In the media controller pipeline1 implementation, we were planning to >> put immutable links between these subdevs. Is that acceptable? > > > Not sure I understand which links you mean exactly. Could you post the > media graph generated by media-ctl (--print-dot) ? > > If you're talking about the on-the-fly (FIFO) links, then it probably > makes sense. The media device driver should respond to the link_notify > events and not to allow data links unsupported in the hardware. If you > create immutable OTF links, then how would you switch between DMA and > OTF interfaces ? Or can all processing blocks of the ISP chain work > simultaneously with the DMA and OTF ? The FD block, for instance, can fed > data from memory _or_ from previous processing block in the chain, right ? > You will need a user interface to control which input is used and the > links configuration seems most natural here. Yes I agree to that. Though in the current driver, there are no subdevs which can change between DMA <-> OTF inputs, in future versions we might add it. So link configuration option will be provided. > > >>> The media driver has a list of media entities (subdevices and video >>> nodes) and I though it could coordinate any requests involving whole >>> video/image processing pipeline originating from /dev/video ioctls/fops. >>> >>> So for instance if /dev/video in this pipeline is opened >>> >>> sensor (sd) -> mipi-csis (sd) -> fimc-lite (sd) -> memory (/dev/video) >>> >>> it would call s_power operation on the above subdevs and additionally >>> on e.g. the isp subdev (or any other we choose as a main subdev >>> implementing the FIMC-IS slave interface). >>> >>> Then couldn't it be done that video node ioctls invoke pipeline >>> operations, and the media device resolves any dependencies/calls >>> order, as in case of the exynos4 driver ? >> >> >> On Exynos4 subdevs, it is well and good since all the subdevs are >> independent IPs. Here in ISP since the same IP can take one input and > > > Not really, there are going to be 2 subdevs exposed by the fimc-is: ISP > and FD. However FD is still not supported in my last patch series. I was > planning this for a subsequent kernel release. > > >> provide multiple outputs, we designed them as separate subdevs. So >> here we cannot make the subdevs independent of each other where only >> the sequence / dependencies is controlled from the media device. > > > I'm not asking you to make the FIMC-IS subdevs more self-contained, > it's of course perfectly fine to have multiple (logical) subdevs exposed > by a complex device like that. I have been thinking only about the > sensor driver, since the sensors are normally shared across ISPs from > various chip manufacturers. But let us leave this topic for now. > > BTW, in my interpretation FIMC-IS is a collection of IPs/peripheral > devices, not a single black box, including an image sensor. And the > firmware should not be the most significant factor how we expose > the whole subsystem to the user. All elements of the ISP chain, i.e. > an IP control registers are visible to both, the main CPU and the > FIMC-IS (Cortex-A5) MCU. Still, it would be possible to create v4l2 > subdevs dynamically, depending on the firmware architecture. > Ok. I will address all these comments and work on v2 patchset. Thanks for the excellent help :) Regards Arun -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/media/soc/exynos5-is.txt b/Documentation/devicetree/bindings/media/soc/exynos5-is.txt new file mode 100644 index 0000000..e0fdf02 --- /dev/null +++ b/Documentation/devicetree/bindings/media/soc/exynos5-is.txt @@ -0,0 +1,81 @@ +Samsung EXYNOS SoC Camera Subsystem +----------------------------------- + +The camera subsystem on Samsung Exynos5 SoC has some changes relative +to previous SoC versions. Exynos5 has almost similar MIPI-CSIS and +FIMC-LITE IPs but removed the FIMC-CAPTURE. Instead it has an improved +FIMC-IS which can provide imate data DMA output. + +The device tree binding remain similar to the Exynos4 bindings which can +be seen at samsung-fimc.txt with the addition of fimc-is sub-node which will +be explained here. + +fimc-is subnode of camera node +------------------------------ + +Required properties: + +- compatible : must be "samsung,exynos5250-fimc-is" +- reg : physical base address and size of the memory mapped + registers +- interrupt-parent : Parent interrupt controller +- interrupts : fimc-is interrupt to the parent combiner + +Board specific properties: + +- pinctrl-names : pinctrl names for camera port pinmux control, at least + "default" needs to be specified. +- pinctrl-0...N : pinctrl properties corresponding to pinctrl-names + +Sensor sub-nodes: + +FIMC-IS IP supports custom built sensors to be controlled exclusively by +the FIMC-IS firmware. These sensor properties are to be defined here. +Sensor nodes are described in the same way as in generic sensors used in +Exynos4 and described in samsung-fimc.txt. + +Example: + +SoC common node: + + fimc_is: fimc-is@13000000 { + compatible = "samsung,exynos5250-fimc-is"; + reg = <0x13000000 0x200000>; + interrupt-parent = <&combiner>; + interrupts = <19 1>; + status = "disabled"; + }; + +Board specific node: + + fimc-is@13000000 { + status = "okay"; + pinctrl-0 = + <&cam_port_a_clk_active + &cam_bayer_clk_active + &isp_uart + &cam_i2c0 + &cam_i2c1>; + pinctrl-names = "default"; + s5k4e5 { + compatible = "samsung,s5k4e5"; + gpios = <&gpx1 2 1>; + clock-frequency = <24000000>; + port { + is_s5k4e5_ep: endpoint { + remote-endpoint = <&csis0_ep>; + }; + }; + }; + s5k6a3 { + compatible = "samsung,s5k6a3"; + gpios = <&gpx1 0 1>; + clock-frequency = <24000000>; + port { + is_s5k6a3_ep: endpoint { + remote-endpoint = <&csis1_ep>; + }; + }; + }; + }; + diff --git a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi index 3caaa21..320c22b 100644 --- a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi +++ b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi @@ -556,6 +556,38 @@ }; pinctrl@13400000 { + gpe1: gpe1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf0: gpf0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf1: gpf1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpg2: gpg2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + gph0: gph0 { gpio-controller; #gpio-cells = <2>; @@ -594,6 +626,34 @@ samsung,pin-pud = <0>; samsung,pin-drv = <0>; }; + + cam_bayer_clk_active: cam-bayer-clk-active { + samsung,pins = "gpg2-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + + isp_uart: isp-uart { + samsung,pins = "gpe1-0", "gpe1-1"; + samsung,pin-function = <3>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + + cam_i2c0: cam-i2c0 { + samsung,pins = "gpf0-0", "gpf0-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + + cam_i2c1: cam-i2c1 { + samsung,pins = "gpf0-2", "gpf0-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; }; pinctrl_3: pinctrl@03680000 { diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts old mode 100644 new mode 100755 index 356e014..0a2da64 --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts @@ -92,6 +92,7 @@ m5mols@1f { compatible = "fujitsu,m-5mols"; + status = "disabled"; reg = <0x1F>; gpios = <&gpx3 3 0xf>, <&gpx1 2 1>; clock-frequency = <24000000>; @@ -242,7 +243,7 @@ csis_0: csis@13C20000 { status = "okay"; - clock-frequency = <166000000>; + clock-frequency = <267000000>; /* s5k4e5 */ #address-cells = <1>; #size-cells = <0>; @@ -250,7 +251,7 @@ port@3 { reg = <3>; csis0_ep: endpoint { - remote-endpoint = <&m5mols_ep>; + remote-endpoint = <&is_s5k4e5_ep>; data-lanes = <1 2 3 4>; samsung,csis-hs-settle = <12>; samsung,csis-wclk; @@ -258,5 +259,54 @@ }; }; + csis_1: csis@13C30000 { + status = "okay"; + clock-frequency = <160000000>; + #address-cells = <1>; + #size-cells = <0>; + + /* Camera D (4) MIPI CSI-2 (CSIS1) */ + port@4 { + reg = <4>; + csis1_ep: endpoint { + remote-endpoint = <&is_s5k6a3_ep>; + data-lanes = <1>; + samsung,csis-hs-settle = <18>; + samsung,csis-wclk; + }; + }; + }; + + fimc-is@13000000 { + status = "okay"; + pinctrl-0 = + <&cam_port_a_clk_active + &cam_bayer_clk_active + &isp_uart + &cam_i2c0 + &cam_i2c1>; + pinctrl-names = "default"; + s5k4e5 { + compatible = "samsung,s5k4e5"; + gpios = <&gpx1 2 1>; + clock-frequency = <24000000>; + port { + is_s5k4e5_ep: endpoint { + remote-endpoint = <&csis0_ep>; + }; + }; + }; + s5k6a3 { + compatible = "samsung,s5k6a3"; + gpios = <&gpx1 0 1>; + clock-frequency = <24000000>; + port { + is_s5k6a3_ep: endpoint { + remote-endpoint = <&csis1_ep>; + }; + }; + }; + }; + }; }; diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi old mode 100644 new mode 100755 index 564c05f..e18df49 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -410,5 +410,13 @@ bus-width = <2>; status = "disabled"; }; + + fimc_is: fimc-is@13000000 { + compatible = "samsung,exynos5250-fimc-is"; + reg = <0x13000000 0x200000>; + interrupt-parent = <&combiner>; + interrupts = <19 1>; + status = "disabled"; + }; }; };