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: <20221222100341.5882c19c@booty>
Date:   Thu, 22 Dec 2022 10:03:41 +0100
From:   Luca Ceresoli <luca.ceresoli@...tlin.com>
To:     Dmitry Osipenko <digetx@...il.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

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?

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com