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

Bjorn Andersson
2021-04-23T16:16:57Z

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@linaro.org>

Regards,
Bjorn

> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.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
>