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

Jon Hunter jonathanh at nvidia.com
Mon Jul 20 01:28:36 PDT 2015


Hi Mikko,

On 20/07/15 08:54, Mikko Perttunen wrote:
> From: Arto Merilainen <amerilainen at nvidia.com>
> 
> This patch adds support for Video Image Compositor engine which
> can be used for 2d operations.
> 
> Signed-off-by: Andrew Chew <achew at nvidia.com>
> Signed-off-by: Arto Merilainen <amerilainen at nvidia.com>
> Signed-off-by: Mikko Perttunen <mperttunen at 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


More information about the dri-devel mailing list