Message-ID: <dadbeb6c-de4b-1028-c651-4ff96614671c@amd.com>
Date:   Mon, 20 Mar 2023 16:21:55 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v2] 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:     Leo Li <sunpeng.li@amd.com>,
        Rodrigo Siqueira <Rodrigo.Siqueira@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>,
        Wenjing Liu <wenjing.liu@amd.com>,
        Sung Joon Kim <sungjoon.kim@amd.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20230310184233.284388-1-hamza.mahfooz@amd.com>
From:   Harry Wentland <harry.wentland@amd.com>
In-Reply-To: <20230310184233.284388-1-hamza.mahfooz@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eDZkSjJSUHFMZFp6bGpXc2EvTS91d1NhelVvVzM4d0JkK3d0d1BYN3pwdWw0?=
 =?utf-8?B?S1dLQWFIL3ZCSUpoTTJudDgvQWxoTkFzOFdFZG94QUNQY1pBdVRLWEd4Rll3?=
 =?utf-8?B?VExvcm01UENKeWtKNzQwT1V0NWNSOFNPc0Q2ZUlva0JybW5vcG00clVGWVFM?=
 =?utf-8?B?d09SRS80b0NHWHUyZ0J2S2oyd2M3QnRUUUxvcHZYY2gvS2g5bGRhb1ZvM1A5?=
 =?utf-8?B?ekI1Q09MNWxSUFNheE5pNkw1SzdXR2tqclpiQWdrWkhWdWNiYnplRlNFRnpF?=
 =?utf-8?B?cnJXVDM1aGhvTkRsWUZac2ZXVjh3RzRZOEdBcmpmSFAxOHJ4cWFvQ0Zpc3JR?=
 =?utf-8?B?QWNVUGU2N3RKNGlManVKM1ZHSUM3L2REVXFFZU5HVTNTakdwTmNDam9NcGNQ?=
 =?utf-8?B?Mmh4bkFjRE9Ub3doc3ZuQjVScFBvMlFxNEdna29BckYrVVdId3cvMlQzbGtJ?=
 =?utf-8?B?NWxjcGRxSUVEMzRPcGVjQ1E0TFhiWStVN05haHMrN2dXSmxsTG4zN0Fzcmp2?=
 =?utf-8?B?R0JIdi9sOTBDZC8wVXZPU2E3VDRJa1ErLzhYMlBQV2ZkZTlSZFBhQ3Y1QWNX?=
 =?utf-8?B?MmFkNENFTGVsYzJsQXY1bi9icEs3djVHZm5Kc3hKMWsxTnd1ZkIzT2ZOSUU5?=
 =?utf-8?B?MEFpZnFuaVA4N0tGalBnQlNrcTZmU3FnME02MGVVVE9tVW1wWGlDZVI3eEhm?=
 =?utf-8?B?eVJmSGtoYVk5QlVCcDdMRkhnMjd1UzRRbUcwYVZDeGpEc1NsMEhrTUl0dG5X?=
 =?utf-8?B?NmxweGQ2N0FQR0lORnBqQ0lJWWovejhjOTRLRjJ6MXQ2amUrZ01qUG9WYVRL?=
 =?utf-8?B?TlZhS0NMQmp3SktzeXJUSU5vRXV1aGtaNHZ5emdidGtoMEdlRnRoNElTaHNY?=
 =?utf-8?B?cDVUckRwS3ZvOXNGLzVhRTRoVXlQTFdlQW1CYkp0YzdSUHI1c0tzWlZtckpk?=
 =?utf-8?B?VFRSaHhKdXUrZ2xheFNFZ0RZa3dJRE1vMkxjamUrZUNuQlZNQmFGTHZMclp5?=
 =?utf-8?B?MS9tR2lsbFozZ0xJSWdhUE5DdWpkTGxxeTZQcUo3cWEranl3U2xxUDdacXlz?=
 =?utf-8?B?YnJIYzIvdHkzWWIwb3pNRkcvR3NjUGFQbnBScFl1TG5vcXVMbDk0ZFQ1Mjds?=
 =?utf-8?B?WVQxU0tMS0NEby9ybW45RnJORmpGcWxobTZPMWVBMm1Oc1hvRW5NYnA0L0Nt?=
 =?utf-8?B?RnVNMS92b0hwVVRNWU96aEFZSFV6WmRwUmlVUHZpS1I0dERHYzlDaWNsVjlo?=
 =?utf-8?B?VkZUYXA1KzFibituQ0JuV3UwVk14UGNITCtwYWN5Q1VxanFweklYcUlSMXJH?=
 =?utf-8?B?SWtoN3JRR0JZOHNmOFBlR2pVOU15M1BqcWxZMnJVcE1qd0c0aEZwL0d5TXpZ?=
 =?utf-8?B?VUdZQ2djRGVhME81UnFVMVdLTGFJWFhQSmd5MWNOSlo3RzN1Um0rL1MwOEZN?=
 =?utf-8?B?V29FUm5sRmtMUHlOUmF0WWtBUXVkUlE5dkV0ZGI1aHdwNWRKMndKMmkxaitK?=
 =?utf-8?B?YnlTY3A3OVJvc1VxZ3BHM3BsNXNQVWppSXAxd2ZxOEN4enhwa0ZQb1RNL29W?=
 =?utf-8?B?TzJpOGEyeFJkOHVDK3NWQkFYcSthdm0waWUyUjhJNnFUODdQV2hka2ZxdFVG?=
 =?utf-8?B?THF0UFNIRkxxSUIwUE91bUJ6K2ZzbXZoS2NSTFk2dGxWazZLWWxNVUN0clUx?=
 =?utf-8?B?KzhLd2dWenFmZWI5VXlvdHE0ZElMOXg2bCtQQ0tLSzFYdXJBcmd2dmlTc3NV?=
 =?utf-8?B?WkNCM2djek5nNlRlRnFJKzRsemdKS1pPK1pmQ0g3YnRrR3NPdzBIT2l4ZXZn?=
 =?utf-8?B?WG5TTGZhRGdtKzA2ZjAvbUIxL1NVYy85TjFiVDZQeDFCVlYzdWJqemxmejBh?=
 =?utf-8?B?NHNIelRYSm03cG9IN1RCSW1KbHVESzkvNWhiRjlNVFRnQjd4VnM4cmluMU9n?=
 =?utf-8?B?K0tKbVZxclZ4Vk8rWG9tZVhCc3N1QzhwSWNPcXpJc0QxZ1dvdUNWdytHSlBn?=
 =?utf-8?B?d203amxpRWxHbVIvdEVsZVk2UXMyNXpwaldRRmJxM1VVZ0tKWURPS2xHR21p?=
 =?utf-8?B?UDRTL0ptK1BSYmZJK0NxdHd0dDRROEJ3Z1hXMm1jblNIcUE3VmlKZTB0NkNP?=
 =?utf-8?Q?8XtYjwsn7pIX8s7icQd6ZSl2C?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4ec748d5-0009-4a97-fe7f-08db2980c279
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5427.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Mar 2023 20:22:00.0897
 (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: XETuhE0LRNMMLpeBHzOUilLAePXy52hLSn4h/YfDew5FyfgK9V+sO+toWI2seBLC3I60RhKYmauJjB4jssPkEA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB8151
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/10/23 13:42, 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.
> So, compare the return value of drm_dp_dpcd_read() to the requested
> read size.
> 
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>

I don't think we can ever hit a case where the return value is greater
than 0 but not equal to size as drm_dp_dpcd_access already checks for it.

Either way, the stricter check makes sense and is
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
> v2: drop the WARN_ON().
> ---
>  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..9c1e91c2179e 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 drm_dp_dpcd_read(&aconnector->dm_dp_aux.aux, address, data,
> +				size) == size;
>  }
>  
>  bool dm_helpers_dp_write_dpcd(