diff mbox series

[v5,08/21] gpu: host1x: Implement /dev/host1x device node

Message ID 20210111130019.3515669-9-mperttunen@nvidia.com
State Rejected
Headers show
Series [v5,01/21] gpu: host1x: Use different lock classes for each client | expand

Commit Message

Mikko Perttunen Jan. 11, 2021, 1 p.m. UTC
Add the /dev/host1x device node, implementing the following
functionality:

- Reading syncpoint values
- Allocating syncpoints (providing syncpoint FDs)
- Incrementing syncpoints (based on syncpoint FD)

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v4:
* Put UAPI under CONFIG_DRM_TEGRA_STAGING
v3:
* Pass process name as syncpoint name when allocating
  syncpoint.
---
 drivers/gpu/host1x/Makefile |   1 +
 drivers/gpu/host1x/dev.c    |   9 ++
 drivers/gpu/host1x/dev.h    |   3 +
 drivers/gpu/host1x/uapi.c   | 282 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/host1x/uapi.h   |  22 +++
 include/linux/host1x.h      |   2 +
 6 files changed, 319 insertions(+)
 create mode 100644 drivers/gpu/host1x/uapi.c
 create mode 100644 drivers/gpu/host1x/uapi.h

Comments

Thierry Reding March 23, 2021, 11:02 a.m. UTC | #1
On Mon, Jan 11, 2021 at 03:00:06PM +0200, Mikko Perttunen wrote:
> Add the /dev/host1x device node, implementing the following
> functionality:
> 
> - Reading syncpoint values
> - Allocating syncpoints (providing syncpoint FDs)
> - Incrementing syncpoints (based on syncpoint FD)
> 
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> v4:
> * Put UAPI under CONFIG_DRM_TEGRA_STAGING
> v3:
> * Pass process name as syncpoint name when allocating
>   syncpoint.
> ---
>  drivers/gpu/host1x/Makefile |   1 +
>  drivers/gpu/host1x/dev.c    |   9 ++
>  drivers/gpu/host1x/dev.h    |   3 +
>  drivers/gpu/host1x/uapi.c   | 282 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/host1x/uapi.h   |  22 +++
>  include/linux/host1x.h      |   2 +
>  6 files changed, 319 insertions(+)
>  create mode 100644 drivers/gpu/host1x/uapi.c
>  create mode 100644 drivers/gpu/host1x/uapi.h
> 
> diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile
> index 096017b8789d..882f928d75e1 100644
> --- a/drivers/gpu/host1x/Makefile
> +++ b/drivers/gpu/host1x/Makefile
> @@ -9,6 +9,7 @@ host1x-y = \
>  	job.o \
>  	debug.o \
>  	mipi.o \
> +	uapi.o \
>  	hw/host1x01.o \
>  	hw/host1x02.o \
>  	hw/host1x04.o \
> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> index d0ebb70e2fdd..641317d23828 100644
> --- a/drivers/gpu/host1x/dev.c
> +++ b/drivers/gpu/host1x/dev.c
> @@ -461,6 +461,12 @@ static int host1x_probe(struct platform_device *pdev)
>  		goto deinit_syncpt;
>  	}
>  
> +	err = host1x_uapi_init(&host->uapi, host);

It's a bit pointless to pass &host->uapi and host to the function since
you can access the former through the latter.

