Re: [PATCH 2/3] drm/tegra: Deliver syncpoint base to user space

2013-10-11 Thread Thierry Reding 
On Wed, Oct 09, 2013 at 02:54:09PM +0300, Arto Merilainen wrote:
 This patch makes the necessary additions to deliver syncpoint base
 to the user space.
 
 This patch splits the index field in the drm_tegra_get_syncpt structure
 into three separate fields (index, support_base, base_id). This allows
 to keep compatibility over kernel versions:
 - The placing of index field stays constant and therefore the interface
   should stay compatible with earlier userspace versions.
 - The old index field was input-only and it was not supposed to be read
   afterwards. Delivering data back in the same field should not introduce
   regressions to any old user space applications.
 - Old kernels left support_base and base_id fields intact (zero) and
   hence the new user space applications get the correct response from
   the kernel (=syncpoint does not have a base).
 
 Signed-off-by: Arto Merilainen amerilai...@nvidia.com
 ---
  drivers/gpu/host1x/drm/drm.c | 2 ++
  include/uapi/drm/tegra_drm.h | 4 +++-
  2 files changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c
 index 8c61cee..4251563 100644
 --- a/drivers/gpu/host1x/drm/drm.c
 +++ b/drivers/gpu/host1x/drm/drm.c
 @@ -468,6 +468,8 @@ static int tegra_get_syncpt(struct drm_device *drm, void 
 *data,
  
   syncpt = context-client-syncpts[args-index];
   args-id = host1x_syncpt_id(syncpt);
 + args-support_base = syncpt-base ? 1 : 0;
 + args-base_id = syncpt-base ? syncpt-base-id : 0;
  
   return 0;
  }
 diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
 index 73bde4e..7a79ca5 100644
 --- a/include/uapi/drm/tegra_drm.h
 +++ b/include/uapi/drm/tegra_drm.h
 @@ -61,7 +61,9 @@ struct drm_tegra_close_channel {
  
  struct drm_tegra_get_syncpt {
   __u64 context;
 - __u32 index;
 + __u16 index;
 + __u8 support_base;
 + __u8 base_id;
   __u32 id;

I don't like this. Can't we just add a separate IOCTL? Something like:

struct drm_tegra_get_syncpt_base {
__u64 context;
__u32 syncpt;
__u32 base;
};

Which could be used somewhat like this:

struct drm_tegra_get_syncpt_base args;

memset(args, 0, sizeof(args));
args.context = context;
args.syncpt = syncpt;

err = ioctl(fd, DRM_IOCTL_TEGRA_GET_SYNCPT_BASE, args);
if (err  0) {
/*
 * perhaps errno == ENOTSUP if the syncpoint has no
 * associated base?
 */
}

base = args.base;

Thierry


pgpKTabFFv3lS.pgp
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/tegra: Deliver syncpoint base to user space

2013-10-11 Thread Arto Merilainen 

On 10/11/2013 12:43 PM, Thierry Reding wrote:

* PGP Signed by an unknown key

On Wed, Oct 09, 2013 at 02:54:09PM +0300, Arto Merilainen wrote:

This patch makes the necessary additions to deliver syncpoint base
to the user space.

This patch splits the index field in the drm_tegra_get_syncpt structure
into three separate fields (index, support_base, base_id). This allows
to keep compatibility over kernel versions:
- The placing of index field stays constant and therefore the interface
   should stay compatible with earlier userspace versions.
- The old index field was input-only and it was not supposed to be read
   afterwards. Delivering data back in the same field should not introduce
   regressions to any old user space applications.
- Old kernels left support_base and base_id fields intact (zero) and
   hence the new user space applications get the correct response from
   the kernel (=syncpoint does not have a base).

Signed-off-by: Arto Merilainen amerilai...@nvidia.com
---
  drivers/gpu/host1x/drm/drm.c | 2 ++
  include/uapi/drm/tegra_drm.h | 4 +++-
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c
index 8c61cee..4251563 100644
--- a/drivers/gpu/host1x/drm/drm.c
+++ b/drivers/gpu/host1x/drm/drm.c
@@ -468,6 +468,8 @@ static int tegra_get_syncpt(struct drm_device *drm, void 
*data,

syncpt = context-client-syncpts[args-index];
args-id = host1x_syncpt_id(syncpt);
+   args-support_base = syncpt-base ? 1 : 0;
+   args-base_id = syncpt-base ? syncpt-base-id : 0;

return 0;
  }
diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index 73bde4e..7a79ca5 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -61,7 +61,9 @@ struct drm_tegra_close_channel {

  struct drm_tegra_get_syncpt {
__u64 context;
-   __u32 index;
+   __u16 index;
+   __u8 support_base;
+   __u8 base_id;
__u32 id;


I don't like this. Can't we just add a separate IOCTL? Something like:


Sure. This felt like a good idea, but I must admit I had a bit mixed 
feelings after looking my own explanation why this would be ok...


- Arto

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/tegra: Deliver syncpoint base to user space

2013-10-09 Thread Arto Merilainen 
This patch makes the necessary additions to deliver syncpoint base
to the user space.

This patch splits the index field in the drm_tegra_get_syncpt structure
into three separate fields (index, support_base, base_id). This allows
to keep compatibility over kernel versions:
- The placing of index field stays constant and therefore the interface
  should stay compatible with earlier userspace versions.
- The old index field was input-only and it was not supposed to be read
  afterwards. Delivering data back in the same field should not introduce
  regressions to any old user space applications.
- Old kernels left support_base and base_id fields intact (zero) and
  hence the new user space applications get the correct response from
  the kernel (=syncpoint does not have a base).

Signed-off-by: Arto Merilainen amerilai...@nvidia.com
---
 drivers/gpu/host1x/drm/drm.c | 2 ++
 include/uapi/drm/tegra_drm.h | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/drm/drm.c b/drivers/gpu/host1x/drm/drm.c
index 8c61cee..4251563 100644
--- a/drivers/gpu/host1x/drm/drm.c
+++ b/drivers/gpu/host1x/drm/drm.c
@@ -468,6 +468,8 @@ static int tegra_get_syncpt(struct drm_device *drm, void 
*data,
 
syncpt = context-client-syncpts[args-index];
args-id = host1x_syncpt_id(syncpt);
+   args-support_base = syncpt-base ? 1 : 0;
+   args-base_id = syncpt-base ? syncpt-base-id : 0;
 
return 0;
 }
diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index 73bde4e..7a79ca5 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -61,7 +61,9 @@ struct drm_tegra_close_channel {
 
 struct drm_tegra_get_syncpt {
__u64 context;
-   __u32 index;
+   __u16 index;
+   __u8 support_base;
+   __u8 base_id;
__u32 id;
 };
 
-- 
1.8.1.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel