linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	linux-kernel@vger.kernel.org, patches@lists.linux.dev,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	yangcong <yangcong5@huaqin.corp-partner.google.com>,
	Douglas Anderson <dianders@chromium.org>,
	Jitao Shi <jitao.shi@mediatek.com>,
	Rob Clark <robdclark@chromium.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Subject: Re: [PATCH] drm/panel: boe-tv101wum-nl6: Ensure DSI writes succeed during disable
Date: Sat, 7 Jan 2023 21:28:41 +0100	[thread overview]
Message-ID: <Y7nV+aeFiq5aD0xU@ravnborg.org> (raw)
In-Reply-To: <20230106030108.2542081-1-swboyd@chromium.org>

Hi Stephen.

On Thu, Jan 05, 2023 at 07:01:08PM -0800, Stephen Boyd wrote:
> The unprepare sequence has started to fail after moving to panel bridge
> code in the msm drm driver (commit 007ac0262b0d ("drm/msm/dsi: switch to
> DRM_PANEL_BRIDGE")). You'll see messages like this in the kernel logs:
> 
>    panel-boe-tv101wum-nl6 ae94000.dsi.0: failed to set panel off: -22
> 
> This is because boe_panel_enter_sleep_mode() needs an operating DSI link
> to set the panel into sleep mode. Performing those writes in the
> unprepare phase of bridge ops is too late, because the link has already
> been torn down by the DSI controller in post_disable, i.e. the PHY has
> been disabled, etc. See dsi_mgr_bridge_post_disable() for more details
> on the DSI .
> 
> Split the unprepare function into a disable part and an unprepare part.
> For now, just the DSI writes to enter sleep mode are put in the disable
> function. This fixes the panel off routine and keeps the panel happy.
> 
> My Wormdingler has an integrated touchscreen that stops responding to
> touch if the panel is only half disabled too. This patch fixes it. And
> finally, this saves power when the screen is off because without this
> fix the regulators for the panel are left enabled when nothing is being
> displayed on the screen.
The pattern we use in several (but not all) panel drivers are that
errors in unprepare/disable are logged but the function do not skip
the remainder of the function. This is to avoid that we do not disable
power supplies etc.

For this case we could ask ourself if the display needs to enter sleep
mode right before we disable the regulator. But if the regulator is
fixed, so the disable has no effect, this seems OK.

Please fix the unprepare to not jump out early, on top of or together
with the other fix.

	Sam

> 
> Fixes: 007ac0262b0d ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE")
> Fixes: a869b9db7adf ("drm/panel: support for boe tv101wum-nl6 wuxga dsi video mode panel")
> Cc: yangcong <yangcong5@huaqin.corp-partner.google.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Jitao Shi <jitao.shi@mediatek.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Rob Clark <robdclark@chromium.org>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> index 857a2f0420d7..c924f1124ebc 100644
> --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c
> @@ -1193,14 +1193,11 @@ static int boe_panel_enter_sleep_mode(struct boe_panel *boe)
>  	return 0;
>  }
>  
> -static int boe_panel_unprepare(struct drm_panel *panel)
> +static int boe_panel_disable(struct drm_panel *panel)
>  {
>  	struct boe_panel *boe = to_boe_panel(panel);
>  	int ret;
>  
> -	if (!boe->prepared)
> -		return 0;
> -
>  	ret = boe_panel_enter_sleep_mode(boe);
>  	if (ret < 0) {
>  		dev_err(panel->dev, "failed to set panel off: %d\n", ret);
> @@ -1209,6 +1206,16 @@ static int boe_panel_unprepare(struct drm_panel *panel)
>  
>  	msleep(150);
>  
> +	return 0;
> +}
> +
> +static int boe_panel_unprepare(struct drm_panel *panel)
> +{
> +	struct boe_panel *boe = to_boe_panel(panel);
> +
> +	if (!boe->prepared)
> +		return 0;
> +
>  	if (boe->desc->discharge_on_disable) {
>  		regulator_disable(boe->avee);
>  		regulator_disable(boe->avdd);
> @@ -1528,6 +1535,7 @@ static enum drm_panel_orientation boe_panel_get_orientation(struct drm_panel *pa
>  }
>  
>  static const struct drm_panel_funcs boe_panel_funcs = {
> +	.disable = boe_panel_disable,
>  	.unprepare = boe_panel_unprepare,
>  	.prepare = boe_panel_prepare,
>  	.enable = boe_panel_enable,
> 
> base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
> -- 
> https://chromeos.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-01-07 20:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06  3:01 [PATCH] drm/panel: boe-tv101wum-nl6: Ensure DSI writes succeed during disable Stephen Boyd
2023-01-07 20:28 ` Sam Ravnborg [this message]
2023-01-10 19:29   ` Stephen Boyd
2023-01-13 14:52     ` Sam Ravnborg
2023-01-13 20:49       ` Stephen Boyd
2023-01-18 18:24         ` Stephen Boyd
2023-01-13 16:27 ` Dave Stevenson
2023-01-13 21:12   ` Stephen Boyd
2023-01-16 14:11     ` Dave Stevenson
2023-01-18  4:50       ` Stephen Boyd
2023-01-18 21:34 ` Doug Anderson
2023-01-27  0:52   ` Doug Anderson
2023-01-31 21:27     ` Doug Anderson
2023-02-01 11:02       ` Thomas Zimmermann

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=Y7nV+aeFiq5aD0xU@ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=dianders@chromium.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jitao.shi@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=patches@lists.linux.dev \
    --cc=robdclark@chromium.org \
    --cc=swboyd@chromium.org \
    --cc=thierry.reding@gmail.com \
    --cc=yangcong5@huaqin.corp-partner.google.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).