> +	if (err) {
> +		dev_err(&pdev->dev, "failed to initialize uapi\n");

s/uapi/UAPI/, and perhaps include the error code to give a better hint
as to why things failed.

> +		goto deinit_intr;
> +	}
> +
>  	host1x_debug_init(host);
>  
>  	if (host->info->has_hypervisor)
> @@ -480,6 +486,8 @@ static int host1x_probe(struct platform_device *pdev)
>  	host1x_unregister(host);
>  deinit_debugfs:
>  	host1x_debug_deinit(host);
> +	host1x_uapi_deinit(&host->uapi);
> +deinit_intr:
>  	host1x_intr_deinit(host);
>  deinit_syncpt:
>  	host1x_syncpt_deinit(host);
> @@ -501,6 +509,7 @@ static int host1x_remove(struct platform_device *pdev)
>  
>  	host1x_unregister(host);
>  	host1x_debug_deinit(host);
> +	host1x_uapi_deinit(&host->uapi);
>  	host1x_intr_deinit(host);
>  	host1x_syncpt_deinit(host);
>  	reset_control_assert(host->rst);
> diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
> index 63010ae37a97..7b8b7e20e32b 100644
> --- a/drivers/gpu/host1x/dev.h
> +++ b/drivers/gpu/host1x/dev.h
> @@ -17,6 +17,7 @@
>  #include "intr.h"
>  #include "job.h"
>  #include "syncpt.h"
> +#include "uapi.h"
>  
>  struct host1x_syncpt;
>  struct host1x_syncpt_base;
> @@ -143,6 +144,8 @@ struct host1x {
>  	struct list_head list;
>  
>  	struct device_dma_parameters dma_parms;
> +
> +	struct host1x_uapi uapi;
>  };
>  
>  void host1x_hypervisor_writel(struct host1x *host1x, u32 r, u32 v);
> diff --git a/drivers/gpu/host1x/uapi.c b/drivers/gpu/host1x/uapi.c
> new file mode 100644
> index 000000000000..27b8761c3f35
> --- /dev/null
> +++ b/drivers/gpu/host1x/uapi.c
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * /dev/host1x syncpoint interface
> + *
> + * Copyright (c) 2020, NVIDIA Corporation.
> + */
> +
> +#include <linux/anon_inodes.h>
> +#include <linux/cdev.h>
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/host1x.h>
> +#include <linux/nospec.h>
> +
> +#include "dev.h"
> +#include "syncpt.h"
> +#include "uapi.h"
> +
> +#include <uapi/linux/host1x.h>
> +
> +static int syncpt_file_release(struct inode *inode, struct file *file)
> +{
> +	struct host1x_syncpt *sp = file->private_data;
> +
> +	host1x_syncpt_put(sp);
> +
> +	return 0;
> +}
> +
> +static int syncpt_file_ioctl_info(struct host1x_syncpt *sp, void __user *data)
> +{
> +	struct host1x_syncpoint_info args;
> +	unsigned long copy_err;
> +
> +	copy_err = copy_from_user(&args, data, sizeof(args));
> +	if (copy_err)
> +		return -EFAULT;
> +
> +	if (args.reserved[0] || args.reserved[1] || args.reserved[2])
> +		return -EINVAL;

Yes! \o/

> +
> +	args.id = sp->id;
> +
> +	copy_err = copy_to_user(data, &args, sizeof(args));
> +	if (copy_err)
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int syncpt_file_ioctl_incr(struct host1x_syncpt *sp, void __user *data)
> +{
> +	struct host1x_syncpoint_increment args;
> +	unsigned long copy_err;
> +	u32 i;
> +
> +	copy_err = copy_from_user(&args, data, sizeof(args));
> +	if (copy_err)
> +		return -EFAULT;
> +
> +	for (i = 0; i < args.count; i++) {
> +		host1x_syncpt_incr(sp);
> +		if (signal_pending(current))
> +			return -EINTR;
> +	}
> +
> +	return 0;
> +}
> +
> +static long syncpt_file_ioctl(struct file *file, unsigned int cmd,
> +			      unsigned long arg)
> +{
> +	void __user *data = (void __user *)arg;
> +	long err;
> +
> +	switch (cmd) {
> +	case HOST1X_IOCTL_SYNCPOINT_INFO:
> +		err = syncpt_file_ioctl_info(file->private_data, data);
> +		break;
> +
> +	case HOST1X_IOCTL_SYNCPOINT_INCREMENT:
> +		err = syncpt_file_ioctl_incr(file->private_data, data);
> +		break;
> +
> +	default:
> +		err = -ENOTTY;
> +	}
> +
> +	return err;
> +}

I wonder if it's worth adding some more logic to this demuxing. I'm
thinking along the lines of what the DRM IOCTL demuxer does, which
ultimately allows the IOCTLs to be extended. It does this by doing a
bit of sanitizing and removing the parameter size field from the cmd
argument so that the same IOCTL may handle different parameter sizes.

> +static const struct file_operations syncpt_file_fops = {
> +	.owner = THIS_MODULE,
> +	.release = syncpt_file_release,
> +	.unlocked_ioctl = syncpt_file_ioctl,
> +	.compat_ioctl = syncpt_file_ioctl,
> +};
> +
> +struct host1x_syncpt *host1x_syncpt_fd_get(int fd)
> +{
> +	struct host1x_syncpt *sp;
> +	struct file *file = fget(fd);
> +
> +	if (!file)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (file->f_op != &syncpt_file_fops) {
> +		fput(file);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	sp = file->private_data;
> +
> +	host1x_syncpt_get(sp);
> +
> +	fput(file);
> +
> +	return sp;
> +}
> +EXPORT_SYMBOL(host1x_syncpt_fd_get);
> +
> +static int dev_file_open(struct inode *inode, struct file *file)

Maybe use the more specific host1x_ as prefix instead of the generic
dev_? That might make things like stack traces more readable.

Otherwise looks good.

Thierry
Mikko Perttunen March 23, 2021, 11:15 a.m. UTC | #2
On 3/23/21 1:02 PM, Thierry Reding wrote:
> On Mon, Jan 11, 2021 at 03:00:06PM +0200, Mikko Perttunen wrote:
>> Add the /dev/host1x device node, implementing the following
>> functionality:
>>
>> - Reading syncpoint values
>> - Allocating syncpoints (providing syncpoint FDs)
>> - Incrementing syncpoints (based on syncpoint FD)
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>> v4:
>> * Put UAPI under CONFIG_DRM_TEGRA_STAGING
>> v3:
>> * Pass process name as syncpoint name when allocating
>>    syncpoint.
>> ---
>>   drivers/gpu/host1x/Makefile |   1 +
>>   drivers/gpu/host1x/dev.c    |   9 ++
>>   drivers/gpu/host1x/dev.h    |   3 +
>>   drivers/gpu/host1x/uapi.c   | 282 ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/host1x/uapi.h   |  22 +++
>>   include/linux/host1x.h      |   2 +
>>   6 files changed, 319 insertions(+)
>>   create mode 100644 drivers/gpu/host1x/uapi.c
>>   create mode 100644 drivers/gpu/host1x/uapi.h
>>
>> diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile
>> index 096017b8789d..882f928d75e1 100644
>> --- a/drivers/gpu/host1x/Makefile
>> +++ b/drivers/gpu/host1x/Makefile
>> @@ -9,6 +9,7 @@ host1x-y = \
>>   	job.o \
>>   	debug.o \
>>   	mipi.o \
>> +	uapi.o \
>>   	hw/host1x01.o \
>>   	hw/host1x02.o \
>>   	hw/host1x04.o \
>> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
>> index d0ebb70e2fdd..641317d23828 100644
>> --- a/drivers/gpu/host1x/dev.c
>> +++ b/drivers/gpu/host1x/dev.c
>> @@ -461,6 +461,12 @@ static int host1x_probe(struct platform_device *pdev)
>>   		goto deinit_syncpt;
>>   	}
>>   
>> +	err = host1x_uapi_init(&host->uapi, host);
> 
> It's a bit pointless to pass &host->uapi and host to the function since
> you can access the former through the latter.

Yeah. I originally did it to separate the uapi module from the rest of 
the code interface-wise as much as possible, but I don't think I have 
done that consistently so it just looks weird.

> 
>> +	if (err) {
>> +		dev_err(&pdev->dev, "failed to initialize uapi\n");
> 
> s/uapi/UAPI/, and perhaps include the error code to give a better hint
> as to why things failed.

Sure (if this code is kept.)

> 
>> +		goto deinit_intr;
>> +	}
>> +
>>   	host1x_debug_init(host);
>>   
>>   	if (host->info->has_hypervisor)
>> @@ -480,6 +486,8 @@ static int host1x_probe(struct platform_device *pdev)
>>   	host1x_unregister(host);
>>   deinit_debugfs:
>>   	host1x_debug_deinit(host);
>> +	host1x_uapi_deinit(&host->uapi);
>> +deinit_intr:
>>   	host1x_intr_deinit(host);
>>   deinit_syncpt:
>>   	host1x_syncpt_deinit(host);
>> @@ -501,6 +509,7 @@ static int host1x_remove(struct platform_device *pdev)
>>   
>>   	host1x_unregister(host);
>>   	host1x_debug_deinit(host);
>> +	host1x_uapi_deinit(&host->uapi);
>>   	host1x_intr_deinit(host);
>>   	host1x_syncpt_deinit(host);
>>   	reset_control_assert(host->rst);
>> diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
>> index 63010ae37a97..7b8b7e20e32b 100644
>> --- a/drivers/gpu/host1x/dev.h
>> +++ b/drivers/gpu/host1x/dev.h
>> @@ -17,6 +17,7 @@
>>   #include "intr.h"
>>   #include "job.h"
>>   #include "syncpt.h"
>> +#include "uapi.h"
>>   
>>   struct host1x_syncpt;
>>   struct host1x_syncpt_base;
>> @@ -143,6 +144,8 @@ struct host1x {
>>   	struct list_head list;
>>   
>>   	struct device_dma_parameters dma_parms;
>> +
>> +	struct host1x_uapi uapi;
>>   };
>>   
>>   void host1x_hypervisor_writel(struct host1x *host1x, u32 r, u32 v);
>> diff --git a/drivers/gpu/host1x/uapi.c b/drivers/gpu/host1x/uapi.c
>> new file mode 100644
>> index 000000000000..27b8761c3f35
>> --- /dev/null
>> +++ b/drivers/gpu/host1x/uapi.c
>> @@ -0,0 +1,282 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * /dev/host1x syncpoint interface
>> + *
>> + * Copyright (c) 2020, NVIDIA Corporation.
>> + */
>> +
>> +#include <linux/anon_inodes.h>
>> +#include <linux/cdev.h>
>> +#include <linux/file.h>
>> +#include <linux/fs.h>
>> +#include <linux/host1x.h>
>> +#include <linux/nospec.h>
>> +
>> +#include "dev.h"
>> +#include "syncpt.h"
>> +#include "uapi.h"
>> +
>> +#include <uapi/linux/host1x.h>
>> +
>> +static int syncpt_file_release(struct inode *inode, struct file *file)
>> +{
>> +	struct host1x_syncpt *sp = file->private_data;
>> +
>> +	host1x_syncpt_put(sp);
>> +
>> +	return 0;
>> +}
>> +
>> +static int syncpt_file_ioctl_info(struct host1x_syncpt *sp, void __user *data)
>> +{
>> +	struct host1x_syncpoint_info args;
>> +	unsigned long copy_err;
>> +
>> +	copy_err = copy_from_user(&args, data, sizeof(args));
>> +	if (copy_err)
>> +		return -EFAULT;
>> +
>> +	if (args.reserved[0] || args.reserved[1] || args.reserved[2])
>> +		return -EINVAL;
> 
> Yes! \o/
> 
>> +
>> +	args.id = sp->id;
>> +
>> +	copy_err = copy_to_user(data, &args, sizeof(args));
>> +	if (copy_err)
>> +		return -EFAULT;
>> +
>> +	return 0;
>> +}
>> +
>> +static int syncpt_file_ioctl_incr(struct host1x_syncpt *sp, void __user *data)
>> +{
>> +	struct host1x_syncpoint_increment args;
>> +	unsigned long copy_err;
>> +	u32 i;
>> +
>> +	copy_err = copy_from_user(&args, data, sizeof(args));
>> +	if (copy_err)
>> +		return -EFAULT;
>> +
>> +	for (i = 0; i < args.count; i++) {
>> +		host1x_syncpt_incr(sp);
>> +		if (signal_pending(current))
>> +			return -EINTR;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static long syncpt_file_ioctl(struct file *file, unsigned int cmd,
>> +			      unsigned long arg)
>> +{
>> +	void __user *data = (void __user *)arg;
>> +	long err;
>> +
>> +	switch (cmd) {
>> +	case HOST1X_IOCTL_SYNCPOINT_INFO:
>> +		err = syncpt_file_ioctl_info(file->private_data, data);
>> +		break;
>> +
>> +	case HOST1X_IOCTL_SYNCPOINT_INCREMENT:
>> +		err = syncpt_file_ioctl_incr(file->private_data, data);
>> +		break;
>> +
>> +	default:
>> +		err = -ENOTTY;
>> +	}
>> +
>> +	return err;
>> +}
> 
> I wonder if it's worth adding some more logic to this demuxing. I'm
> thinking along the lines of what the DRM IOCTL demuxer does, which
> ultimately allows the IOCTLs to be extended. It does this by doing a
> bit of sanitizing and removing the parameter size field from the cmd
> argument so that the same IOCTL may handle different parameter sizes.

Yep, seems like a good idea (if we keep this).

> 
>> +static const struct file_operations syncpt_file_fops = {
>> +	.owner = THIS_MODULE,
>> +	.release = syncpt_file_release,
>> +	.unlocked_ioctl = syncpt_file_ioctl,
>> +	.compat_ioctl = syncpt_file_ioctl,
>> +};
>> +
>> +struct host1x_syncpt *host1x_syncpt_fd_get(int fd)
>> +{
>> +	struct host1x_syncpt *sp;
>> +	struct file *file = fget(fd);
>> +
>> +	if (!file)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	if (file->f_op != &syncpt_file_fops) {
>> +		fput(file);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	sp = file->private_data;
>> +
>> +	host1x_syncpt_get(sp);
>> +
>> +	fput(file);
>> +
>> +	return sp;
>> +}
>> +EXPORT_SYMBOL(host1x_syncpt_fd_get);
>> +
>> +static int dev_file_open(struct inode *inode, struct file *file)
> 
> Maybe use the more specific host1x_ as prefix instead of the generic
> dev_? That might make things like stack traces more readable.

Yep.

> 
> Otherwise looks good.
> 
> Thierry
> 

thanks,
Mikko
diff mbox series

Patch

diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile
index 096017b8789d..882f928d75e1 100644
--- a/drivers/gpu/host1x/Makefile
+++ b/drivers/gpu/host1x/Makefile
@@ -9,6 +9,7 @@  host1x-y = \
 	job.o \
 	debug.o \
 	mipi.o \
+	uapi.o \
 	hw/host1x01.o \
 	hw/host1x02.o \
 	hw/host1x04.o \
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index d0ebb70e2fdd..641317d23828 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -461,6 +461,12 @@  static int host1x_probe(struct platform_device *pdev)
 		goto deinit_syncpt;
 	}
 
+	err = host1x_uapi_init(&host->uapi, host);
+	if (err) {
+		dev_err(&pdev->dev, "failed to initialize uapi\n");
+		goto deinit_intr;
+	}
+
 	host1x_debug_init(host);
 
 	if (host->info->has_hypervisor)
@@ -480,6 +486,8 @@  static int host1x_probe(struct platform_device *pdev)
 	host1x_unregister(host);
 deinit_debugfs:
 	host1x_debug_deinit(host);
+	host1x_uapi_deinit(&host->uapi);
+deinit_intr:
 	host1x_intr_deinit(host);
 deinit_syncpt:
 	host1x_syncpt_deinit(host);
@@ -501,6 +509,7 @@  static int host1x_remove(struct platform_device *pdev)
 
 	host1x_unregister(host);
 	host1x_debug_deinit(host);
+	host1x_uapi_deinit(&host->uapi);
 	host1x_intr_deinit(host);
 	host1x_syncpt_deinit(host);
 	reset_control_assert(host->rst);
diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h
index 63010ae37a97..7b8b7e20e32b 100644
--- a/drivers/gpu/host1x/dev.h
+++ b/drivers/gpu/host1x/dev.h
@@ -17,6 +17,7 @@ 
 #include "intr.h"
 #include "job.h"
 #include "syncpt.h"
+#include "uapi.h"
 
 struct host1x_syncpt;
 struct host1x_syncpt_base;
@@ -143,6 +144,8 @@  struct host1x {
 	struct list_head list;
 
 	struct device_dma_parameters dma_parms;
+
+	struct host1x_uapi uapi;
 };
 
 void host1x_hypervisor_writel(struct host1x *host1x, u32 r, u32 v);
diff --git a/drivers/gpu/host1x/uapi.c b/drivers/gpu/host1x/uapi.c
new file mode 100644
index 000000000000..27b8761c3f35
--- /dev/null
+++ b/drivers/gpu/host1x/uapi.c
@@ -0,0 +1,282 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * /dev/host1x syncpoint interface
+ *
+ * Copyright (c) 2020, NVIDIA Corporation.
+ */
+
+#include <linux/anon_inodes.h>
+#include <linux/cdev.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/host1x.h>
+#include <linux/nospec.h>
+
+#include "dev.h"
+#include "syncpt.h"
+#include "uapi.h"
+
+#include <uapi/linux/host1x.h>
+
+static int syncpt_file_release(struct inode *inode, struct file *file)
+{
+	struct host1x_syncpt *sp = file->private_data;
+
+	host1x_syncpt_put(sp);
+
+	return 0;
+}
+
+static int syncpt_file_ioctl_info(struct host1x_syncpt *sp, void __user *data)
+{
+	struct host1x_syncpoint_info args;
+	unsigned long copy_err;
+
+	copy_err = copy_from_user(&args, data, sizeof(args));
+	if (copy_err)
+		return -EFAULT;
+
+	if (args.reserved[0] || args.reserved[1] || args.reserved[2])
+		return -EINVAL;
+
+	args.id = sp->id;
+
+	copy_err = copy_to_user(data, &args, sizeof(args));
+	if (copy_err)
+		return -EFAULT;
+
+	return 0;
+}
+
+static int syncpt_file_ioctl_incr(struct host1x_syncpt *sp, void __user *data)
+{
+	struct host1x_syncpoint_increment args;
+	unsigned long copy_err;
+	u32 i;
+
+	copy_err = copy_from_user(&args, data, sizeof(args));
+	if (copy_err)
+		return -EFAULT;
+
+	for (i = 0; i < args.count; i++) {
+		host1x_syncpt_incr(sp);
+		if (signal_pending(current))
+			return -EINTR;
+	}
+
+	return 0;
+}
+
+static long syncpt_file_ioctl(struct file *file, unsigned int cmd,
+			      unsigned long arg)
+{
+	void __user *data = (void __user *)arg;
+	long err;
+
+	switch (cmd) {
+	case HOST1X_IOCTL_SYNCPOINT_INFO:
+		err = syncpt_file_ioctl_info(file->private_data, data);
+		break;
+
+	case HOST1X_IOCTL_SYNCPOINT_INCREMENT:
+		err = syncpt_file_ioctl_incr(file->private_data, data);
+		break;
+
+	default:
+		err = -ENOTTY;
+	}
+
+	return err;
+}
+
+static const struct file_operations syncpt_file_fops = {
+	.owner = THIS_MODULE,
+	.release = syncpt_file_release,
+	.unlocked_ioctl = syncpt_file_ioctl,
+	.compat_ioctl = syncpt_file_ioctl,
+};
+
+struct host1x_syncpt *host1x_syncpt_fd_get(int fd)
+{
+	struct host1x_syncpt *sp;
+	struct file *file = fget(fd);
+
+	if (!file)
+		return ERR_PTR(-EINVAL);
+
+	if (file->f_op != &syncpt_file_fops) {
+		fput(file);
+		return ERR_PTR(-EINVAL);
+	}
+
+	sp = file->private_data;
+
+	host1x_syncpt_get(sp);
+
+	fput(file);
+
+	return sp;
+}
+EXPORT_SYMBOL(host1x_syncpt_fd_get);
+
+static int dev_file_open(struct inode *inode, struct file *file)
+{
+	struct host1x_uapi *uapi =
+		container_of(inode->i_cdev, struct host1x_uapi, cdev);
+
+	file->private_data = container_of(uapi, struct host1x, uapi);
+
+	return 0;
+}
+
+static int dev_file_ioctl_read_syncpoint(struct host1x *host1x,
+					 void __user *data)
+{
+	struct host1x_read_syncpoint args;
+	unsigned long copy_err;
+
+	copy_err = copy_from_user(&args, data, sizeof(args));
+	if (copy_err)
+		return -EFAULT;
+
+	if (args.id >= host1x_syncpt_nb_pts(host1x))
+		return -EINVAL;
+
+	args.id = array_index_nospec(args.id, host1x_syncpt_nb_pts(host1x));
+	args.value = host1x_syncpt_read(&host1x->syncpt[args.id]);
+
+	copy_err = copy_to_user(data, &args, sizeof(args));
+	if (copy_err)
+		return -EFAULT;
+
+	return 0;
+}
+
+static int dev_file_ioctl_alloc_syncpoint(struct host1x *host1x,
+					  void __user *data)
+{
+	struct host1x_allocate_syncpoint args;
+	struct host1x_syncpt *sp;
+	unsigned long copy_err;
+	int err;
+
+	copy_err = copy_from_user(&args, data, sizeof(args));
+	if (copy_err)
+		return -EFAULT;
+
+	if (args.reserved[0] || args.reserved[1] || args.reserved[2])
+		return -EINVAL;
+
+	sp = host1x_syncpt_alloc(host1x, HOST1X_SYNCPT_CLIENT_MANAGED,
+				 current->comm);
+	if (!sp)
+		return -EBUSY;
+
+	err = anon_inode_getfd("host1x_syncpt", &syncpt_file_fops, sp,
+			       O_CLOEXEC);
+	if (err < 0)
+		goto free_syncpt;
+
+	args.fd = err;
+
+	copy_err = copy_to_user(data, &args, sizeof(args));
+	if (copy_err) {
+		err = -EFAULT;
+		goto put_fd;
+	}
+
+	return 0;
+
+put_fd:
+	put_unused_fd(args.fd);
+free_syncpt:
+	host1x_syncpt_put(sp);
+
+	return err;
+}
+
+static long dev_file_ioctl(struct file *file, unsigned int cmd,
+			   unsigned long arg)
+{
+	void __user *data = (void __user *)arg;
+	long err;
+
+	switch (cmd) {
+	case HOST1X_IOCTL_READ_SYNCPOINT:
+		err = dev_file_ioctl_read_syncpoint(file->private_data, data);
+		break;
+
+	case HOST1X_IOCTL_ALLOCATE_SYNCPOINT:
+		err = dev_file_ioctl_alloc_syncpoint(file->private_data, data);
+		break;
+
+	default:
+		err = -ENOTTY;
+	}
+
+	return err;
+}
+
+static const struct file_operations dev_file_fops = {
+	.owner = THIS_MODULE,
+	.open = dev_file_open,
+	.unlocked_ioctl = dev_file_ioctl,
+	.compat_ioctl = dev_file_ioctl,
+};
+
+int host1x_uapi_init(struct host1x_uapi *uapi, struct host1x *host1x)
+{
+	int err;
+	dev_t dev_num;
+
+	if (!IS_ENABLED(CONFIG_DRM_TEGRA_STAGING))
+		return 0;
+
+	err = alloc_chrdev_region(&dev_num, 0, 1, "host1x");
+	if (err)
+		return err;
+
+	uapi->class = class_create(THIS_MODULE, "host1x");
+	if (IS_ERR(uapi->class)) {
+		err = PTR_ERR(uapi->class);
+		goto unregister_chrdev_region;
+	}
+
+	cdev_init(&uapi->cdev, &dev_file_fops);
+	err = cdev_add(&uapi->cdev, dev_num, 1);
+	if (err)
+		goto destroy_class;
+
+	uapi->dev = device_create(uapi->class, host1x->dev,
+				  dev_num, NULL, "host1x");
+	if (IS_ERR(uapi->dev)) {
+		err = PTR_ERR(uapi->dev);
+		goto del_cdev;
+	}
+
+	cdev_add(&uapi->cdev, dev_num, 1);
+
+	uapi->dev_num = dev_num;
+
+	return 0;
+
+del_cdev:
+	cdev_del(&uapi->cdev);
+destroy_class:
+	class_destroy(uapi->class);
+unregister_chrdev_region:
+	unregister_chrdev_region(dev_num, 1);
+
+	return err;
+}
+
+void host1x_uapi_deinit(struct host1x_uapi *uapi)
+{
+	if (!IS_ENABLED(CONFIG_DRM_TEGRA_STAGING))
+		return;
+
+	device_destroy(uapi->class, uapi->dev_num);
+	cdev_del(&uapi->cdev);
+	class_destroy(uapi->class);
+	unregister_chrdev_region(uapi->dev_num, 1);
+}
diff --git a/drivers/gpu/host1x/uapi.h b/drivers/gpu/host1x/uapi.h
new file mode 100644
index 000000000000..7beb5e44c1b1
--- /dev/null
+++ b/drivers/gpu/host1x/uapi.h
@@ -0,0 +1,22 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020, NVIDIA Corporation.
+ */
+
+#ifndef HOST1X_UAPI_H
+#define HOST1X_UAPI_H
+
+#include <linux/cdev.h>
+
+struct host1x_uapi {
+	struct class *class;
+
+	struct cdev cdev;
+	struct device *dev;
+	dev_t dev_num;
+};
+
+int host1x_uapi_init(struct host1x_uapi *uapi, struct host1x *host1x);
+void host1x_uapi_deinit(struct host1x_uapi *uapi);
+
+#endif
diff --git a/include/linux/host1x.h b/include/linux/host1x.h
index 107aea29bccb..b3178ae51cae 100644
--- a/include/linux/host1x.h
+++ b/include/linux/host1x.h
@@ -163,6 +163,8 @@  struct host1x_syncpt *host1x_syncpt_alloc(struct host1x *host,
 struct host1x_syncpt_base *host1x_syncpt_get_base(struct host1x_syncpt *sp);
 u32 host1x_syncpt_base_id(struct host1x_syncpt_base *base);
 
+struct host1x_syncpt *host1x_syncpt_fd_get(int fd);
+
 /*
  * host1x channel
  */