| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b8fda72a-1ec6-4d47-ea5a-22f03802f4f3@linux.intel.com>
Date: Thu, 28 Jun 2018 10:31:17 +0200
From: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
To: Enric Balletbo i Serra <enric.balletbo@...labora.com>,
David Airlie <airlied@...ux.ie>
Cc: dnicoara@...omium.org, alexandros.frantzis@...labora.com,
linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
Gustavo Padovan <gustavo.padovan@...labora.com>,
tomasz Figa <tfiga@...omium.org>,
Sean Paul <seanpaul@...gle.com>, kernel@...labora.com,
Stéphane Marchesin <marcheu@...gle.com>,
Gustavo Padovan <gustavo@...ovan.org>,
Sean Paul <seanpaul@...omium.org>
Subject: Re: [RFC PATCH] drm/atomic: add ASYNC_UPDATE flag to the Atomic
IOCTL.
Op 27-06-18 om 23:25 schreef Enric Balletbo i Serra:
> From: Gustavo Padovan <gustavo.padovan@...labora.com>
>
> This flag tells core to jump ahead the queued update if the conditions
> in drm_atomic_async_check() are met. That means we are only able to do an
> async update if no modeset is pending and update for the same plane is
> not queued.
>
> It uses the already in place infrastructure for async updates.
>
> It is useful for cursor updates and async PageFlips over the atomic
> ioctl, otherwise in some cases updates may be delayed to the point the
> user will notice it.
>
> DRM_MODE_ATOMIC_ASYNC_UPDATE should be passed to the Atomic IOCTL to use
> this feature.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@...labora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>
> ---
> Hi,
>
> This is an attempt to introduce the new ASYNC_UPDATE flag for atomic
> operations, see the commit message for a more detailed description.
>
> To test this patch we have created an IGT test that we plan to send to
> the ML but also was tested using a small program that exercises the uAPI
> for easy sanity testing. The program created by Alexandros can be found here
> [2]. To test, just build the program and use the --atomic flag to use the
> cursor plane in normal (blocking mode), and --atomic-async to use the cursor
> plane with the ASYNC_UPDATE flag.E.g.
>
> drm_cursor --atomic
>
> or
>
> drm_cursor --atomic-async
>
> The test worked on a Samsung Chromebook Plus on top of mainline plus
> the patch to update cursors asynchronously through atomic for the
> drm/rockchip driver [3].
>
> Alexandros also did a proof-of-concept to use this flag and draw cursors
> using atomic if possible on ozone [1].
>
> Best regards,
> Enric
>
> [1] https://chromium-review.googlesource.com/c/chromium/src/+/1092711
> [2] https://gitlab.collabora.com/alf/drm-cursor
> [3] https://patchwork.kernel.org/patch/10492693/
>
>
> drivers/gpu/drm/drm_atomic.c | 6 ++++++
> drivers/gpu/drm/drm_atomic_helper.c | 9 ++++++---
> include/uapi/drm/drm_mode.h | 4 +++-
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c825c76edc1d..15b799f46982 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -80,6 +80,7 @@ drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
> * setting this appropriately?
> */
> state->allow_modeset = true;
> + state->async_update = true;
Do we really want this by default?
Wouldn't it make more sense to only enable it if userspace requests it?
>
> state->crtcs = kcalloc(dev->mode_config.num_crtc,
> sizeof(*state->crtcs), GFP_KERNEL);
> @@ -2320,6 +2321,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> return -EINVAL;
>
> + if ((arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET) &&
> + (arg->flags & DRM_MODE_ATOMIC_ASYNC_UPDATE))
> + return -EINVAL;
> +
> drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
>
> state = drm_atomic_state_alloc(dev);
We should probably move it to be a flag only enabled when requested, and reject with the error returned by drm_atomic_helper_async_check() when things fail.
~Maarten
> @@ -2328,6 +2333,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>
> state->acquire_ctx = &ctx;
> state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
> + state->async_update = !!(arg->flags & DRM_MODE_ATOMIC_ASYNC_UPDATE);
>
> retry:
> plane_mask = 0;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index c35654591c12..aeb0523d3bcf 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -891,7 +891,7 @@ int drm_atomic_helper_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> - if (state->legacy_cursor_update)
> + if (state->async_update || state->legacy_cursor_update)
> state->async_update = !drm_atomic_helper_async_check(dev, state);
>
> return ret;
> @@ -1526,13 +1526,16 @@ int drm_atomic_helper_async_check(struct drm_device *dev,
> if (new_plane_state->fence)
> return -EINVAL;
>
> + /* Only do an async update if there is a pending commit. */
> + if (!old_plane_state->commit)
> + return -EINVAL;
> +
> /*
> * Don't do an async update if there is an outstanding commit modifying
> * the plane. This prevents our async update's changes from getting
> * overridden by a previous synchronous update's state.
> */
> - if (old_plane_state->commit &&
> - !try_wait_for_completion(&old_plane_state->commit->hw_done))
> + if (!try_wait_for_completion(&old_plane_state->commit->hw_done))
> return -EBUSY;
>
> return funcs->atomic_async_check(plane, new_plane_state);
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50bcf4214ff9..772e84f0edeb 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -718,13 +718,15 @@ struct drm_mode_destroy_dumb {
> #define DRM_MODE_ATOMIC_TEST_ONLY 0x0100
> #define DRM_MODE_ATOMIC_NONBLOCK 0x0200
> #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400
> +#define DRM_MODE_ATOMIC_ASYNC_UPDATE 0x0800
>
> #define DRM_MODE_ATOMIC_FLAGS (\
> DRM_MODE_PAGE_FLIP_EVENT |\
> DRM_MODE_PAGE_FLIP_ASYNC |\
> DRM_MODE_ATOMIC_TEST_ONLY |\
> DRM_MODE_ATOMIC_NONBLOCK |\
> - DRM_MODE_ATOMIC_ALLOW_MODESET)
> + DRM_MODE_ATOMIC_ALLOW_MODESET |\
> + DRM_MODE_ATOMIC_ASYNC_UPDATE)
>
> struct drm_mode_atomic {
> __u32 flags;
Powered by blists - more mailing lists