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

[Bjorn Andersson]

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@xxxxxxxxxxxxxxxxxxxxxxxxxx
> 

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

Regards,
Bjorn

> Suggested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> ---
> 
> (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
>