[Bug 97909] X-Plane 10 crashes with SIGSEGV on radeonsi
https://bugs.freedesktop.org/show_bug.cgi?id=97909 --- Comment #12 from Christian Inci --- Unfortunately I'm not able to test this till three weeks. I'll let you know the result of the test after that, if nothing unexpected happens. What do you think about closing this bug as WONTFIX? Working around a third-party application bug at the operating system/at a library-level looks like a Microsoft thing to do. (shimming) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100166] Colored terrain/tents in Shadow of Mordor during weather storms
https://bugs.freedesktop.org/show_bug.cgi?id=100166 Timothy Arceri changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #2 from Timothy Arceri --- Does this still happen? If so can you get an apitrace[1] and upload it somewhere like google drive and share the link? [1] https://github.com/apitrace/apitrace/wiki/Steam -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 95659] HD 7450, R9-270, GPU locked when testing glmark2
https://bugs.freedesktop.org/show_bug.cgi?id=95659 Timothy Arceri changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #10 from Timothy Arceri --- Is this still and issue for you on a more recent software stack? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2] drm/amdgpu: limit DMA size to PAGE_SIZE for scatter-gather buffers
On Wed, Apr 11, 2018 at 01:03:59PM +0100, Robin Murphy wrote: > On 10/04/18 21:59, Sinan Kaya wrote: >> Code is expecing to observe the same number of buffers returned from >> dma_map_sg() function compared to sg_alloc_table_from_pages(). This >> doesn't hold true universally especially for systems with IOMMU. > > So why not fix said code? It's clearly not a real hardware limitation, and > the map_sg() APIs have potentially returned fewer than nents since forever, > so there's really no excuse. Yes, relying on dma_map_sg returning the same number of entries as passed it is completely bogus. >> IOMMU driver tries to combine buffers into a single DMA address as much >> as it can. The right thing is to tell the DMA layer how much combining >> IOMMU can do. > > Disagree; this is a dodgy hack, since you'll now end up passing > scatterlists into dma_map_sg() which already violate max_seg_size to begin > with, and I think a conscientious DMA API implementation would be at rights > to fail the mapping for that reason (I know arm64 happens not to, but that > was a deliberate design decision to make my life easier at the time). Agreed. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 97909] X-Plane 10 crashes with SIGSEGV on radeonsi
https://bugs.freedesktop.org/show_bug.cgi?id=97909 Timothy Arceri changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #11 from Timothy Arceri --- Is this still and issue for you? I just gave the X-Plane 10 demo a try had no crash. Although I did see artifacts flickering across the screen similar to the apitrace in bug 87059. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: fix drm-get-put.cocci warnings
From: Fengguang Wu Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and drm_*_unreference() helpers. Generated by: scripts/coccinelle/api/drm-get-put.cocci Fixes: 6784ac15bc68 ("drm: Add ASPEED GFX driver") Signed-off-by: Fengguang Wu Signed-off-by: Julia Lawall --- tree: https://github.com/shenki/linux drm-v1 head: 9680ed7979d5d403c6bde36271a048d62c048180 commit: 6784ac15bc6889280522b04b97f1fb1208ee45e7 [23/27] drm: Add ASPEED GFX driver aspeed_gfx_drv.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -268,7 +268,7 @@ static int aspeed_gfx_probe(struct platf err_unload: aspeed_gfx_unload(drm); err_free: - drm_dev_unref(drm); + drm_dev_put(drm); return ret; } @@ -279,7 +279,7 @@ static int aspeed_gfx_remove(struct plat drm_dev_unregister(drm); aspeed_gfx_unload(drm); - drm_dev_unref(drm); + drm_dev_put(drm); return 0; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104190] Artifacts in CSGO map de_cache when shader level is "high" or "very high (default)"
https://bugs.freedesktop.org/show_bug.cgi?id=104190 Timothy Arceri changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #6 from Timothy Arceri --- (In reply to Kristian Nyborg Dahl from comment #5) > (In reply to Kristian Nyborg Dahl from comment #1) > > This was kisak-valve's comment on their github: > > > > "Hello @Heis, on my AMD test box, mesa 17.2.6 built against llvm 5.0.0 is > > affected, while llvm 4.0.1 is not affected. > > > > This should also be brought to the attention of your driver vendor if it has > > not been already." > > I just tested with latest available mesa (17.3.7) and llvm (6.0.0.4). Seems > to be solved. Great! Thanks for retesting. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103972] GL_ARB_gpu_shader_int64. Compute shader. Shared storage.
https://bugs.freedesktop.org/show_bug.cgi?id=103972 Timothy Arceri changed: What|Removed |Added CC||t_arc...@yahoo.com.au --- Comment #1 from Timothy Arceri --- Created attachment 138773 --> https://bugs.freedesktop.org/attachment.cgi?id=138773&action=edit Piglit test I've turn this into a piglit test. We hit and assert when using the TGSI backend and the NIR backend doesn't currently handle storing or loading shared 64bit values. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105018] Kernel panic when waking up after screen goes blank.
https://bugs.freedesktop.org/show_bug.cgi?id=105018 --- Comment #29 from L.S.S. --- EDIT: Maybe not really fixed in 4.17 (regression again?!)... just now after the screen went blank, I got another panic and had to reboot... :-( When the panic occurred, it spawned two errors. Apr 12 11:32:03 linuxsys systemd[5491]: Started Virtual filesystem service. Apr 12 11:32:03 linuxsys udisksd[1970]: udisks_mount_get_mount_path: assertion 'mount->type == UDISKS_MOUNT_TYPE_FILESYSTEM' failed Apr 12 11:32:13 linuxsys kernel: [drm:dm_vblank_get_counter [amdgpu]] *ERROR* dc_stream_state is NULL for crtc '1'! Apr 12 11:32:13 linuxsys kernel: [drm:dm_crtc_get_scanoutpos [amdgpu]] *ERROR* dc_stream_state is NULL for crtc '1'! Apr 12 11:32:13 linuxsys kernel: [drm:dm_vblank_get_counter [amdgpu]] *ERROR* dc_stream_state is NULL for crtc '1'! Apr 12 11:32:13 linuxsys kernel: WARNING: CPU: 12 PID: 1761 at drivers/gpu/drm/drm_vblank.c:620 drm_calc_vbltimestamp_from_scanoutpos+0x2a8/0x2f0 [drm] Apr 12 11:32:13 linuxsys kernel: Modules linked in: btrfs zstd_compress zstd_decompress xxhash xor raid6_pq ufs hfsplus hfs minix ntfs msdos jfs ext4 mbcache jbd2 fscrypto dm_mod vmw_vsock_vmci_transport vsock cmac rfcomm fuse bnep vmnet(O> Apr 12 11:32:13 linuxsys kernel: agpgart snd_timer syscopyarea rfkill sysfillrect sysimgblt fb_sys_fops aesni_intel snd tpm_crb aes_x86_64 tpm_tis crypto_simd ccp cryptd soundcore tpm_tis_core sp5100_tco glue_helper pcspkr k10temp i2c_pii> Apr 12 11:32:13 linuxsys kernel: CPU: 12 PID: 1761 Comm: xfwm4 Tainted: G O 4.17.0-1-MANJARO #1 Apr 12 11:32:13 linuxsys kernel: Hardware name: ASUSTeK COMPUTER INC. GL702ZC/GL702ZC, BIOS GL702ZC.303 12/15/2017 Apr 12 11:32:13 linuxsys kernel: RIP: 0010:drm_calc_vbltimestamp_from_scanoutpos+0x2a8/0x2f0 [drm] Apr 12 11:32:13 linuxsys kernel: RSP: 0018:a96189f63b28 EFLAGS: 00010082 Apr 12 11:32:13 linuxsys kernel: RAX: c131d9e0 RBX: 9d76fa423000 RCX: Apr 12 11:32:13 linuxsys kernel: RDX: 0001 RSI: c09b98d0 RDI: 0001 Apr 12 11:32:13 linuxsys kernel: RBP: a96189f63b90 R08: R09: c0998ab0 Apr 12 11:32:13 linuxsys kernel: R10: 9d76f74131d8 R11: c114e500 R12: 0001 Apr 12 11:32:13 linuxsys kernel: R13: 9d76f7413000 R14: a96189f63ba4 R15: a96189f63bd8 Apr 12 11:32:13 linuxsys kernel: FS: 7fefa4d35980() GS:9d76fe90() knlGS: Apr 12 11:32:13 linuxsys kernel: CS: 0010 DS: ES: CR0: 80050033 Apr 12 11:32:13 linuxsys kernel: CR2: 7f5f09dfa000 CR3: 0007cc786000 CR4: 003406e0 Apr 12 11:32:13 linuxsys kernel: Call Trace: Apr 12 11:32:13 linuxsys kernel: drm_get_last_vbltimestamp+0x54/0x90 [drm] Apr 12 11:32:13 linuxsys kernel: drm_update_vblank_count+0x79/0x240 [drm] Apr 12 11:32:13 linuxsys kernel: drm_vblank_enable+0xce/0x120 [drm] Apr 12 11:32:13 linuxsys kernel: drm_vblank_get+0x8d/0xb0 [drm] Apr 12 11:32:13 linuxsys kernel: drm_wait_vblank_ioctl+0x12a/0x620 [drm] Apr 12 11:32:13 linuxsys kernel: ? drm_legacy_modeset_ctl_ioctl+0x100/0x100 [drm] Apr 12 11:32:13 linuxsys kernel: drm_ioctl_kernel+0x5b/0xb0 [drm] Apr 12 11:32:13 linuxsys kernel: drm_ioctl+0x2c3/0x360 [drm] Apr 12 11:32:13 linuxsys kernel: ? drm_legacy_modeset_ctl_ioctl+0x100/0x100 [drm] Apr 12 11:32:13 linuxsys kernel: ? do_iter_write+0xdc/0x190 Apr 12 11:32:13 linuxsys kernel: ? vfs_writev+0xb9/0x110 Apr 12 11:32:13 linuxsys kernel: amdgpu_drm_ioctl+0x49/0x80 [amdgpu] Apr 12 11:32:13 linuxsys kernel: do_vfs_ioctl+0xa4/0x630 Apr 12 11:32:13 linuxsys kernel: ? __sys_recvmsg+0x5b/0xa0 Apr 12 11:32:13 linuxsys kernel: ? __sys_recvmsg+0x8a/0xa0 Apr 12 11:32:13 linuxsys kernel: ksys_ioctl+0x70/0x80 Apr 12 11:32:13 linuxsys kernel: SyS_ioctl+0xa/0x10 Apr 12 11:32:13 linuxsys kernel: do_syscall_64+0x74/0x190 Apr 12 11:32:13 linuxsys kernel: entry_SYSCALL_64_after_hwframe+0x3d/0xa2 Apr 12 11:32:13 linuxsys kernel: RIP: 0033:0x7fefa1389d87 Apr 12 11:32:13 linuxsys kernel: RSP: 002b:7ffedb5d0ee8 EFLAGS: 0246 ORIG_RAX: 0010 Apr 12 11:32:13 linuxsys kernel: RAX: ffda RBX: 7ffedb5d0f10 RCX: 7fefa1389d87 Apr 12 11:32:13 linuxsys kernel: RDX: 7ffedb5d0f10 RSI: c018643a RDI: 000c Apr 12 11:32:13 linuxsys kernel: RBP: 00ffad10 R08: 00e00109 R09: Apr 12 11:32:13 linuxsys kernel: R10: R11: 0246 R12: c018643a Apr 12 11:32:13 linuxsys kernel: R13: 00f67cbb R14: 010bf7a0 R15: Apr 12 11:32:13 linuxsys kernel: Code: e9 b5 fd ff ff 44 89 e2 48 c7 c6 d0 98 9b c0 bf 01 00 00 00 e8 fa e9 ff ff 48 8b 83 98 03 00 00 48 83 78 28 00 0f 84 8c fd ff ff <0f> 0b 45 31 ed e9 85 fd ff ff 48 c7 c7 98 98 9b c0 45 31 ed e8 Apr 12 11:32:13 linuxsys kernel: ---[ end trace bc02c50ede9b0814 ]--- Apr 12 11:32:13 linuxsys kernel: [drm:dm_vblank_get_counter [amdgpu]] *
[RFC 0/1] DRM Node Probing functionality
*Resend the whole actual series* This patch implements a simple function for matching DRM device FDs against the desired properties of a device that you are looking for. The properties that are possible to look for are the elements of DrmVersion and DrmDevice. The discussion that led to this series can be found here: https://lists.freedesktop.org/archives/mesa-dev/2018-January/183878.html In the previous discussion we left off having settled on implementing this in mesa/src/loader/loader.c, which I initially did. But the code ended up being so generic that there's no reason for it not to live in libdrm, since it can be used for any compositor and mesa. The implementer will still have to iterate through all of the devices available on the target, and see if they match. This additional functionality could be moved into libdrm at a later point if it turns out that all of the users do this in the same manner. If there is some variety, for example with selecting fallback devices if nothing matches maybe it is best left up to the user of libdrm. The biggest problem with the approach as implemented, the way I see it, is how we match against the DrmVersion/DrmDevice of a given FD. Currently we use DrmVersion & DrmDevice as supplied by the caller to define what we are looking for. This is a little bit problematic, especially for DrmDevice, since all of the elements of the struct do not have enough bitspace to signal that we are uninterested in looking for that specific element. DrmDevice uses drmDevicesEqual() to do what amounts to a memcmp of the DrmDevice argument and the one of the FD. So looking for any device on any PCI bus with just the PCI vendor ID supplied isn't possible. An alternative Daniel Stone suggested is adding enums for different properties and allowing the caller to supply a list of properties that are interesting and their values. In terms of long-term maintainership this might be less pleasant than the approach of the current implementation. Robert Foss (1): xf86drm: Add drmHandleMatch func xf86drm.h | 2 ++ xf86drmMode.c | 69 +++ 2 files changed, 71 insertions(+) -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 1/1] xf86drm: Add drmHandleMatch func
drmHandleMatch is intended to allow for userspace to filter out devices that it does not want to open. Opening specific devices using paths alone is not a reliable due to probing order. This function intends to provide a mechanism for filtering out devices that don't fit what you need using the already existing drmVersion and drmDevice structs. Most fields of drmVersion and drmDevice can be used for comparing between the target device and the actual FD that has been provided. Signed-off-by: Robert Foss --- xf86drm.h | 2 ++ xf86drmMode.c | 69 +++ 2 files changed, 71 insertions(+) diff --git a/xf86drm.h b/xf86drm.h index 7773d71a8030..7ef1e2818301 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -863,6 +863,8 @@ extern int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int max_device extern int drmDevicesEqual(drmDevicePtr a, drmDevicePtr b); +extern int drmHandleMatch(int fd, drmVersionPtr ver, drmDevicePtr dev); + extern int drmSyncobjCreate(int fd, uint32_t flags, uint32_t *handle); extern int drmSyncobjDestroy(int fd, uint32_t handle); extern int drmSyncobjHandleToFD(int fd, uint32_t handle, int *obj_fd); diff --git a/xf86drmMode.c b/xf86drmMode.c index 9a15b5e78dda..e403515a94b7 100644 --- a/xf86drmMode.c +++ b/xf86drmMode.c @@ -946,6 +946,75 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx) return 0; } +#define matchCmpInt(ver, ver_fd, element) \ + if (ver->element > 0 && ver->element != ver_fd->element) \ + goto fail; + +#define matchCmpStr(ver, ver_fd, element) \ + if (ver->element != NULL && !strcmp(ver->element, ver_fd->element)) \ + goto fail; + +static int drmMatchVersion(int fd, drmVersionPtr ver) +{ + drmVersionPtr ver_fd = NULL; + + if (!ver) + goto success; + + ver_fd = drmGetVersion(fd); + if (!ver_fd) + goto fail; + + matchCmpInt(ver, ver_fd, version_major); + matchCmpInt(ver, ver_fd, version_minor); + matchCmpInt(ver, ver_fd, version_patchlevel); + + matchCmpStr(ver, ver_fd, name); + matchCmpStr(ver, ver_fd, date); + matchCmpStr(ver, ver_fd, desc); + +success: + drmFreeVersion(ver_fd); + return 1; +fail: +drmFreeVersion(ver_fd); + return 0; +} + +static int drmMatchDevice(int fd, drmDevicePtr dev) +{ + drmDevicePtr dev_fd = NULL; + + if (!dev) + goto success; + +if (drmGetDevice2(fd, 0, &dev_fd) != 0) { + goto fail; +} + + matchCmpInt(dev, dev_fd, available_nodes) + matchCmpInt(dev, dev_fd, bustype) + + drmDevicesEqual(dev, dev_fd); + +success: + drmFreeDevice(&dev_fd); + return 1; +fail: + drmFreeDevice(&dev_fd); + return 0; +} + +int drmHandleMatch(int fd, drmVersionPtr ver, drmDevicePtr dev) +{ + int ret = 1; + + ret &= drmMatchVersion(fd, ver); + ret &= drmMatchDevice(fd, dev); + + return ret; +} + int drmModePageFlip(int fd, uint32_t crtc_id, uint32_t fb_id, uint32_t flags, void *user_data) { -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98977] Regression on X-Plane on mesa-git
https://bugs.freedesktop.org/show_bug.cgi?id=98977 Timothy Arceri changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #1 from Timothy Arceri --- Was the perf regression fixed in later Mesa releases? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98492] X-Plane 10 Core Dumping when using Real-Weather or any clouds
https://bugs.freedesktop.org/show_bug.cgi?id=98492 Timothy Arceri changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #8 from Timothy Arceri --- *** This bug has been marked as a duplicate of bug 97909 *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 97909] X-Plane 10 crashes with SIGSEGV on radeonsi
https://bugs.freedesktop.org/show_bug.cgi?id=97909 Timothy Arceri changed: What|Removed |Added CC||amarildo-ge...@autistici.or ||g --- Comment #10 from Timothy Arceri --- *** Bug 98492 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105880] [dc][kabini] Cannot find any crtc or sizes
https://bugs.freedesktop.org/show_bug.cgi?id=105880 --- Comment #9 from David Henningsson --- Could add that my graphics card is an RX460: 23:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. [AMD/ATI] Device [1002:67ef] (rev cf) (prog-if 00 [VGA controller]) Subsystem: XFX Pine Group Inc. Device [1682:9460] Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Kernel driver in use: amdgpu Kernel modules: amdgpu -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105880] [dc][kabini] Cannot find any crtc or sizes
https://bugs.freedesktop.org/show_bug.cgi?id=105880 David Henningsson changed: What|Removed |Added CC||di...@ubuntu.com --- Comment #8 from David Henningsson --- Hi, I'm getting the same or similar error: * Black screen, no plymouth * "Cannot find any crtc or sizes" error in dmesg * amdgpu.dc=0 works around the issue ...and I might have a lead on it. The regression is between 4.15rc2 and 4.15rc3 (found by downloading kernels from http://kernel.ubuntu.com/~kernel-ppa/mainline/). The only one commit I could see being relevant between 4.15rc2 and 4.15rc3 is this one: commit a703c55004e1c5076d57e43771b3e7796ea0 Author: Daniel Vetter Date: Mon Dec 4 21:48:18 2017 +0100 drm: safely free connectors from connector_iter ...it seems relevant because the error I get is "Cannot find any crtc or sizes", and the code emitting this error message (in drivers/gpu/drm/drm_fb_helper.c) deals with connectors. So could this commit cause connectors to be freed where the previous code did not? It seems like this could be the case, but only if (obj->free_cb == NULL) - drm_connector_put checks for a obj->free_cb being null whereas drm_connector_put_safe decrements a refcount regardless of whether a free_cb is present or not. Does all this make sense, or am I out on deep water here? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105018] Kernel panic when waking up after screen goes blank.
https://bugs.freedesktop.org/show_bug.cgi?id=105018 --- Comment #28 from L.S.S. --- Just updated to the 4.17-rc0 kernel and it seems the problem has been mostly fixed there. The patches are no longer needed (already in there) and trying to reproduce the issue only resulted in a couple of "Failed to get VBLANK!" errors that aren't fatal. I'm not certain about the flickering issue I mentioned earlier... It looked like one but might actually be some kind of sudden color palette distortion. The problem only appeared since 4.16. I don't recall having the issue in 4.15. I can partially reproduce it in the Firefox new tab page, by quickly hovering over the links on the "Top Sites" and "Highlights". The whole screen would turn a bit darker in color for a very short instant then returns to normal. It happens randomly and it doesn't seem to produce any errors in the log. Not a major issue, just it can be annoying sometimes. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 10/10] drm/i915: Implement proper fallback training for MST
For a while we actually haven't had any way of retraining MST links with fallback link parameters like we do with SST. While uncommon, certain setups such as my Caldigit TS3 + EVGA MST hub require this since otherwise, they end up getting stuck in an infinite MST retraining loop. MST retraining is somewhat different then SST retraining. While it's possible during the normal link retraining sequence for a hub to indicate bad link status, it's also possible for a hub to only indicate this status through ESI messages and it's possible for this to happen after the initial link training succeeds. This can lead to a pattern that looks like this: - Train MST link - Training completes successfully - MST hub sets Channel EQ failed bit in ESI - Retraining starts - Retraining completes successfully - MST hub sets Channel EQ failed bit in ESI again - Rinse and repeat In these situations, we need to be able to actually trigger fallback link training from the ESI handler as well, along with using the ESI handler during retraining to figure out whether or not our retraining actually succeeded. This gets a bit more complicated since we have to ensure that we don't block the ESI handler at all while doing retraining. If we do, due to DisplayPort's general issues with being sensitive to IRQ latency most MST hubs will just stop responding to us if their interrupts aren't handled in a timely manner. So: move retraining into it's own separate handler. Running in a separate handler allows us to avoid stalling the ESI during link retraining, and we can have the ESI signal that the channel EQ bit was cleared through a simple completion struct. Additionally, we take care to stick as much of this into the SST retraining path as possible since sharing is caring. V4: - Move all MST retraining work into hotplug_work - Grab the correct power wells when retraining. - Loop through MST encoders in intel_dp_get_crtc_mask(), quicker/easier than connectors V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ddi.c | 10 +- drivers/gpu/drm/i915/intel_dp.c | 370 -- drivers/gpu/drm/i915/intel_dp_link_training.c | 6 +- drivers/gpu/drm/i915/intel_dp_mst.c | 28 +- drivers/gpu/drm/i915/intel_drv.h | 7 + 5 files changed, 329 insertions(+), 92 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 92cb26b18a9b..7474300cad01 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3042,6 +3042,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder, struct intel_connector *connector) { struct drm_modeset_acquire_ctx ctx; + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); bool changed; int ret; @@ -3063,9 +3064,16 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder, break; } + if (ret == -EINVAL) + ret = intel_dp_handle_train_failure(intel_dp); + drm_modeset_drop_locks(&ctx); drm_modeset_acquire_fini(&ctx); - WARN(ret, "Acquiring modeset locks failed with %i\n", ret); + + if (ret == -EIO) + changed = true; + else + WARN(ret, "Acquiring modeset locks failed with %i\n", ret); return changed; } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index fbb467bc227d..c62d03c69ec3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -45,6 +45,8 @@ #define DP_DPRX_ESI_LEN 14 +#define DP_MST_RETRAIN_TIMEOUT (msecs_to_jiffies(100)) + /* Compliance test status bits */ #define INTEL_DP_RESOLUTION_SHIFT_MASK 0 #define INTEL_DP_RESOLUTION_PREFERRED (1 << INTEL_DP_RESOLUTION_SHIFT_MASK) @@ -2760,6 +2762,7 @@ static void intel_disable_dp(struct intel_encoder *encoder, struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); intel_dp->link_trained = false; + intel_dp->link_is_bad = false; if (old_crtc_state->has_audio) intel_audio_codec_disable(encoder, @@ -4205,8 +4208,134 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Could not write test response to sink\n"); } +/* Get a mask of the CRTCs that are running on the given intel_dp struct. For + * MST, this returns a crtc mask containing all of the CRTCs driving + * downstream sinks, for SST it just returns a mask of the attached + * connector's CRTC. + */ +int +intel_dp_get_crtc_mask(struct intel_dp *intel_dp) +{ + struct drm_device *dev = dp_to_dig_port(intel_dp)->base.base.dev; + struct drm_connector *connector; + struct drm_crtc *crtc; + int crtc_mask = 0; + + WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); + + if (intel_dp-
[PATCH v8 09/10] drm/dp_mst: Add MST fallback retraining helpers
Unlike SST, MST can have far more then a single display hooked up on a single port. What this also means however, is that if the DisplayPort link to the top-level MST branch device becomes unstable then every single branch device also has an unstable link. Additionally, MST has a few more steps that must be taken in order to properly retrain the entire topology under a lower link rate. Since the VCPI allocations for each mstb is determined based off the link rate for the top of the topology, we also have to recalculate all of the VCPI allocations for the downstream ports as well to ensure that we still have enough link bandwidth to drive each mstb. Additionally, since we have multiple downstream connectors per port, setting the link status of the parent mstb's port to bad isn't enough: all of the downstream mstb ports have to have their link status set to bad. This basically follows the same paradigm that our DP link status logic in DRM does, in that we simply tell userspace that all of the mstb ports need retraining and additionally applying the new lower bandwidth constraints to all of the atomic commits on the topology that follow. Additionally; we add helpers that handle automatically checking whether or not a new atomic commit would perform the modesets required to retrain a link and if so, additionally handles updating the link status of each connector on the MST topology. V4: - clarify slightly confusing message in drm_dp_mst_topology_mgr_lower_link_rate() - Fix argument naming - Squash this with the other retrain helper, because now they're intertwined with one another - Track which connectors with CRTCs need modesets in order to retrain a topology in the topology's atomic state. This lets us greatly simplify the helpers, along with alleviating drivers of the responsibility of figuring out when to call the retrain helpers during atomic checks. It also ensures that we can keep zombie connectors that a retrain is dependent on alive until the topology either disappears, or they are disabled. We needed to do most of this anyway, since our original helpers didn't take into account that we need to invoke retraining when the link status prop changes, regardless of whether or not a modeset has been initiated yet. - Handle situation we completely forgot about: adding new connectors to an MST topology that needs fallback retraining (solution: new connectors on a topology inherit the link status of the rest of the topology) - Also make sure to handle connectors that are orphaned due to their MST topology disappearing. Solution: remove from topology state, reset link status to good - Write more docs on the retraining procedure. V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/drm_dp_mst_topology.c | 440 +- include/drm/drm_dp_mst_helper.h | 20 ++ 2 files changed, 455 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 981bd0f7d3ab..cc4b737a47b0 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1199,6 +1199,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, if (created && !port->input) { char proppath[255]; + struct drm_dp_mst_topology_state *state; build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath)); port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr, port, proppath); @@ -1217,6 +1218,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_mode_connector_set_tile_property(port->connector); } + state = to_dp_mst_topology_state(port->mgr->base.state); + if (!list_empty(&state->bad_ports)) { + port->connector->state->link_status = + DRM_LINK_STATUS_BAD; + } (*mstb->mgr->cbs->register_connector)(port->connector); } @@ -2076,7 +2082,7 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, { switch (dp_link_bw) { default: - DRM_DEBUG_KMS("invalid link bandwidth in DPCD: %x (link count: %d)\n", + DRM_DEBUG_KMS("invalid link bandwidth: %x (link count: %d)\n", dp_link_bw, dp_link_count); return false; @@ -2096,10 +2102,409 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, return true; } +static int drm_dp_set_mstb_link_status(struct drm_dp_mst_topology_mgr *mgr, + struct drm_dp_mst_branch *mstb, + enum drm_link_status status) +{ + struct drm_dp_mst_topology_state *st
[PATCH v8 05/10] drm/dp_mst: Make drm_dp_mst_topology_state subclassable
This is useful for drivers (which will probably be all of them soon) which need to track state that is exclusive to the topology, and not a specific connector on said topology. This includes things such as the link rate and lane count that are shared by all of the connectors on the topology. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä V7: - Fix CHECKPATCH errors --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +++- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 46 --- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.h| 4 +- drivers/gpu/drm/drm_dp_mst_topology.c | 95 +- drivers/gpu/drm/i915/intel_dp_mst.c| 13 ++- drivers/gpu/drm/nouveau/nv50_display.c | 17 +++- drivers/gpu/drm/radeon/radeon_dp_mst.c | 13 ++- include/drm/drm_dp_mst_helper.h| 8 ++ 8 files changed, 173 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index e42a28e3adc5..2c3660c36732 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3626,9 +3626,17 @@ static int amdgpu_dm_connector_init(struct amdgpu_display_manager *dm, drm_connector_register(&aconnector->base); - if (connector_type == DRM_MODE_CONNECTOR_DisplayPort - || connector_type == DRM_MODE_CONNECTOR_eDP) - amdgpu_dm_initialize_dp_connector(dm, aconnector); + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || + connector_type == DRM_MODE_CONNECTOR_eDP) { + res = amdgpu_dm_initialize_dp_connector(dm, aconnector); + if (res) { + drm_connector_unregister(&aconnector->base); + drm_connector_cleanup(&aconnector->base); + aconnector->connector_id = -1; + + goto out_free; + } + } #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) ||\ defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 8291d74f26bc..dcaa92d12cbc 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -475,22 +475,48 @@ static const struct drm_dp_mst_topology_cbs dm_mst_cbs = { .register_connector = dm_dp_mst_register_connector }; -void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, - struct amdgpu_dm_connector *aconnector) +static const struct drm_private_state_funcs dm_mst_state_funcs = { + .atomic_duplicate_state = drm_atomic_dp_mst_duplicate_topology_state, + .atomic_destroy_state = drm_atomic_dp_mst_destroy_topology_state, +}; + +int amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, + struct amdgpu_dm_connector *aconnector) { + struct drm_dp_mst_topology_state *state = + kzalloc(sizeof(*state), GFP_KERNEL); + int ret = 0; + + if (!state) + return -ENOMEM; + aconnector->dm_dp_aux.aux.name = "dmdc"; aconnector->dm_dp_aux.aux.dev = dm->adev->dev; aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; - drm_dp_aux_register(&aconnector->dm_dp_aux.aux); + ret = drm_dp_aux_register(&aconnector->dm_dp_aux.aux); + if (ret) + goto err_aux; + aconnector->mst_mgr.cbs = &dm_mst_cbs; - drm_dp_mst_topology_mgr_init( - &aconnector->mst_mgr, - dm->adev->ddev, - &aconnector->dm_dp_aux.aux, - 16, - 4, - aconnector->connector_id); + aconnector->mst_mgr.funcs = &dm_mst_state_funcs; + ret = drm_dp_mst_topology_mgr_init(&aconnector->mst_mgr, + state, + dm->adev->ddev, + &aconnector->dm_dp_aux.aux, + 16, + 4, + aconnector->connector_id); + if (ret) + goto err_mst; + + return 0; + +err_mst: + drm_dp_aux_unregister(&aconnector->dm_dp_aux.aux); +err_aux: + kfree(state); + return ret; } diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h index 8cf51da26657..d28fb456d2d5 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h @@ -29,8 +29,8 @@
[PATCH v8 06/10] drm/dp_mst: Add reset_state callback to topology mgr
This gives drivers subclassing drm_dp_mst_topology_state the ability to prepare their topology states for a new hub to be connected whenever it's detected that the currently connected topology has been disconnected from the system. We'll need this in order to correctly track RX capabilities in i915 from the topology's atomic state. Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul V7: - Fix CHECKPATCH errors --- drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++ include/drm/drm_dp_mst_helper.h | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index fbd7888ebca8..981bd0f7d3ab 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2096,6 +2096,15 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, return true; } +static void drm_dp_mst_topology_reset_state(struct drm_dp_mst_topology_mgr *mgr) +{ + struct drm_dp_mst_topology_state *state = + to_dp_mst_topology_state(mgr->base.state); + + if (mgr->cbs->reset_state) + mgr->cbs->reset_state(state); +} + /** * drm_dp_mst_topology_mgr_set_mst() - Set the MST state for a topology manager * @mgr: manager to set state for @@ -2171,6 +2180,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms mgr->payload_mask = 0; set_bit(0, &mgr->payload_mask); mgr->vcpi_mask = 0; + drm_dp_mst_topology_reset_state(mgr); } out_unlock: diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index ad1aaec8d514..3a7378cd5bd1 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -381,6 +381,7 @@ struct drm_dp_sideband_msg_tx { /* sideband msg handler */ struct drm_dp_mst_topology_mgr; +struct drm_dp_mst_topology_state; struct drm_dp_mst_topology_cbs { /* create a connector for a port */ struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path); @@ -388,7 +389,7 @@ struct drm_dp_mst_topology_cbs { void (*destroy_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_connector *connector); void (*hotplug)(struct drm_dp_mst_topology_mgr *mgr); - + void (*reset_state)(struct drm_dp_mst_topology_state *state); }; #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8) -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 04/10] drm/dp_mst: Remove all evil duplicate state pointers
There's no reason to track the atomic state three times. Unfortunately, this is currently what we're doing, and even worse is that there is only one actually correct state pointer: the one in mst_state->base.state. mgr->state never seems to be used, along with the one in mst_state->state. This confused me for over 4 hours until I realized there was no magic behind these pointers. So, let's save everyone else from the trouble. Signed-off-by: Lyude Paul . Cc: Manasi Navare Cc: Ville Syrjälä --- include/drm/drm_dp_mst_helper.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 2f6203407fd2..5ca77dcf4f90 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -409,7 +409,6 @@ struct drm_dp_payload { struct drm_dp_mst_topology_state { struct drm_private_state base; int avail_slots; - struct drm_atomic_state *state; struct drm_dp_mst_topology_mgr *mgr; }; @@ -497,11 +496,6 @@ struct drm_dp_mst_topology_mgr { */ int pbn_div; - /** -* @state: State information for topology manager -*/ - struct drm_dp_mst_topology_state *state; - /** * @funcs: Atomic helper callbacks */ -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 07/10] drm/i915: Only use one link bw config for MST topologies
When a DP MST link needs retraining, sometimes the hub will detect that the current link bw config is impossible and will update it's RX caps in the DPCD to reflect the new maximum link rate. Currently, we make the assumption that the RX caps in the dpcd will never change like this. This means if the sink changes it's RX caps after we've already set up an MST link and we attempt to add or remove another sink from the topology, we could put ourselves into an invalid state where we've tried to configure different sinks on the same MST topology with different link rates. We could also run into this situation if a sink reports a higher link rate after suspend, usually from us having trained it with a fallback bw configuration before suspending. So: keep the link rate consistent by subclassing drm_dp_mst_topology_state, and tracking it there. For the time being, we only allow the link rate to change when the entire topology has been disconnected. V4: - Track link rate/lane count in the atomic topology state instead of in intel_dp. V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp_mst.c | 79 + drivers/gpu/drm/i915/intel_drv.h| 7 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index cf844cfd2bb0..19de0b5a7a40 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -41,8 +41,11 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_atomic_state *state = pipe_config->base.state; + struct drm_dp_mst_topology_state *mst_state = + drm_atomic_dp_mst_get_topology_state(state, &intel_dp->mst_mgr); + struct intel_dp_mst_topology_state *intel_mst_state; int bpp; - int lane_count, slots; + int slots; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc, @@ -55,18 +58,22 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, DRM_DEBUG_KMS("Setting pipe bpp to %d\n", bpp); } + + intel_mst_state = + to_intel_dp_mst_topology_state(mst_state); /* * for MST we always configure max link bw - the spec doesn't * seem to suggest we should do otherwise. */ - lane_count = intel_dp_max_lane_count(intel_dp); - - pipe_config->lane_count = lane_count; + if (!intel_mst_state->link_rate || !intel_mst_state->lane_count) { + intel_mst_state->link_rate = intel_dp_max_link_rate(intel_dp); + intel_mst_state->lane_count = intel_dp_max_lane_count(intel_dp); + } + pipe_config->lane_count = intel_mst_state->lane_count; + pipe_config->port_clock = intel_mst_state->link_rate; pipe_config->pipe_bpp = bpp; - pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); - if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port)) pipe_config->has_audio = true; @@ -80,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, return false; } - intel_link_compute_m_n(bpp, lane_count, + intel_link_compute_m_n(bpp, intel_mst_state->lane_count, adjusted_mode->crtc_clock, pipe_config->port_clock, &pipe_config->dp_m_n, @@ -530,11 +537,55 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr) drm_kms_helper_hotplug_event(dev); } +static void intel_mst_reset_state(struct drm_dp_mst_topology_state *state) +{ + struct intel_dp_mst_topology_state *intel_mst_state = + to_intel_dp_mst_topology_state(state); + + intel_mst_state->link_rate = 0; + intel_mst_state->lane_count = 0; +} + static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = intel_dp_add_mst_connector, .register_connector = intel_dp_register_mst_connector, .destroy_connector = intel_dp_destroy_mst_connector, .hotplug = intel_dp_mst_hotplug, + .reset_state = intel_mst_reset_state, +}; + +static struct drm_private_state * +intel_dp_mst_duplicate_state(struct drm_private_obj *obj) +{ + struct intel_dp_mst_topology_state *state; + struct drm_dp_mst_topology_mgr *mgr = to_dp_mst_topology_mgr(obj); + + state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_dp_mst_duplicate_topology_state(mgr, &state->base); + + return &state->base.base; +} + +static vo
[PATCH v8 01/10] drm/atomic: Print debug message on atomic check failure
Does what it says on the label, it's a little confusing debugging atomic check failures otherwise. Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_atomic.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 7d25c42f22db..972a7e9634ab 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1705,8 +1705,11 @@ int drm_atomic_check_only(struct drm_atomic_state *state) if (config->funcs->atomic_check) ret = config->funcs->atomic_check(state->dev, state); - if (ret) + if (ret) { + DRM_DEBUG_ATOMIC("atomic driver check for %p failed: %d\n", +state, ret); return ret; + } if (!state->allow_modeset) { for_each_new_crtc_in_state(state, crtc, crtc_state, i) { -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 08/10] drm/i915: Make intel_dp_mst_atomic_check() idempotent
The current way of handling changing VCPI allocations doesn't make a whole ton of sense. Since drm_atomic_helper_check_modeset() can be called multiple times which means that intel_dp_mst_atomic_check() can also be called multiple times. However, since we make the silly mistake of trying to free VCPI slots based off the /new/ atomic state instead of the old atomic state, we'll end up potentially double freeing the vcpi slots for the ports. This also has another unintended consequence that came back up while implementing MST fallback retraining: if a modeset is forced on a connector that's already part of the state, but it's atomic_check() has already been run once and doesn't get run again, we'll end up not freeing the VCPI allocations on the connector at all. The easiest way to solve this is to be clever and, with the exception of connectors that are being disabled and thus will never have compute_config() ran on them, move vcpi freeing out of the atomic check and into compute_config(). Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul V7: - Fix CHECKPATCH errors --- drivers/gpu/drm/i915/intel_dp_mst.c | 84 +++-- 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 19de0b5a7a40..31b37722cd89 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -30,6 +30,38 @@ #include #include +static int +intel_dp_mst_atomic_release_vcpi_slots(struct drm_atomic_state *state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_crtc_state *new_crtc_state; + struct intel_crtc_state *intel_crtc_state = + to_intel_crtc_state(crtc_state); + struct drm_encoder *encoder; + struct drm_dp_mst_topology_mgr *mgr; + int slots, ret; + + slots = intel_crtc_state->dp_m_n.tu; + if (slots <= 0) + return 0; + + encoder = conn_state->best_encoder; + mgr = &enc_to_mst(encoder)->primary->dp.mst_mgr; + + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); + if (ret) { + DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", + slots, ret); + } else { + new_crtc_state = drm_atomic_get_crtc_state(state, + crtc_state->crtc); + to_intel_crtc_state(new_crtc_state)->dp_m_n.tu = 0; + } + + return ret; +} + static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -44,10 +76,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct drm_dp_mst_topology_state *mst_state = drm_atomic_dp_mst_get_topology_state(state, &intel_dp->mst_mgr); struct intel_dp_mst_topology_state *intel_mst_state; + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(state, &connector->base); + struct drm_crtc *old_crtc; int bpp; int slots; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; + int ret; bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_LIMITED_M_N); @@ -80,6 +116,21 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); pipe_config->pbn = mst_pbn; + /* Free any VCPI allocations on this connector from the previous +* state +*/ + old_crtc = old_conn_state->crtc; + if (old_crtc) { + struct drm_crtc_state *old_crtc_state = + drm_atomic_get_old_crtc_state(state, old_crtc); + + ret = intel_dp_mst_atomic_release_vcpi_slots(state, +old_crtc_state, +old_conn_state); + if (ret) + return ret; + } + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, connector->port, mst_pbn); if (slots < 0) { @@ -109,31 +160,22 @@ static int intel_dp_mst_atomic_check(struct drm_connector *connector, { struct drm_atomic_state *state = new_conn_state->state; struct drm_connector_state *old_conn_state; - struct drm_crtc *old_crtc; - struct drm_crtc_state *crtc_state; - int slots, ret = 0; + struct drm_crtc_state *new_crtc_state; + int ret = 0; old_c
[PATCH v8 00/10] drm/i915: Implement proper fallback training for MST
Next version of https://patchwork.freedesktop.org/series/41576/ Only changes are removing duplicate SoBs that git send-email annoyingly added. Sorry about that :( Lyude Paul (10): drm/atomic: Print debug message on atomic check failure drm/i915: Move DP modeset retry work into intel_dp drm/dp_mst: Fix naming on drm_atomic_get_mst_topology_state() drm/dp_mst: Remove all evil duplicate state pointers drm/dp_mst: Make drm_dp_mst_topology_state subclassable drm/dp_mst: Add reset_state callback to topology mgr drm/i915: Only use one link bw config for MST topologies drm/i915: Make intel_dp_mst_atomic_check() idempotent drm/dp_mst: Add MST fallback retraining helpers drm/i915: Implement proper fallback training for MST drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 46 +- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.h| 4 +- drivers/gpu/drm/drm_atomic.c | 5 +- drivers/gpu/drm/drm_dp_mst_topology.c | 550 - drivers/gpu/drm/i915/intel_ddi.c | 10 +- drivers/gpu/drm/i915/intel_display.c | 14 +- drivers/gpu/drm/i915/intel_dp.c| 376 ++ drivers/gpu/drm/i915/intel_dp_link_training.c | 6 +- drivers/gpu/drm/i915/intel_dp_mst.c| 186 +-- drivers/gpu/drm/i915/intel_drv.h | 20 +- drivers/gpu/drm/nouveau/nv50_display.c | 17 +- drivers/gpu/drm/radeon/radeon_dp_mst.c | 13 +- include/drm/drm_dp_mst_helper.h| 43 +- 14 files changed, 1120 insertions(+), 184 deletions(-) -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 02/10] drm/i915: Move DP modeset retry work into intel_dp
While having the modeset_retry_work in intel_connector makes sense with SST, this paradigm doesn't make a whole ton of sense when it comes to MST since we have to deal with multiple connectors. In most cases, it's more useful to just use the intel_dp struct since it indicates whether or not we're dealing with an MST device, along with being able to easily trace the intel_dp struct back to it's respective connector (if there is any). So, move the modeset_retry_work function out of the intel_connector struct and into intel_dp. Signed-off-by: Lyude Paul Reviewed-by: Manasi Navare Cc: Manasi Navare Cc: Ville Syrjälä V2: - Remove accidental duplicate modeset_retry_work in intel_connector V3: - Also check against eDP in intel_hpd_poll_fini() - mdnavare V4: - Don't bother looping over connectors for canceling modeset rety work, just encoders. V7: - Fix CHECKPATCH errors --- drivers/gpu/drm/i915/intel_display.c | 14 +++--- drivers/gpu/drm/i915/intel_dp.c | 10 -- drivers/gpu/drm/i915/intel_dp_link_training.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 6 +++--- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e04050ea3e28..18edb9628a54 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15471,20 +15471,28 @@ void intel_connector_unregister(struct drm_connector *connector) static void intel_hpd_poll_fini(struct drm_device *dev) { - struct intel_connector *connector; struct drm_connector_list_iter conn_iter; + struct intel_connector *connector; + struct intel_encoder *encoder; + struct intel_dp *intel_dp; /* Kill all the work that may have been queued by hpd. */ drm_connector_list_iter_begin(dev, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { - if (connector->modeset_retry_work.func) - cancel_work_sync(&connector->modeset_retry_work); if (connector->hdcp_shim) { cancel_delayed_work_sync(&connector->hdcp_check_work); cancel_work_sync(&connector->hdcp_prop_work); } } drm_connector_list_iter_end(&conn_iter); + + for_each_intel_encoder(dev, encoder) { + if (encoder->type == INTEL_OUTPUT_DP || + encoder->type == INTEL_OUTPUT_EDP) { + intel_dp = enc_to_intel_dp(&encoder->base); + cancel_work_sync(&intel_dp->modeset_retry_work); + } + } } void intel_modeset_cleanup(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 62f82c4298ac..fbb467bc227d 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6249,12 +6249,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, static void intel_dp_modeset_retry_work_fn(struct work_struct *work) { - struct intel_connector *intel_connector; - struct drm_connector *connector; + struct intel_dp *intel_dp = container_of(work, typeof(*intel_dp), +modeset_retry_work); + struct drm_connector *connector = &intel_dp->attached_connector->base; - intel_connector = container_of(work, typeof(*intel_connector), - modeset_retry_work); - connector = &intel_connector->base; DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); @@ -6283,7 +6281,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, int type; /* Initialize the work for modeset in case of link train failure */ - INIT_WORK(&intel_connector->modeset_retry_work, + INIT_WORK(&intel_dp->modeset_retry_work, intel_dp_modeset_retry_work_fn); if (WARN(intel_dig_port->max_lanes < 1, diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index f59b59bb0a21..2cfa58ce1f95 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -340,7 +340,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) intel_dp->link_rate, intel_dp->lane_count)) /* Schedule a Hotplug Uevent to userspace to start modeset */ - schedule_work(&intel_connector->modeset_retry_work); + schedule_work(&intel_dp->modeset_retry_work); } else { DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", intel_connector->base.base.id, d
[PATCH v8 03/10] drm/dp_mst: Fix naming on drm_atomic_get_mst_topology_state()
Since these functions are dealing with an atomic state that needs to be modified during atomic check and commit, change the naming on this function to drm_atomic_dp_mst_get_topology_state() since we're the only one using the function, and it's more consistent with the naming scheme used in drm_atomic.c/drm_atomic_helper.c. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä V7: - Fix CHECKPATCH errors --- drivers/gpu/drm/drm_dp_mst_topology.c | 13 +++-- include/drm/drm_dp_mst_helper.h | 6 -- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 6fac4129e6a2..ba67f1782a04 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2622,7 +2622,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_state *topology_state; int req_slots; - topology_state = drm_atomic_get_mst_topology_state(state, mgr); + topology_state = drm_atomic_dp_mst_get_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); @@ -2662,7 +2662,7 @@ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, { struct drm_dp_mst_topology_state *topology_state; - topology_state = drm_atomic_get_mst_topology_state(state, mgr); + topology_state = drm_atomic_dp_mst_get_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); @@ -3129,7 +3129,7 @@ static const struct drm_private_state_funcs mst_state_funcs = { }; /** - * drm_atomic_get_mst_topology_state: get MST topology state + * drm_atomic_dp_mst_get_topology_state: get MST topology state * * @state: global atomic state * @mgr: MST topology manager, also the private object in this case @@ -3143,15 +3143,16 @@ static const struct drm_private_state_funcs mst_state_funcs = { * * The MST topology state or error pointer. */ -struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr) +struct drm_dp_mst_topology_state * +drm_atomic_dp_mst_get_topology_state(struct drm_atomic_state *state, +struct drm_dp_mst_topology_mgr *mgr) { struct drm_device *dev = mgr->dev; WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state, &mgr->base)); } -EXPORT_SYMBOL(drm_atomic_get_mst_topology_state); +EXPORT_SYMBOL(drm_atomic_dp_mst_get_topology_state); /** * drm_dp_mst_topology_mgr_init - initialise a topology manager diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 7f78d26a0766..2f6203407fd2 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -623,8 +623,10 @@ void drm_dp_mst_dump_topology(struct seq_file *m, void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr); int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr); -struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr); + +struct drm_dp_mst_topology_state * +drm_atomic_dp_mst_get_topology_state(struct drm_atomic_state *state, +struct drm_dp_mst_topology_mgr *mgr); int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn); -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [VERY RFC 0/2] iommu: optionally skip attaching to a DMA domain
On Wed, Apr 11, 2018 at 2:55 PM, Jordan Crouse wrote: > I've been struggling with a problem for a while and I haven't been able to > come > up with a clean solution. Rob convinced me to stop complaining and do > _something_ and hopefully this can spur a good discussion. > > The scenario is basically this: The MSM GPU wants to manage its own IOMMU > virtual address space in lieu of using the DMA API to do so. The most > important > reason is that when per-instance pagetables [1] are enabled each file > descriptor > uses its own pagetable which are dynamically switched between contexts. In > conjunction with this we use a split pagetable scheme to allow global buffers > be persistently mapped so even a single pagetable has multiple ranges that > need > to be utilized based on buffer type. > > Obviously the DMA API isn't suitable for this kind of specialized use. We want > to push it off to the side, allocate our own domain, and use it directly with > the IOMMU APIs. In a perfect world we would just not use the DMA API and > attach > our own domain and that would be the end of the story. Unfortunately this is > not > a perfect world. > > In order to switch the pagetable from the GPU the SoC has some special wiring > so > that the GPU can reprogram the TTBR0 register in the IOMMU to the appropriate > value. For a variety of reasons the hardware is hardcoded to only be able to > access context bank 0 in the SMMU device. Long story short; if the DMA > domain is allocated during bus enumeration and attached to context bank 0 then > by the time we get to the driver domain we have long since lost our chance to > get the context bank we need. > > After going back and forth and trying a few possible options it seems like the > "easiest" solution" is to skip attaching the DMA domain in the first place. > But > we still need to create a default domain and set up the IOMMU groups correctly > so that when the GPU driver comes along it can attach the device and > everything > will Just Work (TM). Rob suggested that we should use a blacklist of devices > that choose to not participate so thats what I'm offering as a starting point > for discussion. in an ideal world, the dma-mapping layer magic would not be forced on a driver, but would be an opt-in thing (ie. driver's ->probe() calls dma_setup_magic_iommu_stuff(dev)), and becomes more of a helper layer that drivers could opt-in to.. but that changes how things work significantly at iommu probe time and probably touches a lot of drivers. A property in dt would be the easy down-stream solution, but this is mostly about the driver (but also about how the firmware works, so maybe dt is not out of bounds?). The black-list approach is fairly simple, although maybe a bit of a point-solution. (Although the number of drivers this problem applies to might not be much greater than 1.) So short version, very interested if anyone has better suggestions. Because we really need to do *something* here. BR, -R > The first patch in this series allows the specific IOMMU driver to gracefully > skip attaching a device by returning -ENOSUPP. In that case, the core will > skip printing error messages and it won't attach the domain to the group but > it still allows the group to get created so that the IOMMU device is properly > set up. In arch_setup_dma_ops() the call to iommu_get_domain_for_dev() will > return NULL and the IOMMU ops won't be set up. > > The second patch implements the blacklist in arm-smmu.c based on the > compatible > string of the GPU device and returns -ENOTSUPP. > > I tested this with my current per-instance pagetable stack and it does the job > but I am very much in the market for better ideas. > > [1] https://patchwork.freedesktop.org/series/38729/ > > Jordan Crouse (2): > iommu: Gracefully allow drivers to not attach to a default domain > iommu/arm-smmu: Add list of devices to opt out of DMA domains > > drivers/iommu/arm-smmu.c | 23 +++ > drivers/iommu/iommu.c| 18 ++ > 2 files changed, 37 insertions(+), 4 deletions(-) > > -- > 2.16.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: RFC for a render API to support adaptive sync and VRR
> From: Michel Dänzer [mailto:mic...@daenzer.net] > Sent: Wednesday, April 11, 2018 05:50 > On 2018-04-11 08:57 AM, Nicolai Hähnle wrote: > > On 10.04.2018 23:45, Cyr, Aric wrote: > >> How does it work fine today given that all kernel seems to know is > >> 'current' or 'current+1' vsyncs. > >> Presumably the applications somehow schedule all this just fine. > >> If this works without variable refresh for 60Hz, will it not work for > >> a fixed-rate "48Hz" monitor (assuming a 24Hz video)? > > > > You're right. I guess a better way to state the point is that it > > *doesn't* really work today with fixed refresh, but if we're going to > > introduce a new API, then why not do so in a way that can fix these > > additional problems as well? > > Exactly. With a fixed frame duration, we'll still have fundamentally the > same issues as we currently do without variable refresh, not making use > of the full potential of variable refresh. I see. Well then, that's makes this sort of orthogonal to the discussion. If you say that there are no media players on Linux today that can maintain audio/video sync with a 60Hz display, then that problem is much larger than the one we're trying to solve here. By the way, I don't believe that is a true statement :) > > Say you have a multi-GPU system, and each GPU has multiple displays > > attached, and a single application is driving them all. The application > > queues flips for all displays with the same target_present_time_ns > > attribute. Starting at some time T, the application simply asks for the > > same present time T + i * 1667 (or whatever) for frame i from all > > displays. [snip] > > Why would that not work to sync up all displays almost perfectly? > > Seconded. It doesn't work that way unfortunately. In theory, sounds great, but if you ask anyone who's worked with framelock/genlock, it is a complicated problem. Easiest explaination is that you need to be able to atomically program registers across multiple GPUs at the same time, this is not possible without hardware assist (see AMD's S400 module for example). We have enough to discuss without this, so let's leave mGPU for another day since we can't solve it here anyways. > > Okay, that's interesting. Does this mean that the display driver still > > programs a refresh rate to some hardware register? Yes, driver can, in some cases, update the minimum and maximum vertical total each flip. In fixed rated example, you would set them equal to achieve your desired refresh rate. We don't program refresh rate, just the vertical total min/max (which translates to affecting refresh rate, given constant pixel clock and horizontal total). Thus, our refresh rate granularity is one line-time, on the order of microsec accuracy. > > How about what I wrote in an earlier mail of having attributes: > > > > - target_present_time_ns > > - hint_frame_time_ns (optional) > > > > ... and if a video player set both, the driver could still do the > > optimizations you've explained? > > FWIW, I don't think a property would be a good mechanism for the target > presentation time. > > At least with VDPAU, video players are already explicitly specifying the > target presentation time, so no changes should be required at that > level. Don't know about other video APIs. > > The X11 Present extension protocol is also prepared for specifying the > target presentation time already, the support for it just needs to be > implemented. I'm perfectly OK with presentation time-based *API*. I get it from a user mode/app perspective, and that's fine. We need that feedback and would like help defining that portions of the stack. However, I think it doesn't make as much sense as a *DDI* because it doesn't correspond to any hardware real or logical (i.e. no one would implement it in HW this way) and the industry specs aren't defined that way. You can have libdrm or some other usermode component translate your presentation time into a frame duration and schedule it. What's the advantage of having this in kernel besides the fact we lose the intent of the application and could prevent features and optimizations. When it gets to kernel, I think it is much more elegant for the flip structure to contain a simple duration that says "hey, show this frame on the screen for this long". Then we don't need any clocks or timers just some simple math and program the hardware. In short, 1) We can simplify media players' lives by helping them get really, really close to their content rate, so they wouldn't need any frame rate conversion. They'll still need A/V syncing though, and variable refresh cannot solve this and thus is way out of scope of what we're proposing. 2) For gaming, don't even try to guess a frame duration, the driver/hardware will do a better job every time, just specify duration=0 and flip as fast as you can. Regards, Aric P.S. Thanks for the Croteam link. Interesting, but basically nullifie
Re: [linux-sunxi] Re: [PATCH 2/3] ARM: dts: sun7i: Add RGB666 pins definition
Hi, Il 12/04/2018 01:09, Paul Kocialkowski ha scritto: Hi, Le jeudi 12 avril 2018 à 00:22 +0200, Giulio Benetti a écrit : Hi, Il 10/04/2018 23:31, Paul Kocialkowski ha scritto: This adds the pins definition for RGB666 LCD panels on the A20. It was imported from the A33 definition, that concernes the same set of pins. Signed-off-by: Paul Kocialkowski --- arch/arm/boot/dts/sun7i-a20.dtsi | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index e529e4ff2174..f46af8675cfa 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -781,6 +781,14 @@ function = "ir1"; }; + lcd_rgb666_pins: lcd_rgb666@0 { I point you to this Thread and back: https://lists.freedesktop.org/archives/dri-devel/2018-March/170688.htm l I did the same for rgb888, so good part is discussed. IMHO I would: - call lcd0_rgb666_pins, since this is for LCD0 interface - same as above, call lcd0-rgb666, take care about using "-" instad of "_" that can cause DTC warnings. - remove @0 since only this set can achieve LCD0 RGB666, and I don't think there will be other combinations. I even responded to that discussion but overlooked these aspects when crafting this patch. Thanks for the summary :) You're welcome, I've forgotten you've been involved discussing that patch so i've pointed you something yours :) Anyway it should help. Best regards -- Giulio Benetti CTO MICRONOVA SRL Sede: Via A. Niedda 3 - 35010 Vigonza (PD) Tel. 049/8931563 - Fax 049/8931346 Cod.Fiscale - P.IVA 02663420285 Capitale Sociale € 26.000 i.v. Iscritta al Reg. Imprese di Padova N. 02663420285 Numero R.E.A. 258642 Kind regards -- Giulio Benetti CTO MICRONOVA SRL Sede: Via A. Niedda 3 - 35010 Vigonza (PD) Tel. 049/8931563 - Fax 049/8931346 Cod.Fiscale - P.IVA 02663420285 Capitale Sociale € 26.000 i.v. Iscritta al Reg. Imprese di Padova N. 02663420285 Numero R.E.A. 258642 + pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7", + "PD10", "PD11", "PD12", "PD13", "PD14", "PD15", + "PD18", "PD19", "PD20", "PD21", "PD22", "PD23", + "PD24", "PD25", "PD26", "PD27"; + function = "lcd0"; + }; + mmc0_pins_a: mmc0@0 { pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5"; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN92
Hi, Le mercredi 11 avril 2018 à 08:28 +0200, Maxime Ripard a écrit : > On Tue, Apr 10, 2018 at 11:31:27PM +0200, Paul Kocialkowski wrote: > > This adds timings for the RGB666 variant of the Innolux AT070TN92 > > panel, > > as found on the Ainol AW1 tablet. > > > > Signed-off-by: Paul Kocialkowski > > --- > > drivers/gpu/drm/panel/panel-simple.c | 26 > > ++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index 5591984a392b..efeb2f2162bc 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -1063,6 +1063,29 @@ static const struct panel_desc > > innolux_at070tn92 = { > > .bus_format = MEDIA_BUS_FMT_RGB888_1X24, > > }; > > > > +static const struct drm_display_mode innolux_at070tn92_rgb666_mode > > = { > > + .clock = 4, > > + .hdisplay = 800, > > + .hsync_start = 800 + 112, > > + .hsync_end = 800 + 112 + 1, > > + .htotal = 800 + 112 + 1 + 87, > > + .vdisplay = 480, > > + .vsync_start = 480 + 141, > > + .vsync_end = 480 + 141 + 1, > > + .vtotal = 480 + 141 + 1 + 38, > > + .vrefresh = 60, > > +}; > > I'm not sure why you'd need different timings if the only difference > is the bus width. I'll try with the other timings, although it might be desirable to have a higher clock speed thanks to the reduced number of wires on the bus. > > +static const struct panel_desc innolux_at070tn92_rgb666 = { > > + .modes = &innolux_at070tn92_rgb666_mode, > > + .num_modes = 1, > > + .size = { > > + .width = 154, > > + .height = 86, > > + }, > > + .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > > +}; > > + > > static const struct display_timing innolux_g101ice_l01_timing = { > > .pixelclock = { 6040, 7110, 7470 }, > > .hactive = { 1280, 1280, 1280 }, > > @@ -2105,6 +2128,9 @@ static const struct of_device_id > > platform_of_match[] = { > > }, { > > .compatible = "innolux,at070tn92", > > .data = &innolux_at070tn92, > > + }, { > > + .compatible = "innolux,at070tn92-rgb666", > > + .data = &innolux_at070tn92_rgb666, > > And this isn't a different device, it's just the integration in the > board that is different, so you shouldn't have a different compatible > here, but rather something like the bus-width property documented in > the Documentation/devicetree/bindings/media/video-interfaces.txt file. I'll take a look at it and try to craft v2 in this direction then. Thanks for the review! -- Paul Kocialkowski, developer of free digital technology and hardware support. Website: https://www.paulk.fr/ Coding blog: https://code.paulk.fr/ Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/ signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: [PATCH 2/3] ARM: dts: sun7i: Add RGB666 pins definition
Hi, Le jeudi 12 avril 2018 à 00:22 +0200, Giulio Benetti a écrit : > Hi, > > Il 10/04/2018 23:31, Paul Kocialkowski ha scritto: > > This adds the pins definition for RGB666 LCD panels on the A20. It > > was > > imported from the A33 definition, that concernes the same set of > > pins. > > > > Signed-off-by: Paul Kocialkowski > > --- > > arch/arm/boot/dts/sun7i-a20.dtsi | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi > > b/arch/arm/boot/dts/sun7i-a20.dtsi > > index e529e4ff2174..f46af8675cfa 100644 > > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > > @@ -781,6 +781,14 @@ > > function = "ir1"; > > }; > > > > + lcd_rgb666_pins: lcd_rgb666@0 { > > I point you to this Thread and back: > https://lists.freedesktop.org/archives/dri-devel/2018-March/170688.htm > l > > I did the same for rgb888, so good part is discussed. > > IMHO I would: > - call lcd0_rgb666_pins, since this is for LCD0 interface > - same as above, call lcd0-rgb666, take care about using "-" instad > of > "_" that can cause DTC warnings. > - remove @0 since only this set can achieve LCD0 RGB666, and I >don't think there will be other combinations. I even responded to that discussion but overlooked these aspects when crafting this patch. Thanks for the summary :) > Kind regards > > -- > Giulio Benetti > CTO > > MICRONOVA SRL > Sede: Via A. Niedda 3 - 35010 Vigonza (PD) > Tel. 049/8931563 - Fax 049/8931346 > Cod.Fiscale - P.IVA 02663420285 > Capitale Sociale € 26.000 i.v. > Iscritta al Reg. Imprese di Padova N. 02663420285 > Numero R.E.A. 258642 > > > > + pins = "PD2", "PD3", "PD4", "PD5", > > "PD6", "PD7", > > + "PD10", "PD11", "PD12", > > "PD13", "PD14", "PD15", > > + "PD18", "PD19", "PD20", > > "PD21", "PD22", "PD23", > > + "PD24", "PD25", "PD26", > > "PD27"; > > + function = "lcd0"; > > + }; > > + > > mmc0_pins_a: mmc0@0 { > > pins = "PF0", "PF1", "PF2", > >"PF3", "PF4", "PF5"; > > > > -- Paul Kocialkowski, developer of free digital technology and hardware support. Website: https://www.paulk.fr/ Coding blog: https://code.paulk.fr/ Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/ signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet
Hi and thanks for the review ! Le mercredi 11 avril 2018 à 09:06 +0200, Maxime Ripard a écrit : > Hi, > > On Tue, Apr 10, 2018 at 11:31:29PM +0200, Paul Kocialkowski wrote: > > This adds support for the Ainol AW1, an A20-based 7" tablet from > > Ainol. > > > > The following board-specific features are supported: > > * LCD panel > > * Backlight > > * USB OTG > > * Buttons > > * Touchscreen (doesn't work without non-free firmware) > > * Accelerometer > > * Battery > > > > The following are untested: > > * Audio output > > * Audio speakers > > * USB via SPCI connector > > > > The following are not supported: > > * Wi-Fi > > * Bluetooth > > * NAND > > * Audio via SPCI connector > > > > Signed-off-by: Paul Kocialkowski > > --- > > arch/arm/boot/dts/Makefile| 1 + > > arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts | 358 > > ++ > > 2 files changed, 359 insertions(+) > > create mode 100644 arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts > > > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > > index 9f7133b6fba0..03bfacebfdbd 100644 > > --- a/arch/arm/boot/dts/Makefile > > +++ b/arch/arm/boot/dts/Makefile > > @@ -929,6 +929,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \ > > sun6i-a31s-sinovoip-bpi-m2.dtb \ > > sun6i-a31s-yones-toptech-bs1078-v2.dtb > > dtb-$(CONFIG_MACH_SUN7I) += \ > > + sun7i-a20-ainol-aw1.dtb \ > > sun7i-a20-bananapi.dtb \ > > sun7i-a20-bananapi-m1-plus.dtb \ > > sun7i-a20-bananapro.dtb \ > > diff --git a/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts > > b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts > > new file mode 100644 > > index ..697586991aea > > --- /dev/null > > +++ b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts > > @@ -0,0 +1,358 @@ > > +/* > > + * Copyright 2018 Paul Kocialkowski > > + * > > + * This file is dual-licensed: you can use it either under the > > terms > > + * of the GPL or the X11 license, at your option. Note that this > > dual > > + * licensing only applies to this file, and not this project as a > > + * whole. > > + * > > + * a) This file is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > as > > + * published by the Free Software Foundation; either version 2 > > of the > > + * License, or (at your option) any later version. > > + * > > + * This file is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty > > of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See > > the > > + * GNU General Public License for more details. > > + * > > + * Or, alternatively, > > + * > > + * b) Permission is hereby granted, free of charge, to any person > > + * obtaining a copy of this software and associated > > documentation > > + * files (the "Software"), to deal in the Software without > > + * restriction, including without limitation the rights to use, > > + * copy, modify, merge, publish, distribute, sublicense, and/or > > + * sell copies of the Software, and to permit persons to whom > > the > > + * Software is furnished to do so, subject to the following > > + * conditions: > > + * > > + * The above copyright notice and this permission notice shall > > be > > + * included in all copies or substantial portions of the > > Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > > KIND, > > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE > > WARRANTIES > > + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT > > + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, > > + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE > > OR > > + * OTHER DEALINGS IN THE SOFTWARE. > > Can you use an SPDX header instead of the whole license text? Sure, will do in v2. > > + */ > > + > > +/dts-v1/; > > +#include "sun7i-a20.dtsi" > > +#include "sunxi-common-regulators.dtsi" > > + > > +#include > > +#include > > +#include > > +#include > > + > > +/ { > > + model = "Ainol AW1"; > > + compatible = "ainol,ainol-aw1", "allwinner,sun7i-a20"; > > + > > + aliases { > > + serial0 = &uart0; > > + }; > > + > > + chosen { > > + stdout-path = "serial0:115200n8"; > > + }; > > + > > + backlight: backlight { > > + compatible = "pwm-backlight"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&backlight_enable_pin>; > > You don't need any of the pinctrl nodes for the GPIOs I tried without the pinctrl nodes and got issues on various controllers (e.g. i2c for the touchscreen) because of the missing pinctrl nodes on 4.16. Maybe I'm missing some patches here? > > + pwms = <&pwm 0 5 PWM_POLARITY_INVERTED>; > > +
[PATCH v7 08/10] drm/i915: Make intel_dp_mst_atomic_check() idempotent
The current way of handling changing VCPI allocations doesn't make a whole ton of sense. Since drm_atomic_helper_check_modeset() can be called multiple times which means that intel_dp_mst_atomic_check() can also be called multiple times. However, since we make the silly mistake of trying to free VCPI slots based off the /new/ atomic state instead of the old atomic state, we'll end up potentially double freeing the vcpi slots for the ports. This also has another unintended consequence that came back up while implementing MST fallback retraining: if a modeset is forced on a connector that's already part of the state, but it's atomic_check() has already been run once and doesn't get run again, we'll end up not freeing the VCPI allocations on the connector at all. The easiest way to solve this is to be clever and, with the exception of connectors that are being disabled and thus will never have compute_config() ran on them, move vcpi freeing out of the atomic check and into compute_config(). Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul --- drivers/gpu/drm/i915/intel_dp_mst.c | 84 +++-- 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 19de0b5a7a40..31b37722cd89 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -30,6 +30,38 @@ #include #include +static int +intel_dp_mst_atomic_release_vcpi_slots(struct drm_atomic_state *state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_crtc_state *new_crtc_state; + struct intel_crtc_state *intel_crtc_state = + to_intel_crtc_state(crtc_state); + struct drm_encoder *encoder; + struct drm_dp_mst_topology_mgr *mgr; + int slots, ret; + + slots = intel_crtc_state->dp_m_n.tu; + if (slots <= 0) + return 0; + + encoder = conn_state->best_encoder; + mgr = &enc_to_mst(encoder)->primary->dp.mst_mgr; + + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); + if (ret) { + DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", + slots, ret); + } else { + new_crtc_state = drm_atomic_get_crtc_state(state, + crtc_state->crtc); + to_intel_crtc_state(new_crtc_state)->dp_m_n.tu = 0; + } + + return ret; +} + static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -44,10 +76,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct drm_dp_mst_topology_state *mst_state = drm_atomic_dp_mst_get_topology_state(state, &intel_dp->mst_mgr); struct intel_dp_mst_topology_state *intel_mst_state; + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(state, &connector->base); + struct drm_crtc *old_crtc; int bpp; int slots; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; + int ret; bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_LIMITED_M_N); @@ -80,6 +116,21 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); pipe_config->pbn = mst_pbn; + /* Free any VCPI allocations on this connector from the previous +* state +*/ + old_crtc = old_conn_state->crtc; + if (old_crtc) { + struct drm_crtc_state *old_crtc_state = + drm_atomic_get_old_crtc_state(state, old_crtc); + + ret = intel_dp_mst_atomic_release_vcpi_slots(state, +old_crtc_state, +old_conn_state); + if (ret) + return ret; + } + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, connector->port, mst_pbn); if (slots < 0) { @@ -109,31 +160,22 @@ static int intel_dp_mst_atomic_check(struct drm_connector *connector, { struct drm_atomic_state *state = new_conn_state->state; struct drm_connector_state *old_conn_state; - struct drm_crtc *old_crtc; - struct drm_crtc_state *crtc_state; - int slots, ret = 0; + struct drm_crtc_state *new_crtc_state; + i
[PATCH v7 10/10] drm/i915: Implement proper fallback training for MST
For a while we actually haven't had any way of retraining MST links with fallback link parameters like we do with SST. While uncommon, certain setups such as my Caldigit TS3 + EVGA MST hub require this since otherwise, they end up getting stuck in an infinite MST retraining loop. MST retraining is somewhat different then SST retraining. While it's possible during the normal link retraining sequence for a hub to indicate bad link status, it's also possible for a hub to only indicate this status through ESI messages and it's possible for this to happen after the initial link training succeeds. This can lead to a pattern that looks like this: - Train MST link - Training completes successfully - MST hub sets Channel EQ failed bit in ESI - Retraining starts - Retraining completes successfully - MST hub sets Channel EQ failed bit in ESI again - Rinse and repeat In these situations, we need to be able to actually trigger fallback link training from the ESI handler as well, along with using the ESI handler during retraining to figure out whether or not our retraining actually succeeded. This gets a bit more complicated since we have to ensure that we don't block the ESI handler at all while doing retraining. If we do, due to DisplayPort's general issues with being sensitive to IRQ latency most MST hubs will just stop responding to us if their interrupts aren't handled in a timely manner. So: move retraining into it's own separate handler. Running in a separate handler allows us to avoid stalling the ESI during link retraining, and we can have the ESI signal that the channel EQ bit was cleared through a simple completion struct. Additionally, we take care to stick as much of this into the SST retraining path as possible since sharing is caring. V4: - Move all MST retraining work into hotplug_work - Grab the correct power wells when retraining. - Loop through MST encoders in intel_dp_get_crtc_mask(), quicker/easier than connectors V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ddi.c | 10 +- drivers/gpu/drm/i915/intel_dp.c | 370 -- drivers/gpu/drm/i915/intel_dp_link_training.c | 6 +- drivers/gpu/drm/i915/intel_dp_mst.c | 28 +- drivers/gpu/drm/i915/intel_drv.h | 7 + 5 files changed, 329 insertions(+), 92 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 92cb26b18a9b..7474300cad01 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3042,6 +3042,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder, struct intel_connector *connector) { struct drm_modeset_acquire_ctx ctx; + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); bool changed; int ret; @@ -3063,9 +3064,16 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder, break; } + if (ret == -EINVAL) + ret = intel_dp_handle_train_failure(intel_dp); + drm_modeset_drop_locks(&ctx); drm_modeset_acquire_fini(&ctx); - WARN(ret, "Acquiring modeset locks failed with %i\n", ret); + + if (ret == -EIO) + changed = true; + else + WARN(ret, "Acquiring modeset locks failed with %i\n", ret); return changed; } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index fbb467bc227d..c62d03c69ec3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -45,6 +45,8 @@ #define DP_DPRX_ESI_LEN 14 +#define DP_MST_RETRAIN_TIMEOUT (msecs_to_jiffies(100)) + /* Compliance test status bits */ #define INTEL_DP_RESOLUTION_SHIFT_MASK 0 #define INTEL_DP_RESOLUTION_PREFERRED (1 << INTEL_DP_RESOLUTION_SHIFT_MASK) @@ -2760,6 +2762,7 @@ static void intel_disable_dp(struct intel_encoder *encoder, struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); intel_dp->link_trained = false; + intel_dp->link_is_bad = false; if (old_crtc_state->has_audio) intel_audio_codec_disable(encoder, @@ -4205,8 +4208,134 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Could not write test response to sink\n"); } +/* Get a mask of the CRTCs that are running on the given intel_dp struct. For + * MST, this returns a crtc mask containing all of the CRTCs driving + * downstream sinks, for SST it just returns a mask of the attached + * connector's CRTC. + */ +int +intel_dp_get_crtc_mask(struct intel_dp *intel_dp) +{ + struct drm_device *dev = dp_to_dig_port(intel_dp)->base.base.dev; + struct drm_connector *connector; + struct drm_crtc *crtc; + int crtc_mask = 0; + + WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); + + if (intel_dp-
[PATCH v7 05/10] drm/dp_mst: Make drm_dp_mst_topology_state subclassable
This is useful for drivers (which will probably be all of them soon) which need to track state that is exclusive to the topology, and not a specific connector on said topology. This includes things such as the link rate and lane count that are shared by all of the connectors on the topology. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +++- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 46 --- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.h| 4 +- drivers/gpu/drm/drm_dp_mst_topology.c | 95 +- drivers/gpu/drm/i915/intel_dp_mst.c| 13 ++- drivers/gpu/drm/nouveau/nv50_display.c | 17 +++- drivers/gpu/drm/radeon/radeon_dp_mst.c | 13 ++- include/drm/drm_dp_mst_helper.h| 8 ++ 8 files changed, 173 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index e42a28e3adc5..2c3660c36732 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3626,9 +3626,17 @@ static int amdgpu_dm_connector_init(struct amdgpu_display_manager *dm, drm_connector_register(&aconnector->base); - if (connector_type == DRM_MODE_CONNECTOR_DisplayPort - || connector_type == DRM_MODE_CONNECTOR_eDP) - amdgpu_dm_initialize_dp_connector(dm, aconnector); + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || + connector_type == DRM_MODE_CONNECTOR_eDP) { + res = amdgpu_dm_initialize_dp_connector(dm, aconnector); + if (res) { + drm_connector_unregister(&aconnector->base); + drm_connector_cleanup(&aconnector->base); + aconnector->connector_id = -1; + + goto out_free; + } + } #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) ||\ defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 8291d74f26bc..dcaa92d12cbc 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -475,22 +475,48 @@ static const struct drm_dp_mst_topology_cbs dm_mst_cbs = { .register_connector = dm_dp_mst_register_connector }; -void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, - struct amdgpu_dm_connector *aconnector) +static const struct drm_private_state_funcs dm_mst_state_funcs = { + .atomic_duplicate_state = drm_atomic_dp_mst_duplicate_topology_state, + .atomic_destroy_state = drm_atomic_dp_mst_destroy_topology_state, +}; + +int amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, + struct amdgpu_dm_connector *aconnector) { + struct drm_dp_mst_topology_state *state = + kzalloc(sizeof(*state), GFP_KERNEL); + int ret = 0; + + if (!state) + return -ENOMEM; + aconnector->dm_dp_aux.aux.name = "dmdc"; aconnector->dm_dp_aux.aux.dev = dm->adev->dev; aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; - drm_dp_aux_register(&aconnector->dm_dp_aux.aux); + ret = drm_dp_aux_register(&aconnector->dm_dp_aux.aux); + if (ret) + goto err_aux; + aconnector->mst_mgr.cbs = &dm_mst_cbs; - drm_dp_mst_topology_mgr_init( - &aconnector->mst_mgr, - dm->adev->ddev, - &aconnector->dm_dp_aux.aux, - 16, - 4, - aconnector->connector_id); + aconnector->mst_mgr.funcs = &dm_mst_state_funcs; + ret = drm_dp_mst_topology_mgr_init(&aconnector->mst_mgr, + state, + dm->adev->ddev, + &aconnector->dm_dp_aux.aux, + 16, + 4, + aconnector->connector_id); + if (ret) + goto err_mst; + + return 0; + +err_mst: + drm_dp_aux_unregister(&aconnector->dm_dp_aux.aux); +err_aux: + kfree(state); + return ret; } diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h index 8cf51da26657..d28fb456d2d5 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_
[PATCH v7 09/10] drm/dp_mst: Add MST fallback retraining helpers
Unlike SST, MST can have far more then a single display hooked up on a single port. What this also means however, is that if the DisplayPort link to the top-level MST branch device becomes unstable then every single branch device also has an unstable link. Additionally, MST has a few more steps that must be taken in order to properly retrain the entire topology under a lower link rate. Since the VCPI allocations for each mstb is determined based off the link rate for the top of the topology, we also have to recalculate all of the VCPI allocations for the downstream ports as well to ensure that we still have enough link bandwidth to drive each mstb. Additionally, since we have multiple downstream connectors per port, setting the link status of the parent mstb's port to bad isn't enough: all of the downstream mstb ports have to have their link status set to bad. This basically follows the same paradigm that our DP link status logic in DRM does, in that we simply tell userspace that all of the mstb ports need retraining and additionally applying the new lower bandwidth constraints to all of the atomic commits on the topology that follow. Additionally; we add helpers that handle automatically checking whether or not a new atomic commit would perform the modesets required to retrain a link and if so, additionally handles updating the link status of each connector on the MST topology. V4: - clarify slightly confusing message in drm_dp_mst_topology_mgr_lower_link_rate() - Fix argument naming - Squash this with the other retrain helper, because now they're intertwined with one another - Track which connectors with CRTCs need modesets in order to retrain a topology in the topology's atomic state. This lets us greatly simplify the helpers, along with alleviating drivers of the responsibility of figuring out when to call the retrain helpers during atomic checks. It also ensures that we can keep zombie connectors that a retrain is dependent on alive until the topology either disappears, or they are disabled. We needed to do most of this anyway, since our original helpers didn't take into account that we need to invoke retraining when the link status prop changes, regardless of whether or not a modeset has been initiated yet. - Handle situation we completely forgot about: adding new connectors to an MST topology that needs fallback retraining (solution: new connectors on a topology inherit the link status of the rest of the topology) - Also make sure to handle connectors that are orphaned due to their MST topology disappearing. Solution: remove from topology state, reset link status to good - Write more docs on the retraining procedure. V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/drm_dp_mst_topology.c | 440 +- include/drm/drm_dp_mst_helper.h | 20 ++ 2 files changed, 455 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 981bd0f7d3ab..cc4b737a47b0 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1199,6 +1199,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, if (created && !port->input) { char proppath[255]; + struct drm_dp_mst_topology_state *state; build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath)); port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr, port, proppath); @@ -1217,6 +1218,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_mode_connector_set_tile_property(port->connector); } + state = to_dp_mst_topology_state(port->mgr->base.state); + if (!list_empty(&state->bad_ports)) { + port->connector->state->link_status = + DRM_LINK_STATUS_BAD; + } (*mstb->mgr->cbs->register_connector)(port->connector); } @@ -2076,7 +2082,7 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, { switch (dp_link_bw) { default: - DRM_DEBUG_KMS("invalid link bandwidth in DPCD: %x (link count: %d)\n", + DRM_DEBUG_KMS("invalid link bandwidth: %x (link count: %d)\n", dp_link_bw, dp_link_count); return false; @@ -2096,10 +2102,409 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, return true; } +static int drm_dp_set_mstb_link_status(struct drm_dp_mst_topology_mgr *mgr, + struct drm_dp_mst_branch *mstb, + enum drm_link_status status) +{ + struct drm_dp_mst_topology_state *st
[PATCH v7 07/10] drm/i915: Only use one link bw config for MST topologies
When a DP MST link needs retraining, sometimes the hub will detect that the current link bw config is impossible and will update it's RX caps in the DPCD to reflect the new maximum link rate. Currently, we make the assumption that the RX caps in the dpcd will never change like this. This means if the sink changes it's RX caps after we've already set up an MST link and we attempt to add or remove another sink from the topology, we could put ourselves into an invalid state where we've tried to configure different sinks on the same MST topology with different link rates. We could also run into this situation if a sink reports a higher link rate after suspend, usually from us having trained it with a fallback bw configuration before suspending. So: keep the link rate consistent by subclassing drm_dp_mst_topology_state, and tracking it there. For the time being, we only allow the link rate to change when the entire topology has been disconnected. V4: - Track link rate/lane count in the atomic topology state instead of in intel_dp. V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp_mst.c | 79 + drivers/gpu/drm/i915/intel_drv.h| 7 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index cf844cfd2bb0..19de0b5a7a40 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -41,8 +41,11 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_atomic_state *state = pipe_config->base.state; + struct drm_dp_mst_topology_state *mst_state = + drm_atomic_dp_mst_get_topology_state(state, &intel_dp->mst_mgr); + struct intel_dp_mst_topology_state *intel_mst_state; int bpp; - int lane_count, slots; + int slots; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc, @@ -55,18 +58,22 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, DRM_DEBUG_KMS("Setting pipe bpp to %d\n", bpp); } + + intel_mst_state = + to_intel_dp_mst_topology_state(mst_state); /* * for MST we always configure max link bw - the spec doesn't * seem to suggest we should do otherwise. */ - lane_count = intel_dp_max_lane_count(intel_dp); - - pipe_config->lane_count = lane_count; + if (!intel_mst_state->link_rate || !intel_mst_state->lane_count) { + intel_mst_state->link_rate = intel_dp_max_link_rate(intel_dp); + intel_mst_state->lane_count = intel_dp_max_lane_count(intel_dp); + } + pipe_config->lane_count = intel_mst_state->lane_count; + pipe_config->port_clock = intel_mst_state->link_rate; pipe_config->pipe_bpp = bpp; - pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); - if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port)) pipe_config->has_audio = true; @@ -80,7 +87,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, return false; } - intel_link_compute_m_n(bpp, lane_count, + intel_link_compute_m_n(bpp, intel_mst_state->lane_count, adjusted_mode->crtc_clock, pipe_config->port_clock, &pipe_config->dp_m_n, @@ -530,11 +537,55 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr) drm_kms_helper_hotplug_event(dev); } +static void intel_mst_reset_state(struct drm_dp_mst_topology_state *state) +{ + struct intel_dp_mst_topology_state *intel_mst_state = + to_intel_dp_mst_topology_state(state); + + intel_mst_state->link_rate = 0; + intel_mst_state->lane_count = 0; +} + static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = intel_dp_add_mst_connector, .register_connector = intel_dp_register_mst_connector, .destroy_connector = intel_dp_destroy_mst_connector, .hotplug = intel_dp_mst_hotplug, + .reset_state = intel_mst_reset_state, +}; + +static struct drm_private_state * +intel_dp_mst_duplicate_state(struct drm_private_obj *obj) +{ + struct intel_dp_mst_topology_state *state; + struct drm_dp_mst_topology_mgr *mgr = to_dp_mst_topology_mgr(obj); + + state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_dp_mst_duplicate_topology_state(mgr, &state->base); + + return &state->base.base; +} + +static vo
[PATCH v7 06/10] drm/dp_mst: Add reset_state callback to topology mgr
This gives drivers subclassing drm_dp_mst_topology_state the ability to prepare their topology states for a new hub to be connected whenever it's detected that the currently connected topology has been disconnected from the system. We'll need this in order to correctly track RX capabilities in i915 from the topology's atomic state. Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++ include/drm/drm_dp_mst_helper.h | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index fbd7888ebca8..981bd0f7d3ab 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2096,6 +2096,15 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, return true; } +static void drm_dp_mst_topology_reset_state(struct drm_dp_mst_topology_mgr *mgr) +{ + struct drm_dp_mst_topology_state *state = + to_dp_mst_topology_state(mgr->base.state); + + if (mgr->cbs->reset_state) + mgr->cbs->reset_state(state); +} + /** * drm_dp_mst_topology_mgr_set_mst() - Set the MST state for a topology manager * @mgr: manager to set state for @@ -2171,6 +2180,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms mgr->payload_mask = 0; set_bit(0, &mgr->payload_mask); mgr->vcpi_mask = 0; + drm_dp_mst_topology_reset_state(mgr); } out_unlock: diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index ad1aaec8d514..3a7378cd5bd1 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -381,6 +381,7 @@ struct drm_dp_sideband_msg_tx { /* sideband msg handler */ struct drm_dp_mst_topology_mgr; +struct drm_dp_mst_topology_state; struct drm_dp_mst_topology_cbs { /* create a connector for a port */ struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path); @@ -388,7 +389,7 @@ struct drm_dp_mst_topology_cbs { void (*destroy_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_connector *connector); void (*hotplug)(struct drm_dp_mst_topology_mgr *mgr); - + void (*reset_state)(struct drm_dp_mst_topology_state *state); }; #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8) -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 04/10] drm/dp_mst: Remove all evil duplicate state pointers
There's no reason to track the atomic state three times. Unfortunately, this is currently what we're doing, and even worse is that there is only one actually correct state pointer: the one in mst_state->base.state. mgr->state never seems to be used, along with the one in mst_state->state. This confused me for over 4 hours until I realized there was no magic behind these pointers. So, let's save everyone else from the trouble. Signed-off-by: Lyude Paul . Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul --- include/drm/drm_dp_mst_helper.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 2f6203407fd2..5ca77dcf4f90 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -409,7 +409,6 @@ struct drm_dp_payload { struct drm_dp_mst_topology_state { struct drm_private_state base; int avail_slots; - struct drm_atomic_state *state; struct drm_dp_mst_topology_mgr *mgr; }; @@ -497,11 +496,6 @@ struct drm_dp_mst_topology_mgr { */ int pbn_div; - /** -* @state: State information for topology manager -*/ - struct drm_dp_mst_topology_state *state; - /** * @funcs: Atomic helper callbacks */ -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 01/10] drm/atomic: Print debug message on atomic check failure
Does what it says on the label, it's a little confusing debugging atomic check failures otherwise. Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_atomic.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 7d25c42f22db..972a7e9634ab 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1705,8 +1705,11 @@ int drm_atomic_check_only(struct drm_atomic_state *state) if (config->funcs->atomic_check) ret = config->funcs->atomic_check(state->dev, state); - if (ret) + if (ret) { + DRM_DEBUG_ATOMIC("atomic driver check for %p failed: %d\n", +state, ret); return ret; + } if (!state->allow_modeset) { for_each_new_crtc_in_state(state, crtc, crtc_state, i) { -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 03/10] drm/dp_mst: Fix naming on drm_atomic_get_mst_topology_state()
Since these functions are dealing with an atomic state that needs to be modified during atomic check and commit, change the naming on this function to drm_atomic_dp_mst_get_topology_state() since we're the only one using the function, and it's more consistent with the naming scheme used in drm_atomic.c/drm_atomic_helper.c. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 13 +++-- include/drm/drm_dp_mst_helper.h | 6 -- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 6fac4129e6a2..ba67f1782a04 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2622,7 +2622,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_state *topology_state; int req_slots; - topology_state = drm_atomic_get_mst_topology_state(state, mgr); + topology_state = drm_atomic_dp_mst_get_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); @@ -2662,7 +2662,7 @@ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, { struct drm_dp_mst_topology_state *topology_state; - topology_state = drm_atomic_get_mst_topology_state(state, mgr); + topology_state = drm_atomic_dp_mst_get_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); @@ -3129,7 +3129,7 @@ static const struct drm_private_state_funcs mst_state_funcs = { }; /** - * drm_atomic_get_mst_topology_state: get MST topology state + * drm_atomic_dp_mst_get_topology_state: get MST topology state * * @state: global atomic state * @mgr: MST topology manager, also the private object in this case @@ -3143,15 +3143,16 @@ static const struct drm_private_state_funcs mst_state_funcs = { * * The MST topology state or error pointer. */ -struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr) +struct drm_dp_mst_topology_state * +drm_atomic_dp_mst_get_topology_state(struct drm_atomic_state *state, +struct drm_dp_mst_topology_mgr *mgr) { struct drm_device *dev = mgr->dev; WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state, &mgr->base)); } -EXPORT_SYMBOL(drm_atomic_get_mst_topology_state); +EXPORT_SYMBOL(drm_atomic_dp_mst_get_topology_state); /** * drm_dp_mst_topology_mgr_init - initialise a topology manager diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 7f78d26a0766..2f6203407fd2 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -623,8 +623,10 @@ void drm_dp_mst_dump_topology(struct seq_file *m, void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr); int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr); -struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr); + +struct drm_dp_mst_topology_state * +drm_atomic_dp_mst_get_topology_state(struct drm_atomic_state *state, +struct drm_dp_mst_topology_mgr *mgr); int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn); -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 00/10] drm/i915: Implement proper fallback training for MST
Next version of https://patchwork.freedesktop.org/series/41576/ All changes in this patch series are just to make checkpatch a little happier, no functional changes. Lyude Paul (10): drm/atomic: Print debug message on atomic check failure drm/i915: Move DP modeset retry work into intel_dp drm/dp_mst: Fix naming on drm_atomic_get_mst_topology_state() drm/dp_mst: Remove all evil duplicate state pointers drm/dp_mst: Make drm_dp_mst_topology_state subclassable drm/dp_mst: Add reset_state callback to topology mgr drm/i915: Only use one link bw config for MST topologies drm/i915: Make intel_dp_mst_atomic_check() idempotent drm/dp_mst: Add MST fallback retraining helpers drm/i915: Implement proper fallback training for MST drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 46 +- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.h| 4 +- drivers/gpu/drm/drm_atomic.c | 5 +- drivers/gpu/drm/drm_dp_mst_topology.c | 550 - drivers/gpu/drm/i915/intel_ddi.c | 10 +- drivers/gpu/drm/i915/intel_display.c | 14 +- drivers/gpu/drm/i915/intel_dp.c| 376 ++ drivers/gpu/drm/i915/intel_dp_link_training.c | 6 +- drivers/gpu/drm/i915/intel_dp_mst.c| 186 +-- drivers/gpu/drm/i915/intel_drv.h | 20 +- drivers/gpu/drm/nouveau/nv50_display.c | 17 +- drivers/gpu/drm/radeon/radeon_dp_mst.c | 13 +- include/drm/drm_dp_mst_helper.h| 43 +- 14 files changed, 1120 insertions(+), 184 deletions(-) -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 02/10] drm/i915: Move DP modeset retry work into intel_dp
While having the modeset_retry_work in intel_connector makes sense with SST, this paradigm doesn't make a whole ton of sense when it comes to MST since we have to deal with multiple connectors. In most cases, it's more useful to just use the intel_dp struct since it indicates whether or not we're dealing with an MST device, along with being able to easily trace the intel_dp struct back to it's respective connector (if there is any). So, move the modeset_retry_work function out of the intel_connector struct and into intel_dp. Signed-off-by: Lyude Paul Reviewed-by: Manasi Navare Cc: Manasi Navare Cc: Ville Syrjälä V2: - Remove accidental duplicate modeset_retry_work in intel_connector V3: - Also check against eDP in intel_hpd_poll_fini() - mdnavare V4: - Don't bother looping over connectors for canceling modeset rety work, just encoders. V7: - Fix CHECKPATCH errors Signed-off-by: Lyude Paul --- drivers/gpu/drm/i915/intel_display.c | 14 +++--- drivers/gpu/drm/i915/intel_dp.c | 10 -- drivers/gpu/drm/i915/intel_dp_link_training.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 6 +++--- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e04050ea3e28..18edb9628a54 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15471,20 +15471,28 @@ void intel_connector_unregister(struct drm_connector *connector) static void intel_hpd_poll_fini(struct drm_device *dev) { - struct intel_connector *connector; struct drm_connector_list_iter conn_iter; + struct intel_connector *connector; + struct intel_encoder *encoder; + struct intel_dp *intel_dp; /* Kill all the work that may have been queued by hpd. */ drm_connector_list_iter_begin(dev, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { - if (connector->modeset_retry_work.func) - cancel_work_sync(&connector->modeset_retry_work); if (connector->hdcp_shim) { cancel_delayed_work_sync(&connector->hdcp_check_work); cancel_work_sync(&connector->hdcp_prop_work); } } drm_connector_list_iter_end(&conn_iter); + + for_each_intel_encoder(dev, encoder) { + if (encoder->type == INTEL_OUTPUT_DP || + encoder->type == INTEL_OUTPUT_EDP) { + intel_dp = enc_to_intel_dp(&encoder->base); + cancel_work_sync(&intel_dp->modeset_retry_work); + } + } } void intel_modeset_cleanup(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 62f82c4298ac..fbb467bc227d 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6249,12 +6249,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, static void intel_dp_modeset_retry_work_fn(struct work_struct *work) { - struct intel_connector *intel_connector; - struct drm_connector *connector; + struct intel_dp *intel_dp = container_of(work, typeof(*intel_dp), +modeset_retry_work); + struct drm_connector *connector = &intel_dp->attached_connector->base; - intel_connector = container_of(work, typeof(*intel_connector), - modeset_retry_work); - connector = &intel_connector->base; DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); @@ -6283,7 +6281,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, int type; /* Initialize the work for modeset in case of link train failure */ - INIT_WORK(&intel_connector->modeset_retry_work, + INIT_WORK(&intel_dp->modeset_retry_work, intel_dp_modeset_retry_work_fn); if (WARN(intel_dig_port->max_lanes < 1, diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index f59b59bb0a21..2cfa58ce1f95 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -340,7 +340,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) intel_dp->link_rate, intel_dp->lane_count)) /* Schedule a Hotplug Uevent to userspace to start modeset */ - schedule_work(&intel_connector->modeset_retry_work); + schedule_work(&intel_dp->modeset_retry_work); } else { DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", intel
Re: [PATCH 2/3] ARM: dts: sun7i: Add RGB666 pins definition
Hi, Il 10/04/2018 23:31, Paul Kocialkowski ha scritto: This adds the pins definition for RGB666 LCD panels on the A20. It was imported from the A33 definition, that concernes the same set of pins. Signed-off-by: Paul Kocialkowski --- arch/arm/boot/dts/sun7i-a20.dtsi | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index e529e4ff2174..f46af8675cfa 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -781,6 +781,14 @@ function = "ir1"; }; + lcd_rgb666_pins: lcd_rgb666@0 { I point you to this Thread and back: https://lists.freedesktop.org/archives/dri-devel/2018-March/170688.html I did the same for rgb888, so good part is discussed. IMHO I would: - call lcd0_rgb666_pins, since this is for LCD0 interface - same as above, call lcd0-rgb666, take care about using "-" instad of "_" that can cause DTC warnings. - remove @0 since only this set can achieve LCD0 RGB666, and I don't think there will be other combinations. Kind regards -- Giulio Benetti CTO MICRONOVA SRL Sede: Via A. Niedda 3 - 35010 Vigonza (PD) Tel. 049/8931563 - Fax 049/8931346 Cod.Fiscale - P.IVA 02663420285 Capitale Sociale € 26.000 i.v. Iscritta al Reg. Imprese di Padova N. 02663420285 Numero R.E.A. 258642 + pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7", + "PD10", "PD11", "PD12", "PD13", "PD14", "PD15", + "PD18", "PD19", "PD20", "PD21", "PD22", "PD23", + "PD24", "PD25", "PD26", "PD27"; + function = "lcd0"; + }; + mmc0_pins_a: mmc0@0 { pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5"; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105990] [CI] igt@gem_eio@in-flight-immediate - fail - Failed assertion: __igt_device_drop_master(fd) == 0
https://bugs.freedesktop.org/show_bug.cgi?id=105990 --- Comment #5 from Chris Wilson --- We do for set_master, but not drop as I didn't think it failed for !master!! -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105990] [CI] igt@gem_eio@in-flight-immediate - fail - Failed assertion: __igt_device_drop_master(fd) == 0
https://bugs.freedesktop.org/show_bug.cgi?id=105990 --- Comment #4 from Martin Peres --- I agree. Maybe we could print the list of the clients when this fails? Bonus points for showing which one is actually master? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/4] drm/vc4: Expose gamma as atomic property
We are an atomic driver so the gamma LUT should also be exposed as a CRTC property through the DRM atomic color management. This will also take care of the legacy path for us. Signed-off-by: Stefan Schake --- v2: Use drm_color_lut_size for LUT length v3: Disable gamma lut when gamma_lut is NULL (Eric) drivers/gpu/drm/vc4/vc4_crtc.c | 37 ++--- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index bf46674..285f88d 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -298,23 +298,21 @@ vc4_crtc_lut_load(struct drm_crtc *crtc) HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_b[i]); } -static int -vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, - uint32_t size, - struct drm_modeset_acquire_ctx *ctx) +static void +vc4_crtc_update_gamma_lut(struct drm_crtc *crtc) { struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); + struct drm_color_lut *lut = crtc->state->gamma_lut->data; + u32 length = drm_color_lut_size(crtc->state->gamma_lut); u32 i; - for (i = 0; i < size; i++) { - vc4_crtc->lut_r[i] = r[i] >> 8; - vc4_crtc->lut_g[i] = g[i] >> 8; - vc4_crtc->lut_b[i] = b[i] >> 8; + for (i = 0; i < length; i++) { + vc4_crtc->lut_r[i] = drm_color_lut_extract(lut[i].red, 8); + vc4_crtc->lut_g[i] = drm_color_lut_extract(lut[i].green, 8); + vc4_crtc->lut_b[i] = drm_color_lut_extract(lut[i].blue, 8); } vc4_crtc_lut_load(crtc); - - return 0; } static u32 vc4_get_fifo_full_level(u32 format) @@ -699,6 +697,22 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *crtc, if (crtc->state->active && old_state->active) vc4_crtc_update_dlist(crtc); + if (crtc->state->color_mgmt_changed) { + u32 dispbkgndx = HVS_READ(SCALER_DISPBKGNDX(vc4_crtc->channel)); + + if (crtc->state->gamma_lut) { + vc4_crtc_update_gamma_lut(crtc); + dispbkgndx |= SCALER_DISPBKGND_GAMMA; + } else { + /* Unsetting DISPBKGND_GAMMA skips the gamma lut step +* in hardware, which is the same as a linear lut that +* DRM expects us to use in absence of a user lut. +*/ + dispbkgndx &= ~SCALER_DISPBKGND_GAMMA; + } + HVS_WRITE(SCALER_DISPBKGNDX(vc4_crtc->channel), dispbkgndx); + } + if (debug_dump_regs) { DRM_INFO("CRTC %d HVS after:\n", drm_crtc_index(crtc)); vc4_hvs_dump_state(dev); @@ -909,7 +923,7 @@ static const struct drm_crtc_funcs vc4_crtc_funcs = { .reset = vc4_crtc_reset, .atomic_duplicate_state = vc4_crtc_duplicate_state, .atomic_destroy_state = vc4_crtc_destroy_state, - .gamma_set = vc4_crtc_gamma_set, + .gamma_set = drm_atomic_helper_legacy_gamma_set, .enable_vblank = vc4_enable_vblank, .disable_vblank = vc4_disable_vblank, }; @@ -1035,6 +1049,7 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) primary_plane->crtc = crtc; vc4_crtc->channel = vc4_crtc->data->hvs_channel; drm_mode_crtc_set_gamma_size(crtc, ARRAY_SIZE(vc4_crtc->lut_r)); + drm_crtc_enable_color_mgmt(crtc, 0, false, crtc->gamma_size); /* Set up some arbitrary number of planes. We're not limited * by a set number of physical registers, just the space in -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/4] drm/vc4: Atomic color management support
This series adds support for the gamma and CTM properties to VC4. The CTM support is somewhat limited in that we can only enable it for one CRTC at a time and coefficients are S0.9 in hardware. The latter seems good enough for the various color corrections Android offers. The CTM support in v3 is an entire rewrite from previous versions since I didn't previously model our CTM hardware as private atomic state, which is needed to correctly limit it to one CRTC. Since v2, Eric has also confirmed from the HDL that CTM in VC4 is applied before gamma lut, matching the documented behavior for the DRM property. Eric Anholt (1): drm/vc4: Add some missing HVS register definitions. Stefan Schake (3): drm/vc4: Expose gamma as atomic property drm/vc4: Move CRTC state to header drm/vc4: Add CTM support drivers/gpu/drm/vc4/vc4_crtc.c | 74 -- drivers/gpu/drm/vc4/vc4_drv.h | 36 + drivers/gpu/drm/vc4/vc4_kms.c | 167 - drivers/gpu/drm/vc4/vc4_regs.h | 96 +++ 4 files changed, 328 insertions(+), 45 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/4] drm/vc4: Move CRTC state to header
We need to access the channel for configuring our CTM hardware. Signed-off-by: Stefan Schake --- v3: New in the series drivers/gpu/drm/vc4/vc4_crtc.c | 33 - drivers/gpu/drm/vc4/vc4_drv.h | 33 + 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 285f88d..08fe8dd 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -42,51 +42,18 @@ #include "vc4_drv.h" #include "vc4_regs.h" -struct vc4_crtc { - struct drm_crtc base; - const struct vc4_crtc_data *data; - void __iomem *regs; - - /* Timestamp at start of vblank irq - unaffected by lock delays. */ - ktime_t t_vblank; - - /* Which HVS channel we're using for our CRTC. */ - int channel; - - u8 lut_r[256]; - u8 lut_g[256]; - u8 lut_b[256]; - /* Size in pixels of the COB memory allocated to this CRTC. */ - u32 cob_size; - - struct drm_pending_vblank_event *event; -}; - struct vc4_crtc_state { struct drm_crtc_state base; /* Dlist area for this CRTC configuration. */ struct drm_mm_node mm; }; -static inline struct vc4_crtc * -to_vc4_crtc(struct drm_crtc *crtc) -{ - return (struct vc4_crtc *)crtc; -} - static inline struct vc4_crtc_state * to_vc4_crtc_state(struct drm_crtc_state *crtc_state) { return (struct vc4_crtc_state *)crtc_state; } -struct vc4_crtc_data { - /* Which channel of the HVS this pixelvalve sources from. */ - int hvs_channel; - - enum vc4_encoder_type encoder_types[4]; -}; - #define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset)) #define CRTC_READ(offset) readl(vc4_crtc->regs + (offset)) diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 1b4cd1f..4288615 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -392,6 +392,39 @@ to_vc4_encoder(struct drm_encoder *encoder) return container_of(encoder, struct vc4_encoder, base); } +struct vc4_crtc_data { + /* Which channel of the HVS this pixelvalve sources from. */ + int hvs_channel; + + enum vc4_encoder_type encoder_types[4]; +}; + +struct vc4_crtc { + struct drm_crtc base; + const struct vc4_crtc_data *data; + void __iomem *regs; + + /* Timestamp at start of vblank irq - unaffected by lock delays. */ + ktime_t t_vblank; + + /* Which HVS channel we're using for our CRTC. */ + int channel; + + u8 lut_r[256]; + u8 lut_g[256]; + u8 lut_b[256]; + /* Size in pixels of the COB memory allocated to this CRTC. */ + u32 cob_size; + + struct drm_pending_vblank_event *event; +}; + +static inline struct vc4_crtc * +to_vc4_crtc(struct drm_crtc *crtc) +{ + return (struct vc4_crtc *)crtc; +} + #define V3D_READ(offset) readl(vc4->v3d->regs + offset) #define V3D_WRITE(offset, val) writel(val, vc4->v3d->regs + offset) #define HVS_READ(offset) readl(vc4->hvs->regs + offset) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/4] drm/vc4: Add some missing HVS register definitions.
From: Eric Anholt At least the RGBA expand field we should have been setting, because we aren't expanding correctly for 565 -> . Other registers are ones that may be interesting for various projects that have been discussed. Signed-off-by: Eric Anholt Cc: Stefan Schake Acked-by: Stefan Schake --- drivers/gpu/drm/vc4/vc4_regs.h | 96 ++ 1 file changed, 96 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h index a141496..4af3e29 100644 --- a/drivers/gpu/drm/vc4/vc4_regs.h +++ b/drivers/gpu/drm/vc4/vc4_regs.h @@ -330,6 +330,21 @@ #define SCALER_DISPCTRL00x0040 # define SCALER_DISPCTRLX_ENABLE BIT(31) # define SCALER_DISPCTRLX_RESETBIT(30) +/* Generates a single frame when VSTART is seen and stops at the last + * pixel read from the FIFO. + */ +# define SCALER_DISPCTRLX_ONESHOT BIT(29) +/* Processes a single context in the dlist and then task switch, + * instead of an entire line. + */ +# define SCALER_DISPCTRLX_ONECTX BIT(28) +/* Set to have DISPSLAVE return 2 16bpp pixels and no status data. */ +# define SCALER_DISPCTRLX_FIFO32 BIT(27) +/* Turns on output to the DISPSLAVE register instead of the normal + * FIFO. + */ +# define SCALER_DISPCTRLX_FIFOREG BIT(26) + # define SCALER_DISPCTRLX_WIDTH_MASK VC4_MASK(23, 12) # define SCALER_DISPCTRLX_WIDTH_SHIFT 12 # define SCALER_DISPCTRLX_HEIGHT_MASK VC4_MASK(11, 0) @@ -402,6 +417,68 @@ */ # define SCALER_GAMADDR_SRAMENBBIT(30) +#define SCALER_OLEDOFFS 0x0080 +/* Clamps R to [16,235] and G/B to [16,240]. */ +# define SCALER_OLEDOFFS_YUVCLAMP BIT(31) + +/* Chooses which display FIFO the matrix applies to. */ +# define SCALER_OLEDOFFS_DISPFIFO_MASK VC4_MASK(25, 24) +# define SCALER_OLEDOFFS_DISPFIFO_SHIFT 24 +# define SCALER_OLEDOFFS_DISPFIFO_DISABLED 0 +# define SCALER_OLEDOFFS_DISPFIFO_0 1 +# define SCALER_OLEDOFFS_DISPFIFO_1 2 +# define SCALER_OLEDOFFS_DISPFIFO_2 3 + +/* Offsets are 8-bit 2s-complement. */ +# define SCALER_OLEDOFFS_RED_MASK VC4_MASK(23, 16) +# define SCALER_OLEDOFFS_RED_SHIFT 16 +# define SCALER_OLEDOFFS_GREEN_MASK VC4_MASK(15, 8) +# define SCALER_OLEDOFFS_GREEN_SHIFT8 +# define SCALER_OLEDOFFS_BLUE_MASK VC4_MASK(7, 0) +# define SCALER_OLEDOFFS_BLUE_SHIFT 0 + +/* The coefficients are S0.9 fractions. */ +#define SCALER_OLEDCOEF00x0084 +# define SCALER_OLEDCOEF0_B_TO_R_MASK VC4_MASK(29, 20) +# define SCALER_OLEDCOEF0_B_TO_R_SHIFT 20 +# define SCALER_OLEDCOEF0_B_TO_G_MASK VC4_MASK(19, 10) +# define SCALER_OLEDCOEF0_B_TO_G_SHIFT 10 +# define SCALER_OLEDCOEF0_B_TO_B_MASK VC4_MASK(9, 0) +# define SCALER_OLEDCOEF0_B_TO_B_SHIFT 0 + +#define SCALER_OLEDCOEF10x0088 +# define SCALER_OLEDCOEF1_G_TO_R_MASK VC4_MASK(29, 20) +# define SCALER_OLEDCOEF1_G_TO_R_SHIFT 20 +# define SCALER_OLEDCOEF1_G_TO_G_MASK VC4_MASK(19, 10) +# define SCALER_OLEDCOEF1_G_TO_G_SHIFT 10 +# define SCALER_OLEDCOEF1_G_TO_B_MASK VC4_MASK(9, 0) +# define SCALER_OLEDCOEF1_G_TO_B_SHIFT 0 + +#define SCALER_OLEDCOEF20x008c +# define SCALER_OLEDCOEF2_R_TO_R_MASK VC4_MASK(29, 20) +# define SCALER_OLEDCOEF2_R_TO_R_SHIFT 20 +# define SCALER_OLEDCOEF2_R_TO_G_MASK VC4_MASK(19, 10) +# define SCALER_OLEDCOEF2_R_TO_G_SHIFT 10 +# define SCALER_OLEDCOEF2_R_TO_B_MASK VC4_MASK(9, 0) +# define SCALER_OLEDCOEF2_R_TO_B_SHIFT 0 + +/* Slave addresses for DMAing from HVS composition output to other + * devices. The top bits are valid only in !FIFO32 mode. + */ +#define SCALER_DISPSLAVE0 0x00c0 +#define SCALER_DISPSLAVE1 0x00c9 +#define SCALER_DISPSLAVE2 0x00d0 +# define SCALER_DISPSLAVE_ISSUE_VSTART BIT(31) +# define SCALER_DISPSLAVE_ISSUE_HSTART BIT(30) +/* Set when the current line has been read and an HSTART is required. */ +# define SCALER_DISPSLAVE_EOL BIT(26) +/* Set when the display FIFO is empty. */ +# define SCALER_DISPSLAVE_EMPTY BIT(25) +/* Set when there is RGB data ready to read. */ +# define SCALER_DISPSLAVE_VALID BIT(24) +# define SCALER_DISPSLAVE_RGB_MASK VC4_MASK(23, 0) +# define SCALER_DISPSLAVE_RGB_SHIFT 0 + #define SCALER_GAMDATA 0x00e0 #define SCALER_DLIST_START 0x2000 #define SCALER_DLIST_SIZE 0x4000 @@ -767,6 +844,10 @@ enum hvs_pixel_format {
[PATCH v3 4/4] drm/vc4: Add CTM support
The hardware has a single block for applying a CTM prior to gamma lut. It can be fed with pixels from one of our CRTC at a time and uses a matrix with S0.9 scalars. Use private atomic state to reject attempts from userland to apply CTM for more than one CRTC at a time and reject matrices with scalars that we can't approximate without integer bits. Signed-off-by: Stefan Schake --- v3: New in the series drivers/gpu/drm/vc4/vc4_crtc.c | 6 +- drivers/gpu/drm/vc4/vc4_drv.h | 3 + drivers/gpu/drm/vc4/vc4_kms.c | 167 - 3 files changed, 174 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 08fe8dd..32bdf13 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1016,7 +1016,11 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data) primary_plane->crtc = crtc; vc4_crtc->channel = vc4_crtc->data->hvs_channel; drm_mode_crtc_set_gamma_size(crtc, ARRAY_SIZE(vc4_crtc->lut_r)); - drm_crtc_enable_color_mgmt(crtc, 0, false, crtc->gamma_size); + + /* We support CTM, but only for one CRTC at a time. It's therefore +* implemented as private driver state in vc4_kms, not here. +*/ + drm_crtc_enable_color_mgmt(crtc, 0, true, crtc->gamma_size); /* Set up some arbitrary number of planes. We're not limited * by a set number of physical registers, just the space in diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 4288615..ee22a5b 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -10,6 +10,7 @@ #include #include #include +#include #include "uapi/drm/vc4_drm.h" @@ -193,6 +194,8 @@ struct vc4_dev { } hangcheck; struct semaphore async_modeset; + + struct drm_private_obj ctm_manager; }; static inline struct vc4_dev * diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index ba60153..f2eca4d 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -23,6 +23,102 @@ #include #include #include "vc4_drv.h" +#include "vc4_regs.h" + +struct vc4_ctm_state { + struct drm_private_state base; + struct drm_color_ctm *ctm; + int fifo; +}; + +static struct vc4_ctm_state *to_vc4_ctm_state(struct drm_private_state *priv) +{ + return container_of(priv, struct vc4_ctm_state, base); +} + +static struct vc4_ctm_state *vc4_get_ctm_state(struct drm_atomic_state *state, + struct drm_private_obj *manager) +{ + return to_vc4_ctm_state(drm_atomic_get_private_obj_state(state, manager)); +} + +static struct drm_private_state * +vc4_ctm_duplicate_state(struct drm_private_obj *obj) +{ + struct vc4_ctm_state *state; + + state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base); + + return &state->base; +} + +static void vc4_ctm_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct vc4_ctm_state *ctm_state = to_vc4_ctm_state(state); + + kfree(ctm_state); +} + +static const struct drm_private_state_funcs vc4_ctm_state_funcs = { + .atomic_duplicate_state = vc4_ctm_duplicate_state, + .atomic_destroy_state = vc4_ctm_destroy_state, +}; + +/* Converts a DRM S31.32 value to the HW S0.9 format. */ +static u16 vc4_ctm_s31_32_to_s0_9(u64 in) +{ + u16 r; + + /* Sign bit. */ + r = in & BIT_ULL(63) ? BIT(9) : 0; + /* We have zero integer bits so we can only saturate here. */ + if ((in & GENMASK_ULL(62, 32)) > 0) + r |= GENMASK(8, 0); + /* Otherwise take the 9 most important fractional bits. */ + else + r |= (in >> 22) & GENMASK(8, 0); + return r; +} + +static void +vc4_ctm_commit(struct vc4_dev *vc4, struct drm_atomic_state *state) +{ + struct vc4_ctm_state *ctm_state = vc4_get_ctm_state(state, + &vc4->ctm_manager); + struct drm_color_ctm *ctm = ctm_state->ctm; + + if (ctm_state->fifo) { + HVS_WRITE(SCALER_OLEDCOEF2, + VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[0]), + SCALER_OLEDCOEF2_R_TO_R) | + VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[3]), + SCALER_OLEDCOEF2_R_TO_G) | + VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[6]), + SCALER_OLEDCOEF2_R_TO_B)); + HVS_WRITE(SCALER_OLEDCOEF1, + VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[1]), + SCA
[Bug 199357] amdgpu: hang a few seconds after logging in, most likely due to regression
https://bugzilla.kernel.org/show_bug.cgi?id=199357 --- Comment #5 from Harry Wentland (harry.wentl...@amd.com) --- I've no idea why this causes "flip_done timed out" and locks the system right now, but we're currently also dealing with some more fallout from that change, in particular blinking/flickering display if redshift/nightlight is on. I'm reluctant to just revert the offending commit as it's not incorrect but seems to expose some other flaws in our atomic check/commit implementation. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105990] [CI] igt@gem_eio@in-flight-immediate - fail - Failed assertion: __igt_device_drop_master(fd) == 0
https://bugs.freedesktop.org/show_bug.cgi?id=105990 Chris Wilson changed: What|Removed |Added Assignee|intel-gfx-bugs@lists.freede |dri-devel@lists.freedesktop |sktop.org |.org QA Contact|intel-gfx-bugs@lists.freede | |sktop.org | Component|DRM/Intel |IGT --- Comment #3 from Chris Wilson --- Hmm, it *is* reporting that is not currently master (I had thought that was a silent return). However, it really, really should be master at that point, that implies something else is and this test should be the only client... -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 05/10] drm/dp_mst: Make drm_dp_mst_topology_state subclassable
This is useful for drivers (which will probably be all of them soon) which need to track state that is exclusive to the topology, and not a specific connector on said topology. This includes things such as the link rate and lane count that are shared by all of the connectors on the topology. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +++- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 35 +++- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.h| 4 +- drivers/gpu/drm/drm_dp_mst_topology.c | 94 +- drivers/gpu/drm/i915/intel_dp_mst.c| 13 ++- drivers/gpu/drm/nouveau/nv50_display.c | 15 +++- drivers/gpu/drm/radeon/radeon_dp_mst.c | 13 ++- include/drm/drm_dp_mst_helper.h| 8 ++ 8 files changed, 165 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index e42a28e3adc5..2c3660c36732 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3626,9 +3626,17 @@ static int amdgpu_dm_connector_init(struct amdgpu_display_manager *dm, drm_connector_register(&aconnector->base); - if (connector_type == DRM_MODE_CONNECTOR_DisplayPort - || connector_type == DRM_MODE_CONNECTOR_eDP) - amdgpu_dm_initialize_dp_connector(dm, aconnector); + if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || + connector_type == DRM_MODE_CONNECTOR_eDP) { + res = amdgpu_dm_initialize_dp_connector(dm, aconnector); + if (res) { + drm_connector_unregister(&aconnector->base); + drm_connector_cleanup(&aconnector->base); + aconnector->connector_id = -1; + + goto out_free; + } + } #if defined(CONFIG_BACKLIGHT_CLASS_DEVICE) ||\ defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 8291d74f26bc..a3a43b1b30df 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -475,22 +475,49 @@ static const struct drm_dp_mst_topology_cbs dm_mst_cbs = { .register_connector = dm_dp_mst_register_connector }; -void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, - struct amdgpu_dm_connector *aconnector) +static const struct drm_private_state_funcs dm_mst_state_funcs = { + .atomic_duplicate_state = drm_atomic_dp_mst_duplicate_topology_state, + .atomic_destroy_state = drm_atomic_dp_mst_destroy_topology_state, +}; + +int amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, + struct amdgpu_dm_connector *aconnector) { + struct drm_dp_mst_topology_state *state = + kzalloc(sizeof(*state), GFP_KERNEL); + int ret = 0; + + if (!state) + return -ENOMEM; + aconnector->dm_dp_aux.aux.name = "dmdc"; aconnector->dm_dp_aux.aux.dev = dm->adev->dev; aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer; aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc; - drm_dp_aux_register(&aconnector->dm_dp_aux.aux); + ret = drm_dp_aux_register(&aconnector->dm_dp_aux.aux); + if (ret) + goto err_aux; + aconnector->mst_mgr.cbs = &dm_mst_cbs; - drm_dp_mst_topology_mgr_init( + aconnector->mst_mgr.funcs = &dm_mst_state_funcs; + ret = drm_dp_mst_topology_mgr_init( &aconnector->mst_mgr, + state, dm->adev->ddev, &aconnector->dm_dp_aux.aux, 16, 4, aconnector->connector_id); + if (ret) + goto err_mst; + + return 0; + +err_mst: + drm_dp_aux_unregister(&aconnector->dm_dp_aux.aux); +err_aux: + kfree(state); + return ret; } diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h index 8cf51da26657..d28fb456d2d5 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h @@ -29,8 +29,8 @@ struct amdgpu_display_manager; struct amdgpu_dm_connector; -void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, - struct amdgpu_dm_connector *aconnector); +int amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm, + struct amdgpu_dm_connector *aconnector); voi
[PATCH v6 10/10] drm/i915: Implement proper fallback training for MST
For a while we actually haven't had any way of retraining MST links with fallback link parameters like we do with SST. While uncommon, certain setups such as my Caldigit TS3 + EVGA MST hub require this since otherwise, they end up getting stuck in an infinite MST retraining loop. MST retraining is somewhat different then SST retraining. While it's possible during the normal link retraining sequence for a hub to indicate bad link status, it's also possible for a hub to only indicate this status through ESI messages and it's possible for this to happen after the initial link training succeeds. This can lead to a pattern that looks like this: - Train MST link - Training completes successfully - MST hub sets Channel EQ failed bit in ESI - Retraining starts - Retraining completes successfully - MST hub sets Channel EQ failed bit in ESI again - Rinse and repeat In these situations, we need to be able to actually trigger fallback link training from the ESI handler as well, along with using the ESI handler during retraining to figure out whether or not our retraining actually succeeded. This gets a bit more complicated since we have to ensure that we don't block the ESI handler at all while doing retraining. If we do, due to DisplayPort's general issues with being sensitive to IRQ latency most MST hubs will just stop responding to us if their interrupts aren't handled in a timely manner. So: move retraining into it's own seperate handler. Running in a seperate handler allows us to avoid stalling the ESI during link retraining, and we can have the ESI signal that the channel EQ bit was cleared through a simple completion struct. Additionally, we take care to stick as much of this into the SST retraining path as possible since sharing is caring. V4: - Move all MST retraining work into hotplug_work - Grab the correct power wells when retraining. - Loop through MST encoders in intel_dp_get_crtc_mask(), quicker/easier than connectors Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ddi.c | 10 +- drivers/gpu/drm/i915/intel_dp.c | 355 -- drivers/gpu/drm/i915/intel_dp_link_training.c | 6 +- drivers/gpu/drm/i915/intel_dp_mst.c | 27 +- drivers/gpu/drm/i915/intel_drv.h | 7 + 5 files changed, 313 insertions(+), 92 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 92cb26b18a9b..effc61db41d2 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3063,9 +3063,17 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder, break; } + if (ret == -EINVAL) + ret = intel_dp_handle_train_failure( + enc_to_intel_dp(&encoder->base)); + drm_modeset_drop_locks(&ctx); drm_modeset_acquire_fini(&ctx); - WARN(ret, "Acquiring modeset locks failed with %i\n", ret); + + if (ret == -EIO) + changed = true; + else + WARN(ret, "Acquiring modeset locks failed with %i\n", ret); return changed; } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index fbb467bc227d..a7fd28929402 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -45,6 +45,8 @@ #define DP_DPRX_ESI_LEN 14 +#define DP_MST_RETRAIN_TIMEOUT (msecs_to_jiffies(100)) + /* Compliance test status bits */ #define INTEL_DP_RESOLUTION_SHIFT_MASK 0 #define INTEL_DP_RESOLUTION_PREFERRED (1 << INTEL_DP_RESOLUTION_SHIFT_MASK) @@ -2760,6 +2762,7 @@ static void intel_disable_dp(struct intel_encoder *encoder, struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); intel_dp->link_trained = false; + intel_dp->link_is_bad = false; if (old_crtc_state->has_audio) intel_audio_codec_disable(encoder, @@ -4205,8 +4208,133 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Could not write test response to sink\n"); } +/* Get a mask of the CRTCs that are running on the given intel_dp struct. For + * MST, this returns a crtc mask containing all of the CRTCs driving + * downstream sinks, for SST it just returns a mask of the attached + * connector's CRTC. + */ +int +intel_dp_get_crtc_mask(struct intel_dp *intel_dp) +{ + struct drm_device *dev = dp_to_dig_port(intel_dp)->base.base.dev; + struct drm_connector *connector; + struct drm_crtc *crtc; + int crtc_mask = 0; + + WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); + + if (intel_dp->is_mst) { + struct intel_dp_mst_encoder *mst_enc; + struct intel_connector *intel_connector; + struct drm_connector_state *conn_state; + int i; + + for (i = 0; i < ARRAY_SIZE(intel_dp->mst_encoders); i++) { +
[PATCH v6 08/10] drm/i915: Make intel_dp_mst_atomic_check() idempotent
The current way of handling changing VCPI allocations doesn't make a whole ton of sense. Since drm_atomic_helper_check_modeset() can be called multiple times which means that intel_dp_mst_atomic_check() can also be called multiple times. However, since we make the silly mistake of trying to free VCPI slots based off the /new/ atomic state instead of the old atomic state, we'll end up potentially double freeing the vcpi slots for the ports. This also has another unintended consequence that came back up while implementing MST fallback retraining: if a modeset is forced on a connector that's already part of the state, but it's atomic_check() has already been run once and doesn't get run again, we'll end up not freeing the VCPI allocations on the connector at all. The easiest way to solve this is to be clever and, with the exception of connectors that are being disabled and thus will never have compute_config() ran on them, move vcpi freeing out of the atomic check and into compute_config(). Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul --- drivers/gpu/drm/i915/intel_dp_mst.c | 78 +++-- 1 file changed, 57 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 40af938e2fe7..74b9ebd7b996 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -30,6 +30,38 @@ #include #include +static int +intel_dp_mst_atomic_release_vcpi_slots(struct drm_atomic_state *state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state) +{ + struct drm_crtc_state *new_crtc_state; + struct intel_crtc_state *intel_crtc_state = + to_intel_crtc_state(crtc_state); + struct drm_encoder *encoder; + struct drm_dp_mst_topology_mgr *mgr; + int slots, ret; + + slots = intel_crtc_state->dp_m_n.tu; + if (slots <= 0) + return 0; + + encoder = conn_state->best_encoder; + mgr = &enc_to_mst(encoder)->primary->dp.mst_mgr; + + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); + if (ret) { + DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", + slots, ret); + } else { + new_crtc_state = drm_atomic_get_crtc_state(state, + crtc_state->crtc); + to_intel_crtc_state(new_crtc_state)->dp_m_n.tu = 0; + } + + return ret; +} + static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -41,11 +73,15 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_atomic_state *state = pipe_config->base.state; + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(state, &connector->base); + struct drm_crtc *old_crtc; struct intel_dp_mst_topology_state *mst_state; int bpp; int slots; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; + int ret; bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_LIMITED_M_N); @@ -78,6 +114,17 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); pipe_config->pbn = mst_pbn; + /* Free any VCPI allocations on this connector from the previous +* state */ + old_crtc = old_conn_state->crtc; + if (old_crtc) { + ret = intel_dp_mst_atomic_release_vcpi_slots( + state, drm_atomic_get_old_crtc_state(state, old_crtc), + old_conn_state); + if (ret) + return ret; + } + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, connector->port, mst_pbn); if (slots < 0) { @@ -107,31 +154,20 @@ static int intel_dp_mst_atomic_check(struct drm_connector *connector, { struct drm_atomic_state *state = new_conn_state->state; struct drm_connector_state *old_conn_state; - struct drm_crtc *old_crtc; - struct drm_crtc_state *crtc_state; - int slots, ret = 0; + int ret = 0; old_conn_state = drm_atomic_get_old_connector_state(state, connector); - old_crtc = old_conn_state->crtc; - if (!old_crtc) - return ret; - crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc); -
[PATCH v6 07/10] drm/i915: Only use one link bw config for MST topologies
When a DP MST link needs retraining, sometimes the hub will detect that the current link bw config is impossible and will update it's RX caps in the DPCD to reflect the new maximum link rate. Currently, we make the assumption that the RX caps in the dpcd will never change like this. This means if the sink changes it's RX caps after we've already set up an MST link and we attempt to add or remove another sink from the topology, we could put ourselves into an invalid state where we've tried to configure different sinks on the same MST topology with different link rates. We could also run into this situation if a sink reports a higher link rate after suspend, usually from us having trained it with a fallback bw configuration before suspending. So: keep the link rate consistent by subclassing drm_dp_mst_topology_state, and tracking it there. For the time being, we only allow the link rate to change when the entire topology has been disconnected. V4: - Track link rate/lane count in the atomic topology state instead of in intel_dp. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp_mst.c | 77 + drivers/gpu/drm/i915/intel_drv.h| 7 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index cf844cfd2bb0..40af938e2fe7 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -41,8 +41,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_atomic_state *state = pipe_config->base.state; + struct intel_dp_mst_topology_state *mst_state; int bpp; - int lane_count, slots; + int slots; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc, @@ -55,18 +56,22 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, DRM_DEBUG_KMS("Setting pipe bpp to %d\n", bpp); } + + mst_state = to_intel_dp_mst_topology_state( + drm_atomic_dp_mst_get_topology_state(state, &intel_dp->mst_mgr)); /* * for MST we always configure max link bw - the spec doesn't * seem to suggest we should do otherwise. */ - lane_count = intel_dp_max_lane_count(intel_dp); - - pipe_config->lane_count = lane_count; + if (!mst_state->link_rate || !mst_state->lane_count) { + mst_state->link_rate = intel_dp_max_link_rate(intel_dp); + mst_state->lane_count = intel_dp_max_lane_count(intel_dp); + } + pipe_config->lane_count = mst_state->lane_count; + pipe_config->port_clock = mst_state->link_rate; pipe_config->pipe_bpp = bpp; - pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); - if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port)) pipe_config->has_audio = true; @@ -80,7 +85,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, return false; } - intel_link_compute_m_n(bpp, lane_count, + intel_link_compute_m_n(bpp, mst_state->lane_count, adjusted_mode->crtc_clock, pipe_config->port_clock, &pipe_config->dp_m_n, @@ -530,11 +535,55 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr) drm_kms_helper_hotplug_event(dev); } +static void intel_mst_reset_state(struct drm_dp_mst_topology_state *state) +{ + struct intel_dp_mst_topology_state *intel_mst_state = + to_intel_dp_mst_topology_state(state); + + intel_mst_state->link_rate = 0; + intel_mst_state->lane_count = 0; +} + static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = intel_dp_add_mst_connector, .register_connector = intel_dp_register_mst_connector, .destroy_connector = intel_dp_destroy_mst_connector, .hotplug = intel_dp_mst_hotplug, + .reset_state = intel_mst_reset_state, +}; + +static struct drm_private_state * +intel_dp_mst_duplicate_state(struct drm_private_obj *obj) +{ + struct intel_dp_mst_topology_state *state; + + state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_dp_mst_duplicate_topology_state( + to_dp_mst_topology_mgr(obj), &state->base); + + return &state->base.base; +} + +static void +intel_dp_mst_destroy_state(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct drm_dp_mst_topology_state *mst_state = + to_dp_mst_top
[PATCH v6 06/10] drm/dp_mst: Add reset_state callback to topology mgr
This gives drivers subclassing drm_dp_mst_topology_state the ability to prepare their topology states for a new hub to be connected whenever it's detected that the currently connected topology has been disconnected from the system. We'll need this in order to correctly track RX capabilities in i915 from the topology's atomic state. Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_dp_mst_topology.c | 11 +++ include/drm/drm_dp_mst_helper.h | 3 ++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 8a72eed0ae05..b55fd545d6a3 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2096,6 +2096,16 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, return true; } +static void drm_dp_mst_topology_reset_state(struct drm_dp_mst_topology_mgr *mgr) +{ + struct drm_dp_mst_topology_state *state = + to_dp_mst_topology_state(mgr->base.state); + + + if (mgr->cbs->reset_state) + mgr->cbs->reset_state(state); +} + /** * drm_dp_mst_topology_mgr_set_mst() - Set the MST state for a topology manager * @mgr: manager to set state for @@ -2171,6 +2181,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms mgr->payload_mask = 0; set_bit(0, &mgr->payload_mask); mgr->vcpi_mask = 0; + drm_dp_mst_topology_reset_state(mgr); } out_unlock: diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index b42922987470..fd856c371a6e 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -381,6 +381,7 @@ struct drm_dp_sideband_msg_tx { /* sideband msg handler */ struct drm_dp_mst_topology_mgr; +struct drm_dp_mst_topology_state; struct drm_dp_mst_topology_cbs { /* create a connector for a port */ struct drm_connector *(*add_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, const char *path); @@ -388,7 +389,7 @@ struct drm_dp_mst_topology_cbs { void (*destroy_connector)(struct drm_dp_mst_topology_mgr *mgr, struct drm_connector *connector); void (*hotplug)(struct drm_dp_mst_topology_mgr *mgr); - + void (*reset_state)(struct drm_dp_mst_topology_state *state); }; #define DP_MAX_PAYLOAD (sizeof(unsigned long) * 8) -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 03/10] drm/dp_mst: Fix naming on drm_atomic_get_mst_topology_state()
Since these functions are dealing with an atomic state that needs to be modified during atomic check and commit, change the naming on this function to drm_atomic_dp_mst_get_topology_state() since we're the only one using the function, and it's more consistent with the naming scheme used in drm_atomic.c/drm_atomic_helper.c. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/drm_dp_mst_topology.c | 13 +++-- include/drm/drm_dp_mst_helper.h | 5 +++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 6fac4129e6a2..ba67f1782a04 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2622,7 +2622,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_state *topology_state; int req_slots; - topology_state = drm_atomic_get_mst_topology_state(state, mgr); + topology_state = drm_atomic_dp_mst_get_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); @@ -2662,7 +2662,7 @@ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, { struct drm_dp_mst_topology_state *topology_state; - topology_state = drm_atomic_get_mst_topology_state(state, mgr); + topology_state = drm_atomic_dp_mst_get_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); @@ -3129,7 +3129,7 @@ static const struct drm_private_state_funcs mst_state_funcs = { }; /** - * drm_atomic_get_mst_topology_state: get MST topology state + * drm_atomic_dp_mst_get_topology_state: get MST topology state * * @state: global atomic state * @mgr: MST topology manager, also the private object in this case @@ -3143,15 +3143,16 @@ static const struct drm_private_state_funcs mst_state_funcs = { * * The MST topology state or error pointer. */ -struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr) +struct drm_dp_mst_topology_state * +drm_atomic_dp_mst_get_topology_state(struct drm_atomic_state *state, +struct drm_dp_mst_topology_mgr *mgr) { struct drm_device *dev = mgr->dev; WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); return to_dp_mst_topology_state(drm_atomic_get_private_obj_state(state, &mgr->base)); } -EXPORT_SYMBOL(drm_atomic_get_mst_topology_state); +EXPORT_SYMBOL(drm_atomic_dp_mst_get_topology_state); /** * drm_dp_mst_topology_mgr_init - initialise a topology manager diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 7f78d26a0766..41a8f08da05d 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -623,8 +623,9 @@ void drm_dp_mst_dump_topology(struct seq_file *m, void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr); int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr); -struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr); + +struct drm_dp_mst_topology_state *drm_atomic_dp_mst_get_topology_state(struct drm_atomic_state *state, + struct drm_dp_mst_topology_mgr *mgr); int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn); -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 09/10] drm/dp_mst: Add MST fallback retraining helpers
Unlike SST, MST can have far more then a single display hooked up on a single port. What this also means however, is that if the DisplayPort link to the top-level MST branch device becomes unstable then every single branch device also has an unstable link. Additionally, MST has a few more steps that must be taken in order to properly retrain the entire topology under a lower link rate. Since the VCPI allocations for each mstb is determined based off the link rate for the top of the topology, we also have to recalculate all of the VCPI allocations for the downstream ports as well to ensure that we still have enough link bandwidth to drive each mstb. Additionally, since we have multiple downstream connectors per port, setting the link status of the parent mstb's port to bad isn't enough: all of the downstream mstb ports have to have their link status set to bad. This basically follows the same paradigm that our DP link status logic in DRM does, in that we simply tell userspace that all of the mstb ports need retraining and additionally applying the new lower bandwidth constraints to all of the atomic commits on the topology that follow. Additionally; we add helpers that handle automatically checking whether or not a new atomic commit would perform the modesets required to retrain a link and if so, additionally handles updating the link status of each connector on the MST topology. V4: - clarify slightly confusing message in drm_dp_mst_topology_mgr_lower_link_rate() - Fix argument naming - Squash this with the other retrain helper, because now they're intertwined with one another - Track which connectors with CRTCs need modesets in order to retrain a topology in the topology's atomic state. This lets us greatly simplify the helpers, along with alleviating drivers of the responsibility of figuring out when to call the retrain helpers during atomic checks. It also ensures that we can keep zombie connectors that a retrain is dependent on alive until the topology either disappears, or they are disabled. We needed to do most of this anyway, since our original helpers didn't take into account that we need to invoke retraining when the link status prop changes, regardless of whether or not a modeset has been initiated yet. - Handle situation we completely forgot about: adding new connectors to an MST topology that needs fallback retraining (solution: new connectors on a topology inherit the link status of the rest of the topology) - Also make sure to handle connectors that are orphaned due to their MST topology disappearing. Solution: remove from topology state, reset link status to good - Write more docs on the retraining procedure. Signed-off-by: Lyude Paul Cc: Manasi Navare Cc: Ville Syrjälä --- drivers/gpu/drm/drm_dp_mst_topology.c | 438 +- include/drm/drm_dp_mst_helper.h | 20 ++ 2 files changed, 453 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index b55fd545d6a3..aca05b10bf18 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1199,6 +1199,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, if (created && !port->input) { char proppath[255]; + struct drm_dp_mst_topology_state *state; build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath)); port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr, port, proppath); @@ -1217,6 +1218,11 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_mode_connector_set_tile_property(port->connector); } + state = to_dp_mst_topology_state(port->mgr->base.state); + if (!list_empty(&state->bad_ports)) { + port->connector->state->link_status = + DRM_LINK_STATUS_BAD; + } (*mstb->mgr->cbs->register_connector)(port->connector); } @@ -2076,7 +2082,7 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, { switch (dp_link_bw) { default: - DRM_DEBUG_KMS("invalid link bandwidth in DPCD: %x (link count: %d)\n", + DRM_DEBUG_KMS("invalid link bandwidth: %x (link count: %d)\n", dp_link_bw, dp_link_count); return false; @@ -2096,11 +2102,408 @@ static bool drm_dp_get_vc_payload_bw(int dp_link_bw, return true; } +static int drm_dp_set_mstb_link_status(struct drm_dp_mst_topology_mgr *mgr, + struct drm_dp_mst_branch *mstb, + enum drm_link_status status) +{ + struct drm_dp_mst_topology_state *state = + to_dp_m
[PATCH v6 02/10] drm/i915: Move DP modeset retry work into intel_dp
While having the modeset_retry_work in intel_connector makes sense with SST, this paradigm doesn't make a whole ton of sense when it comes to MST since we have to deal with multiple connectors. In most cases, it's more useful to just use the intel_dp struct since it indicates whether or not we're dealing with an MST device, along with being able to easily trace the intel_dp struct back to it's respective connector (if there is any). So, move the modeset_retry_work function out of the intel_connector struct and into intel_dp. Signed-off-by: Lyude Paul Reviewed-by: Manasi Navare Cc: Manasi Navare Cc: Ville Syrjälä V2: - Remove accidental duplicate modeset_retry_work in intel_connector V3: - Also check against eDP in intel_hpd_poll_fini() - mdnavare V4: - Don't bother looping over connectors for canceling modeset rety work, just encoders. Signed-off-by: Lyude Paul --- drivers/gpu/drm/i915/intel_display.c | 14 +++--- drivers/gpu/drm/i915/intel_dp.c | 10 -- drivers/gpu/drm/i915/intel_dp_link_training.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 6 +++--- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e04050ea3e28..18edb9628a54 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15471,20 +15471,28 @@ void intel_connector_unregister(struct drm_connector *connector) static void intel_hpd_poll_fini(struct drm_device *dev) { - struct intel_connector *connector; struct drm_connector_list_iter conn_iter; + struct intel_connector *connector; + struct intel_encoder *encoder; + struct intel_dp *intel_dp; /* Kill all the work that may have been queued by hpd. */ drm_connector_list_iter_begin(dev, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { - if (connector->modeset_retry_work.func) - cancel_work_sync(&connector->modeset_retry_work); if (connector->hdcp_shim) { cancel_delayed_work_sync(&connector->hdcp_check_work); cancel_work_sync(&connector->hdcp_prop_work); } } drm_connector_list_iter_end(&conn_iter); + + for_each_intel_encoder(dev, encoder) { + if (encoder->type == INTEL_OUTPUT_DP || + encoder->type == INTEL_OUTPUT_EDP) { + intel_dp = enc_to_intel_dp(&encoder->base); + cancel_work_sync(&intel_dp->modeset_retry_work); + } + } } void intel_modeset_cleanup(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 62f82c4298ac..fbb467bc227d 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6249,12 +6249,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, static void intel_dp_modeset_retry_work_fn(struct work_struct *work) { - struct intel_connector *intel_connector; - struct drm_connector *connector; + struct intel_dp *intel_dp = container_of(work, typeof(*intel_dp), +modeset_retry_work); + struct drm_connector *connector = &intel_dp->attached_connector->base; - intel_connector = container_of(work, typeof(*intel_connector), - modeset_retry_work); - connector = &intel_connector->base; DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); @@ -6283,7 +6281,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, int type; /* Initialize the work for modeset in case of link train failure */ - INIT_WORK(&intel_connector->modeset_retry_work, + INIT_WORK(&intel_dp->modeset_retry_work, intel_dp_modeset_retry_work_fn); if (WARN(intel_dig_port->max_lanes < 1, diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index f59b59bb0a21..2cfa58ce1f95 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -340,7 +340,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) intel_dp->link_rate, intel_dp->lane_count)) /* Schedule a Hotplug Uevent to userspace to start modeset */ - schedule_work(&intel_connector->modeset_retry_work); + schedule_work(&intel_dp->modeset_retry_work); } else { DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d", intel_connector->base.base.id, dif
[PATCH v6 01/10] drm/atomic: Print debug message on atomic check failure
Does what it says on the label, it's a little confusing debugging atomic check failures otherwise. Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul --- drivers/gpu/drm/drm_atomic.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 7d25c42f22db..972a7e9634ab 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1705,8 +1705,11 @@ int drm_atomic_check_only(struct drm_atomic_state *state) if (config->funcs->atomic_check) ret = config->funcs->atomic_check(state->dev, state); - if (ret) + if (ret) { + DRM_DEBUG_ATOMIC("atomic driver check for %p failed: %d\n", +state, ret); return ret; + } if (!state->allow_modeset) { for_each_new_crtc_in_state(state, crtc, crtc_state, i) { -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 04/10] drm/dp_mst: Remove all evil duplicate state pointers
There's no reason to track the atomic state three times. Unfortunately, this is currently what we're doing, and even worse is that there is only one actually correct state pointer: the one in mst_state->base.state. mgr->state never seems to be used, along with the one in mst_state->state. This confused me for over 4 hours until I realized there was no magic behind these pointers. So, let's save everyone else from the trouble. Signed-off-by: Lyude Paul . Cc: Manasi Navare Cc: Ville Syrjälä Signed-off-by: Lyude Paul --- include/drm/drm_dp_mst_helper.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 41a8f08da05d..035963fbcd9d 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -409,7 +409,6 @@ struct drm_dp_payload { struct drm_dp_mst_topology_state { struct drm_private_state base; int avail_slots; - struct drm_atomic_state *state; struct drm_dp_mst_topology_mgr *mgr; }; @@ -497,11 +496,6 @@ struct drm_dp_mst_topology_mgr { */ int pbn_div; - /** -* @state: State information for topology manager -*/ - struct drm_dp_mst_topology_state *state; - /** * @funcs: Atomic helper callbacks */ -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 00/10] drm/i915: Implement proper fallback training for MST
Latest version of PW series 39642, hopefully this should also actually come up on intel-gfx and go through CI. No changes other than rebasing to the current drm-intel-next-queued Lyude Paul (10): drm/atomic: Print debug message on atomic check failure drm/i915: Move DP modeset retry work into intel_dp drm/dp_mst: Fix naming on drm_atomic_get_mst_topology_state() drm/dp_mst: Remove all evil duplicate state pointers drm/dp_mst: Make drm_dp_mst_topology_state subclassable drm/dp_mst: Add reset_state callback to topology mgr drm/i915: Only use one link bw config for MST topologies drm/i915: Make intel_dp_mst_atomic_check() idempotent drm/dp_mst: Add MST fallback retraining helpers drm/i915: Implement proper fallback training for MST drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c| 35 +- .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.h| 4 +- drivers/gpu/drm/drm_atomic.c | 5 +- drivers/gpu/drm/drm_dp_mst_topology.c | 548 - drivers/gpu/drm/i915/intel_ddi.c | 10 +- drivers/gpu/drm/i915/intel_display.c | 14 +- drivers/gpu/drm/i915/intel_dp.c| 361 ++ drivers/gpu/drm/i915/intel_dp_link_training.c | 6 +- drivers/gpu/drm/i915/intel_dp_mst.c| 177 +-- drivers/gpu/drm/i915/intel_drv.h | 20 +- drivers/gpu/drm/nouveau/nv50_display.c | 15 +- drivers/gpu/drm/radeon/radeon_dp_mst.c | 13 +- include/drm/drm_dp_mst_helper.h| 42 +- 14 files changed, 1086 insertions(+), 178 deletions(-) -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[VERY RFC 0/2] iommu: optionally skip attaching to a DMA domain
I've been struggling with a problem for a while and I haven't been able to come up with a clean solution. Rob convinced me to stop complaining and do _something_ and hopefully this can spur a good discussion. The scenario is basically this: The MSM GPU wants to manage its own IOMMU virtual address space in lieu of using the DMA API to do so. The most important reason is that when per-instance pagetables [1] are enabled each file descriptor uses its own pagetable which are dynamically switched between contexts. In conjunction with this we use a split pagetable scheme to allow global buffers be persistently mapped so even a single pagetable has multiple ranges that need to be utilized based on buffer type. Obviously the DMA API isn't suitable for this kind of specialized use. We want to push it off to the side, allocate our own domain, and use it directly with the IOMMU APIs. In a perfect world we would just not use the DMA API and attach our own domain and that would be the end of the story. Unfortunately this is not a perfect world. In order to switch the pagetable from the GPU the SoC has some special wiring so that the GPU can reprogram the TTBR0 register in the IOMMU to the appropriate value. For a variety of reasons the hardware is hardcoded to only be able to access context bank 0 in the SMMU device. Long story short; if the DMA domain is allocated during bus enumeration and attached to context bank 0 then by the time we get to the driver domain we have long since lost our chance to get the context bank we need. After going back and forth and trying a few possible options it seems like the "easiest" solution" is to skip attaching the DMA domain in the first place. But we still need to create a default domain and set up the IOMMU groups correctly so that when the GPU driver comes along it can attach the device and everything will Just Work (TM). Rob suggested that we should use a blacklist of devices that choose to not participate so thats what I'm offering as a starting point for discussion. The first patch in this series allows the specific IOMMU driver to gracefully skip attaching a device by returning -ENOSUPP. In that case, the core will skip printing error messages and it won't attach the domain to the group but it still allows the group to get created so that the IOMMU device is properly set up. In arch_setup_dma_ops() the call to iommu_get_domain_for_dev() will return NULL and the IOMMU ops won't be set up. The second patch implements the blacklist in arm-smmu.c based on the compatible string of the GPU device and returns -ENOTSUPP. I tested this with my current per-instance pagetable stack and it does the job but I am very much in the market for better ideas. [1] https://patchwork.freedesktop.org/series/38729/ Jordan Crouse (2): iommu: Gracefully allow drivers to not attach to a default domain iommu/arm-smmu: Add list of devices to opt out of DMA domains drivers/iommu/arm-smmu.c | 23 +++ drivers/iommu/iommu.c| 18 ++ 2 files changed, 37 insertions(+), 4 deletions(-) -- 2.16.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] iommu/arm-smmu: Add list of devices to opt out of DMA domains
Add a list of compatible strings for devices that wish to opt out of attaching to a DMA domain. This is for devices that prefer to manage their own IOMMU space for any number of reasons. Returning ENOTSUPP for attach device will filter down and force arch_setup_dma_ops() to not set up the iommu DMA ops. Later the client device in question can set up and attach their own domain. Signed-off-by: Jordan Crouse --- drivers/iommu/arm-smmu.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 69e7c60792a8..09fa03a15f4f 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1187,6 +1187,15 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain, return 0; } +/* + * This is a list of compatible strings for devices that wish to manage their + * own IOMMU space instead of the DMA IOMMU ops. Devices on this list will not + * allow themselves to be attached to a IOMMU_DOMAIN_DMA domain + */ +static const char * const arm_smmu_dma_blacklist[] = { + "qcom,adreno", +}; + static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) { int ret; @@ -1209,6 +1218,20 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) if (!fwspec->iommu_priv) return -ENODEV; + /* +* If this is the dfeault DMA domain, check to see if the device is on +* the blacklist and reject if so +*/ + if (domain->type == IOMMU_DOMAIN_DMA && dev->of_node) { + int i; + + for (i = 0; i < ARRAY_SIZE(arm_smmu_dma_blacklist); i++) { + if (of_device_is_compatible(dev->of_node, + arm_smmu_dma_blacklist[i])) + return -ENOTSUPP; + } + } + smmu = fwspec_smmu(fwspec); /* Ensure that the domain is finalised */ ret = arm_smmu_init_domain_context(domain, smmu); -- 2.16.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] iommu: Gracefully allow drivers to not attach to a default domain
Provide individual device drivers the chance to gracefully refuse to attach a device to the default domain. If the attach_device op returns -ENOTSUPP don't print a error message and don't set group->domain but still return success from iommu_group_add_dev(). This allows all the usual APIs to work and the next domain to try to attach will take group->domain for itself and everything will proceed as normal. Signed-off-by: Jordan Crouse --- drivers/iommu/iommu.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 69fef991c651..2403cce138d2 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -592,7 +592,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) if (group->domain) ret = __iommu_attach_device(group->domain, dev); mutex_unlock(&group->mutex); - if (ret) + if (ret && ret != -ENOTSUPP) goto err_put_group; /* Notify any listeners about change to group. */ @@ -617,7 +617,9 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) sysfs_remove_link(&dev->kobj, "iommu_group"); err_free_device: kfree(device); - pr_err("Failed to add device %s to group %d: %d\n", dev_name(dev), group->id, ret); + if (ret != -ENOTSUPP) + pr_err("Failed to add device %s to group %d: %d\n", + dev_name(dev), group->id, ret); return ret; } EXPORT_SYMBOL_GPL(iommu_group_add_device); @@ -1039,8 +1041,16 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev) ret = iommu_group_add_device(group, dev); if (ret) { - iommu_group_put(group); - return ERR_PTR(ret); + /* +* If the driver chooses not to bind the device, reset +* group->domain so a new domain can be added later +*/ + if (ret == -ENOTSUPP) + group->domain = NULL; + else { + iommu_group_put(group); + return ERR_PTR(ret); + } } return group; -- 2.16.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199357] amdgpu: hang a few seconds after logging in, most likely due to regression
https://bugzilla.kernel.org/show_bug.cgi?id=199357 --- Comment #4 from Christian König (christian.koe...@amd.com) --- Thanks, yeah that is clearly DC (display core). Harry can you take a look? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/amdgpu: Allow dma_map_sg() coalescing
Am 11.04.2018 um 19:11 schrieb Robin Murphy: Now that drm_prime_sg_to_page_addr_arrays() understands the case where dma_map_sg() has coalesced segments and returns 0 < count < nents, we can relax the check to only consider genuine failure. That pattern is repeated in pretty much all drivers using TTM. So you would need to fix all of them, but as I said I don't think that this approach is a good idea. We essentially wanted to get rid of the dma_address array in the mid term and that change goes into the exactly opposite direction. Regards, Christian. Signed-off-by: Robin Murphy --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 205da3ff9cd0..f81e96a4242f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -813,7 +813,7 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) r = -ENOMEM; nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction); - if (nents != ttm->sg->nents) + if (nents == 0) goto release_sg; drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/prime: Iterate SG DMA addresses separately
Am 11.04.2018 um 19:11 schrieb Robin Murphy: For dma_map_sg(), DMA API implementations are free to merge consecutive segments into a single DMA mapping if conditions are suitable, thus the resulting DMA addresses may be packed into fewer entries than ttm->sg->nents implies. drm_prime_sg_to_page_addr_arrays() does not account for this, meaning its callers either have to reject the 0 < count < nents case or risk getting bogus addresses back later. Fortunately this is relatively easy to deal with having to rejig structures to also store the mapped count, since the total DMA length should still be equal to the total buffer length. All we need is a separate scatterlist cursor to iterate the DMA addresses separately from the CPU addresses. Mhm, I think I like Sinas approach better. See the hardware actually needs the dma_address on a page by page basis. Joining multiple consecutive pages into one entry is just additional overhead which we don't need. Regards, Christian. Signed-off-by: Robin Murphy --- Off the back of Sinan's proposal for a workaround, I took a closer look and this jumped out - I have no hardware to test it, nor do I really know my way around this code, so I'm probably missing something, but at face value this seems like the only obvious problem, and worth fixing either way. These patches are based on drm-next, and compile-tested (for arm64) only. Robin. drivers/gpu/drm/drm_prime.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 7856a9b3f8a8..db3dc8489afc 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -933,16 +933,18 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, dma_addr_t *addrs, int max_entries) { unsigned count; - struct scatterlist *sg; + struct scatterlist *sg, *dma_sg; struct page *page; - u32 len, index; + u32 len, dma_len, index; dma_addr_t addr; index = 0; + dma_sg = sgt->sgl; + dma_len = sg_dma_len(dma_sg); + addr = sg_dma_address(dma_sg); for_each_sg(sgt->sgl, sg, sgt->nents, count) { len = sg->length; page = sg_page(sg); - addr = sg_dma_address(sg); while (len > 0) { if (WARN_ON(index >= max_entries)) @@ -957,6 +959,12 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, len -= PAGE_SIZE; index++; } + + if (dma_len == 0) { + dma_sg = sg_next(dma_sg); + dma_len = sg_dma_len(dma_sg); + addr = sg_dma_address(dma_sg); + } } return 0; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103246] PoE: GPU hang with mesa >= 17.2.0 + gallium-nine
https://bugs.freedesktop.org/show_bug.cgi?id=103246 --- Comment #2 from i...@yahoo.com --- I just sow that there is already issue opened for the same/similar issue: https://github.com/iXit/Mesa-3D/issues/296 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 103246] PoE: GPU hang with mesa >= 17.2.0 + gallium-nine
https://bugs.freedesktop.org/show_bug.cgi?id=103246 i...@yahoo.com changed: What|Removed |Added Keywords||regression --- Comment #1 from i...@yahoo.com --- The symptoms are typical for shader infinite loop, followed by botched driver restart. Definitely not Wine fault. Does the bug still happen with current mesa3d+llvm? While the game is free, it would still be good idea if you can create an apitrace recording of a place where the game crashes. Ask in #d3d9 at irc.freenode about the ftp access, or use google drive or similar file sharing. (We might use the trace for regression testing in future). It might be good idea if you also fill bugreport to the github Ixit/Mesa-3D issue tracker. I usually place the apitrace wrapper d3d9.dll inside the game directory (main or where game.exe is). Then using `winecfg` add library override for "d3d9" to be "native, built-in". If everything goes well, the wrapper would create a new trace inside the game directory every time the game is started. So don't forget to remove it when you are done. Naturally, use working mesa3d version for the trace. Then install new version of mesa and check if the trace crashes. If you can compile mesa3d on your own, you might be able to help narrow down the problem further. I cannot do that, since my card is using the R600 driver, so it is very unlikely to have the same shader miscompilation. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [1/2] drm/prime: Iterate SG DMA addresses separately
On 11/04/18 18:11, Robin Murphy wrote: For dma_map_sg(), DMA API implementations are free to merge consecutive segments into a single DMA mapping if conditions are suitable, thus the resulting DMA addresses may be packed into fewer entries than ttm->sg->nents implies. drm_prime_sg_to_page_addr_arrays() does not account for this, meaning its callers either have to reject the 0 < count < nents case or risk getting bogus addresses back later. Fortunately this is relatively easy to deal with having to rejig structures to also store the mapped count, since the total DMA length should still be equal to the total buffer length. All we need is a separate scatterlist cursor to iterate the DMA addresses separately from the CPU addresses. Signed-off-by: Robin Murphy --- Off the back of Sinan's proposal for a workaround, I took a closer look and this jumped out - I have no hardware to test it, nor do I really know my way around this code, so I'm probably missing something, but at face value this seems like the only obvious problem, and worth fixing either way. These patches are based on drm-next, and compile-tested (for arm64) only. Robin. drivers/gpu/drm/drm_prime.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 7856a9b3f8a8..db3dc8489afc 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -933,16 +933,18 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, dma_addr_t *addrs, int max_entries) { unsigned count; - struct scatterlist *sg; + struct scatterlist *sg, *dma_sg; struct page *page; - u32 len, index; + u32 len, dma_len, index; dma_addr_t addr; index = 0; + dma_sg = sgt->sgl; + dma_len = sg_dma_len(dma_sg); + addr = sg_dma_address(dma_sg); for_each_sg(sgt->sgl, sg, sgt->nents, count) { len = sg->length; page = sg_page(sg); - addr = sg_dma_address(sg); while (len > 0) { if (WARN_ON(index >= max_entries)) @@ -957,6 +959,12 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, len -= PAGE_SIZE; + dma_len -= PAGE_SIZE; Ugh, somehow that bit got lost. Told you I'd have missed something, although I was rather assuming it would be something less obvious... Robin. index++; } + + if (dma_len == 0) { + dma_sg = sg_next(dma_sg); + dma_len = sg_dma_len(dma_sg); + addr = sg_dma_address(dma_sg); + } } return 0; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199357] amdgpu: hang a few seconds after logging in, most likely due to regression
https://bugzilla.kernel.org/show_bug.cgi?id=199357 --- Comment #3 from Mathias Tillman (master.ho...@gmail.com) --- I've just finished running a bisect now, and I have concluded that commit 36cc549d59864b7161f0e23d710c1c4d1b9cf022 (drm/amd/display: disable CRTCs with NULL FB on their primary plane (V2)) causes the lock-up. Let me know if you need anything else. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105680] [CI] igt@kms_frontbuffer_tracking@* - fail - FBC disabled: mode too large for compression
https://bugs.freedesktop.org/show_bug.cgi?id=105680 --- Comment #8 from Jose Roberto de Souza --- Hi Marta did it ran? Should I ask someone else to review and merge? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/prime: Iterate SG DMA addresses separately
For dma_map_sg(), DMA API implementations are free to merge consecutive segments into a single DMA mapping if conditions are suitable, thus the resulting DMA addresses may be packed into fewer entries than ttm->sg->nents implies. drm_prime_sg_to_page_addr_arrays() does not account for this, meaning its callers either have to reject the 0 < count < nents case or risk getting bogus addresses back later. Fortunately this is relatively easy to deal with having to rejig structures to also store the mapped count, since the total DMA length should still be equal to the total buffer length. All we need is a separate scatterlist cursor to iterate the DMA addresses separately from the CPU addresses. Signed-off-by: Robin Murphy --- Off the back of Sinan's proposal for a workaround, I took a closer look and this jumped out - I have no hardware to test it, nor do I really know my way around this code, so I'm probably missing something, but at face value this seems like the only obvious problem, and worth fixing either way. These patches are based on drm-next, and compile-tested (for arm64) only. Robin. drivers/gpu/drm/drm_prime.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 7856a9b3f8a8..db3dc8489afc 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -933,16 +933,18 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, dma_addr_t *addrs, int max_entries) { unsigned count; - struct scatterlist *sg; + struct scatterlist *sg, *dma_sg; struct page *page; - u32 len, index; + u32 len, dma_len, index; dma_addr_t addr; index = 0; + dma_sg = sgt->sgl; + dma_len = sg_dma_len(dma_sg); + addr = sg_dma_address(dma_sg); for_each_sg(sgt->sgl, sg, sgt->nents, count) { len = sg->length; page = sg_page(sg); - addr = sg_dma_address(sg); while (len > 0) { if (WARN_ON(index >= max_entries)) @@ -957,6 +959,12 @@ int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, len -= PAGE_SIZE; index++; } + + if (dma_len == 0) { + dma_sg = sg_next(dma_sg); + dma_len = sg_dma_len(dma_sg); + addr = sg_dma_address(dma_sg); + } } return 0; } -- 2.16.1.dirty ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/amdgpu: Allow dma_map_sg() coalescing
Now that drm_prime_sg_to_page_addr_arrays() understands the case where dma_map_sg() has coalesced segments and returns 0 < count < nents, we can relax the check to only consider genuine failure. Signed-off-by: Robin Murphy --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 205da3ff9cd0..f81e96a4242f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -813,7 +813,7 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm) r = -ENOMEM; nents = dma_map_sg(adev->dev, ttm->sg->sgl, ttm->sg->nents, direction); - if (nents != ttm->sg->nents) + if (nents == 0) goto release_sg; drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages, -- 2.16.1.dirty ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 0/5] drm/blend: Support generic plane-wide alpha
Maxime Ripard writes: > Hi, > > This serie aims at enhancing the support for plane-wide alpha in the > drivers that are implementing it at the moment, by turning it into a > generic property and converting the drivers (rcar-du and atmel-hclcdc). It > also introduces support for it in the sun4i driver. > > Let me know what you think, > Maxime > > Changes from v5: > - Added back a comment about the pixel values not being pre-multiplied as > suggested by Laurent I like that comment. Thanks! signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104216] Firefox quirks (black and/or white squares)
https://bugs.freedesktop.org/show_bug.cgi?id=104216 --- Comment #24 from Germano Massullo --- Just got a burst of very interesting comments in Firefox bugreport from https://bugzilla.mozilla.org/show_bug.cgi?id=1421353#c19 to comment number 21 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: RFC for a render API to support adaptive sync and VRR
On 2018-04-10 06:26 PM, Cyr, Aric wrote:> > My guess is they prefer to “do nothing” and let driver/HW manage it, > otherwise you exempt all existing games from supporting adaptive sync > without a rewrite or update. Nobody is saying adaptive sync should only work with explicit target presentation times provided by the application. We're just arguing that target presentation time as a mechanism is superior to target refresh rate, both for video and game use cases. It also trivially allows emulating "as early as possible" (target presentation time = 0) and "fixed refresh rate" (target presentation time = start + i * target frame duration) behaviour, even transparently for the application. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2] drm/amdgpu: limit DMA size to PAGE_SIZE for scatter-gather buffers
On 11/04/18 15:33, Sinan Kaya wrote: On 4/11/2018 8:03 AM, Robin Murphy wrote: On 10/04/18 21:59, Sinan Kaya wrote: Code is expecing to observe the same number of buffers returned from dma_map_sg() function compared to sg_alloc_table_from_pages(). This doesn't hold true universally especially for systems with IOMMU. So why not fix said code? It's clearly not a real hardware limitation, and the map_sg() APIs have potentially returned fewer than nents since forever, so there's really no excuse. Sure, I'll take a better fix if there is one. IOMMU driver tries to combine buffers into a single DMA address as much as it can. The right thing is to tell the DMA layer how much combining IOMMU can do. Disagree; this is a dodgy hack, since you'll now end up passing scatterlists into dma_map_sg() which already violate max_seg_size to begin with, and I think a conscientious DMA API implementation would be at rights to fail the mapping for that reason (I know arm64 happens not to, but that was a deliberate design decision to make my life easier at the time). As a short-term fix, at least do something like what i915 does and constrain the table allocation to the desired segment size as well, so things remain self-consistent. But still never claim that faking a hardware constraint as a workaround for a driver shortcoming is "the right thing to do" ;) You are asking for something like this from here, right? https://elixir.bootlin.com/linux/v4.16.1/source/drivers/gpu/drm/i915/i915_gem_dmabuf.c#L58 I just looked for callers of __sg_alloc_table_from_pages() with a limit other than SCATTERLIST_MAX_SEGMENT, and found __i915_gem_userptr_alloc_pages(), which at first glance appears to be doing something vaguely similar to amdgpu_ttm_tt_pin_userptr(). Robin. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/imx: imx-ldb: disable LDB on driver bind
The LVDS signal integrity is only guaranteed when the correct enable sequence (first IPU DI, then LDB) is used. If the LDB display output was active before the imx-drm driver is loaded (like when a bootsplash was active) the DI will be disabled by the full IPU reset we do when loading the driver. The LDB control registers are not part of the IPU range and thus will remain unchanged. This leads to the LDB still being active when the DI is getting enabled, effectively reversing the required enable sequence. Fix this by also disabling the LDB on driver bind. Signed-off-by: Lucas Stach --- drivers/gpu/drm/imx/imx-ldb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 56dd7a9a8e25..17974c0b4be8 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -612,6 +612,9 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) return PTR_ERR(imx_ldb->regmap); } + /* disable LDB by resetting the control register to POR default */ + regmap_write(imx_ldb->regmap, IOMUXC_GPR2, 0); + imx_ldb->dev = dev; if (of_id) -- 2.16.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/imx: imx-ldb: check if channel is enabled before printing warning
If the second LVDS channel has been disabled in the DT when using dual-channel mode we should not print a warning. Signed-off-by: Lucas Stach --- drivers/gpu/drm/imx/imx-ldb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 17974c0b4be8..dd5312b02a8d 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -655,14 +655,14 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) if (ret || i < 0 || i > 1) return -EINVAL; + if (!of_device_is_available(child)) + continue; + if (dual && i > 0) { dev_warn(dev, "dual-channel mode, ignoring second output\n"); continue; } - if (!of_device_is_available(child)) - continue; - channel = &imx_ldb->channel[i]; channel->ldb = imx_ldb; channel->chno = i; -- 2.16.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2] drm/amdkfd: Remove vla
On 2018-04-10 09:02 PM, Laura Abbott wrote: > There's an ongoing effort to remove VLAs[1] from the kernel to eventually > turn on -Wvla. Switch to a constant value that covers all hardware. > > [1] https://lkml.org/lkml/2018/3/7/621 > > Signed-off-by: Laura Abbott > --- > v2: Switch to a larger size to account for other hardware > --- > drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c > b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c > index 035c351f47c5..c3a5a80e31ae 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c > @@ -139,10 +139,12 @@ static void interrupt_wq(struct work_struct *work) > { > struct kfd_dev *dev = container_of(work, struct kfd_dev, > interrupt_work); > + uint32_t ih_ring_entry[8]; > > - uint32_t ih_ring_entry[DIV_ROUND_UP( > - dev->device_info->ih_ring_entry_size, > - sizeof(uint32_t))]; > + if (dev->device_info->ih_ring_entry_size > (8 * sizeof(uint32_t))) { > + dev_err(kfd_chardev(), "Ring entry too small\n"); When the ring entry size really is too small, this will potentially flood the kernel log. Maybe a WARN_ONCE would be more helpful. With that fixed, this patch is Reviewed-by: Felix Kuehling Regards, Felix > + return; > + } > > while (dequeue_ih_ring_entry(dev, ih_ring_entry)) > dev->device_info->event_interrupt_class->interrupt_wq(dev, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/panel: simple: AUO P320HVN03 uses SPWG data ordering
The patch adding support for the AUO P320HVN03 panel was written against a preliminary datasheet, which specified JEIDA data ordering. Testing with real hardware has shown that the actually used data ordering is SPWG. Fixes: 70c0d5b783f5 (drm/panel: simple: add support for AUO P320HVN03) Signed-off-by: Lucas Stach --- drivers/gpu/drm/panel/panel-simple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 5591984a392b..bd15567ba484 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -664,7 +664,7 @@ static const struct panel_desc auo_p320hvn03 = { .enable = 450, .unprepare = 500, }, - .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, }; static const struct drm_display_mode auo_t215hvn01_mode = { -- 2.16.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 18/18] drm_hwcomposer: Flatten scene asynchronously
The steps for flattening a scene on a dedicated crtc are: 1. Find an available and unused crtc(the display connector is disconnected). 2. Copy layers from active composition. 3. Plan layers of copy on the unused crtc. This is realized by using a newly created DrmDisplayCompositor object. 4. Commit copy to the unsed crtc and get the result as a writeback_comp. 5. Copy the writeback comp and commit(if needed) to the display. The copying of the writeback comp is needed because the crtc might not be on the same dri node so the buffers will have to be re-imported. Signed-off-by: Alexandru Gheorghe --- drmdisplaycompositor.cpp | 173 ++- drmdisplaycompositor.h | 5 ++ 2 files changed, 177 insertions(+), 1 deletion(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index cb670e6..72d0226 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -973,6 +973,84 @@ int DrmDisplayCompositor::ApplyComposition( return ret; } +int DrmDisplayCompositor::FlattenOnDisplay( +std::unique_ptr &src, +std::unique_ptr &writeback, +DrmConnector *writeback_conn, DrmMode &src_mode) { + int ret = 0; + ret = writeback_conn->UpdateModes(); + if (ret) { +ALOGE("Failed to update modes %d", ret); +return ret; + } + for (const DrmMode &mode : writeback_conn->modes()) { +if (mode.h_display() == src_mode.h_display() && +mode.v_display() == src_mode.v_display()) { + writeback->SetDisplayMode(mode); + mode_.mode = mode; + if (mode_.blob_id) +drm_->DestroyPropertyBlob(mode_.blob_id); + std::tie(ret, mode_.blob_id) = CreateModeBlob(mode_.mode); + if (ret) { +ALOGE("Failed to create mode blob for display %d", display_); +return ret; + } + mode_.needs_modeset = true; + break; +} + } + if (mode_.blob_id <= 0) { +ALOGE("Failed to find similar mode"); +return -EINVAL; + } + + std::vector primary_planes; + std::vector overlay_planes; + SquashState squash_state; + DrmCrtc *crtc = drm_->GetCrtcForDisplay(display_); + if (!crtc) { +ALOGE("Failed to find crtc for display %d", display_); +return -EINVAL; + } + + // TODO what happens if planes could go to both CRTCs, don't think it's + // handled anywhere + for (auto &plane : drm_->planes()) { +if (!plane->GetCrtcSupported(*crtc)) + continue; +if (plane->type() == DRM_PLANE_TYPE_PRIMARY) + primary_planes.push_back(plane.get()); +else if (plane->type() == DRM_PLANE_TYPE_OVERLAY) + overlay_planes.push_back(plane.get()); + } + ret = src->Plan(&squash_state, &primary_planes, &overlay_planes); + if (ret) { +ALOGE("Failed to plan the composition ret = %d", ret); +return ret; + } + if (src->squash_regions().size() > 0) { +ALOGE("Abandon we don't want to fire up the GPU"); +return -EINVAL; + } + + // Disable the planes we're not using + for (auto i = primary_planes.begin(); i != primary_planes.end();) { +src->AddPlaneDisable(*i); +i = primary_planes.erase(i); + } + for (auto i = overlay_planes.begin(); i != overlay_planes.end();) { +src->AddPlaneDisable(*i); +i = overlay_planes.erase(i); + } + + ret = WritebackComposite(src.get(), writeback.get(), writeback_conn); + if (ret) { +ALOGE("Failed to writeback ret=%d", ret); +return ret; + } + return 0; +} + int DrmDisplayCompositor::WritebackComposite(DrmDisplayComposition *src, DrmDisplayComposition *dst, DrmConnector *writeback_conn) { @@ -1088,6 +1166,97 @@ int DrmDisplayCompositor::FlattenSynchronously(DrmConnector *writeback_conn) { return 0; } +int DrmDisplayCompositor::FlattenAsynchronously( +DrmConnector *writeback_conn) { + if (writeback_conn->display() == display_) { +ALOGE("Cannot flatten asynchronously on the same display"); +return -EINVAL; + } + ALOGI("FlattenAsynchronously on a different display"); + int ret = 0; + ResourceManager *resource_manager = drm_->resource_manager(); + DrmResources *drm_resource = + resource_manager->GetDrmResources(writeback_conn->display()); + if (!drm_resource) { +ALOGE("Failed to find resources for display = %d", + writeback_conn->display()); +return -EINVAL; + } + DrmDisplayCompositor drmdisplaycompositor; + ret = drmdisplaycompositor.Init(drm_resource, writeback_conn->display()); + if (ret) { +ALOGE("Failed to init drmdisplaycompositor = %d", ret); + } + /* Copy of the active_composition, needed because of two things: + * 1) Not to hold the lock for the whole time we are accessing + *active_composition + * 2) Will be committed on a crtc that might not be on the same + *dri node, so buffers need to be imported on the right node. + */ + std::unique_ptr copy_comp = + drmdisplaycompositor.CreateInitializedComposition(); + /* Flattened composition with on
[PATCH hwc v2 16/18] drm_hwcomposer: Find writeback connector for scene flattening
Add logic for finding a suitable writeback connector, there are two possibilities for finding an usable writeback connector: 1) Attached to the same CRTC as the display and can function concurrently with the display connector. 2) On a different CRTC and the display connector is not used (state != DRM_MODE_CONNECTED). What's not handled here and should be handle is what happens if connector changes state while flattening, but since hotplug is not wired yet, it's not something we should worry about. Signed-off-by: Alexandru Gheorghe --- drmresources.cpp| 25 + drmresources.h | 2 +- resourcemanager.cpp | 24 resourcemanager.h | 1 + 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/drmresources.cpp b/drmresources.cpp index fef6835..70126a4 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -269,6 +269,31 @@ DrmConnector *DrmResources::GetWritebackConnectorForDisplay(int display) const { return NULL; } +// TODO what happens when hotplugging +DrmConnector *DrmResources::AvailableWritebackConnector(int display) const { + DrmConnector *writeback_conn = GetWritebackConnectorForDisplay(display); + DrmConnector *display_conn = GetConnectorForDisplay(display); + // If we have a writeback already attached to the same CRTC, just use that, if + // possible + if (display_conn && writeback_conn && + writeback_conn->encoder()->can_clone(display_conn->encoder())) +return writeback_conn; + + // Use another CRTC if available and doesn't have any connector + for (auto &crtc : crtcs_) { +if (crtc->display() == display) + continue; +display_conn = GetConnectorForDisplay(crtc->display()); +// If we have a display connected don't use it for writeback +if (display_conn && display_conn->state() == DRM_MODE_CONNECTED) + continue; +writeback_conn = GetWritebackConnectorForDisplay(crtc->display()); +if (writeback_conn) + return writeback_conn; + } + return NULL; +} + DrmCrtc *DrmResources::GetCrtcForDisplay(int display) const { for (auto &crtc : crtcs_) { if (crtc->display() == display) diff --git a/drmresources.h b/drmresources.h index 4fb17fc..9176b8e 100644 --- a/drmresources.h +++ b/drmresources.h @@ -60,7 +60,7 @@ class DrmResources { DrmConnector *GetConnectorForDisplay(int display) const; DrmConnector *GetWritebackConnectorForDisplay(int display) const; - DrmConnector *FindWritebackConnector(int display) const; + DrmConnector *AvailableWritebackConnector(int display) const; DrmCrtc *GetCrtcForDisplay(int display) const; DrmPlane *GetPlane(uint32_t id) const; DrmEventListener *event_listener(); diff --git a/resourcemanager.cpp b/resourcemanager.cpp index e7b654e..b2a4458 100644 --- a/resourcemanager.cpp +++ b/resourcemanager.cpp @@ -49,6 +49,30 @@ int ResourceManager::Init() { (const hw_module_t **)&gralloc_); } +DrmConnector *ResourceManager::AvailableWritebackConnector(int display) { + DrmResources *drm_resource = GetDrmResources(display); + DrmConnector *writeback_conn = NULL; + if (drm_resource) { +writeback_conn = drm_resource->AvailableWritebackConnector(display); +if (writeback_conn) { + ALOGI("Use writeback connected to display %d\n", +writeback_conn->display()); + return writeback_conn; +} + } + for (auto &drm : drms_) { +if (drm.get() == drm_resource) + continue; +writeback_conn = drm->AvailableWritebackConnector(display); +if (writeback_conn) { + ALOGI("Use writeback connected to display %d\n", +writeback_conn->display()); + return writeback_conn; +} + } + return writeback_conn; +} + DrmResources *ResourceManager::GetDrmResources(int display) { for (uint32_t i = 0; i < drms_.size(); i++) { if (drms_[i]->HandlesDisplay(display)) diff --git a/resourcemanager.h b/resourcemanager.h index b8caa9a..57f7a2a 100644 --- a/resourcemanager.h +++ b/resourcemanager.h @@ -18,6 +18,7 @@ class ResourceManager { DrmResources *GetDrmResources(int display); std::shared_ptr GetImporter(int display); const gralloc_module_t *GetGralloc(); + DrmConnector *AvailableWritebackConnector(int display); private: std::vector> drms_; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 17/18] drm_hwcomposer: Flatten scene synchronously
Flatten scene on the same CRTC as the one driving the display. The active composition is played back to the display with a buffer attached to the writeback connector. Then we build a composition that has only one plane enabled and that uses the result of the writeback as the input. Signed-off-by: Alexandru Gheorghe --- drmdisplaycompositor.cpp | 203 +-- drmdisplaycompositor.h | 7 +- 2 files changed, 204 insertions(+), 6 deletions(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index e535e8a..cb670e6 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -36,6 +36,7 @@ #include "drmplane.h" #include "drmresources.h" #include "glworker.h" +static const uint32_t kWaitWritebackFence = 100; // ms namespace android { @@ -523,7 +524,9 @@ int DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) { } int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, - bool test_only) { + bool test_only, + DrmDisplayComposition *writeback_comp, + DrmConnector *writeback_conn) { ATRACE_CALL(); int ret = 0; @@ -532,6 +535,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, std::vector &comp_planes = display_comp->composition_planes(); uint64_t out_fences[drm_->crtcs().size()]; + int writeback_fence = -1; DrmConnector *connector = drm_->GetConnectorForDisplay(display_); if (!connector) { @@ -550,9 +554,37 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, return -ENOMEM; } + if (writeback_comp != NULL) { +if (writeback_conn == NULL) + return -EINVAL; +if (writeback_conn->writeback_fb_id().id() == 0 || +writeback_conn->writeback_out_fence().id() == 0) { + ALOGE("Writeback properties don't exit"); + return -EINVAL; +} +if (writeback_comp->layers().size() != 1) { + ALOGE("Invalid number of layers for writeback composition"); + return -EINVAL; +} +ret = drmModeAtomicAddProperty( +pset, writeback_conn->id(), writeback_conn->writeback_fb_id().id(), +writeback_comp->layers().back().buffer->fb_id); +if (ret < 0) { + ALOGE("Failed to add writeback_fb_id"); + return ret; +} +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(), + writeback_conn->writeback_out_fence().id(), + (uint64_t)&writeback_fence); +if (ret < 0) { + ALOGE("Failed to add writeback_out_fence"); + return ret; +} + } if (crtc->out_fence_ptr_property().id() != 0) { -ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->out_fence_ptr_property().id(), - (uint64_t) &out_fences[crtc->pipe()]); +ret = drmModeAtomicAddProperty(pset, crtc->id(), + crtc->out_fence_ptr_property().id(), + (uint64_t)&out_fences[crtc->pipe()]); if (ret < 0) { ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret); drmModeAtomicFree(pset); @@ -580,6 +612,15 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, } } + if (writeback_conn != NULL) { +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(), + writeback_conn->crtc_id_property().id(), + crtc->id()); +if (ret < 0) { + ALOGE("Failed to attach writeback"); +} + } + for (DrmCompositionPlane &comp_plane : comp_planes) { DrmPlane *plane = comp_plane.plane(); DrmCrtc *crtc = comp_plane.crtc(); @@ -729,8 +770,18 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, if (!ret) { uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET; -if (test_only) +if (test_only) { flags |= DRM_MODE_ATOMIC_TEST_ONLY; +} else { + if (writeback_comp != NULL) { +if (!CountdownExpired() && active_composition_) { + ALOGE("Writeback composition not needed, abort commit"); + drmModeAtomicFree(pset); + return -EINVAL; +}; +flags |= DRM_MODE_ATOMIC_NONBLOCK; + } +} ret = drmModeAtomicCommit(drm_->fd(), pset, flags, drm_); if (ret) { @@ -769,6 +820,13 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, if (crtc->out_fence_ptr_property().id()) { display_comp->set_out_fence((int) out_fences[crtc->pipe()]); } + if (writeback_fence >= 0) { +if (writeback_comp->layers().size() != 1) { + ALOGE("Invalid numbers of layer for writeback_comp"); + return -EINVAL; +} +writeback_comp->layers()[0].acquire_fence.Set(writeback_fence); + } return ret; } @@ -837,6 +895
[PATCH hwc v2 15/18] drm_hwcomposer: Add worker to trigger scene flattenning
Add a vsync worker that calls back into the DrmDisplayCompositor, for now at every 60 vsyncs if the scene does not change we trigger the flattening of the scene using the writeback connector. Other, more complex and proper heuristics could be implemented later on. Signed-off-by: Alexandru Gheorghe --- drmdisplaycompositor.cpp | 45 ++--- drmdisplaycompositor.h | 12 +++- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index 576539b..e535e8a 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -39,6 +39,20 @@ namespace android { +class CompositorVsyncCallback : public VsyncCallback { + public: + CompositorVsyncCallback(DrmDisplayCompositor *compositor) + : compositor_(compositor) { + } + + void Callback(int display, int64_t timestamp) { +compositor_->Vsync(display, timestamp); + } + + private: + DrmDisplayCompositor *compositor_; +}; + void SquashState::Init(DrmHwcLayer *layers, size_t num_layers) { generation_number_++; valid_history_ = 0; @@ -183,7 +197,8 @@ DrmDisplayCompositor::DrmDisplayCompositor() framebuffer_index_(0), squash_framebuffer_index_(0), dump_frames_composited_(0), - dump_last_timestamp_ns_(0) { + dump_last_timestamp_ns_(0), + flatten_countdown_(FLATTEN_COUNTDOWN_INIT) { struct timespec ts; if (clock_gettime(CLOCK_MONOTONIC, &ts)) return; @@ -193,7 +208,7 @@ DrmDisplayCompositor::DrmDisplayCompositor() DrmDisplayCompositor::~DrmDisplayCompositor() { if (!initialized_) return; - + vsync_worker_.Exit(); int ret = pthread_mutex_lock(&lock_); if (ret) ALOGE("Failed to acquire compositor lock %d", ret); @@ -222,7 +237,9 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int display) { return ret; } planner_ = Planner::CreateInstance(drm); - + vsync_worker_.Init(drm_, display_); + auto callback = std::make_shared(this); + vsync_worker_.RegisterCallback(callback); initialized_ = true; return 0; } @@ -896,6 +913,10 @@ int DrmDisplayCompositor::ApplyComposition( return ret; } +int DrmDisplayCompositor::FlattenScene() { + return -EINVAL; +} + int DrmDisplayCompositor::SquashAll() { AutoLock lock(&lock_, "compositor"); int ret = lock.Lock(); @@ -1044,6 +1065,24 @@ move_layers_back: return ret; } +bool DrmDisplayCompositor::CountdownExpired() const { + return flatten_countdown_ <= 0; +} + +void DrmDisplayCompositor::Vsync(int display, int64_t timestamp) { + AutoLock lock(&lock_, __FUNCTION__); + if (lock.Lock()) +return; + flatten_countdown_--; + if (CountdownExpired()) { +lock.Unlock(); +int ret = FlattenScene(); +ALOGI("scene flattening triggered for display %d at timestamp %" PRIu64 + " result = %d \n", + display, timestamp, ret); + } +} + void DrmDisplayCompositor::Dump(std::ostringstream *out) const { int ret = pthread_mutex_lock(&lock_); if (ret) diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h index b35ef70..26201b9 100644 --- a/drmdisplaycompositor.h +++ b/drmdisplaycompositor.h @@ -29,11 +29,16 @@ #include #include +#include // One for the front, one for the back, and one for cases where we need to // squash a frame that the hw can't display with hw overlays. #define DRM_DISPLAY_BUFFERS 3 +// If a scene is still for this number of vblanks flatten it to reduce power +// consumption. +#define FLATTEN_COUNTDOWN_INIT 60 + namespace android { class GLWorkerCompositor; @@ -92,7 +97,7 @@ class DrmDisplayCompositor { int Composite(); int SquashAll(); void Dump(std::ostringstream *out) const; - + void Vsync(int display, int64_t timestamp); std::tuple GetActiveModeResolution(); SquashState *squash_state() { @@ -128,6 +133,9 @@ class DrmDisplayCompositor { void ClearDisplay(); void ApplyFrame(std::unique_ptr composition, int status, bool writeback = false); + int FlattenScene(); + + bool CountdownExpired() const; std::tuple CreateModeBlob(const DrmMode &mode); @@ -157,6 +165,8 @@ class DrmDisplayCompositor { // we need to reset them on every Dump() call. mutable uint64_t dump_frames_composited_; mutable uint64_t dump_last_timestamp_ns_; + VSyncWorker vsync_worker_; + int64_t flatten_countdown_; std::unique_ptr planner_; }; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 12/18] drm_hwcomposer: Add utility function to create an initialized composition
There is a lot of boilerplate for creating an initialized drmdisplaycomposition. This patch gathers that in a separate method. Signed-off-by: Alexandru Gheorghe --- drmdisplaycompositor.cpp | 23 +++ drmdisplaycompositor.h | 2 ++ 2 files changed, 25 insertions(+) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index e556e86..6e5be24 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -221,6 +221,7 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int display) { ALOGE("Failed to initialize drm compositor lock %d\n", ret); return ret; } + planner_ = Planner::CreateInstance(drm); initialized_ = true; return 0; @@ -231,6 +232,28 @@ std::unique_ptr DrmDisplayCompositor::CreateComposition() return std::unique_ptr(new DrmDisplayComposition()); } +std::unique_ptr +DrmDisplayCompositor::CreateInitializedComposition() const { + DrmCrtc *crtc = drm_->GetCrtcForDisplay(display_); + if (!crtc) { +ALOGE("Failed to find crtc for display = %d", display_); +return std::unique_ptr(); + } + std::unique_ptr comp = CreateComposition(); + std::shared_ptr importer = + drm_->resource_manager()->GetImporter(display_); + if (!importer) { +ALOGE("Failed to find resources for display = %d", display_); +return std::unique_ptr(); + } + int ret = comp->Init(drm_, crtc, importer.get(), planner_.get(), 0); + if (ret) { +ALOGE("Failed to init composition for display = %d", display_); +return std::unique_ptr(); + } + return comp; +} + std::tuple DrmDisplayCompositor::GetActiveModeResolution() { DrmConnector *connector = drm_->GetConnectorForDisplay(display_); diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h index f1965fb..ccaffb4 100644 --- a/drmdisplaycompositor.h +++ b/drmdisplaycompositor.h @@ -87,6 +87,7 @@ class DrmDisplayCompositor { int Init(DrmResources *drm, int display); std::unique_ptr CreateComposition() const; + std::unique_ptr CreateInitializedComposition() const; int ApplyComposition(std::unique_ptr composition); int Composite(); int SquashAll(); @@ -155,6 +156,7 @@ class DrmDisplayCompositor { // we need to reset them on every Dump() call. mutable uint64_t dump_frames_composited_; mutable uint64_t dump_last_timestamp_ns_; + std::unique_ptr planner_; }; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 14/18] drm_hwcomposer: Fix race in ApplyFrame
ApplyFrame holds the lock just when it swaps the value of active_composition_, in a multithread context we could end up in a situation where something is shown on the screen, but something else is set in active_composition_. Fix it by holding the lock during CommitFrame. Signed-off-by: Alexandru Gheorghe --- drmdisplaycompositor.cpp | 40 +--- drmdisplaycompositor.h | 2 +- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index afd3b05..576539b 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -791,11 +791,6 @@ std::tuple DrmDisplayCompositor::CreateModeBlob( } void DrmDisplayCompositor::ClearDisplay() { - AutoLock lock(&lock_, "compositor"); - int ret = lock.Lock(); - if (ret) -return; - if (!active_composition_) return; @@ -808,11 +803,25 @@ void DrmDisplayCompositor::ClearDisplay() { } void DrmDisplayCompositor::ApplyFrame( -std::unique_ptr composition, int status) { +std::unique_ptr composition, int status, +bool writeback) { + AutoLock lock(&lock_, __FUNCTION__); + if (lock.Lock()) +return; int ret = status; - - if (!ret) + if (!ret) { +if (writeback && !CountdownExpired()) { + ALOGE("Abort playing back scene"); + return; +} ret = CommitFrame(composition.get(), false); +if (!ret) { + ++dump_frames_composited_; + if (active_composition_) +active_composition_->SignalCompositionDone(); + active_composition_.swap(composition); +} + } if (ret) { ALOGE("Composite failed for display %d", display_); @@ -821,21 +830,6 @@ void DrmDisplayCompositor::ApplyFrame( ClearDisplay(); return; } - ++dump_frames_composited_; - - if (active_composition_) -active_composition_->SignalCompositionDone(); - - ret = pthread_mutex_lock(&lock_); - if (ret) -ALOGE("Failed to acquire lock for active_composition swap"); - - active_composition_.swap(composition); - - if (!ret) -ret = pthread_mutex_unlock(&lock_); - if (ret) -ALOGE("Failed to release lock for active_composition swap"); } int DrmDisplayCompositor::ApplyComposition( diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h index 0f8daad..b35ef70 100644 --- a/drmdisplaycompositor.h +++ b/drmdisplaycompositor.h @@ -127,7 +127,7 @@ class DrmDisplayCompositor { void ClearDisplay(); void ApplyFrame(std::unique_ptr composition, - int status); + int status, bool writeback = false); std::tuple CreateModeBlob(const DrmMode &mode); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 11/18] drm_hwcomposer: Add utility functions to copy displaycomposition internals
Add utility functions to copy the DrmHwcLayer and DrmCompositionPlanes from another DrmDisplayComposition. Signed-off-by: Alexandru Gheorghe --- drmdisplaycomposition.cpp | 29 + drmdisplaycomposition.h | 3 +++ 2 files changed, 32 insertions(+) diff --git a/drmdisplaycomposition.cpp b/drmdisplaycomposition.cpp index 66e67a4..dd64f46 100644 --- a/drmdisplaycomposition.cpp +++ b/drmdisplaycomposition.cpp @@ -99,6 +99,35 @@ int DrmDisplayComposition::SetLayers(DrmHwcLayer *layers, size_t num_layers, return 0; } +int DrmDisplayComposition::CopyLayers(DrmDisplayComposition *src) { + geometry_changed_ = true; + type_ = DRM_COMPOSITION_TYPE_FRAME; + std::shared_ptr importer = + drm_->resource_manager()->GetImporter(crtc()->display()); + if (!importer) { +ALOGE("Failed to find a valid importer"); +return -EINVAL; + } + for (DrmHwcLayer &src_layer : src->layers()) { +DrmHwcLayer copy; +copy.PopulateFromDrmHwcLayer(&src_layer); +int ret = copy.ImportBuffer(importer.get(), +drm_->resource_manager()->GetGralloc()); +if (ret) { + ALOGE("Failed to import buffer ret = %d", ret); + return -EINVAL; +} +layers_.emplace_back(std::move(copy)); + } + return 0; +} + +void DrmDisplayComposition::CopyCompPlanes(DrmDisplayComposition *src) { + for (auto comp_plane : src->composition_planes()) { +composition_planes_.push_back(comp_plane); + } +} + int DrmDisplayComposition::SetDpmsMode(uint32_t dpms_mode) { if (!validate_composition_type(DRM_COMPOSITION_TYPE_DPMS)) return -EINVAL; diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h index 9183925..c646420 100644 --- a/drmdisplaycomposition.h +++ b/drmdisplaycomposition.h @@ -68,6 +68,7 @@ class DrmCompositionPlane { DrmCompositionPlane() = default; DrmCompositionPlane(DrmCompositionPlane &&rhs) = default; + DrmCompositionPlane(const DrmCompositionPlane &rhs) = default; DrmCompositionPlane &operator=(DrmCompositionPlane &&other) = default; DrmCompositionPlane(Type type, DrmPlane *plane, DrmCrtc *crtc) : type_(type), plane_(plane), crtc_(crtc) { @@ -120,6 +121,8 @@ class DrmDisplayComposition { Planner *planner, uint64_t frame_no); int SetLayers(DrmHwcLayer *layers, size_t num_layers, bool geometry_changed); + int CopyLayers(DrmDisplayComposition *src); + void CopyCompPlanes(DrmDisplayComposition *src); int AddPlaneComposition(DrmCompositionPlane plane); int AddPlaneDisable(DrmPlane *plane); int SetDpmsMode(uint32_t dpms_mode); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 13/18] drm_hwcomposer: Pass buffer sizes to Prepareframebuffer
Currently Prepareframebuffer uses the mode of the connected connector to decide how big the buffer should be, however when using the drmdisplaycompositor just for flattening, the mode had not been set yet, so we need a way to pass the desired buffer sizes. Signed-off-by: Alexandru Gheorghe --- drmdisplaycompositor.cpp | 7 --- drmdisplaycompositor.h | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index 6e5be24..afd3b05 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -268,14 +268,15 @@ DrmDisplayCompositor::GetActiveModeResolution() { } int DrmDisplayCompositor::PrepareFramebuffer( -DrmFramebuffer &fb, DrmDisplayComposition *display_comp) { +DrmFramebuffer &fb, DrmDisplayComposition *display_comp, uint32_t width, +uint32_t height) { int ret = fb.WaitReleased(-1); if (ret) { ALOGE("Failed to wait for framebuffer release %d", ret); return ret; } - uint32_t width, height; - std::tie(width, height, ret) = GetActiveModeResolution(); + if (width == 0 || height == 0) +std::tie(width, height, ret) = GetActiveModeResolution(); if (ret) { ALOGE( "Failed to allocate framebuffer because the display resolution could " diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h index ccaffb4..0f8daad 100644 --- a/drmdisplaycompositor.h +++ b/drmdisplaycompositor.h @@ -115,7 +115,8 @@ class DrmDisplayCompositor { static const int kAcquireWaitTimeoutMs = 100; int PrepareFramebuffer(DrmFramebuffer &fb, - DrmDisplayComposition *display_comp); + DrmDisplayComposition *display_comp, + uint32_t width = 0, uint32_t height = 0); int ApplySquash(DrmDisplayComposition *display_comp); int ApplyPreComposite(DrmDisplayComposition *display_comp); int PrepareFrame(DrmDisplayComposition *display_comp); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 07/18] drm_hwcomposer: Add display field to Drmencoder
In the current implementation TryEncoderForDisplay just looks at the crtc linked to the display, if that's not assigned to a display it means the encoder could be used, otherwise iterate to the list of possible_crtcs and find one which is not used. This logic works fine when you have just one encoder connected to a crtc but with two or more, like is the case when we attach a writeback connector, we need to know if we already assigned the encoder to a display. Signed-off-by: Alexandru Gheorghe --- drmencoder.cpp | 14 ++ drmencoder.h | 4 2 files changed, 18 insertions(+) diff --git a/drmencoder.cpp b/drmencoder.cpp index 3d762f3..1da7ec3 100644 --- a/drmencoder.cpp +++ b/drmencoder.cpp @@ -27,6 +27,7 @@ DrmEncoder::DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc, const std::vector &possible_crtcs) : id_(e->encoder_id), crtc_(current_crtc), + display_(-1), possible_crtcs_(possible_crtcs) { } @@ -40,5 +41,18 @@ DrmCrtc *DrmEncoder::crtc() const { void DrmEncoder::set_crtc(DrmCrtc *crtc) { crtc_ = crtc; + set_display(crtc->display()); +} + +int DrmEncoder::display() const { + return display_; +} + +void DrmEncoder::set_display(int display) { + display_ = display; +} + +bool DrmEncoder::can_bind(int display) const { + return display_ == -1 || display_ == display; } } diff --git a/drmencoder.h b/drmencoder.h index 58ccbfb..7e06691 100644 --- a/drmencoder.h +++ b/drmencoder.h @@ -36,6 +36,9 @@ class DrmEncoder { DrmCrtc *crtc() const; void set_crtc(DrmCrtc *crtc); + bool can_bind(int display) const; + void set_display(int display); + int display() const; const std::vector &possible_crtcs() const { return possible_crtcs_; @@ -44,6 +47,7 @@ class DrmEncoder { private: uint32_t id_; DrmCrtc *crtc_; + int display_; std::vector possible_crtcs_; }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 09/18] drm_hwcomposer: Handle writeback connectors
When writeback connectors are available assign them to displays, in order to be able to use them for flattening of the current displayed scene. The pipeline for each display will look like this: CRTC encoder display connector. |--- writeback enc -- writeback connector. However, the writeback connector will be later used/enabled only if one of the following conditions are met: - Could be a clone of the display connector, as pointed by the possible_clones property. - The display_connector is disconnected, so we are safe to use it for flattening the scene that's already presented on another display. Signed-off-by: Alexandru Gheorghe --- drmresources.cpp | 62 ++-- drmresources.h | 3 +++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/drmresources.cpp b/drmresources.cpp index 39f50be..fef6835 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -64,6 +64,14 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, return ret; } +#ifdef DRM_CLIENT_CAP_WRITEBACK_CONNECTORS + ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1); + if (ret) { +ALOGI("Failed to set writeback cap %d", ret); +ret = 0; + } +#endif + drmModeResPtr res = drmModeGetResources(fd()); if (!res) { ALOGE("Failed to get DrmResources resources"); @@ -169,7 +177,7 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, conn->set_display(0); displays_[0] = 0; found_primary = true; -} else { +} else if (conn->external()) { conn->set_display(display_num); displays_[display_num] = display_num; ++display_num; @@ -230,6 +238,8 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, } for (auto &conn : connectors_) { +if (conn->writeback()) + continue; ret = CreateDisplayPipe(conn.get()); if (ret) { ALOGE("Failed CreateDisplayPipe %d with %d", conn->id(), ret); @@ -245,7 +255,15 @@ bool DrmResources::HandlesDisplay(int display) const { DrmConnector *DrmResources::GetConnectorForDisplay(int display) const { for (auto &conn : connectors_) { -if (conn->display() == display) +if (conn->display() == display && !conn->writeback()) + return conn.get(); + } + return NULL; +} + +DrmConnector *DrmResources::GetWritebackConnectorForDisplay(int display) const { + for (auto &conn : connectors_) { +if (conn->display() == display && conn->writeback()) return conn.get(); } return NULL; @@ -280,6 +298,7 @@ int DrmResources::TryEncoderForDisplay(int display, DrmEncoder *enc) { DrmCrtc *crtc = enc->crtc(); if (crtc && crtc->can_bind(display)) { crtc->set_display(display); +enc->set_display(display); return 0; } @@ -306,6 +325,7 @@ int DrmResources::CreateDisplayPipe(DrmConnector *connector) { if (connector->encoder()) { int ret = TryEncoderForDisplay(display, connector->encoder()); if (!ret) { + AttachWriteback(connector); return 0; } else if (ret != -EAGAIN) { ALOGE("Could not set mode %d/%d", display, ret); @@ -317,6 +337,7 @@ int DrmResources::CreateDisplayPipe(DrmConnector *connector) { int ret = TryEncoderForDisplay(display, enc); if (!ret) { connector->set_encoder(enc); + AttachWriteback(connector); return 0; } else if (ret != -EAGAIN) { ALOGE("Could not set mode %d/%d", display, ret); @@ -328,6 +349,43 @@ int DrmResources::CreateDisplayPipe(DrmConnector *connector) { return -ENODEV; } +/* + * Attach writeback connector to the CRTC linked to the display_conn + * + */ +int DrmResources::AttachWriteback(DrmConnector *display_conn) { + int ret = -EINVAL; + if (display_conn->writeback()) +return -EINVAL; + DrmEncoder *display_enc = display_conn->encoder(); + if (!display_enc) +return -EINVAL; + DrmCrtc *display_crtc = display_enc->crtc(); + if (!display_crtc) +return -EINVAL; + if (GetWritebackConnectorForDisplay(display_crtc->display()) != NULL) +return -EINVAL; + for (auto &writeback_conn : connectors_) { +if (writeback_conn->display() >= 0 || !writeback_conn->writeback()) + continue; +for (DrmEncoder *writeback_enc : writeback_conn->possible_encoders()) { + for (DrmCrtc *possible_crtc : writeback_enc->possible_crtcs()) { +if (possible_crtc != display_crtc) + continue; +// Use just encoders which had not been bound already +if (writeback_enc->can_bind(display_crtc->display())) { + writeback_enc->set_crtc(display_crtc); + writeback_conn->set_encoder(writeback_enc); + writeback_conn->set_display(display_crtc->display()); + writeback_conn->UpdateModes(); + return 0; +} + } +} + } + return ret; +} + int DrmResources::CreatePropertyBlob(void *data, size_t length,
[PATCH hwc v2 06/18] drm_hwcomposer: Add writeback connector support
Writeback connector is a special case of connector, which can be linked to a CRTC in order to get the result of the composition back to a memory buffer. This had not been merged to the mainline kernel yet, latest version of the kernel patches could be found here [1]. [1] https://lists.freedesktop.org/archives/dri-devel/2018-February/167703.html Signed-off-by: Alexandru Gheorghe --- drmconnector.cpp | 42 +- drmconnector.h | 7 +++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/drmconnector.cpp b/drmconnector.cpp index 145518f..e482832 100644 --- a/drmconnector.cpp +++ b/drmconnector.cpp @@ -52,6 +52,26 @@ int DrmConnector::Init() { ALOGE("Could not get CRTC_ID property\n"); return ret; } + if (writeback()) { +ret = drm_->GetConnectorProperty(*this, "WRITEBACK_PIXEL_FORMATS", + &writeback_pixel_formats_); +if (ret) { + ALOGE("Could not get WRITEBACK_PIXEL_FORMATS connector_id = %d\n", id_); + return ret; +} +ret = +drm_->GetConnectorProperty(*this, "WRITEBACK_FB_ID", &writeback_fb_id_); +if (ret) { + ALOGE("Could not get WRITEBACK_FB_ID connector_id = %d\n", id_); + return ret; +} +ret = drm_->GetConnectorProperty(*this, "WRITEBACK_OUT_FENCE_PTR", + &writeback_out_fence_); +if (ret) { + ALOGE("Could not get WRITEBACK_OUT_FENCE_PTR connector_id = %d\n", id_); + return ret; +} + } return 0; } @@ -78,8 +98,16 @@ bool DrmConnector::external() const { type_ == DRM_MODE_CONNECTOR_VGA; } +bool DrmConnector::writeback() const { +#ifdef DRM_MODE_CONNECTOR_WRITEBACK + return type_ == DRM_MODE_CONNECTOR_WRITEBACK; +#else + return false; +#endif +} + bool DrmConnector::valid_type() const { - return internal() || external(); + return internal() || external() || writeback(); } int DrmConnector::UpdateModes() { @@ -130,6 +158,18 @@ const DrmProperty &DrmConnector::crtc_id_property() const { return crtc_id_property_; } +const DrmProperty &DrmConnector::writeback_pixel_formats() const { + return writeback_pixel_formats_; +} + +const DrmProperty &DrmConnector::writeback_fb_id() const { + return writeback_fb_id_; +} + +const DrmProperty &DrmConnector::writeback_out_fence() const { + return writeback_out_fence_; +} + DrmEncoder *DrmConnector::encoder() const { return encoder_; } diff --git a/drmconnector.h b/drmconnector.h index 5601e06..e139730 100644 --- a/drmconnector.h +++ b/drmconnector.h @@ -46,6 +46,7 @@ class DrmConnector { bool internal() const; bool external() const; + bool writeback() const; bool valid_type() const; int UpdateModes(); @@ -58,6 +59,9 @@ class DrmConnector { const DrmProperty &dpms_property() const; const DrmProperty &crtc_id_property() const; + const DrmProperty &writeback_pixel_formats() const; + const DrmProperty &writeback_fb_id() const; + const DrmProperty &writeback_out_fence() const; const std::vector &possible_encoders() const { return possible_encoders_; @@ -88,6 +92,9 @@ class DrmConnector { DrmProperty dpms_property_; DrmProperty crtc_id_property_; + DrmProperty writeback_pixel_formats_; + DrmProperty writeback_fb_id_; + DrmProperty writeback_out_fence_; std::vector possible_encoders_; }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 10/18] drm_hwcomposer: hwcutils: Add function for cloning a DrmHwcLayer
When doing flattening of a composition on a different CRTC we need to be able to clone a layer in order to import it and then pass it to another CRTC. Signed-off-by: Alexandru Gheorghe --- drmhwcomposer.h | 1 + hwcutils.cpp| 11 +++ 2 files changed, 12 insertions(+) diff --git a/drmhwcomposer.h b/drmhwcomposer.h index f8440fb..b256caf 100644 --- a/drmhwcomposer.h +++ b/drmhwcomposer.h @@ -150,6 +150,7 @@ struct DrmHwcLayer { int InitFromHwcLayer(hwc_layer_1_t *sf_layer, Importer *importer, const gralloc_module_t *gralloc); + int PopulateFromDrmHwcLayer(DrmHwcLayer *layer); int ImportBuffer(Importer *importer, const gralloc_module_t *gralloc); void SetTransform(int32_t sf_transform); diff --git a/hwcutils.cpp b/hwcutils.cpp index 53a7d82..ff37c3b 100644 --- a/hwcutils.cpp +++ b/hwcutils.cpp @@ -149,6 +149,17 @@ int DrmHwcLayer::InitFromHwcLayer(hwc_layer_1_t *sf_layer, Importer *importer, return ImportBuffer(importer, gralloc); } +int DrmHwcLayer::PopulateFromDrmHwcLayer(DrmHwcLayer *src_layer) { + blending = src_layer->blending; + sf_handle = src_layer->sf_handle; + acquire_fence = dup(src_layer->acquire_fence.get()); + display_frame = src_layer->display_frame; + alpha = src_layer->alpha; + source_crop = src_layer->source_crop; + transform = src_layer->transform; + return 0; +} + int DrmHwcLayer::ImportBuffer(Importer *importer, const gralloc_module_t *gralloc) { int ret = buffer.ImportBuffer(sf_handle, importer); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 08/18] drm_hwcomposer: Parse and store possible_clones information
drmModeEncoder has a field called possible_clones. It's a bit mask which tells if the encoder could be simultaneously connected, to the same CRTC, with the encoders specified in the possible_clones mask. Signed-off-by: Alexandru Gheorghe --- drmencoder.cpp | 8 drmencoder.h | 4 drmresources.cpp | 9 - 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drmencoder.cpp b/drmencoder.cpp index 1da7ec3..ff675f5 100644 --- a/drmencoder.cpp +++ b/drmencoder.cpp @@ -39,6 +39,14 @@ DrmCrtc *DrmEncoder::crtc() const { return crtc_; } +bool DrmEncoder::can_clone(DrmEncoder *encoder) { + return possible_clones_.find(encoder) != possible_clones_.end(); +} + +void DrmEncoder::add_possible_clone(DrmEncoder *possible_clone) { + possible_clones_[possible_clone] = true; +} + void DrmEncoder::set_crtc(DrmCrtc *crtc) { crtc_ = crtc; set_display(crtc->display()); diff --git a/drmencoder.h b/drmencoder.h index 7e06691..5e7c010 100644 --- a/drmencoder.h +++ b/drmencoder.h @@ -21,6 +21,7 @@ #include #include +#include #include namespace android { @@ -43,6 +44,8 @@ class DrmEncoder { const std::vector &possible_crtcs() const { return possible_crtcs_; } + bool can_clone(DrmEncoder *encoder); + void add_possible_clone(DrmEncoder *possible_clone); private: uint32_t id_; @@ -50,6 +53,7 @@ class DrmEncoder { int display_; std::vector possible_crtcs_; + std::map possible_clones_; }; } diff --git a/drmresources.cpp b/drmresources.cpp index a5ddda0..39f50be 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -97,6 +97,7 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, crtcs_.emplace_back(std::move(crtc)); } + std::vector possible_clones; for (int i = 0; !ret && i < res->count_encoders; ++i) { drmModeEncoderPtr e = drmModeGetEncoder(fd(), res->encoders[i]); if (!e) { @@ -117,12 +118,18 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, std::unique_ptr enc( new DrmEncoder(e, current_crtc, possible_crtcs)); - +possible_clones.push_back(e->possible_clones); drmModeFreeEncoder(e); encoders_.emplace_back(std::move(enc)); } + for (uint32_t i = 0; i < encoders_.size(); i++) { +for (uint32_t j = 0; j < encoders_.size(); j++) + if (possible_clones[i] & (1 << j)) +encoders_[i]->add_possible_clone(encoders_[j].get()); + } + for (int i = 0; !ret && i < res->count_connectors; ++i) { drmModeConnectorPtr c = drmModeGetConnector(fd(), res->connectors[i]); if (!c) { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel