mbox series

[0/3] NVIDIA Tegra NVDEC support

Message ID 20210213101512.3275069-1-mperttunen@nvidia.com
Headers show
Series NVIDIA Tegra NVDEC support | expand

Message

Mikko Perttunen Feb. 13, 2021, 10:15 a.m. UTC 
Hi all,

with the release of documentation headers for Tegra multimedia engines
(NVDEC, NVENC, NVJPG) [1], I have started working on the corresponding
implementations. Here's the first one, NVDEC.

The kernel driver is a simple Falcon boot driver based on the VIC
driver. Some code sharing should be considered there in the future.
The userspace driver to accompany this is a bit more complicated -
I have expanded vaapi-tegra-driver[2] to support MPEG2 decoding.
It should be noted that the implementation is still very clunky
and has poor performance, but it's a start.

This series is based on top of the "Host1x/TegraDRM UAPI" series.
For testing, appropriate firmware should be obtained from a
Linux for Tegra distribution for now; the GPU should also be
enabled in the device tree.

Series was tested on Tegra186.

Thanks!

Mikko

[1] https://github.com/NVIDIA/open-gpu-doc/tree/master/classes/video
[2] https://github.com/cyndis/vaapi-tegra-driver

Mikko Perttunen (3):
  dt-bindings: Add YAML bindings for Host1x and NVDEC
  arm64: tegra: Add NVDEC to Tegra186 device tree
  drm/tegra: Add NVDEC driver

 .../gpu/host1x/nvidia,tegra20-host1x.yaml     | 129 +++++
 .../gpu/host1x/nvidia,tegra210-nvdec.yaml     |  90 ++++
 MAINTAINERS                                   |   1 +
 arch/arm64/boot/dts/nvidia/tegra186.dtsi      |  15 +
 drivers/gpu/drm/tegra/Makefile                |   3 +-
 drivers/gpu/drm/tegra/drm.c                   |   4 +
 drivers/gpu/drm/tegra/drm.h                   |   1 +
 drivers/gpu/drm/tegra/nvdec.c                 | 497 ++++++++++++++++++
 drivers/gpu/host1x/dev.c                      |  12 +
 include/linux/host1x.h                        |   1 +
 10 files changed, 752 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
 create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
 create mode 100644 drivers/gpu/drm/tegra/nvdec.c

Comments

Jon Hunter Feb. 16, 2021, 3:16 p.m. UTC | #1 
On 13/02/2021 10:15, Mikko Perttunen wrote:
> Add support for booting and using NVDEC on Tegra210, Tegra186

> and Tegra194 to the Host1x and TegraDRM drivers. Booting in

> secure mode is not currently supported.

> 

> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>

> ---

>  drivers/gpu/drm/tegra/Makefile |   3 +-

>  drivers/gpu/drm/tegra/drm.c    |   4 +

>  drivers/gpu/drm/tegra/drm.h    |   1 +

>  drivers/gpu/drm/tegra/nvdec.c  | 497 +++++++++++++++++++++++++++++++++

>  drivers/gpu/host1x/dev.c       |  12 +

>  include/linux/host1x.h         |   1 +

>  6 files changed, 517 insertions(+), 1 deletion(-)

>  create mode 100644 drivers/gpu/drm/tegra/nvdec.c


...

> +static int nvdec_probe(struct platform_device *pdev)

> +{

> +	struct device *dev = &pdev->dev;

> +	struct host1x_syncpt **syncpts;

> +	struct resource *regs;

> +	struct nvdec *nvdec;

> +	int err;

> +

> +	/* inherit DMA mask from host1x parent */

> +	err = dma_coerce_mask_and_coherent(dev, *dev->parent->dma_mask);

> +	if (err < 0) {

> +		dev_err(&pdev->dev, "failed to set DMA mask: %d\n", err);

> +		return err;

> +	}

> +

> +	nvdec = devm_kzalloc(dev, sizeof(*nvdec), GFP_KERNEL);

> +	if (!nvdec)

> +		return -ENOMEM;

> +

> +	nvdec->config = of_device_get_match_data(dev);

> +

> +	syncpts = devm_kzalloc(dev, sizeof(*syncpts), GFP_KERNEL);

> +	if (!syncpts)

> +		return -ENOMEM;

> +

> +	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);

> +	if (!regs) {

> +		dev_err(&pdev->dev, "failed to get registers\n");

> +		return -ENXIO;

> +	}

> +

> +	nvdec->regs = devm_ioremap_resource(dev, regs);

