All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
@ 2015-01-29 18:46 Sean Paul
  2015-01-29 18:55 ` Rob Clark
  0 siblings, 1 reply; 14+ messages in thread
From: Sean Paul @ 2015-01-29 18:46 UTC (permalink / raw)
  To: thierry.reding; +Cc: marcheu, dri-devel

On 64-bit targets, tegra_gem_mmap doesn't return the
offset to userspace. As such, subsequent calls to mmap(2)
fail. Add a new tegra_gem_mmap2 ioctl to fix this.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/tegra/drm.c  | 21 +++++++++++++++++++++
 include/uapi/drm/tegra_drm.h |  8 ++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index d4f8275..be5dbe7 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -343,6 +343,26 @@ static int tegra_gem_create(struct drm_device *drm, void *data,
 	return 0;
 }
 
+static int tegra_gem_mmap2(struct drm_device *drm, void *data,
+			  struct drm_file *file)
+{
+	struct drm_tegra_gem_mmap2 *args = data;
+	struct drm_gem_object *gem;
+	struct tegra_bo *bo;
+
+	gem = drm_gem_object_lookup(drm, file, args->handle);
+	if (!gem)
+		return -EINVAL;
+
+	bo = to_tegra_bo(gem);
+
+	args->offset = drm_vma_node_offset_addr(&bo->gem.vma_node);
+
+	drm_gem_object_unreference(gem);
+
+	return 0;
+}
+
 static int tegra_gem_mmap(struct drm_device *drm, void *data,
 			  struct drm_file *file)
 {
@@ -677,6 +697,7 @@ static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_FLAGS, tegra_gem_set_flags, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_FLAGS, tegra_gem_get_flags, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(TEGRA_GEM_MMAP2, tegra_gem_mmap2, DRM_UNLOCKED),
 #endif
 };
 
diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index c15d781..9035257 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -167,6 +167,12 @@ struct drm_tegra_gem_get_flags {
 	__u32 flags;
 };
 
+struct drm_tegra_gem_mmap2 {
+	__u32 handle;
+	__u64 offset;
+};
+
+
 #define DRM_TEGRA_GEM_CREATE		0x00
 #define DRM_TEGRA_GEM_MMAP		0x01
 #define DRM_TEGRA_SYNCPT_READ		0x02
@@ -181,6 +187,7 @@ struct drm_tegra_gem_get_flags {
 #define DRM_TEGRA_GEM_GET_TILING	0x0b
 #define DRM_TEGRA_GEM_SET_FLAGS		0x0c
 #define DRM_TEGRA_GEM_GET_FLAGS		0x0d
+#define DRM_TEGRA_GEM_MMAP2		0x0e
 
 #define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_CREATE, struct drm_tegra_gem_create)
 #define DRM_IOCTL_TEGRA_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP, struct drm_tegra_gem_mmap)
@@ -196,5 +203,6 @@ struct drm_tegra_gem_get_flags {
 #define DRM_IOCTL_TEGRA_GEM_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_TILING, struct drm_tegra_gem_get_tiling)
 #define DRM_IOCTL_TEGRA_GEM_SET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_FLAGS, struct drm_tegra_gem_set_flags)
 #define DRM_IOCTL_TEGRA_GEM_GET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_FLAGS, struct drm_tegra_gem_get_flags)
+#define DRM_IOCTL_TEGRA_GEM_MMAP2 DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP2, struct drm_tegra_gem_mmap2)
 
 #endif
-- 
2.2.0.rc0.207.ga3a616c

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
  2015-01-29 18:46 [PATCH] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets Sean Paul
@ 2015-01-29 18:55 ` Rob Clark
  2015-01-29 19:18   ` [PATCH v2] " Sean Paul
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Clark @ 2015-01-29 18:55 UTC (permalink / raw)
  To: Sean Paul; +Cc: Stéphane Marchesin, dri-devel

On Thu, Jan 29, 2015 at 1:46 PM, Sean Paul <seanpaul@chromium.org> wrote:
>
> diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> index c15d781..9035257 100644
> --- a/include/uapi/drm/tegra_drm.h
> +++ b/include/uapi/drm/tegra_drm.h
> @@ -167,6 +167,12 @@ struct drm_tegra_gem_get_flags {
>         __u32 flags;
>  };
>
> +struct drm_tegra_gem_mmap2 {
> +       __u32 handle;

__u32 pad;

> +       __u64 offset;
> +};
> +

BR,
-R
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
  2015-01-29 18:55 ` Rob Clark
