Hi Hans, On Wed, Apr 05, 2023 at 10:50:37AM +0200, Hans Verkuil wrote: > On 05/04/2023 10:31, Luca Ceresoli wrote: > > On Wed, 5 Apr 2023 05:30:48 +0300 Laurent Pinchart wrote: > >> On Tue, Apr 04, 2023 at 04:12:51PM +0200, Luca Ceresoli wrote: > >>> On Wed, 29 Mar 2023 13:16:22 +0200 Hans Verkuil wrote: > >>> > >>>> Hi Luca, > >>>> > >>>> I finally found the time to test this series. It looks OK, except for this patch. > >>> > >>> Thank you very much for taking the time! > >>> > >>>> The list of supported formats really has to be the intersection of what the tegra > >>>> supports and what the sensor supports. > >>>> > >>>> Otherwise you would advertise pixelformats that cannot be used, and the application > >>>> would have no way of knowing that. > >>> > >>> As far as I understand, I think we should rather make this driver fully > >>> behave as an MC-centric device. It is already using MC quite > >>> successfully after all. > >>> > >>> Do you think this is correct? > >> > >> Given the use cases for this driver, I agree. > > I disagree. > > This driver doesn't use the media controller for anything at the moment. The > /dev/mediaX device just shows the internal topology (i.e. connected sensors), > but otherwise it does nothing. > > While it would be great if we could unlock the ISP on the Tegra, the reality > is that it is entirely closed source and can't be used in a linux driver, and > that's not going to change, sadly. Never say never :-) > That leaves us with just a basic CSI capture driver. Rather than trying to > change this driver to a full MC device with no benefits, just drop this change > and get your code in. Can't the hardware support capturing different virtual channels or data types from the same CSI-2 source ? That would require MC support, the stream API requires subdev device nodes. > Note that this driver will stay in staging since it still fails when I try to > capture from two sensors at the same time: syncpoint errors start appearing > in that case. I think there are locking issues. I think I have someone to take > a look at that, but first I want your series to get merged. > > In the very unlikely event that the ISP can be implemented in a linux driver, > it will probably become a new driver. > > Regards, > > > Ok, thanks for the feedback. I will send a v5 with this change. -- Regards, Laurent Pinchart
