[PATCH v2 0/4] Add VIC support for Tegra124

[PATCH v2 0/4] Add VIC support for Tegra124

[Mikko Perttunen]

>From Arto's original cover letter:

This series adds Video-Image-Compositor (VIC) support for Tegra124. The unit
replaced gr2d engine on T124 and it is effectively used for similar
operations: making simple surface copy and fill operations.

[..]

The series has been tested on Jetson TK1 by first disabling IOMMU (*),
enabling CMA and running a VIC clear test case that is posted to dri-devel
and linux-tegra mailing lists. The firmware image for VIC is publicly
available as part of Linux For Tegra driver package [0].

[0] https://developer.nvidia.com/linux-tegra

(*) Currently Tegra DRM does not support mapping the host1x command buffers
into kernel address space in case IOMMU is enabled.

End of original cover letter.

The aforementioned VIC clear test can be found at
https://patchwork.kernel.org/patch/6454821/.

This series is now composed of a commit that adds a general Falcon helper
library to be used by drivers whose hardware contains a Falcon, which
includes many Host1x clients; and then separately of the VIC driver that
uses that Falcon library. The fixes to host1x this series used to include
are in the 'Host1x/TegraDRM fixes/improvements' series I posted earlier;
that series is a dependency for this series.

Thanks,
Mikko.

Arto Merilainen (3):
  drm/tegra: Add falcon helper library
  drm/tegra: Add VIC support
  ARM: tegra: Add VIC for Tegra124

Mikko Perttunen (1):
  of: Add NVIDIA Tegra VIC binding

 .../bindings/gpu/nvidia,tegra20-host1x.txt         |  15 +
 arch/arm/boot/dts/tegra124.dtsi                    |  12 +
 drivers/gpu/drm/tegra/Makefile                     |   4 +-
 drivers/gpu/drm/tegra/drm.c                        |   7 +
 drivers/gpu/drm/tegra/drm.h                        |   1 +
 drivers/gpu/drm/tegra/falcon.c                     | 256 ++++++++++++
 drivers/gpu/drm/tegra/falcon.h                     | 130 ++++++
 drivers/gpu/drm/tegra/vic.c                        | 456 +++++++++++++++++++++
 drivers/gpu/drm/tegra/vic.h                        |  35 ++
 include/linux/host1x.h                             |   1 +
 10 files changed, 916 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/falcon.c
 create mode 100644 drivers/gpu/drm/tegra/falcon.h
 create mode 100644 drivers/gpu/drm/tegra/vic.c
 create mode 100644 drivers/gpu/drm/tegra/vic.h

-- 
2.1.4

[PATCH v2 1/4] drm/tegra: Add falcon helper library

[Mikko Perttunen]

From: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Add a set of falcon helper routines for use by the tegradrm client drivers
of the various falcon-based engines.

The falcon is a microcontroller that acts as a frontend for the rest of a
particular Tegra engine.  In order to properly utilize these engines, the
frontend must be booted before pushing any commands.

Based on work by Andrew Chew <achew-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Andrew Chew <achew-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/Makefile |   3 +-
 drivers/gpu/drm/tegra/falcon.c | 256 +++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tegra/falcon.h | 130 +++++++++++++++++++++
 3 files changed, 388 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/falcon.c
 create mode 100644 drivers/gpu/drm/tegra/falcon.h

diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 2c66a8d..93e9a4a 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -13,6 +13,7 @@ tegra-drm-y := \
 	sor.o \
 	dpaux.o \
 	gr2d.o \
-	gr3d.o
+	gr3d.o \
+	falcon.o
 
 obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c
