On Sat, Nov 24, 2012 at 09:09:11AM +0200, Terje Bergström wrote:
> On 24.11.2012 01:45, Thierry Reding wrote:
> > 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.
>
> True. I didn't yet have time to implement dynamic allocation and I
> didn't want to wait for that to get the code out. I'll try to find time
> for that.
Okay, sounds good. I agree that it was good to post these patches soon.
The earlier they can be discussed and reviewed the better.
> >> +
> >> +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 host1x specifics (number of channels, pts etc) are description of
> hardware, so they could go to a device tree binding.
I'm not sure that's even required. The number of syncpoints and channels
should be static for a particular version of SoC, right? In that case it
can be derived from the DT compatible property, can't it?
> The allocation of sync points is a software policy that could be handled
> by the driver.
>
> Clocks are waiting for the general discussion about clock bindings in
> device trees, but that data is SoC specific, so that's why I've put that
> in board data.
>
> Clock gating policy is software feature that could be in driver.
>
> >> 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?
>
> I think we had problems in driver acquiring the clocks without this
> (mismatch of driver), but I could retry. The old bindings assumed that
> driver would be called "tegra_grhost", but that's not the case.
I hope that DT clock support will go into 3.9, in which case the issue
will just go away.
Thierry
Attachment:
pgpptU75Z97qU.pgp
Description: PGP signature
