linux-kernel - Re: [PATCH v2 21/21] staging: media: tegra-video: add tegra20 variant

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a64e2aea-2e06-a72e-5ced-a86de4458c61@gmail.com>
Date:   Fri, 23 Dec 2022 15:15:29 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc:     David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Sowjanya Komatineni <skomatineni@...dia.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        linux-media@...r.kernel.org, linux-tegra@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-staging@...ts.linux.dev,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
        Richard Leitner <richard.leitner@...data.com>
Subject: Re: [PATCH v2 21/21] staging: media: tegra-video: add tegra20 variant

22.12.2022 12:03, Luca Ceresoli пишет:
> Hello Dmitry,
> 
> On Wed, 21 Dec 2022 00:40:20 +0300
> Dmitry Osipenko <digetx@...il.com> 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;