new file mode 100644
index 0000000..180b2fd
--- /dev/null
+++ b/drivers/gpu/drm/tegra/falcon.c
@@ -0,0 +1,256 @@
+/*
+ * Copyright (c) 2015, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/firmware.h>
+#include <linux/pci_ids.h>
+#include <linux/iopoll.h>
+
+#include "falcon.h"
+#include "drm.h"
+
+#define FALCON_IDLE_TIMEOUT_US		100000
+#define FALCON_IDLE_CHECK_PERIOD_US	10
+
+enum falcon_memory {
+	FALCON_MEMORY_IMEM,
+	FALCON_MEMORY_DATA,
+};
+
+static void falcon_writel(struct falcon *falcon, u32 value, u32 offset)
+{
+	writel(value, falcon->regs + offset);
+}
+
+int falcon_wait_idle(struct falcon *falcon)
+{
+	u32 idlestate;
+
+	return readl_poll_timeout(falcon->regs + FALCON_IDLESTATE, idlestate,
+				  (!idlestate),
+				  FALCON_IDLE_CHECK_PERIOD_US,
+				  FALCON_IDLE_TIMEOUT_US);
+}
+
+static int falcon_dma_wait_idle(struct falcon *falcon)
+{
+	u32 dmatrfcmd;
+
+	return readl_poll_timeout(falcon->regs + FALCON_DMATRFCMD, dmatrfcmd,
+				  (dmatrfcmd & FALCON_DMATRFCMD_IDLE),
+				  FALCON_IDLE_CHECK_PERIOD_US,
+				  FALCON_IDLE_TIMEOUT_US);
+}
+
+static int falcon_copy_chunk(struct falcon *falcon,
+			     phys_addr_t base,
+			     unsigned long offset,
+			     enum falcon_memory target)
+{
+	u32 cmd = FALCON_DMATRFCMD_SIZE_256B;
+
+	if (target == FALCON_MEMORY_IMEM)
+		cmd |= FALCON_DMATRFCMD_IMEM;
+
+	falcon_writel(falcon, offset, FALCON_DMATRFMOFFS);
+	falcon_writel(falcon, base, FALCON_DMATRFFBOFFS);
+	falcon_writel(falcon, cmd, FALCON_DMATRFCMD);
+
+	return falcon_dma_wait_idle(falcon);
+}
+
+static void falcon_copy_firmware_image(struct falcon *falcon,
+				       const struct firmware *firmware)
+{
+	u32 *firmware_vaddr = falcon->firmware.vaddr;
+	size_t i;
+
+	/* copy the whole thing taking into account endianness */
+	for (i = 0; i < firmware->size / sizeof(u32); i++)
+		firmware_vaddr[i] = le32_to_cpu(((u32 *)firmware->data)[i]);
+
+	/* ensure that caches are flushed and falcon can see the firmware */
+	dma_sync_single_for_device(falcon->dev, virt_to_phys(firmware_vaddr),
+				   falcon->firmware.size, DMA_TO_DEVICE);
+}
+
+static int falcon_parse_firmware_image(struct falcon *falcon)
+{
+	struct falcon_firmware_bin_header_v1 *bin_header =
+		(void *)falcon->firmware.vaddr;
+	struct falcon_firmware_os_header_v1 *os_header;
+
+	/* endian problems would show up right here */
+	if (bin_header->bin_magic != PCI_VENDOR_ID_NVIDIA) {
+		dev_err(falcon->dev, "failed to get firmware magic");
+		return -EINVAL;
+	}
+
+	/* currently only version 1 is supported */
+	if (bin_header->bin_ver != 1) {
+		dev_err(falcon->dev, "unsupported firmware version");
+		return -EINVAL;
+	}
+
+	/* check that the firmware size is consistent */
+	if (bin_header->bin_size > falcon->firmware.size) {
+		dev_err(falcon->dev, "firmware image size inconsistency");
+		return -EINVAL;
+	}
+
+	os_header = (falcon->firmware.vaddr +
+		 bin_header->os_bin_header_offset);
+
+	falcon->firmware.bin_data.size = bin_header->os_bin_size;
+	falcon->firmware.bin_data.offset = bin_header->os_bin_data_offset;
+	falcon->firmware.code.offset = os_header->os_code_offset;
+	falcon->firmware.code.size   = os_header->os_code_size;
+	falcon->firmware.data.offset = os_header->os_data_offset;
+	falcon->firmware.data.size   = os_header->os_data_size;
+
+	return 0;
+}
+
+int falcon_read_firmware(struct falcon *falcon, const char *firmware_name)
+{
+	const struct firmware *firmware;
+	int err;
+
+	if (falcon->firmware.valid)
+		return 0;
+
+	err = request_firmware(&firmware, firmware_name, falcon->dev);
+	if (err < 0) {
+		dev_err(falcon->dev, "failed to get firmware\n");
+		return err;
+	}
+
+	falcon->firmware.size = firmware->size;
+
+	/* allocate iova space for the firmware */
+	falcon->firmware.vaddr = falcon->ops->alloc(falcon, firmware->size,
+						 &falcon->firmware.paddr);
+	if (!falcon->firmware.vaddr) {
+		dev_err(falcon->dev, "dma memory mapping failed");
+		err = -ENOMEM;
+		goto err_alloc_firmware;
+	}
+
+	/* copy firmware image into local area. this also ensures endianness */
+	falcon_copy_firmware_image(falcon, firmware);
+
+	/* parse the image data */
+	err = falcon_parse_firmware_image(falcon);
+	if (err < 0) {
+		dev_err(falcon->dev, "failed to parse firmware image\n");
+		goto err_setup_firmware_image;
+	}
+
+	falcon->firmware.valid = true;
+
+	release_firmware(firmware);
+
+	return 0;
+
+err_setup_firmware_image:
+	falcon->ops->free(falcon, falcon->firmware.size,
+			  falcon->firmware.paddr, falcon->firmware.vaddr);
+err_alloc_firmware:
+	release_firmware(firmware);
+
+	return err;
+}
+
+int falcon_init(struct falcon *falcon)
+{
+	/* check mandatory ops */
+	if (!falcon->ops || !falcon->ops->alloc || !falcon->ops->free)
+		return -EINVAL;
+
+	falcon->firmware.valid = false;
+
+	return 0;
+}
+
+void falcon_exit(struct falcon *falcon)
+{
+	if (!falcon->firmware.valid)
+		return;
+
+	falcon->ops->free(falcon, falcon->firmware.size,
+			  falcon->firmware.paddr,
+			  falcon->firmware.vaddr);
+	falcon->firmware.valid = false;
+}
+
+int falcon_boot(struct falcon *falcon)
+{
+	unsigned long offset;
+	int err = 0;
+
+	if (!falcon->firmware.valid)
+		return -EINVAL;
+
+	falcon_writel(falcon, 0, FALCON_DMACTL);
+
+	/* setup the address of the binary data. Falcon can access it later */
+	falcon_writel(falcon, (falcon->firmware.paddr +
+			       falcon->firmware.bin_data.offset) >> 8,
+		      FALCON_DMATRFBASE);
+
+	/* copy the data segment into Falcon internal memory */
+	for (offset = 0; offset < falcon->firmware.data.size; offset += 256)
+		falcon_copy_chunk(falcon,
+				  falcon->firmware.data.offset + offset,
+				  offset, FALCON_MEMORY_DATA);
+
+	/* copy the first code segment into Falcon internal memory */
+	falcon_copy_chunk(falcon, falcon->firmware.code.offset,
+			  0, FALCON_MEMORY_IMEM);
+
+	/* setup falcon interrupts */
+	falcon_writel(falcon, FALCON_IRQMSET_EXT(0xff) |
+			      FALCON_IRQMSET_SWGEN1 |
+			      FALCON_IRQMSET_SWGEN0 |
+			      FALCON_IRQMSET_EXTERR |
+			      FALCON_IRQMSET_HALT |
+			      FALCON_IRQMSET_WDTMR,
+		      FALCON_IRQMSET);
+	falcon_writel(falcon, FALCON_IRQDEST_EXT(0xff) |
+			      FALCON_IRQDEST_SWGEN1 |
+			      FALCON_IRQDEST_SWGEN0 |
+			      FALCON_IRQDEST_EXTERR |
+			      FALCON_IRQDEST_HALT,
+		      FALCON_IRQDEST);
+
+	/* enable interface */
+	falcon_writel(falcon, FALCON_ITFEN_MTHDEN |
+			      FALCON_ITFEN_CTXEN,
+		      FALCON_ITFEN);
+
+	/* boot falcon */
+	falcon_writel(falcon, 0x00000000, FALCON_BOOTVEC);
+	falcon_writel(falcon, FALCON_CPUCTL_STARTCPU, FALCON_CPUCTL);
+
+	err = falcon_wait_idle(falcon);
+	if (err < 0) {
+		dev_err(falcon->dev, "falcon boot failed due to timeout");
+		return err;
+	}
+
+	dev_info(falcon->dev, "falcon booted");
+
+	return 0;
+}
+
+void falcon_execute_method(struct falcon *falcon, u32 method, u32 data)
+{
+	falcon_writel(falcon, method >> 2, FALCON_UCLASS_METHOD_OFFSET);
+	falcon_writel(falcon, data, FALCON_UCLASS_METHOD_DATA);
+}
diff --git a/drivers/gpu/drm/tegra/falcon.h b/drivers/gpu/drm/tegra/falcon.h
new file mode 100644
index 0000000..56284b9
--- /dev/null
+++ b/drivers/gpu/drm/tegra/falcon.h
@@ -0,0 +1,130 @@
+/*
+ * Copyright (c) 2015, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _FALCON_H_
+#define _FALCON_H_
+
+#include <linux/types.h>
+
+#define FALCON_UCLASS_METHOD_OFFSET		0x00000040
+
+#define FALCON_UCLASS_METHOD_DATA		0x00000044
+
+#define FALCON_IRQMSET				0x00001010
+#define FALCON_IRQMSET_WDTMR			(1 << 1)
+#define FALCON_IRQMSET_HALT			(1 << 4)
+#define FALCON_IRQMSET_EXTERR			(1 << 5)
+#define FALCON_IRQMSET_SWGEN0			(1 << 6)
+#define FALCON_IRQMSET_SWGEN1			(1 << 7)
+#define FALCON_IRQMSET_EXT(v)			(((v) & 0xff) << 8)
+
+#define FALCON_IRQDEST				0x0000101c
+#define FALCON_IRQDEST_HALT			(1 << 4)
+#define FALCON_IRQDEST_EXTERR			(1 << 5)
+#define FALCON_IRQDEST_SWGEN0			(1 << 6)
+#define FALCON_IRQDEST_SWGEN1			(1 << 7)
+#define FALCON_IRQDEST_EXT(v)			(((v) & 0xff) << 8)
+
+#define FALCON_ITFEN				0x00001048
+#define FALCON_ITFEN_CTXEN			(1 << 0)
+#define FALCON_ITFEN_MTHDEN			(1 << 1)
+
+#define FALCON_IDLESTATE			0x0000104c
+
+#define FALCON_CPUCTL				0x00001100
+#define FALCON_CPUCTL_STARTCPU			(1 << 1)
+
+#define FALCON_BOOTVEC				0x00001104
+
+#define FALCON_DMACTL				0x0000110c
+#define FALCON_DMACTL_DMEM_SCRUBBING		(1 << 1)
+#define FALCON_DMACTL_IMEM_SCRUBBING		(1 << 2)
+
+#define FALCON_DMATRFBASE			0x00001110
+
+#define FALCON_DMATRFMOFFS			0x00001114
+
+#define FALCON_DMATRFCMD			0x00001118
+#define FALCON_DMATRFCMD_IDLE			(1 << 1)
+#define FALCON_DMATRFCMD_IMEM			(1 << 4)
+#define FALCON_DMATRFCMD_SIZE_256B		(6 << 8)
+
+#define FALCON_DMATRFFBOFFS			0x0000111c
+
+struct falcon_firmware_bin_header_v1 {
+	u32 bin_magic;		/* 0x10de */
+	u32 bin_ver;		/* cya, versioning of bin format (1) */
+	u32 bin_size;		/* entire image size including this header */
+	u32 os_bin_header_offset;
+	u32 os_bin_data_offset;
+	u32 os_bin_size;
+};
+
+struct falcon_firmware_os_app_v1 {
+	u32 offset;
+	u32 size;
+};
+
+struct falcon_firmware_os_header_v1 {
+	u32 os_code_offset;
+	u32 os_code_size;
+	u32 os_data_offset;
+	u32 os_data_size;
+	u32 num_apps;
+	struct falcon_firmware_os_app_v1 *app_code;
+	struct falcon_firmware_os_app_v1 *app_data;
+	u32 *os_ovl_offset;
+	u32 *os_ovl_size;
+};
+
+struct falcon;
+
+struct falcon_ops {
+	void *(*alloc)(struct falcon *falcon, size_t size,
+		       dma_addr_t *paddr);
+	void (*free)(struct falcon *falcon, size_t size,
+		     dma_addr_t paddr, void *vaddr);
+};
+
+struct falcon_firmware_section {
+	unsigned long offset;
+	size_t size;
+};
+
+struct falcon_firmware {
+	/* Raw firmware data */
+	dma_addr_t paddr;
+	void *vaddr;
+	size_t size;
+
+	/* Parsed firmware information */
+	struct falcon_firmware_section bin_data;
+	struct falcon_firmware_section data;
+	struct falcon_firmware_section code;
+
+	bool valid;
+};
+
+struct falcon {
+	/* Set by falcon client */
+	struct device *dev;
+	void __iomem *regs;
+	const struct falcon_ops *ops;
+	void *data;
+
+	struct falcon_firmware firmware;
+};
+
+int falcon_init(struct falcon *falcon);
+void falcon_exit(struct falcon *falcon);
+int falcon_read_firmware(struct falcon *falcon, const char *firmware_name);
+int falcon_boot(struct falcon *falcon);
+void falcon_execute_method(struct falcon *falcon, u32 method, u32 data);
+int falcon_wait_idle(struct falcon *falcon);
+
+#endif /* _FALCON_H_ */
-- 
2.1.4

[PATCH v2 2/4] drm/tegra: Add VIC support

[Mikko Perttunen]

From: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

This patch adds support for Video Image Compositor engine which
can be used for 2d operations.

Signed-off-by: Andrew Chew <achew-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/Makefile |   3 +-
 drivers/gpu/drm/tegra/drm.c    |   7 +
 drivers/gpu/drm/tegra/drm.h    |   1 +
 drivers/gpu/drm/tegra/vic.c    | 456 +++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tegra/vic.h    |  35 ++++
 include/linux/host1x.h         |   1 +
 6 files changed, 502 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/vic.c
 create mode 100644 drivers/gpu/drm/tegra/vic.h

diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 93e9a4a..6af3a9a 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -14,6 +14,7 @@ tegra-drm-y := \
 	dpaux.o \
 	gr2d.o \
 	gr3d.o \
-	falcon.o
+	falcon.o \
+	vic.o
 
 obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index af4ff86..bc8cc2a 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
 	{ .compatible = "nvidia,tegra124-dc", },
 	{ .compatible = "nvidia,tegra124-sor", },
 	{ .compatible = "nvidia,tegra124-hdmi", },
+	{ .compatible = "nvidia,tegra124-vic", },
 	{ /* sentinel */ }
 };
 
@@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
 	if (err < 0)
 		goto unregister_gr2d;
 
+	err = platform_driver_register(&tegra_vic_driver);
+	if (err < 0)
+		goto unregister_gr3d;
+
 	return 0;
 
+unregister_gr3d:
+	platform_driver_unregister(&tegra_gr3d_driver);
 unregister_gr2d:
 	platform_driver_unregister(&tegra_gr2d_driver);
 unregister_dpaux:
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 58c83b11..2fc7e42 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
 extern struct platform_driver tegra_dpaux_driver;
 extern struct platform_driver tegra_gr2d_driver;
 extern struct platform_driver tegra_gr3d_driver;
+extern struct platform_driver tegra_vic_driver;
 
 #endif /* HOST1X_DRM_H */
diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
new file mode 100644
index 0000000..fce7c04
--- /dev/null
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -0,0 +1,456 @@
+/*
+ * Copyright (c) 2015, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/host1x.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include <soc/tegra/pmc.h>
+
+#include "drm.h"
+#include "falcon.h"
+#include "vic.h"
+
+struct vic_config {
+	/* Firmware name */
+	const char *ucode_name;
+};
+
+struct vic {
+	struct falcon falcon;
+	bool booted;
+
+	void __iomem *regs;
+	struct tegra_drm_client client;
+	struct host1x_channel *channel;
+	struct iommu_domain *domain;
+	struct device *dev;
+	struct clk *clk;
+	struct reset_control *rst;
+
+	/* Platform configuration */
+	const struct vic_config *config;
+
+	/* for firewall - this determines if method 1 should be regarded
+	 * as an address register */
+	bool method_data_is_addr_reg;
+};
+
+static inline struct vic *to_vic(struct tegra_drm_client *client)
+{
+	return container_of(client, struct vic, client);
+}
+
+static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
+{
+	writel(value, vic->regs + offset);
+}
+
+static int vic_runtime_resume(struct device *dev)
+{
+	struct vic *vic = dev_get_drvdata(dev);
+	int err;
+
+	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
+						vic->clk, vic->rst);
+	if (err < 0)
+		dev_err(dev, "failed to power up device\n");
+
+	return err;
+}
+
+static int vic_runtime_suspend(struct device *dev)
+{
+	struct vic *vic = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(vic->clk);
+	reset_control_assert(vic->rst);
+	tegra_powergate_power_off(TEGRA_POWERGATE_VIC);
+
+	vic->booted = false;
+
+	return 0;
+}
+
+static int vic_boot(struct vic *vic)
+{
+	u32 fce_ucode_size, fce_bin_data_offset;
+	void *hdr;
+	int err = 0;
+
+	if (vic->booted)
+		return 0;
+
+	if (!vic->falcon.firmware.valid) {
+		err = falcon_read_firmware(&vic->falcon,
+					   vic->config->ucode_name);
+		if (err < 0)
+			return err;
+	}
+
+	/* ensure that the engine is in sane state */
+	reset_control_assert(vic->rst);
+	usleep_range(10, 100);
+	reset_control_deassert(vic->rst);
+
+	/* setup clockgating registers */
+	vic_writel(vic, CG_IDLE_CG_DLY_CNT(4) |
+			CG_IDLE_CG_EN |
+			CG_WAKEUP_DLY_CNT(4),
+		   NV_PVIC_MISC_PRI_VIC_CG);
+
+	err = falcon_boot(&vic->falcon);
+	if (err < 0)
+		return err;
+
+	hdr = vic->falcon.firmware.vaddr;
+	fce_bin_data_offset = *(u32 *)(hdr + VIC_UCODE_FCE_DATA_OFFSET);
+	hdr = vic->falcon.firmware.vaddr +
+		*(u32 *)(hdr + VIC_UCODE_FCE_HEADER_OFFSET);
+	fce_ucode_size = *(u32 *)(hdr + FCE_UCODE_SIZE_OFFSET);
+
+	falcon_execute_method(&vic->falcon, VIC_SET_APPLICATION_ID, 1);
+	falcon_execute_method(&vic->falcon, VIC_SET_FCE_UCODE_SIZE,
+			      fce_ucode_size);
+	falcon_execute_method(&vic->falcon, VIC_SET_FCE_UCODE_OFFSET,
+			      (vic->falcon.firmware.paddr + fce_bin_data_offset)
+				>> 8);
+
+	err = falcon_wait_idle(&vic->falcon);
+	if (err < 0) {
+		dev_err(vic->dev,
+			"failed to set application ID and FCE base\n");
+		return err;
+	}
+
+	vic->booted = true;
+
+	return 0;
+}
+
+static void *vic_falcon_alloc(struct falcon *falcon, size_t size,
+			       dma_addr_t *iova)
+{
+	struct tegra_drm *tegra = falcon->data;
+
+	return tegra_drm_alloc(tegra, size, iova);
+}
+
+static void vic_falcon_free(struct falcon *falcon, size_t size,
+			    dma_addr_t iova, void *va)
+{
+	struct tegra_drm *tegra = falcon->data;
+
+	return tegra_drm_free(tegra, size, va, iova);
+}
+
+static const struct falcon_ops vic_falcon_ops = {
+	.alloc = vic_falcon_alloc,
+	.free = vic_falcon_free
+};
+
+static int vic_init(struct host1x_client *client)
+{
+	struct tegra_drm_client *drm = host1x_to_drm_client(client);
+	struct drm_device *dev = dev_get_drvdata(client->parent);
+	struct tegra_drm *tegra = dev->dev_private;
+	struct vic *vic = to_vic(drm);
+	int err;
+
+	if (tegra->domain) {
+		err = iommu_attach_device(tegra->domain, vic->dev);
+		if (err < 0) {
+			dev_err(vic->dev, "failed to attach to domain: %d\n",
+				err);
+			return err;
+		}
+
+		vic->domain = tegra->domain;
+	}
+
+	vic->falcon.dev = vic->dev;
+	vic->falcon.regs = vic->regs;
+	vic->falcon.data = tegra;
+	vic->falcon.ops = &vic_falcon_ops;
+	err = falcon_init(&vic->falcon);
+	if (err < 0)
+		goto detach_device;
+
+	vic->channel = host1x_channel_request(client->dev);
+	if (!vic->channel) {
+		err = -ENOMEM;
+		goto exit_falcon;
+	}
+
+	client->syncpts[0] = host1x_syncpt_request(client->dev, 0);
+	if (!client->syncpts[0]) {
+		err = -ENOMEM;
+		goto free_channel;
+	}
+
+	err = tegra_drm_register_client(tegra, drm);
+	if (err < 0)
+		goto free_syncpt;
+
+	return 0;
+
+free_syncpt:
+	host1x_syncpt_free(client->syncpts[0]);
+free_channel:
+	host1x_channel_free(vic->channel);
+exit_falcon:
+	falcon_exit(&vic->falcon);
+detach_device:
+	if (tegra->domain)
+		iommu_detach_device(tegra->domain, vic->dev);
+
+	return err;
+}
+
+static int vic_exit(struct host1x_client *client)
+{
+	struct tegra_drm_client *drm = host1x_to_drm_client(client);
+	struct drm_device *dev = dev_get_drvdata(client->parent);
+	struct tegra_drm *tegra = dev->dev_private;
+	struct vic *vic = to_vic(drm);
+	int err;
+
+	err = tegra_drm_unregister_client(tegra, drm);
+	if (err < 0)
+		return err;
+
+	host1x_syncpt_free(client->syncpts[0]);
+	host1x_channel_free(vic->channel);
+
+	if (vic->booted) {
+		reset_control_assert(vic->rst);
+		usleep_range(10, 100);
+		reset_control_deassert(vic->rst);
+	}
+
+	falcon_exit(&vic->falcon);
+
+	if (vic->domain) {
+		iommu_detach_device(vic->domain, vic->dev);
+		vic->domain = NULL;
+	}
+
+	return 0;
+}
+
+static const struct host1x_client_ops vic_client_ops = {
+	.init = vic_init,
+	.exit = vic_exit,
+};
+
+static int vic_open_channel(struct tegra_drm_client *client,
+			    struct tegra_drm_context *context)
+{
+	struct vic *vic = to_vic(client);
+	int err;
+
+	err = pm_runtime_get_sync(vic->dev);
+	if (err < 0)
+		return err;
+
+	/*
+	 * Try to boot the Falcon microcontroller. Booting is deferred until
+	 * here because the firmware might not yet be available during system
+	 * boot, for example if it's on remote storage.
+	 */
+	err = vic_boot(vic);
+	if (err < 0) {
+		pm_runtime_put(vic->dev);
+		return err;
+	}
+
+	context->channel = host1x_channel_get(vic->channel);
+	if (!context->channel) {
+		pm_runtime_put(vic->dev);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void vic_close_channel(struct tegra_drm_context *context)
+{
+	struct vic *vic = to_vic(context->client);
+
+	host1x_channel_put(context->channel);
+
+	pm_runtime_put(vic->dev);
+}
+
+static int vic_is_addr_reg(struct device *dev, u32 class, u32 offset, u32 val)
+{
+	struct vic *vic = dev_get_drvdata(dev);
+
+	if (class != HOST1X_CLASS_VIC)
+		return false;
+
+	/*
+	 * Method call parameter. Check stored value to see if set method uses
+	 * parameter as memory address.
+	 */
+	if (offset == FALCON_UCLASS_METHOD_DATA >> 2)
+		return vic->method_data_is_addr_reg;
+
+	/* Method call number store. */
+	if (offset == FALCON_UCLASS_METHOD_OFFSET >> 2) {
+		u32 method = val << 2;
+
+		if ((method >= VIC_SET_SURFACE0_SLOT0_LUMA_OFFSET &&
+		     method <= VIC_SET_SURFACE7_SLOT4_CHROMAV_OFFSET) ||
+		    (method >= VIC_SET_CONFIG_STRUCT_OFFSET &&
+		     method <= VIC_SET_OUTPUT_SURFACE_CHROMAV_OFFSET))
+			vic->method_data_is_addr_reg = true;
+		else
+			vic->method_data_is_addr_reg = false;
+	}
+
+	return false;
+}
+
+static const struct tegra_drm_client_ops vic_ops = {
+	.open_channel = vic_open_channel,
+	.close_channel = vic_close_channel,
+	.is_addr_reg = vic_is_addr_reg,
+	.submit = tegra_drm_submit,
+};
+
+static const struct vic_config vic_t124_config = {
+	.ucode_name = "vic03_ucode.bin",
+};
+
+static const struct of_device_id vic_match[] = {
+	{ .compatible = "nvidia,tegra124-vic", .data = &vic_t124_config },
+	{ },
+};
+
+static int vic_probe(struct platform_device *pdev)
+{
+	struct vic_config *vic_config = NULL;
+	struct device *dev = &pdev->dev;
+	struct host1x_syncpt **syncpts;
+	struct resource *regs;
+	const struct of_device_id *match;
+	struct vic *vic;
+	int err;
+
+	match = of_match_device(vic_match, dev);
+	vic_config = (struct vic_config *)match->data;
+
+	vic = devm_kzalloc(dev, sizeof(*vic), GFP_KERNEL);
+	if (!vic)
+		return -ENOMEM;
+
+	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;
+	}
+
+	vic->regs = devm_ioremap_resource(dev, regs);
+	if (IS_ERR(vic->regs))
+		return PTR_ERR(vic->regs);
+
+	vic->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(vic->clk)) {
+		dev_err(&pdev->dev, "failed to get clock\n");
+		return PTR_ERR(vic->clk);
+	}
+
+	vic->rst = devm_reset_control_get(dev, "vic");
+	if (IS_ERR(vic->rst)) {
+		dev_err(&pdev->dev, "cannot get reset\n");
+		return PTR_ERR(vic->rst);
+	}
+
+	platform_set_drvdata(pdev, vic);
+
+	INIT_LIST_HEAD(&vic->client.base.list);
+	vic->client.base.ops = &vic_client_ops;
+	vic->client.base.dev = dev;
+	vic->client.base.class = HOST1X_CLASS_VIC;
+	vic->client.base.syncpts = syncpts;
+	vic->client.base.num_syncpts = 1;
+	vic->dev = dev;
+	vic->config = vic_config;
+
+	INIT_LIST_HEAD(&vic->client.list);
+	vic->client.ops = &vic_ops;
+
+	err = host1x_client_register(&vic->client.base);
+	if (err < 0) {
+		dev_err(dev, "failed to register host1x client: %d\n", err);
+		platform_set_drvdata(pdev, NULL);
+		return err;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+	if (!pm_runtime_enabled(&pdev->dev)) {
+		err = vic_runtime_resume(&pdev->dev);
+		if (err < 0)
+			goto unregister_client;
+	}
+
+	dev_info(&pdev->dev, "initialized");
+
+	return 0;
+
+unregister_client:
+	host1x_client_unregister(&vic->client.base);
+
+	return err;
+}
+
+static int vic_remove(struct platform_device *pdev)
+{
+	struct vic *vic = platform_get_drvdata(pdev);
+	int err;
+
+	err = host1x_client_unregister(&vic->client.base);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to unregister host1x client: %d\n",
+			err);
+		return err;
+	}
+
+	if (pm_runtime_enabled(&pdev->dev))
+		pm_runtime_disable(&pdev->dev);
+	else
+		vic_runtime_suspend(&pdev->dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops vic_pm_ops = {
+	SET_RUNTIME_PM_OPS(vic_runtime_suspend, vic_runtime_resume, NULL)
+};
+
+struct platform_driver tegra_vic_driver = {
+	.driver = {
+		.name = "tegra-vic",
+		.of_match_table = vic_match,
+		.pm = &vic_pm_ops
+	},
+	.probe = vic_probe,
+	.remove = vic_remove,
+};
diff --git a/drivers/gpu/drm/tegra/vic.h b/drivers/gpu/drm/tegra/vic.h
new file mode 100644
index 0000000..eedd219
--- /dev/null
+++ b/drivers/gpu/drm/tegra/vic.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2015, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef TEGRA_VIC_H
+#define TEGRA_VIC_H
+
+/* VIC methods */
+
+#define VIC_SET_APPLICATION_ID			0x00000200
+#define VIC_SET_FCE_UCODE_SIZE			0x0000071C
+#define VIC_SET_FCE_UCODE_OFFSET		0x0000072C
+#define VIC_SET_SURFACE0_SLOT0_LUMA_OFFSET	0x00000400
+#define VIC_SET_SURFACE7_SLOT4_CHROMAV_OFFSET	0x000005dc
+#define VIC_SET_CONFIG_STRUCT_OFFSET		0x00000720
+#define VIC_SET_OUTPUT_SURFACE_CHROMAV_OFFSET	0x00000738
+
+/* VIC registers */
+
+#define NV_PVIC_MISC_PRI_VIC_CG			0x000016d0
+#define CG_IDLE_CG_DLY_CNT(val)			((val & 0x3f) << 0)
+#define CG_IDLE_CG_EN				(1 << 6)
+#define CG_WAKEUP_DLY_CNT(val)			((val & 0xf) << 16)
+
+/* Firmware offsets */
+
+#define VIC_UCODE_FCE_HEADER_OFFSET		(6*4)
+#define VIC_UCODE_FCE_DATA_OFFSET		(7*4)
+#define FCE_UCODE_SIZE_OFFSET			(2*4)
+
+#endif /* TEGRA_VIC_H */
diff --git a/include/linux/host1x.h b/include/linux/host1x.h
index fc86ced..a006dad 100644
--- a/include/linux/host1x.h
+++ b/include/linux/host1x.h
@@ -26,6 +26,7 @@ enum host1x_class {
 	HOST1X_CLASS_HOST1X = 0x1,
 	HOST1X_CLASS_GR2D = 0x51,
 	HOST1X_CLASS_GR2D_SB = 0x52,
+	HOST1X_CLASS_VIC = 0x5D,
 	HOST1X_CLASS_GR3D = 0x60,
 };
 
-- 
2.1.4

[PATCH v2 3/4] of: Add NVIDIA Tegra VIC binding

[Mikko Perttunen]

This adds device tree binding documentation for the Video Image
Compositor (VIC) present on Tegra124 and newer SoC's.

Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/gpu/nvidia,tegra20-host1x.txt     | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
index 009f4bf..1328f3f 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
@@ -240,6 +240,21 @@ of the following host1x client modules:
     - dpaux
   - vdd-supply: phandle of a supply that powers the DisplayPort link
 
+- vic: Video Image Compositor
+  - compatible: For Tegra124, must contain "nvidia,tegra124-vic".  Otherwise,
+    must contain '"nvidia,<chip>-vic", "nvidia,tegra124-vic"', where
+    <chip> is tegra132.
+  - reg: Physical base address and length of the controller's registers.
+  - interrupts: The interrupt outputs from the controller.
+  - clocks: Must contain an entry for each entry in clock-names.
+    See ../clocks/clock-bindings.txt for details.
+  - clock-names: Must include the following entries:
+    - vic: clock input for the VIC hardware
+  - resets: Must contain an entry for each entry in reset-names.
+    See ../reset/reset.txt for details.
+  - reset-names: Must include the following entries:
+    - vic
+
 Example:
 
 / {
-- 
2.1.4

[PATCH v2 4/4] ARM: tegra: Add VIC for Tegra124

[Mikko Perttunen]

From: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

This patch adds VIC device tree node for Video Image Compositor.

Signed-off-by: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra124.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 87318a7..1233f4a 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -137,6 +137,18 @@
 			status = "disabled";
 		};
 
+		vic@0,54340000 {
+			compatible = "nvidia,tegra124-vic";
+			reg = <0x0 0x54340000 0x0 0x00040000>;
+			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&tegra_car TEGRA124_CLK_VIC03>;
+			clock-names = "vic";
+			resets = <&tegra_car TEGRA124_CLK_VIC03>;
+			reset-names = "vic";
+
+			iommus = <&mc TEGRA_SWGROUP_VIC>;
+		};
+
 		sor@0,54540000 {
 			compatible = "nvidia,tegra124-sor";
 			reg = <0x0 0x54540000 0x0 0x00040000>;
-- 
2.1.4

Re: [PATCH v2 2/4] drm/tegra: Add VIC support

[Jon Hunter]

Hi Mikko,

On 20/07/15 08:54, Mikko Perttunen wrote:
> From: Arto Merilainen <amerilainen@nvidia.com>
> 
> This patch adds support for Video Image Compositor engine which
> can be used for 2d operations.
> 
> Signed-off-by: Andrew Chew <achew@nvidia.com>
> Signed-off-by: Arto Merilainen <amerilainen@nvidia.com>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/Makefile |   3 +-
>  drivers/gpu/drm/tegra/drm.c    |   7 +
>  drivers/gpu/drm/tegra/drm.h    |   1 +
>  drivers/gpu/drm/tegra/vic.c    | 456 +++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tegra/vic.h    |  35 ++++
>  include/linux/host1x.h         |   1 +
>  6 files changed, 502 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/tegra/vic.c
>  create mode 100644 drivers/gpu/drm/tegra/vic.h
> 
> diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
> index 93e9a4a..6af3a9a 100644
> --- a/drivers/gpu/drm/tegra/Makefile
> +++ b/drivers/gpu/drm/tegra/Makefile
> @@ -14,6 +14,7 @@ tegra-drm-y := \
>  	dpaux.o \
>  	gr2d.o \
>  	gr3d.o \
> -	falcon.o
> +	falcon.o \
> +	vic.o
>  
>  obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index af4ff86..bc8cc2a 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
>  	{ .compatible = "nvidia,tegra124-dc", },
>  	{ .compatible = "nvidia,tegra124-sor", },
>  	{ .compatible = "nvidia,tegra124-hdmi", },
> +	{ .compatible = "nvidia,tegra124-vic", },
>  	{ /* sentinel */ }
>  };
>  
> @@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
>  	if (err < 0)
>  		goto unregister_gr2d;
>  
> +	err = platform_driver_register(&tegra_vic_driver);
> +	if (err < 0)
> +		goto unregister_gr3d;
> +
>  	return 0;
>  
> +unregister_gr3d:
> +	platform_driver_unregister(&tegra_gr3d_driver);
>  unregister_gr2d:
>  	platform_driver_unregister(&tegra_gr2d_driver);
>  unregister_dpaux:
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 58c83b11..2fc7e42 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
>  extern struct platform_driver tegra_dpaux_driver;
>  extern struct platform_driver tegra_gr2d_driver;
>  extern struct platform_driver tegra_gr3d_driver;
> +extern struct platform_driver tegra_vic_driver;
>  
>  #endif /* HOST1X_DRM_H */
> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> new file mode 100644
> index 0000000..fce7c04
> --- /dev/null
> +++ b/drivers/gpu/drm/tegra/vic.c
> @@ -0,0 +1,456 @@
> +/*
> + * Copyright (c) 2015, NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/host1x.h>
> +#include <linux/iommu.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#include <soc/tegra/pmc.h>
> +
> +#include "drm.h"
> +#include "falcon.h"
> +#include "vic.h"
> +
> +struct vic_config {
> +	/* Firmware name */
> +	const char *ucode_name;
> +};
> +
> +struct vic {
> +	struct falcon falcon;
> +	bool booted;
> +
> +	void __iomem *regs;
> +	struct tegra_drm_client client;
> +	struct host1x_channel *channel;
> +	struct iommu_domain *domain;
> +	struct device *dev;
> +	struct clk *clk;
> +	struct reset_control *rst;
> +
> +	/* Platform configuration */
> +	const struct vic_config *config;
> +
> +	/* for firewall - this determines if method 1 should be regarded
> +	 * as an address register */
> +	bool method_data_is_addr_reg;
> +};
> +
> +static inline struct vic *to_vic(struct tegra_drm_client *client)
> +{
> +	return container_of(client, struct vic, client);
> +}
> +
> +static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
> +{
> +	writel(value, vic->regs + offset);
> +}
> +
> +static int vic_runtime_resume(struct device *dev)
> +{
> +	struct vic *vic = dev_get_drvdata(dev);
> +	int err;
> +
> +	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> +						vic->clk, vic->rst);

