22.12.2022 12:03, Luca Ceresoli пишет: > Hello Dmitry, > > On Wed, 21 Dec 2022 00:40:20 +0300 > Dmitry Osipenko <digetx@xxxxxxxxx> wrote: > >> 28.11.2022 18:23, Luca Ceresoli пишет: >>> +static int tegra20_channel_capture_frame(struct tegra_vi_channel *chan, >>> + struct tegra_channel_buffer *buf) >>> +{ >>> + u32 value; >>> + int err; >>> + >>> + chan->next_out_sp_idx++; >>> + >>> + tegra20_channel_vi_buffer_setup(chan, buf); >>> + >>> + tegra20_vi_write(chan, TEGRA_VI_CAMERA_CONTROL, VI_CAMERA_CONTROL_VIP_ENABLE); >>> + >>> + /* Wait for syncpt counter to reach frame start event threshold */ >>> + err = host1x_syncpt_wait(chan->out_sp, chan->next_out_sp_idx, >>> + TEGRA_VI_SYNCPT_WAIT_TIMEOUT, &value); >> >> You're not using the "value" variable, it should be NULL. > > Ah, sure, good catch. > >> The "chan->out_sp" looks redundant, it duplicates the chan->mw_ack_sp. > > I agree it is redundant and can be improved. > >> AFAICS from the doc, T20 has two VI channels, and thus, two mw_ack_sp, >> like T210. > > I'm confused by this. In the current driver, each VI channel has an > array of 2 mw_ack_sp, the second of which is only used the ganged > CSI ports. I have no docs mentioning ganged ports so I don't know > exactly how they work and whether T20 might need more than 1 syncpt per > channel or not for CSI. Definitely when using VIP only one such syncpt > per each VI (or per each VIP, as per your reply to patch 1) is needed. > > Bottom line: I think I can simply remove the out_sp and in the VIP code > always use chan->mw_ack_sp[0], and document that it's what is called OUT > in VIP terms. > > Does this plan seem good? Older Tegra VI doesn't have ganged ports, but two memory/CSI channels. It feels to me that Tegra VI can capture both channels independently, though downstream driver stack used only one of the channels, IIRC. There is a VI header file from nvddk in downstream kernel, which is pretty much the doc by itself. https://nv-tegra.nvidia.com/r/gitweb?p=linux-2.6.git;a=blob;f=arch/arm/mach-tegra/include/ap20/arvi.h;h=6ce52e8e9a7213e33466d34a71cf3af2b6944b8a;