@ 2015-01-29 19:18   ` Sean Paul
  2015-01-29 20:11     ` Rob Clark
  2015-01-30  9:49     ` Thierry Reding
  0 siblings, 2 replies; 14+ messages in thread
From: Sean Paul @ 2015-01-29 19:18 UTC (permalink / raw)
  To: thierry.reding; +Cc: marcheu, dri-devel

On 64-bit targets, tegra_gem_mmap doesn't return the
offset to userspace. As such, subsequent calls to mmap(2)
fail. Add a new tegra_gem_mmap2 ioctl to fix this.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/tegra/drm.c  | 21 +++++++++++++++++++++
 include/uapi/drm/tegra_drm.h |  9 +++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index d4f8275..be5dbe7 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -343,6 +343,26 @@ static int tegra_gem_create(struct drm_device *drm, void *data,
 	return 0;
 }
 
+static int tegra_gem_mmap2(struct drm_device *drm, void *data,
+			  struct drm_file *file)
+{
+	struct drm_tegra_gem_mmap2 *args = data;
+	struct drm_gem_object *gem;
+	struct tegra_bo *bo;
+
+	gem = drm_gem_object_lookup(drm, file, args->handle);
+	if (!gem)
+		return -EINVAL;
+
+	bo = to_tegra_bo(gem);
+
+	args->offset = drm_vma_node_offset_addr(&bo->gem.vma_node);
+
+	drm_gem_object_unreference(gem);
+
+	return 0;
+}
+
 static int tegra_gem_mmap(struct drm_device *drm, void *data,
 			  struct drm_file *file)
 {
@@ -677,6 +697,7 @@ static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_FLAGS, tegra_gem_set_flags, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_FLAGS, tegra_gem_get_flags, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(TEGRA_GEM_MMAP2, tegra_gem_mmap2, DRM_UNLOCKED),
 #endif
 };
 
diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index c15d781..9057b0f 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -167,6 +167,13 @@ struct drm_tegra_gem_get_flags {
 	__u32 flags;
 };
 
+struct drm_tegra_gem_mmap2 {
+	__u32 handle;
+	__u32 pad;
+	__u64 offset;
+};
+
+
 #define DRM_TEGRA_GEM_CREATE		0x00
 #define DRM_TEGRA_GEM_MMAP		0x01
 #define DRM_TEGRA_SYNCPT_READ		0x02
@@ -181,6 +188,7 @@ struct drm_tegra_gem_get_flags {
 #define DRM_TEGRA_GEM_GET_TILING	0x0b
 #define DRM_TEGRA_GEM_SET_FLAGS		0x0c
 #define DRM_TEGRA_GEM_GET_FLAGS		0x0d
+#define DRM_TEGRA_GEM_MMAP2		0x0e
 
 #define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_CREATE, struct drm_tegra_gem_create)
 #define DRM_IOCTL_TEGRA_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP, struct drm_tegra_gem_mmap)
@@ -196,5 +204,6 @@ struct drm_tegra_gem_get_flags {
 #define DRM_IOCTL_TEGRA_GEM_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_TILING, struct drm_tegra_gem_get_tiling)
 #define DRM_IOCTL_TEGRA_GEM_SET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_FLAGS, struct drm_tegra_gem_set_flags)
 #define DRM_IOCTL_TEGRA_GEM_GET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_FLAGS, struct drm_tegra_gem_get_flags)
+#define DRM_IOCTL_TEGRA_GEM_MMAP2 DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP2, struct drm_tegra_gem_mmap2)
 
 #endif
-- 
2.2.0.rc0.207.ga3a616c

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
  2015-01-29 19:18   ` [PATCH v2] " Sean Paul
