Re: [PATCH 0/3] drm/vkms: add support for multiple overlay planes
Hi Melissa, Thank you very much for your review. > On Thu, Dec 23, 2021 at 07:35:48PM -0100, Melissa Wen wrote: > What test did you run? Indeed, not all kms tests are passing and fixes > are welcome :) > > Last time, I used these testcases for overlay: kms_plane_cursor, > kms_atomic; and these tests were fine too: kms_cursor_crc, kms_writeback, > kms_flip For the different patches I have been working on I have tested mainly with kms_atomic, kms_plane_cursor and kms_plane_alpha_blend. For some reason, kms_cursor_crc suspends my PC. I still need to investigate the cause. I'll include a table with success/skip/fail tests before and after the patch on v2 :) > However, I think we need some limits for this number > of planes. I would suggest to just expand the enable_overlay option to > expose a predefined number of planes > [...] > I don't have a strong opinion on an exact/practical number. I took a > quick look at other drivers and exposing 8 planes seems reasonable to > me. 8 planes sound reasonable to me, I'll change it and send a revision of [1] as well using the new constant. Thanks again for taking the time to review this, José Expósito [1] https://lore.kernel.org/dri-devel/20211223081030.16629-1-jose.exposit...@gmail.com/T/
Re: [PATCH 0/3] drm/vkms: add support for multiple overlay planes
On 12/13, José Expósito wrote: > Hi all, > > First of all, let me quickly introduce myself. I'm José Expósito and > I'm a hobbyist open source developer. > My contributions are usually in the kernel input/HID subsystems and > in its userspace library (libinput) as well as other projects. > > I'm trying to learn more about the GPU and I found VKMS as a good > project to get started. > > So, now that you have been warned about my (lack) of experience in this > subsystem, let's get into the patches. > > The series adds support for multiple overlay planes by adding a new > module parameter that allows to configure the number of overlay planes > to add. > > I checked that the planes are properly created using drm_info[1] and > also executed the IGT test "kms_plane" to make sure that the planes > were listed in the output. > In addition, I checked the vkms_composer and -even though I'm not sure > how to properly test it- it looks like it is already prepared to > compose an arbitrary number of planes. > Having said that, I really hope I didn't make any obvious mistakes. > > I noticed that the docs say: > > For all of these, we also want to review the igt test coverage and > > make sure all relevant igt testcases work on vkms > > I ran some plane related tests, but some of them were already failing > without my code, so I'd appreciate some help with this bit. Hi José, What test did you run? Indeed, not all kms tests are passing and fixes are welcome :) Last time, I used these testcases for overlay: kms_plane_cursor, kms_atomic; and these tests were fine too: kms_cursor_crc, kms_writeback, kms_flip Did you find any regression? Also, a good practice is to report in the commit message which IGT tests you used to check the proposed changes. Btw, thanks for your patchset. Melissa > > Thank you very much in advance for your time. Any suggestions to > improve this patchset or about what to work on next are very welcome. > > Jose > > [1] https://github.com/ascent12/drm_info > > José Expósito (3): > drm/vkms: refactor overlay plane creation > drm/vkms: add support for multiple overlay planes > drm/vkms: drop "Multiple overlay planes" TODO > > Documentation/gpu/vkms.rst | 2 -- > drivers/gpu/drm/vkms/vkms_drv.c| 5 + > drivers/gpu/drm/vkms/vkms_drv.h| 1 + > drivers/gpu/drm/vkms/vkms_output.c | 29 ++--- > 4 files changed, 28 insertions(+), 9 deletions(-) > > -- > 2.25.1 > signature.asc Description: PGP signature
[PATCH 0/3] drm/vkms: add support for multiple overlay planes
Hi all, First of all, let me quickly introduce myself. I'm José Expósito and I'm a hobbyist open source developer. My contributions are usually in the kernel input/HID subsystems and in its userspace library (libinput) as well as other projects. I'm trying to learn more about the GPU and I found VKMS as a good project to get started. So, now that you have been warned about my (lack) of experience in this subsystem, let's get into the patches. The series adds support for multiple overlay planes by adding a new module parameter that allows to configure the number of overlay planes to add. I checked that the planes are properly created using drm_info[1] and also executed the IGT test "kms_plane" to make sure that the planes were listed in the output. In addition, I checked the vkms_composer and -even though I'm not sure how to properly test it- it looks like it is already prepared to compose an arbitrary number of planes. Having said that, I really hope I didn't make any obvious mistakes. I noticed that the docs say: > For all of these, we also want to review the igt test coverage and > make sure all relevant igt testcases work on vkms I ran some plane related tests, but some of them were already failing without my code, so I'd appreciate some help with this bit. Thank you very much in advance for your time. Any suggestions to improve this patchset or about what to work on next are very welcome. Jose [1] https://github.com/ascent12/drm_info José Expósito (3): drm/vkms: refactor overlay plane creation drm/vkms: add support for multiple overlay planes drm/vkms: drop "Multiple overlay planes" TODO Documentation/gpu/vkms.rst | 2 -- drivers/gpu/drm/vkms/vkms_drv.c| 5 + drivers/gpu/drm/vkms/vkms_drv.h| 1 + drivers/gpu/drm/vkms/vkms_output.c | 29 ++--- 4 files changed, 28 insertions(+), 9 deletions(-) -- 2.25.1
