Hi Melissa, Added a note about that in the latest version. Thanks! On Mon, Apr 10, 2023 at 05:17:56PM -0100, Melissa Wen wrote: > On 04/06, Marius Vlad wrote: > > Hi Thomas, > > > > Thanks for the clarifications! Made a couple of remarks in-line. > > > > On 4/6/23 14:29, Thomas Zimmermann wrote: > > > Hi > > > > > > Am 06.04.23 um 11:17 schrieb Marius Vlad: > > > > Hi Maira, > > > > > > > > Thanks a lot for taking a look. Yeah, indeed, this creates a new > > > > connector, a CRTC and planes for it. Terminology wise, multiple > > > > pipes. The idea is to have multiple outputs, each with its own > > > > connector, as to be able to simulate (a few) more outputs. At least > > > > that's what I'm looking for. > > > > > > > > I'll adjust the commits description to clarify that. > > > > > > > > Regarding the planes, it seemed a bit easier to just create a new > > > > tuple of planes for each output, as in to reuse vkms_output_init(). > > > > So I guess that you're suggestion would be to make use the existing > > > > planes. > > > > > > > > Seems a bit more work, not that keen on changing that, but I do have > > > > some follow-up questions for my own curiosity in case I do this: > > > > > > > > - Don't I need an entire pipe (a primary plane, crtc, encoder, > > > > connector) to power up the end side of the sink (display)? > > > > > > Yes, you need at least one full pipeline. I don't see how you'd get > > > something displayed otherwise. > > > > > > > - Can the primary one be sufficient for multiple outputs? > > > > > > We have no concept of "primary pipelines." The individual components > > > have index numbers. There's a primary plane attached to each CRTC, but > > > even that concept comes more from HW limitations and historical designs, > > > than fundamental requirements. > > > > Right, meant the primary plane, rather than pipeline. > > > > > > > > For multiple outputs, you can attach multiple encoders/connectors to the > > > same CRTC. They will then all display the same screen at the same > > > display resolution > > Yeah, this kind of sounds like cloning to me, and would like to display > > different things at the same time, on different outputs, to me it sounds I > > need primary plane and a CRTC for each connector. Right? > > > > > > > > > - Can the overlay planes be shared or I need to > > > > discard the ones that are already in-use by other outputs? > > > > > > Even if your hardware planes support it, you cannot share planes among > > > CRTCs with DRM. At least I'm not aware how to. Each plane struct has a > > > designated CRTC struct. For most flexibility I'd have to match HW planes > > > and DRM planes dynamically. For example if you have 2 CRTCs that can > > > share 10 HW planes, you can allocate 10 DRM planes for each CRTC. The > > > atomic_check functions have to implement the mapping from hardware to > > > software plane and fail if no more hardware planes are available. > > > > > > If you want to display the same screen on multiple CRTCs, it's possible > > > to share the DRM framebuffers among similar the planes. > > > > Aham, understood, thanks again! > > Some drivers allow overlay planes to move between CRTCs. We have briefly > discussed on IRC that it would be interesting to have this (even for all > plane types) in vkms for testing and validation, but in a next step. As > it's not included in this proposal here, I'd suggest you to include this > feature/improvement in the vkms TODO in your next version, to keep it in > our minds for future works. > > BR, > > Melissa > > > > > > > > > Best regards > > > Thomas > > > > > > > > > > > I'll have a look at that writeback test as well see what's causing that > > > > NULL pointer deref. > > > > > > > > > > > > On 4/5/23 16:51, Maíra Canal wrote: > > > > > Hi Marius, > > > > > > > > > > > This patch adds support for creating multiple virtual connectors, in > > > > > > case one would need it. Use module parameters to specify how many, > > > > > > defaulting to just one, allocating from the start, a maximum of 4 > > > > > > possible outputs. > > > > > > > > > > I got a bit confused by this description. The commit message > > > > > says that you > > > > > are creating multiple connectors, but it seems like you are > > > > > creating multiple > > > > > pipes. From what I could see, for each new connector created, > > > > > you are also > > > > > creating a new CRTC and new planes. > > > > > > > > > > Moreover, if your real intention is to create multiple pipes, I > > > > > believe that > > > > > you don't really need to duplicate the planes as well. > > > > > > > > > > I ran the VKMS CI [1] with your patches applied and, although > > > > > most of the > > > > > tests are now passing with multiple pipes, which is really nice, > > > > > the test > > > > > kms_writeback crashes with the following error: > > > > > > > > > > [ 1997.244347] [IGT] kms_writeback: starting subtest > > > > > writeback-check-output > > > > > [ 1997.250673] BUG: kernel NULL pointer dereference, address: > > > > > 000000000000016c > > > > > [ 1997.250691] #PF: supervisor read access in kernel mode > > > > > [ 1997.250693] #PF: error_code(0x0000) - not-present page > > > > > [ 1997.250694] PGD 800000001a877067 P4D 800000001a877067 PUD > > > > > 1a872067 PMD 0 > > > > > [ 1997.250697] Oops: 0000 [#1] PREEMPT SMP PTI > > > > > [ 1997.250699] CPU: 2 PID: 3223 Comm: kms_writeback Not tainted > > > > > 6.3.0-rc4-g8c2c29ba129f-dirty #257 > > > > > [ 1997.250701] Hardware name: QEMU Standard PC (i440FX + PIIX, > > > > > 1996), BIOS 1.16.2-1.fc37 04/01/2014 > > > > > [ 1997.250703] RIP: 0010:drm_vblank_get+0xa/0xe0 > > > > > [ 1997.250710] Code: a9 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 > > > > > 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57 > > > > > 41 56 41 55 41 54 53 <8b> 87 6c 01 00 00 41 bc ea ff ff ff 85 c0 > > > > > 74 74 89 f5 48 89 fb 39 > > > > > [ 1997.250712] RSP: 0018:ffffb84d407a3b08 EFLAGS: 00010202 > > > > > [ 1997.250714] RAX: 0000000000000000 RBX: ffffa2eb02bf8b70 RCX: > > > > > 0000000000000000 > > > > > [ 1997.250718] RDX: ffffa2eb180d2420 RSI: 0000000000000000 RDI: > > > > > 0000000000000000 > > > > > [ 1997.250719] RBP: ffffa2eb02bf99e8 R08: 0000000000000036 R09: > > > > > ffffa2eb01d620c0 > > > > > [ 1997.250720] R10: ffffe82b84027e40 R11: ffffffffc0042520 R12: > > > > > ffffa2eb01c01900 > > > > > [ 1997.250721] R13: ffffa2eb02bf9b60 R14: 0000000000000001 R15: > > > > > ffffa2ea1ecd6b80 > > > > > [ 1997.250722] FS: 00007f7d44e89a80(0000) > > > > > GS:ffffa2eb3bd00000(0000) knlGS:0000000000000000 > > > > > [ 1997.250723] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > > [ 1997.250725] CR2: 000000000000016c CR3: 000000001ec02000 CR4: > > > > > 00000000000006e0 > > > > > [ 1997.250728] Call Trace: > > > > > [ 1997.250735] <TASK> > > > > > [ 1997.250736] vkms_set_composer+0x18/0x60 [vkms] > > > > > [ 1997.250745] vkms_wb_atomic_commit+0x93/0x150 [vkms] > > > > > [ 1997.250749] drm_atomic_helper_commit_modeset_enables+0x1d9/0x250 > > > > > [ 1997.250754] vkms_atomic_commit_tail+0x33/0xb0 [vkms] > > > > > [ 1997.250758] commit_tail+0x8d/0x170 > > > > > [ 1997.250761] drm_atomic_helper_commit+0x26b/0x280 > > > > > [ 1997.250763] drm_atomic_commit+0x9f/0xc0 > > > > > [ 1997.250766] ? __pfx___drm_printfn_info+0x10/0x10 > > > > > [ 1997.250769] drm_mode_atomic_ioctl+0x34c/0x480 > > > > > [ 1997.250771] ? __pfx_drm_mode_atomic_ioctl+0x10/0x10 > > > > > [ 1997.250773] drm_ioctl_kernel+0xd7/0x150 > > > > > [ 1997.250780] drm_ioctl+0x31f/0x490 > > > > > [ 1997.250781] ? __pfx_drm_mode_atomic_ioctl+0x10/0x10 > > > > > [ 1997.250783] __se_sys_ioctl+0x6d/0xb0 > > > > > [ 1997.250789] do_syscall_64+0x43/0x90 > > > > > [ 1997.250795] entry_SYSCALL_64_after_hwframe+0x72/0xdc > > > > > > > > > > [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/tree/master/tests/vkms_ci > > > > > > > > > > Best Regards, > > > > > - Maíra Canal > > > > > > > > > > > > > > > > > This is of particular importance when testing out the DRM backend in > > > > > > compositors, but also to be able to independently handle multiple > > > > > > outputs/connectors, like setting one to off/sleep on while the others > > > > > > are on, and combinations that arise from that. > > > > > > > > > > > > Signed-off-by: Marius Vlad <marius.vlad@xxxxxxxxxxxxx> > > > > > > --- > > > > > > drivers/gpu/drm/vkms/vkms_crtc.c | 3 +-- > > > > > > drivers/gpu/drm/vkms/vkms_drv.c | 26 ++++++++++++++++++++++---- > > > > > > drivers/gpu/drm/vkms/vkms_drv.h | 8 +++++--- > > > > > > drivers/gpu/drm/vkms/vkms_output.c | 5 ++--- > > > > > > drivers/gpu/drm/vkms/vkms_writeback.c | 18 ++++++++---------- > > > > > > 5 files changed, 38 insertions(+), 22 deletions(-) > > > > > > > > >
Attachment:
signature.asc
Description: PGP signature