> +	if (IS_ERR(nvdec->regs))

> +		return PTR_ERR(nvdec->regs);

> +


We should be able to use devm_platform_get_and_ioremap_resource() here.

> +	nvdec->clk = devm_clk_get(dev, NULL);

> +	if (IS_ERR(nvdec->clk)) {

> +		dev_err(&pdev->dev, "failed to get clock\n");

> +		return PTR_ERR(nvdec->clk);

> +	}

> +

> +	if (!dev->pm_domain) {


Looks like the power-domain is required by device-tree and so do we need
this?

> +		nvdec->rst = devm_reset_control_get(dev, "nvdec");

> +		if (IS_ERR(nvdec->rst)) {

> +			dev_err(&pdev->dev, "failed to get reset\n");

> +			return PTR_ERR(nvdec->rst);

> +		}

> +	}

> +

> +	nvdec->falcon.dev = dev;

> +	nvdec->falcon.regs = nvdec->regs;

> +

> +	err = falcon_init(&nvdec->falcon);

> +	if (err < 0)

> +		return err;

> +

> +	platform_set_drvdata(pdev, nvdec);

> +

> +	INIT_LIST_HEAD(&nvdec->client.base.list);

> +	nvdec->client.base.ops = &nvdec_client_ops;

> +	nvdec->client.base.dev = dev;

> +	nvdec->client.base.class = HOST1X_CLASS_NVDEC;

> +	nvdec->client.base.syncpts = syncpts;

> +	nvdec->client.base.num_syncpts = 1;

> +	nvdec->dev = dev;

> +

> +	INIT_LIST_HEAD(&nvdec->client.list);

> +	nvdec->client.version = nvdec->config->version;

> +	nvdec->client.ops = &nvdec_ops;

> +

> +	err = host1x_client_register(&nvdec->client.base);

> +	if (err < 0) {

> +		dev_err(dev, "failed to register host1x client: %d\n", err);

> +		goto exit_falcon;

> +	}

> +

> +	pm_runtime_enable(&pdev->dev);

> +	if (!pm_runtime_enabled(&pdev->dev)) {

> +		err = nvdec_runtime_resume(&pdev->dev);

> +		if (err < 0)

> +			goto unregister_client;

> +	}


pm_runtime should always be enabled for 64-bit Tegra and so we should
not need to check pm_runtime_enabled().

Cheers
Jon

-- 
nvpublic
Rob Herring Feb. 17, 2021, 8:24 p.m. UTC | #2 
On Sat, 13 Feb 2021 12:15:10 +0200, Mikko Perttunen wrote:
> Convert the original Host1x bindings to YAML and add new bindings for
> NVDEC, now in a more appropriate location. The old text bindings
> for Host1x and engines are still kept at display/tegra/ since they
> encompass a lot more engines that haven't been converted over yet.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  .../gpu/host1x/nvidia,tegra20-host1x.yaml     | 129 ++++++++++++++++++
>  .../gpu/host1x/nvidia,tegra210-nvdec.yaml     |  90 ++++++++++++
>  MAINTAINERS                                   |   1 +
>  3 files changed, 220 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
>  create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml:90:1: [warning] too many blank lines (2 > 1) (empty-lines)

dtschema/dtc warnings/errors:

See https://patchwork.ozlabs.org/patch/1440164

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Rob Herring Feb. 17, 2021, 9:49 p.m. UTC | #3 
On Sat, Feb 13, 2021 at 12:15:10PM +0200, Mikko Perttunen wrote:
> Convert the original Host1x bindings to YAML and add new bindings for
> NVDEC, now in a more appropriate location. The old text bindings
> for Host1x and engines are still kept at display/tegra/ since they
> encompass a lot more engines that haven't been converted over yet.
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  .../gpu/host1x/nvidia,tegra20-host1x.yaml     | 129 ++++++++++++++++++
>  .../gpu/host1x/nvidia,tegra210-nvdec.yaml     |  90 ++++++++++++
>  MAINTAINERS                                   |   1 +
>  3 files changed, 220 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
>  create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
> new file mode 100644
> index 000000000000..613c6601f0f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/gpu/host1x/nvidia,tegra20-host1x.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Device tree binding for NVIDIA Host1x
> +
> +maintainers:
> +  - Thierry Reding <treding@gmail.com>
> +  - Mikko Perttunen <mperttunen@nvidia.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^host1x@[0-9a-f]*$"
> +
> +  compatible:
> +    oneOf:
> +      - const: nvidia,tegra20-host1x
> +      - const: nvidia,tegra30-host1x
> +      - const: nvidia,tegra114-host1x
> +      - const: nvidia,tegra124-host1x
> +      - items:
> +          - const: nvidia,tegra132-host1x
> +          - const: nvidia,tegra124-host1x
> +      - const: nvidia,tegra210-host1x
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: Syncpoint threshold interrupt
> +      - description: General interrupt
> +
> +  interrupt-names:
> +    items:
> +      - const: syncpt
> +      - const: host1x
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: host1x
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: host1x
> +
> +  iommus:
> +    maxItems: 1
> +
> +  interconnects:
> +    maxItems: 1
> +
> +  interconnect-names:
> +    items:
> +      - const: dma-mem
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +  ranges: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - '#address-cells'
> +  - '#size-cells'
> +  - ranges
> +
> +additionalProperties:
> +  type: object
> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        anyOf:
> +          - const: nvidia,tegra186-host1x
> +          - const: nvidia,tegra194-host1x

