* [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag
@ 2019-05-22 15:46 Emil Velikov
2019-05-22 15:47 ` [PATCH 2/4] drm/virtio: " Emil Velikov
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Emil Velikov @ 2019-05-22 15:46 UTC (permalink / raw)
To: dri-devel; +Cc: linux-tegra, Thierry Reding, kernel
From: Emil Velikov <emil.velikov@collabora.com>
DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it.
Cc: Thierry Reding <treding@nvidia.com>
Cc: linux-tegra@vger.kernel.org
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
drivers/gpu/drm/tegra/drm.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 0c5f1e6a0446..8836c113b392 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -891,33 +891,33 @@ static int tegra_gem_get_flags(struct drm_device *drm, void *data,
static const struct drm_ioctl_desc tegra_drm_ioctls[] = {
#ifdef CONFIG_DRM_TEGRA_STAGING
DRM_IOCTL_DEF_DRV(TEGRA_GEM_CREATE, tegra_gem_create,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_GEM_MMAP, tegra_gem_mmap,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_READ, tegra_syncpt_read,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_INCR, tegra_syncpt_incr,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_SYNCPT_WAIT, tegra_syncpt_wait,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_OPEN_CHANNEL, tegra_open_channel,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_CLOSE_CHANNEL, tegra_close_channel,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT, tegra_get_syncpt,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_SUBMIT, tegra_submit,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_GET_SYNCPT_BASE, tegra_get_syncpt_base,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_TILING, tegra_gem_set_tiling,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_TILING, tegra_gem_get_tiling,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_GEM_SET_FLAGS, tegra_gem_set_flags,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(TEGRA_GEM_GET_FLAGS, tegra_gem_get_flags,
- DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_RENDER_ALLOW),
#endif
};
--
2.21.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] drm/virtio: remove irrelevant DRM_UNLOCKED flag
2019-05-22 15:46 [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Emil Velikov
@ 2019-05-22 15:47 ` Emil Velikov
2019-05-23 10:19 ` Gerd Hoffmann
2019-05-22 15:47 ` [PATCH 3/4] drm/i915: " Emil Velikov
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Emil Velikov @ 2019-05-22 15:47 UTC (permalink / raw)
To: dri-devel; +Cc: David Airlie, kernel, Daniel Vetter, virtualization
From: Emil Velikov <emil.velikov@collabora.com>
DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it.
Cc: David Airlie <airlied@linux.ie>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 949a264985fc..b7f9dfe61d1c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -553,34 +553,34 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev,
struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = {
DRM_IOCTL_DEF_DRV(VIRTGPU_MAP, virtio_gpu_map_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(VIRTGPU_EXECBUFFER, virtio_gpu_execbuffer_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(VIRTGPU_GETPARAM, virtio_gpu_getparam_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_CREATE,
virtio_gpu_resource_create_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_INFO, virtio_gpu_resource_info_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
/* make transfer async to the main ring? - no sure, can we
* thread these in the underlying GL
*/
DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_FROM_HOST,
virtio_gpu_transfer_from_host_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_TO_HOST,
virtio_gpu_transfer_to_host_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(VIRTGPU_WAIT, virtio_gpu_wait_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(VIRTGPU_GET_CAPS, virtio_gpu_get_caps_ioctl,
- DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+ DRM_AUTH | DRM_RENDER_ALLOW),
};
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] drm/i915: remove irrelevant DRM_UNLOCKED flag
2019-05-22 15:46 [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Emil Velikov
2019-05-22 15:47 ` [PATCH 2/4] drm/virtio: " Emil Velikov
@ 2019-05-22 15:47 ` Emil Velikov
2019-06-07 7:34 ` Daniel Vetter
2019-05-22 15:47 ` [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED Emil Velikov
2019-05-23 9:15 ` [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Thierry Reding
3 siblings, 1 reply; 11+ messages in thread
From: Emil Velikov @ 2019-05-22 15:47 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, kernel
From: Emil Velikov <emil.velikov@collabora.com>
DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it.
Cc: intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
drivers/gpu/drm/i915/i915_drv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ad88e6d7c04..a18c155cff88 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -3145,9 +3145,9 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
- DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_RENDER_ALLOW),
};
static struct drm_driver driver = {
--
2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915: remove irrelevant DRM_UNLOCKED flag
2019-05-22 15:47 ` [PATCH 3/4] drm/i915: " Emil Velikov
@ 2019-06-07 7:34 ` Daniel Vetter
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2019-06-07 7:34 UTC (permalink / raw)
To: Emil Velikov; +Cc: intel-gfx, kernel, dri-devel
On Wed, May 22, 2019 at 04:47:01PM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it.
>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ad88e6d7c04..a18c155cff88 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -3145,9 +3145,9 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_RENDER_ALLOW),
> };
>
> static struct drm_driver driver = {
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED
2019-05-22 15:46 [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Emil Velikov
2019-05-22 15:47 ` [PATCH 2/4] drm/virtio: " Emil Velikov
2019-05-22 15:47 ` [PATCH 3/4] drm/i915: " Emil Velikov
@ 2019-05-22 15:47 ` Emil Velikov
2019-05-22 16:12 ` Daniel Vetter
2019-05-23 9:15 ` [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Thierry Reding
3 siblings, 1 reply; 11+ messages in thread
From: Emil Velikov @ 2019-05-22 15:47 UTC (permalink / raw)
To: dri-devel; +Cc: kernel
From: Emil Velikov <emil.velikov@collabora.com>
Should minimise the copy/paste mistakes, fixed with previous patches.
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
Daniel, not 100% sold on the idea. That plus listing you as a contact
point ;-)
What do you thing?
Emil
---
Documentation/gpu/todo.rst | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 66f05f4e469f..9e67d125f2fd 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -397,6 +397,25 @@ Some of these date from the very introduction of KMS in 2008 ...
end, for which we could add drm_*_cleanup_kfree(). And then there's the (for
historical reasons) misnamed drm_primary_helper_destroy() function.
+Use DRM_LOCKED instead of DRM_UNLOCKED
+--------------------------------------
+
+DRM_UNLOCKED is a remainder from the legacy DRM drivers. Seemingly drivers get
+tricked by it and it ends up in the driver private ioctls.
+
+Today no more legacy drivers are allowed and most core DRM ioctls are unlocked.
+
+Introduce DRM_LOCKED, use it to annotate only the relevant ioctls and kill the
+old DRM_UNLOCKED.
+
+Patch series should be split as follows:
+ - Patch 1: drm: add the new DRM_LOCKED flag and honour it
+ - Patch 2: drm: convert core ioctls from DRM_UNLOCKED to DRM_LOCKED
+ - Patch 3-...: drm/driverX: convert driver ioctls from ...
+ - Patch X: drm: remove no longer used DRM_UNLOCKED, drop todo item
+
+Contact: Emil Velikov, Daniel Vetter
+
Better Testing
==============
--
2.21.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED
2019-05-22 15:47 ` [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED Emil Velikov
@ 2019-05-22 16:12 ` Daniel Vetter
2019-05-24 15:17 ` Emil Velikov
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2019-05-22 16:12 UTC (permalink / raw)
To: Emil Velikov; +Cc: kernel, dri-devel
On Wed, May 22, 2019 at 04:47:02PM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> Should minimise the copy/paste mistakes, fixed with previous patches.
>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Daniel, not 100% sold on the idea. That plus listing you as a contact
> point ;-)
>
> What do you thing?
> Emil
> ---
> Documentation/gpu/todo.rst | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 66f05f4e469f..9e67d125f2fd 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -397,6 +397,25 @@ Some of these date from the very introduction of KMS in 2008 ...
> end, for which we could add drm_*_cleanup_kfree(). And then there's the (for
> historical reasons) misnamed drm_primary_helper_destroy() function.
>
> +Use DRM_LOCKED instead of DRM_UNLOCKED
> +--------------------------------------
> +
> +DRM_UNLOCKED is a remainder from the legacy DRM drivers. Seemingly drivers get
> +tricked by it and it ends up in the driver private ioctls.
> +
> +Today no more legacy drivers are allowed and most core DRM ioctls are unlocked.
> +
> +Introduce DRM_LOCKED, use it to annotate only the relevant ioctls and kill the
> +old DRM_UNLOCKED.
> +
> +Patch series should be split as follows:
> + - Patch 1: drm: add the new DRM_LOCKED flag and honour it
> + - Patch 2: drm: convert core ioctls from DRM_UNLOCKED to DRM_LOCKED
> + - Patch 3-...: drm/driverX: convert driver ioctls from ...
> + - Patch X: drm: remove no longer used DRM_UNLOCKED, drop todo item
Seems like too much bother for legacy drivers ... What I'd do is something
a lot cheaper, since all we're touching here are legacy drivers:
Remove DRM_UNLOCKED from everything except the old vblank ioctl. That one
we need to keep, because it freezes X:
commit 8f4ff2b06afcd6f151868474a432c603057eaf56
Author: Ilija Hadzic <ihadzic@research.bell-labs.com>
Date: Mon Oct 31 17:46:18 2011 -0400
drm: do not sleep on vblank while holding a mutex
Anything else I think is either never used by legacy userspace, or just
doesn't matter. That's simple enough that I don't think we really need a
todo for it :-) I guess if you want to kill DRM_UNLOCKED you could replace
the check with ioctl->func == drm_vblank_ioctl, ofc only in the
DRIVER_LEGACY path.
On patches 1-3 in your series:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cheers, Daniel
> +
> +Contact: Emil Velikov, Daniel Vetter
> +
> Better Testing
> ==============
>
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED
2019-05-22 16:12 ` Daniel Vetter
@ 2019-05-24 15:17 ` Emil Velikov
2019-05-24 15:59 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Emil Velikov @ 2019-05-24 15:17 UTC (permalink / raw)
To: Daniel Vetter; +Cc: kernel, dri-devel
On 2019/05/22, Daniel Vetter wrote:
> On Wed, May 22, 2019 at 04:47:02PM +0100, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > Should minimise the copy/paste mistakes, fixed with previous patches.
> >
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > ---
> > Daniel, not 100% sold on the idea. That plus listing you as a contact
> > point ;-)
> >
> > What do you thing?
> > Emil
> > ---
> > Documentation/gpu/todo.rst | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > index 66f05f4e469f..9e67d125f2fd 100644
> > --- a/Documentation/gpu/todo.rst
> > +++ b/Documentation/gpu/todo.rst
> > @@ -397,6 +397,25 @@ Some of these date from the very introduction of KMS in 2008 ...
> > end, for which we could add drm_*_cleanup_kfree(). And then there's the (for
> > historical reasons) misnamed drm_primary_helper_destroy() function.
> >
> > +Use DRM_LOCKED instead of DRM_UNLOCKED
> > +--------------------------------------
> > +
> > +DRM_UNLOCKED is a remainder from the legacy DRM drivers. Seemingly drivers get
> > +tricked by it and it ends up in the driver private ioctls.
> > +
> > +Today no more legacy drivers are allowed and most core DRM ioctls are unlocked.
> > +
> > +Introduce DRM_LOCKED, use it to annotate only the relevant ioctls and kill the
> > +old DRM_UNLOCKED.
> > +
> > +Patch series should be split as follows:
> > + - Patch 1: drm: add the new DRM_LOCKED flag and honour it
> > + - Patch 2: drm: convert core ioctls from DRM_UNLOCKED to DRM_LOCKED
> > + - Patch 3-...: drm/driverX: convert driver ioctls from ...
> > + - Patch X: drm: remove no longer used DRM_UNLOCKED, drop todo item
>
> Seems like too much bother for legacy drivers ... What I'd do is something
> a lot cheaper, since all we're touching here are legacy drivers:
>
> Remove DRM_UNLOCKED from everything except the old vblank ioctl. That one
> we need to keep, because it freezes X:
>
> commit 8f4ff2b06afcd6f151868474a432c603057eaf56
> Author: Ilija Hadzic <ihadzic@research.bell-labs.com>
> Date: Mon Oct 31 17:46:18 2011 -0400
>
> drm: do not sleep on vblank while holding a mutex
>
> Anything else I think is either never used by legacy userspace, or just
> doesn't matter. That's simple enough that I don't think we really need a
> todo for it :-) I guess if you want to kill DRM_UNLOCKED you could replace
> the check with ioctl->func == drm_vblank_ioctl, ofc only in the
> DRIVER_LEGACY path.
>
Sounds like a much simpler solution indeed. Sadly I don't have much time to
double-check that this won't cause problems elsewhere, so I'll leave it that
to someone else.
> On patches 1-3 in your series:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
Thank you very much.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED
2019-05-24 15:17 ` Emil Velikov
@ 2019-05-24 15:59 ` Daniel Vetter
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2019-05-24 15:59 UTC (permalink / raw)
To: Emil Velikov; +Cc: kernel, dri-devel
On Fri, May 24, 2019 at 04:17:16PM +0100, Emil Velikov wrote:
> On 2019/05/22, Daniel Vetter wrote:
> > On Wed, May 22, 2019 at 04:47:02PM +0100, Emil Velikov wrote:
> > > From: Emil Velikov <emil.velikov@collabora.com>
> > >
> > > Should minimise the copy/paste mistakes, fixed with previous patches.
> > >
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > > ---
> > > Daniel, not 100% sold on the idea. That plus listing you as a contact
> > > point ;-)
> > >
> > > What do you thing?
> > > Emil
> > > ---
> > > Documentation/gpu/todo.rst | 19 +++++++++++++++++++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > > index 66f05f4e469f..9e67d125f2fd 100644
> > > --- a/Documentation/gpu/todo.rst
> > > +++ b/Documentation/gpu/todo.rst
> > > @@ -397,6 +397,25 @@ Some of these date from the very introduction of KMS in 2008 ...
> > > end, for which we could add drm_*_cleanup_kfree(). And then there's the (for
> > > historical reasons) misnamed drm_primary_helper_destroy() function.
> > >
> > > +Use DRM_LOCKED instead of DRM_UNLOCKED
> > > +--------------------------------------
> > > +
> > > +DRM_UNLOCKED is a remainder from the legacy DRM drivers. Seemingly drivers get
> > > +tricked by it and it ends up in the driver private ioctls.
> > > +
> > > +Today no more legacy drivers are allowed and most core DRM ioctls are unlocked.
> > > +
> > > +Introduce DRM_LOCKED, use it to annotate only the relevant ioctls and kill the
> > > +old DRM_UNLOCKED.
> > > +
> > > +Patch series should be split as follows:
> > > + - Patch 1: drm: add the new DRM_LOCKED flag and honour it
> > > + - Patch 2: drm: convert core ioctls from DRM_UNLOCKED to DRM_LOCKED
> > > + - Patch 3-...: drm/driverX: convert driver ioctls from ...
> > > + - Patch X: drm: remove no longer used DRM_UNLOCKED, drop todo item
> >
> > Seems like too much bother for legacy drivers ... What I'd do is something
> > a lot cheaper, since all we're touching here are legacy drivers:
> >
> > Remove DRM_UNLOCKED from everything except the old vblank ioctl. That one
> > we need to keep, because it freezes X:
> >
> > commit 8f4ff2b06afcd6f151868474a432c603057eaf56
> > Author: Ilija Hadzic <ihadzic@research.bell-labs.com>
> > Date: Mon Oct 31 17:46:18 2011 -0400
> >
> > drm: do not sleep on vblank while holding a mutex
> >
> > Anything else I think is either never used by legacy userspace, or just
> > doesn't matter. That's simple enough that I don't think we really need a
> > todo for it :-) I guess if you want to kill DRM_UNLOCKED you could replace
> > the check with ioctl->func == drm_vblank_ioctl, ofc only in the
> > DRIVER_LEGACY path.
> >
> Sounds like a much simpler solution indeed. Sadly I don't have much time to
> double-check that this won't cause problems elsewhere, so I'll leave it that
> to someone else.
I did dig through enough history that I'm confident. I'll type a patch and
some awkward-long commit message :-)
-Daniel
>
> > On patches 1-3 in your series:
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> Thank you very much.
>
> -Emil
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag
2019-05-22 15:46 [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Emil Velikov
` (2 preceding siblings ...)
2019-05-22 15:47 ` [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED Emil Velikov
@ 2019-05-23 9:15 ` Thierry Reding
2019-05-24 15:19 ` Emil Velikov
3 siblings, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2019-05-23 9:15 UTC (permalink / raw)
To: Emil Velikov; +Cc: linux-tegra, kernel, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]
On Wed, May 22, 2019 at 04:46:59PM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it.
>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: linux-tegra@vger.kernel.org
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> drivers/gpu/drm/tegra/drm.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
I assume you want to take this through drm-misc? In that case:
Acked-by: Thierry Reding <treding@nvidia.com>
Otherwise let me know and I'll pick it up into the Tegra tree.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag
2019-05-23 9:15 ` [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Thierry Reding
@ 2019-05-24 15:19 ` Emil Velikov
0 siblings, 0 replies; 11+ messages in thread
From: Emil Velikov @ 2019-05-24 15:19 UTC (permalink / raw)
To: Thierry Reding; +Cc: linux-tegra, kernel, dri-devel
On 2019/05/23, Thierry Reding wrote:
> On Wed, May 22, 2019 at 04:46:59PM +0100, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it.
> >
> > Cc: Thierry Reding <treding@nvidia.com>
> > Cc: linux-tegra@vger.kernel.org
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > ---
> > drivers/gpu/drm/tegra/drm.c | 28 ++++++++++++++--------------
> > 1 file changed, 14 insertions(+), 14 deletions(-)
>
> I assume you want to take this through drm-misc? In that case:
>
> Acked-by: Thierry Reding <treding@nvidia.com>
>
> Otherwise let me know and I'll pick it up into the Tegra tree.
>
Yes, I'll pick it up through drm-misc.
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-07 7:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22 15:46 [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Emil Velikov
2019-05-22 15:47 ` [PATCH 2/4] drm/virtio: " Emil Velikov
2019-05-23 10:19 ` Gerd Hoffmann
2019-05-22 15:47 ` [PATCH 3/4] drm/i915: " Emil Velikov
2019-06-07 7:34 ` Daniel Vetter
2019-05-22 15:47 ` [PATCH 4/4] drm/TODO: add a task to kill DRM_UNLOCKED Emil Velikov
2019-05-22 16:12 ` Daniel Vetter
2019-05-24 15:17 ` Emil Velikov
2019-05-24 15:59 ` Daniel Vetter
2019-05-23 9:15 ` [PATCH 1/4] drm/tegra: remove irrelevant DRM_UNLOCKED flag Thierry Reding
2019-05-24 15:19 ` Emil Velikov
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.