From: Sean Paul <sean@poorly.run>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>,
David Airlie <airlied@linux.ie>,
Linus Walleij <linus.walleij@linaro.org>,
dri-devel@lists.freedesktop.org,
Andrzej Hajda <a.hajda@samsung.com>,
Thierry Reding <thierry.reding@gmail.com>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
Fabio Estevam <festevam@gmail.com>, Marek Vasut <marex@denx.de>,
Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
Joonyoung Shim <jy0922.shim@samsung.com>,
Vincent Abriou <vincent.abriou@st.com>,
Krzysztof Kozlowski <krzk@kernel.org>,
Jonathan Hunter <jonathanh@nvidia.com>,
Maxime Ripard <maxime.ripard@bootlin.com>,
Kukjin Kim <kgene@kernel.org>,
linux-arm-kernel@lists.infradead.org,
Philipp Zabel <p.zabel@pengutronix.de>,
NXP Linux Team <linux-imx@nxp.com>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
Jonas Karlman <jonas@kwiboo.se>,
Sascha Hauer <s.hauer@pengutronix.de>,
Alison Wang <alison.wang@nxp.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>,
Inki Dae <inki.dae@samsung.com>,
Alexios Zavras <alexios.zavras@intel.com>,
linux-samsung-soc@vger.kernel.org, Stefan Agner <stefan@agner.ch>,
linux-tegra@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
Sean Paul <sean@poorly.run>, Allison Randal <allison@lohutok.net>,
Jernej Skrabec <jernej.skrabec@siol.net>,
Shawn Guo <shawnguo@kernel.org>,
Seung-Woo Kim <sw0312.kim@samsung.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Daniel Vetter <daniel@ffwll.ch>, Enrico Weigelt <info@metux.net>
Subject: Re: [PATCH v1 14/16] drm/panel: call prepare/enable only once
Date: Mon, 5 Aug 2019 13:01:20 -0400 [thread overview]
Message-ID: <20190805170120.GZ104440@art_vandelay> (raw)
In-Reply-To: <20190804201637.1240-15-sam@ravnborg.org>
On Sun, Aug 04, 2019 at 10:16:35PM +0200, Sam Ravnborg wrote:
> Many panel drivers duplicate logic to prevent prepare to be called
> for a panel that is already prepared.
> Likewise for enable.
>
> Implement this logic in drm_panel so the individual drivers
> no longer needs this.
> A panel is considered prepared/enabled only if the prepare/enable call
> succeeds.
> For disable/unprepare it is unconditionally considered
> disabled/unprepared.
Hi Sam,
I did a similar thing a few years ago [1]. IIRC it was well received and just
needed some nits cleaned up. Unfortunately I lost interest^W^W switched to other
things and dropped it. So thanks for picking this back up!
Fast forward to today, I still think it's a good idea but I want to make sure
this won't negatively interact with the self refresh helpers. With the helpers
in place, it's possible to call disable consecutively (ie: once to enter self
refresh and again to actually shut down). I did a quick pass and it looks like
this patch might break that behavior, so you might want to take that into
account.
Sean
[1]- https://patchwork.freedesktop.org/series/30712/
>
> This allows calls to prepare/enable again, even if there were
> some issue disabling a regulator or similar during disable/unprepare.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
> drivers/gpu/drm/drm_panel.c | 66 ++++++++++++++++++++++++++++++-------
> include/drm/drm_panel.h | 21 ++++++++++++
> 2 files changed, 75 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index da19d5b4a2f4..0853764040de 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -66,10 +66,21 @@ EXPORT_SYMBOL(drm_panel_init);
> */
> int drm_panel_prepare(struct drm_panel *panel)
> {
> - if (panel && panel->funcs && panel->funcs->prepare)
> - return panel->funcs->prepare(panel);
> + int ret = -ENOSYS;
>
> - return panel ? -ENOSYS : -EINVAL;
> + if (!panel)
> + return -EINVAL;
> +
> + if (panel->prepared)
> + return 0;
> +
> + if (panel->funcs && panel->funcs->prepare)
> + ret = panel->funcs->prepare(panel);
> +
> + if (ret >= 0)
> + panel->prepared = true;
> +
> + return ret;
> }
> EXPORT_SYMBOL(drm_panel_prepare);
>
> @@ -85,10 +96,21 @@ EXPORT_SYMBOL(drm_panel_prepare);
> */
> int drm_panel_enable(struct drm_panel *panel)
> {
> - if (panel && panel->funcs && panel->funcs->enable)
> - return panel->funcs->enable(panel);
> + int ret = -ENOSYS;
>
> - return panel ? -ENOSYS : -EINVAL;
> + if (!panel)
> + return -EINVAL;
> +
> + if (panel->enabled)
> + return 0;
> +
> + if (panel->funcs && panel->funcs->enable)
> + ret = panel->funcs->enable(panel);
> +
> + if (ret >= 0)
> + panel->enabled = true;
> +
> + return ret;
> }
> EXPORT_SYMBOL(drm_panel_enable);
>
> @@ -104,10 +126,20 @@ EXPORT_SYMBOL(drm_panel_enable);
> */
> int drm_panel_disable(struct drm_panel *panel)
> {
> - if (panel && panel->funcs && panel->funcs->disable)
> - return panel->funcs->disable(panel);
> + int ret = -ENOSYS;
>
> - return panel ? -ENOSYS : -EINVAL;
> + if (!panel)
> + return -EINVAL;
> +
> + if (!panel->enabled)
> + return 0;
> +
> + if (panel->funcs && panel->funcs->disable)
> + ret = panel->funcs->disable(panel);
> +
> + panel->enabled = false;
> +
> + return ret;
> }
> EXPORT_SYMBOL(drm_panel_disable);
>
> @@ -124,10 +156,20 @@ EXPORT_SYMBOL(drm_panel_disable);
> */
> int drm_panel_unprepare(struct drm_panel *panel)
> {
> - if (panel && panel->funcs && panel->funcs->unprepare)
> - return panel->funcs->unprepare(panel);
> + int ret = -ENOSYS;
>
> - return panel ? -ENOSYS : -EINVAL;
> + if (!panel)
> + return -EINVAL;
> +
> + if (!panel->prepared)
> + return 0;
> +
> + if (panel->funcs && panel->funcs->unprepare)
> + ret = panel->funcs->unprepare(panel);
> +
> + panel->prepared = false;
> +
> + return ret;
> }
> EXPORT_SYMBOL(drm_panel_unprepare);
>
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 624bd15ecfab..7493500fc9bd 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -65,6 +65,9 @@ struct drm_panel_funcs {
> * @prepare:
> *
> * Turn on panel and perform set up.
> + * When the panel is successfully prepared the prepare() function
> + * will not be called again until the panel has been unprepared.
> + *
> */
> int (*prepare)(struct drm_panel *panel);
>
> @@ -72,6 +75,8 @@ struct drm_panel_funcs {
> * @enable:
> *
> * Enable panel (turn on back light, etc.).
> + * When the panel is successfully enabled the enable() function
> + * will not be called again until the panel has been disabled.
> */
> int (*enable)(struct drm_panel *panel);
>
> @@ -79,6 +84,7 @@ struct drm_panel_funcs {
> * @disable:
> *
> * Disable panel (turn off back light, etc.).
> + * If the panel is already disabled the disable() function is not called.
> */
> int (*disable)(struct drm_panel *panel);
>
> @@ -86,6 +92,7 @@ struct drm_panel_funcs {
> * @unprepare:
> *
> * Turn off panel.
> + * If the panel is already unprepared the unprepare() function is not called.
> */
> int (*unprepare)(struct drm_panel *panel);
>
> @@ -145,6 +152,20 @@ struct drm_panel {
> * Panel entry in registry.
> */
> struct list_head list;
> +
> + /**
> + * @prepared:
> + *
> + * Set to true when the panel is successfully prepared.
> + */
> + bool prepared;
> +
> + /**
> + * @enabled:
> + *
> + * Set to true when the panel is successfully enabled.
> + */
> + bool enabled;
> };
>
> void drm_panel_init(struct drm_panel *panel);
> --
> 2.20.1
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-08-05 17:01 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-04 20:16 [PATCH v1 0/16] drm: panel related updates Sam Ravnborg
2019-08-04 20:16 ` [PATCH v1 01/16] drm/bridge: tc358767: fix opencoded use of drm_panel_* Sam Ravnborg
2019-08-05 9:35 ` Philipp Zabel
2019-08-05 11:53 ` Sam Ravnborg
2019-08-04 20:16 ` [PATCH v1 02/16] drm/exynos: " Sam Ravnborg
2019-12-01 11:32 ` Sam Ravnborg
2019-08-04 20:16 ` [PATCH v1 03/16] " Sam Ravnborg
2019-12-01 11:32 ` Sam Ravnborg
2019-08-04 20:16 ` [PATCH v1 04/16] drm/imx: " Sam Ravnborg
2019-08-05 9:34 ` Philipp Zabel
2019-08-04 20:16 ` [PATCH v1 05/16] drm/fsl-dcu: " Sam Ravnborg
2019-08-05 9:16 ` Stefan Agner
2019-08-05 11:54 ` Sam Ravnborg
2019-08-04 20:16 ` [PATCH v1 06/16] drm/msm: " Sam Ravnborg
2019-12-01 11:33 ` Sam Ravnborg
2019-08-04 20:16 ` [PATCH v1 07/16] drm/mxsfb: " Sam Ravnborg
2019-08-05 9:20 ` Stefan Agner
2019-08-04 20:16 ` [PATCH v1 08/16] drm/sti: " Sam Ravnborg
2019-08-07 11:55 ` Benjamin Gaignard
2019-08-04 20:16 ` [PATCH v1 09/16] drm/tegra: " Sam Ravnborg
2019-12-01 11:33 ` Sam Ravnborg
2019-08-04 20:16 ` [PATCH v1 10/16] drm/panel: ili9322: move bus_flags to get_modes() Sam Ravnborg
2019-08-06 12:56 ` Linus Walleij
2019-08-04 20:16 ` [PATCH v1 11/16] drm/panel: move drm_panel functions to .c file Sam Ravnborg
2019-08-05 10:45 ` Laurent Pinchart
2019-08-04 20:16 ` [PATCH v1 12/16] drm/panel: use inline comments in drm_panel.h Sam Ravnborg
2019-08-05 10:54 ` Laurent Pinchart
2019-08-04 20:16 ` [PATCH v1 13/16] drm/panel: drop return code from drm_panel_detach() Sam Ravnborg
2019-08-05 10:56 ` Laurent Pinchart
2019-08-04 20:16 ` [PATCH v1 14/16] drm/panel: call prepare/enable only once Sam Ravnborg
2019-08-05 10:59 ` Laurent Pinchart
2019-08-05 13:15 ` Emil Velikov
2019-08-05 16:51 ` Sam Ravnborg
2019-12-02 15:22 ` Laurent Pinchart
2019-08-05 17:01 ` Sean Paul [this message]
2019-12-02 15:15 ` Laurent Pinchart
2019-08-04 20:16 ` [PATCH v1 15/16] drm/panel: add backlight support Sam Ravnborg
2019-08-05 11:04 ` Laurent Pinchart
2019-08-04 20:16 ` [PATCH v1 16/16] drm/panel: simple: use drm_panel infrastructure Sam Ravnborg
2019-08-05 11:12 ` Laurent Pinchart
2019-08-05 13:18 ` [PATCH v1 0/16] drm: panel related updates Emil Velikov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190805170120.GZ104440@art_vandelay \
--to=sean@poorly.run \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=a.hajda@samsung.com \
--cc=airlied@linux.ie \
--cc=alexios.zavras@intel.com \
--cc=alison.wang@nxp.com \
--cc=allison@lohutok.net \
--cc=benjamin.gaignard@linaro.org \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=festevam@gmail.com \
--cc=gwan-gyeong.mun@intel.com \
--cc=info@metux.net \
--cc=inki.dae@samsung.com \
--cc=jernej.skrabec@siol.net \
--cc=jonas@kwiboo.se \
--cc=jonathanh@nvidia.com \
--cc=jy0922.shim@samsung.com \
--cc=kernel@pengutronix.de \
--cc=kgene@kernel.org \
--cc=krzk@kernel.org \
--cc=kyungmin.park@samsung.com \
--cc=laurent.pinchart+renesas@ideasonboard.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=marex@denx.de \
--cc=maxime.ripard@bootlin.com \
--cc=narmstrong@baylibre.com \
--cc=p.zabel@pengutronix.de \
--cc=s.hauer@pengutronix.de \
--cc=sam@ravnborg.org \
--cc=shawnguo@kernel.org \
--cc=stefan@agner.ch \
--cc=sw0312.kim@samsung.com \
--cc=tglx@linutronix.de \
--cc=thierry.reding@gmail.com \
--cc=vincent.abriou@st.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).