I would like to deprecate use of the above function [1]. I have been
trying to migrate driver to use a new API instead of the above.

> +	if (err < 0)
> +		dev_err(dev, "failed to power up device\n");
> +
> +	return err;
> +}
> +
> +static int vic_runtime_suspend(struct device *dev)
> +{
> +	struct vic *vic = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(vic->clk);
> +	reset_control_assert(vic->rst);
> +	tegra_powergate_power_off(TEGRA_POWERGATE_VIC);

Similarly here. I would like to get rid of the above API in favour of a
different API to help migrate to generic power domain support.

> +	vic->booted = false;
> +
> +	return 0;
> +}
> +
> +static int vic_boot(struct vic *vic)
> +{
> +	u32 fce_ucode_size, fce_bin_data_offset;
> +	void *hdr;
> +	int err = 0;
> +
> +	if (vic->booted)
> +		return 0;
> +
> +	if (!vic->falcon.firmware.valid) {
> +		err = falcon_read_firmware(&vic->falcon,
> +					   vic->config->ucode_name);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	/* ensure that the engine is in sane state */
> +	reset_control_assert(vic->rst);
> +	usleep_range(10, 100);
> +	reset_control_deassert(vic->rst);
> +
> +	/* setup clockgating registers */
> +	vic_writel(vic, CG_IDLE_CG_DLY_CNT(4) |
> +			CG_IDLE_CG_EN |
> +			CG_WAKEUP_DLY_CNT(4),
> +		   NV_PVIC_MISC_PRI_VIC_CG);
> +
> +	err = falcon_boot(&vic->falcon);
> +	if (err < 0)
> +		return err;
> +
> +	hdr = vic->falcon.firmware.vaddr;
> +	fce_bin_data_offset = *(u32 *)(hdr + VIC_UCODE_FCE_DATA_OFFSET);
> +	hdr = vic->falcon.firmware.vaddr +
> +		*(u32 *)(hdr + VIC_UCODE_FCE_HEADER_OFFSET);
> +	fce_ucode_size = *(u32 *)(hdr + FCE_UCODE_SIZE_OFFSET);
> +
> +	falcon_execute_method(&vic->falcon, VIC_SET_APPLICATION_ID, 1);
> +	falcon_execute_method(&vic->falcon, VIC_SET_FCE_UCODE_SIZE,
> +			      fce_ucode_size);
> +	falcon_execute_method(&vic->falcon, VIC_SET_FCE_UCODE_OFFSET,
> +			      (vic->falcon.firmware.paddr + fce_bin_data_offset)
> +				>> 8);
> +
> +	err = falcon_wait_idle(&vic->falcon);
> +	if (err < 0) {
> +		dev_err(vic->dev,
> +			"failed to set application ID and FCE base\n");
> +		return err;
> +	}
> +
> +	vic->booted = true;
> +
> +	return 0;
> +}
> +
> +static void *vic_falcon_alloc(struct falcon *falcon, size_t size,
> +			       dma_addr_t *iova)
> +{
> +	struct tegra_drm *tegra = falcon->data;
> +
> +	return tegra_drm_alloc(tegra, size, iova);
> +}
> +
> +static void vic_falcon_free(struct falcon *falcon, size_t size,
> +			    dma_addr_t iova, void *va)
> +{
> +	struct tegra_drm *tegra = falcon->data;
> +
> +	return tegra_drm_free(tegra, size, va, iova);
> +}
> +
> +static const struct falcon_ops vic_falcon_ops = {
> +	.alloc = vic_falcon_alloc,
> +	.free = vic_falcon_free
> +};
> +
> +static int vic_init(struct host1x_client *client)
> +{
> +	struct tegra_drm_client *drm = host1x_to_drm_client(client);
> +	struct drm_device *dev = dev_get_drvdata(client->parent);
> +	struct tegra_drm *tegra = dev->dev_private;
> +	struct vic *vic = to_vic(drm);
> +	int err;
> +
> +	if (tegra->domain) {
> +		err = iommu_attach_device(tegra->domain, vic->dev);
> +		if (err < 0) {
> +			dev_err(vic->dev, "failed to attach to domain: %d\n",
> +				err);
> +			return err;
> +		}
> +
> +		vic->domain = tegra->domain;
> +	}
> +
> +	vic->falcon.dev = vic->dev;
> +	vic->falcon.regs = vic->regs;
> +	vic->falcon.data = tegra;
> +	vic->falcon.ops = &vic_falcon_ops;
> +	err = falcon_init(&vic->falcon);
> +	if (err < 0)
> +		goto detach_device;
> +
> +	vic->channel = host1x_channel_request(client->dev);
> +	if (!vic->channel) {
> +		err = -ENOMEM;
> +		goto exit_falcon;
> +	}
> +
> +	client->syncpts[0] = host1x_syncpt_request(client->dev, 0);
> +	if (!client->syncpts[0]) {
> +		err = -ENOMEM;
> +		goto free_channel;
> +	}
> +
> +	err = tegra_drm_register_client(tegra, drm);
> +	if (err < 0)
> +		goto free_syncpt;
> +
> +	return 0;
> +
> +free_syncpt:
> +	host1x_syncpt_free(client->syncpts[0]);
> +free_channel:
> +	host1x_channel_free(vic->channel);
> +exit_falcon:
> +	falcon_exit(&vic->falcon);
> +detach_device:
> +	if (tegra->domain)
> +		iommu_detach_device(tegra->domain, vic->dev);
> +
> +	return err;
> +}
> +
> +static int vic_exit(struct host1x_client *client)
> +{
> +	struct tegra_drm_client *drm = host1x_to_drm_client(client);
> +	struct drm_device *dev = dev_get_drvdata(client->parent);
> +	struct tegra_drm *tegra = dev->dev_private;
> +	struct vic *vic = to_vic(drm);
> +	int err;
> +
> +	err = tegra_drm_unregister_client(tegra, drm);
> +	if (err < 0)
> +		return err;
> +
> +	host1x_syncpt_free(client->syncpts[0]);
> +	host1x_channel_free(vic->channel);
> +
> +	if (vic->booted) {
> +		reset_control_assert(vic->rst);
> +		usleep_range(10, 100);
> +		reset_control_deassert(vic->rst);
> +	}
> +
> +	falcon_exit(&vic->falcon);
> +
> +	if (vic->domain) {
> +		iommu_detach_device(vic->domain, vic->dev);
> +		vic->domain = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct host1x_client_ops vic_client_ops = {
> +	.init = vic_init,
> +	.exit = vic_exit,
> +};
> +
> +static int vic_open_channel(struct tegra_drm_client *client,
> +			    struct tegra_drm_context *context)
> +{
> +	struct vic *vic = to_vic(client);
> +	int err;
> +
> +	err = pm_runtime_get_sync(vic->dev);
> +	if (err < 0)
> +		return err;
> +
> +	/*
> +	 * Try to boot the Falcon microcontroller. Booting is deferred until
> +	 * here because the firmware might not yet be available during system
> +	 * boot, for example if it's on remote storage.
> +	 */
> +	err = vic_boot(vic);
> +	if (err < 0) {
> +		pm_runtime_put(vic->dev);
> +		return err;
> +	}
> +
> +	context->channel = host1x_channel_get(vic->channel);
> +	if (!context->channel) {
> +		pm_runtime_put(vic->dev);
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +static void vic_close_channel(struct tegra_drm_context *context)
> +{
> +	struct vic *vic = to_vic(context->client);
> +
> +	host1x_channel_put(context->channel);
> +
> +	pm_runtime_put(vic->dev);
> +}
> +
> +static int vic_is_addr_reg(struct device *dev, u32 class, u32 offset, u32 val)
> +{
> +	struct vic *vic = dev_get_drvdata(dev);
> +
> +	if (class != HOST1X_CLASS_VIC)
> +		return false;
> +
> +	/*
> +	 * Method call parameter. Check stored value to see if set method uses
> +	 * parameter as memory address.
> +	 */
> +	if (offset == FALCON_UCLASS_METHOD_DATA >> 2)
> +		return vic->method_data_is_addr_reg;
> +
> +	/* Method call number store. */
> +	if (offset == FALCON_UCLASS_METHOD_OFFSET >> 2) {
> +		u32 method = val << 2;
> +
> +		if ((method >= VIC_SET_SURFACE0_SLOT0_LUMA_OFFSET &&
> +		     method <= VIC_SET_SURFACE7_SLOT4_CHROMAV_OFFSET) ||
> +		    (method >= VIC_SET_CONFIG_STRUCT_OFFSET &&
> +		     method <= VIC_SET_OUTPUT_SURFACE_CHROMAV_OFFSET))
> +			vic->method_data_is_addr_reg = true;
> +		else
> +			vic->method_data_is_addr_reg = false;
> +	}
> +
> +	return false;
> +}
> +
> +static const struct tegra_drm_client_ops vic_ops = {
> +	.open_channel = vic_open_channel,
> +	.close_channel = vic_close_channel,
> +	.is_addr_reg = vic_is_addr_reg,
> +	.submit = tegra_drm_submit,
> +};
> +
> +static const struct vic_config vic_t124_config = {
> +	.ucode_name = "vic03_ucode.bin",
> +};
> +
> +static const struct of_device_id vic_match[] = {
> +	{ .compatible = "nvidia,tegra124-vic", .data = &vic_t124_config },
> +	{ },
> +};
> +
> +static int vic_probe(struct platform_device *pdev)
> +{
> +	struct vic_config *vic_config = NULL;
> +	struct device *dev = &pdev->dev;
> +	struct host1x_syncpt **syncpts;
> +	struct resource *regs;
> +	const struct of_device_id *match;
> +	struct vic *vic;
> +	int err;
> +
> +	match = of_match_device(vic_match, dev);
> +	vic_config = (struct vic_config *)match->data;
> +
> +	vic = devm_kzalloc(dev, sizeof(*vic), GFP_KERNEL);
> +	if (!vic)
> +		return -ENOMEM;
> +
> +	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;
> +	}
> +
> +	vic->regs = devm_ioremap_resource(dev, regs);
> +	if (IS_ERR(vic->regs))
> +		return PTR_ERR(vic->regs);
> +
> +	vic->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(vic->clk)) {
> +		dev_err(&pdev->dev, "failed to get clock\n");
> +		return PTR_ERR(vic->clk);
> +	}
> +
> +	vic->rst = devm_reset_control_get(dev, "vic");
> +	if (IS_ERR(vic->rst)) {
> +		dev_err(&pdev->dev, "cannot get reset\n");
> +		return PTR_ERR(vic->rst);
> +	}
> +
> +	platform_set_drvdata(pdev, vic);
> +
> +	INIT_LIST_HEAD(&vic->client.base.list);
> +	vic->client.base.ops = &vic_client_ops;
> +	vic->client.base.dev = dev;
> +	vic->client.base.class = HOST1X_CLASS_VIC;
> +	vic->client.base.syncpts = syncpts;
> +	vic->client.base.num_syncpts = 1;
> +	vic->dev = dev;
> +	vic->config = vic_config;
> +
> +	INIT_LIST_HEAD(&vic->client.list);
> +	vic->client.ops = &vic_ops;
> +
> +	err = host1x_client_register(&vic->client.base);
> +	if (err < 0) {
> +		dev_err(dev, "failed to register host1x client: %d\n", err);
> +		platform_set_drvdata(pdev, NULL);
> +		return err;
> +	}
> +
> +	pm_runtime_enable(&pdev->dev);
> +	if (!pm_runtime_enabled(&pdev->dev)) {
> +		err = vic_runtime_resume(&pdev->dev);
> +		if (err < 0)
> +			goto unregister_client;
> +	}

I don't see why pm_runtime_enable() should ever fail to enable
pm_runtime here. Hence, the if-statement appears to be redundant. If you
are trying to determine whether rpm is supported for the power-domains
then you should simply check to see if the DT node for the device has
the "power-domains" property. See my series [1].

Cheers
Jon

[1] http://www.spinics.net/lists/arm-kernel/msg431051.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/4] drm/tegra: Add VIC support

[Mikko Perttunen]

On 07/20/2015 11:28 AM, Jon Hunter wrote:
> Hi Mikko,

Hi!

> ...
>> +static int vic_runtime_resume(struct device *dev)
>> +{
>> +	struct vic *vic = dev_get_drvdata(dev);
>> +	int err;
>> +
>> +	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
>> +						vic->clk, vic->rst);
> 
> I would like to deprecate use of the above function [1]. I have been
> trying to migrate driver to use a new API instead of the above.

Yes, we will need to coordinate the API change here. I kept the old way
here to not depend on your series for now.

> 
>> +	if (err < 0)
>> +		dev_err(dev, "failed to power up device\n");
>> +
>> +	return err;
>> +}
>> +
>> ...
>> +
>> +	pm_runtime_enable(&pdev->dev);
>> +	if (!pm_runtime_enabled(&pdev->dev)) {
>> +		err = vic_runtime_resume(&pdev->dev);
>> +		if (err < 0)
>> +			goto unregister_client;
>> +	}
> 
> I don't see why pm_runtime_enable() should ever fail to enable
> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> are trying to determine whether rpm is supported for the power-domains
> then you should simply check to see if the DT node for the device has
> the "power-domains" property. See my series [1].

The intention is that if runtime PM is not enabled, the power domain is
still brought up. On a cursory look, it seems like with your patch, this
should indeed work fine without this check if CONFIG_PM is enabled. Now,
that might always be enabled? If so, then everything would be fine; if
this lands after your series, we could even just make the power-domains
prop mandatory and not implement the legacy mode at all. If not, then
AFAIU we must still keep this path.

> 
> Cheers
> Jon
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg431051.html
> 

Cheers,
Mikko.

Re: [PATCH v2 2/4] drm/tegra: Add VIC support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 16040 bytes --]

On Mon, Jul 20, 2015 at 09:28:36AM +0100, Jon Hunter wrote:
> Hi Mikko,
> 
> On 20/07/15 08:54, Mikko Perttunen wrote:
> > From: Arto Merilainen <amerilainen@nvidia.com>
> > 
> > This patch adds support for Video Image Compositor engine which
> > can be used for 2d operations.
> > 
> > Signed-off-by: Andrew Chew <achew@nvidia.com>
> > Signed-off-by: Arto Merilainen <amerilainen@nvidia.com>
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/Makefile |   3 +-
> >  drivers/gpu/drm/tegra/drm.c    |   7 +
> >  drivers/gpu/drm/tegra/drm.h    |   1 +
> >  drivers/gpu/drm/tegra/vic.c    | 456 +++++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/tegra/vic.h    |  35 ++++
> >  include/linux/host1x.h         |   1 +
> >  6 files changed, 502 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/tegra/vic.c
> >  create mode 100644 drivers/gpu/drm/tegra/vic.h
> > 
> > diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
> > index 93e9a4a..6af3a9a 100644
> > --- a/drivers/gpu/drm/tegra/Makefile
> > +++ b/drivers/gpu/drm/tegra/Makefile
> > @@ -14,6 +14,7 @@ tegra-drm-y := \
> >  	dpaux.o \
> >  	gr2d.o \
> >  	gr3d.o \
> > -	falcon.o
> > +	falcon.o \
> > +	vic.o
> >  
> >  obj-$(CONFIG_DRM_TEGRA) += tegra-drm.o
> > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > index af4ff86..bc8cc2a 100644
> > --- a/drivers/gpu/drm/tegra/drm.c
> > +++ b/drivers/gpu/drm/tegra/drm.c
> > @@ -1145,6 +1145,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
> >  	{ .compatible = "nvidia,tegra124-dc", },
> >  	{ .compatible = "nvidia,tegra124-sor", },
> >  	{ .compatible = "nvidia,tegra124-hdmi", },
> > +	{ .compatible = "nvidia,tegra124-vic", },
> >  	{ /* sentinel */ }
> >  };
> >  
> > @@ -1194,8 +1195,14 @@ static int __init host1x_drm_init(void)
> >  	if (err < 0)
> >  		goto unregister_gr2d;
> >  
> > +	err = platform_driver_register(&tegra_vic_driver);
> > +	if (err < 0)
> > +		goto unregister_gr3d;
> > +
> >  	return 0;
> >  
> > +unregister_gr3d:
> > +	platform_driver_unregister(&tegra_gr3d_driver);
> >  unregister_gr2d:
> >  	platform_driver_unregister(&tegra_gr2d_driver);
> >  unregister_dpaux:
> > diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> > index 58c83b11..2fc7e42 100644
> > --- a/drivers/gpu/drm/tegra/drm.h
> > +++ b/drivers/gpu/drm/tegra/drm.h
> > @@ -284,5 +284,6 @@ extern struct platform_driver tegra_hdmi_driver;
> >  extern struct platform_driver tegra_dpaux_driver;
> >  extern struct platform_driver tegra_gr2d_driver;
> >  extern struct platform_driver tegra_gr3d_driver;
> > +extern struct platform_driver tegra_vic_driver;
> >  
> >  #endif /* HOST1X_DRM_H */
> > diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> > new file mode 100644
> > index 0000000..fce7c04
> > --- /dev/null
> > +++ b/drivers/gpu/drm/tegra/vic.c
> > @@ -0,0 +1,456 @@
> > +/*
> > + * Copyright (c) 2015, NVIDIA Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/host1x.h>
> > +#include <linux/iommu.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +
> > +#include <soc/tegra/pmc.h>
> > +
> > +#include "drm.h"
> > +#include "falcon.h"
> > +#include "vic.h"
> > +
> > +struct vic_config {
> > +	/* Firmware name */
> > +	const char *ucode_name;
> > +};
> > +
> > +struct vic {
> > +	struct falcon falcon;
> > +	bool booted;
> > +
> > +	void __iomem *regs;
> > +	struct tegra_drm_client client;
> > +	struct host1x_channel *channel;
> > +	struct iommu_domain *domain;
> > +	struct device *dev;
> > +	struct clk *clk;
> > +	struct reset_control *rst;
> > +
> > +	/* Platform configuration */
> > +	const struct vic_config *config;
> > +
> > +	/* for firewall - this determines if method 1 should be regarded
> > +	 * as an address register */
> > +	bool method_data_is_addr_reg;
> > +};
> > +
> > +static inline struct vic *to_vic(struct tegra_drm_client *client)
> > +{
> > +	return container_of(client, struct vic, client);
> > +}
> > +
> > +static void vic_writel(struct vic *vic, u32 value, unsigned int offset)
> > +{
> > +	writel(value, vic->regs + offset);
> > +}
> > +
> > +static int vic_runtime_resume(struct device *dev)
> > +{
> > +	struct vic *vic = dev_get_drvdata(dev);
> > +	int err;
> > +
> > +	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> > +						vic->clk, vic->rst);
> 
> I would like to deprecate use of the above function [1]. I have been
> trying to migrate driver to use a new API instead of the above.
> 
> > +	if (err < 0)
> > +		dev_err(dev, "failed to power up device\n");
> > +
> > +	return err;
> > +}
> > +
> > +static int vic_runtime_suspend(struct device *dev)
> > +{
> > +	struct vic *vic = dev_get_drvdata(dev);
> > +
> > +	clk_disable_unprepare(vic->clk);
> > +	reset_control_assert(vic->rst);
> > +	tegra_powergate_power_off(TEGRA_POWERGATE_VIC);
> 
> Similarly here. I would like to get rid of the above API in favour of a
> different API to help migrate to generic power domain support.
> 
> > +	vic->booted = false;
> > +
> > +	return 0;
> > +}
> > +
> > +static int vic_boot(struct vic *vic)
> > +{
> > +	u32 fce_ucode_size, fce_bin_data_offset;
> > +	void *hdr;
> > +	int err = 0;
> > +
> > +	if (vic->booted)
> > +		return 0;
> > +
> > +	if (!vic->falcon.firmware.valid) {
> > +		err = falcon_read_firmware(&vic->falcon,
> > +					   vic->config->ucode_name);
> > +		if (err < 0)
> > +			return err;
> > +	}
> > +
> > +	/* ensure that the engine is in sane state */
> > +	reset_control_assert(vic->rst);
> > +	usleep_range(10, 100);
> > +	reset_control_deassert(vic->rst);
> > +
> > +	/* setup clockgating registers */
> > +	vic_writel(vic, CG_IDLE_CG_DLY_CNT(4) |
> > +			CG_IDLE_CG_EN |
> > +			CG_WAKEUP_DLY_CNT(4),
> > +		   NV_PVIC_MISC_PRI_VIC_CG);
> > +
> > +	err = falcon_boot(&vic->falcon);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	hdr = vic->falcon.firmware.vaddr;
> > +	fce_bin_data_offset = *(u32 *)(hdr + VIC_UCODE_FCE_DATA_OFFSET);
> > +	hdr = vic->falcon.firmware.vaddr +
> > +		*(u32 *)(hdr + VIC_UCODE_FCE_HEADER_OFFSET);
> > +	fce_ucode_size = *(u32 *)(hdr + FCE_UCODE_SIZE_OFFSET);
> > +
> > +	falcon_execute_method(&vic->falcon, VIC_SET_APPLICATION_ID, 1);
> > +	falcon_execute_method(&vic->falcon, VIC_SET_FCE_UCODE_SIZE,
> > +			      fce_ucode_size);
> > +	falcon_execute_method(&vic->falcon, VIC_SET_FCE_UCODE_OFFSET,
> > +			      (vic->falcon.firmware.paddr + fce_bin_data_offset)
> > +				>> 8);
> > +
> > +	err = falcon_wait_idle(&vic->falcon);
> > +	if (err < 0) {
> > +		dev_err(vic->dev,
> > +			"failed to set application ID and FCE base\n");
> > +		return err;
> > +	}
> > +
> > +	vic->booted = true;
> > +
> > +	return 0;
> > +}
> > +
> > +static void *vic_falcon_alloc(struct falcon *falcon, size_t size,
> > +			       dma_addr_t *iova)
> > +{
> > +	struct tegra_drm *tegra = falcon->data;
> > +
> > +	return tegra_drm_alloc(tegra, size, iova);
> > +}
> > +
> > +static void vic_falcon_free(struct falcon *falcon, size_t size,
> > +			    dma_addr_t iova, void *va)
> > +{
> > +	struct tegra_drm *tegra = falcon->data;
> > +
> > +	return tegra_drm_free(tegra, size, va, iova);
> > +}
> > +
> > +static const struct falcon_ops vic_falcon_ops = {
> > +	.alloc = vic_falcon_alloc,
> > +	.free = vic_falcon_free
> > +};
> > +
> > +static int vic_init(struct host1x_client *client)
> > +{
> > +	struct tegra_drm_client *drm = host1x_to_drm_client(client);
> > +	struct drm_device *dev = dev_get_drvdata(client->parent);
> > +	struct tegra_drm *tegra = dev->dev_private;
> > +	struct vic *vic = to_vic(drm);
> > +	int err;
> > +
> > +	if (tegra->domain) {
> > +		err = iommu_attach_device(tegra->domain, vic->dev);
> > +		if (err < 0) {
> > +			dev_err(vic->dev, "failed to attach to domain: %d\n",
> > +				err);
> > +			return err;
> > +		}
> > +
> > +		vic->domain = tegra->domain;
> > +	}
> > +
> > +	vic->falcon.dev = vic->dev;
> > +	vic->falcon.regs = vic->regs;
> > +	vic->falcon.data = tegra;
> > +	vic->falcon.ops = &vic_falcon_ops;
> > +	err = falcon_init(&vic->falcon);
> > +	if (err < 0)
> > +		goto detach_device;
> > +
> > +	vic->channel = host1x_channel_request(client->dev);
> > +	if (!vic->channel) {
> > +		err = -ENOMEM;
> > +		goto exit_falcon;
> > +	}
> > +
> > +	client->syncpts[0] = host1x_syncpt_request(client->dev, 0);
> > +	if (!client->syncpts[0]) {
> > +		err = -ENOMEM;
> > +		goto free_channel;
> > +	}
> > +
> > +	err = tegra_drm_register_client(tegra, drm);
> > +	if (err < 0)
> > +		goto free_syncpt;
> > +
> > +	return 0;
> > +
> > +free_syncpt:
> > +	host1x_syncpt_free(client->syncpts[0]);
> > +free_channel:
> > +	host1x_channel_free(vic->channel);
> > +exit_falcon:
> > +	falcon_exit(&vic->falcon);
> > +detach_device:
> > +	if (tegra->domain)
> > +		iommu_detach_device(tegra->domain, vic->dev);
> > +
> > +	return err;
> > +}
> > +
> > +static int vic_exit(struct host1x_client *client)
> > +{
> > +	struct tegra_drm_client *drm = host1x_to_drm_client(client);
> > +	struct drm_device *dev = dev_get_drvdata(client->parent);
> > +	struct tegra_drm *tegra = dev->dev_private;
> > +	struct vic *vic = to_vic(drm);
> > +	int err;
> > +
> > +	err = tegra_drm_unregister_client(tegra, drm);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	host1x_syncpt_free(client->syncpts[0]);
> > +	host1x_channel_free(vic->channel);
> > +
> > +	if (vic->booted) {
> > +		reset_control_assert(vic->rst);
> > +		usleep_range(10, 100);
> > +		reset_control_deassert(vic->rst);
> > +	}
> > +
> > +	falcon_exit(&vic->falcon);
> > +
> > +	if (vic->domain) {
> > +		iommu_detach_device(vic->domain, vic->dev);
> > +		vic->domain = NULL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct host1x_client_ops vic_client_ops = {
> > +	.init = vic_init,
> > +	.exit = vic_exit,
> > +};
> > +
> > +static int vic_open_channel(struct tegra_drm_client *client,
> > +			    struct tegra_drm_context *context)
> > +{
> > +	struct vic *vic = to_vic(client);
> > +	int err;
> > +
> > +	err = pm_runtime_get_sync(vic->dev);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	/*
> > +	 * Try to boot the Falcon microcontroller. Booting is deferred until
> > +	 * here because the firmware might not yet be available during system
> > +	 * boot, for example if it's on remote storage.
> > +	 */
> > +	err = vic_boot(vic);
> > +	if (err < 0) {
> > +		pm_runtime_put(vic->dev);
> > +		return err;
> > +	}
> > +
> > +	context->channel = host1x_channel_get(vic->channel);
> > +	if (!context->channel) {
> > +		pm_runtime_put(vic->dev);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void vic_close_channel(struct tegra_drm_context *context)
> > +{
> > +	struct vic *vic = to_vic(context->client);
> > +
> > +	host1x_channel_put(context->channel);
> > +
> > +	pm_runtime_put(vic->dev);
> > +}
> > +
> > +static int vic_is_addr_reg(struct device *dev, u32 class, u32 offset, u32 val)
> > +{
> > +	struct vic *vic = dev_get_drvdata(dev);
> > +
> > +	if (class != HOST1X_CLASS_VIC)
> > +		return false;
> > +
> > +	/*
> > +	 * Method call parameter. Check stored value to see if set method uses
> > +	 * parameter as memory address.
> > +	 */
> > +	if (offset == FALCON_UCLASS_METHOD_DATA >> 2)
> > +		return vic->method_data_is_addr_reg;
> > +
> > +	/* Method call number store. */
> > +	if (offset == FALCON_UCLASS_METHOD_OFFSET >> 2) {
> > +		u32 method = val << 2;
> > +
> > +		if ((method >= VIC_SET_SURFACE0_SLOT0_LUMA_OFFSET &&
> > +		     method <= VIC_SET_SURFACE7_SLOT4_CHROMAV_OFFSET) ||
> > +		    (method >= VIC_SET_CONFIG_STRUCT_OFFSET &&
> > +		     method <= VIC_SET_OUTPUT_SURFACE_CHROMAV_OFFSET))
> > +			vic->method_data_is_addr_reg = true;
> > +		else
> > +			vic->method_data_is_addr_reg = false;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static const struct tegra_drm_client_ops vic_ops = {
> > +	.open_channel = vic_open_channel,
> > +	.close_channel = vic_close_channel,
> > +	.is_addr_reg = vic_is_addr_reg,
> > +	.submit = tegra_drm_submit,
> > +};
> > +
> > +static const struct vic_config vic_t124_config = {
> > +	.ucode_name = "vic03_ucode.bin",
> > +};
> > +
> > +static const struct of_device_id vic_match[] = {
> > +	{ .compatible = "nvidia,tegra124-vic", .data = &vic_t124_config },
> > +	{ },
> > +};
> > +
> > +static int vic_probe(struct platform_device *pdev)
> > +{
> > +	struct vic_config *vic_config = NULL;
> > +	struct device *dev = &pdev->dev;
> > +	struct host1x_syncpt **syncpts;
> > +	struct resource *regs;
> > +	const struct of_device_id *match;
> > +	struct vic *vic;
> > +	int err;
> > +
> > +	match = of_match_device(vic_match, dev);
> > +	vic_config = (struct vic_config *)match->data;
> > +
> > +	vic = devm_kzalloc(dev, sizeof(*vic), GFP_KERNEL);
> > +	if (!vic)
> > +		return -ENOMEM;
> > +
> > +	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;
> > +	}
> > +
> > +	vic->regs = devm_ioremap_resource(dev, regs);
> > +	if (IS_ERR(vic->regs))
> > +		return PTR_ERR(vic->regs);
> > +
> > +	vic->clk = devm_clk_get(dev, NULL);
> > +	if (IS_ERR(vic->clk)) {
> > +		dev_err(&pdev->dev, "failed to get clock\n");
> > +		return PTR_ERR(vic->clk);
> > +	}
> > +
> > +	vic->rst = devm_reset_control_get(dev, "vic");
> > +	if (IS_ERR(vic->rst)) {
> > +		dev_err(&pdev->dev, "cannot get reset\n");
> > +		return PTR_ERR(vic->rst);
> > +	}
> > +
> > +	platform_set_drvdata(pdev, vic);
> > +
> > +	INIT_LIST_HEAD(&vic->client.base.list);
> > +	vic->client.base.ops = &vic_client_ops;
> > +	vic->client.base.dev = dev;
> > +	vic->client.base.class = HOST1X_CLASS_VIC;
> > +	vic->client.base.syncpts = syncpts;
> > +	vic->client.base.num_syncpts = 1;
> > +	vic->dev = dev;
> > +	vic->config = vic_config;
> > +
> > +	INIT_LIST_HEAD(&vic->client.list);
> > +	vic->client.ops = &vic_ops;
> > +
> > +	err = host1x_client_register(&vic->client.base);
> > +	if (err < 0) {
> > +		dev_err(dev, "failed to register host1x client: %d\n", err);
> > +		platform_set_drvdata(pdev, NULL);
> > +		return err;
> > +	}
> > +
> > +	pm_runtime_enable(&pdev->dev);
> > +	if (!pm_runtime_enabled(&pdev->dev)) {
> > +		err = vic_runtime_resume(&pdev->dev);
> > +		if (err < 0)
> > +			goto unregister_client;
> > +	}
> 
> I don't see why pm_runtime_enable() should ever fail to enable
> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> are trying to determine whether rpm is supported for the power-domains
> then you should simply check to see if the DT node for the device has
> the "power-domains" property. See my series [1].

Merely checking for a device tree property won't tell you anything.
There are no guarantees that some driver will make the power domains
available, even if they are defined in the DT.

Generally checking device tree properties is a bad idea. You should only
ever rely on whatever mechanism the operating system exposed as a result
of such properties instead.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/4] drm/tegra: Add VIC support

[Jon Hunter]

On 20/07/15 09:51, Mikko Perttunen wrote:
> On 07/20/2015 11:28 AM, Jon Hunter wrote:
>> Hi Mikko,
> 
> Hi!
> 
>> ...
>>> +static int vic_runtime_resume(struct device *dev)
>>> +{
>>> +	struct vic *vic = dev_get_drvdata(dev);
>>> +	int err;
>>> +
>>> +	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
>>> +						vic->clk, vic->rst);
>>
>> I would like to deprecate use of the above function [1]. I have been
>> trying to migrate driver to use a new API instead of the above.
> 
> Yes, we will need to coordinate the API change here. I kept the old way
> here to not depend on your series for now.
> 
>>
>>> +	if (err < 0)
>>> +		dev_err(dev, "failed to power up device\n");
>>> +
>>> +	return err;
>>> +}
>>> +
>>> ...
>>> +
>>> +	pm_runtime_enable(&pdev->dev);
>>> +	if (!pm_runtime_enabled(&pdev->dev)) {
>>> +		err = vic_runtime_resume(&pdev->dev);
>>> +		if (err < 0)
>>> +			goto unregister_client;
>>> +	}
>>
>> I don't see why pm_runtime_enable() should ever fail to enable
>> pm_runtime here. Hence, the if-statement appears to be redundant. If you
>> are trying to determine whether rpm is supported for the power-domains
>> then you should simply check to see if the DT node for the device has
>> the "power-domains" property. See my series [1].
> 
> The intention is that if runtime PM is not enabled, the power domain is
> still brought up. On a cursory look, it seems like with your patch, this
> should indeed work fine without this check if CONFIG_PM is enabled. Now,
> that might always be enabled? If so, then everything would be fine; if
> this lands after your series, we could even just make the power-domains
> prop mandatory and not implement the legacy mode at all. If not, then
> AFAIU we must still keep this path.

Ok, I see you just wish to test it is enabled in the kernel config. Then
yes that would make sense and the above is fine.

Cheers
Jon
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/4] drm/tegra: Add VIC support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2679 bytes --]

On Mon, Jul 20, 2015 at 10:56:00AM +0100, Jon Hunter wrote:
> 
> On 20/07/15 09:51, Mikko Perttunen wrote:
> > On 07/20/2015 11:28 AM, Jon Hunter wrote:
> >> Hi Mikko,
> > 
> > Hi!
> > 
> >> ...
> >>> +static int vic_runtime_resume(struct device *dev)
> >>> +{
> >>> +	struct vic *vic = dev_get_drvdata(dev);
> >>> +	int err;
> >>> +
> >>> +	err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VIC,
> >>> +						vic->clk, vic->rst);
> >>
> >> I would like to deprecate use of the above function [1]. I have been
> >> trying to migrate driver to use a new API instead of the above.
> > 
> > Yes, we will need to coordinate the API change here. I kept the old way
> > here to not depend on your series for now.
> > 
> >>
> >>> +	if (err < 0)
> >>> +		dev_err(dev, "failed to power up device\n");
> >>> +
> >>> +	return err;
> >>> +}
> >>> +
> >>> ...
> >>> +
> >>> +	pm_runtime_enable(&pdev->dev);
> >>> +	if (!pm_runtime_enabled(&pdev->dev)) {
> >>> +		err = vic_runtime_resume(&pdev->dev);
> >>> +		if (err < 0)
> >>> +			goto unregister_client;
> >>> +	}
> >>
> >> I don't see why pm_runtime_enable() should ever fail to enable
> >> pm_runtime here. Hence, the if-statement appears to be redundant. If you
> >> are trying to determine whether rpm is supported for the power-domains
> >> then you should simply check to see if the DT node for the device has
> >> the "power-domains" property. See my series [1].
> > 
> > The intention is that if runtime PM is not enabled, the power domain is
> > still brought up. On a cursory look, it seems like with your patch, this
> > should indeed work fine without this check if CONFIG_PM is enabled. Now,
> > that might always be enabled? If so, then everything would be fine; if
> > this lands after your series, we could even just make the power-domains
> > prop mandatory and not implement the legacy mode at all. If not, then
> > AFAIU we must still keep this path.
> 
> Ok, I see you just wish to test it is enabled in the kernel config. Then
> yes that would make sense and the above is fine.

Do we really need to bother about this? If runtime PM isn't enabled we
don't care very much about power management anyway. So in my opinion if
a driver supports runtime PM it should support it all the way and not
pretend to. Having this mix of runtime PM vs. no runtime PM is beside
the point of using something like runtime PM in the first place.

So if this is about supporting legacy vs. runtime PM, then I think it
should be based on some more explicit check, like if (!dev->pm_domain),
but otherwise we should assume that pm_runtime_enable() succeeds.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 3/4] of: Add NVIDIA Tegra VIC binding

