[PATCH 0/2] drm/nouveau: option for staging ioctls and new SET_TILING ioctl

[PATCH 0/2] drm/nouveau: option for staging ioctls and new SET_TILING ioctl

[Alexandre Courbot]

This patchset proposes to introduce a "staging" module option to dynamically
enable features (mostly ioctls) that are merged but may be refined before
they are declared "stable". The second patch illustrates the use of this
staging option with the SET_TILING ioctl, which can be used to specify the
tiling options of a PRIME-imported buffer.

The staging parameter will allow us (especially, us at NVIDIA) to experiment
more freely with new features and avoid carrying patches across version. To
prevent abuse, the number of staging IOCTLs is limited to 8 (range 0x98 to
0xa0) that are to be recycled as staging IOCTLs become stable and are assigned
a final number.

Alexandre Courbot (1):
  drm: add staging module option

Ari Hirvonen (1):
  drm/nouveau: Set tile mode

 drm/nouveau/nouveau_drm.c          | 19 +++++++++++++
 drm/nouveau/nouveau_gem.c          | 55 ++++++++++++++++++++++++++++++++++++++
 drm/nouveau/nouveau_gem.h          |  2 ++
 drm/nouveau/uapi/drm/nouveau_drm.h | 11 ++++++++
 4 files changed, 87 insertions(+)

-- 
2.4.0

[PATCH 1/2] drm/nouveau: add staging module option

[Alexandre Courbot]

Add a module option allowing to enable staging/unstable APIs. This will
allow us to experiment freely with experimental APIs for a while before
setting them in stone.

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drm/nouveau/nouveau_drm.c          | 18 ++++++++++++++++++
 drm/nouveau/uapi/drm/nouveau_drm.h |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
index 89049335b738..e4bd6ed51e73 100644
--- a/drm/nouveau/nouveau_drm.c
+++ b/drm/nouveau/nouveau_drm.c
@@ -75,6 +75,10 @@ MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only default (-1
 int nouveau_runtime_pm = -1;
 module_param_named(runpm, nouveau_runtime_pm, int, 0400);
 
+MODULE_PARM_DESC(staging, "enable staging APIs");
+int nouveau_staging = 0;
+module_param_named(staging, nouveau_staging, int, 0400);
+
 static struct drm_driver driver_stub;
 static struct drm_driver driver_pci;
 static struct drm_driver driver_platform;
@@ -895,6 +899,7 @@ nouveau_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	/* Staging ioctls */
 };
 
 long
@@ -1027,6 +1032,7 @@ static void nouveau_display_options(void)
 	DRM_DEBUG_DRIVER("... runpm        : %d\n", nouveau_runtime_pm);
 	DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
 	DRM_DEBUG_DRIVER("... pstate       : %d\n", nouveau_pstate);
