| 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: <CAKMK7uHGNrgqjQh3DX4gChpNt+xhB_39sVrhdA3BFqnoW-ue2w@mail.gmail.com>
Date: Wed, 17 Nov 2021 20:12:15 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: Brian Norris <briannorris@...omium.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
David Airlie <airlied@...ux.ie>,
linux-rockchip@...ts.infradead.org,
"Kristian H . Kristensen" <hoegsberg@...gle.com>,
Doug Anderson <dianders@...omium.org>,
Rob Clark <robdclark@...omium.org>,
Rob Clark <robdclark@...il.com>
Subject: Re: [PATCH 2/2] drm/self_refresh: Disable self-refresh on input events
On Thu, Nov 4, 2021 at 12:40 AM Brian Norris <briannorris@...omium.org> wrote:
>
> To improve panel self-refresh exit latency, we speculatively start
> exiting when we
> receive input events. Occasionally, this may lead to false positives,
> but most of the time we get a head start on coming out of PSR. Depending
> on how userspace takes to produce a new frame in response to the event,
> this can completely hide the exit latency.
>
> In local tests on Chrome OS (Rockchip RK3399 eDP), we've found that the
> input notifier gives us about a 50ms head start over the
> fb-update-initiated exit.
>
> Leverage a new drm_input_helper library to get easy access to
> likely-relevant input event callbacks.
>
> Inspired-by: Kristian H. Kristensen <hoegsberg@...gle.com>
> Signed-off-by: Brian Norris <briannorris@...omium.org>
Can you pls resend with dri-devel on cc? scripts/get_maintainers.pl
should pick this up, you have all the maintainers but not the list.
-Daniel
> ---
> This was in part picked up from:
>
> https://lore.kernel.org/all/20180405095000.9756-25-enric.balletbo@collabora.com/
> [PATCH v6 24/30] drm/rockchip: Disable PSR on input events
>
> with significant rewrites/reworks:
>
> - moved to common drm_input_helper and drm_self_refresh_helper
> implementation
> - track state only through crtc->state->self_refresh_active
>
> Note that I'm relatively unfamiliar with DRM locking expectations, but I
> believe access to drm_crtc->state (which helps us track redundant
> transitions) is OK under the locking provided by
> drm_atomic_get_crtc_state().
>
> drivers/gpu/drm/drm_self_refresh_helper.c | 54 ++++++++++++++++++++---
> 1 file changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c b/drivers/gpu/drm/drm_self_refresh_helper.c
> index dd33fec5aabd..dcab061cc90a 100644
> --- a/drivers/gpu/drm/drm_self_refresh_helper.c
> +++ b/drivers/gpu/drm/drm_self_refresh_helper.c
> @@ -15,6 +15,7 @@
> #include <drm/drm_connector.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_device.h>
> +#include <drm/drm_input_helper.h>
> #include <drm/drm_mode_config.h>
> #include <drm/drm_modeset_lock.h>
> #include <drm/drm_print.h>
> @@ -58,17 +59,17 @@ DECLARE_EWMA(psr_time, 4, 4)
> struct drm_self_refresh_data {
> struct drm_crtc *crtc;
> struct delayed_work entry_work;
> + struct work_struct exit_work;
> + struct drm_input_handler input_handler;
>
> struct mutex avg_mutex;
> struct ewma_psr_time entry_avg_ms;
> struct ewma_psr_time exit_avg_ms;
> };
>
> -static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> +static void drm_self_refresh_transition(struct drm_self_refresh_data *sr_data,
> + bool enable)
> {
> - struct drm_self_refresh_data *sr_data = container_of(
> - to_delayed_work(work),
> - struct drm_self_refresh_data, entry_work);
> struct drm_crtc *crtc = sr_data->crtc;
> struct drm_device *dev = crtc->dev;
> struct drm_modeset_acquire_ctx ctx;
> @@ -95,6 +96,9 @@ static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> goto out;
> }
>
> + if (crtc->state->self_refresh_active == enable)
> + goto out;
> +
> if (!crtc_state->enable)
> goto out;
>
> @@ -107,8 +111,8 @@ static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> goto out;
> }
>
> - crtc_state->active = false;
> - crtc_state->self_refresh_active = true;
> + crtc_state->active = !enable;
> + crtc_state->self_refresh_active = enable;
>
> ret = drm_atomic_commit(state);
> if (ret)
> @@ -129,6 +133,15 @@ static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> drm_modeset_acquire_fini(&ctx);
> }
>
> +static void drm_self_refresh_helper_entry_work(struct work_struct *work)
> +{
> + struct drm_self_refresh_data *sr_data = container_of(
> + to_delayed_work(work),
> + struct drm_self_refresh_data, entry_work);
> +
> + drm_self_refresh_transition(sr_data, true);
> +}
> +
> /**
> * drm_self_refresh_helper_update_avg_times - Updates a crtc's SR time averages
> * @state: the state which has just been applied to hardware
> @@ -223,6 +236,20 @@ void drm_self_refresh_helper_alter_state(struct drm_atomic_state *state)
> }
> EXPORT_SYMBOL(drm_self_refresh_helper_alter_state);
>
> +static void drm_self_refresh_helper_exit_work(struct work_struct *work)
> +{
> + struct drm_self_refresh_data *sr_data = container_of(
> + work, struct drm_self_refresh_data, exit_work);
> +
> + drm_self_refresh_transition(sr_data, false);
> +}
> +
> +static void drm_self_refresh_input_event(void *data)
> +{
> + struct drm_self_refresh_data *sr_data = data;
> +
> + schedule_work(&sr_data->exit_work);
> +}
> /**
> * drm_self_refresh_helper_init - Initializes self refresh helpers for a crtc
> * @crtc: the crtc which supports self refresh supported displays
> @@ -232,6 +259,7 @@ EXPORT_SYMBOL(drm_self_refresh_helper_alter_state);
> int drm_self_refresh_helper_init(struct drm_crtc *crtc)
> {
> struct drm_self_refresh_data *sr_data = crtc->self_refresh_data;
> + int ret;
>
> /* Helper is already initialized */
> if (WARN_ON(sr_data))
> @@ -243,6 +271,7 @@ int drm_self_refresh_helper_init(struct drm_crtc *crtc)
>
> INIT_DELAYED_WORK(&sr_data->entry_work,
> drm_self_refresh_helper_entry_work);
> + INIT_WORK(&sr_data->exit_work, drm_self_refresh_helper_exit_work);
> sr_data->crtc = crtc;
> mutex_init(&sr_data->avg_mutex);
> ewma_psr_time_init(&sr_data->entry_avg_ms);
> @@ -256,8 +285,19 @@ int drm_self_refresh_helper_init(struct drm_crtc *crtc)
> ewma_psr_time_add(&sr_data->entry_avg_ms, SELF_REFRESH_AVG_SEED_MS);
> ewma_psr_time_add(&sr_data->exit_avg_ms, SELF_REFRESH_AVG_SEED_MS);
>
> + sr_data->input_handler.callback = drm_self_refresh_input_event;
> + sr_data->input_handler.priv = sr_data;
> + ret = drm_input_handle_register(crtc->dev, &sr_data->input_handler);
> + if (ret)
> + goto err;
> +
> crtc->self_refresh_data = sr_data;
> +
> return 0;
> +
> +err:
> + kfree(sr_data);
> + return ret;
> }
> EXPORT_SYMBOL(drm_self_refresh_helper_init);
>
> @@ -275,7 +315,9 @@ void drm_self_refresh_helper_cleanup(struct drm_crtc *crtc)
>
> crtc->self_refresh_data = NULL;
>
> + drm_input_handle_unregister(&sr_data->input_handler);
> cancel_delayed_work_sync(&sr_data->entry_work);
> + cancel_work_sync(&sr_data->exit_work);
> kfree(sr_data);
> }
> EXPORT_SYMBOL(drm_self_refresh_helper_cleanup);
> --
> 2.34.0.rc0.344.g81b53c2807-goog
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists