Received-SPF: pass (google.com: domain of linux-kernel+bounces-97577-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <0255d007-6929-4366-b47e-0826e13b86e8@amd.com>
Date: Fri, 8 Mar 2024 15:57:00 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/7] drm: Fix drm_fixp2int_round() making it add 0.5
Content-Language: en-US
To: Arthur Grillo <arthurgrillo@riseup.net>,
 Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>,
 Melissa Wen <melissa.srw@gmail.com>, =?UTF-8?Q?Ma=C3=ADra_Canal?=
 <mairacanal@riseup.net>, Haneen Mohammed <hamohammed.sa@gmail.com>,
 Daniel Vetter <daniel@ffwll.ch>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>,
 David Airlie <airlied@gmail.com>, Jonathan Corbet <corbet@lwn.net>,
 pekka.paalanen@haloniitty.fi, Louis Chauvet <louis.chauvet@bootlin.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
 jeremie.dautheribes@bootlin.com, miquel.raynal@bootlin.com,
 thomas.petazzoni@bootlin.com, seanpaul@google.com, marcheu@google.com,
 nicolejadeyee@google.com, Pekka Paalanen <pekka.paalanen@collabora.com>
References: <20240306-louis-vkms-conv-v1-0-5bfe7d129fdd@riseup.net>
 <20240306-louis-vkms-conv-v1-1-5bfe7d129fdd@riseup.net>
