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 X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F3157C43460 for ; Thu, 15 Apr 2021 00:58:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CD17E61220 for ; Thu, 15 Apr 2021 00:58:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229450AbhDOA6w (ORCPT ); Wed, 14 Apr 2021 20:58:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58624 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229436AbhDOA6w (ORCPT ); Wed, 14 Apr 2021 20:58:52 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 937B3C061574; Wed, 14 Apr 2021 17:58:29 -0700 (PDT) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 63B3E51E; Thu, 15 Apr 2021 02:58:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1618448306; bh=dT0dNdclklPiYApYQlXIEs77mjYEm26Y9+wMkOD0nvc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RxLAeseP/5Pdt5JDnRvWf/rt6evuAn133rWeH6zaj1mKSEUlv64P8LNG/QdY52rXj kAZ9ksbDJQbfOkAFg7j5b+4VSSoOPa3I2w9S5KrUpQy8BqA705FFPZTDiBtU/g82el Lknu/tWvcMyEHaLL+ZO6r8+4faZeq4oA+AMYXOlk= Date: Thu, 15 Apr 2021 03:58:25 +0300 From: Laurent Pinchart To: Douglas Anderson Cc: Andrzej Hajda , Neil Armstrong , Jonas Karlman , Jernej Skrabec , Sam Ravnborg , Linus W , Bjorn Andersson , robdclark@chromium.org, Stephen Boyd , Steev Klimaszewski , Maarten Lankhorst , linux-arm-msm@vger.kernel.org, Stanislav Lisovskiy , Daniel Vetter , David Airlie , Thierry Reding , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 12/12] drm/panel: panel-simple: Use runtime pm to avoid excessive unprepare / prepare Message-ID: References: <20210402222846.2461042-1-dianders@chromium.org> <20210402152701.v3.12.I9e8bd33b49c496745bfac58ea9ab418bd3b6f5ce@changeid> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210402152701.v3.12.I9e8bd33b49c496745bfac58ea9ab418bd3b6f5ce@changeid> Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Hi Doug, Thank you for the patch. On Fri, Apr 02, 2021 at 03:28:46PM -0700, Douglas Anderson wrote: > Unpreparing and re-preparing a panel can be a really heavy > operation. Panels datasheets often specify something on the order of > 500ms as the delay you should insert after turning off the panel > before turning it on again. In addition, turning on a panel can have > delays on the order of 100ms - 200ms before the panel will assert HPD > (AKA "panel ready"). The above means that we should avoid turning a > panel off if we're going to turn it on again shortly. > > The above becomes a problem when we want to read the EDID of a > panel. The way that ordering works is that userspace wants to read the > EDID of the panel _before_ fully enabling it so that it can set the > initial mode correctly. However, we can't read the EDID until we power > it up. This leads to code that does this dance (like > ps8640_bridge_get_edid()): > > 1. When userspace requests EDID / the panel modes (through an ioctl), > we power on the panel just enough to read the EDID and then power > it off. > 2. Userspace then turns the panel on. > > There's likely not much time between step #1 and #2 and so we want to > avoid powering the panel off and on again between those two steps. > > Let's use Runtime PM to help us. We'll move the existing prepare() and > unprepare() to be runtime resume() and runtime suspend(). Now when we > want to prepare() or unprepare() we just increment or decrement the > refcount. We'll default to a 1 second autosuspend delay which seems > sane given the typical delays we see for panels. > > A few notes: > - It seems the existing unprepare() and prepare() are defined to be > no-ops if called extra times. We'll preserve that behavior. The prepare and unprepare calls are supposed to be balanced, which should allow us to drop this check. Do you have a reason to suspect that it may not be the case ? > - This is a slight change in the ABI of simple panel. If something was > absolutely relying on the unprepare() to happen instantly that > simply won't be the case anymore. I'm not aware of anyone relying on > that behavior, but if there is someone then we'll need to figure out > how to enable (or disable) this new delayed behavior selectively. > - In order for this to work we now have a hard dependency on > "PM". From memory this is a legit thing to assume these days and we > don't have to find some fallback to keep working if someone wants to > build their system without "PM". Sounds fine to me. The code looks good to me. Possibly with the prepared check removed, Reviewed-by: Laurent Pinchart > Signed-off-by: Douglas Anderson > --- > > (no changes since v1) > > drivers/gpu/drm/panel/Kconfig | 1 + > drivers/gpu/drm/panel/panel-simple.c | 93 +++++++++++++++++++++------- > 2 files changed, 73 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index 4894913936e9..ef87d92cdf49 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -80,6 +80,7 @@ config DRM_PANEL_SIMPLE > tristate "support for simple panels" > depends on OF > depends on BACKLIGHT_CLASS_DEVICE > + depends on PM > select VIDEOMODE_HELPERS > help > DRM panel driver for dumb panels that need at most a regulator and > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index be312b5c04dd..6b22872b3281 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > #include > > #include