[Emil Velikov]

Hi Mikko,

On 20 July 2015 at 08:54, Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> This adds device tree binding documentation for the Video Image
> Compositor (VIC) present on Tegra124 and newer SoC's.
>
> Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/gpu/nvidia,tegra20-host1x.txt     | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> index 009f4bf..1328f3f 100644
> --- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> @@ -240,6 +240,21 @@ of the following host1x client modules:
>      - dpaux
>    - vdd-supply: phandle of a supply that powers the DisplayPort link
>
> +- vic: Video Image Compositor
> +  - compatible: For Tegra124, must contain "nvidia,tegra124-vic".  Otherwise,
> +    must contain '"nvidia,<chip>-vic", "nvidia,tegra124-vic"', where
> +    <chip> is tegra132.
Did you make a typo here, or is "tegra124" string really used to
identify the tegra132 chips ?

Thanks
Emil

Re: [PATCH v2 3/4] of: Add NVIDIA Tegra VIC binding

[Mikko Perttunen]

On 07/20/2015 08:20 PM, Emil Velikov wrote:
> Hi Mikko,
> 
> On 20 July 2015 at 08:54, Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> This adds device tree binding documentation for the Video Image
>> Compositor (VIC) present on Tegra124 and newer SoC's.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  .../devicetree/bindings/gpu/nvidia,tegra20-host1x.txt     | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
>> index 009f4bf..1328f3f 100644
>> --- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
>> +++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
>> @@ -240,6 +240,21 @@ of the following host1x client modules:
>>      - dpaux
>>    - vdd-supply: phandle of a supply that powers the DisplayPort link
>>
>> +- vic: Video Image Compositor
>> +  - compatible: For Tegra124, must contain "nvidia,tegra124-vic".  Otherwise,
>> +    must contain '"nvidia,<chip>-vic", "nvidia,tegra124-vic"', where
>> +    <chip> is tegra132.
> Did you make a typo here, or is "tegra124" string really used to
> identify the tegra132 chips ?

The idea here is that the compatible string must either be
  "nvidia,tegra124-vic"
if the board is a Tegra124 chip. If it's not, the board must be a
Tegra132 (though we probably should add Tegra210 here too) it must be
  "nvidia,tegra132-vic", "nvidia,tegra124-vic"
to indicate that while the hardware is compatible with the Tegra124 VIC
(and thus can use the same driver and we don't need to update the
compatible list in the driver), it is actually the Tegra132 version in
case the driver needs to apply some quirks. Other Tegra bindings use the
same system.

> 
> Thanks
> Emil

Thanks,
Mikko