@ 2015-01-29 20:11     ` Rob Clark
  2015-01-30  9:49     ` Thierry Reding
  1 sibling, 0 replies; 14+ messages in thread
From: Rob Clark @ 2015-01-29 20:11 UTC (permalink / raw)
  To: Sean Paul; +Cc: Stéphane Marchesin, dri-devel

On Thu, Jan 29, 2015 at 2:18 PM, Sean Paul <seanpaul@chromium.org> wrote:
> On 64-bit targets, tegra_gem_mmap doesn't return the
> offset to userspace. As such, subsequent calls to mmap(2)
> fail. Add a new tegra_gem_mmap2 ioctl to fix this.
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Reviewed-by: Rob Clark <robdclark@gmail.com>


> ---
>  drivers/gpu/drm/tegra/drm.c  | 21 +++++++++++++++++++++
>  include/uapi/drm/tegra_drm.h |  9 +++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index d4f8275..be5dbe7 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -343,6 +343,26 @@ static int tegra_gem_create(struct drm_device *drm, void *data,
>         return 0;
>  }
>
> +static int tegra_gem_mmap2(struct drm_device *drm, void *data,
> +                         struct drm_file *file)
> +{
> +       struct drm_tegra_gem_mmap2 *args = data;
> +       struct drm_gem_object *gem;
> +       struct tegra_bo *bo;
> +
> +       gem = drm_gem_object_lookup(drm, file, args->handle);
> +       if (!gem)
> +               return -EINVAL;
> +
> +       bo = to_tegra_bo(gem);
> +
> +       args->offset = drm_vma_node_offset_addr(&bo->gem.vma_node);
> +
> +       drm_gem_object_unreference(gem);
> +
> +       return 0;
> +}
> +
>  static int tegra_gem_mmap(struct drm_device *drm, void *data,
>                           struct drm_file *file)
>  {
> @@ -677,6 +697,7 @@ static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
>         DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_FLAGS, tegra_gem_set_flags, DRM_UNLOCKED),
>         DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_FLAGS, tegra_gem_get_flags, DRM_UNLOCKED),
> +       DRM_IOCTL_DEF_DRV(TEGRA_GEM_MMAP2, tegra_gem_mmap2, DRM_UNLOCKED),
>  #endif
>  };
>
> diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> index c15d781..9057b0f 100644
> --- a/include/uapi/drm/tegra_drm.h
> +++ b/include/uapi/drm/tegra_drm.h
> @@ -167,6 +167,13 @@ struct drm_tegra_gem_get_flags {
>         __u32 flags;
>  };
>
> +struct drm_tegra_gem_mmap2 {
> +       __u32 handle;
> +       __u32 pad;
> +       __u64 offset;
> +};
> +
> +
>  #define DRM_TEGRA_GEM_CREATE           0x00
>  #define DRM_TEGRA_GEM_MMAP             0x01
>  #define DRM_TEGRA_SYNCPT_READ          0x02
> @@ -181,6 +188,7 @@ struct drm_tegra_gem_get_flags {
>  #define DRM_TEGRA_GEM_GET_TILING       0x0b
>  #define DRM_TEGRA_GEM_SET_FLAGS                0x0c
>  #define DRM_TEGRA_GEM_GET_FLAGS                0x0d
> +#define DRM_TEGRA_GEM_MMAP2            0x0e
>
>  #define DRM_IOCTL_TEGRA_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_CREATE, struct drm_tegra_gem_create)
>  #define DRM_IOCTL_TEGRA_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP, struct drm_tegra_gem_mmap)
> @@ -196,5 +204,6 @@ struct drm_tegra_gem_get_flags {
>  #define DRM_IOCTL_TEGRA_GEM_GET_TILING DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_TILING, struct drm_tegra_gem_get_tiling)
>  #define DRM_IOCTL_TEGRA_GEM_SET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_SET_FLAGS, struct drm_tegra_gem_set_flags)
>  #define DRM_IOCTL_TEGRA_GEM_GET_FLAGS DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_GET_FLAGS, struct drm_tegra_gem_get_flags)
> +#define DRM_IOCTL_TEGRA_GEM_MMAP2 DRM_IOWR(DRM_COMMAND_BASE + DRM_TEGRA_GEM_MMAP2, struct drm_tegra_gem_mmap2)
>
>  #endif
> --
> 2.2.0.rc0.207.ga3a616c
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
  2015-01-29 19:18   ` [PATCH v2] " Sean Paul
  2015-01-29 20:11     ` Rob Clark
@ 2015-01-30  9:49     ` Thierry Reding
  2015-01-30 10:15       ` Erik Faye-Lund
  2015-01-30 18:57       ` [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap Sean Paul
  1 sibling, 2 replies; 14+ messages in thread
From: Thierry Reding @ 2015-01-30  9:49 UTC (permalink / raw)
  To: Sean Paul; +Cc: marcheu, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1044 bytes --]

On Thu, Jan 29, 2015 at 02:18:41PM -0500, Sean Paul wrote:
> On 64-bit targets, tegra_gem_mmap doesn't return the
> offset to userspace. As such, subsequent calls to mmap(2)
> fail. Add a new tegra_gem_mmap2 ioctl to fix this.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/tegra/drm.c  | 21 +++++++++++++++++++++
>  include/uapi/drm/tegra_drm.h |  9 +++++++++
>  2 files changed, 30 insertions(+)

To be honest, I'd rather just fix the existing IOCTL to do the right
thing on 64-bit. All IOCTLs are still protected by the DRM_TEGRA_STAGING
Kconfig symbol which depends on STAGING. We originally did that
precisely so we'd have some leeway in fixing things up. And we've done
precisely that in the past.

The only user of this IOCTL is libdrm and I don't think that has any
users aside from a few projects that are still under heavy development
(like grate or the xf86-video-opentegra driver).

Cc'ing Erik, who's probably the only one that's ever worked with this,
besides me.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
  2015-01-30  9:49     ` Thierry Reding
@ 2015-01-30 10:15       ` Erik Faye-Lund
  2015-01-30 10:21         ` Thierry Reding
  2015-01-30 18:57       ` [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap Sean Paul
  1 sibling, 1 reply; 14+ messages in thread
From: Erik Faye-Lund @ 2015-01-30 10:15 UTC (permalink / raw)
  To: Thierry Reding; +Cc: marcheu, dri-devel

On Fri, Jan 30, 2015 at 10:49 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Thu, Jan 29, 2015 at 02:18:41PM -0500, Sean Paul wrote:
>> On 64-bit targets, tegra_gem_mmap doesn't return the
>> offset to userspace. As such, subsequent calls to mmap(2)
>> fail. Add a new tegra_gem_mmap2 ioctl to fix this.
>>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>>  drivers/gpu/drm/tegra/drm.c  | 21 +++++++++++++++++++++
>>  include/uapi/drm/tegra_drm.h |  9 +++++++++
>>  2 files changed, 30 insertions(+)
>
> To be honest, I'd rather just fix the existing IOCTL to do the right
> thing on 64-bit. All IOCTLs are still protected by the DRM_TEGRA_STAGING
> Kconfig symbol which depends on STAGING. We originally did that
> precisely so we'd have some leeway in fixing things up. And we've done
> precisely that in the past.
>
> The only user of this IOCTL is libdrm and I don't think that has any
> users aside from a few projects that are still under heavy development
> (like grate or the xf86-video-opentegra driver).
>
> Cc'ing Erik, who's probably the only one that's ever worked with this,
> besides me.

I also saw the patch, and had the same reaction. I'm fine with
changing the ABI, it was done already anyway
(cbfbbabb89b37f6bad05f478d906a385149f288d, "drm/tegra: Remove
gratuitous pad field"). And as you say, this is only in staging so
nobody is really relying on it, except grate and libdrm (in which this
is clearly marked as experimental). I'm fine with just changing it,
and updating grate and libdrm accordingly.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
  2015-01-30 10:15       ` Erik Faye-Lund
@ 2015-01-30 10:21         ` Thierry Reding
  2015-01-30 11:41           ` Emil Velikov
  0 siblings, 1 reply; 14+ messages in thread
From: Thierry Reding @ 2015-01-30 10:21 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: marcheu, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2101 bytes --]

On Fri, Jan 30, 2015 at 11:15:30AM +0100, Erik Faye-Lund wrote:
> On Fri, Jan 30, 2015 at 10:49 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Thu, Jan 29, 2015 at 02:18:41PM -0500, Sean Paul wrote:
> >> On 64-bit targets, tegra_gem_mmap doesn't return the
> >> offset to userspace. As such, subsequent calls to mmap(2)
> >> fail. Add a new tegra_gem_mmap2 ioctl to fix this.
> >>
> >> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> >> ---
> >>  drivers/gpu/drm/tegra/drm.c  | 21 +++++++++++++++++++++
> >>  include/uapi/drm/tegra_drm.h |  9 +++++++++
> >>  2 files changed, 30 insertions(+)
> >
> > To be honest, I'd rather just fix the existing IOCTL to do the right
> > thing on 64-bit. All IOCTLs are still protected by the DRM_TEGRA_STAGING
> > Kconfig symbol which depends on STAGING. We originally did that
> > precisely so we'd have some leeway in fixing things up. And we've done
> > precisely that in the past.
> >
> > The only user of this IOCTL is libdrm and I don't think that has any
> > users aside from a few projects that are still under heavy development
> > (like grate or the xf86-video-opentegra driver).
> >
> > Cc'ing Erik, who's probably the only one that's ever worked with this,
> > besides me.
> 
> I also saw the patch, and had the same reaction. I'm fine with
> changing the ABI, it was done already anyway
> (cbfbbabb89b37f6bad05f478d906a385149f288d, "drm/tegra: Remove
> gratuitous pad field"). And as you say, this is only in staging so
> nobody is really relying on it, except grate and libdrm (in which this
> is clearly marked as experimental). I'm fine with just changing it,
> and updating grate and libdrm accordingly.

Okay, I can prepare a patch for libdrm and push it if we decide to go
ahead with this plan.

Rob, Sean, (anyone,) any objections to just changing the ABI? I made
another pass through the list of IOCTL argument structures and couldn't
spot any others that would have the same issue. Perhaps we're finally
approaching a point where we can remove the dependency on STAGING?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets
  2015-01-30 10:21         ` Thierry Reding
@ 2015-01-30 11:41           ` Emil Velikov
  0 siblings, 0 replies; 14+ messages in thread
From: Emil Velikov @ 2015-01-30 11:41 UTC (permalink / raw)
  To: Thierry Reding, Erik Faye-Lund; +Cc: marcheu, emil.l.velikov, dri-devel

On 30/01/15 10:21, Thierry Reding wrote:
> On Fri, Jan 30, 2015 at 11:15:30AM +0100, Erik Faye-Lund wrote:
>> On Fri, Jan 30, 2015 at 10:49 AM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>>> On Thu, Jan 29, 2015 at 02:18:41PM -0500, Sean Paul wrote:
>>>> On 64-bit targets, tegra_gem_mmap doesn't return the
>>>> offset to userspace. As such, subsequent calls to mmap(2)
>>>> fail. Add a new tegra_gem_mmap2 ioctl to fix this.
>>>>
>>>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>>>> ---
>>>>  drivers/gpu/drm/tegra/drm.c  | 21 +++++++++++++++++++++
>>>>  include/uapi/drm/tegra_drm.h |  9 +++++++++
>>>>  2 files changed, 30 insertions(+)
>>>
>>> To be honest, I'd rather just fix the existing IOCTL to do the right
>>> thing on 64-bit. All IOCTLs are still protected by the DRM_TEGRA_STAGING
>>> Kconfig symbol which depends on STAGING. We originally did that
>>> precisely so we'd have some leeway in fixing things up. And we've done
>>> precisely that in the past.
>>>
>>> The only user of this IOCTL is libdrm and I don't think that has any
>>> users aside from a few projects that are still under heavy development
>>> (like grate or the xf86-video-opentegra driver).
>>>
>>> Cc'ing Erik, who's probably the only one that's ever worked with this,
>>> besides me.
>>
>> I also saw the patch, and had the same reaction. I'm fine with
>> changing the ABI, it was done already anyway
>> (cbfbbabb89b37f6bad05f478d906a385149f288d, "drm/tegra: Remove
>> gratuitous pad field"). And as you say, this is only in staging so
>> nobody is really relying on it, except grate and libdrm (in which this
>> is clearly marked as experimental). I'm fine with just changing it,
>> and updating grate and libdrm accordingly.
> 
> Okay, I can prepare a patch for libdrm and push it if we decide to go
> ahead with this plan.
> 
> Rob, Sean, (anyone,) any objections to just changing the ABI? I made
> another pass through the list of IOCTL argument structures and couldn't
> spot any others that would have the same issue. Perhaps we're finally
> approaching a point where we can remove the dependency on STAGING?
> 
Hi Thierry,

Just a small tip - pahole is nice little helper wrt structure
size/padding. Just scan the 32 & 64bit build and grep + diff the struct
sizes.

Cheers,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap
  2015-01-30  9:49     ` Thierry Reding
  2015-01-30 10:15       ` Erik Faye-Lund
@ 2015-01-30 18:57       ` Sean Paul
  2015-02-06 12:18         ` Thierry Reding
  1 sibling, 1 reply; 14+ messages in thread
From: Sean Paul @ 2015-01-30 18:57 UTC (permalink / raw)
  To: kusmabite, thierry.reding; +Cc: marcheu, dri-devel

On 64-bit targets, tegra_gem_mmap doesn't return the
offset to userspace. As such, subsequent calls to mmap(2)
fail. Alter the args to use 64-bit offset to fix this.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 include/uapi/drm/tegra_drm.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index c15d781..5391780 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -36,7 +36,8 @@ struct drm_tegra_gem_create {
 
 struct drm_tegra_gem_mmap {
 	__u32 handle;
-	__u32 offset;
+	__u32 pad;
+	__u64 offset;
 };
 
 struct drm_tegra_syncpt_read {
-- 
2.2.0.rc0.207.ga3a616c

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap
  2015-01-30 18:57       ` [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap Sean Paul
@ 2015-02-06 12:18         ` Thierry Reding
  2015-06-28 20:54           ` Dmitry
  2015-06-29  4:16           ` Dave Airlie
  0 siblings, 2 replies; 14+ messages in thread
From: Thierry Reding @ 2015-02-06 12:18 UTC (permalink / raw)
  To: Sean Paul; +Cc: marcheu, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 471 bytes --]

On Fri, Jan 30, 2015 at 01:57:01PM -0500, Sean Paul wrote:
> On 64-bit targets, tegra_gem_mmap doesn't return the
> offset to userspace. As such, subsequent calls to mmap(2)
> fail. Alter the args to use 64-bit offset to fix this.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  include/uapi/drm/tegra_drm.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

I've applied this with a slightly tweaked commit message.

Thanks,
Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap
  2015-02-06 12:18         ` Thierry Reding
@ 2015-06-28 20:54           ` Dmitry
  2015-06-29  8:46             ` Thierry Reding
  2015-06-29  4:16           ` Dave Airlie
  1 sibling, 1 reply; 14+ messages in thread
From: Dmitry @ 2015-06-28 20:54 UTC (permalink / raw)
  To: Thierry Reding, Sean Paul; +Cc: marcheu, dri-devel

06.02.2015 15:18, Thierry Reding пишет:
> On Fri, Jan 30, 2015 at 01:57:01PM -0500, Sean Paul wrote:
>> On 64-bit targets, tegra_gem_mmap doesn't return the
>> offset to userspace. As such, subsequent calls to mmap(2)
>> fail. Alter the args to use 64-bit offset to fix this.
>>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>>   include/uapi/drm/tegra_drm.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> I've applied this with a slightly tweaked commit message.
>
> Thanks,
> Thierry
>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Why it doesn't have stable tag?

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap
  2015-02-06 12:18         ` Thierry Reding
  2015-06-28 20:54           ` Dmitry
@ 2015-06-29  4:16           ` Dave Airlie
  2015-06-29  8:50             ` Thierry Reding
  1 sibling, 1 reply; 14+ messages in thread
From: Dave Airlie @ 2015-06-29  4:16 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Stéphane Marchesin, dri-devel

On 6 February 2015 at 22:18, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Fri, Jan 30, 2015 at 01:57:01PM -0500, Sean Paul wrote:
>> On 64-bit targets, tegra_gem_mmap doesn't return the
>> offset to userspace. As such, subsequent calls to mmap(2)
>> fail. Alter the args to use 64-bit offset to fix this.
>>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>>  include/uapi/drm/tegra_drm.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> I've applied this with a slightly tweaked commit message.

Doesn't that break 32-bit ABI?

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap
  2015-06-28 20:54           ` Dmitry
@ 2015-06-29  8:46             ` Thierry Reding
  0 siblings, 0 replies; 14+ messages in thread
From: Thierry Reding @ 2015-06-29  8:46 UTC (permalink / raw)
  To: Dmitry; +Cc: marcheu, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1047 bytes --]

On Sun, Jun 28, 2015 at 11:54:12PM +0300, Dmitry wrote:
> 06.02.2015 15:18, Thierry Reding пишет:
> >On Fri, Jan 30, 2015 at 01:57:01PM -0500, Sean Paul wrote:
> >>On 64-bit targets, tegra_gem_mmap doesn't return the
> >>offset to userspace. As such, subsequent calls to mmap(2)
> >>fail. Alter the args to use 64-bit offset to fix this.
> >>
> >>Signed-off-by: Sean Paul <seanpaul@chromium.org>
> >>---
> >>  include/uapi/drm/tegra_drm.h | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >I've applied this with a slightly tweaked commit message.
> >
> >Thanks,
> >Thierry
> >
> >
> >
> >_______________________________________________
> >dri-devel mailing list
> >dri-devel@lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> Why it doesn't have stable tag?

It's an experimental feature (guarded by STAGING) that doesn't have any
users except proof of concept code, hence I didn't think it appropriate
to burden stable kernel maintainers with it.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap
  2015-06-29  4:16           ` Dave Airlie
@ 2015-06-29  8:50             ` Thierry Reding
  0 siblings, 0 replies; 14+ messages in thread
From: Thierry Reding @ 2015-06-29  8:50 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Stéphane Marchesin, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1052 bytes --]

On Mon, Jun 29, 2015 at 02:16:17PM +1000, Dave Airlie wrote:
> On 6 February 2015 at 22:18, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Fri, Jan 30, 2015 at 01:57:01PM -0500, Sean Paul wrote:
> >> On 64-bit targets, tegra_gem_mmap doesn't return the
> >> offset to userspace. As such, subsequent calls to mmap(2)
> >> fail. Alter the args to use 64-bit offset to fix this.
> >>
> >> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> >> ---
> >>  include/uapi/drm/tegra_drm.h | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > I've applied this with a slightly tweaked commit message.
> 
> Doesn't that break 32-bit ABI?

Yes it does. This was discussed earlier in this thread. The original
patch was to add a separate IOCTL to be used on 64-bit architectures
because the 32-bit IOCTL was broken. After some discussion everybody
involved agreed that it'd be best to fix the IOCTL while we can (the
driver-specific IOCTLs in the Tegra driver are all guarded by the
STAGING Kconfig symbol).

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-06-29  8:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-29 18:46 [PATCH] drm/tegra: Add tegra_gem_mmap2 to fix 64-bit offsets Sean Paul
2015-01-29 18:55 ` Rob Clark
2015-01-29 19:18   ` [PATCH v2] " Sean Paul
2015-01-29 20:11     ` Rob Clark
2015-01-30  9:49     ` Thierry Reding
2015-01-30 10:15       ` Erik Faye-Lund
2015-01-30 10:21         ` Thierry Reding
2015-01-30 11:41           ` Emil Velikov
2015-01-30 18:57       ` [PATCH v3] drm/tegra: Use 64-bit offset for tegra_gem_mmap Sean Paul
2015-02-06 12:18         ` Thierry Reding
2015-06-28 20:54           ` Dmitry
2015-06-29  8:46             ` Thierry Reding
2015-06-29  4:16           ` Dave Airlie
2015-06-29  8:50             ` Thierry Reding

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.