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-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html