Re: [PATCH] MAINTAINERS: Add myself as VKMS Maintainer
On 9/11/24 10:01, Maxime Ripard wrote: On Wed, Sep 11, 2024 at 02:15:05PM GMT, Louis Chauvet wrote: Le 10/09/24 - 15:57, Maira Canal a écrit : On 9/10/24 12:10, Louis Chauvet wrote: I've been actively working on VKMS to provide new features and participated in reviews and testing. To help Maìra with her work, add myself as co-maintainer of VKMS. Signed-off-by: Louis Chauvet Acked-by: Maíra Canal Please, check the procedures to apply for commit rights in drm-misc and apply it. This way you will be able to commit your patches. Thanks for your support! I just checked the rules to become a commiter, and it requires at least 10 non-trivial patches, so I can't apply right now. As far as I'm concerned, being a maintainer of a driver in drm-misc gives you automatically that right :) +1 Best Regards, - Maíra Maxime
Re: [PATCH] MAINTAINERS: remove myself as a VKMS maintainer
On 9/11/24 10:50, Rodrigo Siqueira wrote: I haven't been able to follow or review the work on the driver for a long time and I don't see the situation improving anytime soon. Hence, this commit removes me from the maintainers list. Signed-off-by: Rodrigo Siqueira Acked-by: Maíra Canal Thanks for all your work on VKMS, Rodrigo! Would you mind applying this patch on drm-misc? I'm a bit on a hurry this week. Best Regards, - Maíra --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 333ed0718175..1e6356a1b6c7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7346,7 +7346,6 @@ T:git https://gitlab.freedesktop.org/drm/misc/kernel.git F:drivers/gpu/drm/udl/ DRM DRIVER FOR VIRTUAL KERNEL MODESETTING (VKMS) -M: Rodrigo Siqueira M:Maíra Canal R:Haneen Mohammed R:Simona Vetter
Re: [PATCH] MAINTAINERS: Add myself as VKMS Maintainer
On 9/10/24 12:10, Louis Chauvet wrote: I've been actively working on VKMS to provide new features and participated in reviews and testing. To help Maìra with her work, add myself as co-maintainer of VKMS. Signed-off-by: Louis Chauvet Acked-by: Maíra Canal Please, check the procedures to apply for commit rights in drm-misc and apply it. This way you will be able to commit your patches. Thanks for volunteering! Best Regards, - Maíra --- Hi everyone, This series [1] has been waiting for a while now, it was proposed first in February. The first iterations had few reactions (thanks to Arthur, Pekka, Maìra, ...), but since v8 (in May) no major issues were reported, Maìra seemed satisfied, and only minor cosmetic changes were reported. Two other series ([2] and [3]), that I submitted first in May, did not have receive any reactions. In addition, there is also some significant addition to VKMS being proposed, such as ConfigFS support, and without a clear maintainer having the time to review and approve these changes, these changes have very little changes to get in. VKMS is not a fundamental driver for "normal" Linux users, but I had some feedback from userspace developpers that VKMS could be a very good testing tool if only it had more features (I think P0xx formats were asked to test HDR for example). This could also help to detect issues in DRM core by emulating a wide range of configurations. I believe the only active maintainer is Maìra, but as she's mentioned before, she doesn't have much time to work on VKMS. So, I'd like to volunteer as a maintainer. I have plenty of time to dedicate to VKMS. I hope I've gained enough understanding of VKMS to be helful with this role. I am eager to move forward and improve this subsystem. I've also talked to Sean about this, and he agrees that it would be good if I could commit code to drm-misc. [1]: https://lore.kernel.org/all/20240809-yuv-v10-0-1a7c76416...@bootlin.com/ [2]: https://lore.kernel.org/all/20240814-b4-new-color-formats-v2-0-8b3499cfe...@bootlin.com/ [3]: https://lore.kernel.org/all/20240814-writeback_line_by_line-v2-0-36541c717...@bootlin.com/ --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 10430778c998b57944c1a6c5f07d676127e47faa..62f10356e11ab7fa9c8f79ba63b335eb6580d0a8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7340,6 +7340,7 @@ DRM DRIVER FOR VIRTUAL KERNEL MODESETTING (VKMS) M:Rodrigo Siqueira M:Melissa Wen M:Maíra Canal +M: Louis Chauvet R:Haneen Mohammed R:Daniel Vetter L:dri-devel@lists.freedesktop.org --- base-commit: da3ea35007d0af457a0afc87e84fddaebc4e0b63 change-id: 20240910-vkms-maintainer-7b3d2210cc1b Best regards,
Re: [PATCH v8 00/17] drm/vkms: Reimplement line-per-line pixel conversion for plane reading
Hi Louis, On 7/1/24 10:06, Louis Chauvet wrote: Hi everyone, I sent this iteration over a month ago, and I haven't received any feedback since then. The same goes for the two other series [1] and [2], which I sent after discussing with Melissa. As you may know, Melissa is stepping down on her maintainership duties for VKMS [1]. I'm a bit surprised that nothing has been reviewed or merged, as Maíra mentioned in [3] that she wanted to merge at least the first 11 patches. I just checked, and this series applies to drm-misc-next. However, if you encounter any issues, I can send a rebased version. I want to get the series merged, but, as me and Melissa pointed out last XDC, we are volunteers. AFAIK VKMS has been maintained by volunteers since its beginning. We are doing our best to maintain VKMS, but we do it in our free time. I need to take some time to review and test this series properly. Then, I can push it to drm-misc-next. As you can see, I have more series ready ([2], [3]), and I am working on additional features (configfs [4], variable refresh rate, mst...). However, I am currently waiting for feedback on this series before proceeding further with the next topics. What should I do to move those series forward? I appreciate the work you are doing for VKMS. But, apart from adding new features, we first need to ensure that VKMS is stable. Also, when reviewing new features, I need to make even more tests to make sure that everything is still stable. Therefore, it is a slow process, but I hope we can start to move forward. [1] https://lore.kernel.org/dri-devel/20240525142637.82586-1-melissa@gmail.com/ Best Regards, - Maíra Best regards, Louis Chauvet [1]: https://lore.kernel.org/all/20240516-b4-new-color-formats-v1-0-74cf9fe07...@bootlin.com/ [2]: https://lore.kernel.org/all/20240516-writeback_line_by_line-v1-0-7b2e3bf9f...@bootlin.com/ [3]: https://lore.kernel.org/all/c83255f4-745e-43e6-98e0-2e89c31d5...@igalia.com/ [4]: https://github.com/Fomys/linux/tree/b4/new-configfs
Re: [PATCH v2 0/9] drm/vkms: Reimplement line-per-line pixel conversion for plane reading
Hi Louis,
On 2/23/24 08:37, Louis Chauvet wrote:
This patchset is the second version of [1]. It is almost a complete
rewrite to use a line-by-line algorithm for the composition.
It can be divided in three parts:
- PATCH 1 to 4: no functional change is intended, only some formatting and
documenting
(PATCH 2 is taken from [2])
- PATCH 5: main patch for this series, it reintroduce the
line-by-line algorithm
- PATCH 6 to 9: taken from Arthur's series [2], with sometimes adaptation
to use the pixel-by-pixel algorithm.
The PATCH 5 aims to restore the line-by-line pixel reading algorithm. It
was introduced in 8ba1648567e2 ("drm: vkms: Refactor the plane composer to
accept new formats") but removed in 8ba1648567e2 ("drm: vkms: Refactor the
plane composer to accept new formats") in a over-simplification effort.
At this time, nobody noticed the performance impact of this commit. After
the first iteration of my series, poeple notice performance impact, and it
was the case. Pekka suggested to reimplement the line-by-line algorithm.
Expiriments on my side shown great improvement for the line-by-line
algorithm, and the performances are the same as the original line-by-line
algorithm. I targeted my effort to make the code working for all the
rotations and translations. The usage of helpers from drm_rect_* avoid
reimplementing existing logic.
The only "complex" part remaining is the clipping of the coordinate to
avoid reading/writing outside of src/dst. Thus I added a lot of comments
to help when someone will want to add some features (framebuffer resizing
for example).
The YUV part is not mandatory for this series, but as my first effort was
to help the integration of YUV, I decided to rebase Arthur's series on
mine to help. I took [3], [4], [5] and [6] and adapted them to use the
line-by-line reading. If I did something wrong here, please let me
know.
My series was mainly tested with:
- kms_plane (for color conversions)
- kms_rotation_crc (for rotations of planes)
- kms_cursor_crc (for translations)
The benchmark used to measure the improvment was done with:
- kms_fb_stress
[1]: https://lore.kernel.org/r/20240201-yuv-v1-0-3ca376f27...@bootlin.com
[2]: https://lore.kernel.org/all/20240110-vkms-yuv-v2-0-952fcaa5a...@riseup.net/
[3]: https://lore.kernel.org/all/20240110-vkms-yuv-v2-3-952fcaa5a...@riseup.net/
[4]: https://lore.kernel.org/all/20240110-vkms-yuv-v2-5-952fcaa5a...@riseup.net/
[5]: https://lore.kernel.org/all/20240110-vkms-yuv-v2-6-952fcaa5a...@riseup.net/
[6]: https://lore.kernel.org/all/20240110-vkms-yuv-v2-7-952fcaa5a...@riseup.net/
To: Rodrigo Siqueira
To: Melissa Wen
To: Maíra Canal
To: Haneen Mohammed
To: Daniel Vetter
To: Maarten Lankhorst
To: Maxime Ripard
To: Thomas Zimmermann
To: David Airlie
To: arthurgri...@riseup.net
To: Jonathan Corbet
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Cc: jeremie.dautheri...@bootlin.com
Cc: miquel.ray...@bootlin.com
Cc: thomas.petazz...@bootlin.com
Signed-off-by: Louis Chauvet
Note: after my changes, those tests seems to pass, so [7] may need
updating (I did not check, it was maybe already the case):
- kms_cursor_legacy@flip-vs-cursor-atomic
- kms_pipe_crc_basic@nonblocking-crc
- kms_pipe_crc_basic@nonblocking-crc-frame-sequence
- kms_writeback@writeback-pixel-formats
- kms_writeback@writeback-invalid-parameters
- kms_flip@flip-vs-absolute-wf_vblank-interruptible
And those tests pass, I did not investigate why the runners fails:
- kms_flip@flip-vs-expired-vblank-interruptible
- kms_flip@flip-vs-expired-vblank
- kms_flip@plain-flip-fb-recreate
- kms_flip@plain-flip-fb-recreate-interruptible
- kms_flip@plain-flip-ts-check-interruptible
- kms_cursor_legacy@cursorA-vs-flipA-toggle
- kms_pipe_crc_basic@nonblocking-crc
- kms_prop_blob@invalid-get-prop
- kms_flip@flip-vs-absolute-wf_vblank-interruptible
- kms_invalid_mode@zero-hdisplay
- kms_invalid_mode@bad-vtotal
- kms_cursor_crc.* (everything is SUCCEED or SKIP, but no fails)
This is great news! Could you just adjust the series fixing the
compiling errors?
Best Regards,
- Maíra
[7]:
https://lore.kernel.org/all/20240201065346.801038-1-vignesh.ra...@collabora.com/
Changes in v2:
- Rebased the series on top of drm-misc/drm-misc-net
- Extract the typedef for pixel_read/pixel_write
- Introduce the line-by-line algorithm per pixel format
- Add some documentation for existing and new code
- Port the series [1] to use line-by-line algorithm
- Link to v1:
https://lore.kernel.org/r/20240201-yuv-v1-0-3ca376f27...@bootlin.com
---
Arthur Grillo (5):
drm/vkms: Use drm_frame directly
drm/vkms: Add YUV support
drm/vkms: Add range and encoding properties to pixel_read function
drm/vkms: Drop YUV formats TODO
drm/vkms: Create KUnit tests for YUV conversions
Louis Chauvet (4):
drm/vkms: Code formatting
drm/vkms: write/update the documentation for pixel conversion and pixel
write functions
drm/vkms: Add typedef and documentation fo
Re: [PATCH 0/2] Better support for complex pixel formats
Hi Louis, Thanks for your patches! Could you please rebase them on top of drm-misc-next? It would make it easier for me to review and test the patches. Best Regards, - Maíra On 2/1/24 14:31, Louis Chauvet wrote: This patchset aims to solve issues I found in [1], and at the same time simplify the composition algorithm. I sent more igt-gpu-tools test [2] to cover more things and detect the issues in [1]. This patchset is based on [1]. Patch 1/2: This patch is a no-op, but make the code more readable regarding the pixel_read functions. Patch 2/2: This patch is more complex. My main target was to solve issues I found in [1], but as it was very complex to do it "in place", I choose to rework the composition function. The main two advantages are: - It's now possible to create conversion function for packed & grouped pixels. Some pixel formats need absolute x/y position and not only an offset in the buffer to extract the correct value. This part also solve the issues I found in [1]. - The rotation management is now way easier to understand, there is no more switch case in different places and instead of copy/pasting rotation formula I used drm_rect_* helpers. [1]: https://lore.kernel.org/dri-devel/20240110-vkms-yuv-v2-0-952fcaa5a...@riseup.net/ [2]: https://lore.kernel.org/igt-dev/20240201-kms_tests-v1-0-bc34c5d28...@bootlin.com/T/#t Signed-off-by: Louis Chauvet --- Louis Chauvet (2): drm/vkms: Create a type to check a function pointer validity drm/vkms: Use a simpler composition function drivers/gpu/drm/vkms/vkms_composer.c | 97 - drivers/gpu/drm/vkms/vkms_drv.h | 32 - drivers/gpu/drm/vkms/vkms_formats.c | 254 ++- drivers/gpu/drm/vkms/vkms_formats.h | 2 +- drivers/gpu/drm/vkms/vkms_plane.c| 13 +- 5 files changed, 236 insertions(+), 162 deletions(-) --- base-commit: 5d189d57bb335a87ec38ea26fe43a5f3ed31ced7 change-id: 20240201-yuv-1337d90d9576 Best regards,
Re: [PATCH] drm/v3d: Show the memory-management stats on debugfs
On 1/5/24 12:32, Melissa Wen wrote:
On 01/05, Maíra Canal wrote:
Dump the contents of the DRM MM allocator of the V3D driver. This will
help us to debug the VA ranges allocated.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_debugfs.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index f843a50d5dce..cdfe1d3bf5ee 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -260,11 +260,26 @@ static int v3d_measure_clock(struct seq_file *m, void
*unused)
return 0;
}
+static int v3d_debugfs_mm(struct seq_file *m, void *unused)
+{
+ struct drm_printer p = drm_seq_file_printer(m);
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
+ struct v3d_dev *v3d = to_v3d_dev(dev);
+
+ spin_lock(&v3d->mm_lock);
+ drm_mm_print(&v3d->mm, &p);
+ spin_unlock(&v3d->mm_lock);
+
+ return 0;
+}
LGTM.
Reviewed-by: Melissa Wen
Pushed to drm-misc/drm-misc-next!
Best Regards,
- Maíra
+
static const struct drm_debugfs_info v3d_debugfs_list[] = {
{"v3d_ident", v3d_v3d_debugfs_ident, 0},
{"v3d_regs", v3d_v3d_debugfs_regs, 0},
{"measure_clock", v3d_measure_clock, 0},
{"bo_stats", v3d_debugfs_bo_stats, 0},
+ {"v3d_mm", v3d_debugfs_mm, 0},
};
void
--
2.43.0
Re: [PATCH] drm/v3d: Free the job and assign it to NULL if initialization fails
On 1/11/24 04:13, Iago Toral wrote: Ok, thanks for checking, you can add my R-B on the original patch then. Applied to drm-misc/drm-misc-next-fixes! Best Regards, - Maíra Iago El mié, 10-01-2024 a las 07:42 -0300, Maira Canal escribió: Hi Iago, On 1/10/24 03:48, Iago Toral wrote: I think this is fine, but I was wondering if it would be simpler and easier to just remove the sched cleanup from v3d_job_init and instead always rely on callers to eventually call v3d_job_cleanup for fail paths, where we are already calling v3d_job_cleanup. If we just remove `drm_sched_job_cleanup()` from `v3d_job_init()` we trigger a use-after-free warning by the job refcount: [ 34.367222] [ cut here ] [ 34.367235] refcount_t: underflow; use-after-free. [ 34.367274] WARNING: CPU: 0 PID: 1922 at lib/refcount.c:28 refcount_warn_saturate+0x108/0x148 [ 34.367298] Modules linked in: rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device algif_hash aes_neon_bs aes_neon_blk algif_skcipher af_alg bnep hid_logitech_hidpp brcmfmac_wcc brcmfmac hci_uart btbcm bluetooth vc4 brcmutil cfg80211 bcm2835_v4l2(C) bcm2835_mmal_vchiq(C) binfmt_misc snd_soc_hdmi_codec videobuf2_v4l2 cec ecdh_generic drm_display_helper videobuf2_vmalloc ecc videobuf2_memops drm_dma_helper videobuf2_common drm_kms_helper dwc2 raspberrypi_hwmon videodev snd_soc_core i2c_brcmstb rfkill libaes hid_logitech_dj mc v3d snd_bcm2835(C) i2c_bcm2835 pwm_bcm2835 snd_pcm_dmaengine snd_pcm gpu_sched snd_timer drm_shmem_helper snd nvmem_rmem uio_pdrv_genirq uio i2c_dev drm dm_mod fuse drm_panel_orientation_quirks backlight configfs ip_tables x_tables ipv6 [ 34.367434] CPU: 0 PID: 1922 Comm: v3d_submit_cl Tainted: G C 6.7.0-rc3-g632ca3c92f38-dirty #74 [ 34.367441] Hardware name: Raspberry Pi 4 Model B Rev 1.5 (DT) [ 34.367444] pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 34.367450] pc : refcount_warn_saturate+0x108/0x148 [ 34.367456] lr : refcount_warn_saturate+0x108/0x148 [ 34.367462] sp : ffc08341bb90 [ 34.367465] x29: ffc08341bb90 x28: ff8102962400 x27: ffee5592de88 [ 34.367473] x26: ff8116503e00 x25: ff81213e8800 x24: 0048 [ 34.367481] x23: ff8101088000 x22: ffc08341bcf0 x21: ffea [ 34.367489] x20: ff8102962400 x19: ff8102962600 x18: ffee5beb3468 [ 34.367497] x17: 0001 x16: x15: 0004 [ 34.367504] x14: ffee5c163738 x13: 0fff x12: 0003 [ 34.367512] x11: x10: 0027 x9 : ada342fc9d5acc00 [ 34.367519] x8 : ada342fc9d5acc00 x7 : 65646e75203a745f x6 : 746e756f63666572 [ 34.367526] x5 : ffee5c3129da x4 : ffee5c2fc59e x3 : [ 34.367533] x2 : x1 : ffc08341b930 x0 : 0026 [ 34.367541] Call trace: [ 34.367544] refcount_warn_saturate+0x108/0x148 [ 34.367550] v3d_submit_cl_ioctl+0x4cc/0x5e8 [v3d] [ 34.367588] drm_ioctl_kernel+0xe0/0x120 [drm] [ 34.367767] drm_ioctl+0x264/0x408 [drm] [ 34.367866] __arm64_sys_ioctl+0x9c/0xe0 [ 34.367877] invoke_syscall+0x4c/0x118 [ 34.367887] el0_svc_common+0xb8/0xf0 [ 34.367892] do_el0_svc+0x28/0x40 [ 34.367898] el0_svc+0x38/0x88 [ 34.367906] el0t_64_sync_handler+0x84/0x100 [ 34.367913] el0t_64_sync+0x190/0x198 [ 34.367917] ---[ end trace ]--- Best Regards, - Maíra Iago El mar, 09-01-2024 a las 11:28 -0300, Maíra Canal escribió: Currently, if `v3d_job_init()` fails (e.g. in the IGT test "bad- in- sync", where we submit an invalid in-sync to the IOCTL), then we end up with the following NULL pointer dereference: [ 34.146279] Unable to handle kernel NULL pointer dereference at virtual address 0078 [ 34.146301] Mem abort info: [ 34.146306] ESR = 0x9605 [ 34.146315] EC = 0x25: DABT (current EL), IL = 32 bits [ 34.146322] SET = 0, FnV = 0 [ 34.146328] EA = 0, S1PTW = 0 [ 34.146334] FSC = 0x05: level 1 translation fault [ 34.146340] Data abort info: [ 34.146345] ISV = 0, ISS = 0x0005, ISS2 = 0x [ 34.146351] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 34.146357] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 34.146366] user pgtable: 4k pages, 39-bit VAs, pgdp=0001232e6000 [ 34.146375] [0078] pgd=, p4d=, pud= [ 34.146399] Internal error: Oops: 9605 [#1] PREEMPT SMP [ 34.146406] Modules linked in: rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device algif_hash aes_neon_bs aes_neon_blk algif_skcipher af_alg bnep hid_logitech_hidpp brcmfmac_wcc brcmfmac brcmutil hci_uart vc4 btbcm cfg80211 bluetooth bcm2835_v4l2(C) snd_soc_hdmi_codec binfmt_misc cec drm_display_helper hid_logitech_dj bcm2835_mmal_vchiq(C) drm_dma_helper drm_kms_helper videobuf2_v4l2 raspberrypi_hwmon ecdh_generic videobuf2_vmalloc videobuf2_
Re: [PATCH] drm/vc4: don't check if plane->state->fb == state->fb
On 1/8/24 05:43, Maxime Ripard wrote: On Fri, 5 Jan 2024 14:58:36 -0300, Ma=C3=ADra Canal wrote: Currently, when using non-blocking commits, we can see the following kernel warning: =20 [ 110.908514] [ cut here ] [ 110.908529] refcount_t: underflow; use-after-free. =20 [ ... ] Acked-by: Maxime Ripard Pushed to drm-misc/drm-misc-next! Best Regards, - Maíra Thanks! Maxime
Re: [PATCH] drm/v3d: Free the job and assign it to NULL if initialization fails
Hi Iago, On 1/10/24 03:48, Iago Toral wrote: I think this is fine, but I was wondering if it would be simpler and easier to just remove the sched cleanup from v3d_job_init and instead always rely on callers to eventually call v3d_job_cleanup for fail paths, where we are already calling v3d_job_cleanup. If we just remove `drm_sched_job_cleanup()` from `v3d_job_init()` we trigger a use-after-free warning by the job refcount: [ 34.367222] [ cut here ] [ 34.367235] refcount_t: underflow; use-after-free. [ 34.367274] WARNING: CPU: 0 PID: 1922 at lib/refcount.c:28 refcount_warn_saturate+0x108/0x148 [ 34.367298] Modules linked in: rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device algif_hash aes_neon_bs aes_neon_blk algif_skcipher af_alg bnep hid_logitech_hidpp brcmfmac_wcc brcmfmac hci_uart btbcm bluetooth vc4 brcmutil cfg80211 bcm2835_v4l2(C) bcm2835_mmal_vchiq(C) binfmt_misc snd_soc_hdmi_codec videobuf2_v4l2 cec ecdh_generic drm_display_helper videobuf2_vmalloc ecc videobuf2_memops drm_dma_helper videobuf2_common drm_kms_helper dwc2 raspberrypi_hwmon videodev snd_soc_core i2c_brcmstb rfkill libaes hid_logitech_dj mc v3d snd_bcm2835(C) i2c_bcm2835 pwm_bcm2835 snd_pcm_dmaengine snd_pcm gpu_sched snd_timer drm_shmem_helper snd nvmem_rmem uio_pdrv_genirq uio i2c_dev drm dm_mod fuse drm_panel_orientation_quirks backlight configfs ip_tables x_tables ipv6 [ 34.367434] CPU: 0 PID: 1922 Comm: v3d_submit_cl Tainted: G C 6.7.0-rc3-g632ca3c92f38-dirty #74 [ 34.367441] Hardware name: Raspberry Pi 4 Model B Rev 1.5 (DT) [ 34.367444] pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 34.367450] pc : refcount_warn_saturate+0x108/0x148 [ 34.367456] lr : refcount_warn_saturate+0x108/0x148 [ 34.367462] sp : ffc08341bb90 [ 34.367465] x29: ffc08341bb90 x28: ff8102962400 x27: ffee5592de88 [ 34.367473] x26: ff8116503e00 x25: ff81213e8800 x24: 0048 [ 34.367481] x23: ff8101088000 x22: ffc08341bcf0 x21: ffea [ 34.367489] x20: ff8102962400 x19: ff8102962600 x18: ffee5beb3468 [ 34.367497] x17: 0001 x16: x15: 0004 [ 34.367504] x14: ffee5c163738 x13: 0fff x12: 0003 [ 34.367512] x11: x10: 0027 x9 : ada342fc9d5acc00 [ 34.367519] x8 : ada342fc9d5acc00 x7 : 65646e75203a745f x6 : 746e756f63666572 [ 34.367526] x5 : ffee5c3129da x4 : ffee5c2fc59e x3 : [ 34.367533] x2 : x1 : ffc08341b930 x0 : 0026 [ 34.367541] Call trace: [ 34.367544] refcount_warn_saturate+0x108/0x148 [ 34.367550] v3d_submit_cl_ioctl+0x4cc/0x5e8 [v3d] [ 34.367588] drm_ioctl_kernel+0xe0/0x120 [drm] [ 34.367767] drm_ioctl+0x264/0x408 [drm] [ 34.367866] __arm64_sys_ioctl+0x9c/0xe0 [ 34.367877] invoke_syscall+0x4c/0x118 [ 34.367887] el0_svc_common+0xb8/0xf0 [ 34.367892] do_el0_svc+0x28/0x40 [ 34.367898] el0_svc+0x38/0x88 [ 34.367906] el0t_64_sync_handler+0x84/0x100 [ 34.367913] el0t_64_sync+0x190/0x198 [ 34.367917] ---[ end trace ]--- Best Regards, - Maíra Iago El mar, 09-01-2024 a las 11:28 -0300, Maíra Canal escribió: Currently, if `v3d_job_init()` fails (e.g. in the IGT test "bad-in- sync", where we submit an invalid in-sync to the IOCTL), then we end up with the following NULL pointer dereference: [ 34.146279] Unable to handle kernel NULL pointer dereference at virtual address 0078 [ 34.146301] Mem abort info: [ 34.146306] ESR = 0x9605 [ 34.146315] EC = 0x25: DABT (current EL), IL = 32 bits [ 34.146322] SET = 0, FnV = 0 [ 34.146328] EA = 0, S1PTW = 0 [ 34.146334] FSC = 0x05: level 1 translation fault [ 34.146340] Data abort info: [ 34.146345] ISV = 0, ISS = 0x0005, ISS2 = 0x [ 34.146351] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 34.146357] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 34.146366] user pgtable: 4k pages, 39-bit VAs, pgdp=0001232e6000 [ 34.146375] [0078] pgd=, p4d=, pud= [ 34.146399] Internal error: Oops: 9605 [#1] PREEMPT SMP [ 34.146406] Modules linked in: rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device algif_hash aes_neon_bs aes_neon_blk algif_skcipher af_alg bnep hid_logitech_hidpp brcmfmac_wcc brcmfmac brcmutil hci_uart vc4 btbcm cfg80211 bluetooth bcm2835_v4l2(C) snd_soc_hdmi_codec binfmt_misc cec drm_display_helper hid_logitech_dj bcm2835_mmal_vchiq(C) drm_dma_helper drm_kms_helper videobuf2_v4l2 raspberrypi_hwmon ecdh_generic videobuf2_vmalloc videobuf2_memops ecc videobuf2_common rfkill videodev libaes snd_soc_core dwc2 i2c_brcmstb snd_pcm_dmaengine snd_bcm2835(C) i2c_bcm2835 pwm_bcm2835 snd_pcm mc v3d snd_timer snd gpu_sched drm_shmem_helper nvmem_rmem uio_pdrv_genirq uio i2c_dev drm
Re: [PATCH] drm/v3d: Fix support for register debugging on the RPi 4
On 1/9/24 09:07, Iago Toral wrote:
Thanks Maíra!
Reviewed-by: Iago Toral Quiroga
Applied to drm-misc/drm-misc-next-fixes!
Best Regards,
- Maíra
El mar, 09-01-2024 a las 08:30 -0300, Maíra Canal escribió:
RPi 4 uses V3D 4.2, which is currently not supported by the register
definition stated at `v3d_core_reg_defs`. We should be able to
support
V3D 4.2, therefore, change the maximum version of the register
definition to 42, not 41.
Fixes: 0ad5bc1ce463 ("drm/v3d: fix up register addresses for V3D
7.x")
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_debugfs.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index f843a50d5dce..94eafcecc65b 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -62,9 +62,9 @@ static const struct v3d_reg_def v3d_core_reg_defs[]
= {
REGDEF(33, 71, V3D_PTB_BPCA),
REGDEF(33, 71, V3D_PTB_BPCS),
- REGDEF(33, 41, V3D_GMP_STATUS(33)),
- REGDEF(33, 41, V3D_GMP_CFG(33)),
- REGDEF(33, 41, V3D_GMP_VIO_ADDR(33)),
+ REGDEF(33, 42, V3D_GMP_STATUS(33)),
+ REGDEF(33, 42, V3D_GMP_CFG(33)),
+ REGDEF(33, 42, V3D_GMP_VIO_ADDR(33)),
REGDEF(33, 71, V3D_ERR_FDBGO),
REGDEF(33, 71, V3D_ERR_FDBGB),
@@ -74,13 +74,13 @@ static const struct v3d_reg_def
v3d_core_reg_defs[] = {
static const struct v3d_reg_def v3d_csd_reg_defs[] = {
REGDEF(41, 71, V3D_CSD_STATUS),
- REGDEF(41, 41, V3D_CSD_CURRENT_CFG0(41)),
- REGDEF(41, 41, V3D_CSD_CURRENT_CFG1(41)),
- REGDEF(41, 41, V3D_CSD_CURRENT_CFG2(41)),
- REGDEF(41, 41, V3D_CSD_CURRENT_CFG3(41)),
- REGDEF(41, 41, V3D_CSD_CURRENT_CFG4(41)),
- REGDEF(41, 41, V3D_CSD_CURRENT_CFG5(41)),
- REGDEF(41, 41, V3D_CSD_CURRENT_CFG6(41)),
+ REGDEF(41, 42, V3D_CSD_CURRENT_CFG0(41)),
+ REGDEF(41, 42, V3D_CSD_CURRENT_CFG1(41)),
+ REGDEF(41, 42, V3D_CSD_CURRENT_CFG2(41)),
+ REGDEF(41, 42, V3D_CSD_CURRENT_CFG3(41)),
+ REGDEF(41, 42, V3D_CSD_CURRENT_CFG4(41)),
+ REGDEF(41, 42, V3D_CSD_CURRENT_CFG5(41)),
+ REGDEF(41, 42, V3D_CSD_CURRENT_CFG6(41)),
REGDEF(71, 71, V3D_CSD_CURRENT_CFG0(71)),
REGDEF(71, 71, V3D_CSD_CURRENT_CFG1(71)),
REGDEF(71, 71, V3D_CSD_CURRENT_CFG2(71)),
--
2.43.0
Re: [PATCH] drm/vc4: plane: check drm_gem_plane_helper_prepare_fb() return value
Hi Simon,
On 12/16/23 11:15, Simon Ser wrote:
Bubble up any error to the caller.
Signed-off-by: Simon Ser
Cc: Maxime Ripard
Cc: Kees Cook
Cc: Dave Stevenson
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
drivers/gpu/drm/vc4/vc4_plane.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 00e713faecd5..b8184374332c 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -1497,13 +1497,16 @@ static int vc4_prepare_fb(struct drm_plane *plane,
struct drm_plane_state *state)
{
struct vc4_bo *bo;
+ int ret;
if (!state->fb)
return 0;
bo = to_vc4_bo(&drm_fb_dma_get_gem_obj(state->fb, 0)->base);
- drm_gem_plane_helper_prepare_fb(plane, state);
+ ret = drm_gem_plane_helper_prepare_fb(plane, state);
+ if (ret)
+ return ret;
if (plane->state->fb == state->fb)
return 0;
Re: [PATCH v4 00/17] drm/v3d: Introduce CPU jobs
On 11/30/23 13:40, Maíra Canal wrote: Maíra Canal (11): drm/v3d: Don't allow two multisync extensions in the same job drm/v3d: Decouple job allocation from job initiation drm/v3d: Use v3d_get_extensions() to parse CPU job data drm/v3d: Create tracepoints to track the CPU job drm/v3d: Enable BO mapping drm/v3d: Create a CPU job extension for a indirect CSD job drm/v3d: Create a CPU job extension for the timestamp query job drm/v3d: Create a CPU job extension for the reset timestamp job drm/v3d: Create a CPU job extension to copy timestamp query to a buffer drm/v3d: Create a CPU job extension for the reset performance query job drm/v3d: Create a CPU job extension for the copy performance query job Melissa Wen (6): drm/v3d: Remove unused function header drm/v3d: Move wait BO ioctl to the v3d_bo file drm/v3d: Detach job submissions IOCTLs to a new specific file drm/v3d: Simplify job refcount handling drm/v3d: Add a CPU job submission drm/v3d: Detach the CSD job BO setup Pushed to drm-misc/drm-misc-next! Best Regards, - Maíra drivers/gpu/drm/v3d/Makefile |3 +- drivers/gpu/drm/v3d/v3d_bo.c | 51 ++ drivers/gpu/drm/v3d/v3d_drv.c|4 + drivers/gpu/drm/v3d/v3d_drv.h| 134 ++- drivers/gpu/drm/v3d/v3d_gem.c| 768 - drivers/gpu/drm/v3d/v3d_sched.c | 315 +++ drivers/gpu/drm/v3d/v3d_submit.c | 1318 ++ drivers/gpu/drm/v3d/v3d_trace.h | 57 ++ include/uapi/drm/v3d_drm.h | 240 +- 9 files changed, 2110 insertions(+), 780 deletions(-) create mode 100644 drivers/gpu/drm/v3d/v3d_submit.c -- 2.42.0
Re: [PATCH v3 10/17] drm/v3d: Detach the CSD job BO setup
Hi Iago,
On 11/28/23 05:42, Iago Toral wrote:
El lun, 27-11-2023 a las 15:48 -0300, Maíra Canal escribió:
From: Melissa Wen
Detach CSD job setup from CSD submission ioctl to reuse it in CPU
submission ioctl for indirect CSD job.
Signed-off-by: Melissa Wen
Co-developed-by: Maíra Canal
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_submit.c | 68 --
--
1 file changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c
b/drivers/gpu/drm/v3d/v3d_submit.c
index c134b113b181..eb26fe1e27e3 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -256,6 +256,45 @@ v3d_attach_fences_and_unlock_reservation(struct
drm_file *file_priv,
}
}
+static int
+v3d_setup_csd_jobs_and_bos(struct drm_file *file_priv,
+ struct v3d_dev *v3d,
+ struct drm_v3d_submit_csd *args,
+ struct v3d_csd_job **job,
+ struct v3d_job **clean_job,
+ struct v3d_submit_ext *se,
+ struct ww_acquire_ctx *acquire_ctx)
+{
+ int ret;
+
+ ret = v3d_job_allocate((void *)job, sizeof(**job));
+ if (ret)
+ return ret;
+
+ ret = v3d_job_init(v3d, file_priv, &(*job)->base,
+ v3d_job_free, args->in_sync, se, V3D_CSD);
+ if (ret)
We should free the job here.
+ return ret;
+
+ ret = v3d_job_allocate((void *)clean_job,
sizeof(**clean_job));
+ if (ret)
+ return ret;
+
+ ret = v3d_job_init(v3d, file_priv, *clean_job,
+ v3d_job_free, 0, NULL, V3D_CACHE_CLEAN);
+ if (ret)
We should free job and clean_job here.
+ return ret;
+
+ (*job)->args = *args;
+
+ ret = v3d_lookup_bos(&v3d->drm, file_priv, *clean_job,
+ args->bo_handles, args-
bo_handle_count);
+ if (ret)
Same here.
I think we probably want to have a fail label where we do this and just
jump there from all the paths I mentioned above.
Actually, we are freeing the job in `v3d_submit_csd_ioctl`. Take a look
here:
48 ret = v3d_setup_csd_jobs_and_bos(file_priv, v3d, args,
47 &job, &clean_job, &se,
46 &acquire_ctx);
45 if (ret)
44 goto fail;
If `v3d_setup_csd_jobs_and_bos` fails, we go to fail.
43
42 if (args->perfmon_id) {
41 job->base.perfmon = v3d_perfmon_find(v3d_priv,
40
args->perfmon_id);
39 if (!job->base.perfmon) {
38 ret = -ENOENT;
37 goto fail_perfmon;
36 }
35 }
34
33 mutex_lock(&v3d->sched_lock);
32 v3d_push_job(&job->base);
31
30 ret = drm_sched_job_add_dependency(&clean_job->base,
29
dma_fence_get(job->base.done_fence));
28 if (ret)
27 goto fail_unreserve;
26
25 v3d_push_job(clean_job);
24 mutex_unlock(&v3d->sched_lock);
23
22 v3d_attach_fences_and_unlock_reservation(file_priv,
21 clean_job,
20 &acquire_ctx,
19 args->out_sync,
18 &se,
17
clean_job->done_fence);
16
15 v3d_job_put(&job->base);
14 v3d_job_put(clean_job);
13
12 return 0;
11
10 fail_unreserve:
9 mutex_unlock(&v3d->sched_lock);
8 fail_perfmon:
7 drm_gem_unlock_reservations(clean_job->bo,
clean_job->bo_count,
6 &acquire_ctx);
5 fail:
4 v3d_job_cleanup((void *)job);
3 v3d_job_cleanup(clean_job);
Here we cleanup `job` and `clean_job`. This will call `v3d_job_free` and
free the jobs.
Best Regards,
- Maíra
2 v3d_put_multisync_post_deps(&se);
1
1167 return ret;
+ return ret;
+
+ return v3d_lock_bo_reservations(*clean_job, acquire_ctx);
+}
+
static void
v3d_put_multisync_post_deps(struct v3d_submit_ext *se)
{
@@ -700,32 +739,9 @@ v3d_submit_csd_ioctl(struct drm_device *dev,
void *data,
}
}
- ret = v3d_job_allocate((void *)&job, sizeof(*job));
- if (ret)
- return ret;
-
- ret = v3d_job_init(v3d, file_priv, &job->base,
- v3d_job_free, args->in_sync, &se,
V3D_CSD);
- if (ret)
- goto fail;
-
- ret = v3d_job_allocate((void *)&clean_job,
sizeof(*clean_job));
- if (ret)
- goto fail;
-
- ret = v3d_job_init(v3d, file_priv, clean_job,
- v3d_job_free, 0, NULL, V3D_CACHE_CLEAN)
Re: [PATCH v2 13/17] drm/v3d: Create a CPU job extension for the timestamp query job
Hi Iago,
On 11/27/23 06:16, Iago Toral wrote:
El jue, 23-11-2023 a las 21:47 -0300, Maíra Canal escribió:
(...)
+static void
+v3d_timestamp_query(struct v3d_cpu_job *job)
+{
+ struct v3d_timestamp_query_info *timestamp_query = &job-
timestamp_query;
+ struct v3d_bo *bo = to_v3d_bo(job->base.bo[0]);
I presume there is an implicit expectation here that a timestamp query
job only has one BO where we are writing query results, right? Maybe we
should document this more explicitly in the uAPI and also check that we
have at least that one BO before trying to map it and write to it?
I'll do it.
Also, is there a reason why we take the job reference from job-
base.bo[0] instead of job->bo[0]?
job is a struct v3d_cpu_job, which has a struct v3d_job as base.
The BOs are stored on struct v3d_job, as this is a common functionality
of all job types.
Best Regards,
- Maíra
Iago
+ u8 *value_addr;
+
+ v3d_get_bo_vaddr(bo);
+
+ for (int i = 0; i < timestamp_query->count; i++) {
+ value_addr = ((u8 *) bo->vaddr) + timestamp_query-
queries[i].offset;
+ *((u64 *) value_addr) = i == 0 ? ktime_get_ns() :
0ull;
+
+ drm_syncobj_replace_fence(timestamp_query-
queries[i].syncobj,
+ job->base.done_fence);
+ }
+
+ v3d_put_bo_vaddr(bo);
+}
+
static struct dma_fence *
v3d_cpu_job_run(struct drm_sched_job *sched_job)
{
@@ -315,6 +352,7 @@ v3d_cpu_job_run(struct drm_sched_job *sched_job)
void (*v3d_cpu_job_fn[])(struct v3d_cpu_job *job) = {
[V3D_CPU_JOB_TYPE_INDIRECT_CSD] =
v3d_rewrite_csd_job_wg_counts_from_indirect,
+ [V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY] =
v3d_timestamp_query,
};
v3d->cpu_job = job;
@@ -504,7 +542,7 @@ static const struct drm_sched_backend_ops
v3d_cache_clean_sched_ops = {
static const struct drm_sched_backend_ops v3d_cpu_sched_ops = {
.run_job = v3d_cpu_job_run,
.timedout_job = v3d_generic_job_timedout,
- .free_job = v3d_sched_job_free
+ .free_job = v3d_cpu_job_free
};
int
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c
b/drivers/gpu/drm/v3d/v3d_submit.c
index b6707ef42706..2f03c8bca593 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -438,6 +438,64 @@ v3d_get_cpu_indirect_csd_params(struct drm_file
*file_priv,
NULL, &info->acquire_ctx);
}
+/* Get data for the query timestamp job submission. */
+static int
+v3d_get_cpu_timestamp_query_params(struct drm_file *file_priv,
+ struct drm_v3d_extension __user
*ext,
+ struct v3d_cpu_job *job)
+{
+ u32 __user *offsets, *syncs;
+ struct drm_v3d_timestamp_query timestamp;
+
+ if (!job) {
+ DRM_DEBUG("CPU job extension was attached to a GPU
job.\n");
+ return -EINVAL;
+ }
+
+ if (job->job_type) {
+ DRM_DEBUG("Two CPU job extensions were added to the
same CPU job.\n");
+ return -EINVAL;
+ }
+
+ if (copy_from_user(×tamp, ext, sizeof(timestamp)))
+ return -EFAULT;
+
+ if (timestamp.pad)
+ return -EINVAL;
+
+ job->job_type = V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY;
+
+ job->timestamp_query.queries =
kvmalloc_array(timestamp.count,
+ sizeof(struct
v3d_timestamp_query),
+ GFP_KERNEL);
+ if (!job->timestamp_query.queries)
+ return -ENOMEM;
+
+ offsets = u64_to_user_ptr(timestamp.offsets);
+ syncs = u64_to_user_ptr(timestamp.syncs);
+
+ for (int i = 0; i < timestamp.count; i++) {
+ u32 offset, sync;
+
+ if (copy_from_user(&offset, offsets++,
sizeof(offset))) {
+ kvfree(job->timestamp_query.queries);
+ return -EFAULT;
+ }
+
+ job->timestamp_query.queries[i].offset = offset;
+
+ if (copy_from_user(&sync, syncs++, sizeof(sync))) {
+ kvfree(job->timestamp_query.queries);
+ return -EFAULT;
+ }
+
+ job->timestamp_query.queries[i].syncobj =
drm_syncobj_find(file_priv, sync);
+ }
+ job->timestamp_query.count = timestamp.count;
+
+ return 0;
+}
+
/* Whenever userspace sets ioctl extensions, v3d_get_extensions
parses data
* according to the extension id (name).
*/
@@ -466,6 +524,9 @@ v3d_get_extensions(struct drm_file *file_priv,
case DRM_V3D_EXT_ID_CPU_INDIRECT_CSD:
ret =
v3d_get_cpu_indirect_csd_params(file_priv, user_ext, job);
break;
+ case DRM_V3D_EXT_ID_CPU_TIMESTAMP_QUERY:
+ ret =
v3d_get_cpu_timestamp_que
Re: [PATCH v2 06/17] drm/v3d: Decouple job allocation from job initiation
Hi Iago,
On 11/27/23 05:12, Iago Toral wrote:
El jue, 23-11-2023 a las 21:47 -0300, Maíra Canal escribió:
We want to allow the IOCTLs to allocate the job without initiating
it.
This will be useful for the CPU job submission IOCTL, as the CPU job
has
the need to use information from the user extensions. Currently, the
user extensions are parsed before the job allocation, making it
impossible to fill the CPU job when parsing the user extensions.
Therefore, decouple the job allocation from the job initiation.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_submit.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c
b/drivers/gpu/drm/v3d/v3d_submit.c
index fe46dd316ca0..ed1a310bbd2f 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -135,6 +135,21 @@ void v3d_job_put(struct v3d_job *job)
kref_put(&job->refcount, job->free);
}
+static int
+v3d_job_allocate(void **container, size_t size)
+{
+ if (*container)
+ return 0;
Mmm... is this really what we want? At least right now we expect
Yeah, it is.
v3d_job_allocate to always allocate memory, is there any scenario in
which we would expect to call this with an already allocated container?
Take a look here:
19 int
18 v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
17 struct drm_file *file_priv)
16 {
15 struct v3d_dev *v3d = to_v3d_dev(dev);
14 struct drm_v3d_submit_cpu *args = data;
13 struct v3d_submit_ext se = {0};
12 struct v3d_submit_ext *out_se = NULL;
11 struct v3d_cpu_job *cpu_job = NULL;
10 struct v3d_csd_job *csd_job = NULL;
9 struct v3d_job *clean_job = NULL;
8 struct ww_acquire_ctx acquire_ctx;
7 int ret;
6
5 if (args->flags && !(args->flags &
DRM_V3D_SUBMIT_EXTENSION)) {
4 DRM_INFO("invalid flags: %d\n", args->flags);
3 return -EINVAL;
2 }
1
1187 ret = v3d_job_allocate((void *)&cpu_job, sizeof(*cpu_job));
1 if (ret)
2 return ret;
Here we allocate the CPU job, as we need it allocated to fill its fields
with the information in the extensions.
3
4 if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
5 ret = v3d_get_extensions(file_priv,
args->extensions, &se, cpu_job);
So, here we filling the CPU job fields with relevant information.
6 if (ret) {
7 DRM_DEBUG("Failed to get extensions.\n");
8 goto fail;
9 }
10 }
11
12 /* Every CPU job must have a CPU job user extension */
13 if (!cpu_job->job_type) {
14 DRM_DEBUG("CPU job must have a CPU job user
extension.\n");
15 goto fail;
16 }
17
18 trace_v3d_submit_cpu_ioctl(&v3d->drm, cpu_job->job_type);
19
20 ret = v3d_job_init(v3d, file_priv, (void *)&cpu_job,
sizeof(*cpu_job),
21v3d_job_free, 0, &se, V3D_CPU);
Here we are initiating the job (drm_sched_job_init and syncobjs stuff).
If I don't have this condition in v3d_job_allocate, I would allocate
the container twice.
22 if (ret)
23 goto fail;
Best Regards,
- Maíra
Iago
+
+ *container = kcalloc(1, size, GFP_KERNEL);
+ if (!*container) {
+ DRM_ERROR("Cannot allocate memory for V3D job.\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int
v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
void **container, size_t size, void (*free)(struct kref
*ref),
@@ -145,11 +160,9 @@ v3d_job_init(struct v3d_dev *v3d, struct
drm_file *file_priv,
bool has_multisync = se && (se->flags &
DRM_V3D_EXT_ID_MULTI_SYNC);
int ret, i;
- *container = kcalloc(1, size, GFP_KERNEL);
- if (!*container) {
- DRM_ERROR("Cannot allocate memory for v3d job.");
- return -ENOMEM;
- }
+ ret = v3d_job_allocate(container, size);
+ if (ret)
+ return ret;
job = *container;
job->v3d = v3d;
Re: [PATCH] MAINTAINERS: Add Maira to V3D maintainers
On 11/8/23 10:05, Melissa Wen wrote: On 11/06, Maíra Canal wrote: I've been contributing to V3D with improvements, reviews, testing and debugging. Therefore, add myself as a co-maintainer of the V3D driver. Signed-off-by: Maíra Canal Acked-by: Melissa Wen Applied to drm-misc/drm-misc-next! Best Regards, - Maíra --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index f13e476ed803..3213563756cb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7108,6 +7108,7 @@ F:drivers/gpu/drm/omapdrm/ DRM DRIVERS FOR V3D M:Emma Anholt M:Melissa Wen +M: Maíra Canal S:Supported T:git git://anongit.freedesktop.org/drm/drm-misc F:Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml -- 2.41.0
Re: [RFC PATCH 2/2] vc4: introduce DMA-BUF heap
Hi Simon,
Thanks for working on this feature!
On 11/9/23 04:45, Simon Ser wrote:
User-space sometimes needs to allocate scanout-capable memory for
GPU rendering purposes. On a vc4/v3d split render/display SoC, this
is achieved via DRM dumb buffers: the v3d user-space driver opens
the primary vc4 node, allocates a DRM dumb buffer there, exports it
as a DMA-BUF, imports it into the v3d render node, and renders to it.
However, DRM dumb buffers are only meant for CPU rendering, they are
not intended to be used for GPU rendering. Primary nodes should only
be used for mode-setting purposes, other programs should not attempt
to open it. Moreover, opening the primary node is already broken on
some setups: systemd grants permission to open primary nodes to
physically logged in users, but this breaks when the user is not
physically logged in (e.g. headless setup) and when the distribution
is using a different init (e.g. Alpine Linux uses openrc).
We need an alternate way for v3d to allocate scanout-capable memory.
For me, it is a bit unclear how we import the vc4 DMA-BUF heap into v3d.
Should we create an IOCTL for it on v3d?
Also, if you need some help testing with the RPi 5, you can ping on IRC
and I can try to help by testing.
Best Regards,
- Maíra
Leverage DMA heaps for this purpose: expose a CMA heap to user-space.
Preliminary testing has been done with wlroots [1].
This is still an RFC. Open questions:
- Does this approach make sense to y'all in general?
- What would be a good name for the heap? "vc4" is maybe a bit naive and
not precise enough. Something with "cma"? Do we need to plan a naming
scheme to accomodate for multiple vc4 devices?
- Right now root privileges are necessary to open the heap. Should we
allow everybody to open the heap by default (after all, user-space can
already allocate arbitrary amounts of GPU memory)? Should we leave it
up to user-space to solve this issue (via logind/seatd or a Wayland
protocol or something else)?
TODO:
- Need to add !vc5 support.
- Need to the extend DMA heaps API to allow vc4 to unregister the heap
on unload.
[1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4414
Signed-off-by: Simon Ser
Cc: Maxime Ripard
Cc: Daniel Vetter
Cc: Daniel Stone
Cc: Erico Nunes
Cc: Iago Toral Quiroga
Cc: Maíra Canal
Cc: Thomas Zimmermann
---
drivers/gpu/drm/vc4/Makefile | 2 ++
drivers/gpu/drm/vc4/vc4_dma_heap.c | 51 ++
drivers/gpu/drm/vc4/vc4_drv.c | 6
drivers/gpu/drm/vc4/vc4_drv.h | 5 +++
4 files changed, 64 insertions(+)
create mode 100644 drivers/gpu/drm/vc4/vc4_dma_heap.c
diff --git a/drivers/gpu/drm/vc4/Makefile b/drivers/gpu/drm/vc4/Makefile
index c41f89a15a55..e4048870cec7 100644
--- a/drivers/gpu/drm/vc4/Makefile
+++ b/drivers/gpu/drm/vc4/Makefile
@@ -34,4 +34,6 @@ vc4-$(CONFIG_DRM_VC4_KUNIT_TEST) += \
vc4-$(CONFIG_DEBUG_FS) += vc4_debugfs.o
+vc4-$(CONFIG_DMABUF_HEAPS) += vc4_dma_heap.o
+
obj-$(CONFIG_DRM_VC4) += vc4.o
diff --git a/drivers/gpu/drm/vc4/vc4_dma_heap.c
b/drivers/gpu/drm/vc4/vc4_dma_heap.c
new file mode 100644
index ..010d0a88f3fa
--- /dev/null
+++ b/drivers/gpu/drm/vc4/vc4_dma_heap.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright © 2023 Simon Ser
+ */
+
+#include
+#include
+
+#include "vc4_drv.h"
+
+static struct dma_buf *vc4_dma_heap_allocate(struct dma_heap *heap,
+unsigned long size,
+unsigned long fd_flags,
+unsigned long heap_flags)
+{
+ struct vc4_dev *vc4 = dma_heap_get_drvdata(heap);
+ //DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ struct drm_gem_dma_object *dma_obj;
+ struct dma_buf *dmabuf;
+
+ if (WARN_ON_ONCE(!vc4->is_vc5))
+ return ERR_PTR(-ENODEV);
+
+ dma_obj = drm_gem_dma_create(&vc4->base, size);
+ if (IS_ERR(dma_obj))
+ return ERR_CAST(dma_obj);
+
+ dmabuf = drm_gem_prime_export(&dma_obj->base, fd_flags);
+ drm_gem_object_put(&dma_obj->base);
+ return dmabuf;
+}
+
+static const struct dma_heap_ops vc4_dma_heap_ops = {
+ .allocate = vc4_dma_heap_allocate,
+};
+
+int vc4_dma_heap_create(struct vc4_dev *vc4)
+{
+ struct dma_heap_export_info exp_info;
+ struct dma_heap *heap;
+
+ exp_info.name = "vc4"; /* TODO: allow multiple? */
+ exp_info.ops = &vc4_dma_heap_ops;
+ exp_info.priv = vc4; /* TODO: unregister when unloading */
+
+ heap = dma_heap_add(&exp_info);
+ if (IS_ERR(heap))
+ return PTR_ERR(heap);
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index c133e96b8aca..c7297dd7d9d5 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -391,6 +391,12 @@ static int vc4_drm_bind(struct device *dev)
drm_fbdev_dma_
Re: [PATCH v3 0/2] drm/v3d: Expose GPU usage stats
Hi, I've just applied this patchset to drm-misc/drm-misc-next. Thanks Melissa and Chema for reviewing it! Best Regards, - Maíra On 9/5/23 18:06, Maíra Canal wrote: This patchset exposes GPU usages stats both globally and per-file descriptor. The first patch exposes the accumulated amount of active time per client through the fdinfo infrastructure. The amount of active time is exposed for each V3D queue. Moreover, it exposes the number of jobs submitted to each queue. The second patch exposes the accumulated amount of active time for each V3D queue, independent of the client. This data is exposed through the sysfs interface. With these patches, it is possible to calculate the GPU usage percentage per queue globally and per-file descriptor. * Example fdinfo output: $ cat /proc/1140/fdinfo/4 pos:0 flags: 0242 mnt_id: 24 ino:209 drm-driver: v3d drm-client-id: 44 drm-engine-bin: 1661076898 ns v3d-jobs-bin: 19576 jobs drm-engine-render: 31469427170 ns v3d-jobs-render:19575 jobs drm-engine-tfu: 5002964 ns v3d-jobs-tfu: 13 jobs drm-engine-csd: 188038329691 ns v3d-jobs-csd: 250393 jobs drm-engine-cache_clean: 27736024038 ns v3d-jobs-cache_clean: 250392 job * Example gputop output: DRM minor 128 PID bin render tfucsd cache_clean NAME 1140 |▎||██▋ || ||█▍ ||█▋ | computecloth 1158 |▍||▉ || || || | gears 1002 |▏||█▎|| || || | chromium-browse Best Regards, - Maíra --- v1 -> v2: https://lore.kernel.org/dri-devel/20230727142929.1275149-1-mca...@igalia.com/T/ * Use sysfs to expose global GPU stats (Tvrtko Ursulin) v2 -> v3: https://lore.kernel.org/dri-devel/20230807211849.49867-1-mca...@igalia.com/T/ * Document the expected behavior in case of a GPU reset (Melissa Wen) * Add a brief description about the sysfs outputs (Melissa Wen) * Instead of having multiple sysfs files, use only one sysfs file, called gpu_stats, with all the information (Chema Casanova) * Add the number of jobs submitted in the global GPU stats (Chema Casanova) * Now, the number of jobs submitted is only incremented if the job was completed Maíra Canal (2): drm/v3d: Implement show_fdinfo() callback for GPU usage stats drm/v3d: Expose the total GPU usage stats on sysfs drivers/gpu/drm/v3d/Makefile| 3 +- drivers/gpu/drm/v3d/v3d_drv.c | 45 - drivers/gpu/drm/v3d/v3d_drv.h | 31 +++ drivers/gpu/drm/v3d/v3d_gem.c | 7 +++- drivers/gpu/drm/v3d/v3d_irq.c | 49 +++ drivers/gpu/drm/v3d/v3d_sched.c | 33 drivers/gpu/drm/v3d/v3d_sysfs.c | 69 + 7 files changed, 234 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/v3d/v3d_sysfs.c -- 2.41.0
Re: [PATCH 1/2] drm/v3d: wait for all jobs to finish before unregistering
Hi Iago, On 10/30/23 09:20, Iago Toral wrote: El mar, 24-10-2023 a las 07:05 -0300, Maira Canal escribió: Hi Iago, On 10/24/23 02:57, Iago Toral wrote: El lun, 23-10-2023 a las 07:58 -0300, Maíra Canal escribió: Currently, we are only warning the user if the BIN or RENDER jobs don't finish before we unregister V3D. We must wait for all jobs to finish before unregistering. Therefore, warn the user if TFU or CSD jobs are not done by the time the driver is unregistered. Signed-off-by: Maíra Canal --- drivers/gpu/drm/v3d/v3d_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 2e94ce788c71..afa7d170d1ff 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -1072,6 +1072,8 @@ v3d_gem_destroy(struct drm_device *dev) */ WARN_ON(v3d->bin_job); WARN_ON(v3d->render_job); + WARN_ON(v3d->tfu_job); + WARN_ON(v3d->csd_job); I guess we should do this for cache clean jobs too, right? As the cache clean jobs are synchronous, we don't keep track of the current cache clean job. When I say that the cache clean jobs are synchronous, it means that the end of the job is not determined by an interruption. Therefore, there is no need to make sure that the cache clean jobs are still running. I see, thanks Maíra. Reviewed-by: Iago Toral Quiroga Applied to drm-misc/drm-misc-next! Thanks, - Maíra Best Regards, - Maíra Iago drm_mm_takedown(&v3d->mm);
Re: [PATCH v2 0/4] V3D module changes for Pi5
Hi Iago, The whole series is: Reviewed-by: Maíra Canal You can add my r-b in the next version (adding the DTS maintainers in CC). Best Regards, - Maíra On 10/30/23 05:28, Iago Toral Quiroga wrote: This series includes patches to update the V3D kernel module that drives the VideoCore VI GPU in Raspberry Pi 4 to also support the Video Core VII iteration present in Raspberry Pi 5. The first patch in the series adds a small uAPI update required for TFU jobs, the second patch addresses the bulk of the work and involves mostly updates to register addresses, the third and fourth patches match the 'brcm,2712-v3d' device string from Pi5 with the V3D driver. The changes for the user-space driver can be found in the corresponding Mesa MR here: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25450 Iago Toral Quiroga (4): drm/v3d: update UAPI to match user-space for V3D 7.x drm/v3d: fix up register addresses for V3D 7.x dt-bindings: gpu: v3d: Add BCM2712's compatible drm/v3d: add brcm,2712-v3d as a compatible V3D device .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml | 1 + drivers/gpu/drm/v3d/v3d_debugfs.c | 178 ++ drivers/gpu/drm/v3d/v3d_drv.c | 1 + drivers/gpu/drm/v3d/v3d_gem.c | 4 +- drivers/gpu/drm/v3d/v3d_irq.c | 46 +++-- drivers/gpu/drm/v3d/v3d_regs.h| 94 + drivers/gpu/drm/v3d/v3d_sched.c | 38 ++-- include/uapi/drm/v3d_drm.h| 5 + 8 files changed, 211 insertions(+), 156 deletions(-)
Re: [PATCH v2 2/2] drm/todo: Add entry to clean up former seltests suites
Hi Maxime, Wouldn't be nice to add to the TODO list an item regarding the deleted drm_mm tests? Something just to remember us to develop new tests for it in the future. Best Regards, - Maíra On 10/25/23 10:24, Maxime Ripard wrote: Most of those suites are undocumented and aren't really clear about what they are testing. Let's add a TODO entry as a future task to get started into KUnit and DRM. Signed-off-by: Maxime Ripard --- Documentation/gpu/todo.rst | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 03fe5d1247be..b62c7fa0c2bc 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -621,6 +621,23 @@ Contact: Javier Martinez Canillas Level: Intermediate +Clean up and document former selftests suites +- + +Some KUnit test suites (drm_buddy, drm_cmdline_parser, drm_damage_helper, +drm_format, drm_framebuffer, drm_dp_mst_helper, drm_mm, drm_plane_helper and +drm_rect) are former selftests suites that have been converted over when KUnit +was first introduced. + +These suites were fairly undocumented, and with different goals than what unit +tests can be. Trying to identify what each test in these suites actually test +for, whether that makes sense for a unit test, and either remove it if it +doesn't or document it if it does would be of great help. + +Contact: Maxime Ripard + +Level: Intermediate + Enable trinity for DRM --
Re: Evoc proposal
Hi David, EVoC is on hold at the current moment due to some bureaucracy issues. I'm CCing other possible mentors, but at the moment, I'm not sure if a EVoC project is possible. Best Regards, - Maíra On 10/24/23 22:57, DAVID WALTERS wrote: Hello, I have a draft of a proposal that I would like feedback on from Maíra Canal (or another mentor). If you could please let me know their email address (or I could send you the draft and you could forward it to them). It's for the KUnit and DRM project. Thanks, David Walters.
Re: [PATCH 1/2] drm/v3d: wait for all jobs to finish before unregistering
Hi Iago, On 10/24/23 02:57, Iago Toral wrote: El lun, 23-10-2023 a las 07:58 -0300, Maíra Canal escribió: Currently, we are only warning the user if the BIN or RENDER jobs don't finish before we unregister V3D. We must wait for all jobs to finish before unregistering. Therefore, warn the user if TFU or CSD jobs are not done by the time the driver is unregistered. Signed-off-by: Maíra Canal --- drivers/gpu/drm/v3d/v3d_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 2e94ce788c71..afa7d170d1ff 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -1072,6 +1072,8 @@ v3d_gem_destroy(struct drm_device *dev) */ WARN_ON(v3d->bin_job); WARN_ON(v3d->render_job); + WARN_ON(v3d->tfu_job); + WARN_ON(v3d->csd_job); I guess we should do this for cache clean jobs too, right? As the cache clean jobs are synchronous, we don't keep track of the current cache clean job. When I say that the cache clean jobs are synchronous, it means that the end of the job is not determined by an interruption. Therefore, there is no need to make sure that the cache clean jobs are still running. Best Regards, - Maíra Iago drm_mm_takedown(&v3d->mm);
Re: [PATCH 1/3] drm/tests: Fix kunit_release_action ctx argument
Hi Arthur,
On 9/27/23 19:52, Arthur Grillo wrote:
On 27/09/23 19:47, Maira Canal wrote:
Hi Arthur,
On 9/20/23 03:11, Arthur Grillo wrote:
The kunit_action_platform_driver_unregister is added with
&fake_platform_driver as ctx, but the kunit_release_action is called
pdev as ctx. Fix that by replacing it with &fake_platform_driver.
Fixes: 4f2b0b583baa ("drm/tests: helpers: Switch to kunit actions")
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
Do you need me to apply this patch to drm-misc-fixes?
Yes, please do, if possible.
Applied to drm-misc/drm-misc-fixes!
Thanks,
- Maíra
Thanks,
~Arthur Grillo
Best Regards,
- Maíra
---
drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c
b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index 3d624ff2f651..3150dbc647ee 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -118,7 +118,7 @@ void drm_kunit_helper_free_device(struct kunit *test,
struct device *dev)
kunit_release_action(test,
kunit_action_platform_driver_unregister,
- pdev);
+ &fake_platform_driver);
}
EXPORT_SYMBOL_GPL(drm_kunit_helper_free_device);
Re: [PATCH RESEND v3 0/2] Add KUnit tests for drm_fb_blit()
Hi Arthur, On 9/18/23 20:51, Arthur Grillo wrote: This patchset tests the drm_fb_blit() function. As this function can be used with already tested formats, the first patch adds calls to drm_fb_blit() on the tests of supported formats. Some supported formats were not yet covered by the existing tests because they are only supported by drm_fb_blit(). The second patch adds those format conversion tests. Signed-off-by: Arthur Grillo Applied to drm-misc/drm-misc-next! Thanks, - Maíra --- Changes in v3: - Fix memset sizes to avoid out-of-bound access - Link to v2: https://lore.kernel.org/r/20230905-final-gsoc-v2-0-b52e8cb06...@riseup.net Changes in v2: - Split the patch into two (Maíra Canal) - Link to v1: https://lore.kernel.org/r/20230901-final-gsoc-v1-1-e310c7685...@riseup.net --- Arthur Grillo (2): drm/tests: Add calls to drm_fb_blit() on supported format conversion tests drm/tests: Add new format conversion tests to better cover drm_fb_blit() drivers/gpu/drm/tests/drm_format_helper_test.c | 285 + 1 file changed, 285 insertions(+) --- base-commit: 37454bcbb68601c326b58ac45f508067047d791f change-id: 20230901-final-gsoc-395a84443c8f Best regards,
Re: [PATCH 9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by
Hi Kees,
On 9/22/23 14:32, Kees Cook wrote:
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct v3d_perfmon.
[1]
https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Emma Anholt
Cc: Melissa Wen
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Kees Cook
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 7f664a4b2a75..106454f28956 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -59,7 +59,7 @@ struct v3d_perfmon {
* values can't be reset, but you can fake a reset by
* destroying the perfmon and creating a new one.
*/
- u64 values[];
+ u64 values[] __counted_by(ncounters);
};
struct v3d_dev {
Re: [PATCH 3/3] drm/v3d: add brcm, 2712-v3d as a compatible V3D device
Please, add a commit message and your s-o-b.
Apart from that,
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
On 9/28/23 08:45, Iago Toral Quiroga wrote:
---
drivers/gpu/drm/v3d/v3d_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index ffbbe9d527d3..0ed2e7ba8b33 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -186,6 +186,7 @@ static const struct drm_driver v3d_drm_driver = {
};
static const struct of_device_id v3d_of_match[] = {
+ { .compatible = "brcm,2712-v3d" },
{ .compatible = "brcm,2711-v3d" },
{ .compatible = "brcm,7268-v3d" },
{ .compatible = "brcm,7278-v3d" },
Re: [PATCH 2/3] drm/v3d: update UAPI to match user-space for V3D 7.x
Hi Iago,
On 9/28/23 08:45, Iago Toral Quiroga wrote:
V3D t.x takes a new parameter to configure TFU jobs that needs
I believe t.x should be 7.x.
to be provided by user space.
As I mentioned before, please, add your s-o-b.
---
include/uapi/drm/v3d_drm.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index 3dfc0af8756a..1a7d7a689de3 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -319,6 +319,11 @@ struct drm_v3d_submit_tfu {
/* Pointer to an array of ioctl extensions*/
__u64 extensions;
+
+ struct {
+ __u32 ioc;
+ __u32 pad;
+ } v71;
Is there any possibility that the name of the struct could be more
meaningful?
Best Regards,
- Maíra
};
/* Submits a compute shader for dispatch. This job will block on any
Re: [PATCH 1/3] drm/v3d: fix up register addresses for V3D 7.x
Hi Iago, On 9/28/23 08:45, Iago Toral Quiroga wrote: Please, add a commit message and your s-o-b to the patch. Here is a reference on how to format your patches [1]. Also, please, run checkpatch on this patch and address the warnings. [1] https://docs.kernel.org/process/submitting-patches.html --- drivers/gpu/drm/v3d/v3d_debugfs.c | 173 +- drivers/gpu/drm/v3d/v3d_gem.c | 3 + drivers/gpu/drm/v3d/v3d_irq.c | 47 drivers/gpu/drm/v3d/v3d_regs.h| 51 - drivers/gpu/drm/v3d/v3d_sched.c | 41 --- 5 files changed, 200 insertions(+), 115 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c index 330669f51fa7..90b2b5b2710c 100644 --- a/drivers/gpu/drm/v3d/v3d_debugfs.c +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c @@ -12,69 +12,83 @@ #include "v3d_drv.h" #include "v3d_regs.h" [...] diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h index 3663e0d6bf76..9fbcbfedaae1 100644 --- a/drivers/gpu/drm/v3d/v3d_regs.h +++ b/drivers/gpu/drm/v3d/v3d_regs.h @@ -57,6 +57,7 @@ #define V3D_HUB_INT_MSK_STS0x0005c #define V3D_HUB_INT_MSK_SET0x00060 #define V3D_HUB_INT_MSK_CLR0x00064 +# define V3D_V7_HUB_INT_GMPV BIT(6) # define V3D_HUB_INT_MMU_WRV BIT(5) # define V3D_HUB_INT_MMU_PTI BIT(4) # define V3D_HUB_INT_MMU_CAP BIT(3) @@ -64,6 +65,7 @@ # define V3D_HUB_INT_TFUC BIT(1) # define V3D_HUB_INT_TFUF BIT(0) +/* GCA registers only exist in V3D < 41 */ #define V3D_GCA_CACHE_CTRL 0xc # define V3D_GCA_CACHE_CTRL_FLUSH BIT(0) @@ -87,6 +89,7 @@ # define V3D_TOP_GR_BRIDGE_SW_INIT_1_V3D_CLK_108_SW_INIT BIT(0) #define V3D_TFU_CS 0x00400 +#define V3D_V7_TFU_CS 0x00700 What do you think about something like #define V3D_TFU_CS(ver) (ver >= 71) ? 0x00700 : 0x00400 I believe that the code would get much cleaner and would avoid a bunch of the if statements that you used. I would apply this format to all the new registers. Best Regards, - Maíra /* Stops current job, empties input fifo. */ # define V3D_TFU_CS_TFURST BIT(31) # define V3D_TFU_CS_CVTCT_MASK V3D_MASK(23, 16) @@ -96,6 +99,7 @@ # define V3D_TFU_CS_BUSY BIT(0) #define V3D_TFU_SU 0x00404 +#define V3D_V7_TFU_SU 0x00704 /* Interrupt when FINTTHR input slots are free (0 = disabled) */ # define V3D_TFU_SU_FINTTHR_MASK V3D_MASK(13, 8) # define V3D_TFU_SU_FINTTHR_SHIFT 8 @@ -107,38 +111,53 @@ # define V3D_TFU_SU_THROTTLE_SHIFT 0 #define V3D_TFU_ICFG 0x00408 +#define V3D_V7_TFU_ICFG0x00708 /* Interrupt when the conversion is complete. */ # define V3D_TFU_ICFG_IOC BIT(0) /* Input Image Address */ #define V3D_TFU_IIA0x0040c +#define V3D_V7_TFU_IIA 0x0070c /* Input Chroma Address */ #define V3D_TFU_ICA0x00410 +#define V3D_V7_TFU_ICA 0x00710 /* Input Image Stride */ #define V3D_TFU_IIS0x00414 +#define V3D_V7_TFU_IIS 0x00714 /* Input Image U-Plane Address */ #define V3D_TFU_IUA0x00418 +#define V3D_V7_TFU_IUA 0x00718 +/* Image output config (VD 7.x only) */ +#define V3D_V7_TFU_IOC 0x0071c /* Output Image Address */ #define V3D_TFU_IOA0x0041c +#define V3D_V7_TFU_IOA 0x00720 /* Image Output Size */ #define V3D_TFU_IOS0x00420 +#define V3D_V7_TFU_IOS 0x00724 /* TFU YUV Coefficient 0 */ #define V3D_TFU_COEF0 0x00424 -/* Use these regs instead of the defaults. */ +#define V3D_V7_TFU_COEF0 0x00728 +/* Use these regs instead of the defaults (V3D 4.x only) */ # define V3D_TFU_COEF0_USECOEF BIT(31) /* TFU YUV Coefficient 1 */ #define V3D_TFU_COEF1 0x00428 +#define V3D_V7_TFU_COEF1 0x0072c /* TFU YUV Coefficient 2 */ #define V3D_TFU_COEF2 0x0042c +#define V3D_V7_TFU_C
Re: [PATCH 1/3] drm/tests: Fix kunit_release_action ctx argument
Hi Arthur,
On 9/20/23 03:11, Arthur Grillo wrote:
The kunit_action_platform_driver_unregister is added with
&fake_platform_driver as ctx, but the kunit_release_action is called
pdev as ctx. Fix that by replacing it with &fake_platform_driver.
Fixes: 4f2b0b583baa ("drm/tests: helpers: Switch to kunit actions")
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
Do you need me to apply this patch to drm-misc-fixes?
Best Regards,
- Maíra
---
drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c
b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index 3d624ff2f651..3150dbc647ee 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -118,7 +118,7 @@ void drm_kunit_helper_free_device(struct kunit *test,
struct device *dev)
kunit_release_action(test,
kunit_action_platform_driver_unregister,
-pdev);
+&fake_platform_driver);
}
EXPORT_SYMBOL_GPL(drm_kunit_helper_free_device);
Re: [PATCH] drm/tests: Fix incorrect argument in drm_test_mm_insert_range
On 9/15/23 11:17, Janusz Krzysztofik wrote:
Hi Maíra,
Thanks for review.
On Friday, 15 September 2023 16:01:31 CEST Maira Canal wrote:
Hi,
On 9/11/23 10:03, Janusz Krzysztofik wrote:
While drm_mm test was converted form igt selftest to kunit, unexpected
value of "end" argument equal "start" was introduced to one of calls to a
function that executes the drm_test_mm_insert_range for specific start/end
pair of arguments. As a consequence, DRM_MM_BUG_ON(end <= start) is
triggered. Fix it by restoring the original value.
Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
Signed-off-by: Janusz Krzysztofik
Reviewed-by: Maíra Canal
Do you need me to push it to drm-misc-fixes?
Yes, please do if you can.
Pushed to drm-misc/drm-misc-fixes. Thanks!
Best Regards,
- Maíra
Thanks,
Janusz
Best Regards,
- Maíra
Cc: "Maíra Canal"
Cc: Arthur Grillo
Cc: Javier Martinez Canillas
Cc: Daniel Latypov
Cc: sta...@vger.kernel.org # v6.1+
---
drivers/gpu/drm/tests/drm_mm_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_mm_test.c
b/drivers/gpu/drm/tests/drm_mm_test.c
index 186b28dc70380..05d5e7af6d250 100644
--- a/drivers/gpu/drm/tests/drm_mm_test.c
+++ b/drivers/gpu/drm/tests/drm_mm_test.c
@@ -939,7 +939,7 @@ static void drm_test_mm_insert_range(struct kunit *test)
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size, 0, max - 1));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size, 0, max / 2));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size,
- max / 2,
max / 2));
+ max / 2,
max));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size,
max / 4 +
1, 3 * max / 4 - 1));
Re: [PATCH] drm/tests: Fix incorrect argument in drm_test_mm_insert_range
Hi,
On 9/11/23 10:03, Janusz Krzysztofik wrote:
While drm_mm test was converted form igt selftest to kunit, unexpected
value of "end" argument equal "start" was introduced to one of calls to a
function that executes the drm_test_mm_insert_range for specific start/end
pair of arguments. As a consequence, DRM_MM_BUG_ON(end <= start) is
triggered. Fix it by restoring the original value.
Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
Signed-off-by: Janusz Krzysztofik
Reviewed-by: Maíra Canal
Do you need me to push it to drm-misc-fixes?
Best Regards,
- Maíra
Cc: "Maíra Canal"
Cc: Arthur Grillo
Cc: Javier Martinez Canillas
Cc: Daniel Latypov
Cc: sta...@vger.kernel.org # v6.1+
---
drivers/gpu/drm/tests/drm_mm_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_mm_test.c
b/drivers/gpu/drm/tests/drm_mm_test.c
index 186b28dc70380..05d5e7af6d250 100644
--- a/drivers/gpu/drm/tests/drm_mm_test.c
+++ b/drivers/gpu/drm/tests/drm_mm_test.c
@@ -939,7 +939,7 @@ static void drm_test_mm_insert_range(struct kunit *test)
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size, 0, max - 1));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size, 0, max / 2));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size,
- max / 2,
max / 2));
+ max / 2,
max));
KUNIT_ASSERT_FALSE(test, __drm_test_mm_insert_range(test,
count, size,
max / 4 +
1, 3 * max / 4 - 1));
Re: [PATCH v2 2/2] drm/tests: Add new format conversion tests to better cover drm_fb_blit()
Hi Arthur,
On 9/5/23 18:27, Arthur Grillo wrote:
To fully cover drm_fb_blit(), add format conversion tests that are only
supported through drm_fb_blit().
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 142 +
1 file changed, 142 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index b888f7334510..889287245b1e 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -81,6 +81,16 @@ struct fb_swab_result {
const u32 expected[TEST_BUF_SIZE];
};
+struct convert_to_xbgr_result {
+ unsigned int dst_pitch;
+ const u32 expected[TEST_BUF_SIZE];
+};
+
+struct convert_to_abgr_result {
+ unsigned int dst_pitch;
+ const u32 expected[TEST_BUF_SIZE];
+};
+
struct convert_xrgb_case {
const char *name;
unsigned int pitch;
@@ -98,6 +108,8 @@ struct convert_xrgb_case {
struct convert_to_argb2101010_result argb2101010_result;
struct convert_to_mono_result mono_result;
struct fb_swab_result swab_result;
+ struct convert_to_xbgr_result xbgr_result;
+ struct convert_to_abgr_result abgr_result;
};
static struct convert_xrgb_case convert_xrgb_cases[] = {
@@ -155,6 +167,14 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFF01 },
},
+ .xbgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0x01FF },
+ },
+ .abgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFFFF },
+ },
},
{
.name = "single_pixel_clip_rectangle",
@@ -213,6 +233,14 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFF10 },
},
+ .xbgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0x10FF },
+ },
+ .abgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFFFF },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -343,6 +371,24 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0x0077, 0x0088,
},
},
+ .xbgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = {
+ 0x11FF, 0x2200,
+ 0x33FF, 0x4400FF00,
+ 0x55FF, 0x66FF00FF,
+ 0x7700, 0x8800,
+ },
+ },
+ .abgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = {
+ 0x, 0xFF00,
+ 0xFFFF, 0xFF00FF00,
+ 0x, 0x00FF,
+ 0xFF00, 0xFF00,
+ },
+ },
},
{
/* Randomly picked colors. Full buffer within the clip area. */
@@ -458,6 +504,22 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0x0303A8C2, 0x73F06CD2, 0x9C440EA3, 0x,
0x,
},
},
+ .xbgr_result = {
+ .dst_pitch = 20,
+ .expected = {
+ 0xA19C440E, 0xB1054D11, 0xC103F3A8, 0x,
0x,
+ 0xD173F06C, 0xA29C440E, 0xB2054D11, 0x,
0x,
+ 0xC20303A8, 0xD273F06C, 0xA39C440E, 0x,
0x,
+ },
+ },
+ .abgr_result = {
+ .dst_pitch = 20,
+ .expected = {
+ 0xFF9C440E, 0xFF054D11, 0xFF03F3A8, 0x,
0x,
+ 0xFF73F06C, 0xFF9C440E, 0xFF054D11, 0x,
0x,
+ 0xFF0303A8, 0xFF73F06C, 0xFF9C440E, 0x,
0x,
+ },
+ },
},
};
@@ -1082,6 +1144,84 @@ static
Re: [PATCH v2 1/2] drm/tests: Add calls to drm_fb_blit() on supported format conversion tests
Hi Arthur, On 9/5/23 18:27, Arthur Grillo wrote: Add a call to drm_fb_blit() on existing format conversion tests that has support. Signed-off-by: Arthur Grillo Reviewed-by: Maíra Canal Best Regards, - Maíra --- drivers/gpu/drm/tests/drm_format_helper_test.c | 142 + 1 file changed, 142 insertions(+) diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c index 79bc9d4bbd71..b888f7334510 100644 --- a/drivers/gpu/drm/tests/drm_format_helper_test.c +++ b/drivers/gpu/drm/tests/drm_format_helper_test.c @@ -643,6 +643,18 @@ static void drm_test_fb_xrgb_to_rgb565(struct kunit *test) drm_fb_xrgb_to_rgb565(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip, true); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected_swab, dst_size); + + buf = dst.vaddr; + memset(buf, 0, TEST_BUF_SIZE); + + int blit_result = 0; + + blit_result = drm_fb_blit(&dst, dst_pitch, DRM_FORMAT_RGB565, &src, &fb, ¶ms->clip); + + buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); + + KUNIT_EXPECT_FALSE(test, blit_result); + KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } static void drm_test_fb_xrgb_to_xrgb1555(struct kunit *test) @@ -677,6 +689,18 @@ static void drm_test_fb_xrgb_to_xrgb1555(struct kunit *test) drm_fb_xrgb_to_xrgb1555(&dst, dst_pitch, &src, &fb, ¶ms->clip); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); + + buf = dst.vaddr; /* restore original value of buf */ + memset(buf, 0, TEST_BUF_SIZE); + + int blit_result = 0; + + blit_result = drm_fb_blit(&dst, dst_pitch, DRM_FORMAT_XRGB1555, &src, &fb, ¶ms->clip); + + buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); + + KUNIT_EXPECT_FALSE(test, blit_result); + KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } static void drm_test_fb_xrgb_to_argb1555(struct kunit *test) @@ -711,6 +735,18 @@ static void drm_test_fb_xrgb_to_argb1555(struct kunit *test) drm_fb_xrgb_to_argb1555(&dst, dst_pitch, &src, &fb, ¶ms->clip); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); + + buf = dst.vaddr; /* restore original value of buf */ + memset(buf, 0, TEST_BUF_SIZE); + + int blit_result = 0; + + blit_result = drm_fb_blit(&dst, dst_pitch, DRM_FORMAT_ARGB1555, &src, &fb, ¶ms->clip); + + buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); + + KUNIT_EXPECT_FALSE(test, blit_result); + KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } static void drm_test_fb_xrgb_to_rgba5551(struct kunit *test) @@ -745,6 +781,18 @@ static void drm_test_fb_xrgb_to_rgba5551(struct kunit *test) drm_fb_xrgb_to_rgba5551(&dst, dst_pitch, &src, &fb, ¶ms->clip); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); + + buf = dst.vaddr; /* restore original value of buf */ + memset(buf, 0, TEST_BUF_SIZE); + + int blit_result = 0; + + blit_result = drm_fb_blit(&dst, dst_pitch, DRM_FORMAT_RGBA5551, &src, &fb, ¶ms->clip); + + buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); + + KUNIT_EXPECT_FALSE(test, blit_result); + KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } static void drm_test_fb_xrgb_to_rgb888(struct kunit *test) @@ -782,6 +830,16 @@ static void drm_test_fb_xrgb_to_rgb888(struct kunit *test) drm_fb_xrgb_to_rgb888(&dst, dst_pitch, &src, &fb, ¶ms->clip); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); + + buf = dst.vaddr; /* restore original value of buf */ + memset(buf, 0, TEST_BUF_SIZE); + + int blit_result = 0; + + blit_result = drm_fb_blit(&dst, dst_pitch, DRM_FORMAT_RGB888, &src, &fb, ¶ms->clip); + + KUNIT_EXPECT_FALSE(test, blit_result); + KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } static void drm_test_fb_xrgb_to_argb(struct kunit *test) @@ -816,6 +874,18 @@ static void drm_test_fb_xrgb_to_argb(struct kunit *test) drm_fb_xrgb_to_argb(&dst, dst_pitch, &src, &fb, ¶ms->clip); buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size / sizeof(u32)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); + + buf = dst.vaddr; /* restore original value of buf */ + memset(buf, 0, TEST_BUF_SIZE);
Re: [PATCH] drm/tests: Zero initialize fourccs_out
Hi Arthur,
On 9/1/23 15:52, Arthur Grillo wrote:
fourccs_out array is not initialized. As the
drm_fb_build_fourcc_list() doesn't necessarily change all the array,
and the test compares all of it, the comparison could fail if the
array is not initialized. Zero initialize the array to fix this.
Fixes: 371e0b186a13 ("drm/tests: Add KUnit tests for
drm_fb_build_fourcc_list()")
Signed-off-by: Arthur Grillo
Applied to drm-misc/drm-misc-next!
Thanks!
- Maíra
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 79bc9d4bbd71..1a6bd291345d 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -1165,7 +1165,7 @@ KUNIT_ARRAY_PARAM(fb_build_fourcc_list,
fb_build_fourcc_list_cases, fb_build_fou
static void drm_test_fb_build_fourcc_list(struct kunit *test)
{
const struct fb_build_fourcc_list_case *params = test->param_value;
- u32 fourccs_out[TEST_BUF_SIZE];
+ u32 fourccs_out[TEST_BUF_SIZE] = {0};
size_t nfourccs_out;
struct drm_device *drm;
struct device *dev;
---
base-commit: 8e455145d8f163aefa6b9cc29478e0a9f82276e6
change-id: 20230901-zero-init-fourcc-list-test-2c934b6b7eb8
Best regards,
Re: [PATCH] drm/debugfs: Add inline to drm_debugfs_dev_init() to suppres -Wunused-function
Hi Arthur,
On 9/1/23 15:05, Arthur Grillo wrote:
When CONFIG_DEBUG_FS is not set -Wunused-function warnings appear,
make the static function inline to suppress that.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202309012114.t8vlfaf8-...@intel.com/
Closes:
https://lore.kernel.org/oe-kbuild-all/202309012131.feakbzej-...@intel.com/
Signed-off-by: Arthur Grillo
Applied to drm-misc/drm-misc-next!
Thanks!
- Maíra
---
include/drm/drm_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 9850fe73b739..e2640dc64e08 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -584,7 +584,7 @@ static inline bool drm_firmware_drivers_only(void)
#if defined(CONFIG_DEBUG_FS)
void drm_debugfs_dev_init(struct drm_device *dev, struct dentry *root);
#else
-static void drm_debugfs_dev_init(struct drm_device *dev, struct dentry *root)
+static inline void drm_debugfs_dev_init(struct drm_device *dev, struct dentry
*root)
{
}
#endif
---
base-commit: 8e455145d8f163aefa6b9cc29478e0a9f82276e6
change-id: 20230901-debugfs-fix-unused-function-warning-9ebbecbd6a5a
Best regards,
Re: [PATCH 05/10] drm/tests: Add test for drm_framebuffer_cleanup()
Hi Carlos,
On 9/4/23 14:22, Carlos wrote:
Hi Maíra,
On 8/26/23 11:06, Maíra Canal wrote:
Hi Carlos,
On 8/25/23 13:07, Carlos Eduardo Gallo Filho wrote:
Add a single KUnit test case for the drm_framebuffer_cleanup function.
Signed-off-by: Carlos Eduardo Gallo Filho
---
drivers/gpu/drm/tests/drm_framebuffer_test.c | 49
1 file changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c
b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index 0e0e8216bbbc..16d9cf4bed88 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -370,6 +370,9 @@ static int drm_framebuffer_test_init(struct kunit
*test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mock);
dev = &mock->dev;
+ mutex_init(&dev->mode_config.fb_lock);
What about drmm_mutex_init()?
I took a look into it and as far I understand it would be useful if
the drm_device was allocated with drmm_kalloc(), sure? As far we
I'm not sure if we can allocate drm_device with drmm_kzalloc(), as we
need a DRM device for drmm_kzalloc(). drmm_kzalloc() is just a
&drm_device managed version of kzalloc().
are using kunit_kalloc here and the drm_device is automatically
deallocated when the test finishes, what would be the better by
using drmm_mutex_init?
Actually, thinking it better now, I think we cannot use
drmm_mutex_init() here, as we are not calling drm_dev_put().
Best Regards,
- Maíra
It isn't that I don't wanna use it, I just didn't understand how
exactly it works and how could I use it in that code. Should I
replace the drm_device allocation to use drmm?
Thanks,
Carlos
Best Regards,
- Maíra
+ INIT_LIST_HEAD(&dev->mode_config.fb_list);
+ dev->mode_config.num_fb = 0;
dev->mode_config.min_width = MIN_WIDTH;
dev->mode_config.max_width = MAX_WIDTH;
dev->mode_config.min_height = MIN_HEIGHT;
@@ -380,6 +383,14 @@ static int drm_framebuffer_test_init(struct
kunit *test)
return 0;
}
+static void drm_framebuffer_test_exit(struct kunit *test)
+{
+ struct drm_mock *mock = test->priv;
+ struct drm_device *dev = &mock->dev;
+
+ mutex_destroy(&dev->mode_config.fb_lock);
+}
+
static void drm_test_framebuffer_create(struct kunit *test)
{
const struct drm_framebuffer_test *params = test->param_value;
@@ -483,7 +494,44 @@ static void check_src_coords_test_to_desc(const
struct check_src_coords_case *t,
KUNIT_ARRAY_PARAM(check_src_coords, check_src_coords_cases,
check_src_coords_test_to_desc);
+static void drm_test_framebuffer_cleanup(struct kunit *test)
+{
+ struct drm_mock *mock = test->priv;
+ struct drm_device *dev = &mock->dev;
+ struct list_head *fb_list = &dev->mode_config.fb_list;
+ struct drm_framebuffer fb1 = { .dev = dev };
+ struct drm_framebuffer fb2 = { .dev = dev };
+
+ /* This must result on [fb_list] -> fb1 -> fb2 */
+ list_add_tail(&fb1.head, fb_list);
+ list_add_tail(&fb2.head, fb_list);
+ dev->mode_config.num_fb = 2;
+
+ KUNIT_ASSERT_PTR_EQ(test, fb_list->prev, &fb2.head);
+ KUNIT_ASSERT_PTR_EQ(test, fb_list->next, &fb1.head);
+ KUNIT_ASSERT_PTR_EQ(test, fb1.head.prev, fb_list);
+ KUNIT_ASSERT_PTR_EQ(test, fb1.head.next, &fb2.head);
+ KUNIT_ASSERT_PTR_EQ(test, fb2.head.prev, &fb1.head);
+ KUNIT_ASSERT_PTR_EQ(test, fb2.head.next, fb_list);
+
+ drm_framebuffer_cleanup(&fb1);
+
+ /* Now [fb_list] -> fb2 */
+ KUNIT_ASSERT_PTR_EQ(test, fb_list->prev, &fb2.head);
+ KUNIT_ASSERT_PTR_EQ(test, fb_list->next, &fb2.head);
+ KUNIT_ASSERT_PTR_EQ(test, fb2.head.prev, fb_list);
+ KUNIT_ASSERT_PTR_EQ(test, fb2.head.next, fb_list);
+ KUNIT_ASSERT_EQ(test, dev->mode_config.num_fb, 1);
+
+ drm_framebuffer_cleanup(&fb2);
+
+ /* Now fb_list is empty */
+ KUNIT_ASSERT_TRUE(test, list_empty(fb_list));
+ KUNIT_ASSERT_EQ(test, dev->mode_config.num_fb, 0);
+}
+
static struct kunit_case drm_framebuffer_tests[] = {
+ KUNIT_CASE(drm_test_framebuffer_cleanup),
KUNIT_CASE(drm_test_framebuffer_modifiers_not_supported),
KUNIT_CASE_PARAM(drm_test_framebuffer_check_src_coords,
check_src_coords_gen_params),
KUNIT_CASE_PARAM(drm_test_framebuffer_create,
drm_framebuffer_create_gen_params),
@@ -493,6 +541,7 @@ static struct kunit_case drm_framebuffer_tests[] = {
static struct kunit_suite drm_framebuffer_test_suite = {
.name = "drm_framebuffer",
.init = drm_framebuffer_test_init,
+ .exit = drm_framebuffer_test_exit,
.test_cases = drm_framebuffer_tests,
};
Re: [PATCH 06/10] drm/tests: Add test for drm_framebuffer_lookup()
Hi Carlos,
On 9/4/23 15:52, Carlos wrote:
Hi Maíra,
On 8/26/23 11:13, Maíra Canal wrote:
Hi Carlos,
On 8/25/23 13:07, Carlos Eduardo Gallo Filho wrote:
Add a single KUnit test case for the drm_framebuffer_lookup function.
Signed-off-by: Carlos Eduardo Gallo Filho
---
drivers/gpu/drm/tests/drm_framebuffer_test.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c
b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index 16d9cf4bed88..3d14d35b4c4d 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
#include
@@ -370,6 +371,10 @@ static int drm_framebuffer_test_init(struct
kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mock);
dev = &mock->dev;
+ dev->driver = kunit_kzalloc(test, sizeof(*dev->driver),
GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev->driver);
+
+ idr_init_base(&dev->mode_config.object_idr, 1);
Shouldn't we start to use drm_framebuffer_init()?
Do you mean about replace drm_mode_object_add() to drm_framebuffer_init()?
Yeah.
If so, what could be the advantages of using it? It seems to just do the
same of drm_mode_object_add() (by actually calling it) but doing some more
things which is not really needed by this test (like adding fb to device
fb_list and etc). Am I missing something important?
I just suggested to minimize open-code in the tests. If we use the
function, we are incentivizing the usage of common code, which leads
to improved maintenance and less code repetition.
Best Regards,
- Maíra
Thanks,
Carlos
Best Regards,
- Maíra
mutex_init(&dev->mode_config.fb_lock);
INIT_LIST_HEAD(&dev->mode_config.fb_list);
dev->mode_config.num_fb = 0;
@@ -530,8 +535,31 @@ static void drm_test_framebuffer_cleanup(struct
kunit *test)
KUNIT_ASSERT_EQ(test, dev->mode_config.num_fb, 0);
}
+static void drm_test_framebuffer_lookup(struct kunit *test)
+{
+ struct drm_mock *mock = test->priv;
+ struct drm_device *dev = &mock->dev;
+ struct drm_framebuffer fb1 = { };
+ struct drm_framebuffer *fb2;
+ uint32_t id = 0;
+ int ret;
+
+ ret = drm_mode_object_add(dev, &fb1.base, DRM_MODE_OBJECT_FB);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+ id = fb1.base.id;
+
+ /* Looking for fb1 */
+ fb2 = drm_framebuffer_lookup(dev, NULL, id);
+ KUNIT_EXPECT_PTR_EQ(test, fb2, &fb1);
+
+ /* Looking for an inexistent framebuffer */
+ fb2 = drm_framebuffer_lookup(dev, NULL, id + 1);
+ KUNIT_EXPECT_NULL(test, fb2);
+}
+
static struct kunit_case drm_framebuffer_tests[] = {
KUNIT_CASE(drm_test_framebuffer_cleanup),
+ KUNIT_CASE(drm_test_framebuffer_lookup),
KUNIT_CASE(drm_test_framebuffer_modifiers_not_supported),
KUNIT_CASE_PARAM(drm_test_framebuffer_check_src_coords,
check_src_coords_gen_params),
KUNIT_CASE_PARAM(drm_test_framebuffer_create,
drm_framebuffer_create_gen_params),
Re: [PATCH 03/10] drm/tests: Add test case for drm_internal_framebuffer_create()
Hi Carlos,
On 9/4/23 13:57, Carlos wrote:
Hi Maíra,
On 8/26/23 10:58, Maíra Canal wrote:
Hi Carlos,
On 8/25/23 13:07, Carlos Eduardo Gallo Filho wrote:
Introduce a test to cover the creation of framebuffer with
modifier on a device that doesn't support it.
Signed-off-by: Carlos Eduardo Gallo Filho
---
drivers/gpu/drm/tests/drm_framebuffer_test.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c
b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index aeaf2331f9cc..b20871e88995 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -396,7 +396,35 @@ static void drm_framebuffer_test_to_desc(const
struct drm_framebuffer_test *t, c
KUNIT_ARRAY_PARAM(drm_framebuffer_create,
drm_framebuffer_create_cases,
drm_framebuffer_test_to_desc);
+/*
+ * This test is very similar to drm_test_framebuffer_create, except
that it
+ * set mock->mode_config.fb_modifiers_not_supported member to 1,
covering
+ * the case of trying to create a framebuffer with modifiers without
the
+ * device really supporting it.
+ */
+static void drm_test_framebuffer_modifiers_not_supported(struct
kunit *test)
+{
+ struct drm_mock *mock = test->priv;
+ struct drm_device *dev = &mock->dev;
+ int buffer_created = 0;
+
+ /* A valid cmd with modifier */
+ struct drm_mode_fb_cmd2 cmd = {
+ .width = MAX_WIDTH, .height = MAX_HEIGHT,
+ .pixel_format = DRM_FORMAT_ABGR, .handles = { 1, 0, 0 },
+ .offsets = { UINT_MAX / 2, 0, 0 }, .pitches = { 4 *
MAX_WIDTH, 0, 0 },
+ .flags = DRM_MODE_FB_MODIFIERS,
+ };
+
+ mock->private = &buffer_created;
+ dev->mode_config.fb_modifiers_not_supported = 1;
+
+ drm_internal_framebuffer_create(dev, &cmd, NULL);
+ KUNIT_EXPECT_EQ(test, 0, buffer_created);
+}
+
static struct kunit_case drm_framebuffer_tests[] = {
+ KUNIT_CASE(drm_test_framebuffer_modifiers_not_supported),
Could we preserve alphabetical order?
I've see a lot of other tests files with this ordered by every KUNIT_CASE()
coming before KUNIT_CASE_PARAM(), with each set ordered among themselves.
Did younoticed that or are you suggesting ordering it even so? Or maybe
you're referring about another unordered thing that I didn't noticed?
Actually, I was suggesting to keep the alphabetical order related to the
tests naming. So, drm_test_framebuffer_create would come ahead of
drm_test_framebuffer_modifiers_not_supported.
Best Regards,
- Maíra
Thanks,
Carlos
Best Regards,
- Maíra
KUNIT_CASE_PARAM(drm_test_framebuffer_create,
drm_framebuffer_create_gen_params),
{ }
};
Re: [PATCH 07/10] drm/tests: Add test for drm_framebuffer_init()
Hi Carlos,
On 9/4/23 14:41, Carlos wrote:
Hi Maíra,
On 8/26/23 11:16, Maíra Canal wrote:
Hi Carlos,
On 8/25/23 13:11, Carlos Eduardo Gallo Filho wrote:
Add a single KUnit test case for the drm_framebuffer_init function.
Signed-off-by: Carlos Eduardo Gallo Filho
---
drivers/gpu/drm/tests/drm_framebuffer_test.c | 52
1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c
b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index 3d14d35b4c4d..50d88bf3fa65 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -557,8 +557,60 @@ static void drm_test_framebuffer_lookup(struct
kunit *test)
KUNIT_EXPECT_NULL(test, fb2);
}
+static void drm_test_framebuffer_init(struct kunit *test)
+{
+ struct drm_mock *mock = test->priv;
+ struct drm_device *dev = &mock->dev;
+ struct drm_device wrong_drm = { };
+ struct drm_format_info format = { };
+ struct drm_framebuffer fb1 = { .dev = dev, .format = &format };
+ struct drm_framebuffer *fb2;
+ struct drm_framebuffer_funcs funcs = { };
+ int ret;
+
+ /* Fails if fb->dev doesn't point to the drm_device passed on
first arg */
+ fb1.dev = &wrong_drm;
+ ret = drm_framebuffer_init(dev, &fb1, &funcs);
+ KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+ fb1.dev = dev;
+
+ /* Fails if fb.format isn't set */
+ fb1.format = NULL;
+ ret = drm_framebuffer_init(dev, &fb1, &funcs);
+ KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+ fb1.format = &format;
+
+ ret = drm_framebuffer_init(dev, &fb1, &funcs);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /*
+ * Check if fb->funcs is actually set to the drm_framebuffer_funcs
+ * passed to it
+ */
+ KUNIT_EXPECT_PTR_EQ(test, fb1.funcs, &funcs);
+
+ /* The fb->comm must be set to the current running process */
+ KUNIT_EXPECT_STREQ(test, fb1.comm, current->comm);
+
+ /* The fb->base must be successfully initialized */
+ KUNIT_EXPECT_EQ(test, fb1.base.id, 1);
+ KUNIT_EXPECT_EQ(test, fb1.base.type, DRM_MODE_OBJECT_FB);
+ KUNIT_EXPECT_EQ(test, kref_read(&fb1.base.refcount), 1);
+ KUNIT_EXPECT_PTR_EQ(test, fb1.base.free_cb, &drm_framebuffer_free);
BTW I believe we should also make sure that dev->mode_config.num_fb was
incremented by 1.
+
+ /* Checks if the fb is really published and findable */
+ fb2 = drm_framebuffer_lookup(dev, NULL, fb1.base.id);
+ KUNIT_EXPECT_PTR_EQ(test, fb2, &fb1);
+
+ /* There must be just that one fb initialized */
+ KUNIT_EXPECT_EQ(test, dev->mode_config.num_fb, 1);
+ KUNIT_EXPECT_PTR_EQ(test, dev->mode_config.fb_list.prev,
&fb1.head);
+ KUNIT_EXPECT_PTR_EQ(test, dev->mode_config.fb_list.next,
&fb1.head);
Shouldn't we clean the framebuffer object?
What did you mean by "clean"? Firstly I supposed that it would be about
freeing some dynamically allocated frambuffer, but it's statically
allocated, so I believe it isn't what you are meaning. Is there some
collateral effect I'm not taking into account?
I was talking about calling the function `drm_framebuffer_cleanup()`.
Best Regards,
- Maíra
Thanks,
Carlos
Best Regards,
- Maíra
+}
+
static struct kunit_case drm_framebuffer_tests[] = {
KUNIT_CASE(drm_test_framebuffer_cleanup),
+ KUNIT_CASE(drm_test_framebuffer_init),
KUNIT_CASE(drm_test_framebuffer_lookup),
KUNIT_CASE(drm_test_framebuffer_modifiers_not_supported),
KUNIT_CASE_PARAM(drm_test_framebuffer_check_src_coords,
check_src_coords_gen_params),
Re: [PATCH] drm/tests: Add KUnit tests for drm_fb_blit()
Hi Arthur,
On 9/1/23 14:08, Arthur Grillo wrote:
Insert parameterized test for the drm_fb_blit() to ensure correctness
and prevent future regressions.
The test is done by adding a call to drm_fb_blit() on every format
that has support. Also, to fully test the function, add new format
conversion tests.
Wouldn't be better to separate this patches into two patches: one adding
new format conversion tests and another adding a call to drm_fb_blit()?
Best Regards,
- Maíra
Signed-off-by: Arthur Grillo
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 284 +
1 file changed, 284 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 79bc9d4bbd71..889287245b1e 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -81,6 +81,16 @@ struct fb_swab_result {
const u32 expected[TEST_BUF_SIZE];
};
+struct convert_to_xbgr_result {
+ unsigned int dst_pitch;
+ const u32 expected[TEST_BUF_SIZE];
+};
+
+struct convert_to_abgr_result {
+ unsigned int dst_pitch;
+ const u32 expected[TEST_BUF_SIZE];
+};
+
struct convert_xrgb_case {
const char *name;
unsigned int pitch;
@@ -98,6 +108,8 @@ struct convert_xrgb_case {
struct convert_to_argb2101010_result argb2101010_result;
struct convert_to_mono_result mono_result;
struct fb_swab_result swab_result;
+ struct convert_to_xbgr_result xbgr_result;
+ struct convert_to_abgr_result abgr_result;
};
static struct convert_xrgb_case convert_xrgb_cases[] = {
@@ -155,6 +167,14 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFF01 },
},
+ .xbgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0x01FF },
+ },
+ .abgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFFFF },
+ },
},
{
.name = "single_pixel_clip_rectangle",
@@ -213,6 +233,14 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFF10 },
},
+ .xbgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0x10FF },
+ },
+ .abgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFFFF },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -343,6 +371,24 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0x0077, 0x0088,
},
},
+ .xbgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = {
+ 0x11FF, 0x2200,
+ 0x33FF, 0x4400FF00,
+ 0x55FF, 0x66FF00FF,
+ 0x7700, 0x8800,
+ },
+ },
+ .abgr_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = {
+ 0x, 0xFF00,
+ 0xFFFF, 0xFF00FF00,
+ 0x, 0x00FF,
+ 0xFF00, 0xFF00,
+ },
+ },
},
{
/* Randomly picked colors. Full buffer within the clip area. */
@@ -458,6 +504,22 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0x0303A8C2, 0x73F06CD2, 0x9C440EA3, 0x,
0x,
},
},
+ .xbgr_result = {
+ .dst_pitch = 20,
+ .expected = {
+ 0xA19C440E, 0xB1054D11, 0xC103F3A8, 0x,
0x,
+ 0xD173F06C, 0xA29C440E, 0xB2054D11, 0x,
0x,
+ 0xC20303A8, 0xD273F06C, 0xA39C440E, 0x,
0x,
+ },
+ },
+ .abgr_result = {
+ .dst_pitch = 20,
+ .expected = {
+ 0xFF9C440E, 0xFF054D11, 0xFF03F3A8, 0x,
0x,
+
Re: [PATCH] drm/debugfs: Add inline to drm_debugfs_dev_init() to suppres -Wunused-function
On 9/1/23 15:05, Arthur Grillo wrote:
When CONFIG_DEBUG_FS is not set -Wunused-function warnings appear,
make the static function inline to suppress that.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202309012114.t8vlfaf8-...@intel.com/
Closes:
https://lore.kernel.org/oe-kbuild-all/202309012131.feakbzej-...@intel.com/
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
include/drm/drm_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 9850fe73b739..e2640dc64e08 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -584,7 +584,7 @@ static inline bool drm_firmware_drivers_only(void)
#if defined(CONFIG_DEBUG_FS)
void drm_debugfs_dev_init(struct drm_device *dev, struct dentry *root);
#else
-static void drm_debugfs_dev_init(struct drm_device *dev, struct dentry *root)
+static inline void drm_debugfs_dev_init(struct drm_device *dev, struct dentry
*root)
{
}
#endif
---
base-commit: 8e455145d8f163aefa6b9cc29478e0a9f82276e6
change-id: 20230901-debugfs-fix-unused-function-warning-9ebbecbd6a5a
Best regards,
Re: [PATCH] drm/tests: Zero initialize fourccs_out
On 9/1/23 15:52, Arthur Grillo wrote:
fourccs_out array is not initialized. As the
drm_fb_build_fourcc_list() doesn't necessarily change all the array,
and the test compares all of it, the comparison could fail if the
array is not initialized. Zero initialize the array to fix this.
Fixes: 371e0b186a13 ("drm/tests: Add KUnit tests for
drm_fb_build_fourcc_list()")
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 79bc9d4bbd71..1a6bd291345d 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -1165,7 +1165,7 @@ KUNIT_ARRAY_PARAM(fb_build_fourcc_list,
fb_build_fourcc_list_cases, fb_build_fou
static void drm_test_fb_build_fourcc_list(struct kunit *test)
{
const struct fb_build_fourcc_list_case *params = test->param_value;
- u32 fourccs_out[TEST_BUF_SIZE];
+ u32 fourccs_out[TEST_BUF_SIZE] = {0};
size_t nfourccs_out;
struct drm_device *drm;
struct device *dev;
---
base-commit: 8e455145d8f163aefa6b9cc29478e0a9f82276e6
change-id: 20230901-zero-init-fourcc-list-test-2c934b6b7eb8
Best regards,
Re: [PATCH v2 1/6] drm/tests: Test default pitch fallback
Hi Arthur,
On 8/11/23 15:17, Arthur Grillo wrote:
Test the default pitch fallback when NULL is passed as the dst_pitch on
the conversion procedures.
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 126 -
1 file changed, 81 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 474bb7a1c4ee..938d4fdb4291 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -16,6 +16,8 @@
#define TEST_BUF_SIZE 50
+#define TEST_USE_DEFAULT_PITCH 0
+
struct convert_to_gray8_result {
unsigned int dst_pitch;
const u8 expected[TEST_BUF_SIZE];
@@ -97,48 +99,48 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.clip = DRM_RECT_INIT(0, 0, 1, 1),
.xrgb = { 0x01FF },
.gray8_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x4C },
},
.rgb332_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xE0 },
},
.rgb565_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF800 },
.expected_swab = { 0x00F8 },
},
.xrgb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x7C00 },
},
.argb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFC00 },
},
.rgba5551_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF801 },
},
.rgb888_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x00, 0x00, 0xFF },
},
.argb_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x },
},
.xrgb2101010_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x3FF0 },
},
.argb2101010_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFFF0 },
},
.mono_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0b0 },
},
},
@@ -151,48 +153,48 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0x, 0x10FF,
},
.gray8_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x4C },
},
.rgb332_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xE0 },
},
.rgb565_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF800 },
.expected_swab = { 0x00F8 },
},
.xrgb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x7C00 },
},
.argb1555_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xFC00 },
},
.rgba5551_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0xF801 },
},
.rgb888_result = {
- .dst_pitch = 0,
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0x00, 0x00, 0xFF },
},
.argb88
Re: [PATCH v2 4/6] drm/tests: Add KUnit tests for drm_fb_build_fourcc_list()
On 8/11/23 15:17, Arthur Grillo wrote:
Insert parameterized test for the drm_fb_build_fourcc_list() to ensure
correctness and prevent future regressions.
Signed-off-by: Arthur Grillo
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 148 +
1 file changed, 148 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 7f24da0b1e00..2b55d9f025f9 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -3,11 +3,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -1041,6 +1043,151 @@ static void drm_test_fb_clip_offset(struct kunit *test)
KUNIT_EXPECT_EQ(test, offset, params->expected_offset);
}
+struct fb_build_fourcc_list_case {
+ const char *name;
+ u32 native_fourccs[TEST_BUF_SIZE];
+ u32 expected[TEST_BUF_SIZE];
+ size_t fourccs_size;
+};
+
+static struct fb_build_fourcc_list_case fb_build_fourcc_list_cases[] = {
+ {
+ .name = "no native formats",
+ .native_fourccs = { },
+ .expected = { DRM_FORMAT_XRGB },
+ .fourccs_size = 1,
+ },
+ {
+ .name = "XRGB as native format",
+ .native_fourccs = { DRM_FORMAT_XRGB },
+ .expected = { DRM_FORMAT_XRGB },
+ .fourccs_size = 1,
+ },
+ {
+ .name = "remove duplicates",
+ .native_fourccs = {
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_XRGB,
+ },
+ .expected = {
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB565,
+ },
+ .fourccs_size = 3,
+ },
+ {
+ .name = "convert alpha formats",
+ .native_fourccs = {
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGRA5551,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_ARGB2101010,
+ DRM_FORMAT_ABGR2101010,
+ DRM_FORMAT_RGBA1010102,
+ DRM_FORMAT_BGRA1010102,
+ },
+ .expected = {
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_XBGR1555,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGRX5551,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_RGBX1010102,
+ DRM_FORMAT_BGRX1010102,
+ },
+ .fourccs_size = 12,
+ },
+ {
+ .name = "random formats",
+ .native_fourccs = {
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR16161616F,
+ DRM_FORMAT_C8,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGR565_A8,
+ DRM_FORMAT_R10,
+ DRM_FORMAT_XYUV,
+ },
+ .expected = {
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_ABGR16161616F,
+ DRM_FORMAT_C8,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGR565_A8,
+ DRM_FORMAT_R10,
+ DRM_FORMAT_XYUV,
+ DRM_FORMAT_XRGB,
+ },
+ .fourccs_size = 10,
+ },
+};
+
+static void fb_build_fourcc_list_case_desc(struct fb_build_fourcc_list_case
*t, char *desc)
+{
+ strscpy(desc, t->name, KUNIT_PARAM_DESC
Re: [PATCH v2 6/6] drm/tests: Add KUnit tests for drm_fb_memcpy()
Hi Arthur,
On 8/11/23 15:17, Arthur Grillo wrote:
Insert parameterized test for the drm_fb_memcpy() to ensure correctness
and prevent future regressions. The test case can accept different
formats.
Signed-off-by: Arthur Grillo
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 391 +
1 file changed, 391 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 08071b6c00f8..09214ae65091 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -1188,6 +1188,396 @@ static void drm_test_fb_build_fourcc_list(struct kunit
*test)
KUNIT_EXPECT_MEMEQ(test, fourccs_out, params->expected, TEST_BUF_SIZE);
}
+struct fb_memcpy_result {
+ unsigned int dst_pitches[DRM_FORMAT_MAX_PLANES];
+ const u32 expected[DRM_FORMAT_MAX_PLANES][TEST_BUF_SIZE];
+};
+
+struct multi_plane_op_case {
I'm not sure if this is the best name to describe the test. Maybe a name
related to drm_fb_memcpy would be more appropriate.
+ const char *name;
+ u32 format;
+ struct drm_rect clip;
+ unsigned int src_pitches[DRM_FORMAT_MAX_PLANES];
+ const u32 src[DRM_FORMAT_MAX_PLANES][TEST_BUF_SIZE];
+ struct fb_memcpy_result memcpy_result;
Could you write
struct {
unsigned int dst_pitches[DRM_FORMAT_MAX_PLANES];
const u32 expected[DRM_FORMAT_MAX_PLANES][TEST_BUF_SIZE];
} memcpy_result;
instead of creating a named struct?
+};
+
+/* The `src` and `expected` buffers are u32 arrays. To deal with planes that
+ * have a cpp != 4 the values are stored together on the same u32 number in a
+ * way so the order in memory is correct in a little-endian machine.
+ *
+ * Because of that, on some occasions, parts of a u32 will not be part of the
+ * test, to make this explicit the 0xFF byte is used on those parts.
+ */
+
+static struct multi_plane_op_case multi_plane_op_cases[] = {
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1 * 4 },
+ .src = {{ 0x01020304 }},
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {{ 0x01020304 }},
+ }
+ },
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_XRGB_A8,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1 * 4, 1 },
+ .src = {
+ { 0x01020304 },
+ { 0xFF01 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ { 0x0001 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_YUV444,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1, 1, 1 },
+ .src = {
+ { 0xFF01 },
+ { 0xFF01 },
+ { 0xFF01 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x0001 },
+ { 0x0001 },
+ { 0x0001 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_rectangle",
+ .format = DRM_FORMAT_XBGR,
+ .clip = DRM_RECT_INIT(1, 1, 1, 1),
+ .src_pitches = { 2 * 4 },
+ .src = {
+ {
+ 0x, 0x,
+ 0x, 0x01020304,
+ },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_rectangle",
+ .format = DRM_FORMAT_XRGB_A8,
+ .clip = DRM_RECT_INIT(1, 1, 1, 1),
+ .src_pitches = { 2 * 4, 2 * 1 },
+ .src = {
+ {
+ 0x, 0x,
+ 0x, 0x01020304,
+ },
+ { 0x0100 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+
Re: [PATCH v2 2/9] drm/sched: Move schedule policy to scheduler / entity
Hi Matthew,
I'm not sure in which tree you plan to apply this series, but if you
plan to apply it on drm-misc-next, it would be nice to rebase on top of
it. It would make it easier for driver maintainers to review it.
Apart from the small nit below it, I tested the Xe tree on v3d and
things seems to be running smoothly.
On 8/10/23 23:31, Matthew Brost wrote:
Rather than a global modparam for scheduling policy, move the scheduling
policy to scheduler / entity so user can control each scheduler / entity
policy.
v2:
- s/DRM_SCHED_POLICY_MAX/DRM_SCHED_POLICY_COUNT (Luben)
- Only include policy in scheduler (Luben)
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
drivers/gpu/drm/etnaviv/etnaviv_sched.c| 3 ++-
drivers/gpu/drm/lima/lima_sched.c | 3 ++-
drivers/gpu/drm/msm/msm_ringbuffer.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_sched.c| 3 ++-
drivers/gpu/drm/panfrost/panfrost_job.c| 3 ++-
drivers/gpu/drm/scheduler/sched_entity.c | 24 ++
drivers/gpu/drm/scheduler/sched_main.c | 23 +++--
drivers/gpu/drm/v3d/v3d_sched.c| 15 +-
include/drm/gpu_scheduler.h| 20 --
10 files changed, 72 insertions(+), 26 deletions(-)
[...]
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 38e092ea41e6..5e3fe77fa991 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -391,7 +391,8 @@ v3d_sched_init(struct v3d_dev *v3d)
&v3d_bin_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_bin", v3d->drm.dev);
+NULL, "v3d_bin", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
return ret;
@@ -399,7 +400,8 @@ v3d_sched_init(struct v3d_dev *v3d)
&v3d_render_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_render", v3d->drm.dev);
+ULL, "v3d_render", DRM_SCHED_POLICY_DEFAULT,
Small nit: s/ULL/NULL
Best Regards,
- Maíra
+v3d->drm.dev);
if (ret)
goto fail;
@@ -407,7 +409,8 @@ v3d_sched_init(struct v3d_dev *v3d)
&v3d_tfu_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_tfu", v3d->drm.dev);
+NULL, "v3d_tfu", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
goto fail;
@@ -416,7 +419,8 @@ v3d_sched_init(struct v3d_dev *v3d)
&v3d_csd_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_csd", v3d->drm.dev);
+NULL, "v3d_csd", DRM_SCHED_POLICY_DEFAULT,
+v3d->drm.dev);
if (ret)
goto fail;
@@ -424,7 +428,8 @@ v3d_sched_init(struct v3d_dev *v3d)
&v3d_cache_clean_sched_ops, NULL,
hw_jobs_limit, job_hang_limit,
msecs_to_jiffies(hang_limit_ms), NULL,
-NULL, "v3d_cache_clean", v3d->drm.dev);
+NULL, "v3d_cache_clean",
+DRM_SCHED_POLICY_DEFAULT, v3d->drm.dev);
if (ret)
goto fail;
}
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 278106e358a8..897d52a4ff4f 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -72,11 +72,15 @@ enum drm_sched_priority {
DRM_SCHED_PRIORITY_UNSET = -2
};
-/* Used to chose between FIFO and RR jobs scheduling */
-extern int drm_sched_policy;
-
-#define DRM_SCHED_POLICY_RR0
-#define DRM_SCHED_POLICY_FIFO 1
+/* Used to chose default scheduling policy*/
+extern int default_drm_sched_policy;
+
+enum drm_sched_policy {
+ DRM_SCHED_POLICY_DEFAULT,
+ DRM_SCHED_POLICY_RR,
+ DRM_SCHED_POLICY_FIFO,
+ DRM_SCHED_POLICY_COUNT,
+};
/**
* struct drm_sched_entity - A wrapper around a job queue (typically
@@ -489,6 +493,7 @@ struct drm_sched_backend_ops {
* guilty and it will no longer be considered for scheduling.
* @score: score to help loadbalancer pick a idle s
Re: [PATCH 6/6] drm/format-helper: Add KUnit tests for drm_fb_memcpy()
On 7/21/23 15:23, Arthur Grillo wrote:
Insert parameterized test for the drm_fb_memcpy() to ensure correctness
and prevent future regressions. The test case can accept different
formats.
Signed-off-by: Arthur Grillo
---
.../gpu/drm/tests/drm_format_helper_test.c| 391 ++
1 file changed, 391 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 6ecd92898e8e..3db4b95f3a98 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -1189,6 +1189,396 @@ static void drm_test_fb_build_fourcc_list(struct kunit
*test)
KUNIT_EXPECT_MEMEQ(test, fourccs_out, params->expected, TEST_BUF_SIZE);
}
+struct fb_memcpy_result {
+ unsigned int dst_pitches[DRM_FORMAT_MAX_PLANES];
+ const u32 expected[DRM_FORMAT_MAX_PLANES][TEST_BUF_SIZE];
+};
+
+struct multi_plane_op_case {
+ const char *name;
+ u32 format;
+ struct drm_rect clip;
+ unsigned int src_pitches[DRM_FORMAT_MAX_PLANES];
+ const u32 src[DRM_FORMAT_MAX_PLANES][TEST_BUF_SIZE];
+ struct fb_memcpy_result memcpy_result;
+};
+
+/* The `src` and `expected` buffers are u32 arrays. To deal with planes that
+ * have a cpp != 4 the values are stored together on the same u32 number in a
+ * way so the order in memory is correct in a little-endian machine.
+ *
+ * Because of that, on some occasions, parts of a u32 will not be part of the
+ * test, to make this explicit the 0xFF byte is used on those parts.
+ */
+
+static struct multi_plane_op_case multi_plane_op_cases[] = {
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1 * 4 },
+ .src = {{ 0x01020304 }},
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {{ 0x01020304 }},
+ }
+ },
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_XRGB_A8,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1 * 4, 1 },
+ .src = {
+ { 0x01020304 },
+ { 0xFF01 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ { 0x0001 },
+ },
+ },
+ },
Some tests have the same description name. Could you distinct them with
different names?
Best Regards,
- Maíra
+ {
+ .name = "single_pixel_source_buffer",
+ .format = DRM_FORMAT_YUV444,
+ .clip = DRM_RECT_INIT(0, 0, 1, 1),
+ .src_pitches = { 1, 1, 1 },
+ .src = {
+ { 0xFF01 },
+ { 0xFF01 },
+ { 0xFF01 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x0001 },
+ { 0x0001 },
+ { 0x0001 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_rectangle",
+ .format = DRM_FORMAT_XBGR,
+ .clip = DRM_RECT_INIT(1, 1, 1, 1),
+ .src_pitches = { 2 * 4 },
+ .src = {
+ {
+ 0x, 0x,
+ 0x, 0x01020304,
+ },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_rectangle",
+ .format = DRM_FORMAT_XRGB_A8,
+ .clip = DRM_RECT_INIT(1, 1, 1, 1),
+ .src_pitches = { 2 * 4, 2 * 1 },
+ .src = {
+ {
+ 0x, 0x,
+ 0x, 0x01020304,
+ },
+ { 0x0100 },
+ },
+ .memcpy_result = {
+ .dst_pitches = { TEST_USE_DEFAULT_PITCH },
+ .expected = {
+ { 0x01020304 },
+ { 0x0001 },
+ },
+ },
+ },
+ {
+ .name = "single_pixel_clip_r
Re: [PATCH 5/6] drm/format-helper-test: Add multi-plane support to conversion_buf_size()
On 7/21/23 15:23, Arthur Grillo wrote:
The drm_fb_memcpy() supports multi-plane formats. To fully test it in
the future, add multi-plane support to the conversion_buf_size() helper.
Signed-off-by: Arthur Grillo
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
.../gpu/drm/tests/drm_format_helper_test.c| 28 +--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index de4677868647..6ecd92898e8e 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -472,7 +472,7 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
* The size of the destination buffer or negative value on error.
*/
static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
- const struct drm_rect *clip)
+ const struct drm_rect *clip, int plane)
{
const struct drm_format_info *dst_fi = drm_format_info(dst_format);
@@ -480,7 +480,7 @@ static size_t conversion_buf_size(u32 dst_format, unsigned int dst_pitch,
return -EINVAL;
if (!dst_pitch)
- dst_pitch = drm_format_info_min_pitch(dst_fi, 0,
drm_rect_width(clip));
+ dst_pitch = drm_format_info_min_pitch(dst_fi, plane,
drm_rect_width(clip));
return dst_pitch * drm_rect_height(clip);
}
@@ -554,7 +554,7 @@ static void drm_test_fb_xrgb_to_gray8(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_R8, result->dst_pitch,
- ¶ms->clip);
+ ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -587,7 +587,7 @@ static void drm_test_fb_xrgb_to_rgb332(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGB332, result->dst_pitch,
- ¶ms->clip);
+ ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -620,7 +620,7 @@ static void drm_test_fb_xrgb_to_rgb565(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGB565, result->dst_pitch,
- ¶ms->clip);
+ ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -663,7 +663,7 @@ static void drm_test_fb_xrgb_to_xrgb1555(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_XRGB1555, result->dst_pitch,
- ¶ms->clip);
+ ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -697,7 +697,7 @@ static void drm_test_fb_xrgb_to_argb1555(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_ARGB1555, result->dst_pitch,
- ¶ms->clip);
+ ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -731,7 +731,7 @@ static void drm_test_fb_xrgb_to_rgba5551(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGBA5551, result->dst_pitch,
- ¶ms->clip);
+ ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -765,7 +765,7 @@ static void drm_test_fb_xrgb_to_rgb888(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_RGB888, result->dst_pitch,
- ¶ms->clip);
+ ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -803,7 +803,7 @@ static void drm_test_fb_xrgb_to_argb(struct kunit
*test)
};
dst_size = conversion_buf_size(DRM_FORMAT_ARGB,
- result->dst_pitch, ¶ms->clip);
+ result->dst_pitch, ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -838,7 +838,7 @@ static void drm_test_fb_xrgb_to_xrgb2101010(struct
kunit *test)
};
dst_size = conversion_buf_size(DRM_FORMAT_XRGB2101010,
- result->dst_pitch, ¶ms->clip);
+ result->dst_pitch, ¶ms->clip, 0);
KUNIT_ASSERT_GT(test, dst_size, 0);
buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
@@ -872,7 +872,7 @@ static void drm_test_fb_xrgb_to_argb2
Re: [PATCH 4/6] drm/format-helper: Add KUnit tests for drm_fb_build_fourcc_list()
On 7/21/23 15:23, Arthur Grillo wrote:> Insert parameterized test for
the drm_fb_build_fourcc_list() to ensure
correctness and prevent future regressions.
Signed-off-by: Arthur Grillo
---
.../gpu/drm/tests/drm_format_helper_test.c| 143 ++
1 file changed, 143 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 2e1c5463f063..de4677868647 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#include
#include
#include
@@ -11,6 +12,7 @@
#include
#include
#include
+#include
#include "../drm_crtc_internal.h"
@@ -1047,6 +1049,146 @@ static void drm_test_fb_clip_offset(struct kunit *test)
KUNIT_EXPECT_EQ(test, offset, params->expected_offset);
}
+struct fb_build_fourcc_list_case {
+ const char *name;
+ u32 native_fourccs[TEST_BUF_SIZE];
+ u32 expected[TEST_BUF_SIZE];
+};
+
+struct fb_build_fourcc_list_case fb_build_fourcc_list_cases[] = {
+ {
+ .name = "no native formats",
+ .native_fourccs = { },
+ .expected = { DRM_FORMAT_XRGB },
+ },
+ {
+ .name = "XRGB as native format",
+ .native_fourccs = { DRM_FORMAT_XRGB },
+ .expected = { DRM_FORMAT_XRGB },
+ },
+ {
+ .name = "remove duplicates",
+ .native_fourccs = {
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_XRGB,
+ },
+ .expected = {
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_RGB565,
+ },
+ },
+ {
+ .name = "convert alpha formats",
+ .native_fourccs = {
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGRA5551,
+ DRM_FORMAT_ARGB,
+ DRM_FORMAT_ABGR,
+ DRM_FORMAT_RGBA,
+ DRM_FORMAT_BGRA,
+ DRM_FORMAT_ARGB2101010,
+ DRM_FORMAT_ABGR2101010,
+ DRM_FORMAT_RGBA1010102,
+ DRM_FORMAT_BGRA1010102,
+ },
+ .expected = {
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_XBGR1555,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGRX5551,
+ DRM_FORMAT_XRGB,
+ DRM_FORMAT_XBGR,
+ DRM_FORMAT_RGBX,
+ DRM_FORMAT_BGRX,
+ DRM_FORMAT_XRGB2101010,
+ DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_RGBX1010102,
+ DRM_FORMAT_BGRX1010102,
+ },
+ },
+ {
+ .name = "random formats",
+ .native_fourccs = {
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_ARGB1555,
+ DRM_FORMAT_ABGR16161616F,
+ DRM_FORMAT_C8,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_RGBA5551,
+ DRM_FORMAT_BGR565_A8,
+ DRM_FORMAT_R10,
+ DRM_FORMAT_XYUV,
+ },
+ .expected = {
+ DRM_FORMAT_Y212,
+ DRM_FORMAT_XRGB1555,
+ DRM_FORMAT_ABGR16161616F,
+ DRM_FORMAT_C8,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGBX5551,
+ DRM_FORMAT_BGR565_A8,
+ DRM_FORMAT_R10,
+ DRM_FORMAT_XYUV,
+ DRM_FORMAT_XRGB,
+ },
+ },
+};
+
+static void fb_build_fourcc_list_case_desc(struct fb_build_fourcc_list_case
*t, char *desc)
+{
+ strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(fb_build_fourcc_list, fb_build_fourcc_list_cases,
fb_build_fourcc_list_case_desc);
+
+static size_t get_nfourccs(const u32 *fourccs)
+{
+ size_t i;
+
Re: [PATCH 3/6] drm/format-helper: Add KUnit tests for drm_fb_clip_offset()
On 7/21/23 15:23, Arthur Grillo wrote:
Insert parameterized test for the drm_fb_clip_offset() to ensure
correctness and prevent future regressions.
Signed-off-by: Arthur Grillo
Could you please use the prefix drm/tests in the commit message for all
patches? Besides that:
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
.../gpu/drm/tests/drm_format_helper_test.c| 91 +++
1 file changed, 91 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index abeda642d84a..2e1c5463f063 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -957,6 +957,96 @@ static void drm_test_fb_swab(struct kunit *test)
KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}
+struct clip_offset_case {
+ const char *name;
+ unsigned int pitch;
+ u32 format;
+ struct drm_rect clip;
+ unsigned int expected_offset;
+};
+
+static struct clip_offset_case clip_offset_cases[] = {
+ {
+ .name = "pass through",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 0, 3, 3),
+ .expected_offset = 0
+ },
+ {
+ .name = "horizontal offset",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 0, 3, 3),
+ .expected_offset = 4,
+ },
+ {
+ .name = "vertical offset",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 1, 3, 3),
+ .expected_offset = 12,
+ },
+ {
+ .name = "horizontal and vertical offset",
+ .pitch = TEST_USE_DEFAULT_PITCH,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 1, 3, 3),
+ .expected_offset = 16,
+ },
+ {
+ .name = "horizontal offset (custom pitch)",
+ .pitch = 20,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 0, 3, 3),
+ .expected_offset = 4,
+ },
+ {
+ .name = "vertical offset (custom pitch)",
+ .pitch = 20,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(0, 1, 3, 3),
+ .expected_offset = 20,
+ },
+ {
+ .name = "horizontal and vertical offset (custom pitch)",
+ .pitch = 20,
+ .format = DRM_FORMAT_XRGB,
+ .clip = DRM_RECT_INIT(1, 1, 3, 3),
+ .expected_offset = 24,
+ },
+};
+
+static void clip_offset_case_desc(struct clip_offset_case *t, char *desc)
+{
+ strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(clip_offset, clip_offset_cases, clip_offset_case_desc);
+
+static void drm_test_fb_clip_offset(struct kunit *test)
+{
+ const struct clip_offset_case *params = test->param_value;
+ const struct drm_format_info *format_info =
drm_format_info(params->format);
+
+ unsigned int offset = -1;
+
+ unsigned int pitch = params->pitch;
+
+ if (pitch == TEST_USE_DEFAULT_PITCH)
+ pitch = drm_format_info_min_pitch(format_info, 0,
+
drm_rect_width(¶ms->clip));
+
+ /* Assure that the pitch is not zero, because this will inevitable
cause the
+* wrong expected result
+*/
+ KUNIT_ASSERT_NE(test, pitch, 0);
+
+ offset = drm_fb_clip_offset(pitch, format_info, ¶ms->clip);
+
+ KUNIT_EXPECT_EQ(test, offset, params->expected_offset);
+}
+
static struct kunit_case drm_format_helper_test_cases[] = {
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_gray8,
convert_xrgb_gen_params),
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_rgb332,
convert_xrgb_gen_params),
@@ -970,6 +1060,7 @@ static struct kunit_case drm_format_helper_test_cases[] = {
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_argb2101010,
convert_xrgb_gen_params),
KUNIT_CASE_PARAM(drm_test_fb_xrgb_to_mono,
convert_xrgb_gen_params),
KUNIT_CASE_PARAM(drm_test_fb_swab, convert_xrgb_gen_params),
+ KUNIT_CASE_PARAM(drm_test_fb_clip_offset, clip_offset_gen_params),
{}
};
Re: [PATCH 2/6] drm/format-helper: Add KUnit tests for drm_fb_swab()
On 7/21/23 15:23, Arthur Grillo wrote:
Insert parameterized test for the drm_fb_swab() to ensure correctness
and prevent future regressions.
Signed-off-by: Arthur Grillo
With the nit I pointed in patch #1 addressed,
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
.../gpu/drm/tests/drm_format_helper_test.c| 66 +++
1 file changed, 66 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c
b/drivers/gpu/drm/tests/drm_format_helper_test.c
index bc6894f0a202..abeda642d84a 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -74,6 +74,11 @@ struct convert_to_mono_result {
const u8 expected[TEST_BUF_SIZE];
};
+struct fb_swab_result {
+ unsigned int dst_pitch;
+ const u32 expected[TEST_BUF_SIZE];
+};
+
struct convert_xrgb_case {
const char *name;
unsigned int pitch;
@@ -90,6 +95,7 @@ struct convert_xrgb_case {
struct convert_to_xrgb2101010_result xrgb2101010_result;
struct convert_to_argb2101010_result argb2101010_result;
struct convert_to_mono_result mono_result;
+ struct fb_swab_result swab_result;
};
static struct convert_xrgb_case convert_xrgb_cases[] = {
@@ -143,6 +149,10 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0b0 },
},
+ .swab_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFF01 },
+ },
},
{
.name = "single_pixel_clip_rectangle",
@@ -197,6 +207,10 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
.dst_pitch = TEST_USE_DEFAULT_PITCH,
.expected = { 0b0 },
},
+ .swab_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = { 0xFF10 },
+ },
},
{
/* Well known colors: White, black, red, green, blue, magenta,
@@ -318,6 +332,15 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0b11,
},
},
+ .swab_result = {
+ .dst_pitch = TEST_USE_DEFAULT_PITCH,
+ .expected = {
+ 0xFF11, 0x0022,
+ 0xFF33, 0x00FF0044,
+ 0xFF55, 0xFF00FF66,
+ 0x0077, 0x0088,
+ },
+ },
},
{
/* Randomly picked colors. Full buffer within the clip area. */
@@ -425,6 +448,14 @@ static struct convert_xrgb_case
convert_xrgb_cases[] = {
0b010, 0b000,
},
},
+ .swab_result = {
+ .dst_pitch = 20,
+ .expected = {
+ 0x9C440EA1, 0x054D11B1, 0x03F3A8C1, 0x,
0x,
+ 0x73F06CD1, 0x9C440EA2, 0x054D11B2, 0x,
0x,
+ 0x0303A8C2, 0x73F06CD2, 0x9C440EA3, 0x,
0x,
+ },
+ },
},
};
@@ -892,6 +923,40 @@ static void drm_test_fb_xrgb_to_mono(struct kunit *test)
KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}
+static void drm_test_fb_swab(struct kunit *test)
+{
+ const struct convert_xrgb_case *params = test->param_value;
+ const struct fb_swab_result *result = ¶ms->swab_result;
+ size_t dst_size;
+ u32 *buf = NULL;
+ __le32 *xrgb = NULL;
+ struct iosys_map dst, src;
+
+ struct drm_framebuffer fb = {
+ .format = drm_format_info(DRM_FORMAT_XRGB),
+ .pitches = { params->pitch, 0, 0 },
+ };
+
+ dst_size = conversion_buf_size(DRM_FORMAT_XRGB, result->dst_pitch,
¶ms->clip);
+
+ KUNIT_ASSERT_GT(test, dst_size, 0);
+
+ buf = kunit_kzalloc(test, dst_size, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buf);
+ iosys_map_set_vaddr(&dst, buf);
+
+ xrgb = cpubuf_to_le32(test, params->xrgb, TEST_BUF_SIZE);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb);
+ iosys_map_set_vaddr(&src, xrgb);
+
+ if (result->dst_pitch == TEST_USE_DEFAULT_PITCH)
+ drm_fb_swab(&dst, NULL, &src, &fb, ¶ms->clip, false);
+ else
+ drm_fb_swab(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip,
false);
+ buf = le32buf_to_cpu(test, (__force const __le32 *)buf, dst_size /
sizeof(u32));
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected,
Re: [PATCH 1/6] drm/format-helper: Test default pitch fallback
Hi Arthur, Just nitpicking, but... On 7/21/23 15:23, Arthur Grillo wrote: Test the default pitch fallback when NULL is passed as the dst_pitch on the conversion procedures. Signed-off-by: Arthur Grillo --- .../gpu/drm/tests/drm_format_helper_test.c| 132 -- 1 file changed, 87 insertions(+), 45 deletions(-) [...] @@ -530,7 +532,10 @@ static void drm_test_fb_xrgb_to_gray8(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb); iosys_map_set_vaddr(&src, xrgb); - drm_fb_xrgb_to_gray8(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); + if (result->dst_pitch == TEST_USE_DEFAULT_PITCH) + drm_fb_xrgb_to_gray8(&dst, NULL, &src, &fb, ¶ms->clip); + else + drm_fb_xrgb_to_gray8(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); Couldn't we do something like: unsigned int *dst_pitch = (result->dst_pitch == TEST_USE_DEFAULT_PITCH) ? NULL : &result->dst_pitch; [...] drm_fb_xrgb_to_gray8(&dst, dst_pitch, &src, &fb, ¶ms->clip); I believe the code would be cleaner. Best Regards, - Maíra KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } @@ -560,7 +565,10 @@ static void drm_test_fb_xrgb_to_rgb332(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb); iosys_map_set_vaddr(&src, xrgb); - drm_fb_xrgb_to_rgb332(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); + if (result->dst_pitch == TEST_USE_DEFAULT_PITCH) + drm_fb_xrgb_to_rgb332(&dst, NULL, &src, &fb, ¶ms->clip); + else + drm_fb_xrgb_to_rgb332(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } @@ -590,12 +598,19 @@ static void drm_test_fb_xrgb_to_rgb565(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb); iosys_map_set_vaddr(&src, xrgb); - drm_fb_xrgb_to_rgb565(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip, false); + if (result->dst_pitch == TEST_USE_DEFAULT_PITCH) + drm_fb_xrgb_to_rgb565(&dst, NULL, &src, &fb, ¶ms->clip, false); + else + drm_fb_xrgb_to_rgb565(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip, + false); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); buf = dst.vaddr; /* restore original value of buf */ - drm_fb_xrgb_to_rgb565(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip, true); + if (result->dst_pitch == TEST_USE_DEFAULT_PITCH) + drm_fb_xrgb_to_rgb565(&dst, NULL, &src, &fb, ¶ms->clip, true); + else + drm_fb_xrgb_to_rgb565(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip, true); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected_swab, dst_size); } @@ -626,7 +641,10 @@ static void drm_test_fb_xrgb_to_xrgb1555(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb); iosys_map_set_vaddr(&src, xrgb); - drm_fb_xrgb_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); + if (result->dst_pitch == TEST_USE_DEFAULT_PITCH) + drm_fb_xrgb_to_xrgb1555(&dst, NULL, &src, &fb, ¶ms->clip); + else + drm_fb_xrgb_to_xrgb1555(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } @@ -657,7 +675,10 @@ static void drm_test_fb_xrgb_to_argb1555(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb); iosys_map_set_vaddr(&src, xrgb); - drm_fb_xrgb_to_argb1555(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); + if (result->dst_pitch == TEST_USE_DEFAULT_PITCH) + drm_fb_xrgb_to_argb1555(&dst, NULL, &src, &fb, ¶ms->clip); + else + drm_fb_xrgb_to_argb1555(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); buf = le16buf_to_cpu(test, (__force const __le16 *)buf, dst_size / sizeof(__le16)); KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size); } @@ -688,7 +709,10 @@ static void drm_test_fb_xrgb_to_rgba5551(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, xrgb); iosys_map_set_vaddr(&src, xrgb); - drm_fb_xrgb_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); + if (result->dst_pitch == TEST_USE_DEFAULT_PITCH) + drm_fb_xrgb_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); + else + drm_fb_xrgb_to_rgba5551(&dst, &result->dst_pitch, &src, &fb, ¶ms->clip); buf = le16buf_to_cpu(test, (__f
Re: [PATCH] drm/vkms: avoid race-condition between flushing and destroying
On 8/3/23 17:52, Sebastian Wick wrote:
On Sun, Jul 30, 2023 at 12:51 AM Maíra Canal wrote:
After we flush the workqueue at the commit tale, we need to make sure
that no work is queued until we destroy the state. Currently, new work
can be queued in the workqueue, even after the commit tale, as the
vblank thread is still running.
Therefore, to avoid a race-condition that will lead to the trigger of a
WARN_ON() at the function vkms_atomic_crtc_destroy_state(), add a mutex
to protect the sections where the queue is manipulated.
This way we can make sure that no work will be added to the workqueue
between flushing the queue (at the commit tail) and destroying the
state.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/vkms/vkms_crtc.c | 10 +-
drivers/gpu/drm/vkms/vkms_drv.c | 1 +
drivers/gpu/drm/vkms/vkms_drv.h | 8
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 3c5ebf106b66..e5ec21a0da05 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -49,7 +49,9 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
hrtimer *timer)
state->crc_pending = true;
spin_unlock(&output->composer_lock);
+ mutex_lock(&state->queue_lock);
ret = queue_work(output->composer_workq,
&state->composer_work);
+ mutex_unlock(&state->queue_lock);
if (!ret)
DRM_DEBUG_DRIVER("Composer worker already queued\n");
}
@@ -129,6 +131,7 @@ vkms_atomic_crtc_duplicate_state(struct drm_crtc *crtc)
__drm_atomic_helper_crtc_duplicate_state(crtc, &vkms_state->base);
+ mutex_init(&vkms_state->queue_lock);
INIT_WORK(&vkms_state->composer_work, vkms_composer_worker);
return &vkms_state->base;
@@ -142,6 +145,9 @@ static void vkms_atomic_crtc_destroy_state(struct drm_crtc
*crtc,
__drm_atomic_helper_crtc_destroy_state(state);
WARN_ON(work_pending(&vkms_state->composer_work));
+ mutex_unlock(&vkms_state->queue_lock);
+
+ mutex_destroy(&vkms_state->queue_lock);
kfree(vkms_state->active_planes);
kfree(vkms_state);
}
@@ -155,8 +161,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
vkms_atomic_crtc_destroy_state(crtc, crtc->state);
__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
- if (vkms_state)
+ if (vkms_state) {
+ mutex_init(&vkms_state->queue_lock);
INIT_WORK(&vkms_state->composer_work, vkms_composer_worker);
+ }
}
static const struct drm_crtc_funcs vkms_crtc_funcs = {
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index dd0af086e7fa..9212686ca88a 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -84,6 +84,7 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state
*old_state)
struct vkms_crtc_state *vkms_state =
to_vkms_crtc_state(old_crtc_state);
+ mutex_lock(&vkms_state->queue_lock);
I haven't looked at the code in detail but doesn't this need to be
unlocked after flush_work again?
The idea is to hold the lock until we have destroyed the CRTC state.
This way we can make sure that no job was queued between flushing
and destroying the state. So, this lock is unlocked at
vkms_atomic_crtc_destroy_state().
Best Regards,
- Maíra
flush_work(&vkms_state->composer_work);
}
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index c7ae6c2ba1df..83767692469a 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -89,6 +89,14 @@ struct vkms_crtc_state {
struct vkms_writeback_job *active_writeback;
struct vkms_color_lut gamma_lut;
+ /* protects the access to the workqueue
+*
+* we need to hold this lock between flushing the workqueue and
+* destroying the state to avoid work to be queued by the worker
+* thread
+*/
+ struct mutex queue_lock;
+
/* below four are protected by vkms_output.composer_lock */
bool crc_pending;
bool wb_pending;
--
2.41.0
Re: [PATCH v2] drm/vkms: Fix race-condition between the hrtimer and the atomic commit
On 5/23/23 09:32, Maíra Canal wrote:
Currently, it is possible for the composer to be set as enabled and then
as disabled without a proper call for the vkms_vblank_simulate(). This
is problematic, because the driver would skip one CRC output, causing CRC
tests to fail. Therefore, we need to make sure that, for each time the
composer is set as enabled, a composer job is added to the queue.
In order to provide this guarantee, add a mutex that will lock before
the composer is set as enabled and will unlock only after the composer
job is added to the queue. This way, we can have a guarantee that the
driver won't skip a CRC entry.
This race-condition is affecting the IGT test "writeback-check-output",
making the test fail and also, leaking writeback framebuffers, as the
writeback job is queued, but it is not signaled. This patch avoids both
problems.
[v2]:
* Create a new mutex and keep the spinlock across the atomic commit in
order to avoid interrupts that could result in deadlocks.
Signed-off-by: Maíra Canal
Applied to drm-misc/drm-misc-next.
Best Regards,
- Maíra
---
drivers/gpu/drm/vkms/vkms_composer.c | 9 +++--
drivers/gpu/drm/vkms/vkms_crtc.c | 9 +
drivers/gpu/drm/vkms/vkms_drv.h | 4 +++-
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 906d3df40cdb..b12188fd6b38 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -320,10 +320,15 @@ void vkms_set_composer(struct vkms_output *out, bool
enabled)
if (enabled)
drm_crtc_vblank_get(&out->crtc);
- spin_lock_irq(&out->lock);
+ mutex_lock(&out->enabled_lock);
old_enabled = out->composer_enabled;
out->composer_enabled = enabled;
- spin_unlock_irq(&out->lock);
+
+ /* the composition wasn't enabled, so unlock the lock to make sure the
lock
+* will be balanced even if we have a failed commit
+*/
+ if (!out->composer_enabled)
+ mutex_unlock(&out->enabled_lock);
if (old_enabled)
drm_crtc_vblank_put(&out->crtc);
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 515f6772b866..3079013c8b32 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -16,7 +16,7 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
hrtimer *timer)
struct drm_crtc *crtc = &output->crtc;
struct vkms_crtc_state *state;
u64 ret_overrun;
- bool ret, fence_cookie;
+ bool ret, fence_cookie, composer_enabled;
fence_cookie = dma_fence_begin_signalling();
@@ -25,15 +25,15 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
if (ret_overrun != 1)
pr_warn("%s: vblank timer overrun\n", __func__);
- spin_lock(&output->lock);
ret = drm_crtc_handle_vblank(crtc);
if (!ret)
DRM_ERROR("vkms failure on handling vblank");
state = output->composer_state;
- spin_unlock(&output->lock);
+ composer_enabled = output->composer_enabled;
+ mutex_unlock(&output->enabled_lock);
- if (state && output->composer_enabled) {
+ if (state && composer_enabled) {
u64 frame = drm_crtc_accurate_vblank_count(crtc);
/* update frame_start only if a queued vkms_composer_worker()
@@ -292,6 +292,7 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc
*crtc,
spin_lock_init(&vkms_out->lock);
spin_lock_init(&vkms_out->composer_lock);
+ mutex_init(&vkms_out->enabled_lock);
vkms_out->composer_workq = alloc_ordered_workqueue("vkms_composer", 0);
if (!vkms_out->composer_workq)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 5f1a0a44a78c..dcf4e302fb4d 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -100,8 +100,10 @@ struct vkms_output {
struct workqueue_struct *composer_workq;
/* protects concurrent access to composer */
spinlock_t lock;
+ /* guarantees that if the composer is enabled, a job will be queued */
+ struct mutex enabled_lock;
- /* protected by @lock */
+ /* protected by @enabled_lock */
bool composer_enabled;
struct vkms_crtc_state *composer_state;
Re: [PATCH v10] drm: Add initial ci/ subdirectory
Hi Helen,
Great to see this coming to the DRM!
Just wondering, any chance we could add a stage to perform tests on
VKMS? The main way of validating VKMS is through IGT tests, so I feel
it would be a perfect match to have VKMS as a stage on the CI. As a
generic KMS driver, VKMS is also great to validate changes on DRM core.
Another question, could we add V3D to the default arm and arm64 config?
Best Regards,
- Maíra
On 7/20/23 12:27, Helen Koike wrote:
From: Tomeu Vizoso
Developers can easily execute several tests on different devices
by just pushing their branch to their fork in a repository hosted
on gitlab.freedesktop.org which has an infrastructure to run jobs
in several runners and farms with different devices.
There are also other automated tools that uprev dependencies,
monitor the infra, and so on that are already used by the Mesa
project, and we can reuse them too.
Also, store expectations about what the DRM drivers are supposed
to pass in the IGT test suite. By storing the test expectations
along with the code, we can make sure both stay in sync with each
other so we can know when a code change breaks those expectations.
Also, include a configuration file that points to the out-of-tree
CI scripts.
This will allow all contributors to drm to reuse the infrastructure
already in gitlab.freedesktop.org to test the driver on several
generations of the hardware.
Signed-off-by: Tomeu Vizoso
Signed-off-by: Helen Koike
---
Hello,
I'm re-spining this patch sent originally by Tomeu.
This is meant to be an auxiliary tool where developers and
maintainers can just submit their code to fdo and see if
tests passes, than they can decide if it is worthy merging
it or not.
This tool has proven its value on the Mesa community
and it can bring a lot of value here too.
Please review and let me know your thoughts.
You can also see this patch on
https://gitlab.freedesktop.org/helen.fornazier/linux/-/tree/drm-ci-tests
Thanks!
v2:
- Fix names of result expectation files to match SoC
- Don't execute tests that are going to skip on all boards
v3:
- Remove tracking of dmesg output during test execution
v4:
- Move up to drivers/gpu/drm
- Add support for a bunch of other drivers
- Explain how to incorporate fixes for CI from a
${TARGET_BRANCH}-external-fixes branch
- Remove tests that pass from expected results file, to reduce the
size of in-tree files
- Add docs about how to deal with outages in automated testing labs
- Specify the exact SHA of the CI scripts to be used
v5:
- Remove unneeded skips from Meson expectations file
- Use a more advanced runner that detects flakes automatically
- Use a more succint format for the expectations
- Run many more tests (and use sharding to finish in time)
- Use skip lists to avoid hanging machines
- Add some build testing
- Build IGT in each pipeline for faster uprevs
- List failures in the GitLab UI
v6:
- Rebase on top of latest drm-next
- Lower priority of LAVA jobs to not impact Mesa CI as much
- Update docs
v7:
- Rebase on top of latest drm-next
v8:
- Move all files specific to testing the kernel into the kernel tree
(thus I have dropped the r-bs I had collected so far)
- Uprev Gitlab CI infrastructure scripts to the latest from Mesa
- Add MAINTAINERS entry
- Fix boot on MT8173 by adding some Kconfigs that are now needed
- Link to the docs from index.rst and hard-wrap the file
v9:
- Only automatically run the pipelines for merge requests
- Switch to zstd for the build artifacts to align with Mesa
- Add Qcom USB PHYs to config as they are now =m in the defconfig
v10:
- Include ci yml files from mesa/mesa (where the development is
current active) instead of a spin off project.
- Uprev Gitlab CI infrastructure scripts to the latest from Mesa
- Update MAINTAINERS entry
- Uprev igt tool
- add LAVA_JOB_PRIORITY: 30
- pipeline example:
https://gitlab.freedesktop.org/helen.fornazier/linux/-/pipelines/940506
---
Documentation/gpu/automated_testing.rst | 144 +
Documentation/gpu/index.rst |1 +
MAINTAINERS |8 +
drivers/gpu/drm/ci/arm.config | 69 +
drivers/gpu/drm/ci/arm64.config | 199 ++
drivers/gpu/drm/ci/build-igt.sh | 35 +
drivers/gpu/drm/ci/build.sh | 157 +
drivers/gpu/drm/ci/build.yml | 110 +
drivers/gpu/drm/ci/check-patch.py | 57 +
drivers/gpu/drm/ci/container.yml | 61 +
drivers/gpu/drm/ci/gitlab-ci.yml | 252 ++
drivers/gpu/drm/ci/igt_runner.sh | 77 +
drivers/gpu/drm/ci/image-tags.yml | 15 +
drivers/gpu/drm/ci/lava-submit.sh | 57 +
drivers/gpu/drm/ci/static-checks.yml | 12 +
drivers/gpu/drm/ci/test.yml | 335 ++
drivers/gpu/drm/ci/testli
Re: [PATCH 2/2] drm/v3d: Expose the total GPU usage stats on debugfs
Hi,
On 7/28/23 07:16, Tvrtko Ursulin wrote:
Hi,
On 27/07/2023 15:23, Maíra Canal wrote:
The previous patch exposed the accumulated amount of active time per
client for each V3D queue. But this doesn't provide a global notion of
the GPU usage.
Therefore, provide the accumulated amount of active time for each V3D
queue (BIN, RENDER, CSD, TFU and CACHE_CLEAN), considering all the jobs
submitted to the queue, independent of the client.
This data is exposed through the debugfs interface, so that if the
interface is queried at two different points of time the usage percentage
of each of the queues can be calculated.
Just passing observation - I've noticed a mismatch between fdinfo and
debugfs in terms of ABI stability and production availability.
Not sure if it matters for your intended use cases, just saying that if
you plan to have an user facing tool similar to what we have in
intel_gpu_top, debugfs may not be the best choice.
Do you have a suggestion of a better interface that could be used to
expose this data?
It would be nice to have something generic, similar to fdinfo, to expose
global GPU stats. This way we could expose global GPU stats on gputop,
which would be great.
Best Regards,
- Maíra
Regards,
Tvrtko
Co-developed-by: Jose Maria Casanova Crespo
Signed-off-by: Jose Maria Casanova Crespo
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_debugfs.c | 27 +++
drivers/gpu/drm/v3d/v3d_drv.h | 3 +++
drivers/gpu/drm/v3d/v3d_gem.c | 5 -
drivers/gpu/drm/v3d/v3d_irq.c | 24
drivers/gpu/drm/v3d/v3d_sched.c | 13 -
5 files changed, 66 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 330669f51fa7..3b7329343649 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#include
#include
@@ -236,11 +237,37 @@ static int v3d_measure_clock(struct seq_file *m,
void *unused)
return 0;
}
+static int v3d_debugfs_gpu_usage(struct seq_file *m, void *unused)
+{
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
+ struct v3d_dev *v3d = to_v3d_dev(dev);
+ enum v3d_queue queue;
+ u64 timestamp = local_clock();
+ u64 active_runtime;
+
+ seq_printf(m, "timestamp: %llu\n", timestamp);
+
+ for (queue = 0; queue < V3D_MAX_QUEUES; queue++) {
+ if (v3d->queue[queue].start_ns)
+ active_runtime = timestamp - v3d->queue[queue].start_ns;
+ else
+ active_runtime = 0;
+
+ seq_printf(m, "%s: %llu ns\n",
+ v3d_queue_to_string(queue),
+ v3d->queue[queue].enabled_ns + active_runtime);
+ }
+
+ return 0;
+}
+
static const struct drm_debugfs_info v3d_debugfs_list[] = {
{"v3d_ident", v3d_v3d_debugfs_ident, 0},
{"v3d_regs", v3d_v3d_debugfs_regs, 0},
{"measure_clock", v3d_measure_clock, 0},
{"bo_stats", v3d_debugfs_bo_stats, 0},
+ {"gpu_usage", v3d_debugfs_gpu_usage, 0},
};
void
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h
b/drivers/gpu/drm/v3d/v3d_drv.h
index ee5e12d0db1c..b41b32ecd991 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -38,6 +38,9 @@ struct v3d_queue_state {
u64 fence_context;
u64 emit_seqno;
+
+ u64 start_ns;
+ u64 enabled_ns;
};
/* Performance monitor object. The perform lifetime is controlled by
userspace
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c
b/drivers/gpu/drm/v3d/v3d_gem.c
index 40ed0c7c3fad..630ea2db8f8f 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -1014,8 +1014,11 @@ v3d_gem_init(struct drm_device *dev)
u32 pt_size = 4096 * 1024;
int ret, i;
- for (i = 0; i < V3D_MAX_QUEUES; i++)
+ for (i = 0; i < V3D_MAX_QUEUES; i++) {
v3d->queue[i].fence_context = dma_fence_context_alloc(1);
+ v3d->queue[i].start_ns = 0;
+ v3d->queue[i].enabled_ns = 0;
+ }
spin_lock_init(&v3d->mm_lock);
spin_lock_init(&v3d->job_lock);
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c
b/drivers/gpu/drm/v3d/v3d_irq.c
index c898800ae9c2..be4ff7559309 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -102,9 +102,13 @@ v3d_irq(int irq, void *arg)
struct v3d_fence *fence =
to_v3d_fence(v3d->bin_job->base.irq_fence);
struct v3d_file_priv *file =
v3d->bin_job->base.file->driver_priv;
+ u64 runtime = local_clock() - file->start_ns[V3D_BIN];
- file->enabled_ns[V3D_BIN] += local_clock() -
file->start_ns[V3D_BIN];
file->start_ns[V3D_BIN] = 0;
+ v3d->queue[V3D_BIN].start_ns = 0;
+
+ file->enabled_ns[V3D_BIN] += runtime;
+ v3d->queue[V3D_BIN].enabled_ns += runtime;
trace_v3d_bcl_irq(&v3d->drm, fence->seqno);
dma_fence_
Re: [PATCH v5] drm/vkms: Add support to 1D gamma LUT
Hi Arthur,
On 7/8/23 22:38, Arthur Grillo wrote:
Support a 1D gamma LUT with interpolation for each color channel on the
VKMS driver. Add a check for the LUT length by creating
vkms_atomic_check().
Enable VKMS to run the test igt@kms_plane@pixel-format.
Tested with:
igt@kms_color@gamma
igt@kms_color@legacy-gamma
igt@kms_color@invalid-gamma-lut-sizes
v2:
- Add interpolation between the values of the LUT (Simon Ser)
v3:
- s/ratio/delta (Pekka)
- s/color_channel/channel_value (Pekka)
- s/lut_area/lut_channel
- Store the `drm_color_lut`, `lut_length`, and
`channel_value2index_ratio` inside a struct called `vkms_lut`
(Pekka)
- Pre-compute some constants values used through the LUT procedure
(Pekka)
- Change the switch statement to a cast to __u16* (Pekka)
- Make the apply_lut_to_channel_value return the computation result
(Pekka)
v4:
- Add a comment explaining that `enum lut_area` depends on the
layout of `struct drm_color_lut` (Pekka)
- Remove unused variable (kernel test robot)
v5:
- Mention that this will make it possible to run the test
igt@kms_plane@pixel-format (Maíra)
- s/had/has (Maíra)
Signed-off-by: Arthur Grillo
Acked-by: Pekka Paalanen
Reviewed-by: Maíra Canal
Applied to drm-misc/drm-misc-next. Thanks for your contribution to VKMS!
Best Regards,
- Maíra
---
Maíra asked me to run a benchmark on some IGT tests:
Each test ran 100 times. The result with 'X' are tests that were not able to
run because of the absence of gamma LUT.
+--+---+-+---+
| Test | No LUT | Unoptimized
LUT | Optimized LUT |
+
-+---+-+---+
| igt@kms_rotation@primary-rotation-180| 174.298s |181.130s
| 178.800s|
+
-+---+-+---+
| igt@kms_plane@pixel-format |X |1420.453s
| 1218.229s |
+
-+---+-+---+
| igt@kms_plane@pixel-format-source-clamping |X |704.236s
| 612.318s|
+
-+---+-+---+
| igt@kms_plane@plane-position-covered | 12.535s |12.864s
| 12.025s |
+
-+---+-+---+
| igt@kms_plane@plane-position-hole| 11.752s |12.873s
| 11.202s |
+
-+---+-+---+
| igt@kms_plane@plane-position-hole-dpms | 15.188s |15.238s
| 15.652s |
+
-+---+-+---+
| igt@kms_plane@plane-panning-top-left | 10.797s |11.873s
| 11.041s |
+
-+---+-+---+
| igt@kms_plane@plane-bottom-right | 10.764s |11.613s
| 10.053s |
+
-+---+-+---+
| igt@kms_plane@plane-panning-bottom-right-suspend | 2011.344s |2009.410s
| 2011.496s |
+
-+---+-+---+
| igt@kms_cursor_crc@cursor-onscreen-512x5112 | 359.209s |337.830s
| 308.168s|
+
-+---+-+---+
| igt@kms_color@gamma |X |137.702s
| 118.139s|
+
-+---+-+---+
---
drivers/gpu/drm/vkms/vkms_composer.c | 86
drivers/gpu/drm/vkms/vkms_crtc.c | 3 +
drivers/gpu/drm/vkms/vkms_drv.c | 20 ++-
drivers/gpu/drm/vkms/vkms_drv.h | 9 +++
4 files changed, 117 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 906d3df40cdb..e3a79dcd2e38 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -89,6 +90,73 @@ static void fill_background(const struct pixel_argb_u16
*background_color,
output_buffer->pixels[i] = *background_color;
}
+// lerp(a, b, t) = a + (b - a) * t
+static u16 lerp_u16(u16 a, u16 b, s64 t)
+{
+ s64 a_fp = drm_int2fixp(a);
+ s64 b_fp = drm_int2fixp(b);
+
+ s6
Re: [PATCH] drm/tests: Alloc drm_device on drm_exec tests
Hi Arthur,
On 7/27/23 16:22, Arthur Grillo wrote:
The drm_exec tests where crashing[0] because of a null dereference. This
is caused by a new access of the `driver` attribute of `struct
drm_driver` on drm_gem_private_object_init(). Alloc the drm_device to
fix that.
[0]
[15:05:24] == drm_exec (6 subtests) ===
[15:05:24] [PASSED] sanitycheck
^CERROR:root:Build interruption occurred. Cleaning console.
[15:05:50] [ERROR] Test: drm_exec: missing expected subtest!
[15:05:50] BUG: kernel NULL pointer dereference, address: 00b0
[15:05:50] #PF: supervisor read access in kernel mode
[15:05:50] #PF: error_code(0x) - not-present page
[15:05:50] PGD 0 P4D 0
[15:05:50] Oops: [#1] PREEMPT NOPTI
[15:05:50] CPU: 0 PID: 23 Comm: kunit_try_catch Tainted: G N
6.4.0-rc7-02032-ge6303f323b1a #69
[15:05:50] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.16.2-1.fc37 04/01/2014
[15:05:50] RIP: 0010:drm_gem_private_object_init+0x60/0xc0
Fixes: e6303f323b1a ("drm: manager to keep track of GPUs VA mappings")
Signed-off-by: Arthur Grillo
---
drivers/gpu/drm/tests/drm_exec_test.c | 36 +--
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_exec_test.c
b/drivers/gpu/drm/tests/drm_exec_test.c
index 727ac267682e..df31f89a7945 100644
--- a/drivers/gpu/drm/tests/drm_exec_test.c
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -12,11 +12,31 @@
#include
#include
+#include
#include
+#include
#include "../lib/drm_random.h"
-static struct drm_device dev;
+static struct device *dev;
+static struct drm_device *drm;
Maybe we could use test->priv to store those variables.
+
+static int test_init(struct kunit *test)
Nitpick: wouldn't be better to call it drm_exec_test_init()?
Anyway, with those fixes or not,
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
+{
+ dev = drm_kunit_helper_alloc_device(test);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+
+ drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0,
+ DRIVER_MODESET);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
+
+ return 0;
+}
+
+static void test_exit(struct kunit *test)
+{
+ drm_kunit_helper_free_device(test, dev);
+}
static void sanitycheck(struct kunit *test)
{
@@ -33,7 +53,7 @@ static void test_lock(struct kunit *test)
struct drm_exec exec;
int ret;
- drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+ drm_gem_private_object_init(drm, &gobj, PAGE_SIZE);
drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
drm_exec_until_all_locked(&exec) {
@@ -52,7 +72,7 @@ static void test_lock_unlock(struct kunit *test)
struct drm_exec exec;
int ret;
- drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+ drm_gem_private_object_init(drm, &gobj, PAGE_SIZE);
drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
drm_exec_until_all_locked(&exec) {
@@ -78,7 +98,7 @@ static void test_duplicates(struct kunit *test)
struct drm_exec exec;
int ret;
- drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+ drm_gem_private_object_init(drm, &gobj, PAGE_SIZE);
drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES);
drm_exec_until_all_locked(&exec) {
@@ -106,7 +126,7 @@ static void test_prepare(struct kunit *test)
struct drm_exec exec;
int ret;
- drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+ drm_gem_private_object_init(drm, &gobj, PAGE_SIZE);
drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
drm_exec_until_all_locked(&exec) {
@@ -127,8 +147,8 @@ static void test_prepare_array(struct kunit *test)
struct drm_exec exec;
int ret;
- drm_gem_private_object_init(&dev, &gobj1, PAGE_SIZE);
- drm_gem_private_object_init(&dev, &gobj2, PAGE_SIZE);
+ drm_gem_private_object_init(drm, &gobj1, PAGE_SIZE);
+ drm_gem_private_object_init(drm, &gobj2, PAGE_SIZE);
drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
drm_exec_until_all_locked(&exec)
@@ -150,6 +170,8 @@ static struct kunit_case drm_exec_tests[] = {
static struct kunit_suite drm_exec_test_suite = {
.name = "drm_exec",
+ .init = test_init,
+ .exit = test_exit,
.test_cases = drm_exec_tests,
};
Re: [PATCH 0/2] Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()
On 7/18/23 18:44, Nathan Chancellor wrote: Hi all, A proposed update to clang's -Wconstant-logical-operand [1] to warn when the left hand side is a constant as well now triggers with the modulo expression in nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ, such as CONFIG_HZ=300: drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] 189 | if (NSEC_PER_SEC % HZ && | ~ ^ drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: use '&' for a bitwise operation 189 | if (NSEC_PER_SEC % HZ && | ^~ | & drivers/gpu/drm/i915/gem/i915_gem_wait.c:189:24: note: remove constant to silence this warning 1 warning generated. In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12: drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand] 343 | if (NSEC_PER_SEC % HZ && | ~ ^ drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation 343 | if (NSEC_PER_SEC % HZ && | ^~ | & drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this warning 1 warning generated. These patches add an explicit comparison to zero to make the expression a boolean, which clears up the warning. The patches have no real dependency on each other but I felt like they made send together since it is the same code. If these could go into mainline sooner rather than later to avoid breaking builds that can hit this with CONFIG_WERROR, that would be nice, but I won't insist since I don't think our own CI has builds that has those conditions, but others might. --- Nathan Chancellor (2): drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout() drm/i915: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout() Applied both patches to drm-misc/drm-misc-next! Best Regards, - Maíra drivers/gpu/drm/i915/gem/i915_gem_wait.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.h| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) --- base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c change-id: 20230718-nsecs_to_jiffies_timeout-constant-logical-operand-4a944690f3e9 Best regards,
Re: [PATCH 1/2] drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()
Hi Nathan,
On 7/18/23 18:44, Nathan Chancellor wrote:
A proposed update to clang's -Wconstant-logical-operand to warn when the
left hand side is a constant shows the following instance in
nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ,
such as CONFIG_HZ=300:
In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12:
drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with
constant operand [-Wconstant-logical-operand]
343 | if (NSEC_PER_SEC % HZ &&
| ~ ^
drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation
343 | if (NSEC_PER_SEC % HZ &&
| ^~
| &
drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this
warning
1 warning generated.
Turn this into an explicit comparison against zero to make the
expression a boolean to make it clear this should be a logical check,
not a bitwise one.
Link: https://reviews.llvm.org/D142609
Signed-off-by: Nathan Chancellor
Reviewed-by: Maíra Canal
Thanks for all the hard work with clang! Let me know if you need someone
to push it to drm-misc-next.
Best Regards,
- Maíra
---
drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index b74b1351bfc8..7f664a4b2a75 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -340,7 +340,7 @@ struct v3d_submit_ext {
static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
{
/* nsecs_to_jiffies64() does not guard against overflow */
- if (NSEC_PER_SEC % HZ &&
+ if ((NSEC_PER_SEC % HZ) != 0 &&
div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
return MAX_JIFFY_OFFSET;
Re: [PATCH v2] drm/vkms: Implement all blend mode properties
Hi Melissa,
On 7/23/23 18:00, Melissa Wen wrote:
On 07/23, Maíra Canal wrote:
Following the DRM assumption, VKMS currently assumes that the alpha is
pre-multiplied. Moreover, it doesn't support the alpha property.
So, first, implement the alpha property to VKMS and then, the blend
mode property. In order to support all possible supported modes,
change the pre_mul_blend_channel() function to check the plane blend
mode and apply the correct blend formula, following the DRM
convention, using the proper plane alpha value.
Tested with igt@kms_plane_alpha_blend.
Signed-off-by: Maíra Canal
---
v1 -> v2:
https://lore.kernel.org/dri-devel/20230428122751.24271-1-mca...@igalia.com/T/
* Rebased on top of drm-misc-next.
---
drivers/gpu/drm/vkms/vkms_composer.c | 49 ++--
drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
drivers/gpu/drm/vkms/vkms_plane.c| 9 +
3 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index d170a8e89b95..68a476461824 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -12,33 +12,47 @@
#include "vkms_drv.h"
-static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha)
+static u16 blend_channel(struct vkms_frame_info *frame_info, u16 src, u16 dst,
u16 alpha)
{
u32 new_color;
- new_color = (src * 0x + dst * (0x - alpha));
+ switch (frame_info->pixel_blend_mode) {
+ case DRM_MODE_BLEND_PIXEL_NONE:
+ new_color = 0x * frame_info->alpha * src
+ + (0xfffe0001 - 0x * frame_info->alpha) * dst;
+ break;
+ case DRM_MODE_BLEND_COVERAGE:
+ new_color = alpha * frame_info->alpha * src
+ + (0xfffe0001 - alpha * frame_info->alpha) * dst;
+ break;
+ case DRM_MODE_BLEND_PREMULTI:
+ default:
+ new_color = 0x * frame_info->alpha * src
+ + (0xfffe0001 - alpha * frame_info->alpha) * dst;
Hi Maíra,
First, thank you for keep working on this feature and sorry for feedback
delay.
I didn't complete understand this 0xfffe0001 calculation. Could you
describe a little more (adding a comment here) each formula that you are
using here?
The idea here is the same as before, but now that we have two factors
multiplying src and dst (alpha and frame_info->alpha), so we need to
divide by 0x * 0x = 0xfffe0001. This helps us not to lose
precision when dividing the alpha and frame_info->alpha values by
0x.
I'm using the formulas specified in the DRM documentation [1]:
"None": out.rgb = plane_alpha * fg.rgb + (1 - plane_alpha) * bg.rgb
"Pre-multiplied": out.rgb = plane_alpha * fg.rgb +
(1 - (plane_alpha * fg.alpha)) * bg.rgb
"Coverage": out.rgb = plane_alpha * fg.alpha * fg.rgb +
(1 - (plane_alpha * fg.alpha)) * bg.rgb
I can add some comments in the code to make it clear that 0xfffe0001 is
0x * 0x.
[1] https://docs.kernel.org/gpu/drm-kms.html#plane-composition-properties
Also, as we are changing pixels of both outputs that are part of the
comparisons, but we just compare the CRC (not pixel by pixel). Did you
verify a sample of the pixels resulted from this calculation to check if
they make sense?
I verified a sample of the resulting pixels while debugging the
implementation with the IGT tests. It looked okay to me, but let me know
if you find something suspecious.
IIRC, we don't have a negative test for this property in
kms_plane_alpha_blend. In this sense, did you verify if the test fail
when setting a property value but using a different equation that not
correspond to the pixel blend mode setup? Overall the change makes
sense, but I tried some invalid scenarios and didn't get the expected
result.
I was able to reproduce this failure as well. Maybe we need some
improviments on the IGT tests to make sure about the correctness of the
pixel blend mode. For this patch, I based myself on the correctness of
the formulas provided by the DRM documentation.
Would you need more IGT tests to approve this feature?
Best Regards,
- Maíra
Melissa
+ break;
+ }
- return DIV_ROUND_CLOSEST(new_color, 0x);
+ return DIV_ROUND_CLOSEST(new_color, 0xfffe0001);
}
/**
- * pre_mul_alpha_blend - alpha blending equation
+ * alpha_blend - alpha blending equation
* @frame_info: Source framebuffer's metadata
* @stage_buffer: The line with the pixels from src_plane
* @output_buffer: A line buffer that receives all the blends output
*
* Using the information from the `frame_info`, this blends only the
* necessary pixels from the `stage_buffer` to the `output_buffer`
- * using premultiplied blend formula.
+ * using the adequate blend formula depending on the plane blend mode
+ * (see blend_channel()).
*
- * The current DRM ass
Re: [PATCH v2 00/11] drm: kunit: Switch to kunit actions
Hi Maxime, On 7/20/23 08:15, Maxime Ripard wrote: Hi, Since v6.5-rc1, kunit gained a devm/drmm-like mechanism that makes tests resources much easier to cleanup. This series converts the existing tests to use those new actions where relevant. > Let me know what you think, With the problems pointed out by kernel test bot fixed, the whole series is: Reviewed-by: Maíra Canal Best Regards, - Maíra Maxime Signed-off-by: Maxime Ripard --- Changes in v2: - Fix some typos - Use plaltform_device_del instead of removing the call to platform_device_put after calling platform_device_add - Link to v1: https://lore.kernel.org/r/20230710-kms-kunit-actions-rework-v1-0-722c58d72...@kernel.org --- Maxime Ripard (11): drm/tests: helpers: Switch to kunit actions drm/tests: client-modeset: Remove call to drm_kunit_helper_free_device() drm/tests: modes: Remove call to drm_kunit_helper_free_device() drm/tests: probe-helper: Remove call to drm_kunit_helper_free_device() drm/tests: helpers: Create a helper to allocate a locking ctx drm/tests: helpers: Create a helper to allocate an atomic state drm/vc4: tests: pv-muxing: Remove call to drm_kunit_helper_free_device() drm/vc4: tests: mock: Use a kunit action to unregister DRM device drm/vc4: tests: pv-muxing: Switch to managed locking init drm/vc4: tests: Switch to atomic state allocation helper drm/vc4: tests: pv-muxing: Document test scenario drivers/gpu/drm/tests/drm_client_modeset_test.c | 8 -- drivers/gpu/drm/tests/drm_kunit_helpers.c | 108 +- drivers/gpu/drm/tests/drm_modes_test.c | 8 -- drivers/gpu/drm/tests/drm_probe_helper_test.c | 8 -- drivers/gpu/drm/vc4/tests/vc4_mock.c| 5 ++ drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 115 +--- include/drm/drm_kunit_helpers.h | 7 ++ 7 files changed, 158 insertions(+), 101 deletions(-) --- base-commit: c58c49dd89324b18a812762a2bfa5a0458e4f252 change-id: 20230710-kms-kunit-actions-rework-5d163762c93b Best regards,
Re: [PATCH 6/7] drm/v3d: switch to using drm_exec
Hi Christian,
I believe that `select DRM_EXEC` is missing on v3d's Kconfig file. If
we don't select it, we will get some compilation errors.
Apart from this problem, I ran some tests on the RPi 4 and didn't see
any problems.
Best Regards,
- Maíra
On 7/12/23 09:47, Christian König wrote:
Just a straightforward conversion without any optimization.
Only compile tested for now.
Signed-off-by: Christian König
Cc: Emma Anholt
Cc: Melissa Wen
---
drivers/gpu/drm/v3d/v3d_gem.c | 44 ---
1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 2e94ce788c71..190e2a9f64a4 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -10,6 +10,7 @@
#include
#include
+#include
#include
#include
#include
@@ -249,20 +250,17 @@ v3d_invalidate_caches(struct v3d_dev *v3d)
* to v3d, so we don't attach dma-buf fences to them.
*/
static int
-v3d_lock_bo_reservations(struct v3d_job *job,
-struct ww_acquire_ctx *acquire_ctx)
+v3d_lock_bo_reservations(struct v3d_job *job, struct drm_exec *exec)
{
int i, ret;
- ret = drm_gem_lock_reservations(job->bo, job->bo_count, acquire_ctx);
+ drm_exec_init(exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_until_all_locked(exec)
+ ret = drm_exec_prepare_array(exec, job->bo, job->bo_count, 1);
if (ret)
- return ret;
+ goto fail;
for (i = 0; i < job->bo_count; i++) {
- ret = dma_resv_reserve_fences(job->bo[i]->resv, 1);
- if (ret)
- goto fail;
-
ret = drm_sched_job_add_implicit_dependencies(&job->base,
job->bo[i], true);
if (ret)
@@ -272,7 +270,7 @@ v3d_lock_bo_reservations(struct v3d_job *job,
return 0;
fail:
- drm_gem_unlock_reservations(job->bo, job->bo_count, acquire_ctx);
+ drm_exec_fini(exec);
return ret;
}
@@ -477,7 +475,7 @@ v3d_push_job(struct v3d_job *job)
static void
v3d_attach_fences_and_unlock_reservation(struct drm_file *file_priv,
struct v3d_job *job,
-struct ww_acquire_ctx *acquire_ctx,
+struct drm_exec *exec,
u32 out_sync,
struct v3d_submit_ext *se,
struct dma_fence *done_fence)
@@ -492,7 +490,7 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file
*file_priv,
DMA_RESV_USAGE_WRITE);
}
- drm_gem_unlock_reservations(job->bo, job->bo_count, acquire_ctx);
+ drm_exec_fini(exec);
/* Update the return sync object for the job */
/* If it only supports a single signal semaphore*/
@@ -669,7 +667,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
struct v3d_render_job *render = NULL;
struct v3d_job *clean_job = NULL;
struct v3d_job *last_job;
- struct ww_acquire_ctx acquire_ctx;
+ struct drm_exec exec;
int ret = 0;
trace_v3d_submit_cl_ioctl(&v3d->drm, args->rcl_start, args->rcl_end);
@@ -731,7 +729,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
if (ret)
goto fail;
- ret = v3d_lock_bo_reservations(last_job, &acquire_ctx);
+ ret = v3d_lock_bo_reservations(last_job, &exec);
if (ret)
goto fail;
@@ -775,7 +773,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
v3d_attach_fences_and_unlock_reservation(file_priv,
last_job,
-&acquire_ctx,
+&exec,
args->out_sync,
&se,
last_job->done_fence);
@@ -791,8 +789,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
fail_unreserve:
mutex_unlock(&v3d->sched_lock);
fail_perfmon:
- drm_gem_unlock_reservations(last_job->bo,
- last_job->bo_count, &acquire_ctx);
+ drm_exec_fini(&exec);
fail:
v3d_job_cleanup((void *)bin);
v3d_job_cleanup((void *)render);
@@ -819,7 +816,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
struct drm_v3d_submit_tfu *args = data;
struct v3d_submit_ext se = {0};
struct v3d_tfu_job *job = NULL;
- struct ww_acquire_ctx acquire_ctx;
+ struct drm_exec exec;
int ret = 0;
trace_v3d_submit_tfu_ioctl(&v3d->drm, args->iia);
@@ -870,7 +867,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
Re: [PATCH] drm/gem-fb-helper: Consistenly use drm_dbg_kms()
On 7/12/23 10:42, Geert Uytterhoeven wrote:
All debug messages in drm_gem_framebuffer_helper.c use drm_dbg_kms(),
except for one, which uses drm_dbg().
Replace the outlier by drm_dbg_kms() to restore consistency.
Fixes: c91acda3a380bcaf ("drm/gem: Check for valid formats")
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Maíra Canal
Thanks for sending this fix!
Best Regards,
- Maíra
---
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index b8a615a138cd675f..3bdb6ba37ff42fb6 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -168,8 +168,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
if (drm_drv_uses_atomic_modeset(dev) &&
!drm_any_plane_has_format(dev, mode_cmd->pixel_format,
mode_cmd->modifier[0])) {
- drm_dbg(dev, "Unsupported pixel format %p4cc / modifier
0x%llx\n",
- &mode_cmd->pixel_format, mode_cmd->modifier[0]);
+ drm_dbg_kms(dev, "Unsupported pixel format %p4cc / modifier
0x%llx\n",
+ &mode_cmd->pixel_format, mode_cmd->modifier[0]);
return -EINVAL;
}
Re: [PATCH 2/6] drm: add drm_exec selftests v4
On 7/11/23 10:31, Christian König wrote:
Exercise at least all driver facing functions of this new component.
v2: add array test as well
v3: some kunit cleanups
v4: more tests and cleanups
Signed-off-by: Christian König
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/tests/Makefile| 3 +-
drivers/gpu/drm/tests/drm_exec_test.c | 159 ++
3 files changed, 162 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tests/drm_exec_test.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index c0b4063a3ee6..22c1ba9ea28c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -80,6 +80,7 @@ config DRM_KUNIT_TEST
select DRM_BUDDY
select DRM_EXPORT_FOR_TESTS if m
select DRM_KUNIT_TEST_HELPERS
+ select DRM_EXEC
default KUNIT_ALL_TESTS
help
This builds unit tests for DRM. This option is not useful for
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index bca726a8f483..ba7baa622675 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_modes_test.o \
drm_plane_helper_test.o \
drm_probe_helper_test.o \
- drm_rect_test.o
+ drm_rect_test.o \
+ drm_exec_test.o
CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN)
diff --git a/drivers/gpu/drm/tests/drm_exec_test.c
b/drivers/gpu/drm/tests/drm_exec_test.c
new file mode 100644
index ..727ac267682e
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ */
+
+#define pr_fmt(fmt) "drm_exec: " fmt
Do we need this pr_fmt(fmt)?
Best Regards,
- Maíra
+
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include "../lib/drm_random.h"
+
+static struct drm_device dev;
+
+static void sanitycheck(struct kunit *test)
+{
+ struct drm_exec exec;
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_fini(&exec);
+ KUNIT_SUCCEED(test);
+}
+
+static void test_lock(struct kunit *test)
+{
+ struct drm_gem_object gobj = { };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_until_all_locked(&exec) {
+ ret = drm_exec_lock_obj(&exec, &gobj);
+ drm_exec_retry_on_contention(&exec);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ if (ret)
+ break;
+ }
+ drm_exec_fini(&exec);
+}
+
+static void test_lock_unlock(struct kunit *test)
+{
+ struct drm_gem_object gobj = { };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_until_all_locked(&exec) {
+ ret = drm_exec_lock_obj(&exec, &gobj);
+ drm_exec_retry_on_contention(&exec);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ if (ret)
+ break;
+
+ drm_exec_unlock_obj(&exec, &gobj);
+ ret = drm_exec_lock_obj(&exec, &gobj);
+ drm_exec_retry_on_contention(&exec);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ if (ret)
+ break;
+ }
+ drm_exec_fini(&exec);
+}
+
+static void test_duplicates(struct kunit *test)
+{
+ struct drm_gem_object gobj = { };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+
+ drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES);
+ drm_exec_until_all_locked(&exec) {
+ ret = drm_exec_lock_obj(&exec, &gobj);
+ drm_exec_retry_on_contention(&exec);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ if (ret)
+ break;
+
+ ret = drm_exec_lock_obj(&exec, &gobj);
+ drm_exec_retry_on_contention(&exec);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ if (ret)
+ break;
+ }
+ drm_exec_unlock_obj(&exec, &gobj);
+ drm_exec_fini(&exec);
+}
+
+
+
+static void test_prepare(struct kunit *test)
+{
+ struct drm_gem_object gobj = { };
+ struct drm_exec exec;
+ int ret;
+
+ drm_gem_private_object_init(&dev, &gobj, PAGE_SIZE);
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT);
+ drm_exec_until_all_locked(&exec) {
+ ret = drm_exec_prepare_obj(&exec, &gobj, 1);
+ drm_exec_retry_on_contention(&exec);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ if (ret)
+ break;
+ }
+ drm_exec_fini(&exec);
+}
+
+static void test_prepare_array(struct k
Re: [PATCH v4] drm/vkms: Add support to 1D gamma LUT
On 6/27/23 05:12, Pekka Paalanen wrote: On Mon, 26 Jun 2023 14:35:25 -0300 Maira Canal wrote: Hi Pekka, On 6/26/23 05:17, Pekka Paalanen wrote: On Sat, 24 Jun 2023 18:48:08 -0300 Maira Canal wrote: Hi Arthur, Thanks for working on this feature for the VKMS! On 6/21/23 16:41, Arthur Grillo wrote: Support a 1D gamma LUT with interpolation for each color channel on the VKMS driver. Add a check for the LUT length by creating vkms_atomic_check(). Tested with: igt@kms_color@gamma igt@kms_color@legacy-gamma igt@kms_color@invalid-gamma-lut-sizes Could you also mention that this will make it possible to run the test igt@kms_plane@pixel-format? Also, you mentioned to me that the performance degraded with this new feature, but I wasn't able to see it while running the VKMS CI. I performed a couple of tests and I didn't see any significant performance issue. Could you please run a benchmark and share the results with us? This way we can atest that this new feature will not affect significantly the VKMS performance. It would be nice to have a small brief of this benchmark on the commit message as well. Attesting that there isn't a performance issue and adding those nits to the commit message, you can add my Reviewed-by: Maíra Canal on the next version. Hi, perfomance testing is good indeed. As future work, could there be a document describing how to test VKMS performance? I'll try to select a couple of more meaningful IGT tests to describe how to test the VKMS performance and also add a document to describe how to run this tests. Recently, I added a VKMS must-pass testlist to IGT. This testlist tries to assure that regressions will not be introduced into VKMS. But, I failed to introduce a documentation on the kernel side pointing to this new testlist... I'll also work on it. "I ran IGT@blah 100 times and it took xx seconds before and yy seconds after" does not really give someone like me an idea of what was actually measured. For example blending overhead increase could be completely lost in opaque pixel copying noise if the test case has only few pixels to blend, e.g. a cursor plane, not to mention the overhead of launching an IGT test in the first place. About the IGT overhead, I don't know exactly how we could escape from it. Maybe writing KUnit tests to the VKMS's composition functions, such as blend(). Anyway, we would have the overhead of the KUnit framework. I mean, for whatever framework we choose, there'll be an overhead... Do you have any other ideas on how to test VKMS with less overhead? Maybe put the repeat loop and time measurement inside the code of a few chosen IGT tests? So that it loops only the KMS programming and somehow ensures VKMS has finished processing each update before doing the next cycle. I presume VKMS does not have a timer-based refresh cycle that might add CPU idle time? Writeback should be included in the measurement too, but inspecting writeback results should not. Once all that is in place, then each performance test needs to use appropriate operations. E.g. if testing blending performance, use almost full-screen planes. ^ Grillo, any chance you could work on something like this for the performance measurements? What's the overhead of KUnit framework? Can you not do the same there, put the repeat loop and time measurement inside the test to cover only the interesting code? Unit-testing the composition function performance might be ideal. I'll try to work on some unit tests for, at least, the composition section of VKMS. I believe that they will be very valuable for the maintenance and performance evaluation. Thanks for your valuable inputs on the VKMS! Best Regards, - Maíra Depending on the type of test, if the CRTC mode and planes are big enough, maybe there is no need to repeat even. But testing presumably fast things like moving a cursor plane will likely need repeating in order to produce stable numbers. Thanks, pq Best Regards, - Maíra Something that would guide new developers in running meaningful benchmarks would be nice. Should e.g. IGT have explicit (VKMS) performance tests that need to be run manually, since evaluation of the result is not feasible automatically? Or a benchmark mode in correctness tests that would run the identical operation N times and measure the time before checking for correctness? The correctness verification in IGT tests, if done by image comparison which they undoubtedly will need to be in the future, may dominate the CPU run time measurements if included. Thanks, pq
Re: [PATCH v2] drm/tests: Fix swapped drm_framebuffer tests parameter names
On 6/24/23 18:29, Carlos Eduardo Gallo Filho wrote:
Swap tests parameters names so they actually reflect what is being tested.
v1: https://lore.kernel.org/all/20230623152518.8603-1-gcar...@disroot.org/
v2: Simplified commit message.
Signed-off-by: Carlos Eduardo Gallo Filho
Reviewed-by: André Almeida
Reviewed-by: Maíra Canal
Applied to drm-misc/drm-misc-next. Thanks!
Best Regards,
- Maíra
---
drivers/gpu/drm/tests/drm_framebuffer_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c
b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index df235b7fdaa5..f759d9f3b76e 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -178,13 +178,13 @@ static const struct drm_framebuffer_test
drm_framebuffer_create_cases[] = {
.handles = { 1, 1, 1 }, .pitches = { 600, 600, 600 },
}
},
-{ .buffer_created = 1, .name = "YVU420 Normal sizes",
+{ .buffer_created = 1, .name = "YVU420 DRM_MODE_FB_MODIFIERS set without
modifier",
.cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_YVU420,
.handles = { 1, 1, 1 }, .flags = DRM_MODE_FB_MODIFIERS,
.pitches = { 600, 300, 300 },
}
},
-{ .buffer_created = 1, .name = "YVU420 DRM_MODE_FB_MODIFIERS set without
modifier",
+{ .buffer_created = 1, .name = "YVU420 Normal sizes",
.cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_YVU420,
.handles = { 1, 1, 1 }, .pitches = { 600, 300, 300 },
}
Re: [PATCH v4] drm/vkms: Add support to 1D gamma LUT
Hi Pekka, On 6/26/23 05:17, Pekka Paalanen wrote: On Sat, 24 Jun 2023 18:48:08 -0300 Maira Canal wrote: Hi Arthur, Thanks for working on this feature for the VKMS! On 6/21/23 16:41, Arthur Grillo wrote: Support a 1D gamma LUT with interpolation for each color channel on the VKMS driver. Add a check for the LUT length by creating vkms_atomic_check(). Tested with: igt@kms_color@gamma igt@kms_color@legacy-gamma igt@kms_color@invalid-gamma-lut-sizes Could you also mention that this will make it possible to run the test igt@kms_plane@pixel-format? Also, you mentioned to me that the performance degraded with this new feature, but I wasn't able to see it while running the VKMS CI. I performed a couple of tests and I didn't see any significant performance issue. Could you please run a benchmark and share the results with us? This way we can atest that this new feature will not affect significantly the VKMS performance. It would be nice to have a small brief of this benchmark on the commit message as well. Attesting that there isn't a performance issue and adding those nits to the commit message, you can add my Reviewed-by: Maíra Canal on the next version. Hi, perfomance testing is good indeed. As future work, could there be a document describing how to test VKMS performance? I'll try to select a couple of more meaningful IGT tests to describe how to test the VKMS performance and also add a document to describe how to run this tests. Recently, I added a VKMS must-pass testlist to IGT. This testlist tries to assure that regressions will not be introduced into VKMS. But, I failed to introduce a documentation on the kernel side pointing to this new testlist... I'll also work on it. "I ran IGT@blah 100 times and it took xx seconds before and yy seconds after" does not really give someone like me an idea of what was actually measured. For example blending overhead increase could be completely lost in opaque pixel copying noise if the test case has only few pixels to blend, e.g. a cursor plane, not to mention the overhead of launching an IGT test in the first place. About the IGT overhead, I don't know exactly how we could escape from it. Maybe writing KUnit tests to the VKMS's composition functions, such as blend(). Anyway, we would have the overhead of the KUnit framework. I mean, for whatever framework we choose, there'll be an overhead... Do you have any other ideas on how to test VKMS with less overhead? Best Regards, - Maíra Something that would guide new developers in running meaningful benchmarks would be nice. Should e.g. IGT have explicit (VKMS) performance tests that need to be run manually, since evaluation of the result is not feasible automatically? Or a benchmark mode in correctness tests that would run the identical operation N times and measure the time before checking for correctness? The correctness verification in IGT tests, if done by image comparison which they undoubtedly will need to be in the future, may dominate the CPU run time measurements if included. Thanks, pq
Re: [PATCH v2 4/6] drm/vkms: Add ConfigFS scaffolding to VKMS
Hi Jim, On 6/23/23 19:23, Jim Shargo wrote: This change adds the basic scaffolding for ConfigFS, including setting up the default directories. It does not allow for the registration of configfs-backed devices, which is complex and provided in a follow-up commit. This CL includes docs about using ConfigFS with VKMS, but I'll summarize in brief here as well (assuming ConfigFS is mounted at /config/): To create a new device, you can do so via `mkdir /config/vkms/my-device`. This will create a number of directories and files automatically: /config `-- vkms `-- my-device |-- connectors |-- crtcs |-- encoders |-- planes `-- enabled You can then configure objects by mkdir'ing in each of the directories. When you're satisfied, you can `echo 1 > /config/vkms/my-device/enabled`. This will create a new device according to your configuration. For now, this will fail, but the next change will add support for it. Signed-off-by: Jim Shargo --- Documentation/gpu/vkms.rst | 17 +- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/vkms/Makefile| 1 + drivers/gpu/drm/vkms/vkms_configfs.c | 646 +++ drivers/gpu/drm/vkms/vkms_drv.c | 52 ++- drivers/gpu/drm/vkms/vkms_drv.h | 92 +++- drivers/gpu/drm/vkms/vkms_output.c | 5 + 7 files changed, 799 insertions(+), 15 deletions(-) create mode 100644 drivers/gpu/drm/vkms/vkms_configfs.c diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst index ba04ac7c2167..2c342ef0fb7b 100644 --- a/Documentation/gpu/vkms.rst +++ b/Documentation/gpu/vkms.rst @@ -51,6 +51,12 @@ To disable the driver, use :: sudo modprobe -r vkms +Configuration With ConfigFS +=== + +.. kernel-doc:: drivers/gpu/drm/vkms/vkms_configfs.c + :doc: ConfigFS Support for VKMS + Testing With IGT @@ -135,22 +141,15 @@ project. Runtime Configuration - -We want to be able to reconfigure vkms instance without having to reload the -module. Use/Test-cases: +We want to vkms instances without having to reload the module. Such I believe that there is a verb missing here. +configuration can be added as extensions to vkms's ConfigFS support. Use-cases: - Hotplug/hotremove connectors on the fly (to be able to test DP MST handling of compositors). -- Configure planes/crtcs/connectors (we'd need some code to have more than 1 of - them first). - - Change output configuration: Plug/unplug screens, change EDID, allow changing the refresh rate. -The currently proposed solution is to expose vkms configuration through -configfs. All existing module options should be supported through configfs -too. - Writeback support - diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index afb3b2f5f425..71eb913b378f 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -262,6 +262,7 @@ config DRM_VKMS depends on DRM && MMU select DRM_KMS_HELPER select DRM_GEM_SHMEM_HELPER + select CONFIGFS_FS select CRC32 default n help diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile index 1b28a6a32948..6b83907ad554 100644 --- a/drivers/gpu/drm/vkms/Makefile +++ b/drivers/gpu/drm/vkms/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only vkms-y := \ + vkms_configfs.o \ vkms_drv.o \ vkms_plane.o \ vkms_output.o \ diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c new file mode 100644 index ..544024735d19 --- /dev/null +++ b/drivers/gpu/drm/vkms/vkms_configfs.c @@ -0,0 +1,646 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include +#include +#include + +#include +#include + +#include "vkms_drv.h" + +/** + * DOC: ConfigFS Support for VKMS + * + * VKMS is instrumented with support for configuration via :doc:`ConfigFS + * <../filesystems/configfs>`. + * + * With VKMS installed, you can mount ConfigFS at ``/config/`` like so:: + * + * mkdir -p /config/ + * sudo mount -t configfs none /config + * + * This allows you to configure multiple virtual devices in addition to an + * immutable "default" device created by the driver at initialization time. Note + * that the default device is immutable because we cannot pre-populate ConfigFS + * directories with normal files. + * + * To set up a new device, create a new directory under the VKMS configfs + * directory:: + * + * mkdir /config/vkms/test + * + * With your device created you'll find an new directory ready to be + * configured:: + * + * /config + * `-- vkms + * |-- default + * | `-- enabled + * `-- test + * |-- connectors + * |-- crtcs + * |-- encoders + * |-- planes + * `-- enabled I follo
Re: [PATCH v2 6/6] drm/vkms: Add a module param to enable/disable the default device
Hi Jim,
On 6/23/23 19:23, Jim Shargo wrote:
In many testing circumstances, we will want to just create a new device
and test against that. If we create a default device, it can be annoying
to have to manually select the new device instead of choosing the only
one that exists.
The param, enable_default, is defaulted to true to maintain backwards
compatibility.
Signed-off-by: Jim Shargo
---
drivers/gpu/drm/vkms/vkms_drv.c | 44 ++---
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 314a04659c5f..1cb56c090a65 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -42,17 +42,26 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
+static bool enable_default_device = true;
+module_param_named(enable_default_device, enable_default_device, bool, 0444);
+MODULE_PARM_DESC(enable_default_device,
+"Enable/Disable creating the default device");
Wouldn't be better to just call it "enable_default"?
Also, could you add this parameter to vkms_config debugfs file?
Best Regards,
- Maíra
+
static bool enable_cursor = true;
module_param_named(enable_cursor, enable_cursor, bool, 0444);
-MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
+MODULE_PARM_DESC(enable_cursor,
+"Enable/Disable cursor support for the default device");
static bool enable_writeback = true;
module_param_named(enable_writeback, enable_writeback, bool, 0444);
-MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector
support");
+MODULE_PARM_DESC(
+ enable_writeback,
+ "Enable/Disable writeback connector support for the default device");
static bool enable_overlay;
module_param_named(enable_overlay, enable_overlay, bool, 0444);
-MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
+MODULE_PARM_DESC(enable_overlay,
+"Enable/Disable overlay support for the default device");
DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
@@ -278,10 +287,7 @@ void vkms_remove_device(struct vkms_device *vkms_device)
static int __init vkms_init(void)
{
int ret;
- struct platform_device *pdev;
- struct vkms_device_setup vkms_device_setup = {
- .configfs = NULL,
- };
+ struct platform_device *default_pdev = NULL;
ret = platform_driver_register(&vkms_platform_driver);
if (ret) {
@@ -289,19 +295,27 @@ static int __init vkms_init(void)
return ret;
}
- pdev = platform_device_register_data(NULL, DRIVER_NAME, 0,
-&vkms_device_setup,
-sizeof(vkms_device_setup));
- if (IS_ERR(pdev)) {
- DRM_ERROR("Unable to register default vkms device\n");
- platform_driver_unregister(&vkms_platform_driver);
- return PTR_ERR(pdev);
+ if (enable_default_device) {
+ struct vkms_device_setup vkms_device_setup = {
+ .configfs = NULL,
+ };
+
+ default_pdev = platform_device_register_data(
+ NULL, DRIVER_NAME, 0, &vkms_device_setup,
+ sizeof(vkms_device_setup));
+ if (IS_ERR(default_pdev)) {
+ DRM_ERROR("Unable to register default vkms device\n");
+ platform_driver_unregister(&vkms_platform_driver);
+ return PTR_ERR(default_pdev);
+ }
}
ret = vkms_init_configfs();
if (ret) {
DRM_ERROR("Unable to initialize configfs\n");
- platform_device_unregister(pdev);
+ if (default_pdev)
+ platform_device_unregister(default_pdev);
+
platform_driver_unregister(&vkms_platform_driver);
}
Re: [PATCH v2 2/6] drm/vkms: Support multiple DRM objects (crtcs, etc.) per VKMS device
Hi Jim,
On 6/23/23 19:23, Jim Shargo wrote:
This change supports multiple CRTCs, encoders, connectors instead of one
of each per device.
Since ConfigFS-based devices will support multiple crtcs, it's useful to
move all of the writeback/composition data from being per-"output" to
being per-CRTC.
Since there's still only ever one CRTC, this should be a no-op refactor.
Signed-off-by: Jim Shargo
---
drivers/gpu/drm/vkms/vkms_composer.c | 28 +++---
drivers/gpu/drm/vkms/vkms_crtc.c | 95 +++-
drivers/gpu/drm/vkms/vkms_drv.c | 14 +--
drivers/gpu/drm/vkms/vkms_drv.h | 65 +-
drivers/gpu/drm/vkms/vkms_output.c| 123 ++
drivers/gpu/drm/vkms/vkms_plane.c | 44 ++---
drivers/gpu/drm/vkms/vkms_writeback.c | 26 +++---
7 files changed, 252 insertions(+), 143 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 1636ce3a79f9..75fa42e0ec07 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -230,13 +230,13 @@ void vkms_composer_worker(struct work_struct *work)
composer_work);
struct drm_crtc *crtc = crtc_state->base.crtc;
struct vkms_writeback_job *active_wb = crtc_state->active_writeback;
- struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+ struct vkms_crtc *vkms_crtc = drm_crtc_to_vkms_crtc(crtc);
bool crc_pending, wb_pending;
u64 frame_start, frame_end;
u32 crc32 = 0;
int ret;
- spin_lock_irq(&out->composer_lock);
+ spin_lock_irq(&vkms_crtc->composer_lock);
frame_start = crtc_state->frame_start;
frame_end = crtc_state->frame_end;
crc_pending = crtc_state->crc_pending;
@@ -244,7 +244,7 @@ void vkms_composer_worker(struct work_struct *work)
crtc_state->frame_start = 0;
crtc_state->frame_end = 0;
crtc_state->crc_pending = false;
- spin_unlock_irq(&out->composer_lock);
+ spin_unlock_irq(&vkms_crtc->composer_lock);
/*
* We raced with the vblank hrtimer and previous work already computed
@@ -262,10 +262,10 @@ void vkms_composer_worker(struct work_struct *work)
return;
if (wb_pending) {
- drm_writeback_signal_completion(&out->wb_connector, 0);
- spin_lock_irq(&out->composer_lock);
+ drm_writeback_signal_completion(&vkms_crtc->wb_connector, 0);
+ spin_lock_irq(&vkms_crtc->composer_lock);
crtc_state->wb_pending = false;
- spin_unlock_irq(&out->composer_lock);
+ spin_unlock_irq(&vkms_crtc->composer_lock);
}
/*
@@ -315,25 +315,25 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
char *src_name,
return 0;
}
-void vkms_set_composer(struct vkms_output *out, bool enabled)
+void vkms_set_composer(struct vkms_crtc *vkms_crtc, bool enabled)
{
bool old_enabled;
if (enabled)
- drm_crtc_vblank_get(&out->crtc);
+ drm_crtc_vblank_get(&vkms_crtc->base);
- spin_lock_irq(&out->lock);
- old_enabled = out->composer_enabled;
- out->composer_enabled = enabled;
- spin_unlock_irq(&out->lock);
+ spin_lock_irq(&vkms_crtc->lock);
+ old_enabled = vkms_crtc->composer_enabled;
+ vkms_crtc->composer_enabled = enabled;
+ spin_unlock_irq(&vkms_crtc->lock);
if (old_enabled)
- drm_crtc_vblank_put(&out->crtc);
+ drm_crtc_vblank_put(&vkms_crtc->base);
}
int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name)
{
- struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+ struct vkms_crtc *out = drm_crtc_to_vkms_crtc(crtc);
bool enabled = false;
int ret = 0;
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 515f6772b866..d91e49c53adc 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -11,35 +11,34 @@
static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
{
- struct vkms_output *output = container_of(timer, struct vkms_output,
- vblank_hrtimer);
- struct drm_crtc *crtc = &output->crtc;
+ struct vkms_crtc *vkms_crtc = timer_to_vkms_crtc(timer);
+ struct drm_crtc *crtc = &vkms_crtc->base;
struct vkms_crtc_state *state;
u64 ret_overrun;
bool ret, fence_cookie;
fence_cookie = dma_fence_begin_signalling();
- ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer,
- output->period_ns);
+ ret_overrun = hrtimer_forward_now(&vkms_crtc->vblank_hrtimer,
+ vkms_crtc->period_ns);
if (ret_overrun != 1)
pr_warn("%s: vblank timer ove
Re: [PATCH v2 1/6] drm/vkms: Back VKMS with DRM memory management instead of static objects
Hi Jim,
Thanks for working on this great feature for the VKMS!
On 6/23/23 19:23, Jim Shargo wrote:
This is a small refactor to make ConfigFS support easier. Once we
support ConfigFS, there can be multiple devices instantiated by the
driver, and so moving everything into managed memory makes things much
easier.
This should be a no-op refactor.
Signed-off-by: Jim Shargo
---
drivers/gpu/drm/vkms/vkms_drv.c| 130 +++--
drivers/gpu/drm/vkms/vkms_drv.h| 4 +-
drivers/gpu/drm/vkms/vkms_output.c | 6 +-
3 files changed, 72 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index e3c9c9571c8d..f07454fff046 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -9,10 +9,12 @@
* the GPU in DRM API tests.
*/
+#include
#include
#include
#include
+#include
#include
#include
#include
@@ -37,8 +39,6 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
-static struct vkms_config *default_config;
-
static bool enable_cursor = true;
module_param_named(enable_cursor, enable_cursor, bool, 0444);
MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
@@ -92,13 +92,13 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state
*old_state)
static int vkms_config_show(struct seq_file *m, void *data)
{
- struct drm_debugfs_entry *entry = m->private;
- struct drm_device *dev = entry->dev;
- struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
+ struct drm_info_node *drm_info = m->private;
The VKMS already moved to the new DRM debugfs interface, there is no
need to move back to the old interface. I believe you should keep
using struct drm_debugfs_entry.
Also, that's probably the reason why the vkms_config file is not being
properly updated:
[root@fedora ~]# modprobe vkms enable_overlay=1 enable_cursor=1
enable_writeback=1
[root@fedora ~]# cd /sys/kernel/debug/dri/0
[root@fedora 0]# cat vkms_config
writeback=0
cursor=0
overlay=0
+ struct vkms_device *vkms_device =
+ drm_device_to_vkms_device(drm_info->minor->dev);
- seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
- seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
- seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
+ seq_printf(m, "writeback=%d\n", vkms_device->config.writeback);
+ seq_printf(m, "cursor=%d\n", vkms_device->config.cursor);
+ seq_printf(m, "overlay=%d\n", vkms_device->config.overlay);
return 0;
}
@@ -155,114 +155,120 @@ static int vkms_modeset_init(struct vkms_device
*vkmsdev)
return vkms_output_init(vkmsdev, 0);
}
-static int vkms_create(struct vkms_config *config)
+static int vkms_platform_probe(struct platform_device *pdev)
{
int ret;
- struct platform_device *pdev;
struct vkms_device *vkms_device;
+ void *grp;
- pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
- if (IS_ERR(pdev))
- return PTR_ERR(pdev);
-
- if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
- ret = -ENOMEM;
- goto out_unregister;
+ grp = devres_open_group(&pdev->dev, NULL, GFP_KERNEL);
+ if (!grp) {
+ return -ENOMEM;
}
vkms_device = devm_drm_dev_alloc(&pdev->dev, &vkms_driver,
struct vkms_device, drm);
if (IS_ERR(vkms_device)) {
ret = PTR_ERR(vkms_device);
- goto out_devres;
+ goto out_release_group;
}
+
vkms_device->platform = pdev;
- vkms_device->config = config;
- config->dev = vkms_device;
+ vkms_device->config.cursor = enable_cursor;
+ vkms_device->config.writeback = enable_writeback;
+ vkms_device->config.overlay = enable_overlay;
ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
DMA_BIT_MASK(64));
-
if (ret) {
DRM_ERROR("Could not initialize DMA support\n");
- goto out_devres;
+ goto out_release_group;
}
ret = drm_vblank_init(&vkms_device->drm, 1);
if (ret) {
DRM_ERROR("Failed to vblank\n");
- goto out_devres;
+ goto out_release_group;
}
ret = vkms_modeset_init(vkms_device);
- if (ret)
- goto out_devres;
+ if (ret) {
+ DRM_ERROR("Unable to initialize modesetting\n");
+ goto out_release_group;
+ }
drm_debugfs_add_files(&vkms_device->drm, vkms_config_debugfs_list,
ARRAY_SIZE(vkms_config_debugfs_list));
ret = drm_dev_register(&vkms_device->drm, 0);
- if (ret)
- goto out_devres;
+ if (ret) {
+ DRM_ERROR("Unable to register device\n");
+
Re: [PATCH v4] drm/vkms: Add support to 1D gamma LUT
Hi Arthur,
Thanks for working on this feature for the VKMS!
On 6/21/23 16:41, Arthur Grillo wrote:
Support a 1D gamma LUT with interpolation for each color channel on the
VKMS driver. Add a check for the LUT length by creating
vkms_atomic_check().
Tested with:
igt@kms_color@gamma
igt@kms_color@legacy-gamma
igt@kms_color@invalid-gamma-lut-sizes
Could you also mention that this will make it possible to run the test
igt@kms_plane@pixel-format?
Also, you mentioned to me that the performance degraded with this new
feature, but I wasn't able to see it while running the VKMS CI. I
performed a couple of tests and I didn't see any significant performance
issue.
Could you please run a benchmark and share the results with us? This way
we can atest that this new feature will not affect significantly the
VKMS performance. It would be nice to have a small brief of this
benchmark on the commit message as well.
Attesting that there isn't a performance issue and adding those nits to
the commit message, you can add my
Reviewed-by: Maíra Canal
on the next version.
v2:
- Add interpolation between the values of the LUT (Simon Ser)
v3:
- s/ratio/delta (Pekka)
- s/color_channel/channel_value (Pekka)
- s/lut_area/lut_channel
- Store the `drm_color_lut`, `lut_length`, and
`channel_value2index_ratio` inside a struct called `vkms_lut`
(Pekka)
- Pre-compute some constants values used through the LUT procedure
(Pekka)
- Change the switch statement to a cast to __u16* (Pekka)
- Make the apply_lut_to_channel_value return the computation result
(Pekka)
v4:
- Add a comment explaining that `enum lut_area` depends on the
layout of `struct drm_color_lut` (Pekka)
- Remove unused variable (kernel test robot)
Signed-off-by: Arthur Grillo
Acked-by: Pekka Paalanen
---
drivers/gpu/drm/vkms/vkms_composer.c | 86
drivers/gpu/drm/vkms/vkms_crtc.c | 3 +
drivers/gpu/drm/vkms/vkms_drv.c | 20 ++-
drivers/gpu/drm/vkms/vkms_drv.h | 9 +++
4 files changed, 117 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 906d3df40cdb..620c0bafbe56 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -89,6 +90,73 @@ static void fill_background(const struct pixel_argb_u16
*background_color,
output_buffer->pixels[i] = *background_color;
}
+// lerp(a, b, t) = a + (b - a) * t
+static u16 lerp_u16(u16 a, u16 b, s64 t)
+{
+ s64 a_fp = drm_int2fixp(a);
+ s64 b_fp = drm_int2fixp(b);
+
+ s64 delta = drm_fixp_mul(b_fp - a_fp, t);
+
+ return drm_fixp2int(a_fp + delta);
+}
+
+static s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value)
+{
+ s64 color_channel_fp = drm_int2fixp(channel_value);
+
+ return drm_fixp_mul(color_channel_fp, lut->channel_value2index_ratio);
+}
+
+/*
+ * This enum is related to the positions of the variables inside
+ * `struct drm_color_lut`, so the order of both needs to be the same.
+ */
+enum lut_channel {
+ LUT_RED = 0,
+ LUT_GREEN,
+ LUT_BLUE,
+ LUT_RESERVED
+};
+
+static u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16
channel_value,
+ enum lut_channel channel)
+{
+ s64 lut_index = get_lut_index(lut, channel_value);
+
+ /*
+* This checks if `struct drm_color_lut` had any gap added by the
compiler
s/had/has
Best Regards,
- Maíra
+* between the struct fields.
+*/
+ static_assert(sizeof(struct drm_color_lut) == sizeof(__u16) * 4);
+
+ u16 *floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
+ u16 *ceil_lut_value = (__u16 *)&lut->base[drm_fixp2int_ceil(lut_index)];
+
+ u16 floor_channel_value = floor_lut_value[channel];
+ u16 ceil_channel_value = ceil_lut_value[channel];
+
+ return lerp_u16(floor_channel_value, ceil_channel_value,
+ lut_index & DRM_FIXED_DECIMAL_MASK);
+}
+
+static void apply_lut(const struct vkms_crtc_state *crtc_state, struct
line_buffer *output_buffer)
+{
+ if (!crtc_state->gamma_lut.base)
+ return;
+
+ if (!crtc_state->gamma_lut.lut_length)
+ return;
+
+ for (size_t x = 0; x < output_buffer->n_pixels; x++) {
+ struct pixel_argb_u16 *pixel = &output_buffer->pixels[x];
+
+ pixel->r = apply_lut_to_channel_value(&crtc_state->gamma_lut,
pixel->r, LUT_RED);
+ pixel->g = apply_lut_to_channel_value(&crtc_state->gamma_lut,
pixel->g, LUT_GREEN);
+ pixel->b = apply_lut_to_channel_value(&crtc_state->gamma_lut,
pixel->b, LUT_BLUE);
+ }
+}
+
/**
* @wb_frame_info: The writeback frame b
Re: [PATCH] Tercera entrega completa
Hi edagarmarjara,
First, you need to include a commit message to the patch. Check [1] to
see a basic guide to submit patches.
On 6/19/23 20:22, edagarmarjara wrote:
---
drivers/gpu/drm/tests/drm_rect_test.c | 30 +++
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
b/drivers/gpu/drm/tests/drm_rect_test.c
index e9809ea32696..d03e1d9b208d 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -35,6 +35,7 @@ static void drm_test_rect_clip_scaled_div_by_zero(struct
kunit *test)
KUNIT_EXPECT_FALSE_MSG(test, drm_rect_visible(&src), "Source should not be
visible\n");
}
+
This line is not needed. You can run checkpatch.sh to catch common style
mistakes.
static void drm_test_rect_clip_scaled_not_clipped(struct kunit *test)
{
struct drm_rect src, dst, clip;
@@ -196,11 +197,40 @@ static void
drm_test_rect_clip_scaled_signed_vs_unsigned(struct kunit *test)
KUNIT_EXPECT_FALSE_MSG(test, drm_rect_visible(&src), "Source should not be
visible\n");
}
+static void drm_test_rect_clip_over_scaled_signed_vs_unsigned(struct kunit *test)
+{
+
+ const void* gem_params(const void *prev, char *desc);
Hum... I guess you don't need this function signature here.
+ struct drm_rect src, dst, clip;
+ bool visible;
+
+ /*
+* 'clip.x2 - dst.x1 >= dst width' could result a negative
+* src rectangle width which is no longer expected by the
+* code as it's using unsigned types. This could lead to
+* the clipped source rectangle appering visible when it
+* should have been fully clipped. Make sure both rectangles
+* end up invisible.
+* en esta parte cambio los valores y hago por aun mas afuera para el
clip scaled
+* para poder saber si al exagerar mas aun la escala sigue funcionando
I believe you can try to explain the test in smaller comments. Sometimes
the tests explain by itself. Also, avoid to use Spanish in comments.
+*/
+ drm_rect_init(&src, 2, 2, INT_MAX, INT_MAX);
+ drm_rect_init(&dst, 2, 2, 4, 4);
+ drm_rect_init(&clip, 6, 6, 3, 3);
+
+ visible = drm_rect_clip_scaled(&src, &dst, &clip);
+
+ KUNIT_EXPECT_FALSE_MSG(test, visible, "Destination should not be
visible\n");
+ KUNIT_EXPECT_FALSE_MSG(test, drm_rect_visible(&src), "Source should not be
visible\n");
I believe you could introduce more test cases for this test instead of
only one.
+}
+
+
static struct kunit_case drm_rect_tests[] = {
KUNIT_CASE(drm_test_rect_clip_scaled_div_by_zero),
KUNIT_CASE(drm_test_rect_clip_scaled_not_clipped),
KUNIT_CASE(drm_test_rect_clip_scaled_clipped),
KUNIT_CASE(drm_test_rect_clip_scaled_signed_vs_unsigned),
+ KUNIT_CASE(drm_test_rect_clip_over_scaled_signed_vs_unsigned), //Test
entrega 2
I believe you could remove the comment here.
[1] https://docs.kernel.org/process/submitting-patches.html
Best Regards,
- Maíra
{ }
};
Re: [PATCH] drm/tests: Fix swapped test parameter names
Hi Carlos,
Great catch!
On 6/23/23 12:25, Carlos Eduardo Gallo Filho wrote:
The "YVU420 DRM_MODE_FB_MODIFIERS set without modifier" test
hadn't DRM_MODE_FB_MODIFIERS set, so that it was in fact testing
another case, while the "YVU420 Normal sizes" test in turn was with
DRM_MODE_FB_MODIFIERS set and without modifiers, what should be
the case tested by the former, which also in turn fit in what
"YVU320 Normal sizes" should be, meaning that they were swapped.
Signed-off-by: Carlos Eduardo Gallo Filho
With André's comment addressed,
Reviewed-by: Maíra Canal
Best Regards,
- Maíra
---
drivers/gpu/drm/tests/drm_framebuffer_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c
b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index df235b7fdaa5..f759d9f3b76e 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -178,13 +178,13 @@ static const struct drm_framebuffer_test
drm_framebuffer_create_cases[] = {
.handles = { 1, 1, 1 }, .pitches = { 600, 600, 600 },
}
},
-{ .buffer_created = 1, .name = "YVU420 Normal sizes",
+{ .buffer_created = 1, .name = "YVU420 DRM_MODE_FB_MODIFIERS set without
modifier",
.cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_YVU420,
.handles = { 1, 1, 1 }, .flags = DRM_MODE_FB_MODIFIERS,
.pitches = { 600, 300, 300 },
}
},
-{ .buffer_created = 1, .name = "YVU420 DRM_MODE_FB_MODIFIERS set without
modifier",
+{ .buffer_created = 1, .name = "YVU420 Normal sizes",
.cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_YVU420,
.handles = { 1, 1, 1 }, .pitches = { 600, 300, 300 },
}
