Re: [RESEND PATCH v4 03/21] staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return all formats

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: Luca Ceresoli <luca.ceresoli@bootlin.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Sowjanya Komatineni <skomatineni@nvidia.com>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Dmitry Osipenko <digetx@gmail.com>,
	linux-media@vger.kernel.org, linux-tegra@vger.kernel.org,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Richard Leitner <richard.leitner@skidata.com>
Subject: Re: [RESEND PATCH v4 03/21] staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return all formats
Date: Wed, 5 Apr 2023 10:50:37 +0200	[thread overview]
Message-ID: <dddd76a7-f882-f1dd-0781-fcc1f9b4e060@xs4all.nl> (raw)
In-Reply-To: <20230405103134.2ae10766@booty>

On 05/04/2023 10:31, Luca Ceresoli wrote:
> Hi Laurent,
> 
> On Wed, 5 Apr 2023 05:30:48 +0300
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> 
>> Hi Luca,
>>
>> 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.

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.

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,

	Hans

> 
> Ok, thanks for the feedback. I will send a v5 with this change.
> 
> Best regards,
> Luca
> 