From: Andrew Chew <[email protected]>
Make some Tegra syncpt routines accessible by kernel drivers that need
to use the syncpt mechanism. This is similar to how some nvmap stuff
is made accessible to kernel drivers.
Change-Id: Id6cd2800e75223e5da29d6c0764bac5b233be185
Signed-off-by: Andrew Chew <[email protected]>
---
arch/arm/mach-tegra/include/mach/nvsyncpt.h | 36 +++++++++++++++++++++++++++
drivers/video/tegra/host/dev.c | 4 +++
drivers/video/tegra/host/nvhost_syncpt.c | 7 +++++
drivers/video/tegra/host/nvhost_syncpt.h | 2 +
4 files changed, 49 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-tegra/include/mach/nvsyncpt.h
diff --git a/arch/arm/mach-tegra/include/mach/nvsyncpt.h b/arch/arm/mach-tegra/include/mach/nvsyncpt.h
new file mode 100644
index 0000000..021d498
--- /dev/null
+++ b/arch/arm/mach-tegra/include/mach/nvsyncpt.h
@@ -0,0 +1,36 @@
+/*
+ * Tegra host syncpt exports
+ *
+ * Copyright (c) 2009-2010, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef __NVSYNCPT_H
+#define __NVSYNCPT_H
+
+#if defined(__KERNEL__)
+
+struct nvhost_syncpt;
+
+extern struct nvhost_syncpt *nvhost_getsyncpt(void);
+extern u32 nvhost_syncpt_read(struct nvhost_syncpt *sp, u32 id);
+extern int nvhost_syncpt_wait_timeout(struct nvhost_syncpt *sp,
+ u32 id, u32 thresh, u32 timeout);
+extern void nvhost_syncpt_cpu_incr(struct nvhost_syncpt *sp, u32 id);
+
+#endif
+
+#endif
diff --git a/drivers/video/tegra/host/dev.c b/drivers/video/tegra/host/dev.c
index 20a4eda..af70a41 100644
--- a/drivers/video/tegra/host/dev.c
+++ b/drivers/video/tegra/host/dev.c
@@ -721,6 +721,8 @@ static int __devinit nvhost_probe(struct platform_device *pdev)
nvhost_debug_init(host);
+ nvhost_syncpt = &host->syncpt;
+
dev_info(&pdev->dev, "initialized\n");
return 0;
@@ -734,6 +736,8 @@ fail:
static int __exit nvhost_remove(struct platform_device *pdev)
{
+ nvhost_syncpt = NULL;
+
return 0;
}
diff --git a/drivers/video/tegra/host/nvhost_syncpt.c b/drivers/video/tegra/host/nvhost_syncpt.c
index dd2ab0d..315050d 100644
--- a/drivers/video/tegra/host/nvhost_syncpt.c
+++ b/drivers/video/tegra/host/nvhost_syncpt.c
@@ -27,6 +27,8 @@
#define syncpt_to_dev(sp) container_of(sp, struct nvhost_master, syncpt)
#define SYNCPT_CHECK_PERIOD 2*HZ
+struct nvhost_syncpt *nvhost_syncpt;
+
static bool check_max(struct nvhost_syncpt *sp, u32 id, u32 real)
{
u32 max;
@@ -254,3 +256,8 @@ void nvhost_syncpt_debug(struct nvhost_syncpt *sp)
}
}
+
+struct nvhost_syncpt *nvhost_getsyncpt(void)
+{
+ return nvhost_syncpt;
+}
diff --git a/drivers/video/tegra/host/nvhost_syncpt.h b/drivers/video/tegra/host/nvhost_syncpt.h
index f161f20..65f0c95 100644
--- a/drivers/video/tegra/host/nvhost_syncpt.h
+++ b/drivers/video/tegra/host/nvhost_syncpt.h
@@ -72,6 +72,8 @@ struct nvhost_syncpt {
u32 base_val[NV_HOST1X_SYNCPT_NB_BASES];
};
+extern struct nvhost_syncpt *nvhost_syncpt;
+
/**
* Updates the value sent to hardware.
*/
--
1.7.0.4
If you want to use syncpts you should be an nvhost_driver link the dc.
On Mon, Feb 14, 2011 at 3:27 PM, <[email protected]> wrote:
> From: Andrew Chew <[email protected]>
>
> Make some Tegra syncpt routines accessible by kernel drivers that need
> to use the syncpt mechanism. ?This is similar to how some nvmap stuff
> is made accessible to kernel drivers.
>
> Change-Id: Id6cd2800e75223e5da29d6c0764bac5b233be185
> Signed-off-by: Andrew Chew <[email protected]>
> ---
> ?arch/arm/mach-tegra/include/mach/nvsyncpt.h | ? 36 +++++++++++++++++++++++++++
> ?drivers/video/tegra/host/dev.c ? ? ? ? ? ? ?| ? ?4 +++
> ?drivers/video/tegra/host/nvhost_syncpt.c ? ?| ? ?7 +++++
> ?drivers/video/tegra/host/nvhost_syncpt.h ? ?| ? ?2 +
> ?4 files changed, 49 insertions(+), 0 deletions(-)
> ?create mode 100644 arch/arm/mach-tegra/include/mach/nvsyncpt.h
>
> diff --git a/arch/arm/mach-tegra/include/mach/nvsyncpt.h b/arch/arm/mach-tegra/include/mach/nvsyncpt.h
> new file mode 100644
> index 0000000..021d498
> --- /dev/null
> +++ b/arch/arm/mach-tegra/include/mach/nvsyncpt.h
> @@ -0,0 +1,36 @@
> +/*
> + * Tegra host syncpt exports
> + *
> + * Copyright (c) 2009-2010, NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. ?See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA ?02110-1301, USA.
> + */
> +
> +#ifndef __NVSYNCPT_H
> +#define __NVSYNCPT_H
> +
> +#if defined(__KERNEL__)
> +
> +struct nvhost_syncpt;
> +
> +extern struct nvhost_syncpt *nvhost_getsyncpt(void);
> +extern u32 nvhost_syncpt_read(struct nvhost_syncpt *sp, u32 id);
> +extern int nvhost_syncpt_wait_timeout(struct nvhost_syncpt *sp,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u32 id, u32 thresh, u32 timeout);
> +extern void nvhost_syncpt_cpu_incr(struct nvhost_syncpt *sp, u32 id);
> +
> +#endif
> +
> +#endif
> diff --git a/drivers/video/tegra/host/dev.c b/drivers/video/tegra/host/dev.c
> index 20a4eda..af70a41 100644
> --- a/drivers/video/tegra/host/dev.c
> +++ b/drivers/video/tegra/host/dev.c
> @@ -721,6 +721,8 @@ static int __devinit nvhost_probe(struct platform_device *pdev)
>
> ? ? ? ?nvhost_debug_init(host);
>
> + ? ? ? nvhost_syncpt = &host->syncpt;
> +
> ? ? ? ?dev_info(&pdev->dev, "initialized\n");
> ? ? ? ?return 0;
>
> @@ -734,6 +736,8 @@ fail:
>
> ?static int __exit nvhost_remove(struct platform_device *pdev)
> ?{
> + ? ? ? nvhost_syncpt = NULL;
> +
> ? ? ? ?return 0;
> ?}
>
> diff --git a/drivers/video/tegra/host/nvhost_syncpt.c b/drivers/video/tegra/host/nvhost_syncpt.c
> index dd2ab0d..315050d 100644
> --- a/drivers/video/tegra/host/nvhost_syncpt.c
> +++ b/drivers/video/tegra/host/nvhost_syncpt.c
> @@ -27,6 +27,8 @@
> ?#define syncpt_to_dev(sp) container_of(sp, struct nvhost_master, syncpt)
> ?#define SYNCPT_CHECK_PERIOD 2*HZ
>
> +struct nvhost_syncpt *nvhost_syncpt;
> +
> ?static bool check_max(struct nvhost_syncpt *sp, u32 id, u32 real)
> ?{
> ? ? ? ?u32 max;
> @@ -254,3 +256,8 @@ void nvhost_syncpt_debug(struct nvhost_syncpt *sp)
>
> ? ? ? ?}
> ?}
> +
> +struct nvhost_syncpt *nvhost_getsyncpt(void)
> +{
> + ? ? ? return nvhost_syncpt;
> +}
> diff --git a/drivers/video/tegra/host/nvhost_syncpt.h b/drivers/video/tegra/host/nvhost_syncpt.h
> index f161f20..65f0c95 100644
> --- a/drivers/video/tegra/host/nvhost_syncpt.h
> +++ b/drivers/video/tegra/host/nvhost_syncpt.h
> @@ -72,6 +72,8 @@ struct nvhost_syncpt {
> ? ? ? ?u32 base_val[NV_HOST1X_SYNCPT_NB_BASES];
> ?};
>
> +extern struct nvhost_syncpt *nvhost_syncpt;
> +
> ?/**
> ?* Updates the value sent to hardware.
> ?*/
> --
> 1.7.0.4
>
>
> If you want to use syncpts you should be an nvhost_driver link the dc.
Looking at drivers/video/tegra/dc, I notice that it gets access to the syncpt functions by using a relative path (../host/dev.h) to include dev.h, which in turn includes nvhost_syncpt.h (in drivers/video/tegra/host). Is this proper form?
This syncpt access is for a Tegra V4L2 driver, which will live in drivers/media/video. I was trying to avoid using relative paths for #includes. I assumed that the header files in drivers/video/tegra/host were not meant to be publicly available to other drivers, which is why I looked for some precedent in how nvhost functions were made available to other kernel drivers, hence I modeled this after nvmap.-
Yeah, including the relative path was a bit of a hack since the nvhost
drivers needs some serious work before it's upstreamable. A "right"
way to do this is to have the syncpt functions take an nvhost_device
pointer (or create helper functions that do that,) then expose those
functions in mach/nvhost.h. This obviates the need for a global
nvhost_sycpt pointer.
-Erik
On Mon, Feb 14, 2011 at 4:08 PM, Andrew Chew <[email protected]> wrote:
>> If you want to use syncpts you should be an nvhost_driver link the dc.
>
> Looking at drivers/video/tegra/dc, I notice that it gets access to the syncpt functions by using a relative path (../host/dev.h) to include dev.h, which in turn includes nvhost_syncpt.h (in drivers/video/tegra/host). ?Is this proper form?
>
> This syncpt access is for a Tegra V4L2 driver, which will live in drivers/media/video. ?I was trying to avoid using relative paths for #includes. ?I assumed that the header files in drivers/video/tegra/host were not meant to be publicly available to other drivers, which is why I looked for some precedent in how nvhost functions were made available to other kernel drivers, hence I modeled this after nvmap.
> Yeah, including the relative path was a bit of a hack since the nvhost
> drivers needs some serious work before it's upstreamable. A "right"
> way to do this is to have the syncpt functions take an nvhost_device
> pointer (or create helper functions that do that,) then expose those
> functions in mach/nvhost.h. This obviates the need for a global
> nvhost_sycpt pointer.
>
> -Erik
You're right about that global nvhost_syncpt pointer. That drives me nuts as well. Nevertheless, I'm not sure I'm knowledgeable enough to do the required work to do this the "right" way. As far as I know, nvhost is a guaranteed singleton in practice (not in implementation), so while this is ugly, it will work fine for existing projects.
Can we use nvmap as precedent and do it this way for now? I'd like to get the V4L2 camera host driver up to the Tegra community as soon as possible. When the problem is solved for the dc driver, I can adapt that to the V4L2 camera host driver.
> On Mon, Feb 14, 2011 at 4:08 PM, Andrew Chew <[email protected]> wrote:
> >> If you want to use syncpts you should be an nvhost_driver
> link the dc.
> >
> > Looking at drivers/video/tegra/dc, I notice that it gets
> access to the syncpt functions by using a relative path
> (../host/dev.h) to include dev.h, which in turn includes
> nvhost_syncpt.h (in drivers/video/tegra/host). ?Is this proper form?
> >
> > This syncpt access is for a Tegra V4L2 driver, which will
> live in drivers/media/video. ?I was trying to avoid using
> relative paths for #includes. ?I assumed that the header
> files in drivers/video/tegra/host were not meant to be
> publicly available to other drivers, which is why I looked
> for some precedent in how nvhost functions were made
> available to other kernel drivers, hence I modeled this after nvmap.
> -
On Mon, Feb 14, 2011 at 4:47 PM, Andrew Chew <[email protected]> wrote:
>> Yeah, including the relative path was a bit of a hack since the nvhost
>> drivers needs some serious work before it's upstreamable. ?A "right"
>> way to do this is to have the syncpt functions take an nvhost_device
>> pointer (or create helper functions that do that,) then expose those
>> functions in mach/nvhost.h. ?This obviates the need for a global
>> nvhost_sycpt pointer.
>>
>> -Erik
>
> You're right about that global nvhost_syncpt pointer. ?That drives me nuts as well. ?Nevertheless, I'm not sure I'm knowledgeable enough to do the required work to do this the "right" way. ?As far as I know, nvhost is a guaranteed singleton in practice (not in implementation), so while this is ugly, it will work fine for existing projects.
If you feel uncomfortable making this change, I recommend you include
the private header file the way the dc does.
> Can we use nvmap as precedent and do it this way for now? ?I'd like to get the V4L2 camera host driver up to the Tegra community as soon as possible. ?When the problem is solved for the dc driver, I can adapt that to the V4L2 camera host driver.
nvmap (another piece of code that needs a lot of work before it's
upstream) is a horrible example to use as a precedent. This whole
discussion is somewhat moot as you're building your driver against two
pieces that have no clear path to being upstreamed. You are welcome
to make any changes you want in your own repository for supporing V4L2
but I won't be pulling this syncpt patch into
git://android.git.kernel.org/kernel/tegra.git.
-Erik
> > You're right about that global nvhost_syncpt pointer. ?That
> drives me nuts as well. ?Nevertheless, I'm not sure I'm
> knowledgeable enough to do the required work to do this the
> "right" way. ?As far as I know, nvhost is a guaranteed
> singleton in practice (not in implementation), so while this
> is ugly, it will work fine for existing projects.
>
> If you feel uncomfortable making this change, I recommend you include
> the private header file the way the dc does.
Okay, I'll do it this way then.-