Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <db6ab6f5-e3b1-f068-37ac-807e1ff074a8@amd.com>
Date:   Mon, 4 Jul 2022 15:49:19 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH] drm/amdpgu/debugfs: Simplify some exit paths
Content-Language: en-US
To:     =?UTF-8?Q?Andr=c3=a9_Almeida?= <andrealmeid@igalia.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        'Pan Xinhui' <Xinhui.Pan@amd.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Hawking Zhang <Hawking.Zhang@amd.com>,
        Tao Zhou <tao.zhou1@amd.com>,
        Felix Kuehling <Felix.Kuehling@amd.com>,
        Jack Xiao <Jack.Xiao@amd.com>, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Cc:     kernel-dev@igalia.com
References: <20220704134532.103876-1-andrealmeid@igalia.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20220704134532.103876-1-andrealmeid@igalia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OEMrYVp0YnRHU2cybElZd0NBODk0dHNUYnN4azNEQVJvS2dFT1JuaW8rL2lh?=
 =?utf-8?B?aldvOHk1OVV1L3RQaUZKRWo0M29YTkxaVHF0OWF3a1dPYlhxbWJOczY1VXRZ?=
 =?utf-8?B?cml2dzBQZUhPTEk3QXRFY0xoRk9SNmN1dmdpKy9jZlZOZE4rSmRPdU0wVWFl?=
 =?utf-8?B?NURJdmJoREZSL2x0VkJqdXFHTDNhaEtMZXpLRDYvYjFZcVk3S2U1bHNKQlpQ?=
 =?utf-8?B?Q0lRYUhyaktQdTdKTzZ0eEgrNGtXQmNCaHl6SFg1WlpNUjVYZ3ZROExLQlFI?=
 =?utf-8?B?cTRtTDkvNmJrS0pORlpnV1ZWWEdNOGZCYmh5WUhMQXMvam9ocjJrSFRndkt3?=
 =?utf-8?B?MGpzVzdCVlB3SzJ0TzdjZC9qWFB3djdXeXZoK28wT3BXbVNvNkE5bWRwUkZS?=
 =?utf-8?B?eWFmTmNpSjNpNmNTa0wvdUx5UEZyOUVBL3JtTlRRMDVpSUxCS1ArTTVldzJS?=
 =?utf-8?B?Y043WVVCK3dsWUE2eTBmcm1PTStNMkVLdDdsTHZVL1gyTzdaUE4rK21ncS9V?=
 =?utf-8?B?RWJSdUZrcG9vZzhvYndSUnlIZ1Q2QXROZXAyWmQ5dE1aR2ZsQmY0bG5nR3pY?=
 =?utf-8?B?cy8za09UeGh1d2d2OHJNb1dMM3kxdmsyTDZaRXQvcmdGZHBtWmEyc04yRjJ3?=
 =?utf-8?B?dGxYWllMK1VEM2pEc1Nrci9FY29ZdWc2eWxVRXIvb0YzVjR1R3dhYU5GSGxB?=
 =?utf-8?B?MEhzYTdNdVlVdFhhdFo4QW8vRDFrR2RtSUhOLzEyWDB6elJtYlZyR0xGMWJE?=
 =?utf-8?B?YnpCWlV6OU1TaTZrS2hobkxBaVJVM0RPbEd2ajhxRzVTSkxwZkZqVE90QXI2?=
 =?utf-8?B?eW9wVncwcFYwSFpqc091cys2NjYrNkZ6QTF2M2ZiZ0QxS3lPQkNPZHprYUVi?=
 =?utf-8?B?VHNUbkdrS3Z1K2ljTVJiNDl3OVNPU1cya0E5ZUNsRWQ2OW5oYWlDY25VaWV3?=
 =?utf-8?B?RjRhcHB4ZHZnMWhxemc2d1pXVUpaSnExSytxSWk5YW5lZ2ZZN2ZBQndyc09M?=
 =?utf-8?B?ZFZKNEVLcG9OWW1GRHdyeG83WUFDZHRLWGlqaXlhTnlEanZLbkRHcDFHaThz?=
 =?utf-8?B?S0g4dnhVT0lqK2R4L25jbUdIbHJnd1Y0NE9VR09KcnYwQ29rVTY5Q29LZVFt?=
 =?utf-8?B?U0YwbFhhSlBsV1A4Q3dydHpJNjV6QWxYS3VTT3BvTkQ1THFyZUIvQzIrZW5Z?=
 =?utf-8?B?NHRkdmp0a0ZtMk9rVmt0WjYvbFhlUURwSHdub2RoL2ZZSXNVa3IwQnZXTWNz?=
 =?utf-8?B?eEcwRy9nMEIxT3FwRmI5QzVDUFJRZGQ3R0FPbFoydHFuTDQwTWdBb2k0VS93?=
 =?utf-8?B?eHR6M0RGakRObU9tdXZxeFhlb1NFV1JDdzFtUFE5Y0p6V1N6bHRWQXpqcnFJ?=
 =?utf-8?B?NTJFaGh3RTZQK0hlOC8rekVwd2liWDRlbE5ZYllUYS9KTWwzd1crZmZlc2tx?=
 =?utf-8?B?Y1FWTHV2QzZaWm05N2JNcjZzYjk5ZU1HN2doR1F5MENPR0ZpNUloWExUNjQ3?=
 =?utf-8?B?SXd5dGs5VXlsaElRbTVxVEFqaFkzUEVGWmRtNWFQMU4rRWZpcHNlVWpMRS9V?=
 =?utf-8?B?NDdFV3FYQWh4QUdOK3pVMUtjbFZaWXhaT2duKzJITHlHZlJlNzM2K3NqOW1r?=
 =?utf-8?B?bExZeUtIc3owM1M2L2QyU0VDQU4zeGkrY0JSSEllTWhkTHRvUlN2dTB5UDBD?=
 =?utf-8?B?NExJNUxDNVlodG9ZSTJGUGhKbklBa05pMUd2aGhmT0ZxQUtaSDdYMzc2V1k1?=
 =?utf-8?B?aWhaYnlDUlpLSnZqYkFjMW50aDlhKzNjS3lFNXlZZFd2NER1emhHN090Nkk1?=
 =?utf-8?B?a3MxNnA3MXc2dWorUmFiWndkWGV2YndsQWpRNTdOeHFnVzZHZ01mMWlEMkxl?=
 =?utf-8?B?V2RKMzBhMmtNTityQjB2cFFpRDZNV2ZvTlVjdU5zazMwdjNWRFhVMDRXRjVE?=
 =?utf-8?B?T2VvRDIydnhydTZmc09HMExBQnlFYnVQMzRTdEw2bWErRXNUQXA1QUd5dXlV?=
 =?utf-8?B?T3o2N2hsZGJGV0pqQlV1RXZ1VVlNOVZPL1BadVdDT3o3aGN4eCtKbktZTDRF?=
 =?utf-8?B?T2FZSURrYWtzNVFHZnRIbTlGN0djQkErUW16U3NPZ3ErM1JURkVXTHBjN1hE?=
 =?utf-8?B?M1RRc0RCU3A1UWRGejhKejhtSmlZczdKd0VHWHlLT1hHWEpWU2RRcHNyK2Vh?=
 =?utf-8?Q?ttPOJ+1B7t66gVLaQ5G/mk/oVF7WbVWrflgx5q7k4frn?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 84d23044-07dd-4581-7f2c-08da5dc4019e
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jul 2022 13:49:25.0202
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: kOV5Ly27Vmiz1RN9WK21IUWL0JuAZ2TtXhd+lr2jmiyuwhicPngFDLel1bAbYOMc
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4440
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 04.07.22 um 15:45 schrieb André Almeida:
> To avoid code repetition, unify the function exit path when possible. No
> functional changes.
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 107 ++++++++------------
>   1 file changed, 42 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index f3ac7912c29c..f3b3c688e4e7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -383,12 +383,8 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
>   
>   		value = RREG32_PCIE(*pos);
>   		r = put_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			amdgpu_virt_disable_access_debugfs(adev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		result += 4;
>   		buf += 4;
> @@ -396,11 +392,12 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file *f, char __user *buf,
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -
>   	amdgpu_virt_disable_access_debugfs(adev);
> -	return result;
> +	return r;
>   }
>   
>   /**
> @@ -441,12 +438,8 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user
>   		uint32_t value;
>   
>   		r = get_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			amdgpu_virt_disable_access_debugfs(adev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		WREG32_PCIE(*pos, value);
>   
> @@ -456,11 +449,12 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct file *f, const char __user
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -
>   	amdgpu_virt_disable_access_debugfs(adev);
> -	return result;
> +	return r;
>   }
>   
>   /**
> @@ -502,12 +496,8 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char __user *buf,
>   
>   		value = RREG32_DIDT(*pos >> 2);
>   		r = put_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			amdgpu_virt_disable_access_debugfs(adev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		result += 4;
>   		buf += 4;
> @@ -515,11 +505,12 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char __user *buf,
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -
>   	amdgpu_virt_disable_access_debugfs(adev);
> -	return result;
> +	return r;
>   }
>   
>   /**
> @@ -560,12 +551,8 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
>   		uint32_t value;
>   
>   		r = get_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			amdgpu_virt_disable_access_debugfs(adev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		WREG32_DIDT(*pos >> 2, value);
>   
> @@ -575,11 +562,12 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -
>   	amdgpu_virt_disable_access_debugfs(adev);
> -	return result;
> +	return r;
>   }
>   
>   /**
> @@ -621,12 +609,8 @@ static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, char __user *buf,
>   
>   		value = RREG32_SMC(*pos);
>   		r = put_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			amdgpu_virt_disable_access_debugfs(adev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		result += 4;
>   		buf += 4;
> @@ -634,11 +618,12 @@ static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, char __user *buf,
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -
>   	amdgpu_virt_disable_access_debugfs(adev);
> -	return result;
> +	return r;
>   }
>   
>   /**
> @@ -679,12 +664,8 @@ static ssize_t amdgpu_debugfs_regs_smc_write(struct file *f, const char __user *
>   		uint32_t value;
>   
>   		r = get_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			amdgpu_virt_disable_access_debugfs(adev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		WREG32_SMC(*pos, value);
>   
> @@ -694,11 +675,12 @@ static ssize_t amdgpu_debugfs_regs_smc_write(struct file *f, const char __user *
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -
>   	amdgpu_virt_disable_access_debugfs(adev);
> -	return result;
> +	return r;
>   }
>   
>   /**
> @@ -1090,11 +1072,8 @@ static ssize_t amdgpu_debugfs_gfxoff_write(struct file *f, const char __user *bu
>   		uint32_t value;
>   
>   		r = get_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		amdgpu_gfx_off_ctrl(adev, value ? true : false);
>   
> @@ -1104,10 +1083,12 @@ static ssize_t amdgpu_debugfs_gfxoff_write(struct file *f, const char __user *bu
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
>   
> -	return result;
> +	return r;
>   }
>   
>   
> @@ -1139,18 +1120,12 @@ static ssize_t amdgpu_debugfs_gfxoff_read(struct file *f, char __user *buf,
>   		uint32_t value;
>   
>   		r = amdgpu_get_gfx_off_status(adev, &value);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		r = put_user(value, (uint32_t *)buf);
> -		if (r) {
> -			pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> -			pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> -			return r;
> -		}
> +		if (r)
> +			goto out;
>   
>   		result += 4;
>   		buf += 4;
> @@ -1158,10 +1133,12 @@ static ssize_t amdgpu_debugfs_gfxoff_read(struct file *f, char __user *buf,
>   		size -= 4;
>   	}
>   
> +	r = result;
> +out:
>   	pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
>   	pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
>   
> -	return result;
> +	return r;
>   }
>   
>   static const struct file_operations amdgpu_debugfs_regs2_fops = {