[01/13] drm/msm/dpu: cleanup dpu_kms_hw_init error path

Message ID	20230707203724.3820757-2-dmitry.baryshkov@linaro.org (mailing list archive)
State	New, archived
Headers	show Return-Path: <dri-devel-bounces@lists.freedesktop.org> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> To: Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>, Abhinav Kumar <quic_abhinavk@quicinc.com>, Marijn Suijten <marijn.suijten@somainline.org> Subject: [PATCH 01/13] drm/msm/dpu: cleanup dpu_kms_hw_init error path Date: Fri, 7 Jul 2023 23:37:12 +0300 Message-Id: <20230707203724.3820757-2-dmitry.baryshkov@linaro.org> In-Reply-To: <20230707203724.3820757-1-dmitry.baryshkov@linaro.org> References: <20230707203724.3820757-1-dmitry.baryshkov@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: list Cc: freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, Bjorn Andersson <andersson@kernel.org>, dri-devel@lists.freedesktop.org, Stephen Boyd <swboyd@chromium.org>, Konrad Dybcio <konrad.dybcio@linaro.org> Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>
Series	drm/msm/dpu: use managed memory allocations \| expand [00/13] drm/msm/dpu: use managed memory allocations [01/13] drm/msm/dpu: cleanup dpu_kms_hw_init error path [02/13] drm/msm/dpu: remove IS_ERR_OR_NULL for dpu_hw_intr_init() error handling [03/13] drm/msm/dpu: use devres-managed allocation for interrupts data [04/13] drm/msm/dpu: use devres-managed allocation for VBIF data [05/13] drm/msm/dpu: use devres-managed allocation for MDP TOP [06/13] drm/msm/dpu: use devres-managed allocation for HW blocks [07/13] drm/msm/dpu: drop unused dpu_plane::lock [08/13] drm/msm/dpu: remove QoS teardown on plane destruction [09/13] drm/msm/dpu: use drmm-managed allocation for dpu_plane [10/13] drm/msm/dpu: use drmm-managed allocation for dpu_crtc [11/13] drm/msm/dpu: use drmm-managed allocation for dpu_encoder_phys [12/13] drm/msm/dpu: drop dpu_encoder_phys_ops::destroy [13/13] drm/msm/dpu: use drmm-managed allocation for dpu_encoder_virt

Message ID

20230707203724.3820757-2-dmitry.baryshkov@linaro.org (mailing list archive)

State

New, archived

Headers

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
 Abhinav Kumar <quic_abhinavk@quicinc.com>,
 Marijn Suijten <marijn.suijten@somainline.org>
Subject: [PATCH 01/13] drm/msm/dpu: cleanup dpu_kms_hw_init error path
Date: Fri,  7 Jul 2023 23:37:12 +0300
Message-Id: <20230707203724.3820757-2-dmitry.baryshkov@linaro.org>
In-Reply-To: <20230707203724.3820757-1-dmitry.baryshkov@linaro.org>
References: <20230707203724.3820757-1-dmitry.baryshkov@linaro.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: list
Cc: freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
 Bjorn Andersson <andersson@kernel.org>, dri-devel@lists.freedesktop.org,
 Stephen Boyd <swboyd@chromium.org>, Konrad Dybcio <konrad.dybcio@linaro.org>
Errors-To: dri-devel-bounces@lists.freedesktop.org
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>

Series

drm/msm/dpu: use managed memory allocations | expand

Commit Message

Dmitry Baryshkov July 7, 2023, 8:37 p.m. UTC

It was noticed that dpu_kms_hw_init()'s error path contains several
labels which point to the same code path. Replace all of them with a
single label.

Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Comments

Konrad Dybcio July 7, 2023, 11:25 p.m. UTC | #1

On 7.07.2023 22:37, Dmitry Baryshkov wrote:
> It was noticed that dpu_kms_hw_init()'s error path contains several
> labels which point to the same code path. Replace all of them with a
> single label.
> 
> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
it's the first time I'm seeing this code

