Message-ID: <300c4dd2-29bc-0f9f-1d6d-b8b65bf6c69c@amd.com>
Date:   Fri, 10 Mar 2023 10:42:51 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH] drm/amd/display: use a more accurate check in
 dm_helpers_dp_read_dpcd()
Content-Language: en-US
To:     Hamza Mahfooz <hamza.mahfooz@amd.com>,
        amd-gfx@lists.freedesktop.org
Cc:     Harry Wentland <harry.wentland@amd.com>,
        Leo Li <sunpeng.li@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, Lyude Paul <lyude@redhat.com>,
        Wayne Lin <Wayne.Lin@amd.com>,
        Qingqing Zhuo <qingqing.zhuo@amd.com>,
        Ian Chen <ian.chen@amd.com>,
        Sung Joon Kim <sungjoon.kim@amd.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20230309213027.256243-1-hamza.mahfooz@amd.com>
From:   Rodrigo Siqueira Jordao <Rodrigo.Siqueira@amd.com>
In-Reply-To: <20230309213027.256243-1-hamza.mahfooz@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QlQwVVdobnlseWI0RFRFUjZWSi9GaHVCblNGbnFkZnJJYTE4aVJWZ21Rb29C?=
 =?utf-8?B?QnlTbFBoNUN0YjJseEVvb2NMTWNXTlhNVXppU2ZMRkJVd09GbjE0bXQxS2hu?=
 =?utf-8?B?dnAzbHpUbER6L1drcm5ZUzhNcXk4dWUvT1h1ZGs0TDlNalg0SE5UakgxMU81?=
 =?utf-8?B?Z0c3MVZ2dzVIcW9McWlWSEp1RmJOYWY2OFNzeUR4aWh2RyszN1owbmozVVd1?=
 =?utf-8?B?VjAzd2ZRaDJ1QWY2VHVDeVFDbkVoTG93Z2JlZGMxUnJleGhjTFlyMVlFZlBB?=
 =?utf-8?B?NEZIcXhTRmhoaE91SnhHTUFMaDlvUmdEclUram5mbWZIbEluQ1hwTWFRK1RY?=
 =?utf-8?B?T2NWcjRvUVNCZEcrL2NwbTRaSW8vdGZGa3IxN0VEM05tK2lTVTJDMVEzYktp?=
 =?utf-8?B?ckNGTUp6LzRVVG1lSEM2bUtEVmFNVkRpeW9ZYytGNjRSZ3VsRDVhaGhTWDgw?=
 =?utf-8?B?WW4rUytKNFR0Ym9HZjA5b3A3UThUUDJhOGp6OS9mRnNaaTZVbXJPQ2Z2V0dR?=
 =?utf-8?B?QlN4Nk9rVG1OT3o5ZzJETUpRYy91T2FQRUNCT0xOSU83cHJQM1hkUytnaHU3?=
 =?utf-8?B?SzF2am9TRjNhaVQzN3k2eklJSTdKbTVaTXpHYjJuN0pZQ1FSOFQrRU1NcWJK?=
 =?utf-8?B?WEpHUVpHK2M4cG9BVWZaQnpDV1pZbHhmRjhQOVd1YVpxMjZVemxkemVDWnZa?=
 =?utf-8?B?ek9BVHhVTzg1Nks5ZUtYbmtud3ZvaURtK1o0UmltejRBZ0ZYaG9RMWJmSFV4?=
 =?utf-8?B?R3c2c2ZySEtEajFGeXdrNTVodG41Q3lqR0NXMGF3ZXl1a2dXakFxQXh2UnJo?=
 =?utf-8?B?Sks2bkJCNXFrb3Z0MzVjcDlhZld4WkV1bWlVcGlHNDFvOGlBdFdqa2MwODBt?=
 =?utf-8?B?YzFNT2dFTXZHWWlmMGIvUWFJTEJrK3VmR2xWYUFPOHMxU2ZrSU5pR0I5cm5J?=
 =?utf-8?B?VVM3MzZFNHorN3AxNzRSVjhQUmZ6Qys5TjhNSmZsc1Q3RDJGY01ZcmFaMjl1?=
 =?utf-8?B?b2NnK1h0aEJKQWJWS1IvWUlReitrc2c2d1d0aUdYY1h1QXNaelhSeTE1VDdq?=
 =?utf-8?B?ZS9EVHpTMENrUnVIcW9CYlRndHpUMU9DS0crSUI4T0crci9QWHZTS3FaUEhK?=
 =?utf-8?B?c0ZJbVlJWWxPTnZ2RWtiQVRWVDRuekVjYmFBcHUySWx3emhUK1Z1aWt0NzNR?=
 =?utf-8?B?UlpYY2ZXeEgvNFZKYU9BemZ4anJiVDU0bGFYYjdvYlp4SytUSkpKWm1RTWVt?=
 =?utf-8?B?QmJVMFllRGZoTFBMbzNweFVBandkNEM3dllpeVRCMEVjc3ZBQ3Z2Q2JCV2Fi?=
 =?utf-8?B?ekMyTnRzM3NrUGZIQUJlSExBa0dYaFRUZXh3RElBemdoVmYrYjNoSU9MRExJ?=
 =?utf-8?B?OEdiQU4vSnlwZHI4cHVZV01iMkx0bUdKbWNHSGh0d2pMVWFqRGdBMDBhVE4x?=
 =?utf-8?B?NVlEc1NmT0U3NGNmeEFlOGpwamxHRnBGRU9UcWRUeXZielM2bGk0QWdkc05a?=
 =?utf-8?B?ODJMYkxCd0QvS3pNUlc4dDJ5cUNPbUNJdnQ0VERiL285Wmw1OUY2R01VMmpk?=
 =?utf-8?B?N1RIZnl4WFgyaFlFVkRPNk1xQVVTSkQ2YUZXMUNrU2RsS29ValpJQllQZGlw?=
 =?utf-8?B?Q1I1RXhSNXlnd3BmR0pCSDRQS1pOSUFHSUtzWlBmN2xmYllnY01DNjU5Nzdo?=
 =?utf-8?B?aTNWWEtXWDFkWWJQaVB4cDRUSEJqMlByZzVlVW91UUlzSlFGYUZwRWV0M3Bz?=
 =?utf-8?B?TDdCOEtBSWp1N2xUeFdiQ1hxNDk0bVQwd3FreFVscGRoaWFnaFh5YVMwMFRU?=
 =?utf-8?B?TXA1Z2oybU1hekx5TVhyNGg2QWtkYUJSZnhSbWZBeTRLVFB2Mlp0YWlZUUZX?=
 =?utf-8?B?Y1hJWkx6cW04cVBDZTI2dnNUbjh2TEZXaHpSUkY4bUthVjFBcWJISU5CTVBr?=
 =?utf-8?B?UHNNQnh4My81OWVRQzNtV1I5eWtUZDRWWm5MZkZtM2xOZWtBbUIyU2ViajFw?=
 =?utf-8?B?b25lWE9HTFJ3cjA2NlRhTS92eE9KOHJkVm9wczFwa3lENjhnTkdvK0RISTZo?=
 =?utf-8?B?SEZLeUcxMWlvU1ZEK2pnemxYQmNld2dFaVpaR1NIMVlPOERlTmJxRzlXVXZo?=
 =?utf-8?B?YlpJZnkyNkdobFJOT0RveFUwc2xDd0ZpNHFtemVFenFLemNYUFovaVhoK0dQ?=
 =?utf-8?Q?2MELNqH3ksmE1AuovYO3XmGfzIzW/e93spB8TgDtraUI?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f02582a3-5812-437e-3d20-08db218ee1b2
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB2941.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Mar 2023 17:42:56.1184
 (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: FD4zHpzNgZVCHuRSSoRMmH/+lsBRIR18HSGudd0L+2oWIB8ChX3r7eLg/VBLH91USLZpFvT+j+rHusVpoZ7jdg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB6880
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 3/9/23 14:30, Hamza Mahfooz wrote:
> We should be checking if drm_dp_dpcd_read() returns the size that we are
> asking it to read instead of just checking if it is greater than zero.
> Also, we should WARN_ON() here since this condition is only ever met, if
> there is an issue worth investigating. So, compare the return value of
> drm_dp_dpcd_read() to size and WARN_ON() if they aren't equal.
> 
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> index 8d598b322e5b..ed2ed7b1d869 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> @@ -511,8 +511,8 @@ bool dm_helpers_dp_read_dpcd(
>   		return false;
>   	}
>   
> -	return drm_dp_dpcd_read(&aconnector->dm_dp_aux.aux, address,
> -			data, size) > 0;
> +	return !WARN_ON(drm_dp_dpcd_read(&aconnector->dm_dp_aux.aux, address,
> +					 data, size) != size);
>   }
>   
>   bool dm_helpers_dp_write_dpcd(


Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>

and pushed to amd-staging-drm-next.

Thanks
Siqueira