From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF24AC433EF for ; Sat, 13 Nov 2021 00:52:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CCEDC608FE for ; Sat, 13 Nov 2021 00:52:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232571AbhKMAzM (ORCPT ); Fri, 12 Nov 2021 19:55:12 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40942 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230300AbhKMAzL (ORCPT ); Fri, 12 Nov 2021 19:55:11 -0500 Received: from mail-io1-xd34.google.com (mail-io1-xd34.google.com [IPv6:2607:f8b0:4864:20::d34]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EFE2FC061766 for ; Fri, 12 Nov 2021 16:52:19 -0800 (PST) Received: by mail-io1-xd34.google.com with SMTP id m9so13453287iop.0 for ; Fri, 12 Nov 2021 16:52:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Ar+96otnAzeN5XyQtZwRbRvg/01cns8FSyH4rDQT4T4=; b=EGyRVXShdBOHi4a5JP4tWIcCl0qnPG5C3nLBIQVAP3oPkc2D14VyhTCDf7WNGPpRWr mqinLcayYYsxIC9Uqr88/5qSk85g0nMYmUhOLIr1YMBQRgvwgY3WTPCSU3lMRDcrnE+d I6OXzQ9Pqmk6L5tz0hqrgUcLf1+d8cQjHVzuU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Ar+96otnAzeN5XyQtZwRbRvg/01cns8FSyH4rDQT4T4=; b=Sw4PTe+vb6tvKS81A8g0yIsDt0SkN5aMMcpVnYLNLl5gcdaNmXcvohiqLHEq3gW7LD QgsOvGcym4BNLb/HsZIFpz6zjK/DyiWi+ihkw6mOh38QNJUvVU1YDdaBd7SPtgHFaz/t grwesRXNV1QnBeCSlo9IYTPqd8CYgyepqbv6LDGt94COJ548cumhVBU7S0lSAC6w3Eaj tgM6n32w17W8KtH4Zo1VM3NOm792Z4ozn+MuQizM34uViKGb0SQaFJw/mZ1rh3H6e4Lt nBABlQbPEstc+RnExGrEhuDHSkiX0FzOCPkg0sLPjaC4VlntqtXcwakiStOrR5ZaFdI9 FbVA== X-Gm-Message-State: AOAM533KjTi0M0VS43/NJcCDaL76iCHJl5Y/3jNKqz6aYACXwSw+cIpF VclU4kVO/q1Ya0aNeZs55m0I8Q1WwtHe0A== X-Google-Smtp-Source: ABdhPJxbP55T7iR44wq4CK2sdaT2ufMzvNjJU6ePes/q3VsSpJ1LjGmu0t2YNsPaSBhobWLj5i5N4A== X-Received: by 2002:a5d:9ed6:: with SMTP id a22mr13404685ioe.167.1636764738168; Fri, 12 Nov 2021 16:52:18 -0800 (PST) Received: from mail-io1-f53.google.com (mail-io1-f53.google.com. [209.85.166.53]) by smtp.gmail.com with ESMTPSA id h14sm4565009ils.75.2021.11.12.16.52.15 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 12 Nov 2021 16:52:17 -0800 (PST) Received: by mail-io1-f53.google.com with SMTP id x10so13368643ioj.9 for ; Fri, 12 Nov 2021 16:52:15 -0800 (PST) X-Received: by 2002:a5d:9753:: with SMTP id c19mr13234220ioo.136.1636764735260; Fri, 12 Nov 2021 16:52:15 -0800 (PST) MIME-Version: 1.0 References: <20211103234018.4009771-1-briannorris@chromium.org> <20211103164002.2.Ie6c485320b35b89fd49e15a73f0a68e3bb49eef9@changeid> In-Reply-To: <20211103164002.2.Ie6c485320b35b89fd49e15a73f0a68e3bb49eef9@changeid> From: Doug Anderson Date: Fri, 12 Nov 2021 16:52:03 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/2] drm/self_refresh: Disable self-refresh on input events To: Brian Norris Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Andrzej Hajda , Dmitry Torokhov , linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, David Airlie , linux-rockchip@lists.infradead.org, "Kristian H . Kristensen" , Rob Clark , Rob Clark , Daniel Vetter Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wed, Nov 3, 2021 at 4:40 PM Brian Norris 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. So IMO this is a really useful thing and I'm in support of it landing. It's not much code and it clearly gives a big benefit. However, I would request a CONFIG option to control this so that if someone really finds some use case where it isn't needed or if they find a good way to do this in userspace without latency problems then they can turn it off. Does that sound reasonable? > Inspired-by: Kristian H. Kristensen > Signed-off-by: Brian Norris > --- > 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(). Yeah, I'm no expert here either. I gave a review a shot anyway since it's been all quiet, but adult supervision is probably required... I can believe that you are safe from corrupting things, but I think you still have locking problems, don't you? What about this: 1. PSR is _not_ active but we're 1 microsecond away from entering PSR 2. Input event comes through. 3. Start executing drm_self_refresh_transition(false). 4. PSR timer expires and starts executing drm_self_refresh_transition(true). 5. Input event "wins the race" but sees that PSR is already disabled => noop 6. PSR timer gets the lock now. Starts PSR transition. Wouldn't it be better to cancel / reschedule any PSR entry as soon as you see the input event? -Doug