Re: [PATCH 2/6] ARM: tegra: Add auxiliary data for nvhost

[Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

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

On Thu, Nov 22, 2012 at 11:48:00AM +0200, Terje Bergstrom wrote:
> Add SoC specific auxiliary data to host1x and gr2d. nvhost uses
> this data.
> 
> Change-Id: Idb04b262c8b6432e56cffb6c7ed64cf7ef4545b3
> Signed-off-by: Terje Bergstrom <tbergstrom-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Arto Merilainen <amerilainen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm/mach-tegra/board-dt-tegra20.c    |   38 ++++++++++++++++++++++++++++-
>  arch/arm/mach-tegra/board-dt-tegra30.c    |   38 ++++++++++++++++++++++++++++-
>  arch/arm/mach-tegra/tegra20_clocks_data.c |    8 +++---
>  arch/arm/mach-tegra/tegra30_clocks_data.c |    2 ++
>  4 files changed, 80 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c
> index 1d30eac..c695392 100644
> --- a/arch/arm/mach-tegra/board-dt-tegra20.c
> +++ b/arch/arm/mach-tegra/board-dt-tegra20.c
> @@ -33,6 +33,7 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c-tegra.h>
>  #include <linux/usb/tegra_usb_phy.h>
> +#include <linux/nvhost.h>
>  
>  #include <asm/hardware/gic.h>
>  #include <asm/mach-types.h>
> @@ -45,6 +46,38 @@
>  #include "common.h"
>  #include "iomap.h"
>  
> +static const char *host1x_syncpt_names[32] = {
> +	[0] = "gfx_host",
> +	[NVSYNCPT_2D_0] = "2d_0",
> +	[NVSYNCPT_2D_1] = "2d_1",
> +	[NVSYNCPT_VBLANK0] = "vblank0",
> +	[NVSYNCPT_VBLANK1] = "vblank1",
> +};

I think I remember a discussion about this back when we designed the DT
bindings. The result I seem to remember was that since syncpoints can be
arbitrarily assigned they wouldn't have to be statically allocated at
compile time.

Instead we could just have them allocated when a host1x client registers
with host1x. Or have each client request its required syncpoints
explicitly. The latter would make it trivial to associate a label with
it.

> +
> +static struct host1x_device_info host1x_info = {
> +	.nb_channels	= 8,
> +	.nb_pts		= 32,
> +	.nb_mlocks	= 16,
> +	.nb_bases	= 8,
> +	.syncpt_names	= host1x_syncpt_names,
> +	.client_managed	= NVSYNCPTS_CLIENT_MANAGED,
> +};
> +
> +static struct nvhost_device_data tegra_host1x_info = {
> +	.clocks = { {"host1x", UINT_MAX} },
> +	NVHOST_MODULE_NO_POWERGATE_IDS,
> +	.private_data = &host1x_info,
> +};
> +
> +static struct nvhost_device_data tegra_gr2d_info = {
> +	.index		= 2,
> +	.syncpts	= BIT(NVSYNCPT_2D_0) | BIT(NVSYNCPT_2D_1),
> +	.clocks		= { {"gr2d", UINT_MAX, true}, {"epp", UINT_MAX, true} },
> +	NVHOST_MODULE_NO_POWERGATE_IDS,
> +	.clockgate_delay = 0,
> +	.serialize	= true,
> +};

Again, this doesn't seem like it should be statically configured in the
board data.

The same comments apply to the Tegra30 hunk that I've left out.

> diff --git a/arch/arm/mach-tegra/tegra20_clocks_data.c b/arch/arm/mach-tegra/tegra20_clocks_data.c
> index 7f049ac..3314e50 100644
> --- a/arch/arm/mach-tegra/tegra20_clocks_data.c
> +++ b/arch/arm/mach-tegra/tegra20_clocks_data.c
> @@ -1041,10 +1041,10 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
>  	CLK_DUPLICATE("usbd",	"utmip-pad",	NULL),
>  	CLK_DUPLICATE("usbd",	"tegra-ehci.0",	NULL),
>  	CLK_DUPLICATE("usbd",	"tegra-otg",	NULL),
> -	CLK_DUPLICATE("2d",	"tegra_grhost",	"gr2d"),
> -	CLK_DUPLICATE("3d",	"tegra_grhost",	"gr3d"),
> -	CLK_DUPLICATE("epp",	"tegra_grhost",	"epp"),
> -	CLK_DUPLICATE("mpe",	"tegra_grhost",	"mpe"),
> +	CLK_DUPLICATE("2d",	NULL,		"gr2d"),
> +	CLK_DUPLICATE("3d",	NULL,		"gr3d"),
> +	CLK_DUPLICATE("epp",	NULL,		"epp"),
> +	CLK_DUPLICATE("mpe",	NULL,		"mpe"),

Are these actually required here?

>  	CLK_DUPLICATE("cop",	"tegra-avp",	"cop"),
>  	CLK_DUPLICATE("vde",	"tegra-aes",	"vde"),
>  	CLK_DUPLICATE("cclk",	NULL,		"cpu"),
> diff --git a/arch/arm/mach-tegra/tegra30_clocks_data.c b/arch/arm/mach-tegra/tegra30_clocks_data.c
> index 6942c7a..f30bd54 100644
> --- a/arch/arm/mach-tegra/tegra30_clocks_data.c
> +++ b/arch/arm/mach-tegra/tegra30_clocks_data.c
> @@ -1338,6 +1338,8 @@ struct clk_duplicate tegra_clk_duplicates[] = {
>  	CLK_DUPLICATE("pll_p", "tegradc.0", "parent"),
>  	CLK_DUPLICATE("pll_p", "tegradc.1", "parent"),
>  	CLK_DUPLICATE("pll_d2_out0", "hdmi", "parent"),
> +	CLK_DUPLICATE("2d", NULL, "gr2d"),
> +	CLK_DUPLICATE("epp", NULL, "epp"),

Same here.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]