Konrad
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index c11b3ab572ab..e7ac02e92f42 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1037,7 +1037,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  	if (!dpu_kms->catalog) {
>  		DPU_ERROR("device config not known!\n");
>  		rc = -EINVAL;
> -		goto power_error;
> +		goto err_pm_put;
>  	}
>  
>  	/*
> @@ -1047,13 +1047,13 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  	rc = _dpu_kms_mmu_init(dpu_kms);
>  	if (rc) {
>  		DPU_ERROR("dpu_kms_mmu_init failed: %d\n", rc);
> -		goto power_error;
> +		goto err_pm_put;
>  	}
>  
>  	rc = dpu_rm_init(&dpu_kms->rm, dpu_kms->catalog, dpu_kms->mmio);
>  	if (rc) {
>  		DPU_ERROR("rm init failed: %d\n", rc);
> -		goto power_error;
> +		goto err_pm_put;
>  	}
>  
>  	dpu_kms->rm_init = true;
> @@ -1065,7 +1065,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  		rc = PTR_ERR(dpu_kms->hw_mdp);
>  		DPU_ERROR("failed to get hw_mdp: %d\n", rc);
>  		dpu_kms->hw_mdp = NULL;
> -		goto power_error;
> +		goto err_pm_put;
>  	}
>  
>  	for (i = 0; i < dpu_kms->catalog->vbif_count; i++) {
> @@ -1076,7 +1076,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  		if (IS_ERR(hw)) {
>  			rc = PTR_ERR(hw);
>  			DPU_ERROR("failed to init vbif %d: %d\n", vbif->id, rc);
> -			goto power_error;
> +			goto err_pm_put;
>  		}
>  
>  		dpu_kms->hw_vbif[vbif->id] = hw;
> @@ -1092,7 +1092,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  	rc = dpu_core_perf_init(&dpu_kms->perf, dpu_kms->catalog->perf, max_core_clk_rate);
>  	if (rc) {
>  		DPU_ERROR("failed to init perf %d\n", rc);
> -		goto perf_err;
> +		goto err_pm_put;
>  	}
>  
>  	dpu_kms->hw_intr = dpu_hw_intr_init(dpu_kms->mmio, dpu_kms->catalog);
> @@ -1100,7 +1100,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  		rc = PTR_ERR(dpu_kms->hw_intr);
>  		DPU_ERROR("hw_intr init failed: %d\n", rc);
>  		dpu_kms->hw_intr = NULL;
> -		goto hw_intr_init_err;
> +		goto err_pm_put;
>  	}
>  
>  	dev->mode_config.min_width = 0;
> @@ -1125,7 +1125,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  	rc = _dpu_kms_drm_obj_init(dpu_kms);
>  	if (rc) {
>  		DPU_ERROR("modeset init failed: %d\n", rc);
> -		goto drm_obj_init_err;
> +		goto err_pm_put;
>  	}
>  
>  	dpu_vbif_init_memtypes(dpu_kms);
> @@ -1134,10 +1134,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  
>  	return 0;
>  
> -drm_obj_init_err:
> -hw_intr_init_err:
> -perf_err:
> -power_error:
> +err_pm_put:
>  	pm_runtime_put_sync(&dpu_kms->pdev->dev);
>  error:
>  	_dpu_kms_hw_destroy(dpu_kms);

Dmitry Baryshkov July 7, 2023, 11:48 p.m. UTC | #2

On 08/07/2023 02:25, Konrad Dybcio wrote:
> On 7.07.2023 22:37, Dmitry Baryshkov wrote:
>> It was noticed that dpu_kms_hw_init()'s error path contains several
>> labels which point to the same code path. Replace all of them with a
>> single label.
>>
>> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> it's the first time I'm seeing this code
> 

It is Suggested-by, not something else. And you pointed it out in 
https://lore.kernel.org/linux-arm-msm/6d598438-f10f-8678-7878-829b8b3ae790@linaro.org/

> Konrad
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++++++++------------
>>   1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> index c11b3ab572ab..e7ac02e92f42 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> @@ -1037,7 +1037,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   	if (!dpu_kms->catalog) {
>>   		DPU_ERROR("device config not known!\n");
>>   		rc = -EINVAL;
>> -		goto power_error;
>> +		goto err_pm_put;
>>   	}
>>   
>>   	/*
>> @@ -1047,13 +1047,13 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   	rc = _dpu_kms_mmu_init(dpu_kms);
>>   	if (rc) {
>>   		DPU_ERROR("dpu_kms_mmu_init failed: %d\n", rc);
>> -		goto power_error;
>> +		goto err_pm_put;
>>   	}
>>   
>>   	rc = dpu_rm_init(&dpu_kms->rm, dpu_kms->catalog, dpu_kms->mmio);
>>   	if (rc) {
>>   		DPU_ERROR("rm init failed: %d\n", rc);
>> -		goto power_error;
>> +		goto err_pm_put;
>>   	}
>>   
>>   	dpu_kms->rm_init = true;
>> @@ -1065,7 +1065,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   		rc = PTR_ERR(dpu_kms->hw_mdp);
>>   		DPU_ERROR("failed to get hw_mdp: %d\n", rc);
>>   		dpu_kms->hw_mdp = NULL;
>> -		goto power_error;
>> +		goto err_pm_put;
>>   	}
>>   
>>   	for (i = 0; i < dpu_kms->catalog->vbif_count; i++) {
>> @@ -1076,7 +1076,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   		if (IS_ERR(hw)) {
>>   			rc = PTR_ERR(hw);
>>   			DPU_ERROR("failed to init vbif %d: %d\n", vbif->id, rc);
>> -			goto power_error;
>> +			goto err_pm_put;
>>   		}
>>   
>>   		dpu_kms->hw_vbif[vbif->id] = hw;
>> @@ -1092,7 +1092,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   	rc = dpu_core_perf_init(&dpu_kms->perf, dpu_kms->catalog->perf, max_core_clk_rate);
>>   	if (rc) {
>>   		DPU_ERROR("failed to init perf %d\n", rc);
>> -		goto perf_err;
>> +		goto err_pm_put;
>>   	}
>>   
>>   	dpu_kms->hw_intr = dpu_hw_intr_init(dpu_kms->mmio, dpu_kms->catalog);
>> @@ -1100,7 +1100,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   		rc = PTR_ERR(dpu_kms->hw_intr);
>>   		DPU_ERROR("hw_intr init failed: %d\n", rc);
>>   		dpu_kms->hw_intr = NULL;
>> -		goto hw_intr_init_err;
>> +		goto err_pm_put;
>>   	}
>>   
>>   	dev->mode_config.min_width = 0;
>> @@ -1125,7 +1125,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   	rc = _dpu_kms_drm_obj_init(dpu_kms);
>>   	if (rc) {
>>   		DPU_ERROR("modeset init failed: %d\n", rc);
>> -		goto drm_obj_init_err;
>> +		goto err_pm_put;
>>   	}
>>   
>>   	dpu_vbif_init_memtypes(dpu_kms);
>> @@ -1134,10 +1134,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>   
>>   	return 0;
>>   
>> -drm_obj_init_err:
>> -hw_intr_init_err:
>> -perf_err:
>> -power_error:
>> +err_pm_put:
>>   	pm_runtime_put_sync(&dpu_kms->pdev->dev);
>>   error:
>>   	_dpu_kms_hw_destroy(dpu_kms);

Konrad Dybcio July 8, 2023, 10:01 a.m. UTC | #3

On 8.07.2023 01:48, Dmitry Baryshkov wrote:
> On 08/07/2023 02:25, Konrad Dybcio wrote:
>> On 7.07.2023 22:37, Dmitry Baryshkov wrote:
>>> It was noticed that dpu_kms_hw_init()'s error path contains several
>>> labels which point to the same code path. Replace all of them with a
>>> single label.
>>>
>>> Suggested-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> it's the first time I'm seeing this code
>>
> 
> It is Suggested-by, not something else. And you pointed it out in https://lore.kernel.org/linux-arm-msm/6d598438-f10f-8678-7878-829b8b3ae790@linaro.org/
Oh, thanks

Konrad
> 
>> Konrad
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++++++++------------
>>>   1 file changed, 9 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> index c11b3ab572ab..e7ac02e92f42 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> @@ -1037,7 +1037,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>       if (!dpu_kms->catalog) {
>>>           DPU_ERROR("device config not known!\n");
>>>           rc = -EINVAL;
>>> -        goto power_error;
>>> +        goto err_pm_put;
>>>       }
>>>         /*
>>> @@ -1047,13 +1047,13 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>       rc = _dpu_kms_mmu_init(dpu_kms);
>>>       if (rc) {
>>>           DPU_ERROR("dpu_kms_mmu_init failed: %d\n", rc);
>>> -        goto power_error;
>>> +        goto err_pm_put;
>>>       }
>>>         rc = dpu_rm_init(&dpu_kms->rm, dpu_kms->catalog, dpu_kms->mmio);
>>>       if (rc) {
>>>           DPU_ERROR("rm init failed: %d\n", rc);
>>> -        goto power_error;
>>> +        goto err_pm_put;
>>>       }
>>>         dpu_kms->rm_init = true;
>>> @@ -1065,7 +1065,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>           rc = PTR_ERR(dpu_kms->hw_mdp);
>>>           DPU_ERROR("failed to get hw_mdp: %d\n", rc);
>>>           dpu_kms->hw_mdp = NULL;
>>> -        goto power_error;
>>> +        goto err_pm_put;
>>>       }
>>>         for (i = 0; i < dpu_kms->catalog->vbif_count; i++) {
>>> @@ -1076,7 +1076,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>           if (IS_ERR(hw)) {
>>>               rc = PTR_ERR(hw);
>>>               DPU_ERROR("failed to init vbif %d: %d\n", vbif->id, rc);
>>> -            goto power_error;
>>> +            goto err_pm_put;
>>>           }
>>>             dpu_kms->hw_vbif[vbif->id] = hw;
>>> @@ -1092,7 +1092,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>       rc = dpu_core_perf_init(&dpu_kms->perf, dpu_kms->catalog->perf, max_core_clk_rate);
>>>       if (rc) {
>>>           DPU_ERROR("failed to init perf %d\n", rc);
>>> -        goto perf_err;
>>> +        goto err_pm_put;
>>>       }
>>>         dpu_kms->hw_intr = dpu_hw_intr_init(dpu_kms->mmio, dpu_kms->catalog);
>>> @@ -1100,7 +1100,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>           rc = PTR_ERR(dpu_kms->hw_intr);
>>>           DPU_ERROR("hw_intr init failed: %d\n", rc);
>>>           dpu_kms->hw_intr = NULL;
>>> -        goto hw_intr_init_err;
>>> +        goto err_pm_put;
>>>       }
>>>         dev->mode_config.min_width = 0;
>>> @@ -1125,7 +1125,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>       rc = _dpu_kms_drm_obj_init(dpu_kms);
>>>       if (rc) {
>>>           DPU_ERROR("modeset init failed: %d\n", rc);
>>> -        goto drm_obj_init_err;
>>> +        goto err_pm_put;
>>>       }
>>>         dpu_vbif_init_memtypes(dpu_kms);
>>> @@ -1134,10 +1134,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>>         return 0;
>>>   -drm_obj_init_err:
>>> -hw_intr_init_err:
>>> -perf_err:
>>> -power_error:
>>> +err_pm_put:
>>>       pm_runtime_put_sync(&dpu_kms->pdev->dev);
>>>   error:
>>>       _dpu_kms_hw_destroy(dpu_kms);
>

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index c11b3ab572ab..e7ac02e92f42 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1037,7 +1037,7 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 	if (!dpu_kms->catalog) {
 		DPU_ERROR("device config not known!\n");
 		rc = -EINVAL;
-		goto power_error;
+		goto err_pm_put;
 	}
 
 	/*
@@ -1047,13 +1047,13 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 	rc = _dpu_kms_mmu_init(dpu_kms);
 	if (rc) {
 		DPU_ERROR("dpu_kms_mmu_init failed: %d\n", rc);
-		goto power_error;
+		goto err_pm_put;
 	}
 
 	rc = dpu_rm_init(&dpu_kms->rm, dpu_kms->catalog, dpu_kms->mmio);
 	if (rc) {
 		DPU_ERROR("rm init failed: %d\n", rc);
-		goto power_error;
+		goto err_pm_put;
 	}
 
 	dpu_kms->rm_init = true;
@@ -1065,7 +1065,7 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 		rc = PTR_ERR(dpu_kms->hw_mdp);
 		DPU_ERROR("failed to get hw_mdp: %d\n", rc);
 		dpu_kms->hw_mdp = NULL;
-		goto power_error;
+		goto err_pm_put;
 	}
 
 	for (i = 0; i < dpu_kms->catalog->vbif_count; i++) {
@@ -1076,7 +1076,7 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 		if (IS_ERR(hw)) {
 			rc = PTR_ERR(hw);
 			DPU_ERROR("failed to init vbif %d: %d\n", vbif->id, rc);
-			goto power_error;
+			goto err_pm_put;
 		}
 
 		dpu_kms->hw_vbif[vbif->id] = hw;
@@ -1092,7 +1092,7 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 	rc = dpu_core_perf_init(&dpu_kms->perf, dpu_kms->catalog->perf, max_core_clk_rate);
 	if (rc) {
 		DPU_ERROR("failed to init perf %d\n", rc);
-		goto perf_err;
+		goto err_pm_put;
 	}
 
 	dpu_kms->hw_intr = dpu_hw_intr_init(dpu_kms->mmio, dpu_kms->catalog);
@@ -1100,7 +1100,7 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 		rc = PTR_ERR(dpu_kms->hw_intr);
 		DPU_ERROR("hw_intr init failed: %d\n", rc);
 		dpu_kms->hw_intr = NULL;
-		goto hw_intr_init_err;
+		goto err_pm_put;
 	}
 
 	dev->mode_config.min_width = 0;
@@ -1125,7 +1125,7 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 	rc = _dpu_kms_drm_obj_init(dpu_kms);
 	if (rc) {
 		DPU_ERROR("modeset init failed: %d\n", rc);
-		goto drm_obj_init_err;
+		goto err_pm_put;
 	}
 
 	dpu_vbif_init_memtypes(dpu_kms);
@@ -1134,10 +1134,7 @@  static int dpu_kms_hw_init(struct msm_kms *kms)
 
 	return 0;
 
-drm_obj_init_err:
-hw_intr_init_err:
-perf_err:
-power_error:
+err_pm_put:
 	pm_runtime_put_sync(&dpu_kms->pdev->dev);
 error:
 	_dpu_kms_hw_destroy(dpu_kms);

[01/13] drm/msm/dpu: cleanup dpu_kms_hw_init error path

Commit Message

Comments

Patch