Just use 'enum' instead of 'anyOf' and 'const'.

> +then:
> +  properties:
> +    reg:
> +      items:
> +        - description: Hypervisor-accessible register area
> +        - description: VM-accessible register area

If you test this, it will fail due to the 'maxItems: 1' above. The main 
section has to pass for all conditions and then if/them schema add 
constraints.

> +    reg-names:
> +      items:
> +        - const: hypervisor
> +        - const: vm
> +  required:
> +    - reg-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/tegra20-car.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    host1x@50000000 {
> +        compatible = "nvidia,tegra20-host1x";
> +        reg = <0x50000000 0x00024000>;
> +        interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, /* syncpt */
> +                      <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; /* general */
> +        interrupt-names = "syncpt", "host1x";
> +        clocks = <&tegra_car TEGRA20_CLK_HOST1X>;
> +        clock-names = "host1x";
> +        resets = <&tegra_car 28>;
> +        reset-names = "host1x";
> +
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        ranges = <0x54000000 0x54000000 0x04000000>;
> +    };
> diff --git a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> new file mode 100644
> index 000000000000..9a6334d930c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/gpu/host1x/nvidia,tegra210-nvdec.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Device tree binding for NVIDIA Tegra VIC

I'm left wondering what NVDEC and VIC are?

> +
> +maintainers:
> +  - Thierry Reding <treding@gmail.com>
> +  - Mikko Perttunen <mperttunen@nvidia.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^nvdec@[0-9a-f]*$"
> +
> +  compatible:
> +    enum:
> +      - nvidia,tegra210-nvdec
> +      - nvidia,tegra186-nvdec
> +      - nvidia,tegra194-nvdec
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: nvdec
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: nvdec
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  iommus:
> +    maxItems: 1
> +
> +  interconnects:
> +    items:
> +      - description: DMA read memory client
> +      - description: DMA write memory client
> +
> +  interconnect-names:
> +    items:
> +      - const: dma-mem
> +      - const: write
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/tegra186-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/memory/tegra186-mc.h>
> +    #include <dt-bindings/power/tegra186-powergate.h>
> +    #include <dt-bindings/reset/tegra186-reset.h>
> +
> +    nvdec@15480000 {
> +            compatible = "nvidia,tegra186-nvdec";
> +            reg = <0x15480000 0x40000>;
> +            clocks = <&bpmp TEGRA186_CLK_NVDEC>;
> +            clock-names = "nvdec";
> +            resets = <&bpmp TEGRA186_RESET_NVDEC>;
> +            reset-names = "nvdec";
> +
> +            power-domains = <&bpmp TEGRA186_POWER_DOMAIN_NVDEC>;
> +            interconnects = <&mc TEGRA186_MEMORY_CLIENT_NVDECSRD &emc>,
> +                            <&mc TEGRA186_MEMORY_CLIENT_NVDECSWR &emc>;
> +            interconnect-names = "dma-mem", "write";
> +            iommus = <&smmu TEGRA186_SID_NVDEC>;
> +    };
> +
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8170b40d6236..b892419c6564 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5950,6 +5950,7 @@ L:	linux-tegra@vger.kernel.org
>  S:	Supported
>  T:	git git://anongit.freedesktop.org/tegra/linux.git
>  F:	Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> +F:	Documentation/devicetree/bindings/gpu/host1x/
>  F:	drivers/gpu/drm/tegra/
>  F:	drivers/gpu/host1x/
>  F:	include/linux/host1x.h
> -- 
> 2.30.0
>