linux-kernel - Re: [PATCH v4 27/27] drm/panel: panel-simple: Prepare/unprepare are refcounted, not forced

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YILy8jbYcwAt4Bqw@builder.lan>
Date:   Fri, 23 Apr 2021 11:16:50 -0500
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Douglas Anderson <dianders@...omium.org>
Cc:     Andrzej Hajda <a.hajda@...sung.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Sam Ravnborg <sam@...nborg.org>, Wolfram Sang <wsa@...nel.org>,
        Stephen Boyd <swboyd@...omium.org>, robdclark@...omium.org,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Stanislav Lisovskiy <stanislav.lisovskiy@...el.com>,
        Steev Klimaszewski <steev@...i.org>,
        linux-arm-msm@...r.kernel.org, Linus W <linus.walleij@...aro.org>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Thierry Reding <thierry.reding@...il.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 27/27] drm/panel: panel-simple: Prepare/unprepare are
 refcounted, not forced

On Fri 16 Apr 17:39 CDT 2021, Douglas Anderson wrote:

> Historically simple-panel had code to make sure that if prepare() was
> called on an already-prepared panel that it was a no-op. Similarly if
> unprepare() was called on an already-unprepared panel it was also a
> no-op. Essentially it means that these functions always "forced" the
> value to be whatever the caller wanted it to be. You can see that the
> forcing behavior dates back at least as far as 2014 by looking at
> commit 613a633e7a56 ("drm/panel: simple: Add proper definition for
> prepare and unprepare").
> 
> Apparently the code supporting the historical behavior may not be
> needed [1] and prepare() / unprepare() are supposed to be
> balanced. Let's try removing it and see if anyone breaks! If they do
> then we can have a debate about whether we should change that code or
> revert this patch. :-) If nobody breaks then we've nicely saved a few
> lines of code and some complexity.
> 
> [1] https://lore.kernel.org/r/YHePsQgqOau1V5lD@pendragon.ideasonboard.com
> 

Acked-by: Bjorn Andersson <bjorn.andersson@...aro.org>

Regards,
Bjorn

> Suggested-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> 
> (no changes since v1)
> 
>  drivers/gpu/drm/panel/panel-simple.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 5a2953c4ca44..a2c3008af7e5 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -176,8 +176,6 @@ struct panel_simple {
>  	bool enabled;
>  	bool no_hpd;
>  
> -	bool prepared;
> -
>  	ktime_t prepared_time;
>  	ktime_t unprepared_time;
>  
> @@ -355,18 +353,12 @@ static int panel_simple_suspend(struct device *dev)
>  
>  static int panel_simple_unprepare(struct drm_panel *panel)
>  {
> -	struct panel_simple *p = to_panel_simple(panel);
>  	int ret;
>  
> -	/* Unpreparing when already unprepared is a no-op */
> -	if (!p->prepared)
> -		return 0;
> -
>  	pm_runtime_mark_last_busy(panel->dev);
>  	ret = pm_runtime_put_autosuspend(panel->dev);
>  	if (ret < 0)
>  		return ret;
> -	p->prepared = false;
>  
>  	return 0;
>  }
> @@ -475,18 +467,12 @@ static int panel_simple_prepare(struct drm_panel *panel)
>  	struct panel_simple *p = to_panel_simple(panel);
>  	int ret;
>  
> -	/* Preparing when already prepared is a no-op */
> -	if (p->prepared)
> -		return 0;
> -
>  	ret = pm_runtime_get_sync(panel->dev);
>  	if (ret < 0) {
>  		pm_runtime_put_autosuspend(panel->dev);
>  		return ret;
>  	}
>  
> -	p->prepared = true;
> -
>  	return 0;
>  }
>  
> -- 
> 2.31.1.368.gbe11c130af-goog
>