diff mbox series

drm/msm/dsi: properly handle the case of empty OPP table in dsi_mgr_bridge_mode_valid

Message ID 20230124203600.3488766-1-dmitry.baryshkov@linaro.org (mailing list archive)
State New, archived
Headers show
Series drm/msm/dsi: properly handle the case of empty OPP table in dsi_mgr_bridge_mode_valid | expand

Commit Message

Dmitry Baryshkov Jan. 24, 2023, 8:36 p.m. UTC
It was left unnoticed during the review that even if there is no OPP
table in device tree, one will be created by a call to the function
devm_pm_opp_set_clkname(). This leads to dsi_mgr_bridge_mode_valid()
rejecting all modes if DT contains no OPP table for the DSI host.

Rework dsi_mgr_bridge_mode_valid() to handle this case by actually
checking that the table is populated with frequency entries before
returning an error.

Fixes: 8328041b8c82 ("drm/msm/dsi: implement opp table based check for dsi_mgr_bridge_mode_valid()")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Abhinav Kumar Jan. 24, 2023, 8:49 p.m. UTC | #1
On 1/24/2023 12:36 PM, Dmitry Baryshkov wrote:
> It was left unnoticed during the review that even if there is no OPP
> table in device tree, one will be created by a call to the function
> devm_pm_opp_set_clkname(). This leads to dsi_mgr_bridge_mode_valid()
> rejecting all modes if DT contains no OPP table for the DSI host.
> 
> Rework dsi_mgr_bridge_mode_valid() to handle this case by actually
> checking that the table is populated with frequency entries before
> returning an error.
> 
> Fixes: 8328041b8c82 ("drm/msm/dsi: implement opp table based check for dsi_mgr_bridge_mode_valid()")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_manager.c | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index b20fddb534a7..1bbac72dad35 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -456,18 +456,19 @@ static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
>   
>   	byte_clk_rate = dsi_byte_clk_get_rate(host, IS_BONDED_DSI(), mode);
>   
> -	/*
> -	 * fail all errors except -ENODEV as that could mean that opp
> -	 * table is not yet implemented
> -	 */
>   	opp = dev_pm_opp_find_freq_ceil(&pdev->dev, &byte_clk_rate);
> -	if (IS_ERR(opp)) {
> -		if (PTR_ERR(opp) == -ERANGE)
> +	if (!IS_ERR(opp)) {
> +		dev_pm_opp_put(opp);
> +	} else if (PTR_ERR(opp) == -ERANGE) {
> +		/*
> +		 * An empty table is created by devm_pm_opp_set_clkname() even
> +		 * if there is none. Thus find_freq_ceil will still return
> +		 * -ERANGE in such case.
> +		 */
> +		if (dev_pm_opp_get_opp_count(&pdev->dev) != 0)
>   			return MODE_CLOCK_RANGE;
> -		else if (PTR_ERR(opp) != -ENODEV)
> -			return MODE_ERROR;
>   	} else {
> -		dev_pm_opp_put(opp);
> +			return MODE_ERROR;
>   	}
>   
>   	return msm_dsi_host_check_dsc(host, mode);
Dmitry Baryshkov Jan. 26, 2023, 7 p.m. UTC | #2
On Tue, 24 Jan 2023 22:36:00 +0200, Dmitry Baryshkov wrote:
> It was left unnoticed during the review that even if there is no OPP
> table in device tree, one will be created by a call to the function
> devm_pm_opp_set_clkname(). This leads to dsi_mgr_bridge_mode_valid()
> rejecting all modes if DT contains no OPP table for the DSI host.
> 
> Rework dsi_mgr_bridge_mode_valid() to handle this case by actually
> checking that the table is populated with frequency entries before
> returning an error.
> 
> [...]

Applied, thanks!

[1/1] drm/msm/dsi: properly handle the case of empty OPP table in dsi_mgr_bridge_mode_valid
      https://gitlab.freedesktop.org/lumag/msm/-/commit/2ec56b232b97

Best regards,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index b20fddb534a7..1bbac72dad35 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -456,18 +456,19 @@  static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
 
 	byte_clk_rate = dsi_byte_clk_get_rate(host, IS_BONDED_DSI(), mode);
 
-	/*
-	 * fail all errors except -ENODEV as that could mean that opp
-	 * table is not yet implemented
-	 */
 	opp = dev_pm_opp_find_freq_ceil(&pdev->dev, &byte_clk_rate);
-	if (IS_ERR(opp)) {
-		if (PTR_ERR(opp) == -ERANGE)
+	if (!IS_ERR(opp)) {
+		dev_pm_opp_put(opp);
+	} else if (PTR_ERR(opp) == -ERANGE) {
+		/*
+		 * An empty table is created by devm_pm_opp_set_clkname() even
+		 * if there is none. Thus find_freq_ceil will still return
+		 * -ERANGE in such case.
+		 */
+		if (dev_pm_opp_get_opp_count(&pdev->dev) != 0)
 			return MODE_CLOCK_RANGE;
-		else if (PTR_ERR(opp) != -ENODEV)
-			return MODE_ERROR;
 	} else {
-		dev_pm_opp_put(opp);
+			return MODE_ERROR;
 	}
 
 	return msm_dsi_host_check_dsc(host, mode);