Re: upstream unicam not returning any buffer

[Maxime Ripard <mripard@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 11106 bytes --]

Hi Dave,

On Tue, May 06, 2025 at 02:14:51PM +0100, Dave Stevenson wrote:
> On Tue, 6 May 2025 at 13:07, Maxime Ripard <mripard@redhat.com> wrote:
> > On Wed, Apr 30, 2025 at 12:09:51PM +0100, Dave Stevenson wrote:
> > > On Fri, 25 Apr 2025 at 08:05, Maxime Ripard <mripard@redhat.com> wrote:
> > > >
> > > > Hi Dave,
> > > >
> > > > On Thu, Apr 24, 2025 at 06:12:14PM +0100, Dave Stevenson wrote:
> > > > > Hi Maxime
> > > > >
> > > > > On Thu, 24 Apr 2025 at 15:58, 'Maxime Ripard' via kernel-list
> > > > > <kernel-list@raspberrypi.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I've been trying to port a v4l2 application from the downstream unicam
> > > > > > driver (with the 6.6 kernel) to upstream (6.13.11) kernel.
> > > > > >
> > > > > > While the application seemed to work ok with the downstream driver, the
> > > > > > same code will never get any buffer from the upstream unicode driver.
> > > > > > Either dqbuf will block forever (if it's blocking), or never return a
> > > > > > buffer (if it's non-blocking).
> > > > > >
> > > > > > The media-controller topology after the application configured it is:
> > > > > >
> > > > > > Media controller API version 6.13.11
> > > > > >
> > > > > > Media device information
> > > > > > ------------------------
> > > > > > driver          unicam
> > > > > > model           unicam
> > > > > > serial
> > > > > > bus info        platform:fe801000.csi
> > > > > > hw revision     0x0
> > > > > > driver version  6.13.11
> > > > > >
> > > > > > Device topology
> > > > > > - entity 1: unicam (3 pads, 3 links, 0 routes)
> > > > > >             type V4L2 subdev subtype Unknown flags 0
> > > > > >             device node name /dev/v4l-subdev0
> > > > > >         pad0: SINK
> > > > > >                 [stream:0 fmt:RGB888_1X24/1280x720 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> > > > > >                 <- "tc358743 5-000f":0 [ENABLED,IMMUTABLE]
> > > > > >         pad1: SOURCE
> > > > > >                 [stream:0 fmt:RGB888_1X24/1280x720 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> > > > > >                 -> "unicam-image":0 [ENABLED,IMMUTABLE]
> > > > > >         pad2: SOURCE
> > > > > >                 -> "unicam-embedded":0 [ENABLED,IMMUTABLE]
> > > > > >
> > > > > > - entity 5: tc358743 5-000f (1 pad, 1 link, 0 routes)
> > > > > >             type V4L2 subdev subtype Unknown flags 0
> > > > > >             device node name /dev/v4l-subdev1
> > > > > >         pad0: SOURCE
> > > > > >                 [stream:0 fmt:RGB888_1X24/1280x720 field:none colorspace:srgb]
> > > > > >                 [dv.caps:BT.656/1120 min:640x350@13000000 max:1920x1200@165000000 stds:CEA-861,DMT,CVT,GTF caps:progressive,reduced-blanking,custom]
> > > > > >                 [dv.detect:BT.656/1120 1280x720p60 (1650x750) stds: flags:]
> > > > > >                 [dv.current:BT.656/1120 1280x720p60 (1650x750) stds: flags:]
> > > > > >                 -> "unicam":0 [ENABLED,IMMUTABLE]
> > > > > >
> > > > > > - entity 9: unicam-image (1 pad, 1 link)
> > > > > >             type Node subtype V4L flags 1
> > > > > >             device node name /dev/video0
> > > > > >         pad0: SINK
> > > > > >                 <- "unicam":1 [ENABLED,IMMUTABLE]
> > > > > >
> > > > > > - entity 15: unicam-embedded (1 pad, 1 link)
> > > > > >              type Node subtype V4L flags 0
> > > > > >              device node name /dev/video1
> > > > > >         pad0: SINK
> > > > > >                 <- "unicam":2 [ENABLED,IMMUTABLE]
> > > > > >
> > > > > > The interrupt count in /proc/interrupts increases with a somewhat
> > > > > > reasonable rate, so it looks like I'm getting interrupts.
> > > > > >
> > > > > > Enabling the debug output of the unicam driver gives the following logs:
> > > > > >
> > > > > > [ 2522.686656] unicam fe801000.csi: Starting stream on image device
> > > > > > [ 2522.687230] unicam fe801000.csi: Running with 2 data lanes, nodes 1
> > > > > > [ 2522.847079] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BE, sequence 0, lines done 0
> > > > > > [ 2522.847105] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2522.914545] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BC, sequence 0, lines done 819
> > > > > > [ 2522.914569] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2522.947477] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BE, sequence 0, lines done 1638
> > > > > > [ 2522.947499] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2522.964145] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BC, sequence 0, lines done 2457
> > > > > > [ 2522.964167] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2522.997077] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BE, sequence 0, lines done 3276
> > > > > > [ 2522.997100] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2523.013744] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BC, sequence 0, lines done 1059226
> > > > > > [ 2523.013764] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2523.064544] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BE, sequence 0, lines done 1059226
> > > > > > [ 2523.064566] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2523.081210] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BC, sequence 0, lines done 1059226
> > > > > > [ 2523.081230] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2523.130810] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BE, sequence 0, lines done 1059226
> > > > > > [ 2523.130833] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > > [ 2523.247880] unicam fe801000.csi: ISR: ISTA: 0x1, STA: 0x50BC, sequence 0, lines done 1059226
> > > > > > [ 2523.247902] unicam fe801000.csi: Scheduling dummy buffer for node 0
> > > > > >
> > > > > > Do you know what could be going on? I tried to compare the upstream and
> > > > > > downstream unicam drivers, and the interrupt handler seems to be
> > > > > > slightly different, but I'm afraid I'm a bit out of my depth there.
> > > > >
> > > > > I must admit to not having run the upstream driver in anger.
> > > > >
> > > > > Your interrupt traces are odd for at least two reasons.
> > > > >
> > > > > ISTA always being 0x1 means only frame start events. No frame end
> > > > > (0x2) or line count interrupts (0x4).
> > > > > You should be getting 4 line count interrupts every frame or every 128
> > > > > lines if greater, and frame end at the end of each frame. Your
> > > > > interrupts are every 16.6ms (ish), which would follow with being FS
> > > > > only.
> > > > >
> > > > > "lines done" is incrementing by more than the 720 lines expected for
> > > > > the configured resolution. I suspect that it has updated the write
> > > > > address to the new buffer, and they just happen to be contiguous (at
> > > > > least to start with).
> > > > > Downstream has code that (incorrectly) returns the buffer to the
> > > > > internal queue if we get repeated FS events, but it probably wants to
> > > > > return them to userspace with VB2_BUF_STATE_ERROR.
> > > > >
> > > > > I'll try to find a few minutes to battle the upstream driver with
> > > > > tc358743 and see what I get.
> > > >
> > > > I guess if it was RGB, it could be a vsync polarity issue then and
> > > > probably a DT issue, but I can't recall something similar for CSI.
> > > >
> > > > The DT overlay I've been using is:
> > > >
> > > > #include <dt-bindings/media/video-interfaces.h>
> > > >
> > > > /dts-v1/;
> > > > /plugin/;
> > > >
> > > > &csi1 {
> > > >         status = "okay";
> > > >
> > > >         port {
> > > >                 csi1_ep: endpoint {
> > > >                         remote-endpoint = <&tc358743_ep>;
> > > >                         bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
> > > >                         clock-noncontinuous;
> > > >                         data-lanes = <1 2>;
> > > >                 };
> > > >         };
> > > > };
> > > >
> > > > &i2c0 {
> > > >         status = "okay";
> > > > };
> > > >
> > > > &i2c0mux {
> > > >         status = "okay";
> > > > };
> > > >
> > > > &i2c0_1 {
> > > >         #address-cells = <1>;
> > > >         #size-cells = <0>;
> > > >         status = "okay";
> > > >
> > > >         tc358743@f {
> > > >                 compatible = "toshiba,tc358743";
> > > >                 reg = <0x0f>;
> > > >                 clock-names = "refclk";
> > > >                 clocks = <&clk_27MHz>;
> > > >
> > > >                 port {
> > > >                         tc358743_ep: endpoint {
> > > >                                 remote-endpoint = <&csi1_ep>;
> > > >                                 clock-noncontinuous;
> > > >                                 clock-lanes = <0>;
> > > >                                 data-lanes = <1 2>;
> > > >                                 link-frequencies = /bits/ 64 <486000000>;
> > >
> > > As just relayed to you via IM, this looks to be the problem.
> > > The mainline driver for tc358743 only supports a link freq of 297MHz,
> > > and will log "untested bps per lane: X bps" if you ask for something
> > > different. The PLL will be set for 486MHz, but the PHY settings remain
> > > those for 297MHz.
> > >
> > > The Raspberry Pi kernel has a couple of patches that add support for
> > > 486MHz / 972Mbit/s as that is then sufficient bandwidth to get 1080P30
> > > RGB or 1080P50 UYVY over 2 data lanes. I did try upstreaming them ages
> > > back, but memory says it sank in needing to verify FIFO fill level
> > > register settings, which is nigh impossible to prove with the data
> > > that Toshiba provide.
> > >
> > > Quick calcs say 2 lanes @ 297MHz / 594Mbit/s is enough for 720p60
> > > UYVY, but won't allow 720p60 RGB.
> > > 4 lanes @297MHz can do 1080p60 UYVY, but not RGB.
> >
> > You're right, thanks for the pointer to the patches in the RPi repo to
> > enable the higher data rates, that was the culprit.
> >
> > Should we send them upstream?
> 
> I did back in 2017, and my memory was right that it the FIFO level
> change that was the main stumbling block -
> https://lore.kernel.org/all/cover.1505826082.git.dave.stevenson@raspberrypi.org/
> Use of g_mbus_config has already been resolved.
>
> I haven't checked if it's possible to support the faster link
> frequency without the FIFO level change. Gut feel is that it will fail
> as the FIFO will underflow due to starting to empty too soon.
> It may be more acceptable to set the FIFO level based on the link
> frequency. I don't believe I have sufficient information to make it
> fully dynamic as Mauro had suggested.

If one of the blocker is gone, and the other is (close to) impossible to
solve right now, it might be worth resending the patches, acknowledging
that another, better, solution might be on the table in the future?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]