From: Harry Wentland <harry.wentland@amd.com>
In-Reply-To: <20240306-louis-vkms-conv-v1-1-5bfe7d129fdd@riseup.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?SEVsWTdZbjdUaG1ZanpNNEUyZjdlTU9NUGNISjJzS3FjNVVITDdDa1FvemRy?=
 =?utf-8?B?T1ZBTjZDY2dQR0pGczhzU3FZMHM4bFpWd0VxQUVlUUV3MWdITTF4K0VYK1Fw?=
 =?utf-8?B?RWZKWC92RFNpMUZkM3ZOdS9hT2ZNaGdVMkgvUERTRkFzdWxML2plczVDdkNu?=
 =?utf-8?B?bHZRclVXN2JuWWVESFNhbC90VTVlTlZYeTlaZlVoMkVLT0x6S1hyNHFxTURK?=
 =?utf-8?B?cDI2NnljZG5JeVl0dGhOUmMxL3dpV3RaVGVLbFdCUVVhMERJRTJ6MVpXdUtz?=
 =?utf-8?B?S3JCWGxsOUZ2VGg4enh2TkdHVllKVzlXYUxNcUgzblQ4RkNWMU11ZjNnVldS?=
 =?utf-8?B?OHBMUXhuVklrV1hQcFFKcy91VGJ3V0IrZ2xnc01Ud1JTMUwwRWZZMW1Wejl6?=
 =?utf-8?B?NjhhWnl1VER4STd2aERPVzlYYWtiM3ozNkVzRzdjeVh6VlcyK1hoTS9Yb2lY?=
 =?utf-8?B?SGFvSmhGVitsQjRVdXJva2VYUDZjS2psMG9zbm4xbk9xc0JWOVBsbTRnMkNB?=
 =?utf-8?B?SzhHZzl5N0tMc0xnY01lNmRrcEc2b3ZxY3pmbHRVckRHY0p0bEx3ejN1eFZx?=
 =?utf-8?B?c2w3OTJXbzc5UzRncThiVGF2UzB3a0JWZmMzcWsvd01pekRVU2orQVFPYkJE?=
 =?utf-8?B?ZVZJQi9XRFdtcDQwbHdvcWxoM3lhM2RBREZzYmNuaC9CM0ZTcXlMMjNLY2dw?=
 =?utf-8?B?bDZTQ0UrZGFIZVZEY1BuOWFFQVVqY3kyb2xONzk0TVJUZkJReTMraCtmcUg0?=
 =?utf-8?B?eVFlOHcwSEt4bDliSERxclRaQjBibzJnQWMzSGZVMXh4bkhyR1JyZGVPSVhu?=
 =?utf-8?B?cjV1eHdmM2Z5VGo2bG5ldFZDT3RNekJaaEcrdEZYZkZ6a0h5RFFPYnpWMTl0?=
 =?utf-8?B?aEc2MHA4Y3BiQm1KOHB5NUxMQy9RbkdVRGJtcFVsOEplMkl6SGRyRjRXSGo5?=
 =?utf-8?B?ZjIxcnhZSGpoWUlGMndqNFF4T3QweVR1RktJTkF1QUNaV2lpK1k5YUNWMWsr?=
 =?utf-8?B?dThyKzNrUzl3bmt6cHJkRUw3NWNLc0RHbmlhU1VpMXZVRHFWY0h6ZUVHUUVL?=
 =?utf-8?B?aFBack1pT2FEZ2oyRzFLWFM3SGhmK1JQM1l6MFBKakhjemRoWnJrSjZjSEFJ?=
 =?utf-8?B?a1Yyb3dXbytpcmVPNWxZT2pqVWxMaGtBdWFFbEhWSW14cmpQckFnS05IN3I1?=
 =?utf-8?B?cjVkb3dFcU1CWHdOV0ZRYkdqK2hyVkVsNy8xRnZYNHJVaXZrekJiZ0VMOFU5?=
 =?utf-8?B?Rzh5WGxTZk1HRDYzdkJvNVZITkNCSDE5bjZBNFZCT09ZOHltOUhaV3dneGNi?=
 =?utf-8?B?a3FJUjVGeDRLZTB2dEUzaFRYb2dySWd3aGxHZ3lFU2Jhc2xaeHRVYVhjSG9Y?=
 =?utf-8?B?UnNmL2ZLb2xNMmNBOFcwMmlldVlhN3VGcDF2clNrcDRDdzBQQVY2dWluZUh3?=
 =?utf-8?B?TUZPVnBKbEl0RFp6eXNOdWRHTjVIWGpEWWkzRWxKUEVyZW9KL3k2TDBmZ0lI?=
 =?utf-8?B?MVo1dTJjcVJHSDZna2RrVFhPRzRXU2JIbkhWN1BQZWhJVDZmMXVKOGQ1NzJs?=
 =?utf-8?B?VmJwcjI1OXBsLzI5ZGc0bjd2NGVPc2loYlZXRXFKUXBnTitBeVVJVWYzQ1dI?=
 =?utf-8?B?Y3g2bWZJbTJJMHZ1MWdxUG1ZOWhVM1ZadU5ZTnpMeGVjSVhJWGdzOU9Md1Y4?=
 =?utf-8?B?aEhxSDBBSGMrM0tCZGtobDBHRkV1V0xlV1pDWVR3TDd5bDFYc3lIRlVYc2Rw?=
 =?utf-8?B?OE1NdXpMelhVejhXaVNablJrYnRBVTFvaDU4cVVTR2I2WmduOHRkZHE2RzB2?=
 =?utf-8?B?ZFlMYlJJcjNZOFlwU1FGeituYkZCc2ZHeW1OaDdyeG1QSXFJNGdza1lrRmFn?=
 =?utf-8?B?Y0lreTVxRkFKT01DSG1qZFg3TjhzdVBzWjl2WXFjQ280MTh2WmtHNVM2c0gw?=
 =?utf-8?B?ZkxVK0d1aEQ0RUh4Y054NjcwcTBQTXZSeFR4S0dyanpjbTdwNEVlTUlOdHBa?=
 =?utf-8?B?L1RISnh4T3NuYjFXa1BPaU5uM21oUE96NDNialNpNDl3VExYQi9PL284M0E4?=
 =?utf-8?B?eGJHVUovWXltSjFCWlA0M2VnRGpZTFp6Y0c3L3lLMldXQXU5KzcyRHhEaGtJ?=
 =?utf-8?Q?Ds1sYCAwXPyWz392ncqWPebD/?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ad1ec340-bd88-44a2-699c-08dc3fb25012
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5427.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2024 20:57:06.1575
 (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: QemGvLDKP8dmvgQibB2AmL1+AJsFIH2SufWVz6gHY/0zCGL/05nPQTxCrwSKBHqdbbsXTJwJUERtdBdQcwGJEg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4373

On 2024-03-06 15:03, Arthur Grillo wrote:
> As well noted by Pekka[1], the rounding of drm_fixp2int_round is wrong.
> To round a number, you need to add 0.5 to the number and floor that,
> drm_fixp2int_round() is adding 0.0000076. Make it add 0.5.
> 
> [1]: https://lore.kernel.org/all/20240301135327.22efe0dd.pekka.paalanen@collabora.com/
> 
> Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> Signed-off-by: Arthur Grillo <arthurgrillo@riseup.net>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

I had a different jab at this [1], but your patch is cleaner.

https://patchwork.freedesktop.org/patch/579978/?series=123446&rev=4

Harry

> ---
>   include/drm/drm_fixed.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_fixed.h b/include/drm/drm_fixed.h
> index 0c9f917a4d4b..de3a79909ac9 100644
> --- a/include/drm/drm_fixed.h
> +++ b/include/drm/drm_fixed.h
> @@ -90,7 +90,7 @@ static inline int drm_fixp2int(s64 a)
>   
>   static inline int drm_fixp2int_round(s64 a)
>   {
> -	return drm_fixp2int(a + (1 << (DRM_FIXED_POINT_HALF - 1)));
> +	return drm_fixp2int(a + DRM_FIXED_ONE / 2);
>   }
>   
>   static inline int drm_fixp2int_ceil(s64 a)
>