diff mbox series

[v5,1/9] drm: vkms: Alloc the compose frame using vzalloc

Message ID 20220404204515.42144-2-igormtorrente@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add new formats support to vkms | expand

Commit Message

Igor Matheus Andrade Torrente April 4, 2022, 8:45 p.m. UTC
Currently, the memory to the composition frame is being allocated using
the kzmalloc. This comes with the limitation of maximum size of one
page size(which in the x86_64 is 4Kb and 4MB for default and hugepage
respectively).

Somes test of igt (e.g. kms_plane@pixel-format) uses more than 4MB when
testing some pixel formats like ARGB16161616 and the following error were
showing up when running kms_plane@plane-panning-bottom-right*:

[drm:vkms_composer_worker [vkms]] *ERROR* Cannot allocate memory for
output frame.

This problem is addessed by allocating the memory using kvzalloc that
circunvents this limitation.

V5: Improve the commit message and drop the debugging issues in VKMS
TO-DO(Melissa Wen).

Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Igor Torrente <igormtorrente@gmail.com>
---
 Documentation/gpu/vkms.rst           | 6 ------
 drivers/gpu/drm/vkms/vkms_composer.c | 6 +++---
 2 files changed, 3 insertions(+), 9 deletions(-)

Comments

André Almeida April 5, 2022, 2:05 p.m. UTC | #1
Hi Igor,

Thanks for your patch!

Às 17:45 de 04/04/22, Igor Torrente escreveu:
> Currently, the memory to the composition frame is being allocated using
> the kzmalloc. This comes with the limitation of maximum size of one
> page size(which in the x86_64 is 4Kb and 4MB for default and hugepage
> respectively).
>
> Somes test of igt (e.g. kms_plane@pixel-format) uses more than 4MB when
> testing some pixel formats like ARGB16161616 and the following error were
> showing up when running kms_plane@plane-panning-bottom-right*:
>
> [drm:vkms_composer_worker [vkms]] *ERROR* Cannot allocate memory for
> output frame.
>
> This problem is addessed by allocating the memory using kvzalloc that

addessed -> addressed

OTOH, I would write this in imperative mood, as in "Address this by
allocating..." or "Fix this..."

> circunvents this limitation.

circunvents -> circumvents

>
> V5: Improve the commit message and drop the debugging issues in VKMS
> TO-DO(Melissa Wen).
>

Patch changelog are very useful for the mailing list, but not very
useful for the git log. For that reason, I usually put this right after
the --- in the patch, so the log will be dropped when the patch is applied.

Those comment applies for the rest of your series.

> Reviewed-by: Melissa Wen <mwen@igalia.com>
> Signed-off-by: Igor Torrente <igormtorrente@gmail.com>
> ---
Igor Matheus Andrade Torrente April 5, 2022, 7:03 p.m. UTC | #2
Hi André,

On 4/5/22 11:05, André Almeida wrote:
> Hi Igor,
> 
> Thanks for your patch!
> 
> Às 17:45 de 04/04/22, Igor Torrente escreveu:
>> Currently, the memory to the composition frame is being allocated using
>> the kzmalloc. This comes with the limitation of maximum size of one
>> page size(which in the x86_64 is 4Kb and 4MB for default and hugepage
>> respectively).
>>
>> Somes test of igt (e.g. kms_plane@pixel-format) uses more than 4MB when
>> testing some pixel formats like ARGB16161616 and the following error were
>> showing up when running kms_plane@plane-panning-bottom-right*:
>>
>> [drm:vkms_composer_worker [vkms]] *ERROR* Cannot allocate memory for
>> output frame.
>>
>> This problem is addessed by allocating the memory using kvzalloc that
> 
> addessed -> addressed
> 
> OTOH, I would write this in imperative mood, as in "Address this by
> allocating..." or "Fix this..."
> 
>> circunvents this limitation.
> 
> circunvents -> circumvents

Thanks, I will fix them!

> 
>>
>> V5: Improve the commit message and drop the debugging issues in VKMS
>> TO-DO(Melissa Wen).
>>
> 
> Patch changelog are very useful for the mailing list, but not very
> useful for the git log. For that reason, I usually put this right after
> the --- in the patch, so the log will be dropped when the patch is applied.
> 
> Those comment applies for the rest of your series.

Well, drivers in the DRM subsystem maintain the change history. As you
can see in the commit below.

4db3189ce0621be901f249f8cd8226c977dd601d
d80976d9ffd9d7f89a26134a299b236910477f3b
84ec374bd580364a32818c9fc269c19d6e931cab
50fff206c5e3a04fcb239ad58d89cad166711b7f

Aside from that, the current VKMS maintainer asked me to add them to the
commit body.

And for that two reasons, I will keep them.

Thanks!
---
Igor Torrente

> 
>> Reviewed-by: Melissa Wen <mwen@igalia.com>
>> Signed-off-by: Igor Torrente <igormtorrente@gmail.com>
>> ---
diff mbox series

Patch

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index 9c873c3912cc..973e2d43108b 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -102,12 +102,6 @@  Debugging:
 
 - kms_plane: some test cases are failing due to timeout on capturing CRC;
 
-- kms_flip: when running test cases in sequence, some successful individual
-  test cases are failing randomly; when individually, some successful test
-  cases display in the log the following error::
-
-  [drm:vkms_prepare_fb [vkms]] ERROR vmap failed: -4
-
 Virtual hardware (vblank-less) mode:
 
 - VKMS already has support for vblanks simulated via hrtimers, which can be
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 9e8204be9a14..82f79e508f81 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -180,7 +180,7 @@  static int compose_active_planes(void **vaddr_out,
 	int i;
 
 	if (!*vaddr_out) {
-		*vaddr_out = kzalloc(gem_obj->size, GFP_KERNEL);
+		*vaddr_out = kvzalloc(gem_obj->size, GFP_KERNEL);
 		if (!*vaddr_out) {
 			DRM_ERROR("Cannot allocate memory for output frame.");
 			return -ENOMEM;
@@ -263,7 +263,7 @@  void vkms_composer_worker(struct work_struct *work)
 				    crtc_state);
 	if (ret) {
 		if (ret == -EINVAL && !wb_pending)
-			kfree(vaddr_out);
+			kvfree(vaddr_out);
 		return;
 	}
 
@@ -275,7 +275,7 @@  void vkms_composer_worker(struct work_struct *work)
 		crtc_state->wb_pending = false;
 		spin_unlock_irq(&out->composer_lock);
 	} else {
-		kfree(vaddr_out);
+		kvfree(vaddr_out);
 	}
 
 	/*