| Message ID | 20221128152336.133953-1-luca.ceresoli@bootlin.com |
|---|---|
| Headers | show |
| Series | Add Tegra20 parallel video input capture | expand |
On Mon, Nov 28, 2022 at 04:23:16PM +0100, Luca Ceresoli wrote: > VIP is the parallel video capture component within the video input > subsystem of Tegra20 (and other Tegra chips, apparently). > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > --- > > Changed in v2 (suggested by Krzysztof Kozlowski): > - remove redundant "bindings" from subject line > - remove $nodename > - add channel@0 description > - add reg: const: 0 > --- > .../display/tegra/nvidia,tegra20-vip.yaml | 63 +++++++++++++++++++ > MAINTAINERS | 7 +++ > 2 files changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > new file mode 100644 > index 000000000000..44be2e16c9b4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > @@ -0,0 +1,63 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NVIDIA Tegra VIP (parallel video capture) controller > + > +maintainers: > + - Luca Ceresoli <luca.ceresoli@bootlin.com> > + > +properties: > + compatible: > + enum: > + - nvidia,tegra20-vip > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + channel@0: Kind of odd there is only 1 channel with a unit-address. Are more channels coming? Please make the binding as complete as possible even if no driver support yet. > + description: parallel video capture interface for the VI > + type: object > + > + properties: > + reg: > + const: 0 > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + Port receiving the video stream from the sensor > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + Port sending the video stream to the VI > + > + required: > + - port@0 > + - port@1 > + > + additionalProperties: false A bit easier to read the indented cases if this is above 'properties'. > + > + required: > + - reg > + - ports > + > +unevaluatedProperties: false > + > +required: > + - compatible > + - "#address-cells" > + - "#size-cells" > + - channel@0 > + > +# see nvidia,tegra20-vi.yaml for an example > diff --git a/MAINTAINERS b/MAINTAINERS > index 69565ac0c224..92c762f85f17 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20315,6 +20315,13 @@ S: Maintained > F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml > F: drivers/staging/media/tegra-video/ > > +TEGRA VIDEO DRIVER FOR TEGRA20 VIP (PARALLEL VIDEO CAPTURE) > +M: Luca Ceresoli <luca.ceresoli@bootlin.com> > +L: linux-media@vger.kernel.org > +L: linux-tegra@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > + > TEGRA XUSB PADCTL DRIVER > M: JC Kuo <jckuo@nvidia.com> > S: Supported > -- > 2.34.1 > >
Hello Rob, Thanks for your review. On Thu, 1 Dec 2022 17:19:36 -0600 Rob Herring <robh@kernel.org> wrote: > On Mon, Nov 28, 2022 at 04:23:16PM +0100, Luca Ceresoli wrote: > > VIP is the parallel video capture component within the video input > > subsystem of Tegra20 (and other Tegra chips, apparently). > > > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > > > --- > > > > Changed in v2 (suggested by Krzysztof Kozlowski): > > - remove redundant "bindings" from subject line > > - remove $nodename > > - add channel@0 description > > - add reg: const: 0 > > --- > > .../display/tegra/nvidia,tegra20-vip.yaml | 63 +++++++++++++++++++ > > MAINTAINERS | 7 +++ > > 2 files changed, 70 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > > > > diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > > new file mode 100644 > > index 000000000000..44be2e16c9b4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml > > @@ -0,0 +1,63 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/tegra/nvidia,tegra20-vip.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: NVIDIA Tegra VIP (parallel video capture) controller > > + > > +maintainers: > > + - Luca Ceresoli <luca.ceresoli@bootlin.com> > > + > > +properties: > > + compatible: > > + enum: > > + - nvidia,tegra20-vip > > + > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > + channel@0: > > Kind of odd there is only 1 channel with a unit-address. Are more > channels coming? Please make the binding as complete as possible even if > no driver support yet. This was discussed in v1 with Krzysztof and the outcome was that it's OK because it's likely that other SoCs have more, but the documentation is not public so I cannot add examples. Full discussion (pretty short indeed): https://lore.kernel.org/linux-devicetree/5292cc1b-c951-c5c5-b2ef-c154baf6d7fd@linaro.org/ Do you agree that the unit-address should be kept? > > + description: parallel video capture interface for the VI > > + type: object > > + > > + properties: > > + reg: > > + const: 0 > > + > > + ports: > > + $ref: /schemas/graph.yaml#/properties/ports > > + > > + properties: > > + port@0: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + Port receiving the video stream from the sensor > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: > > + Port sending the video stream to the VI > > + > > + required: > > + - port@0 > > + - port@1 > > + > > + additionalProperties: false > > A bit easier to read the indented cases if this is above 'properties'. Sure, will do in v3.
28.11.2022 18:23, Luca Ceresoli пишет: > Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can > receive from either MIPI CSI-2 or parallel video (called respectively "CSI" > and "VIP" in the documentation). The kernel currently has a staging driver > for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP > capture. > > Unfortunately I had no real documentation available to base this work on. > I only had a working downstream 3.1 kernel, so I started with the driver > found there and heavily reworked it to fit into the mainline tegra-video > driver structure. The existing code appears written with the intent of > being modular and allow adding new input mechanisms and new SoCs while > keeping a unique VI core module. However its modularity and extensibility > was not enough to add Tegra20 VIP support, so I added some hooks to turn > hard-coded behaviour into per-SoC or per-bus customizable code. There are > also a fix, some generic cleanups and DT bindings. > > Quick tour of the patches: > > * Device tree bindings and minor DTS improvements > > 01. dt-bindings: display: tegra: add Tegra20 VIP > 02. dt-bindings: display: tegra: vi: add 'vip' property and example This series adds the new DT node, but there are no board DTs in upstream that will use VIP? Will we see the board patches? In any case, given that you're likely the only one here who has access to hardware with VIP, you should promote yourself to the tegra-video driver maintainers and confirm that you will be able to maintain and test this code for years to come.
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. The "chan->out_sp" looks redundant, it duplicates the chan->mw_ack_sp. AFAICS from the doc, T20 has two VI channels, and thus, two mw_ack_sp, like T210.
Hello Dmitry, thanks for your review. On Tue, 20 Dec 2022 23:21:49 +0300 Dmitry Osipenko <digetx@gmail.com> wrote: > 28.11.2022 18:23, Luca Ceresoli пишет: > > Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can > > receive from either MIPI CSI-2 or parallel video (called respectively "CSI" > > and "VIP" in the documentation). The kernel currently has a staging driver > > for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP > > capture. > > > > Unfortunately I had no real documentation available to base this work on. > > I only had a working downstream 3.1 kernel, so I started with the driver > > found there and heavily reworked it to fit into the mainline tegra-video > > driver structure. The existing code appears written with the intent of > > being modular and allow adding new input mechanisms and new SoCs while > > keeping a unique VI core module. However its modularity and extensibility > > was not enough to add Tegra20 VIP support, so I added some hooks to turn > > hard-coded behaviour into per-SoC or per-bus customizable code. There are > > also a fix, some generic cleanups and DT bindings. > > > > Quick tour of the patches: > > > > * Device tree bindings and minor DTS improvements > > > > 01. dt-bindings: display: tegra: add Tegra20 VIP > > 02. dt-bindings: display: tegra: vi: add 'vip' property and example > > This series adds the new DT node, but there are no board DTs in upstream > that will use VIP? Will we see the board patches? I'm afraid I have no such plan. I don't have any public hardware with Tegra20, with or without a parallel sensor. I have a custom board. > In any case, given that you're likely the only one here who has access > to hardware with VIP, Likely indeed. > you should promote yourself to the tegra-video > driver maintainers and confirm that you will be able to maintain and > test this code for years to come. I can definitely add myself as a maintainer of this driver and join the maintenance effort, I'm adding that in v3. I also have a board that I can permanently use for testing.
Hello Dmitry, On Wed, 21 Dec 2022 00:40:20 +0300 Dmitry Osipenko <digetx@gmail.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?
22.12.2022 12:03, Luca Ceresoli пишет: > Hello Dmitry, > > On Wed, 21 Dec 2022 00:40:20 +0300 > Dmitry Osipenko <digetx@gmail.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;
23.12.2022 15:15, Dmitry Osipenko пишет: > 22.12.2022 12:03, Luca Ceresoli пишет: >> Hello Dmitry, >> >> On Wed, 21 Dec 2022 00:40:20 +0300 >> Dmitry Osipenko <digetx@gmail.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; Although, after a bit closer look, I see that there is only one port selector there. Hence there only one port can be active at a time.
28.11.2022 18:23, Luca Ceresoli пишет: > +/* -------------------------------------------------------------------------- > + * VIP > + */ > + > +/* > + * VIP-specific configuration for stream start. > + * > + * Whatever is common among VIP and CSI is done by the VI component (see > + * tegra20_vi_start_streaming()). Here we do what is VIP-specific. > + */ > +static int tegra20_vip_start_streaming(struct tegra_vip_channel *vip_chan) > +{ > + struct tegra_vi_channel *vi_chan = v4l2_get_subdev_hostdata(&vip_chan->subdev); > + int width = vi_chan->format.width; > + int height = vi_chan->format.height; > + > + unsigned int main_input_format; > + unsigned int yuv_input_format; > + > + tegra20_vi_get_input_formats(vi_chan, &main_input_format, &yuv_input_format); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_VI_CORE_CONTROL, 0); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_VI_INPUT_CONTROL, > + VI_INPUT_VIP_INPUT_ENABLE | main_input_format | yuv_input_format); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_V_DOWNSCALE_CONTROL, 0); > + tegra20_vi_write(vi_chan, TEGRA_VI_H_DOWNSCALE_CONTROL, 0); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_VIP_V_ACTIVE, height << VI_VIP_V_ACTIVE_PERIOD_SFT); > + tegra20_vi_write(vi_chan, TEGRA_VI_VIP_H_ACTIVE, > + roundup(width, 2) << VI_VIP_H_ACTIVE_PERIOD_SFT); > + > + /* > + * For VIP, D9..D2 is mapped to the video decoder's P7..P0. > + * Disable/mask out the other Dn wires. When not in BT656 > + * mode we also need the V/H sync. > + */ > + tegra20_vi_write(vi_chan, TEGRA_VI_PIN_INPUT_ENABLE, > + GENMASK(9, 2) << VI_PIN_INPUT_VD_SFT | > + VI_PIN_INPUT_HSYNC | VI_PIN_INPUT_VSYNC); > + tegra20_vi_write(vi_chan, TEGRA_VI_VI_DATA_INPUT_CONTROL, > + GENMASK(9, 2) << VI_DATA_INPUT_SFT); > + tegra20_vi_write(vi_chan, TEGRA_VI_PIN_INVERSION, 0); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_CONT_SYNCPT_OUT_1, > + VI_CONT_SYNCPT_OUT_1_CONTINUOUS_SYNCPT | > + host1x_syncpt_id(vi_chan->out_sp) << VI_CONT_SYNCPT_OUT_1_SYNCPT_IDX_SFT); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_CAMERA_CONTROL, VI_CAMERA_CONTROL_STOP_CAPTURE); > + > + return 0; > +} > + > +static const struct tegra_vip_ops tegra20_vip_ops = { > + .vip_start_streaming = tegra20_vip_start_streaming, > +}; > + > +const struct tegra_vip_soc tegra20_vip_soc = { > + .ops = &tegra20_vip_ops, > +}; Shouldn't this be placed in vip.c? Also looks like patch #20 won't link because tegra20_vip_soc is defined in patch #21.
28.11.2022 18:23, Luca Ceresoli пишет: > +static int tegra20_vip_start_streaming(struct tegra_vip_channel *vip_chan) > +{ > + struct tegra_vi_channel *vi_chan = v4l2_get_subdev_hostdata(&vip_chan->subdev); > + int width = vi_chan->format.width; > + int height = vi_chan->format.height; > + > + unsigned int main_input_format; > + unsigned int yuv_input_format; > + > + tegra20_vi_get_input_formats(vi_chan, &main_input_format, &yuv_input_format); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_VI_CORE_CONTROL, 0); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_VI_INPUT_CONTROL, > + VI_INPUT_VIP_INPUT_ENABLE | main_input_format | yuv_input_format); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_V_DOWNSCALE_CONTROL, 0); > + tegra20_vi_write(vi_chan, TEGRA_VI_H_DOWNSCALE_CONTROL, 0); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_VIP_V_ACTIVE, height << VI_VIP_V_ACTIVE_PERIOD_SFT); > + tegra20_vi_write(vi_chan, TEGRA_VI_VIP_H_ACTIVE, > + roundup(width, 2) << VI_VIP_H_ACTIVE_PERIOD_SFT); > + > + /* > + * For VIP, D9..D2 is mapped to the video decoder's P7..P0. > + * Disable/mask out the other Dn wires. When not in BT656 > + * mode we also need the V/H sync. > + */ > + tegra20_vi_write(vi_chan, TEGRA_VI_PIN_INPUT_ENABLE, > + GENMASK(9, 2) << VI_PIN_INPUT_VD_SFT | > + VI_PIN_INPUT_HSYNC | VI_PIN_INPUT_VSYNC); > + tegra20_vi_write(vi_chan, TEGRA_VI_VI_DATA_INPUT_CONTROL, > + GENMASK(9, 2) << VI_DATA_INPUT_SFT); > + tegra20_vi_write(vi_chan, TEGRA_VI_PIN_INVERSION, 0); > + > + tegra20_vi_write(vi_chan, TEGRA_VI_CONT_SYNCPT_OUT_1, > + VI_CONT_SYNCPT_OUT_1_CONTINUOUS_SYNCPT | > + host1x_syncpt_id(vi_chan->out_sp) << VI_CONT_SYNCPT_OUT_1_SYNCPT_IDX_SFT); Wondering whether you also need to set up the sypct_incr condition to op_done, it should be immediate by default. I see that T210 VI code sets up the incr condition.
23.12.2022 16:02, Dmitry Osipenko пишет: > 28.11.2022 18:23, Luca Ceresoli пишет: >> +static int tegra20_vip_start_streaming(struct tegra_vip_channel *vip_chan) >> +{ >> + struct tegra_vi_channel *vi_chan = v4l2_get_subdev_hostdata(&vip_chan->subdev); >> + int width = vi_chan->format.width; >> + int height = vi_chan->format.height; >> + >> + unsigned int main_input_format; >> + unsigned int yuv_input_format; >> + >> + tegra20_vi_get_input_formats(vi_chan, &main_input_format, &yuv_input_format); >> + >> + tegra20_vi_write(vi_chan, TEGRA_VI_VI_CORE_CONTROL, 0); >> + >> + tegra20_vi_write(vi_chan, TEGRA_VI_VI_INPUT_CONTROL, >> + VI_INPUT_VIP_INPUT_ENABLE | main_input_format | yuv_input_format); >> + >> + tegra20_vi_write(vi_chan, TEGRA_VI_V_DOWNSCALE_CONTROL, 0); >> + tegra20_vi_write(vi_chan, TEGRA_VI_H_DOWNSCALE_CONTROL, 0); >> + >> + tegra20_vi_write(vi_chan, TEGRA_VI_VIP_V_ACTIVE, height << VI_VIP_V_ACTIVE_PERIOD_SFT); >> + tegra20_vi_write(vi_chan, TEGRA_VI_VIP_H_ACTIVE, >> + roundup(width, 2) << VI_VIP_H_ACTIVE_PERIOD_SFT); >> + >> + /* >> + * For VIP, D9..D2 is mapped to the video decoder's P7..P0. >> + * Disable/mask out the other Dn wires. When not in BT656 >> + * mode we also need the V/H sync. >> + */ >> + tegra20_vi_write(vi_chan, TEGRA_VI_PIN_INPUT_ENABLE, >> + GENMASK(9, 2) << VI_PIN_INPUT_VD_SFT | >> + VI_PIN_INPUT_HSYNC | VI_PIN_INPUT_VSYNC); >> + tegra20_vi_write(vi_chan, TEGRA_VI_VI_DATA_INPUT_CONTROL, >> + GENMASK(9, 2) << VI_DATA_INPUT_SFT); >> + tegra20_vi_write(vi_chan, TEGRA_VI_PIN_INVERSION, 0); >> + >> + tegra20_vi_write(vi_chan, TEGRA_VI_CONT_SYNCPT_OUT_1, >> + VI_CONT_SYNCPT_OUT_1_CONTINUOUS_SYNCPT | >> + host1x_syncpt_id(vi_chan->out_sp) << VI_CONT_SYNCPT_OUT_1_SYNCPT_IDX_SFT); > > Wondering whether you also need to set up the sypct_incr condition to > op_done, it should be immediate by default. I see that T210 VI code sets > up the incr condition. Found in the doc "Continuous syncpt always use OP_DONE as condition", the current code is fine.
Tegra20 and other Tegra SoCs have a video input (VI) peripheral that can receive from either MIPI CSI-2 or parallel video (called respectively "CSI" and "VIP" in the documentation). The kernel currently has a staging driver for Tegra210 CSI capture. This patch set adds support for Tegra20 VIP capture. Unfortunately I had no real documentation available to base this work on. I only had a working downstream 3.1 kernel, so I started with the driver found there and heavily reworked it to fit into the mainline tegra-video driver structure. The existing code appears written with the intent of being modular and allow adding new input mechanisms and new SoCs while keeping a unique VI core module. However its modularity and extensibility was not enough to add Tegra20 VIP support, so I added some hooks to turn hard-coded behaviour into per-SoC or per-bus customizable code. There are also a fix, some generic cleanups and DT bindings. Quick tour of the patches: * Device tree bindings and minor DTS improvements 01. dt-bindings: display: tegra: add Tegra20 VIP 02. dt-bindings: display: tegra: vi: add 'vip' property and example * A fix 03. staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return all formats * Minor improvements to logging, comments, cleanups 04. staging: media: tegra-video: improve documentation of tegra_video_format fields 05. staging: media: tegra-video: document tegra_channel_get_remote_source_subdev 06. staging: media: tegra-video: fix typos in comment 07. staging: media: tegra-video: improve error messages 08. staging: media: tegra-video: slightly simplify cleanup on errors 09. staging: media: tegra-video: move private struct declaration to C file 10. staging: media: tegra-video: remove unneeded include * Preparation to make the VI module generic enough to host Tegra20 and VIP 11. staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 12. staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op 13. staging: media: tegra-video: move default format to soc-specific data 14. staging: media: tegra-video: move MIPI calibration calls from VI to CSI 15. staging: media: tegra-video: add a per-soc enable/disable op 16. staging: media: tegra-video: move syncpt init/free to a per-soc op 17. staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi 18. staging: media: tegra-video: add hooks for planar YUV and H/V flip 19. staging: media: tegra-video: add H/V flip controls * Implementation of VIP and Tegra20 20. staging: media: tegra-video: add support for VIP (parallel video input) 21. staging: media: tegra-video: add tegra20 variant Enjoy! Changed in v2: - improved dt-bindings patches based on reviews - removed patches 3 and 4 adding DT labels without a mainline user - two small fixes to the last patch [v1] https://lore.kernel.org/linux-tegra/20221124155634.5bc2a423@booty/T/#t Luca Luca Ceresoli (21): dt-bindings: display: tegra: add Tegra20 VIP dt-bindings: display: tegra: vi: add 'vip' property and example staging: media: tegra-video: fix .vidioc_enum_fmt_vid_cap to return all formats staging: media: tegra-video: improve documentation of tegra_video_format fields staging: media: tegra-video: document tegra_channel_get_remote_source_subdev staging: media: tegra-video: fix typos in comment staging: media: tegra-video: improve error messages staging: media: tegra-video: slightly simplify cleanup on errors staging: media: tegra-video: move private struct declaration to C file staging: media: tegra-video: remove unneeded include staging: media: tegra-video: Kconfig: allow TPG only on Tegra210 staging: media: tegra-video: move tegra_channel_fmt_align to a per-soc op staging: media: tegra-video: move default format to soc-specific data staging: media: tegra-video: move MIPI calibration calls from VI to CSI staging: media: tegra-video: add a per-soc enable/disable op staging: media: tegra-video: move syncpt init/free to a per-soc op staging: media: tegra-video: add syncpts for Tegra20 to struct tegra_vi staging: media: tegra-video: add hooks for planar YUV and H/V flip staging: media: tegra-video: add H/V flip controls staging: media: tegra-video: add support for VIP (parallel video input) staging: media: tegra-video: add tegra20 variant .../display/tegra/nvidia,tegra20-vi.yaml | 68 ++ .../display/tegra/nvidia,tegra20-vip.yaml | 63 ++ MAINTAINERS | 10 + drivers/staging/media/tegra-video/Kconfig | 1 + drivers/staging/media/tegra-video/Makefile | 2 + drivers/staging/media/tegra-video/csi.c | 44 ++ drivers/staging/media/tegra-video/tegra20.c | 661 ++++++++++++++++++ drivers/staging/media/tegra-video/tegra210.c | 97 ++- drivers/staging/media/tegra-video/vi.c | 321 ++------- drivers/staging/media/tegra-video/vi.h | 76 +- drivers/staging/media/tegra-video/video.c | 5 + drivers/staging/media/tegra-video/video.h | 2 +- drivers/staging/media/tegra-video/vip.c | 298 ++++++++ drivers/staging/media/tegra-video/vip.h | 72 ++ 14 files changed, 1434 insertions(+), 286 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-vip.yaml create mode 100644 drivers/staging/media/tegra-video/tegra20.c create mode 100644 drivers/staging/media/tegra-video/vip.c create mode 100644 drivers/staging/media/tegra-video/vip.h