[PATCH] drm/amd/display: rewrite the check for mods

[Jiasheng Jiang]

On Thu, 17 Nov 2022 15:56:09 +0800, Simon Ser wrote:
>> @@ -638,11 +638,14 @@ static int get_plane_modifiers(struct amdgpu_device 
>> *adev, unsigned int plane_ty
>>  return 0;
>>  
>>  *mods = kmalloc(capacity * sizeof(uint64_t), GFP_KERNEL);
>> +if (!*mods)
>> +return -ENOMEM;
>> +
>>  
>>  if (plane_type == DRM_PLANE_TYPE_CURSOR) {
>>  add_modifier(mods, , , DRM_FORMAT_MOD_LINEAR);
>>  add_modifier(mods, , , DRM_FORMAT_MOD_INVALID);
>> -return *mods ? 0 : -ENOMEM;
>> +return 0;
>>  }
>>  
>>  switch (adev->family) {
>> @@ -671,9 +674,6 @@ static int get_plane_modifiers(struct amdgpu_device 
>> *adev, unsigned int plane_ty
>>  /* INVALID marks the end of the list. */
>>  add_modifier(mods, , , DRM_FORMAT_MOD_INVALID);
>>  
>> -if (!*mods)
>> -return -ENOMEM;
>> -
>>  return 0;
>>  }
 
> This breaks the "size" out-parameter.

No, it will not change the value of the "size".
The "size" can only be modified by add_modifier().
However, when the "*mods" is NULL, add_modifier() will return immediately,
without the execution of "*size += 1;".
Therefore, when the "*mods" is NULL, the rest of the function is useless,
which should be better to skip.

Jiang

Re: [PATCH] drm/amd/display: rewrite the check for mods

[Simon Ser]

This breaks the "size" out-parameter.

[PATCH] drm/amd/display: rewrite the check for mods

[Jiasheng Jiang]

When the *mods is NULL, it should be better to return error immediately,
rather than continue with redundant operations.

Signed-off-by: Jiasheng Jiang 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index e6854f7270a6..05efc76b2226 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -638,11 +638,14 @@ static int get_plane_modifiers(struct amdgpu_device 
*adev, unsigned int plane_ty
return 0;
 
*mods = kmalloc(capacity * sizeof(uint64_t), GFP_KERNEL);
+   if (!*mods)
+   return -ENOMEM;
+
 
if (plane_type == DRM_PLANE_TYPE_CURSOR) {
add_modifier(mods, , , DRM_FORMAT_MOD_LINEAR);
add_modifier(mods, , , DRM_FORMAT_MOD_INVALID);
-   return *mods ? 0 : -ENOMEM;
+   return 0;
}
 
switch (adev->family) {
@@ -671,9 +674,6 @@ static int get_plane_modifiers(struct amdgpu_device *adev, 
unsigned int plane_ty
/* INVALID marks the end of the list. */
add_modifier(mods, , , DRM_FORMAT_MOD_INVALID);
 
-   if (!*mods)
-   return -ENOMEM;
-
return 0;
 }
 
-- 
2.25.1