+	DRM_DEBUG_DRIVER("... staging      : %d\n", nouveau_staging);
 }
 
 static const struct dev_pm_ops nouveau_pm_ops = {
@@ -1088,6 +1094,18 @@ err_free:
 static int __init
 nouveau_drm_init(void)
 {
+	/* Do not register staging ioctsl if option not specified */
+	if (!nouveau_staging) {
+		unsigned i;
+
+		/* This keeps us safe is no staging ioctls are defined */
+		i = min(driver_stub.num_ioctls, DRM_NOUVEAU_STAGING_IOCTL);
+		while (!nouveau_ioctls[i - 1].func)
+			i--;
+
+		driver_stub.num_ioctls = i;
+	}
+
 	driver_pci = driver_stub;
 	driver_pci.set_busid = drm_pci_set_busid;
 	driver_platform = driver_stub;
diff --git a/drm/nouveau/uapi/drm/nouveau_drm.h b/drm/nouveau/uapi/drm/nouveau_drm.h
index 5507eead5863..4e7e21f41b5c 100644
--- a/drm/nouveau/uapi/drm/nouveau_drm.h
+++ b/drm/nouveau/uapi/drm/nouveau_drm.h
@@ -140,11 +140,14 @@ struct drm_nouveau_gem_cpu_fini {
 #define DRM_NOUVEAU_GEM_CPU_PREP       0x42
 #define DRM_NOUVEAU_GEM_CPU_FINI       0x43
 #define DRM_NOUVEAU_GEM_INFO           0x44
+/* range 0x98..DRM_COMMAND_END (8 entries) is reserved for staging, unstable ioctls */
+#define DRM_NOUVEAU_STAGING_IOCTL      0x58
 
 #define DRM_IOCTL_NOUVEAU_GEM_NEW            DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_NEW, struct drm_nouveau_gem_new)
 #define DRM_IOCTL_NOUVEAU_GEM_PUSHBUF        DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_PUSHBUF, struct drm_nouveau_gem_pushbuf)
 #define DRM_IOCTL_NOUVEAU_GEM_CPU_PREP       DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_CPU_PREP, struct drm_nouveau_gem_cpu_prep)
 #define DRM_IOCTL_NOUVEAU_GEM_CPU_FINI       DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_CPU_FINI, struct drm_nouveau_gem_cpu_fini)
 #define DRM_IOCTL_NOUVEAU_GEM_INFO           DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_INFO, struct drm_nouveau_gem_info)
+/* staging ioctls */
 
 #endif /* __NOUVEAU_DRM_H__ */
-- 
2.4.0

[PATCH 2/2] drm/nouveau: Set tile mode

[Alexandre Courbot]

From: Ari Hirvonen <ahirvonen@nvidia.com>

Add new NOUVEAU_GEM_SET_TILING ioctl to set correct tiling
mode for imported dma-bufs. This ioctl is staging for now.

Signed-off-by: Ari Hirvonen <ahirvonen@nvidia.com>
[acourbot@nvidia.com: carry upstream, fix style]
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drm/nouveau/nouveau_drm.c          |  1 +
 drm/nouveau/nouveau_gem.c          | 55 ++++++++++++++++++++++++++++++++++++++
 drm/nouveau/nouveau_gem.h          |  2 ++
 drm/nouveau/uapi/drm/nouveau_drm.h |  8 ++++++
 4 files changed, 66 insertions(+)

diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
index e4bd6ed51e73..1c7898ec100c 100644
--- a/drm/nouveau/nouveau_drm.c
+++ b/drm/nouveau/nouveau_drm.c
@@ -900,6 +900,7 @@ nouveau_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 	/* Staging ioctls */
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_SET_TILING, nouveau_gem_ioctl_set_tiling, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 };
 
 long
diff --git a/drm/nouveau/nouveau_gem.c b/drm/nouveau/nouveau_gem.c
index 0e690bf19fc9..46fa4df0b390 100644
--- a/drm/nouveau/nouveau_gem.c
+++ b/drm/nouveau/nouveau_gem.c
@@ -173,6 +173,61 @@ nouveau_gem_object_close(struct drm_gem_object *gem, struct drm_file *file_priv)
 }
 
 int
+nouveau_gem_ioctl_set_tiling(struct drm_device *dev, void *data,
+			     struct drm_file *file_priv)
+{
+	struct nouveau_drm *drm = nouveau_drm(dev);
+	struct nouveau_cli *cli = nouveau_cli(file_priv);
+	struct nvkm_fb *pfb = nvxx_fb(&drm->device);
+	struct drm_nouveau_gem_set_tiling *req = data;
+	struct drm_gem_object *gem;
+	struct nouveau_bo *nvbo;
+	struct nvkm_mem *mem;
+	struct nvkm_vma *vma;
+	int ret = 0;
+
+	if (!pfb->memtype_valid(pfb, req->tile_flags)) {
+		NV_PRINTK(error, cli, "bad memtype: 0x%08x\n", req->tile_flags);
+		return -EINVAL;
+	}
+
+	gem = drm_gem_object_lookup(dev, file_priv, req->handle);
+	if (!gem)
+		return -ENOENT;
+
+	nvbo = nouveau_gem_object(gem);
+
+	if (nvbo->tile_mode != req->tile_mode ||
+	    nvbo->tile_flags != req->tile_flags) {
+
+		ret = ttm_bo_reserve(&nvbo->bo, false, false, false, NULL);
+		if (ret)
+			goto out;
+
+		vma = nouveau_bo_vma_find(nvbo, cli->vm);
+		if (!vma) {
+			ret = -ENOENT;
+			goto unreserve;
+		}
+
+		mem = nvbo->bo.mem.mm_node;
+		nvbo->tile_mode = req->tile_mode;
+		nvbo->tile_flags = req->tile_flags;
+
+		/* Need to rewrite page tables */
+		mem->memtype = (nvbo->tile_flags >> 8) & 0xff;
+		nvkm_vm_map(vma, nvbo->bo.mem.mm_node);
+
+unreserve:
+		ttm_bo_unreserve(&nvbo->bo);
+	}
+
+out:
+	drm_gem_object_unreference_unlocked(gem);
+	return ret;
+}
+
+int
 nouveau_gem_new(struct drm_device *dev, int size, int align, uint32_t domain,
 		uint32_t tile_mode, uint32_t tile_flags,
 		struct nouveau_bo **pnvbo)
diff --git a/drm/nouveau/nouveau_gem.h b/drm/nouveau/nouveau_gem.h
index e4049faca780..56e741d98bcd 100644
--- a/drm/nouveau/nouveau_gem.h
+++ b/drm/nouveau/nouveau_gem.h
@@ -23,6 +23,8 @@ extern void nouveau_gem_object_del(struct drm_gem_object *);
 extern int nouveau_gem_object_open(struct drm_gem_object *, struct drm_file *);
 extern void nouveau_gem_object_close(struct drm_gem_object *,
 				     struct drm_file *);
+extern int nouveau_gem_ioctl_set_tiling(struct drm_device *, void *,
+					struct drm_file *);
 extern int nouveau_gem_ioctl_new(struct drm_device *, void *,
 				 struct drm_file *);
 extern int nouveau_gem_ioctl_pushbuf(struct drm_device *, void *,
diff --git a/drm/nouveau/uapi/drm/nouveau_drm.h b/drm/nouveau/uapi/drm/nouveau_drm.h
index 4e7e21f41b5c..8f10b16b1473 100644
--- a/drm/nouveau/uapi/drm/nouveau_drm.h
+++ b/drm/nouveau/uapi/drm/nouveau_drm.h
@@ -64,6 +64,12 @@ struct drm_nouveau_gem_new {
 	uint32_t align;
 };
 
+struct drm_nouveau_gem_set_tiling {
+	uint32_t handle;
+	uint32_t tile_mode;
+	uint32_t tile_flags;
+};
+
 #define NOUVEAU_GEM_MAX_BUFFERS 1024
 struct drm_nouveau_gem_pushbuf_bo_presumed {
 	uint32_t valid;
@@ -142,6 +148,7 @@ struct drm_nouveau_gem_cpu_fini {
 #define DRM_NOUVEAU_GEM_INFO           0x44
 /* range 0x98..DRM_COMMAND_END (8 entries) is reserved for staging, unstable ioctls */
 #define DRM_NOUVEAU_STAGING_IOCTL      0x58
+#define DRM_NOUVEAU_GEM_SET_TILING     (DRM_NOUVEAU_STAGING_IOCTL + 0x0)
 
 #define DRM_IOCTL_NOUVEAU_GEM_NEW            DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_NEW, struct drm_nouveau_gem_new)
 #define DRM_IOCTL_NOUVEAU_GEM_PUSHBUF        DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_PUSHBUF, struct drm_nouveau_gem_pushbuf)
@@ -149,5 +156,6 @@ struct drm_nouveau_gem_cpu_fini {
 #define DRM_IOCTL_NOUVEAU_GEM_CPU_FINI       DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_CPU_FINI, struct drm_nouveau_gem_cpu_fini)
 #define DRM_IOCTL_NOUVEAU_GEM_INFO           DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_INFO, struct drm_nouveau_gem_info)
 /* staging ioctls */
+#define DRM_IOCTL_NOUVEAU_GEM_SET_TILING     DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_SET_TILING, struct drm_nouveau_gem_set_tiling)
 
 #endif /* __NOUVEAU_DRM_H__ */
-- 
2.4.0

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 1/2] drm/nouveau: add staging module option

[Ben Skeggs]

On 20 May 2015 at 15:56, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Add a module option allowing to enable staging/unstable APIs. This will
> allow us to experiment freely with experimental APIs for a while before
> setting them in stone.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drm/nouveau/nouveau_drm.c          | 18 ++++++++++++++++++
>  drm/nouveau/uapi/drm/nouveau_drm.h |  3 +++
>  2 files changed, 21 insertions(+)
>
> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
> index 89049335b738..e4bd6ed51e73 100644
> --- a/drm/nouveau/nouveau_drm.c
> +++ b/drm/nouveau/nouveau_drm.c
> @@ -75,6 +75,10 @@ MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only default (-1
>  int nouveau_runtime_pm = -1;
>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>
> +MODULE_PARM_DESC(staging, "enable staging APIs");
> +int nouveau_staging = 0;
> +module_param_named(staging, nouveau_staging, int, 0400);
> +
>  static struct drm_driver driver_stub;
>  static struct drm_driver driver_pci;
>  static struct drm_driver driver_platform;
> @@ -895,6 +899,7 @@ nouveau_ioctls[] = {
>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
> +       /* Staging ioctls */
>  };
>
>  long
> @@ -1027,6 +1032,7 @@ static void nouveau_display_options(void)
>         DRM_DEBUG_DRIVER("... runpm        : %d\n", nouveau_runtime_pm);
>         DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
>         DRM_DEBUG_DRIVER("... pstate       : %d\n", nouveau_pstate);
> +       DRM_DEBUG_DRIVER("... staging      : %d\n", nouveau_staging);
>  }
>
>  static const struct dev_pm_ops nouveau_pm_ops = {
> @@ -1088,6 +1094,18 @@ err_free:
>  static int __init
>  nouveau_drm_init(void)
>  {
> +       /* Do not register staging ioctsl if option not specified */
> +       if (!nouveau_staging) {
> +               unsigned i;
> +
> +               /* This keeps us safe is no staging ioctls are defined */
> +               i = min(driver_stub.num_ioctls, DRM_NOUVEAU_STAGING_IOCTL);
> +               while (!nouveau_ioctls[i - 1].func)
> +                       i--;
> +
> +               driver_stub.num_ioctls = i;
> +       }
Hey Alex,

I've got no specific objection.  But I'm curious as to why you took
this approach as opposed to just adding "if (!nouveau_staging) return
-EINVAL;" directly in the experimental ioctls?  I think, in line with
what's been done in other places, having module options per-api is
perhaps a better choice too.

Ben.

> +
>         driver_pci = driver_stub;
>         driver_pci.set_busid = drm_pci_set_busid;
>         driver_platform = driver_stub;
> diff --git a/drm/nouveau/uapi/drm/nouveau_drm.h b/drm/nouveau/uapi/drm/nouveau_drm.h
> index 5507eead5863..4e7e21f41b5c 100644
> --- a/drm/nouveau/uapi/drm/nouveau_drm.h
> +++ b/drm/nouveau/uapi/drm/nouveau_drm.h
> @@ -140,11 +140,14 @@ struct drm_nouveau_gem_cpu_fini {
>  #define DRM_NOUVEAU_GEM_CPU_PREP       0x42
>  #define DRM_NOUVEAU_GEM_CPU_FINI       0x43
>  #define DRM_NOUVEAU_GEM_INFO           0x44
> +/* range 0x98..DRM_COMMAND_END (8 entries) is reserved for staging, unstable ioctls */
> +#define DRM_NOUVEAU_STAGING_IOCTL      0x58
>
>  #define DRM_IOCTL_NOUVEAU_GEM_NEW            DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_NEW, struct drm_nouveau_gem_new)
>  #define DRM_IOCTL_NOUVEAU_GEM_PUSHBUF        DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_PUSHBUF, struct drm_nouveau_gem_pushbuf)
>  #define DRM_IOCTL_NOUVEAU_GEM_CPU_PREP       DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_CPU_PREP, struct drm_nouveau_gem_cpu_prep)
>  #define DRM_IOCTL_NOUVEAU_GEM_CPU_FINI       DRM_IOW (DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_CPU_FINI, struct drm_nouveau_gem_cpu_fini)
>  #define DRM_IOCTL_NOUVEAU_GEM_INFO           DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_GEM_INFO, struct drm_nouveau_gem_info)
> +/* staging ioctls */
>
>  #endif /* __NOUVEAU_DRM_H__ */
> --
> 2.4.0
>
> _______________________________________________
> 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

Re: [PATCH 1/2] drm/nouveau: add staging module option

[Alexandre Courbot]

On Thu, May 21, 2015 at 1:48 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 20 May 2015 at 15:56, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> Add a module option allowing to enable staging/unstable APIs. This will
>> allow us to experiment freely with experimental APIs for a while before
>> setting them in stone.
>>
>> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drm/nouveau/nouveau_drm.c          | 18 ++++++++++++++++++
>>  drm/nouveau/uapi/drm/nouveau_drm.h |  3 +++
>>  2 files changed, 21 insertions(+)
>>
>> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
>> index 89049335b738..e4bd6ed51e73 100644
>> --- a/drm/nouveau/nouveau_drm.c
>> +++ b/drm/nouveau/nouveau_drm.c
>> @@ -75,6 +75,10 @@ MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only default (-1
>>  int nouveau_runtime_pm = -1;
>>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>>
>> +MODULE_PARM_DESC(staging, "enable staging APIs");
>> +int nouveau_staging = 0;
>> +module_param_named(staging, nouveau_staging, int, 0400);
>> +
>>  static struct drm_driver driver_stub;
>>  static struct drm_driver driver_pci;
>>  static struct drm_driver driver_platform;
>> @@ -895,6 +899,7 @@ nouveau_ioctls[] = {
>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>> +       /* Staging ioctls */
>>  };
>>
>>  long
>> @@ -1027,6 +1032,7 @@ static void nouveau_display_options(void)
>>         DRM_DEBUG_DRIVER("... runpm        : %d\n", nouveau_runtime_pm);
>>         DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
>>         DRM_DEBUG_DRIVER("... pstate       : %d\n", nouveau_pstate);
>> +       DRM_DEBUG_DRIVER("... staging      : %d\n", nouveau_staging);
>>  }
>>
>>  static const struct dev_pm_ops nouveau_pm_ops = {
>> @@ -1088,6 +1094,18 @@ err_free:
>>  static int __init
>>  nouveau_drm_init(void)
>>  {
>> +       /* Do not register staging ioctsl if option not specified */
>> +       if (!nouveau_staging) {
>> +               unsigned i;
>> +
>> +               /* This keeps us safe is no staging ioctls are defined */
>> +               i = min(driver_stub.num_ioctls, DRM_NOUVEAU_STAGING_IOCTL);
>> +               while (!nouveau_ioctls[i - 1].func)
>> +                       i--;
>> +
>> +               driver_stub.num_ioctls = i;
>> +       }
> Hey Alex,
>
> I've got no specific objection.  But I'm curious as to why you took
> this approach as opposed to just adding "if (!nouveau_staging) return
> -EINVAL;" directly in the experimental ioctls?

Mainly because we will likely forget to add this check (or to remove
it) in some of the staging ioctls. The current solution doesn't
require us to think about that - and the less things to think about,
the better.

> I think, in line with
> what's been done in other places, having module options per-api is
> perhaps a better choice too.

Do you mean that each experimental ioctl should have its own enable
option? I don't mind going that way if you think it is preferable. And
in that case my comment above is void.

But actually I wonder if having these experimental ioctls enabled as
compile options (either individually or as a whole) would not be
better. Some experimental ioctls may require code in staging (like the
PUSHBUF_2 ioctl that I would like to submit next), and I don't think
it is desirable to force extra code or kernel options (in this case,
CONFIG_STAGING) to Nouveau users who will not make use of them. I
remember that we concluded in favor or module options on IRC, but in
the light of this, wouldn't a config option be a less intrusive
choice?

Re: [PATCH 1/2] drm/nouveau: add staging module option

[Ben Skeggs]

On 21 May 2015 at 15:49, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, May 21, 2015 at 1:48 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 20 May 2015 at 15:56, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>> Add a module option allowing to enable staging/unstable APIs. This will
>>> allow us to experiment freely with experimental APIs for a while before
>>> setting them in stone.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  drm/nouveau/nouveau_drm.c          | 18 ++++++++++++++++++
>>>  drm/nouveau/uapi/drm/nouveau_drm.h |  3 +++
>>>  2 files changed, 21 insertions(+)
>>>
>>> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
>>> index 89049335b738..e4bd6ed51e73 100644
>>> --- a/drm/nouveau/nouveau_drm.c
>>> +++ b/drm/nouveau/nouveau_drm.c
>>> @@ -75,6 +75,10 @@ MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only default (-1
>>>  int nouveau_runtime_pm = -1;
>>>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>>>
>>> +MODULE_PARM_DESC(staging, "enable staging APIs");
>>> +int nouveau_staging = 0;
>>> +module_param_named(staging, nouveau_staging, int, 0400);
>>> +
>>>  static struct drm_driver driver_stub;
>>>  static struct drm_driver driver_pci;
>>>  static struct drm_driver driver_platform;
>>> @@ -895,6 +899,7 @@ nouveau_ioctls[] = {
>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>> +       /* Staging ioctls */
>>>  };
>>>
>>>  long
>>> @@ -1027,6 +1032,7 @@ static void nouveau_display_options(void)
>>>         DRM_DEBUG_DRIVER("... runpm        : %d\n", nouveau_runtime_pm);
>>>         DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
>>>         DRM_DEBUG_DRIVER("... pstate       : %d\n", nouveau_pstate);
>>> +       DRM_DEBUG_DRIVER("... staging      : %d\n", nouveau_staging);
>>>  }
>>>
>>>  static const struct dev_pm_ops nouveau_pm_ops = {
>>> @@ -1088,6 +1094,18 @@ err_free:
>>>  static int __init
>>>  nouveau_drm_init(void)
>>>  {
>>> +       /* Do not register staging ioctsl if option not specified */
>>> +       if (!nouveau_staging) {
>>> +               unsigned i;
>>> +
>>> +               /* This keeps us safe is no staging ioctls are defined */
>>> +               i = min(driver_stub.num_ioctls, DRM_NOUVEAU_STAGING_IOCTL);
>>> +               while (!nouveau_ioctls[i - 1].func)
>>> +                       i--;
>>> +
>>> +               driver_stub.num_ioctls = i;
>>> +       }
>> Hey Alex,
>>
>> I've got no specific objection.  But I'm curious as to why you took
>> this approach as opposed to just adding "if (!nouveau_staging) return
>> -EINVAL;" directly in the experimental ioctls?
>
> Mainly because we will likely forget to add this check (or to remove
> it) in some of the staging ioctls. The current solution doesn't
> require us to think about that - and the less things to think about,
> the better.
>
>> I think, in line with
>> what's been done in other places, having module options per-api is
>> perhaps a better choice too.
>
> Do you mean that each experimental ioctl should have its own enable
> option? I don't mind going that way if you think it is preferable. And
> in that case my comment above is void.
That would be more preferable I think, and obvious as to what exactly
you're enabling.

>
> But actually I wonder if having these experimental ioctls enabled as
> compile options (either individually or as a whole) would not be
> better. Some experimental ioctls may require code in staging (like the
> PUSHBUF_2 ioctl that I would like to submit next), and I don't think
> it is desirable to force extra code or kernel options (in this case,
> CONFIG_STAGING) to Nouveau users who will not make use of them. I
> remember that we concluded in favor or module options on IRC, but in
> the light of this, wouldn't a config option be a less intrusive
> choice?
Right, but the whole point of this is to encourage the ioctls to not
live there for too long, and progress to fully supported interfaces.

Ben.

Re: [PATCH 1/2] drm/nouveau: add staging module option

[Alexandre Courbot]

On Thu, May 21, 2015 at 2:55 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 21 May 2015 at 15:49, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Thu, May 21, 2015 at 1:48 PM, Ben Skeggs <skeggsb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On 20 May 2015 at 15:56, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>> Add a module option allowing to enable staging/unstable APIs. This will
>>>> allow us to experiment freely with experimental APIs for a while before
>>>> setting them in stone.
>>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>  drm/nouveau/nouveau_drm.c          | 18 ++++++++++++++++++
>>>>  drm/nouveau/uapi/drm/nouveau_drm.h |  3 +++
>>>>  2 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
>>>> index 89049335b738..e4bd6ed51e73 100644
>>>> --- a/drm/nouveau/nouveau_drm.c
>>>> +++ b/drm/nouveau/nouveau_drm.c
>>>> @@ -75,6 +75,10 @@ MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only default (-1
>>>>  int nouveau_runtime_pm = -1;
>>>>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>>>>
>>>> +MODULE_PARM_DESC(staging, "enable staging APIs");
>>>> +int nouveau_staging = 0;
>>>> +module_param_named(staging, nouveau_staging, int, 0400);
>>>> +
>>>>  static struct drm_driver driver_stub;
>>>>  static struct drm_driver driver_pci;
>>>>  static struct drm_driver driver_platform;
>>>> @@ -895,6 +899,7 @@ nouveau_ioctls[] = {
>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>> +       /* Staging ioctls */
>>>>  };
>>>>
>>>>  long
>>>> @@ -1027,6 +1032,7 @@ static void nouveau_display_options(void)
>>>>         DRM_DEBUG_DRIVER("... runpm        : %d\n", nouveau_runtime_pm);
>>>>         DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
>>>>         DRM_DEBUG_DRIVER("... pstate       : %d\n", nouveau_pstate);
>>>> +       DRM_DEBUG_DRIVER("... staging      : %d\n", nouveau_staging);
>>>>  }
>>>>
>>>>  static const struct dev_pm_ops nouveau_pm_ops = {
>>>> @@ -1088,6 +1094,18 @@ err_free:
>>>>  static int __init
>>>>  nouveau_drm_init(void)
>>>>  {
>>>> +       /* Do not register staging ioctsl if option not specified */
>>>> +       if (!nouveau_staging) {
>>>> +               unsigned i;
>>>> +
>>>> +               /* This keeps us safe is no staging ioctls are defined */
>>>> +               i = min(driver_stub.num_ioctls, DRM_NOUVEAU_STAGING_IOCTL);
>>>> +               while (!nouveau_ioctls[i - 1].func)
>>>> +                       i--;
>>>> +
>>>> +               driver_stub.num_ioctls = i;
>>>> +       }
>>> Hey Alex,
>>>
>>> I've got no specific objection.  But I'm curious as to why you took
>>> this approach as opposed to just adding "if (!nouveau_staging) return
>>> -EINVAL;" directly in the experimental ioctls?
>>
>> Mainly because we will likely forget to add this check (or to remove
>> it) in some of the staging ioctls. The current solution doesn't
>> require us to think about that - and the less things to think about,
>> the better.
>>
>>> I think, in line with
>>> what's been done in other places, having module options per-api is
>>> perhaps a better choice too.
>>
>> Do you mean that each experimental ioctl should have its own enable
>> option? I don't mind going that way if you think it is preferable. And
>> in that case my comment above is void.
> That would be more preferable I think, and obvious as to what exactly
> you're enabling.
>
>>
>> But actually I wonder if having these experimental ioctls enabled as
>> compile options (either individually or as a whole) would not be
>> better. Some experimental ioctls may require code in staging (like the
>> PUSHBUF_2 ioctl that I would like to submit next), and I don't think
>> it is desirable to force extra code or kernel options (in this case,
>> CONFIG_STAGING) to Nouveau users who will not make use of them. I
>> remember that we concluded in favor or module options on IRC, but in
>> the light of this, wouldn't a config option be a less intrusive
>> choice?
> Right, but the whole point of this is to encourage the ioctls to not
> live there for too long, and progress to fully supported interfaces.

Definitely, but my concern is that doing this will make Nouveau depend
on STAGING for at least short periods of time. Do we really want this?

Re: [PATCH 1/2] drm/nouveau: add staging module option

[Ben Skeggs]

On 21 May 2015 at 16:04, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Thu, May 21, 2015 at 2:55 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>> On 21 May 2015 at 15:49, Alexandre Courbot <gnurou@gmail.com> wrote:
>>> On Thu, May 21, 2015 at 1:48 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>> On 20 May 2015 at 15:56, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>>>> Add a module option allowing to enable staging/unstable APIs. This will
>>>>> allow us to experiment freely with experimental APIs for a while before
>>>>> setting them in stone.
>>>>>
>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>>> ---
>>>>>  drm/nouveau/nouveau_drm.c          | 18 ++++++++++++++++++
>>>>>  drm/nouveau/uapi/drm/nouveau_drm.h |  3 +++
>>>>>  2 files changed, 21 insertions(+)
>>>>>
>>>>> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
>>>>> index 89049335b738..e4bd6ed51e73 100644
>>>>> --- a/drm/nouveau/nouveau_drm.c
>>>>> +++ b/drm/nouveau/nouveau_drm.c
>>>>> @@ -75,6 +75,10 @@ MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only default (-1
>>>>>  int nouveau_runtime_pm = -1;
>>>>>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>>>>>
>>>>> +MODULE_PARM_DESC(staging, "enable staging APIs");
>>>>> +int nouveau_staging = 0;
>>>>> +module_param_named(staging, nouveau_staging, int, 0400);
>>>>> +
>>>>>  static struct drm_driver driver_stub;
>>>>>  static struct drm_driver driver_pci;
>>>>>  static struct drm_driver driver_platform;
>>>>> @@ -895,6 +899,7 @@ nouveau_ioctls[] = {
>>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>> +       /* Staging ioctls */
>>>>>  };
>>>>>
>>>>>  long
>>>>> @@ -1027,6 +1032,7 @@ static void nouveau_display_options(void)
>>>>>         DRM_DEBUG_DRIVER("... runpm        : %d\n", nouveau_runtime_pm);
>>>>>         DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
>>>>>         DRM_DEBUG_DRIVER("... pstate       : %d\n", nouveau_pstate);
>>>>> +       DRM_DEBUG_DRIVER("... staging      : %d\n", nouveau_staging);
>>>>>  }
>>>>>
>>>>>  static const struct dev_pm_ops nouveau_pm_ops = {
>>>>> @@ -1088,6 +1094,18 @@ err_free:
>>>>>  static int __init
>>>>>  nouveau_drm_init(void)
>>>>>  {
>>>>> +       /* Do not register staging ioctsl if option not specified */
>>>>> +       if (!nouveau_staging) {
>>>>> +               unsigned i;
>>>>> +
>>>>> +               /* This keeps us safe is no staging ioctls are defined */
>>>>> +               i = min(driver_stub.num_ioctls, DRM_NOUVEAU_STAGING_IOCTL);
>>>>> +               while (!nouveau_ioctls[i - 1].func)
>>>>> +                       i--;
>>>>> +
>>>>> +               driver_stub.num_ioctls = i;
>>>>> +       }
>>>> Hey Alex,
>>>>
>>>> I've got no specific objection.  But I'm curious as to why you took
>>>> this approach as opposed to just adding "if (!nouveau_staging) return
>>>> -EINVAL;" directly in the experimental ioctls?
>>>
>>> Mainly because we will likely forget to add this check (or to remove
>>> it) in some of the staging ioctls. The current solution doesn't
>>> require us to think about that - and the less things to think about,
>>> the better.
>>>
>>>> I think, in line with
>>>> what's been done in other places, having module options per-api is
>>>> perhaps a better choice too.
>>>
>>> Do you mean that each experimental ioctl should have its own enable
>>> option? I don't mind going that way if you think it is preferable. And
>>> in that case my comment above is void.
>> That would be more preferable I think, and obvious as to what exactly
>> you're enabling.
>>
>>>
>>> But actually I wonder if having these experimental ioctls enabled as
>>> compile options (either individually or as a whole) would not be
>>> better. Some experimental ioctls may require code in staging (like the
>>> PUSHBUF_2 ioctl that I would like to submit next), and I don't think
>>> it is desirable to force extra code or kernel options (in this case,
>>> CONFIG_STAGING) to Nouveau users who will not make use of them. I
>>> remember that we concluded in favor or module options on IRC, but in
>>> the light of this, wouldn't a config option be a less intrusive
>>> choice?
>> Right, but the whole point of this is to encourage the ioctls to not
>> live there for too long, and progress to fully supported interfaces.
>
> Definitely, but my concern is that doing this will make Nouveau depend
> on STAGING for at least short periods of time. Do we really want this?
I admit to having slightly misread your last paragraph.  For cases
such as thas, a config option that depends on STAGING *and* the kernel
parameter should be used.

What is pushbuf2 doing that requires staging btw?  You've linked me to
patches previously, but I missed that.

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

Re: [PATCH 1/2] drm/nouveau: add staging module option

[Alexandre Courbot]

On Thu, May 21, 2015 at 5:35 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
> On 21 May 2015 at 16:04, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Thu, May 21, 2015 at 2:55 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>> On 21 May 2015 at 15:49, Alexandre Courbot <gnurou@gmail.com> wrote:
>>>> On Thu, May 21, 2015 at 1:48 PM, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>>> On 20 May 2015 at 15:56, Alexandre Courbot <acourbot@nvidia.com> wrote:
>>>>>> Add a module option allowing to enable staging/unstable APIs. This will
>>>>>> allow us to experiment freely with experimental APIs for a while before
>>>>>> setting them in stone.
>>>>>>
>>>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>>>> ---
>>>>>>  drm/nouveau/nouveau_drm.c          | 18 ++++++++++++++++++
>>>>>>  drm/nouveau/uapi/drm/nouveau_drm.h |  3 +++
>>>>>>  2 files changed, 21 insertions(+)
>>>>>>
>>>>>> diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c
>>>>>> index 89049335b738..e4bd6ed51e73 100644
>>>>>> --- a/drm/nouveau/nouveau_drm.c
>>>>>> +++ b/drm/nouveau/nouveau_drm.c
>>>>>> @@ -75,6 +75,10 @@ MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only default (-1
>>>>>>  int nouveau_runtime_pm = -1;
>>>>>>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
>>>>>>
>>>>>> +MODULE_PARM_DESC(staging, "enable staging APIs");
>>>>>> +int nouveau_staging = 0;
>>>>>> +module_param_named(staging, nouveau_staging, int, 0400);
>>>>>> +
>>>>>>  static struct drm_driver driver_stub;
>>>>>>  static struct drm_driver driver_pci;
>>>>>>  static struct drm_driver driver_platform;
>>>>>> @@ -895,6 +899,7 @@ nouveau_ioctls[] = {
>>>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>>>         DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
>>>>>> +       /* Staging ioctls */
>>>>>>  };
>>>>>>
>>>>>>  long
>>>>>> @@ -1027,6 +1032,7 @@ static void nouveau_display_options(void)
>>>>>>         DRM_DEBUG_DRIVER("... runpm        : %d\n", nouveau_runtime_pm);
>>>>>>         DRM_DEBUG_DRIVER("... vram_pushbuf : %d\n", nouveau_vram_pushbuf);
>>>>>>         DRM_DEBUG_DRIVER("... pstate       : %d\n", nouveau_pstate);
>>>>>> +       DRM_DEBUG_DRIVER("... staging      : %d\n", nouveau_staging);
>>>>>>  }
>>>>>>
>>>>>>  static const struct dev_pm_ops nouveau_pm_ops = {
>>>>>> @@ -1088,6 +1094,18 @@ err_free:
>>>>>>  static int __init
>>>>>>  nouveau_drm_init(void)
>>>>>>  {
>>>>>> +       /* Do not register staging ioctsl if option not specified */
>>>>>> +       if (!nouveau_staging) {
>>>>>> +               unsigned i;
>>>>>> +
>>>>>> +               /* This keeps us safe is no staging ioctls are defined */
>>>>>> +               i = min(driver_stub.num_ioctls, DRM_NOUVEAU_STAGING_IOCTL);
>>>>>> +               while (!nouveau_ioctls[i - 1].func)
>>>>>> +                       i--;
>>>>>> +
>>>>>> +               driver_stub.num_ioctls = i;
>>>>>> +       }
>>>>> Hey Alex,
>>>>>
>>>>> I've got no specific objection.  But I'm curious as to why you took
>>>>> this approach as opposed to just adding "if (!nouveau_staging) return
>>>>> -EINVAL;" directly in the experimental ioctls?
>>>>
>>>> Mainly because we will likely forget to add this check (or to remove
>>>> it) in some of the staging ioctls. The current solution doesn't
>>>> require us to think about that - and the less things to think about,
>>>> the better.
>>>>
>>>>> I think, in line with
>>>>> what's been done in other places, having module options per-api is
>>>>> perhaps a better choice too.
>>>>
>>>> Do you mean that each experimental ioctl should have its own enable
>>>> option? I don't mind going that way if you think it is preferable. And
>>>> in that case my comment above is void.
>>> That would be more preferable I think, and obvious as to what exactly
>>> you're enabling.
>>>
>>>>
>>>> But actually I wonder if having these experimental ioctls enabled as
>>>> compile options (either individually or as a whole) would not be
>>>> better. Some experimental ioctls may require code in staging (like the
>>>> PUSHBUF_2 ioctl that I would like to submit next), and I don't think
>>>> it is desirable to force extra code or kernel options (in this case,
>>>> CONFIG_STAGING) to Nouveau users who will not make use of them. I
>>>> remember that we concluded in favor or module options on IRC, but in
>>>> the light of this, wouldn't a config option be a less intrusive
>>>> choice?
>>> Right, but the whole point of this is to encourage the ioctls to not
>>> live there for too long, and progress to fully supported interfaces.
>>
>> Definitely, but my concern is that doing this will make Nouveau depend
>> on STAGING for at least short periods of time. Do we really want this?
> I admit to having slightly misread your last paragraph.  For cases
> such as thas, a config option that depends on STAGING *and* the kernel
> parameter should be used.

Sounds good!

> What is pushbuf2 doing that requires staging btw?  You've linked me to
> patches previously, but I missed that.

It depends on the Android sync framework. Actually we also have a
patch that makes a change in the Android sync code that pushbuf2
depends on, and without pushbuf2 it does not make much sense by itself
(http://www.spinics.net/lists/linux-driver-devel/msg63832.html ), so I
will send the pushbuf2 patches along with it.
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau