[Bug 109206] Kernel 5.3.11 and onward amdgpu fails to load firmware on Raven Ridge
https://bugs.freedesktop.org/show_bug.cgi?id=109206 Luya Tshimbalanga changed: What|Removed |Added Summary|Kernel 4.20 amdgpu fails to |Kernel 5.3.11 and onward |load firmware on Ryzen |amdgpu fails to load |2500U |firmware on Raven Ridge --- Comment #65 from Luya Tshimbalanga --- Change the title reflecting the impact on Raven Ridge -- 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 205521] 5.3.11 update broke AMDGPU Raven Ridge
https://bugzilla.kernel.org/show_bug.cgi?id=205521 Luya Tshimbalanga (l...@fedoraproject.org) changed: What|Removed |Added URL||https://bugs.freedesktop.or ||g/show_bug.cgi?id=109206 --- Comment #2 from Luya Tshimbalanga (l...@fedoraproject.org) --- Added similar report from freedesktop.org -- 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 109206] Kernel 4.20 amdgpu fails to load firmware on Ryzen 2500U
https://bugs.freedesktop.org/show_bug.cgi?id=109206 --- Comment #64 from Luya Tshimbalanga --- Created attachment 145952 --> https://bugs.freedesktop.org/attachment.cgi?id=145952=edit Journal boot from hp envy x360 cp0xxx The bug caused the Raven Ridge firmware to fail is back on kernel 5.3.11. Even the latest git version is affected -- 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/komeda: Clean warnings: candidate for 'gnu_printf’ format attribute
komeda/komeda_pipeline.c: In function ‘komeda_component_add’:
komeda/komeda_pipeline.c:213:3: warning: function ‘komeda_component_add’ might
be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
vsnprintf(c->name, sizeof(c->name), name_fmt, args);
^
komeda/komeda_event.c: In function ‘komeda_sprintf’:
komeda/komeda_event.c:31:2: warning: function ‘komeda_sprintf’ might be a
candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
num = vsnprintf(str->str + str->len, free_sz, fmt, args);
Signed-off-by: james qian wang (Arm Technology China)
---
drivers/gpu/drm/arm/display/komeda/komeda_event.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_event.c
b/drivers/gpu/drm/arm/display/komeda/komeda_event.c
index bf269683f811..977c38d516da 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_event.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_event.c
@@ -17,6 +17,7 @@ struct komeda_str {
/* return 0 on success, < 0 on no space.
*/
+__printf(2, 3)
static int komeda_sprintf(struct komeda_str *str, const char *fmt, ...)
{
va_list args;
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205523] New: AMDGPU display lockup during boot with 5.4 RC on Ryzen 2700u
https://bugzilla.kernel.org/show_bug.cgi?id=205523 Bug ID: 205523 Summary: AMDGPU display lockup during boot with 5.4 RC on Ryzen 2700u Product: Drivers Version: 2.5 Kernel Version: 5.4.0-rc7 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: briancsch...@gmail.com Regression: No Created attachment 285907 --> https://bugzilla.kernel.org/attachment.cgi?id=285907=edit journalctl output I'm getting a black screen on boot when testing the 5.4 release candidate kernels on a Dell Inspiron 7375 laptop. The system responds to the magic sysrq, so the kernel is not completely dead, but the display is. The system boots correctly with a 5.3.1 kernel. The log showing the stack trace is attached. I'll start working on bisecting the problem soon. -- 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 112267] nebila
https://bugs.freedesktop.org/show_bug.cgi?id=112267 Andre Klapper changed: What|Removed |Added Product|DRI |Spam Status|NEW |RESOLVED Group||spam Component|DRM/other |Two Resolution|--- |INVALID Version|XOrg git|unspecified --- Comment #1 from Andre Klapper --- Go away and test somewhere else. -- 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 205521] 5.3.11 update broke AMDGPU Raven Ridge
https://bugzilla.kernel.org/show_bug.cgi?id=205521 Luya Tshimbalanga (l...@fedoraproject.org) changed: What|Removed |Added Regression|No |Yes -- 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 205521] 5.3.11 update broke AMDGPU Raven Ridge
https://bugzilla.kernel.org/show_bug.cgi?id=205521 --- Comment #1 from Luya Tshimbalanga (l...@fedoraproject.org) --- Created attachment 285905 --> https://bugzilla.kernel.org/attachment.cgi?id=285905=edit dmesg with the latest git snapshot Recent kernel git snapshot is also affected: -- 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 205521] New: 5.3.11 update broke AMDGPU Raven Ridge
https://bugzilla.kernel.org/show_bug.cgi?id=205521 Bug ID: 205521 Summary: 5.3.11 update broke AMDGPU Raven Ridge Product: Drivers Version: 2.5 Kernel Version: 5.3.11 and up Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: blocking Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: l...@fedoraproject.org Regression: No Created attachment 285903 --> https://bugzilla.kernel.org/attachment.cgi?id=285903=edit dmesg reporting broken amd raven ridge firmware AMD Raven Ridge firware is currently broken with the recent stable kernel release resulting a blank screen on boot and preventing booting on the login screen either graphical and text mode. Extract from boot: nov 13 13:53:55 kernel: amdgpu :03:00.0: Direct firmware load for amdgpu/raven_gpu_info.bin failed with error -2 nov 13 13:53:55 kernel: amdgpu :03:00.0: Failed to load gpu_info firmware "amdgpu/raven_gpu_info.bin" nov 13 13:53:55 kernel: amdgpu :03:00.0: Fatal error during GPU init -- 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 v4 09/23] mm/gup: introduce pin_user_pages*() and FOLL_PIN
On 11/13/19 3:22 PM, John Hubbard wrote:
> On 11/13/19 2:43 AM, Jan Kara wrote:
> ...
>> How does FOLL_PIN result in grabbing (at least normal, for now) page
>> reference?
>> I didn't find that anywhere in this patch but it is a prerequisite to
>> converting any user to pin_user_pages() interface, right?
>
>
> ohhh, I messed up on this intermediate patch: it doesn't quite stand alone as
> it should, as you noticed. To correct this, I can do one of the following:
>
> a) move the new pin*() routines into the later patch 16 ("mm/gup:
> track FOLL_PIN pages"), or
>
> b) do a temporary thing here, such as setting FOLL_GET and adding a TODO,
> within the pin*() implementations. And this switching it over to FOLL_PIN
> in patch 16.
>
> I'm thinking (a) is less error-prone, so I'm going with that unless someone
> points out that that is stupid. :)
>
OK, just to save anyone from wasting time reading the above: (a) is, in fact,
stupid, after all. ha. That is because pin_user_pages() is called in the
intervening patches.
So anyway, I'll work out an ordering to fix it up, it's not complicated.
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-fixes
Hi Dave and Daniel, Here goes drm-intel-fixes-2019-11-13: - MOCS table fixes for EHL and TGL - Update Display's rawclock on resume - GVT's dmabuf reference drop fix Thanks, Rodrigo. The following changes since commit 31f4f5b495a62c9a8b15b1c3581acd5efeb9af8c: Linux 5.4-rc7 (2019-11-10 16:17:15 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2019-11-13 for you to fetch changes up to 1c602006d1dcb7501ae1c569fdabe1b8e1f082a4: drm/i915/tgl: MOCS table update (2019-11-13 13:23:12 -0800) - MOCS table fixes for EHL and TGL - Update Display's rawclock on resume - GVT's dmabuf reference drop fix Jani Nikula (1): drm/i915: update rawclk also on resume Matt Roper (2): Revert "drm/i915/ehl: Update MOCS table for EHL" drm/i915/tgl: MOCS table update Pan Bian (1): drm/i915/gvt: fix dropping obj reference twice Rodrigo Vivi (1): Merge tag 'gvt-fixes-2019-11-12' of https://github.com/intel/gvt-linux into drm-intel-fixes drivers/gpu/drm/i915/display/intel_display_power.c | 3 +++ drivers/gpu/drm/i915/gt/intel_mocs.c | 10 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 4 ++-- drivers/gpu/drm/i915/i915_drv.c| 3 --- 4 files changed, 6 insertions(+), 14 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: manual merge of the hmm tree with the drm tree
Hi all,
Today's linux-next merge of the hmm tree got a conflict in:
drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
between commit:
4d8e54d2b9d3 ("drm/amdgpu/mn: fix documentation for amdgpu_mn_read_lock")
from the drm tree and commit:
a2849b5dcc9e ("drm/amdgpu: Use mmu_interval_insert instead of hmm_mirror")
from the hmm tree.
I fixed it up (the latter removed the code updated by the former, so I
did that) and can carry the fix as necessary. This is now fixed as far as
linux-next is concerned, but any non trivial conflicts should be mentioned
to your upstream maintainer when your tree is submitted for merging.
You may also want to consider cooperating with the maintainer of the
conflicting tree to minimise any particularly complex conflicts.
--
Cheers,
Stephen Rothwell
pgpz35CGSC4Y6.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 1/1] drm/radeon: fix bad DMA from INTERRUPT_CNTL2
On Wed, Nov 13, 2019 at 9:53 PM Sam Bobroff wrote: > > Currently, binding the radeon driver to a Radeon FirePro (PCI > 1002:68f2) that has been passed through to a guest on a Power8 machine > causes a bad DMA read which causes the device to be frozen, leading to > the driver failing to bind and other problems. > > The bad DMA is to the address written into the INTERRUPT_CNTL2 > register during r600_irq_init(), and it can be fixed by substituting a > valid (dummy) DMA address. The patch is correct. INTERRUPT_CNTL2 takes a bus address not a GPU MC address. Care to update si.c and cik.c as well? Thanks, Alex > --- > Hello, > > I've been tracking down a bug, described above, and I've been able to hack > in a workaround but I could use some help understanding how to fix it > properly. > > What seems to be happening is that when the first CRTC is enabled (when > evergreen_irq_set() writes to GRPH_INT_CONTROL) the device immediately > performs a DMA read from the address that's been programmed into > INTERRUPT_CNTL2 by r600_irq_init(). > > That address isn't a valid DMA address, so it triggers the problem. The > address comes from rdev->ih.gpu_addr, which seems to be a 'linear GPU > address', calculated from the size of the card's VRAM. It definitely hasn't > come from a DMA mapping operation. > > Based on the nearby comment, "set dummy read address to ring address", I > tried substituting a valid dummy DMA address (using the address mapped to > the 'dummy page' used by the GART) and it does prevent the problem. > However, I don't know what that register is supposed to do or what > information the device might be communicating via that DMA, so presumably > it breaks something. > > Note: I have tried loading the patched driver with "test=1" and all the > self tests succeed. > > Could anyone offer some insight into this problem? > What does that register do? What kind of address is it expecting? > What might be a good way of fixing it? > > Thanks in advance, > Sam. > > drivers/gpu/drm/radeon/r600.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c > index e937cc01910d..033bc466a862 100644 > --- a/drivers/gpu/drm/radeon/r600.c > +++ b/drivers/gpu/drm/radeon/r600.c > @@ -3696,8 +3696,8 @@ int r600_irq_init(struct radeon_device *rdev) > } > > /* setup interrupt control */ > - /* set dummy read address to ring address */ > - WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8); > + /* set dummy read address to dummy page address */ > + WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8); > interrupt_cntl = RREG32(INTERRUPT_CNTL); > /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled > without msi > * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN > -- > 2.22.0.216.g00a2a96fc9 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC 1/1] drm/radeon: fix bad DMA from INTERRUPT_CNTL2
Currently, binding the radeon driver to a Radeon FirePro (PCI 1002:68f2) that has been passed through to a guest on a Power8 machine causes a bad DMA read which causes the device to be frozen, leading to the driver failing to bind and other problems. The bad DMA is to the address written into the INTERRUPT_CNTL2 register during r600_irq_init(), and it can be fixed by substituting a valid (dummy) DMA address. --- Hello, I've been tracking down a bug, described above, and I've been able to hack in a workaround but I could use some help understanding how to fix it properly. What seems to be happening is that when the first CRTC is enabled (when evergreen_irq_set() writes to GRPH_INT_CONTROL) the device immediately performs a DMA read from the address that's been programmed into INTERRUPT_CNTL2 by r600_irq_init(). That address isn't a valid DMA address, so it triggers the problem. The address comes from rdev->ih.gpu_addr, which seems to be a 'linear GPU address', calculated from the size of the card's VRAM. It definitely hasn't come from a DMA mapping operation. Based on the nearby comment, "set dummy read address to ring address", I tried substituting a valid dummy DMA address (using the address mapped to the 'dummy page' used by the GART) and it does prevent the problem. However, I don't know what that register is supposed to do or what information the device might be communicating via that DMA, so presumably it breaks something. Note: I have tried loading the patched driver with "test=1" and all the self tests succeed. Could anyone offer some insight into this problem? What does that register do? What kind of address is it expecting? What might be a good way of fixing it? Thanks in advance, Sam. drivers/gpu/drm/radeon/r600.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index e937cc01910d..033bc466a862 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -3696,8 +3696,8 @@ int r600_irq_init(struct radeon_device *rdev) } /* setup interrupt control */ - /* set dummy read address to ring address */ - WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8); + /* set dummy read address to dummy page address */ + WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8); interrupt_cntl = RREG32(INTERRUPT_CNTL); /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN -- 2.22.0.216.g00a2a96fc9 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 112266] [Navi] Pathfinder: Kingmaker is causing a GPU hang: flip_done timed out error
https://bugs.freedesktop.org/show_bug.cgi?id=112266 Bug ID: 112266 Summary: [Navi] Pathfinder: Kingmaker is causing a GPU hang: flip_done timed out error Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: not set Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: shtetl...@gmail.com When running Pathfinder: Kingmaker (latest GOG release, which should be the same as latest Steam one) on Sapphire Pulse RX 5700 XT, it's causing a weird GPU hang with flip_done timed out error (see below for detailed log), that doesn't look like the common shader hangs with ring gfx_0.0.0 timeout or common sdma hangs. The game is using OpenGL, and I run the game on Debian testing, using this configuration: kernel: 5.4-rc7 radeonsi: Mesa-master / llvm10: OpenGL renderer string: AMD NAVI10 (DRM 3.35.0, 5.4.0-rc7, LLVM 10.0.0) OpenGL core profile version string: 4.5 (Core Profile) Mesa 20.0.0-devel (git-eb6352162d) llvm: 10~+201911120943210600592dd459242 from this llvm10 snapshot: https://tracker.debian.org/news/1079513/accepted-llvm-toolchain-snapshot-110201911120943210600592dd459242-1exp1-source-into-experimental/ DE: KDE Plasma 5.14.5 (X session). GPU: Sapphire Pulse RX 5700 XT Monitor: LG 27GL85-B (2560x1440, 144 Hz, DisplayPort 1.4 connection, adaptive sync activated in Xorg configuration). When launching, I'm using AMD_DEBUG=nodma,nongg Recording apitrace doesn't help, since replaying it is not reproducing the hang. So it could be some amdgpu issue? Please let me know, what additional info can be useful to help you narrow it down. However the hang is quite reproducible, and you can try it yourself with Pathfinder: Kingmaker. The hang produces this in dmesg: [ 659.445501] [drm:drm_atomic_helper_wait_for_dependencies [drm_kms_helper]] *ERROR* [CRTC:62:crtc-0] flip_done timed out [ 669.685601] [drm:drm_atomic_helper_wait_for_dependencies [drm_kms_helper]] *ERROR* [PLANE:55:plane-5] flip_done timed out [ 669.685644] [ cut here ] [ 669.685729] WARNING: CPU: 6 PID: 1018 at drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:5851 amdgpu_dm_atomic_commit_tail+0x1c56/0x1d70 [amdgpu] [ 669.685730] Modules linked in: rfcomm(E) nf_tables(E) nfnetlink(E) bnep(E) edac_mce_amd(E) kvm_amd(E) kvm(E) irqbypass(E) crct10dif_pclmul(E) btusb(E) btrtl(E) snd_hda_codec_realtek(E) btbcm(E) crc32_pclmul(E) btintel(E) iwlmvm(E) snd_hda_codec_generic(E) bluetooth(E) ghash_clmulni_intel(E) ledtrig_audio(E) mac80211(E) libarc4(E) snd_hda_codec_hdmi(E) uvcvideo(E) snd_hda_intel(E) videobuf2_vmalloc(E) snd_usb_audio(E) snd_intel_nhlt(E) videobuf2_memops(E) drbg(E) snd_hda_codec(E) videobuf2_v4l2(E) snd_usbmidi_lib(E) iwlwifi(E) nls_ascii(E) snd_hda_core(E) snd_rawmidi(E) videobuf2_common(E) snd_seq_device(E) snd_hwdep(E) efi_pstore(E) nls_cp437(E) ansi_cprng(E) snd_pcm(E) videodev(E) sp5100_tco(E) aesni_intel(E) cfg80211(E) vfat(E) ecdh_generic(E) crypto_simd(E) ecc(E) snd_timer(E) fat(E) ccp(E) snd(E) cryptd(E) mc(E) glue_helper(E) crc16(E) wmi_bmof(E) pcspkr(E) efivars(E) k10temp(E) watchdog(E) sg(E) rfkill(E) soundcore(E) rng_core(E) evdev(E) acpi_cpufreq(E) nct6775(E) hwmon_vid(E) [ 669.685753] parport_pc(E) ppdev(E) lp(E) parport(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) xfs(E) btrfs(E) xor(E) zstd_decompress(E) zstd_compress(E) raid6_pq(E) libcrc32c(E) crc32c_generic(E) sd_mod(E) hid_generic(E) usbhid(E) hid(E) amdgpu(E) gpu_sched(E) mxm_wmi(E) ahci(E) ttm(E) libahci(E) drm_kms_helper(E) xhci_pci(E) crc32c_intel(E) xhci_hcd(E) i2c_piix4(E) libata(E) drm(E) igb(E) dca(E) mfd_core(E) ptp(E) scsi_mod(E) usbcore(E) pps_core(E) i2c_algo_bit(E) nvme(E) nvme_core(E) wmi(E) button(E) [ 669.685770] CPU: 6 PID: 1018 Comm: Xorg Tainted: GE 5.4.0-rc7 #31 [ 669.685771] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./X570 Taichi, BIOS P2.50 11/02/2019 [ 669.685846] RIP: 0010:amdgpu_dm_atomic_commit_tail+0x1c56/0x1d70 [amdgpu] [ 669.685847] Code: 67 fb ff ff 41 8b 4c 24 60 48 c7 c2 60 d6 a2 c0 bf 02 00 00 00 48 c7 c6 80 f8 a9 c0 e8 e3 7d bb ff 49 8b 47 08 e9 31 e5 ff ff <0f> 0b e9 b4 ec ff ff 0f 0b 0f 0b e9 cb ec ff ff 48 8b 85 b0 fd ff [ 669.685848] RSP: 0018:b80fc1a978d0 EFLAGS: 00010002 [ 669.685849] RAX: 0002 RBX: 9454b5d54c00 RCX: 9455ec2c6170 [ 669.685850] RDX: 0001 RSI: 0206 RDI: 9455eaba6158 [ 669.685851] RBP: b80fc1a97b80 R08: 0005 R09: [ 669.685851] R10: b80fc1a97838 R11: b80fc1a9783c R12: 0206 [ 669.685852] R13: 9455ec2c6000 R14: 94559d443800 R15: 9455eda2 [ 669.685853] FS: 7fc6a5a21f00() GS:9455fe98() knlGS: [ 669.685854] CS: 0010 DS: ES: CR0:
Re: [PATCH v2 4/5] dt-bindings: display: rockchip-dsi: add px30 compatible
On Fri, 8 Nov 2019 01:02:52 +0100, Heiko Stuebner wrote: > The px30 SoC also uses a dw-mipi-dsi controller, so add the > compatible value for it. > > Signed-off-by: Heiko Stuebner > --- > .../bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv2 3/4] drm/komeda: use afbc helpers
On Wed, Nov 13, 2019 at 12:39:54PM +0100, Daniel Vetter wrote:
> On Wed, Nov 13, 2019 at 02:01:53AM +, james qian wang (Arm Technology
> China) wrote:
> > On Fri, Nov 08, 2019 at 04:09:54PM +, Ayan Halder wrote:
> > > On Mon, Nov 04, 2019 at 11:12:27PM +0100, Andrzej Pietrasiewicz wrote:
> > > > There are afbc helpers available.
> > > >
> > > > Signed-off-by: Andrzej Pietrasiewicz
> > > > ---
> > > > .../arm/display/komeda/komeda_format_caps.h | 1 -
> > > > .../arm/display/komeda/komeda_framebuffer.c | 44 +++
> > > > 2 files changed, 17 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h
> > > > b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h
> > > > index 32273cf18f7c..607eea80e60c 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_format_caps.h
> > > > @@ -33,7 +33,6 @@
> > > >
> > > > #define AFBC_TH_LAYOUT_ALIGNMENT 8
> > > > #define AFBC_HEADER_SIZE 16
> > > > -#define AFBC_SUPERBLK_ALIGNMENT128
> > > > #define AFBC_SUPERBLK_PIXELS 256
> > > > #define AFBC_BODY_START_ALIGNMENT 1024
> > > > #define AFBC_TH_BODY_START_ALIGNMENT 4096
> > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > > > b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > > > index 1b01a625f40e..e9c87551a5b8 100644
> > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> > > > @@ -4,6 +4,7 @@
> > > > * Author: James.Qian.Wang
> > > > *
> > > > */
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -43,8 +44,7 @@ komeda_fb_afbc_size_check(struct komeda_fb *kfb,
> > > > struct drm_file *file,
> > > > struct drm_framebuffer *fb = >base;
> > > > const struct drm_format_info *info = fb->format;
> > > > struct drm_gem_object *obj;
> > > > - u32 alignment_w = 0, alignment_h = 0, alignment_header,
> > > > n_blocks, bpp;
> > > > - u64 min_size;
> > > > + u32 alignment_w = 0, alignment_h = 0, alignment_header, bpp;
> > > >
> > > > obj = drm_gem_object_lookup(file, mode_cmd->handles[0]);
> > > > if (!obj) {
> > > > @@ -52,19 +52,15 @@ komeda_fb_afbc_size_check(struct komeda_fb *kfb,
> > > > struct drm_file *file,
> > > > return -ENOENT;
> > > > }
> > > >
> > > > - switch (fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> > > > - case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> > > > - alignment_w = 32;
> > > > - alignment_h = 8;
> > > > - break;
> > > > - case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> > > > - alignment_w = 16;
> > > > - alignment_h = 16;
> > > > - break;
> > > > - default:
> > > > - WARN(1, "Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> > > > -fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> > > > - break;
> > > > + if (!drm_afbc_get_superblk_wh(fb->modifier, _w,
> > > > _h))
> > > > + return -EINVAL;
> > > > +
> > > > + if ((alignment_w != 16 || alignment_h != 16) &&
> > > > + (alignment_w != 32 || alignment_h != 8)) {
> > > > + DRM_DEBUG_KMS("Unsupported afbc tile w/h [%d/%d]\n",
> > > > + alignment_w, alignment_h);
> > > > +
> > > > + return -EINVAL;
> > > To be honest, the previous code looks much more readable
> > > > }
> > > >
> > > > /* tiled header afbc */
> > > > @@ -84,20 +80,14 @@ komeda_fb_afbc_size_check(struct komeda_fb *kfb,
> > > > struct drm_file *file,
> > > > goto check_failed;
> > > > }
> > > >
> > > > - n_blocks = (kfb->aligned_w * kfb->aligned_h) /
> > > > AFBC_SUPERBLK_PIXELS;
> > > > - kfb->offset_payload = ALIGN(n_blocks * AFBC_HEADER_SIZE,
> > > > - alignment_header);
> > > > -
> > > > bpp = komeda_get_afbc_format_bpp(info, fb->modifier);
> > > > - kfb->afbc_size = kfb->offset_payload + n_blocks *
> > > > -ALIGN(bpp * AFBC_SUPERBLK_PIXELS / 8,
> > > > - AFBC_SUPERBLK_ALIGNMENT);
> > > > - min_size = kfb->afbc_size + fb->offsets[0];
> > > > - if (min_size > obj->size) {
> > > > - DRM_DEBUG_KMS("afbc size check failed, obj_size: 0x%zx.
> > > > min_size 0x%llx.\n",
> > > > - obj->size, min_size);
> > > We need kfb->offset_payload and kfb->afbc_size to set some registers
> > > in d71_layer_update(). At this moment I feel like punching myself for
> > > making the suggestion to consider abstracting some of the komeda's afbc
> > > checks. To me it does not look like
Re: [PATCH v2 2/5] dt-bindings: display: rockchip-dsi: document external phys
On Fri, 8 Nov 2019 01:02:50 +0100, Heiko Stuebner wrote: > Some dw-mipi-dsi instances in Rockchip SoCs use external dphys. > In these cases the needs clock will also be generated externally > so these don't need the ref-clock as well. > > Signed-off-by: Heiko Stuebner > --- > .../bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
drm-next + i915 CVE yolo merge
The landing of the i915 CVE fixes into Linus tree has created a bit of a mess in linux-next and downstream in drm-next trees. I talked to Daniel and he had talked to Joonas a bit, and I decided to go with what Daniel describes as the YOLO merge, where I just solve it and pray, and everyone else verifies/fixes it. In my favour I've been reading these patches for a couple of months now and applied them to a lot of places, so I'm quite familiar with what they are doing. The worst culprit was the RC6 ctx corruption fix since the whole of rc6 got refactored in next. However I also had access to a version of this patch Jon wrote on drm-tip a couple of weeks ago. I took that patch, applied it and fixed it up on top of drm-next. Then I backmerged the commit that also went into Linus' tree. Then I removed any evidence of the RC6 patch from Linus' tree and left the newer version pieces in place. The other stuff mostly merged fine and the results looked fine, but I'd definitely think everyone at Intel should be staring at it, and my dinq tip resolutions ASAP and hopefully it goes into CI and comes out smelling of something good. Let me know if it's all horrible asap, Thanks, Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/3] drm/virtio: various cleanups
On Tue, Oct 22, 2019 at 11:25 PM Gerd Hoffmann wrote:
>
> Little patch series doing some cleanups in the virtio driver.
> Patches #1 + #2 have been on the list before as single patches,
> includes here again for patch dependency reasons.
>
> v2:
> - fix sparse errors.
> - drop merged patches (#1 + #2).
>
> please review,
Series is:
Reviewed-by: Gurchetan Singh
> Gerd
>
> Gerd Hoffmann (3):
> drm/virtio: fix byteorder handling in
> virtio_gpu_cmd_transfer_{from,to}_host_3d functions
> drm/virtio: Simplify virtio_gpu_primary_plane_update workflow.
> drm/virtio: factor out virtio_gpu_update_dumb_bo
>
> drivers/gpu/drm/virtio/virtgpu_drv.h | 5 +-
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 22 ++--
> drivers/gpu/drm/virtio/virtgpu_plane.c | 70 +++---
> drivers/gpu/drm/virtio/virtgpu_vq.c| 19 +--
> 4 files changed, 61 insertions(+), 55 deletions(-)
>
> --
> 2.18.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9
https://bugs.freedesktop.org/show_bug.cgi?id=111481 --- Comment #230 from Daniel Suarez --- (In reply to John Smith from comment #225) > (In reply to Pierre-Eric Pelloux-Prayer from comment #141) > > > For radeonsi the AMD_DEBUG=nodma environment variable is a workaround until > > we figure out a proper fix. > > Is this seriously what AMD calls "support"? No offense but this is > ridiculous, this card has been out for four months and it still can't even > browse firefox reliably, even after these "workarounds" and "patches". > > Then we waited two months for the drivers to even get properly released, and > all this wait was for nothing because the drivers are useless, you can't > even browse firefox or let alone play any actual games. What is the point of > having open source drivers if they don't even work? Nvidia's GPUs have had > day one support, and unlike AMD, "support" actually means the GPU works for > something that is meaningful. I wouldn't really call what is happening here "support". Really feels like us Linux users were thrown to the side with little consideration. -- 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
[radeon-alex:amd-mainline-dkms-5.2 2701/2834] include/kcl/kcl_drm.h:236:1: error: static declaration of 'drm_atomic_helper_update_legacy_modeset_state' follows non-static declaration
Hi Flora, FYI, the error/warning still remains. tree: git://people.freedesktop.org/~agd5f/linux.git amd-mainline-dkms-5.2 head: a48b0cc1cdf3900e3e73801f9de64afbb70dc193 commit: cc8e420623914e7a903534abddf086dad609a455 [2701/2834] drm/amdkcl: drop kcl_drm_atomic_helper_update_legacy_modeset_state config: x86_64-randconfig-e004-201944 (attached as .config) compiler: gcc-7 (Debian 7.4.0-14) 7.4.0 reproduce: git checkout cc8e420623914e7a903534abddf086dad609a455 # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/kcl/kcl_drm.h:154:1: error: redefinition of 'drm_fb_helper_remove_conflicting_pci_framebuffers' drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev, ^ In file included from include/kcl/kcl_drm.h:7:0, from include/kcl/kcl_drm_backport.h:5, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/drm/drm_fb_helper.h:612:1: note: previous definition of 'drm_fb_helper_remove_conflicting_pci_framebuffers' was here drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev, ^ In file included from include/kcl/kcl_drm_backport.h:5:0, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/kcl/kcl_drm.h:175:6: error: static declaration of 'drm_fb_helper_cfb_fillrect' follows non-static declaration void drm_fb_helper_cfb_fillrect(struct fb_info *info, ^~ In file included from include/kcl/kcl_drm.h:7:0, from include/kcl/kcl_drm_backport.h:5, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/drm/drm_fb_helper.h:289:6: note: previous declaration of 'drm_fb_helper_cfb_fillrect' was here void drm_fb_helper_cfb_fillrect(struct fb_info *info, ^~ In file included from include/kcl/kcl_drm_backport.h:5:0, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/kcl/kcl_drm.h:182:6: error: static declaration of 'drm_fb_helper_cfb_copyarea' follows non-static declaration void drm_fb_helper_cfb_copyarea(struct fb_info *info, ^~ In file included from include/kcl/kcl_drm.h:7:0, from include/kcl/kcl_drm_backport.h:5, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/drm/drm_fb_helper.h:291:6: note: previous declaration of 'drm_fb_helper_cfb_copyarea' was here void drm_fb_helper_cfb_copyarea(struct fb_info *info, ^~ In file included from include/kcl/kcl_drm_backport.h:5:0, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/kcl/kcl_drm.h:189:6: error: static declaration of 'drm_fb_helper_cfb_imageblit' follows non-static declaration void drm_fb_helper_cfb_imageblit(struct fb_info *info, ^~~ In file included from include/kcl/kcl_drm.h:7:0, from include/kcl/kcl_drm_backport.h:5, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/drm/drm_fb_helper.h:293:6: note: previous declaration of 'drm_fb_helper_cfb_imageblit' was here void drm_fb_helper_cfb_imageblit(struct fb_info *info, ^~~ In file included from include/kcl/kcl_drm_backport.h:5:0, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/kcl/kcl_drm.h:201:17: error: static declaration of 'drm_fb_helper_alloc_fbi' follows non-static declaration struct fb_info *drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper) ^~~ In file included from include/kcl/kcl_drm.h:7:0, from include/kcl/kcl_drm_backport.h:5, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/drm/drm_fb_helper.h:265:17: note: previous declaration of 'drm_fb_helper_alloc_fbi' was here struct fb_info *drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper); ^~~ In file included from include/kcl/kcl_drm_backport.h:5:0, from drivers/gpu/drm/ttm/backport/backport.h:6, from :0: include/kcl/kcl_drm.h:208:6: error: static declaration of 'drm_fb_helper_unregister_fbi' follows non-static declaration void drm_fb_helper_unregister_fbi(struct
[Bug 205517] nouveau MMIO read of 00000000 FAULT at 619444
https://bugzilla.kernel.org/show_bug.cgi?id=205517 --- Comment #1 from Todd Brandt (todd.e.bra...@linux.intel.com) --- Created attachment 285895 --> https://bugzilla.kernel.org/attachment.cgi?id=285895=edit otcpl-dell-p5510-xeon-1_mem.html -- 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 205517] nouveau MMIO read of 00000000 FAULT at 619444
https://bugzilla.kernel.org/show_bug.cgi?id=205517 Todd Brandt (todd.e.bra...@linux.intel.com) changed: What|Removed |Added Blocks||178231 Referenced Bugs: https://bugzilla.kernel.org/show_bug.cgi?id=178231 [Bug 178231] Meta-bug: Linux suspend-to-mem and freeze performance optimization -- 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 205517] New: nouveau MMIO read of 00000000 FAULT at 619444
https://bugzilla.kernel.org/show_bug.cgi?id=205517 Bug ID: 205517 Summary: nouveau MMIO read of FAULT at 619444 Product: Drivers Version: 2.5 Kernel Version: 5.3.0 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: todd.e.bra...@linux.intel.com Regression: No Created attachment 285893 --> https://bugzilla.kernel.org/attachment.cgi?id=285893=edit issue.def NVidia GPU driver fault on Dell systems during resume. [33133.398092] nouveau :01:00.0: calling nv_msi_ht_cap_quirk_leaf+0x0/0x30 @ 27 [33133.398102] nouveau :01:00.0: nv_msi_ht_cap_quirk_leaf+0x0/0x30 took 5 usecs [33133.398104] nouveau :01:00.0: calling quirk_nvidia_hda+0x0/0xc0 @ 27 [33133.398107] nouveau :01:00.0: Enabling HDA controller [33133.398110] nouveau :01:00.0: quirk_nvidia_hda+0x0/0xc0 took 3 usecs [33133.399220] done. [33133.540201] nouveau :01:00.0: bus: MMIO read of FAULT at 619444 [ IBUS ] [33133.556094] PM: suspend exit -- 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 v4 23/23] mm/gup: remove support for gup(FOLL_LONGTERM)
On 11/13/19 11:09 AM, Ira Weiny wrote:
...
diff --git a/mm/gup.c b/mm/gup.c
index 82e7e4ce5027..90f5f95ee7ac 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1756,11 +1756,11 @@ long get_user_pages(unsigned long start, unsigned long
nr_pages,
struct vm_area_struct **vmas)
{
/*
-* FOLL_PIN must only be set internally by the pin_user_page*() and
-* pin_longterm_*() APIs, never directly by the caller, so enforce that
-* with an assertion:
+* FOLL_PIN and FOLL_LONGTERM must only be set internally by the
+* pin_user_page*() and pin_longterm_*() APIs, never directly by the
+* caller, so enforce that with an assertion:
*/
- if (WARN_ON_ONCE(gup_flags & FOLL_PIN))
+ if (WARN_ON_ONCE(gup_flags & (FOLL_PIN | FOLL_LONGTERM)))
Don't we want to block FOLL_LONGTERM in get_user_pages_fast() as well after all
this?
Yes. But with the latest idea to restore FOLL_LONGTERM to its original glory,
that won't be an issue in the next version. heh.
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 09/23] mm/gup: introduce pin_user_pages*() and FOLL_PIN
On 11/13/19 2:43 AM, Jan Kara wrote:
...
How does FOLL_PIN result in grabbing (at least normal, for now) page reference?
I didn't find that anywhere in this patch but it is a prerequisite to
converting any user to pin_user_pages() interface, right?
ohhh, I messed up on this intermediate patch: it doesn't quite stand alone as
it should, as you noticed. To correct this, I can do one of the following:
a) move the new pin*() routines into the later patch 16 ("mm/gup:
track FOLL_PIN pages"), or
b) do a temporary thing here, such as setting FOLL_GET and adding a TODO,
within the pin*() implementations. And this switching it over to FOLL_PIN
in patch 16.
I'm thinking (a) is less error-prone, so I'm going with that unless someone
points out that that is stupid. :)
...
I was somewhat wondering about the number of functions you add here. So we
have:>
pin_user_pages()
pin_user_pages_fast()
pin_user_pages_remote()
and then longterm variants:
pin_longterm_pages()
pin_longterm_pages_fast()
pin_longterm_pages_remote()
and obviously we have gup like:
get_user_pages()
get_user_pages_fast()
get_user_pages_remote()
... and some other gup variants ...
I think we really should have pin_* vs get_* variants as they are very
different in terms of guarantees and after conversion, any use of get_*
variant in non-mm code should be closely scrutinized. OTOH pin_longterm_*
don't look *that* useful to me and just using pin_* instead with
FOLL_LONGTERM flag would look OK to me and somewhat reduce the number of
functions which is already large enough? What do people think? I don't feel
too strongly about this but wanted to bring this up.
Honza
Sounds just right to me, and I see that Dan and Ira also like it.
So I'll proceed with that.
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 02/23] mm/gup: factor out duplicate code from four routines
On 11/13/19 3:15 AM, Jan Kara wrote:
On Tue 12-11-19 20:26:49, John Hubbard wrote:
There are four locations in gup.c that have a fair amount of code
duplication. This means that changing one requires making the same
changes in four places, not to mention reading the same code four
times, and wondering if there are subtle differences.
Factor out the common code into static functions, thus reducing the
overall line count and the code's complexity.
Also, take the opportunity to slightly improve the efficiency of the
error cases, by doing a mass subtraction of the refcount, surrounded
by get_page()/put_page().
Also, further simplify (slightly), by waiting until the the successful
end of each routine, to increment *nr.
Reviewed-by: Jérôme Glisse
Cc: Ira Weiny
Cc: Christoph Hellwig
Cc: Aneesh Kumar K.V
Signed-off-by: John Hubbard
diff --git a/mm/gup.c b/mm/gup.c
index 85caf76b3012..199da99e8ffc 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1969,6 +1969,34 @@ static int __gup_device_huge_pud(pud_t pud, pud_t *pudp,
unsigned long addr,
}
#endif
+static int __record_subpages(struct page *page, unsigned long addr,
+unsigned long end, struct page **pages, int nr)
+{
+ int nr_recorded_pages = 0;
+
+ do {
+ pages[nr] = page;
+ nr++;
+ page++;
+ nr_recorded_pages++;
+ } while (addr += PAGE_SIZE, addr != end);
+ return nr_recorded_pages;
+}
Why don't you pass in already pages + nr?
Aha, that does save a function argument. Will do.
...
+static void __huge_pt_done(struct page *head, int nr_recorded_pages, int *nr)
+{
+ *nr += nr_recorded_pages;
+ SetPageReferenced(head);
+}
I don't find this last helper very useful. It seems to muddy water more
than necessary...
Yes, I suspect it's rather unloved, and the fact that it was hard to accurately
name should have been a big hint to not do it. I'll remove the helper and
put the lines back in directly.
thanks,
--
John Hubbard
NVIDIA
Other than that the cleanup looks nice to me.
Honza
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[radeon-alex:amd-mainline-dkms-5.2 2163/2834] include/kcl/kcl_drm.h:614:19: error: static declaration of 'drm_crtc_accurate_vblank_count' follows non-static declaration
Hi Adam,
FYI, the error/warning still remains.
tree: git://people.freedesktop.org/~agd5f/linux.git amd-mainline-dkms-5.2
head: a48b0cc1cdf3900e3e73801f9de64afbb70dc193
commit: ce2ac7946ad9ad370cf8212eab6d5651ba5520c6 [2163/2834] drm/amdkcl: use
kcl to leverage drm_crtc_accurate_vblank_count change
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
git checkout ce2ac7946ad9ad370cf8212eab6d5651ba5520c6
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
int drm_universal_plane_init(struct drm_device *dev,
^~~~
In file included from drivers/gpu/drm/ttm/backport/backport.h:6:0,
from :0:
include/kcl/kcl_drm.h: In function 'kcl_drm_gem_object_lookup':
include/kcl/kcl_drm.h:306:32: error: passing argument 1 of
'drm_gem_object_lookup' from incompatible pointer type
[-Werror=incompatible-pointer-types]
return drm_gem_object_lookup(dev, filp, handle);
^~~
In file included from include/kcl/kcl_drm.h:9:0,
from drivers/gpu/drm/ttm/backport/backport.h:6,
from :0:
include/drm/drm_gem.h:386:24: note: expected 'struct drm_file *' but
argument is of type 'struct drm_device *'
struct drm_gem_object *drm_gem_object_lookup(struct drm_file *filp, u32
handle);
^
In file included from drivers/gpu/drm/ttm/backport/backport.h:6:0,
from :0:
include/kcl/kcl_drm.h:306:37: warning: passing argument 2 of
'drm_gem_object_lookup' makes integer from pointer without a cast
[-Wint-conversion]
return drm_gem_object_lookup(dev, filp, handle);
^~~~
In file included from include/kcl/kcl_drm.h:9:0,
from drivers/gpu/drm/ttm/backport/backport.h:6,
from :0:
include/drm/drm_gem.h:386:24: note: expected 'u32 {aka unsigned int}' but
argument is of type 'struct drm_file *'
struct drm_gem_object *drm_gem_object_lookup(struct drm_file *filp, u32
handle);
^
In file included from drivers/gpu/drm/ttm/backport/backport.h:6:0,
from :0:
include/kcl/kcl_drm.h:306:10: error: too many arguments to function
'drm_gem_object_lookup'
return drm_gem_object_lookup(dev, filp, handle);
^
In file included from include/kcl/kcl_drm.h:9:0,
from drivers/gpu/drm/ttm/backport/backport.h:6,
from :0:
include/drm/drm_gem.h:386:24: note: declared here
struct drm_gem_object *drm_gem_object_lookup(struct drm_file *filp, u32
handle);
^
In file included from drivers/gpu/drm/ttm/backport/backport.h:6:0,
from :0:
include/kcl/kcl_drm.h: At top level:
include/kcl/kcl_drm.h:315:8: error: redefinition of 'struct
drm_format_name_buf'
struct drm_format_name_buf {
^~~
In file included from include/drm/drmP.h:69:0,
from include/kcl/kcl_drm.h:6,
from drivers/gpu/drm/ttm/backport/backport.h:6,
from :0:
include/drm/drm_fourcc.h:142:8: note: originally defined here
struct drm_format_name_buf {
^~~
In file included from drivers/gpu/drm/ttm/backport/backport.h:6:0,
from :0:
include/kcl/kcl_drm.h: In function 'kcl_drm_gem_object_put_unlocked':
include/kcl/kcl_drm.h:347:9: error: implicit declaration of function
'drm_gem_object_unreference_unlocked'; did you mean
'drm_gem_object_put_unlocked'? [-Werror=implicit-function-declaration]
return drm_gem_object_unreference_unlocked(obj);
^~~
drm_gem_object_put_unlocked
include/kcl/kcl_drm.h:347:9: warning: 'return' with a value, in function
returning void
return drm_gem_object_unreference_unlocked(obj);
^~~~
include/kcl/kcl_drm.h:344:20: note: declared here
static inline void kcl_drm_gem_object_put_unlocked(struct drm_gem_object
*obj)
^~~
include/kcl/kcl_drm.h: At top level:
include/kcl/kcl_drm.h:532:34: error: redefinition of 'drm_debug_printer'
static inline struct drm_printer drm_debug_printer(const char *prefix)
^
In file included from include/drm/drm_mm.h:49:0,
from include/drm/drm_vma_manager.h:26,
from include/kcl/kcl_drm_vma_manager.h:8,
from drivers/gpu/drm/ttm/backport/backport.h:5,
from :0:
Re: [PATCH v4 04/23] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On Wed, Nov 13, 2019 at 02:00:06PM -0800, Dan Williams wrote:
> On Wed, Nov 13, 2019 at 11:23 AM Dan Williams
> wrote:
> >
> > On Tue, Nov 12, 2019 at 8:27 PM John Hubbard wrote:
> > >
> > > An upcoming patch changes and complicates the refcounting and
> > > especially the "put page" aspects of it. In order to keep
> > > everything clean, refactor the devmap page release routines:
> > >
> > > * Rename put_devmap_managed_page() to page_is_devmap_managed(),
> > > and limit the functionality to "read only": return a bool,
> > > with no side effects.
> > >
> > > * Add a new routine, put_devmap_managed_page(), to handle checking
> > > what kind of page it is, and what kind of refcount handling it
> > > requires.
> > >
> > > * Rename __put_devmap_managed_page() to free_devmap_managed_page(),
> > > and limit the functionality to unconditionally freeing a devmap
> > > page.
> > >
> > > This is originally based on a separate patch by Ira Weiny, which
> > > applied to an early version of the put_user_page() experiments.
> > > Since then, Jérôme Glisse suggested the refactoring described above.
> > >
> > > Suggested-by: Jérôme Glisse
> > > Signed-off-by: Ira Weiny
> > > Signed-off-by: John Hubbard
> > > ---
> > > include/linux/mm.h | 27 ---
> > > mm/memremap.c | 67 --
> > > 2 files changed, 53 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index a2adf95b3f9c..96228376139c 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -967,9 +967,10 @@ static inline bool is_zone_device_page(const struct
> > > page *page)
> > > #endif
> > >
> > > #ifdef CONFIG_DEV_PAGEMAP_OPS
> > > -void __put_devmap_managed_page(struct page *page);
> > > +void free_devmap_managed_page(struct page *page);
> > > DECLARE_STATIC_KEY_FALSE(devmap_managed_key);
> > > -static inline bool put_devmap_managed_page(struct page *page)
> > > +
> > > +static inline bool page_is_devmap_managed(struct page *page)
> > > {
> > > if (!static_branch_unlikely(_managed_key))
> > > return false;
> > > @@ -978,7 +979,6 @@ static inline bool put_devmap_managed_page(struct
> > > page *page)
> > > switch (page->pgmap->type) {
> > > case MEMORY_DEVICE_PRIVATE:
> > > case MEMORY_DEVICE_FS_DAX:
> > > - __put_devmap_managed_page(page);
> > > return true;
> > > default:
> > > break;
> > > @@ -986,6 +986,27 @@ static inline bool put_devmap_managed_page(struct
> > > page *page)
> > > return false;
> > > }
> > >
> > > +static inline bool put_devmap_managed_page(struct page *page)
> > > +{
> > > + bool is_devmap = page_is_devmap_managed(page);
> > > +
> > > + if (is_devmap) {
> > > + int count = page_ref_dec_return(page);
> > > +
> > > + /*
> > > +* devmap page refcounts are 1-based, rather than
> > > 0-based: if
> > > +* refcount is 1, then the page is free and the refcount
> > > is
> > > +* stable because nobody holds a reference on the page.
> > > +*/
> > > + if (count == 1)
> > > + free_devmap_managed_page(page);
> > > + else if (!count)
> > > + __put_page(page);
> > > + }
> > > +
> > > + return is_devmap;
> > > +}
> > > +
> > > #else /* CONFIG_DEV_PAGEMAP_OPS */
> > > static inline bool put_devmap_managed_page(struct page *page)
> > > {
> > > diff --git a/mm/memremap.c b/mm/memremap.c
> > > index 03ccbdfeb697..bc7e2a27d025 100644
> > > --- a/mm/memremap.c
> > > +++ b/mm/memremap.c
> > > @@ -410,48 +410,39 @@ struct dev_pagemap *get_dev_pagemap(unsigned long
> > > pfn,
> > > EXPORT_SYMBOL_GPL(get_dev_pagemap);
> > >
> > > #ifdef CONFIG_DEV_PAGEMAP_OPS
> > > -void __put_devmap_managed_page(struct page *page)
> > > +void free_devmap_managed_page(struct page *page)
> > > {
> > > - int count = page_ref_dec_return(page);
> > > + /* Clear Active bit in case of parallel mark_page_accessed */
> > > + __ClearPageActive(page);
> > > + __ClearPageWaiters(page);
> > > +
> > > + mem_cgroup_uncharge(page);
> >
> > Ugh, when did all this HMM specific manipulation sneak into the
> > generic ZONE_DEVICE path? It used to be gated by pgmap type with its
> > own put_zone_device_private_page(). For example it's certainly
> > unnecessary and might be broken (would need to check) to call
> > mem_cgroup_uncharge() on a DAX page. ZONE_DEVICE users are not a
> > monolith and the HMM use case leaks pages into code paths that DAX
> > explicitly avoids.
>
> It's been this way for a while and I did not react previously,
> apologies for that. I think __ClearPageActive, __ClearPageWaiters, and
> mem_cgroup_uncharge, belong behind a device-private conditional. The
> history here is:
>
> Move some, but
Re: [PATCH v4 04/23] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On 11/13/19 2:55 PM, Dan Williams wrote: On Wed, Nov 13, 2019 at 2:49 PM John Hubbard wrote: On 11/13/19 2:00 PM, Dan Williams wrote: ... Ugh, when did all this HMM specific manipulation sneak into the generic ZONE_DEVICE path? It used to be gated by pgmap type with its own put_zone_device_private_page(). For example it's certainly unnecessary and might be broken (would need to check) to call mem_cgroup_uncharge() on a DAX page. ZONE_DEVICE users are not a monolith and the HMM use case leaks pages into code paths that DAX explicitly avoids. It's been this way for a while and I did not react previously, apologies for that. I think __ClearPageActive, __ClearPageWaiters, and mem_cgroup_uncharge, belong behind a device-private conditional. The history here is: Move some, but not all HMM specifics to hmm_devmem_free(): 2fa147bdbf67 mm, dev_pagemap: Do not clear ->mapping on final put Remove the clearing of mapping since no upstream consumers needed it: b7a523109fb5 mm: don't clear ->mapping in hmm_devmem_free Add it back in once an upstream consumer arrived: 7ab0ad0e74f8 mm/hmm: fix ZONE_DEVICE anon page mapping reuse We're now almost entirely free of ->page_free callbacks except for that weird nouveau case, can that FIXME in nouveau_dmem_page_free() also result in killing the ->page_free() callback altogether? In the meantime I'm proposing a cleanup like this: OK, assuming this is acceptable (no obvious problems jump out at me, and we can also test it with HMM), then how would you like to proceed, as far as patches go: add such a patch as part of this series here, or as a stand-alone patch either before or after this series? Or something else? And did you plan on sending it out as such? I think it makes sense to include it in your series since you're looking to refactor the implementation. I can send you one based on current linux-next as a lead-in cleanup before the refactor. Does that work for you? That would be perfect! Also, the diffs didn't quite make it through intact to my "git apply", so I'm re-posting the diff in hopes that this time it survives: Apologies for that. For quick "how about this" patch examples, I just copy and paste into gmail and it sometimes clobbers it. No problem at all, I do the same thing and *usually* it works. ha. And as you say, it's good enough for discussions. thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 04/23] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On Wed, Nov 13, 2019 at 2:49 PM John Hubbard wrote:
>
> On 11/13/19 2:00 PM, Dan Williams wrote:
> ...
> >> Ugh, when did all this HMM specific manipulation sneak into the
> >> generic ZONE_DEVICE path? It used to be gated by pgmap type with its
> >> own put_zone_device_private_page(). For example it's certainly
> >> unnecessary and might be broken (would need to check) to call
> >> mem_cgroup_uncharge() on a DAX page. ZONE_DEVICE users are not a
> >> monolith and the HMM use case leaks pages into code paths that DAX
> >> explicitly avoids.
> >
> > It's been this way for a while and I did not react previously,
> > apologies for that. I think __ClearPageActive, __ClearPageWaiters, and
> > mem_cgroup_uncharge, belong behind a device-private conditional. The
> > history here is:
> >
> > Move some, but not all HMM specifics to hmm_devmem_free():
> > 2fa147bdbf67 mm, dev_pagemap: Do not clear ->mapping on final put
> >
> > Remove the clearing of mapping since no upstream consumers needed it:
> > b7a523109fb5 mm: don't clear ->mapping in hmm_devmem_free
> >
> > Add it back in once an upstream consumer arrived:
> > 7ab0ad0e74f8 mm/hmm: fix ZONE_DEVICE anon page mapping reuse
> >
> > We're now almost entirely free of ->page_free callbacks except for
> > that weird nouveau case, can that FIXME in nouveau_dmem_page_free()
> > also result in killing the ->page_free() callback altogether? In the
> > meantime I'm proposing a cleanup like this:
>
>
> OK, assuming this is acceptable (no obvious problems jump out at me,
> and we can also test it with HMM), then how would you like to proceed, as
> far as patches go: add such a patch as part of this series here, or as a
> stand-alone patch either before or after this series? Or something else?
> And did you plan on sending it out as such?
I think it makes sense to include it in your series since you're
looking to refactor the implementation. I can send you one based on
current linux-next as a lead-in cleanup before the refactor. Does that
work for you?
>
> Also, the diffs didn't quite make it through intact to my "git apply", so
> I'm re-posting the diff in hopes that this time it survives:
Apologies for that. For quick "how about this" patch examples, I just
copy and paste into gmail and it sometimes clobbers it.
>
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index f9f76f6ba07b..21db1ce8c0ae 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -338,13 +338,7 @@ static void pmem_release_disk(void *__pmem)
> put_disk(pmem->disk);
> }
>
> -static void pmem_pagemap_page_free(struct page *page)
> -{
> - wake_up_var(>_refcount);
> -}
> -
> static const struct dev_pagemap_ops fsdax_pagemap_ops = {
> - .page_free = pmem_pagemap_page_free,
> .kill = pmem_pagemap_kill,
> .cleanup= pmem_pagemap_cleanup,
> };
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 03ccbdfeb697..157edb8f7cf8 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -419,12 +419,6 @@ void __put_devmap_managed_page(struct page *page)
> * holds a reference on the page.
> */
> if (count == 1) {
> - /* Clear Active bit in case of parallel mark_page_accessed */
> - __ClearPageActive(page);
> - __ClearPageWaiters(page);
> -
> - mem_cgroup_uncharge(page);
> -
> /*
> * When a device_private page is freed, the page->mapping
> field
> * may still contain a (stale) mapping value. For example, the
> @@ -446,10 +440,17 @@ void __put_devmap_managed_page(struct page *page)
> * handled differently or not done at all, so there is no need
> * to clear page->mapping.
> */
> - if (is_device_private_page(page))
> - page->mapping = NULL;
> + if (is_device_private_page(page)) {
> + /* Clear Active bit in case of parallel
> mark_page_accessed */
> + __ClearPageActive(page);
> + __ClearPageWaiters(page);
>
> - page->pgmap->ops->page_free(page);
> + mem_cgroup_uncharge(page);
> +
> + page->mapping = NULL;
> + page->pgmap->ops->page_free(page);
> + } else
> + wake_up_var(>_refcount);
> } else if (!count)
> __put_page(page);
> }
> --
> 2.24.0
>
>
> thanks,
> --
> John Hubbard
> NVIDIA
>
> >
> > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> > index ad8e4df1282b..4eae441f86c9 100644
> > --- a/drivers/nvdimm/pmem.c
> > +++ b/drivers/nvdimm/pmem.c
> > @@ -337,13 +337,7 @@ static void pmem_release_disk(void *__pmem)
> > put_disk(pmem->disk);
> > }
> >
> > -static void pmem_pagemap_page_free(struct page *page)
> > -{
> > -
Re: [PATCH v4 04/23] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On 11/13/19 2:00 PM, Dan Williams wrote:
...
Ugh, when did all this HMM specific manipulation sneak into the
generic ZONE_DEVICE path? It used to be gated by pgmap type with its
own put_zone_device_private_page(). For example it's certainly
unnecessary and might be broken (would need to check) to call
mem_cgroup_uncharge() on a DAX page. ZONE_DEVICE users are not a
monolith and the HMM use case leaks pages into code paths that DAX
explicitly avoids.
It's been this way for a while and I did not react previously,
apologies for that. I think __ClearPageActive, __ClearPageWaiters, and
mem_cgroup_uncharge, belong behind a device-private conditional. The
history here is:
Move some, but not all HMM specifics to hmm_devmem_free():
2fa147bdbf67 mm, dev_pagemap: Do not clear ->mapping on final put
Remove the clearing of mapping since no upstream consumers needed it:
b7a523109fb5 mm: don't clear ->mapping in hmm_devmem_free
Add it back in once an upstream consumer arrived:
7ab0ad0e74f8 mm/hmm: fix ZONE_DEVICE anon page mapping reuse
We're now almost entirely free of ->page_free callbacks except for
that weird nouveau case, can that FIXME in nouveau_dmem_page_free()
also result in killing the ->page_free() callback altogether? In the
meantime I'm proposing a cleanup like this:
OK, assuming this is acceptable (no obvious problems jump out at me,
and we can also test it with HMM), then how would you like to proceed, as
far as patches go: add such a patch as part of this series here, or as a
stand-alone patch either before or after this series? Or something else?
And did you plan on sending it out as such?
Also, the diffs didn't quite make it through intact to my "git apply", so
I'm re-posting the diff in hopes that this time it survives:
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index f9f76f6ba07b..21db1ce8c0ae 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -338,13 +338,7 @@ static void pmem_release_disk(void *__pmem)
put_disk(pmem->disk);
}
-static void pmem_pagemap_page_free(struct page *page)
-{
- wake_up_var(>_refcount);
-}
-
static const struct dev_pagemap_ops fsdax_pagemap_ops = {
- .page_free = pmem_pagemap_page_free,
.kill = pmem_pagemap_kill,
.cleanup= pmem_pagemap_cleanup,
};
diff --git a/mm/memremap.c b/mm/memremap.c
index 03ccbdfeb697..157edb8f7cf8 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -419,12 +419,6 @@ void __put_devmap_managed_page(struct page *page)
* holds a reference on the page.
*/
if (count == 1) {
- /* Clear Active bit in case of parallel mark_page_accessed */
- __ClearPageActive(page);
- __ClearPageWaiters(page);
-
- mem_cgroup_uncharge(page);
-
/*
* When a device_private page is freed, the page->mapping field
* may still contain a (stale) mapping value. For example, the
@@ -446,10 +440,17 @@ void __put_devmap_managed_page(struct page *page)
* handled differently or not done at all, so there is no need
* to clear page->mapping.
*/
- if (is_device_private_page(page))
- page->mapping = NULL;
+ if (is_device_private_page(page)) {
+ /* Clear Active bit in case of parallel
mark_page_accessed */
+ __ClearPageActive(page);
+ __ClearPageWaiters(page);
- page->pgmap->ops->page_free(page);
+ mem_cgroup_uncharge(page);
+
+ page->mapping = NULL;
+ page->pgmap->ops->page_free(page);
+ } else
+ wake_up_var(>_refcount);
} else if (!count)
__put_page(page);
}
--
2.24.0
thanks,
--
John Hubbard
NVIDIA
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index ad8e4df1282b..4eae441f86c9 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -337,13 +337,7 @@ static void pmem_release_disk(void *__pmem)
put_disk(pmem->disk);
}
-static void pmem_pagemap_page_free(struct page *page)
-{
- wake_up_var(>_refcount);
-}
-
static const struct dev_pagemap_ops fsdax_pagemap_ops = {
- .page_free = pmem_pagemap_page_free,
.kill = pmem_pagemap_kill,
.cleanup= pmem_pagemap_cleanup,
};
diff --git a/mm/memremap.c b/mm/memremap.c
index 03ccbdfeb697..157edb8f7cf8 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -419,12 +419,6 @@ void __put_devmap_managed_page(struct page *page)
* holds a reference on the page.
*/
if (count == 1) {
- /* Clear Active bit in case of parallel mark_page_accessed */
- __ClearPageActive(page);
- __ClearPageWaiters(page);
-
-
[CI v2] drm/fbdev: Fallback to non tiled mode if all tiles not present
In case of tiled displays, if we hotplug just one connector,
fbcon currently just selects the preferred mode and if it is
tiled mode then that becomes a problem if rest of the tiles are
not present.
So in the fbdev driver on hotplug when we probe the client modeset,
if we dont find all the connectors for all tiles, then on a connector
with one tile, just fallback to the first available non tiled mode
to display over a single connector.
On the hotplug of the consecutive tiled connectors, if the tiled mode
no longer exists because of fbcon size limitation, then return
no modes for consecutive tiles but retain the non tiled mode
on the 0th tile.
Use the same logic in case of connected boot case as well.
This has been tested with Dell UP328K tiled monitor.
v2:
* Set the modes on consecutive hotplugged tiles to no mode
if tiled mode is pruned (Dave)
v1:
* Just handle the 1st connector hotplug case
* v1 Reviewed-by: Dave Airlie
Suggested-by: Ville Syrjälä
Suggested-by: Dave Airlie
Cc: Ville Syrjälä
Cc: Dave Airlie
Signed-off-by: Manasi Navare
Reviewed-by: Dave Airlie
---
drivers/gpu/drm/drm_client_modeset.c | 70
1 file changed, 70 insertions(+)
diff --git a/drivers/gpu/drm/drm_client_modeset.c
b/drivers/gpu/drm/drm_client_modeset.c
index 895b73f23079..f2150a0bac4c 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -114,6 +114,33 @@ drm_client_find_modeset(struct drm_client_dev *client,
struct drm_crtc *crtc)
return NULL;
}
+static struct drm_display_mode *
+drm_connector_get_tiled_mode(struct drm_connector *connector)
+{
+ struct drm_display_mode *mode;
+
+ list_for_each_entry(mode, >modes, head) {
+ if (mode->hdisplay == connector->tile_h_size &&
+ mode->vdisplay == connector->tile_v_size)
+ return mode;
+ }
+ return NULL;
+}
+
+static struct drm_display_mode *
+drm_connector_fallback_non_tiled_mode(struct drm_connector *connector)
+{
+ struct drm_display_mode *mode;
+
+ list_for_each_entry(mode, >modes, head) {
+ if (mode->hdisplay == connector->tile_h_size &&
+ mode->vdisplay == connector->tile_v_size)
+ continue;
+ return mode;
+ }
+ return NULL;
+}
+
static struct drm_display_mode *
drm_connector_has_preferred_mode(struct drm_connector *connector, int width,
int height)
{
@@ -348,8 +375,14 @@ static bool drm_client_target_preferred(struct
drm_connector **connectors,
struct drm_connector *connector;
u64 conn_configured = 0;
int tile_pass = 0;
+ int num_tiled_conns = 0;
int i;
+ for (i = 0; i < connector_count; i++) {
+ if (connectors[i]->has_tile)
+ num_tiled_conns++;
+ }
+
retry:
for (i = 0; i < connector_count; i++) {
connector = connectors[i];
@@ -399,6 +432,28 @@ static bool drm_client_target_preferred(struct
drm_connector **connectors,
list_for_each_entry(modes[i], >modes, head)
break;
}
+ /*
+* In case of tiled mode if all tiles not present fallback to
+* first available non tiled mode.
+* After all tiles are present, try to find the tiled mode
+* for all and if tiled mode not present due to fbcon size
+* limitations, use first non tiled mode only for
+* tile 0,0 and set to no mode for all other tiles.
+*/
+ if (connector->has_tile) {
+ if (num_tiled_conns <
+ connector->num_h_tile * connector->num_v_tile ||
+ (connector->tile_h_loc == 0 &&
+connector->tile_v_loc == 0 &&
+!drm_connector_get_tiled_mode(connector))) {
+ DRM_DEBUG_KMS("Falling back to non tiled mode
on Connector %d\n",
+ connector->base.id);
+ modes[i] =
drm_connector_fallback_non_tiled_mode(connector);
+ } else {
+ modes[i] =
drm_connector_get_tiled_mode(connector);
+ }
+ }
+
DRM_DEBUG_KMS("found mode %s\n", modes[i] ? modes[i]->name :
"none");
conn_configured |= BIT_ULL(i);
@@ -515,6 +570,7 @@ static bool drm_client_firmware_config(struct
drm_client_dev *client,
bool fallback = true, ret = true;
int num_connectors_enabled = 0;
int num_connectors_detected = 0;
+ int num_tiled_conns = 0;
struct drm_modeset_acquire_ctx ctx;
if (!drm_drv_uses_atomic_modeset(dev))
@@ -532,6 +588,10 @@ static bool
[Bug 112265] Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
https://bugs.freedesktop.org/show_bug.cgi?id=112265 --- Comment #3 from john.p.donne...@oracle.com --- Created attachment 145950 --> https://bugs.freedesktop.org/attachment.cgi?id=145950=edit Running startx on the console This likely doesn't help much On a 4.18 kernel ; when I do "startx" on the console ; it eventually runs gnone. On the bad kernel ; I just see x11 noise ; then nothing . -- 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 v4 04/23] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On Wed, Nov 13, 2019 at 11:23 AM Dan Williams wrote:
>
> On Tue, Nov 12, 2019 at 8:27 PM John Hubbard wrote:
> >
> > An upcoming patch changes and complicates the refcounting and
> > especially the "put page" aspects of it. In order to keep
> > everything clean, refactor the devmap page release routines:
> >
> > * Rename put_devmap_managed_page() to page_is_devmap_managed(),
> > and limit the functionality to "read only": return a bool,
> > with no side effects.
> >
> > * Add a new routine, put_devmap_managed_page(), to handle checking
> > what kind of page it is, and what kind of refcount handling it
> > requires.
> >
> > * Rename __put_devmap_managed_page() to free_devmap_managed_page(),
> > and limit the functionality to unconditionally freeing a devmap
> > page.
> >
> > This is originally based on a separate patch by Ira Weiny, which
> > applied to an early version of the put_user_page() experiments.
> > Since then, Jérôme Glisse suggested the refactoring described above.
> >
> > Suggested-by: Jérôme Glisse
> > Signed-off-by: Ira Weiny
> > Signed-off-by: John Hubbard
> > ---
> > include/linux/mm.h | 27 ---
> > mm/memremap.c | 67 --
> > 2 files changed, 53 insertions(+), 41 deletions(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index a2adf95b3f9c..96228376139c 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -967,9 +967,10 @@ static inline bool is_zone_device_page(const struct
> > page *page)
> > #endif
> >
> > #ifdef CONFIG_DEV_PAGEMAP_OPS
> > -void __put_devmap_managed_page(struct page *page);
> > +void free_devmap_managed_page(struct page *page);
> > DECLARE_STATIC_KEY_FALSE(devmap_managed_key);
> > -static inline bool put_devmap_managed_page(struct page *page)
> > +
> > +static inline bool page_is_devmap_managed(struct page *page)
> > {
> > if (!static_branch_unlikely(_managed_key))
> > return false;
> > @@ -978,7 +979,6 @@ static inline bool put_devmap_managed_page(struct page
> > *page)
> > switch (page->pgmap->type) {
> > case MEMORY_DEVICE_PRIVATE:
> > case MEMORY_DEVICE_FS_DAX:
> > - __put_devmap_managed_page(page);
> > return true;
> > default:
> > break;
> > @@ -986,6 +986,27 @@ static inline bool put_devmap_managed_page(struct page
> > *page)
> > return false;
> > }
> >
> > +static inline bool put_devmap_managed_page(struct page *page)
> > +{
> > + bool is_devmap = page_is_devmap_managed(page);
> > +
> > + if (is_devmap) {
> > + int count = page_ref_dec_return(page);
> > +
> > + /*
> > +* devmap page refcounts are 1-based, rather than 0-based:
> > if
> > +* refcount is 1, then the page is free and the refcount is
> > +* stable because nobody holds a reference on the page.
> > +*/
> > + if (count == 1)
> > + free_devmap_managed_page(page);
> > + else if (!count)
> > + __put_page(page);
> > + }
> > +
> > + return is_devmap;
> > +}
> > +
> > #else /* CONFIG_DEV_PAGEMAP_OPS */
> > static inline bool put_devmap_managed_page(struct page *page)
> > {
> > diff --git a/mm/memremap.c b/mm/memremap.c
> > index 03ccbdfeb697..bc7e2a27d025 100644
> > --- a/mm/memremap.c
> > +++ b/mm/memremap.c
> > @@ -410,48 +410,39 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn,
> > EXPORT_SYMBOL_GPL(get_dev_pagemap);
> >
> > #ifdef CONFIG_DEV_PAGEMAP_OPS
> > -void __put_devmap_managed_page(struct page *page)
> > +void free_devmap_managed_page(struct page *page)
> > {
> > - int count = page_ref_dec_return(page);
> > + /* Clear Active bit in case of parallel mark_page_accessed */
> > + __ClearPageActive(page);
> > + __ClearPageWaiters(page);
> > +
> > + mem_cgroup_uncharge(page);
>
> Ugh, when did all this HMM specific manipulation sneak into the
> generic ZONE_DEVICE path? It used to be gated by pgmap type with its
> own put_zone_device_private_page(). For example it's certainly
> unnecessary and might be broken (would need to check) to call
> mem_cgroup_uncharge() on a DAX page. ZONE_DEVICE users are not a
> monolith and the HMM use case leaks pages into code paths that DAX
> explicitly avoids.
It's been this way for a while and I did not react previously,
apologies for that. I think __ClearPageActive, __ClearPageWaiters, and
mem_cgroup_uncharge, belong behind a device-private conditional. The
history here is:
Move some, but not all HMM specifics to hmm_devmem_free():
2fa147bdbf67 mm, dev_pagemap: Do not clear ->mapping on final put
Remove the clearing of mapping since no upstream consumers needed it:
b7a523109fb5 mm: don't clear ->mapping in hmm_devmem_free
Add it back in once an upstream consumer arrived:
7ab0ad0e74f8 mm/hmm:
[Bug 112265] Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
https://bugs.freedesktop.org/show_bug.cgi?id=112265 --- Comment #2 from john.p.donne...@oracle.com --- debugfs content : With gnome running # for f in `find . -type f ` ; do > echo "$f : `cat $f` " > done ./VGA-1/edid_override : ./VGA-1/force : unspecified ./internal_clients : ./framebuffer : framebuffer[35]: allocated by = Xorg refcount=2 format=XR24 little-endian (0x34325258) modifier=0x0 size=1024x768 layers: size[0]=1024x768 pitch[0]=4096 offset[0]=0 obj[0]:(null) framebuffer[34]: allocated by = [fbcon] refcount=1 format=XR24 little-endian (0x34325258) modifier=0xb7e2c7450010 size=1024x768 layers: size[0]=1024x768 pitch[0]=4096 offset[0]=4294967295 obj[0]:(null) ./gem_names :name size handles refcount ./clients : command pid dev master a uid magic systemd-logind 1563 0 yy 0 0 ./name : mgag200 dev=:3d:00.0 unique=:3d:00.0 -- 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: Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
> On Nov 13, 2019, at 2:06 PM, Daniel Vetter wrote: > > On Wed, Nov 13, 2019 at 6:42 PM John Donnelly > wrote: >> >> Hi Thomas: See below >> >>> On Nov 13, 2019, at 2:17 AM, Thomas Zimmermann wrote: >>> >>> Hi John >>> >>> Am 12.11.19 um 20:13 schrieb John Donnelly: In short . I started with : git bisect start git bisect bad be8454afc50f git bisect good fec88ab0af97 And at the the end of bisects showed this was the offending commit : c0a74c732568 commit c0a74c732568ad347f7b3de281922808dab30504 (refs/bisect/bad) Author: Jani Nikula Date: Fri May 24 20:35:22 2019 +0300 drm/i915: Update DRIVER_DATE to 20190524 Signed-off-by: Jani Nikula That does not have any real relevance >>> >>> No, that doesn't look right. The other bad commits you list below also >>> don't seem to be related. >> >> >> >> Thank you for your patience ; I started over and the bisect took to me to >> this change that certainly reflects the behavior I am seeing : >> >> commit 81da87f63a1edebcf8cbb811d387e353d9f89c7a (refs/bisect/bad) >> Author: Thomas Zimmermann >> Date: Tue May 21 13:08:29 2019 +0200 >> >>drm: Replace drm_gem_vram_push_to_system() with kunmap + unpin >> >>The push-to-system function forces a buffer out of video RAM. This >> decision >>should rather be made by the memory manager. By replacing the function >> with >>calls to the kunmap and unpin functions, the buffer's memory becomes >> available, >>but the buffer remains in VRAM until it's evicted by a pin operation. >> >>This patch replaces the remaining instances of >> drm_gem_vram_push_to_system() >>in ast and mgag200, and removes the function from DRM. > > Yeah that looks a lot more plausible as the culprit. > >> My 1st impression is we need a method that restores the previous behavior >> that pushes the content to the device . > > Can you pls grab the debugsfs for the above broken kernel (without any > of the later reworks on top), both for console mode and after you > started gnome. Plus I guess booting with drm.debug=0xfe and full dmesg > (please note the timestamp when you started gnome, that helps with > sifting through it all, it's going to be a lot). You might need to > grab the full dmesg from logs on disk, please make sure it includes > everything from boot-up. > > Thanks, Daniel Hi I started a Bugzilla https://bugs.freedesktop.org/show_bug.cgi?id=112265 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 112265] Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
https://bugs.freedesktop.org/show_bug.cgi?id=112265 john.p.donne...@oracle.com changed: What|Removed |Added CC||john.p.donne...@oracle.com --- Comment #1 from john.p.donne...@oracle.com --- Created attachment 145949 --> https://bugs.freedesktop.org/attachment.cgi?id=145949=edit dmesg and message file on bi-sected kernel Starting gnome See messages for " starting gnome " and " Stopping gnome " -- 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 112265] Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
https://bugs.freedesktop.org/show_bug.cgi?id=112265 Bug ID: 112265 Summary: Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: major Priority: not set Component: DRM/other Assignee: dri-devel@lists.freedesktop.org Reporter: john.p.donne...@oracle.com bisect took to me to this change that certainly reflects the behavior I am seeing : 5.1.0-rc5 commit 81da87f63a1edebcf8cbb811d387e353d9f89c7a (refs/bisect/bad) Author: Thomas Zimmermann Date: Tue May 21 13:08:29 2019 +0200 drm: Replace drm_gem_vram_push_to_system() with kunmap + unpin The push-to-system function forces a buffer out of video RAM. This decision should rather be made by the memory manager. By replacing the function with calls to the kunmap and unpin functions, the buffer's memory becomes available, but the buffer remains in VRAM until it's evicted by a pin operation. This patch replaces the remaining instances of drm_gem_vram_push_to_system() in ast and mgag200, and removes the function from DRM. My 1st impression is we need a method that restores the previous behavior that pushes the content to the device . I found this issue using gnome-desktop3-3.28.2-1.el8.x86_64 If there is a more specific. RPM I can look at for guidance I will . -- 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
[PULL] drm-misc-next-fixes
Hi Dave & Daniel, Just one msm patch this week. Looks like -misc is going to be perfect when merge window rolls around :-) drm-misc-next-fixes-2019-11-13: - Fix memory leak in gpu debugfs node's release (Johan) Cc: Johan Hovold Cheers, Sean The following changes since commit 3ca3a9eab7085b3c938b5d088c3020269cfecdc8: Merge tag 'drm-misc-next-fixes-2019-11-06' of git://anongit.freedesktop.org/drm/drm-misc into drm-next (2019-11-08 16:48:22 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2019-11-13 for you to fetch changes up to a64fc11b9a520c55ca34d82e5ca32274f49b6b15: drm/msm: fix memleak on release (2019-11-13 15:34:15 -0500) - Fix memory leak in gpu debugfs node's release (Johan) Cc: Johan Hovold Johan Hovold (1): drm/msm: fix memleak on release drivers/gpu/drm/msm/msm_debugfs.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/23] mm/gup: track dma-pinned pages: FOLL_PIN, FOLL_LONGTERM
On 11/13/19 3:43 AM, Daniel Vetter wrote: ... Can't we call this unpin_user_page then, for some symmetry? Or is that even more churn? Looking from afar the naming here seems really confusing. That look from afar is valuable, because I'm too close to the problem to see how the naming looks. :) unpin_user_page() sounds symmetrical. It's true that it would cause more churn (which is why I started off with a proposal that avoids changing the names of put_user_page*() APIs). But OTOH, the amount of churn is proportional to the change in direction here, and it's really only 10 or 20 lines changed, in the end. So I'm open to changing to that naming. It would be nice to hear what others prefer, too... FWIW I'd find unpin_user_page() also better than put_user_page() as a counterpart to pin_user_pages(). One more point from afar on pin/unpin: We use that a lot in graphics for permanently pinned graphics buffer objects. Which really only should be used for scanout. So at least graphics folks should have an appropriate mindset and try to make sure we don't overuse this stuff. -Daniel OK, Ira also likes "unpin", and so far no one has said *anything* in favor of the "put_user_page" names, so I think we have a winner! I'll change the names to unpin_user_page*(). thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 09/23] mm/gup: introduce pin_user_pages*() and FOLL_PIN
On 11/13/19 10:59 AM, Ira Weiny wrote: On Tue, Nov 12, 2019 at 08:26:56PM -0800, John Hubbard wrote: Introduce pin_user_pages*() variations of get_user_pages*() calls, and also pin_longterm_pages*() variations. These variants all set FOLL_PIN, which is also introduced, and thoroughly documented. The pin_longterm*() variants also set FOLL_LONGTERM, in addition to FOLL_PIN: pin_user_pages() pin_user_pages_remote() pin_user_pages_fast() pin_longterm_pages() pin_longterm_pages_remote() pin_longterm_pages_fast() At some point in this conversation I thought we were going to put in "unpin_*" versions of these. Is that still in the plans? Why yes it is! :) Daniel Vetter and Jan Kara both already weighed in [1], in favor of "unpin_user_page*()", rather than "put_user_page*()". I'll change those names. [1] https://lore.kernel.org/r/20191113101210.gd6...@quack2.suse.cz thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 08/23] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On 11/13/19 11:17 AM, Ira Weiny wrote:
...
@@ -348,33 +347,13 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
long vaddr,
flags |= FOLL_WRITE;
down_read(>mmap_sem);
- if (mm == current->mm) {
- ret = get_user_pages(vaddr, 1, flags | FOLL_LONGTERM, page,
-vmas);
- } else {
- ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags, page,
- vmas, NULL);
- /*
-* The lifetime of a vaddr_get_pfn() page pin is
-* userspace-controlled. In the fs-dax case this could
-* lead to indefinite stalls in filesystem operations.
-* Disallow attempts to pin fs-dax pages via this
-* interface.
-*/
- if (ret > 0 && vma_is_fsdax(vmas[0])) {
- ret = -EOPNOTSUPP;
- put_page(page[0]);
- }
- }
- up_read(>mmap_sem);
-
+ ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags | FOLL_LONGTERM,
+ page, NULL, NULL);
if (ret == 1) {
*pfn = page_to_pfn(page[0]);
return 0;
Mind the return with the lock held this needs some goto unwind
Ah yea... retract my reviewed by... :-(
ooops, embarrassed that I missed that, good catch. Will repost with it fixed.
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/vmwgfx: Use dma-coherent memory for high-bandwidth port messaging
Hi, Nathan,
On 11/13/19 9:01 PM, Nathan Chancellor wrote:
On Wed, Nov 13, 2019 at 10:51:42AM +0100, Thomas Hellström (VMware) wrote:
From: Thomas Hellstrom
With AMD-SEV high-bandwidth port messaging runs into trouble since the
message content is encrypted using the vm-specific key, and the
hypervisor is unable to read it.
So use unencrypted dma-coherent bounce buffers for temporary message
storage space. Allocating that memory is expensive so a future
optimization might include a static unencrypted memory area for messages.
Signed-off-by: Thomas Hellstrom
Reviewed-by: Brian Paul
Hi Thomas,
The 0day team has been doing clang builds for us and sending the results
to our mailing list for triage; this patch causes the following warning.
Seems legitimate, mind taking a look at it and resolving it how you see
fit?
Cheers,
Nathan
This should be harmless as dma_free_coherent() with reply == NULL is a
nop, but anyway I'll respin to silence the warning.
Thanks,
Thomas
On Thu, Nov 14, 2019 at 03:36:44AM +0800, kbuild test robot wrote:
CC: kbuild-...@lists.01.org
In-Reply-To: <20191113095144.2981-1-thomas...@shipmail.org>
References: <20191113095144.2981-1-thomas...@shipmail.org>
TO: "Thomas Hellström (VMware)"
CC: dri-devel@lists.freedesktop.org, Thomas Hellstrom , Brian Paul
, Thomas Hellstrom , Brian Paul
CC: Thomas Hellstrom , Brian Paul
Hi "Thomas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.4-rc7 next-20191113]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Thomas-Hellstr-m-VMware/drm-vmwgfx-Use-dma-coherent-memory-for-high-bandwidth-port-messaging/20191114-020818
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
0e3f1ad80fc8cb0c517fd9a9afb22752b741fa76
config: x86_64-rhel-7.6 (attached as .config)
compiler: clang version 10.0.0 (git://gitmirror/llvm_project
335ac2eb662ce5f1888e2a50310b01fba2d40d68)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>):
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:441:6: warning: variable 'reply_handle' is
used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (vmw_send_msg(, msg) ||
^~~
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:467:47: note: uninitialized use occurs
here
dma_free_coherent(dev, reply_len + 1, reply, reply_handle);
^~~~
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:441:6: note: remove the '||' if its
condition is always false
if (vmw_send_msg(, msg) ||
^~
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:421:37: note: initialize the variable
'reply_handle' to silence this warning
dma_addr_t req_handle, reply_handle;
^
= 0
1 warning generated.
vim +441 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
89da76fde68de1 Sinclair Yeh 2016-04-27 400
89da76fde68de1 Sinclair Yeh 2016-04-27 401
89da76fde68de1 Sinclair Yeh 2016-04-27 402 /**
89da76fde68de1 Sinclair Yeh 2016-04-27 403 * vmw_host_get_guestinfo:
Gets a GuestInfo parameter
89da76fde68de1 Sinclair Yeh 2016-04-27 404 *
89da76fde68de1 Sinclair Yeh 2016-04-27 405 * Gets the value of a
GuestInfo.* parameter. The value returned will be in
89da76fde68de1 Sinclair Yeh 2016-04-27 406 * a string, and it is up to
the caller to post-process.
89da76fde68de1 Sinclair Yeh 2016-04-27 407 *
6bdb21230a2a01 Thomas Hellstrom 2019-11-13 408 * @dev: Pointer to struct
device used for coherent memory allocation
89da76fde68de1 Sinclair Yeh 2016-04-27 409 * @guest_info_param:
Parameter to get, e.g. GuestInfo.svga.gl3
89da76fde68de1 Sinclair Yeh 2016-04-27 410 * @buffer: if NULL,
*reply_len will contain reply size.
89da76fde68de1 Sinclair Yeh 2016-04-27 411 * @length: size of the
reply_buf. Set to size of reply upon return
89da76fde68de1 Sinclair Yeh 2016-04-27 412 *
89da76fde68de1 Sinclair Yeh 2016-04-27 413 * Returns: 0 on success
89da76fde68de1 Sinclair Yeh 2016-04-27 414 */
6bdb21230a2a01 Thomas Hellstrom 2019-11-13 415 int
vmw_host_get_guestinfo(struct device *dev, const char *guest_info_param,
89da76fde68de1 Sinclair Yeh 2016-04-27 416 char
*buffer, size_t *length)
89da76fde68de1 Sinclair Yeh 2016-04-27 417 {
89da76fde6
Re: Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
On Wed, Nov 13, 2019 at 6:42 PM John Donnelly wrote: > > Hi Thomas: See below > > > On Nov 13, 2019, at 2:17 AM, Thomas Zimmermann wrote: > > > > Hi John > > > > Am 12.11.19 um 20:13 schrieb John Donnelly: > >> > >> In short . I started with : > >> > >> git bisect start > >> > >> git bisect bad be8454afc50f > >> > >> git bisect good fec88ab0af97 > >> > >> And at the the end of bisects showed this was the offending commit : > >> > >> c0a74c732568 > >> > >> commit c0a74c732568ad347f7b3de281922808dab30504 (refs/bisect/bad) > >> Author: Jani Nikula > >> Date: Fri May 24 20:35:22 2019 +0300 > >> > >>drm/i915: Update DRIVER_DATE to 20190524 > >> > >>Signed-off-by: Jani Nikula > >> > >> That does not have any real relevance > > > > No, that doesn't look right. The other bad commits you list below also > > don't seem to be related. > > > > Thank you for your patience ; I started over and the bisect took to me to > this change that certainly reflects the behavior I am seeing : > > commit 81da87f63a1edebcf8cbb811d387e353d9f89c7a (refs/bisect/bad) > Author: Thomas Zimmermann > Date: Tue May 21 13:08:29 2019 +0200 > > drm: Replace drm_gem_vram_push_to_system() with kunmap + unpin > > The push-to-system function forces a buffer out of video RAM. This > decision > should rather be made by the memory manager. By replacing the function > with > calls to the kunmap and unpin functions, the buffer's memory becomes > available, > but the buffer remains in VRAM until it's evicted by a pin operation. > > This patch replaces the remaining instances of > drm_gem_vram_push_to_system() > in ast and mgag200, and removes the function from DRM. Yeah that looks a lot more plausible as the culprit. > My 1st impression is we need a method that restores the previous behavior > that pushes the content to the device . Can you pls grab the debugsfs for the above broken kernel (without any of the later reworks on top), both for console mode and after you started gnome. Plus I guess booting with drm.debug=0xfe and full dmesg (please note the timestamp when you started gnome, that helps with sifting through it all, it's going to be a lot). You might need to grab the full dmesg from logs on disk, please make sure it includes everything from boot-up. Thanks, Daniel > > > > > > > > You could also bisect between your original working commit (v4.18?) and > > the one you want to upgrade to (v5.3?). It's only a few more builds but > > might be might give better results. > > > > BTW, are you also converting Gnome from X11 to Wayland. I found that > > Gnome's Wayland code is so slow on mgag200 that it's unusable. They > > recently added a shadow FB [1] to improve performance, but it won't be > > available before Gnome 3.36. > > > I found this issue using > > gnome-desktop3-3.28.2-1.el8.x86_64 > > If there is a more specific. RPM I can look at for guidance I will . > > > > > > Best regards > > Thomas > > > > [1] https://gitlab.gnome.org/GNOME/mutter/merge_requests/877/diffs > > > >> > >> > >> I am not sure if I did the bisects correctly . After each test I did : > >> > >> > >> #1 git bisect bad 827440a90146 > >> > >> #2 git bisect bad f5b07b04e5f0 > >> > >> #3 git bisect bad c0a74c732568 > >> > >> #4 git bisect good 818f5cb3e8fb > >> > >> #5 git bisect good 6cfe7ec02e85 > >> > >> #6 git bisect good f71e01a78bee > >> > >> #7 git bisect good 09a93ef3d60f > >> > >> #8 git bisect good f1e6b336bafa > >> > >> #9 git bisect good eaf20e6933dc > >> > >> #10 git bisect good 63e8dcdb4f8e > >> > >> #11 git bisect good 397049a03022 > >> > >> I’ve restarted the bisect without appending the after a the > >> “bad|good “ , and so far git is showing the same selections. > > > snip > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
> On Nov 13, 2019, at 11:42 AM, John Donnelly > wrote: > > Hi Thomas: See below > >> On Nov 13, 2019, at 2:17 AM, Thomas Zimmermann wrote: >> >> Hi John >> >> Am 12.11.19 um 20:13 schrieb John Donnelly: >>> >>> In short . I started with : >>> >>> git bisect start >>> >>> git bisect bad be8454afc50f >>> >>> git bisect good fec88ab0af97 >>> >>> And at the the end of bisects showed this was the offending commit : >>> >>> c0a74c732568 >>> >>> commit c0a74c732568ad347f7b3de281922808dab30504 (refs/bisect/bad) >>> Author: Jani Nikula >>> Date: Fri May 24 20:35:22 2019 +0300 >>> >>> drm/i915: Update DRIVER_DATE to 20190524 >>> >>> Signed-off-by: Jani Nikula >>> >>> That does not have any real relevance >> >> No, that doesn't look right. The other bad commits you list below also >> don't seem to be related. > > > > Thank you for your patience ; I started over and the bisect took to me to > this change that certainly reflects the behavior I am seeing : > > commit 81da87f63a1edebcf8cbb811d387e353d9f89c7a (refs/bisect/bad) > Author: Thomas Zimmermann > Date: Tue May 21 13:08:29 2019 +0200 > >drm: Replace drm_gem_vram_push_to_system() with kunmap + unpin > >The push-to-system function forces a buffer out of video RAM. This decision >should rather be made by the memory manager. By replacing the function with >calls to the kunmap and unpin functions, the buffer's memory becomes > available, >but the buffer remains in VRAM until it's evicted by a pin operation. > >This patch replaces the remaining instances of > drm_gem_vram_push_to_system() >in ast and mgag200, and removes the function from DRM. > > > My 1st impression is we need a method that restores the previous behavior > that pushes the content to the device . It appears many of the code changes that were involved in 81da87f63a1 ( above ) have been superseded by : commit 5d17718997367c435dbe5341a8e270d9b19478d3 Author: Thomas Zimmermann Date: Thu Jun 27 10:09:09 2019 +0200 drm/mgag200: Replace struct mga_framebuffer with GEM framebuffer helpers Are there any other methods (tools ) you could recommend outside of starting Gnome that I could use to get a display activity shown on the console ? ===. snipped === ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 04/23] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On Tue, Nov 12, 2019 at 8:27 PM John Hubbard wrote:
>
> An upcoming patch changes and complicates the refcounting and
> especially the "put page" aspects of it. In order to keep
> everything clean, refactor the devmap page release routines:
>
> * Rename put_devmap_managed_page() to page_is_devmap_managed(),
> and limit the functionality to "read only": return a bool,
> with no side effects.
>
> * Add a new routine, put_devmap_managed_page(), to handle checking
> what kind of page it is, and what kind of refcount handling it
> requires.
>
> * Rename __put_devmap_managed_page() to free_devmap_managed_page(),
> and limit the functionality to unconditionally freeing a devmap
> page.
>
> This is originally based on a separate patch by Ira Weiny, which
> applied to an early version of the put_user_page() experiments.
> Since then, Jérôme Glisse suggested the refactoring described above.
>
> Suggested-by: Jérôme Glisse
> Signed-off-by: Ira Weiny
> Signed-off-by: John Hubbard
> ---
> include/linux/mm.h | 27 ---
> mm/memremap.c | 67 --
> 2 files changed, 53 insertions(+), 41 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a2adf95b3f9c..96228376139c 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -967,9 +967,10 @@ static inline bool is_zone_device_page(const struct page
> *page)
> #endif
>
> #ifdef CONFIG_DEV_PAGEMAP_OPS
> -void __put_devmap_managed_page(struct page *page);
> +void free_devmap_managed_page(struct page *page);
> DECLARE_STATIC_KEY_FALSE(devmap_managed_key);
> -static inline bool put_devmap_managed_page(struct page *page)
> +
> +static inline bool page_is_devmap_managed(struct page *page)
> {
> if (!static_branch_unlikely(_managed_key))
> return false;
> @@ -978,7 +979,6 @@ static inline bool put_devmap_managed_page(struct page
> *page)
> switch (page->pgmap->type) {
> case MEMORY_DEVICE_PRIVATE:
> case MEMORY_DEVICE_FS_DAX:
> - __put_devmap_managed_page(page);
> return true;
> default:
> break;
> @@ -986,6 +986,27 @@ static inline bool put_devmap_managed_page(struct page
> *page)
> return false;
> }
>
> +static inline bool put_devmap_managed_page(struct page *page)
> +{
> + bool is_devmap = page_is_devmap_managed(page);
> +
> + if (is_devmap) {
> + int count = page_ref_dec_return(page);
> +
> + /*
> +* devmap page refcounts are 1-based, rather than 0-based: if
> +* refcount is 1, then the page is free and the refcount is
> +* stable because nobody holds a reference on the page.
> +*/
> + if (count == 1)
> + free_devmap_managed_page(page);
> + else if (!count)
> + __put_page(page);
> + }
> +
> + return is_devmap;
> +}
> +
> #else /* CONFIG_DEV_PAGEMAP_OPS */
> static inline bool put_devmap_managed_page(struct page *page)
> {
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 03ccbdfeb697..bc7e2a27d025 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -410,48 +410,39 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn,
> EXPORT_SYMBOL_GPL(get_dev_pagemap);
>
> #ifdef CONFIG_DEV_PAGEMAP_OPS
> -void __put_devmap_managed_page(struct page *page)
> +void free_devmap_managed_page(struct page *page)
> {
> - int count = page_ref_dec_return(page);
> + /* Clear Active bit in case of parallel mark_page_accessed */
> + __ClearPageActive(page);
> + __ClearPageWaiters(page);
> +
> + mem_cgroup_uncharge(page);
Ugh, when did all this HMM specific manipulation sneak into the
generic ZONE_DEVICE path? It used to be gated by pgmap type with its
own put_zone_device_private_page(). For example it's certainly
unnecessary and might be broken (would need to check) to call
mem_cgroup_uncharge() on a DAX page. ZONE_DEVICE users are not a
monolith and the HMM use case leaks pages into code paths that DAX
explicitly avoids.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from 'gfx_v6_0.c' and 'gfx_v7_0.c'
On Wed, Nov 13, 2019 at 11:56 AM Joe Perches wrote: > > On Wed, 2019-11-13 at 20:44 +0800, yu kuai wrote: > > Fixes gcc '-Wunused-but-set-variable' warning: > > > > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c: In function > > ‘gfx_v6_0_constants_init’: > > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:1579:6: warning: variable > > ‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable] > [] > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > > b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > [] > > @@ -1678,7 +1678,6 @@ static void gfx_v6_0_constants_init(struct > > amdgpu_device *adev) > > > > WREG32(mmBIF_FB_EN, BIF_FB_EN__FB_READ_EN_MASK | > > BIF_FB_EN__FB_WRITE_EN_MASK); > > > > - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP); > > I do not know the hardware but frequently hardware like > this has read ordering requirements and various registers > can not be read in a random order. > > Does removing this read have no effect on the hardware? There is no dependency. It's safe. Same thing below. Alex > > > adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG); > > mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg; > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > [] > > @@ -4336,7 +4336,6 @@ static void gfx_v7_0_gpu_early_init(struct > > amdgpu_device *adev) > > break; > > } > > > > - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP); > > adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG); > > mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg; > > Same question. > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 08/23] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On Wed, Nov 13, 2019 at 09:02:02AM -0400, Jason Gunthorpe wrote:
> On Tue, Nov 12, 2019 at 08:26:55PM -0800, John Hubbard wrote:
> > As it says in the updated comment in gup.c: current FOLL_LONGTERM
> > behavior is incompatible with FAULT_FLAG_ALLOW_RETRY because of the
> > FS DAX check requirement on vmas.
> >
> > However, the corresponding restriction in get_user_pages_remote() was
> > slightly stricter than is actually required: it forbade all
> > FOLL_LONGTERM callers, but we can actually allow FOLL_LONGTERM callers
> > that do not set the "locked" arg.
> >
> > Update the code and comments accordingly, and update the VFIO caller
> > to take advantage of this, fixing a bug as a result: the VFIO caller
> > is logically a FOLL_LONGTERM user.
> >
> > Also, remove an unnessary pair of calls that were releasing and
> > reacquiring the mmap_sem. There is no need to avoid holding mmap_sem
> > just in order to call page_to_pfn().
> >
> > Also, move the DAX check ("if a VMA is DAX, don't allow long term
> > pinning") from the VFIO call site, all the way into the internals
> > of get_user_pages_remote() and __gup_longterm_locked(). That is:
> > get_user_pages_remote() calls __gup_longterm_locked(), which in turn
> > calls check_dax_vmas(). It's lightly explained in the comments as well.
> >
> > Thanks to Jason Gunthorpe for pointing out a clean way to fix this,
> > and to Dan Williams for helping clarify the DAX refactoring.
> >
> > Suggested-by: Jason Gunthorpe
> > Cc: Dan Williams
> > Cc: Jerome Glisse
> > Cc: Ira Weiny
> > Signed-off-by: John Hubbard
> > drivers/vfio/vfio_iommu_type1.c | 25 ++---
> > mm/gup.c| 27 ++-
> > 2 files changed, 24 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/vfio/vfio_iommu_type1.c
> > b/drivers/vfio/vfio_iommu_type1.c
> > index d864277ea16f..7301b710c9a4 100644
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -340,7 +340,6 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> > long vaddr,
> > {
> > struct page *page[1];
> > struct vm_area_struct *vma;
> > - struct vm_area_struct *vmas[1];
> > unsigned int flags = 0;
> > int ret;
> >
> > @@ -348,33 +347,13 @@ static int vaddr_get_pfn(struct mm_struct *mm,
> > unsigned long vaddr,
> > flags |= FOLL_WRITE;
> >
> > down_read(>mmap_sem);
> > - if (mm == current->mm) {
> > - ret = get_user_pages(vaddr, 1, flags | FOLL_LONGTERM, page,
> > -vmas);
> > - } else {
> > - ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags, page,
> > - vmas, NULL);
> > - /*
> > -* The lifetime of a vaddr_get_pfn() page pin is
> > -* userspace-controlled. In the fs-dax case this could
> > -* lead to indefinite stalls in filesystem operations.
> > -* Disallow attempts to pin fs-dax pages via this
> > -* interface.
> > -*/
> > - if (ret > 0 && vma_is_fsdax(vmas[0])) {
> > - ret = -EOPNOTSUPP;
> > - put_page(page[0]);
> > - }
> > - }
> > - up_read(>mmap_sem);
> > -
> > + ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags | FOLL_LONGTERM,
> > + page, NULL, NULL);
> > if (ret == 1) {
> > *pfn = page_to_pfn(page[0]);
> > return 0;
>
> Mind the return with the lock held this needs some goto unwind
Ah yea... retract my reviewed by... :-(
Ira
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 04/23] mm: devmap: refactor 1-based refcounting for ZONE_DEVICE pages
On Tue, Nov 12, 2019 at 08:26:51PM -0800, John Hubbard wrote:
> An upcoming patch changes and complicates the refcounting and
> especially the "put page" aspects of it. In order to keep
> everything clean, refactor the devmap page release routines:
>
> * Rename put_devmap_managed_page() to page_is_devmap_managed(),
> and limit the functionality to "read only": return a bool,
> with no side effects.
>
> * Add a new routine, put_devmap_managed_page(), to handle checking
> what kind of page it is, and what kind of refcount handling it
> requires.
>
> * Rename __put_devmap_managed_page() to free_devmap_managed_page(),
> and limit the functionality to unconditionally freeing a devmap
> page.
>
> This is originally based on a separate patch by Ira Weiny, which
> applied to an early version of the put_user_page() experiments.
> Since then, Jérôme Glisse suggested the refactoring described above.
>
> Suggested-by: Jérôme Glisse
> Signed-off-by: Ira Weiny
> Signed-off-by: John Hubbard
Reviewed-by: Jérôme Glisse
> ---
> include/linux/mm.h | 27 ---
> mm/memremap.c | 67 --
> 2 files changed, 53 insertions(+), 41 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a2adf95b3f9c..96228376139c 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -967,9 +967,10 @@ static inline bool is_zone_device_page(const struct page
> *page)
> #endif
>
> #ifdef CONFIG_DEV_PAGEMAP_OPS
> -void __put_devmap_managed_page(struct page *page);
> +void free_devmap_managed_page(struct page *page);
> DECLARE_STATIC_KEY_FALSE(devmap_managed_key);
> -static inline bool put_devmap_managed_page(struct page *page)
> +
> +static inline bool page_is_devmap_managed(struct page *page)
> {
> if (!static_branch_unlikely(_managed_key))
> return false;
> @@ -978,7 +979,6 @@ static inline bool put_devmap_managed_page(struct page
> *page)
> switch (page->pgmap->type) {
> case MEMORY_DEVICE_PRIVATE:
> case MEMORY_DEVICE_FS_DAX:
> - __put_devmap_managed_page(page);
> return true;
> default:
> break;
> @@ -986,6 +986,27 @@ static inline bool put_devmap_managed_page(struct page
> *page)
> return false;
> }
>
> +static inline bool put_devmap_managed_page(struct page *page)
> +{
> + bool is_devmap = page_is_devmap_managed(page);
> +
> + if (is_devmap) {
> + int count = page_ref_dec_return(page);
> +
> + /*
> + * devmap page refcounts are 1-based, rather than 0-based: if
> + * refcount is 1, then the page is free and the refcount is
> + * stable because nobody holds a reference on the page.
> + */
> + if (count == 1)
> + free_devmap_managed_page(page);
> + else if (!count)
> + __put_page(page);
> + }
> +
> + return is_devmap;
> +}
> +
> #else /* CONFIG_DEV_PAGEMAP_OPS */
> static inline bool put_devmap_managed_page(struct page *page)
> {
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 03ccbdfeb697..bc7e2a27d025 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -410,48 +410,39 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn,
> EXPORT_SYMBOL_GPL(get_dev_pagemap);
>
> #ifdef CONFIG_DEV_PAGEMAP_OPS
> -void __put_devmap_managed_page(struct page *page)
> +void free_devmap_managed_page(struct page *page)
> {
> - int count = page_ref_dec_return(page);
> + /* Clear Active bit in case of parallel mark_page_accessed */
> + __ClearPageActive(page);
> + __ClearPageWaiters(page);
> +
> + mem_cgroup_uncharge(page);
>
> /*
> - * If refcount is 1 then page is freed and refcount is stable as nobody
> - * holds a reference on the page.
> + * When a device_private page is freed, the page->mapping field
> + * may still contain a (stale) mapping value. For example, the
> + * lower bits of page->mapping may still identify the page as
> + * an anonymous page. Ultimately, this entire field is just
> + * stale and wrong, and it will cause errors if not cleared.
> + * One example is:
> + *
> + * migrate_vma_pages()
> + *migrate_vma_insert_page()
> + * page_add_new_anon_rmap()
> + *__page_set_anon_rmap()
> + * ...checks page->mapping, via PageAnon(page) call,
> + *and incorrectly concludes that the page is an
> + *anonymous page. Therefore, it incorrectly,
> + *silently fails to set up the new anon rmap.
> + *
> + * For other types of ZONE_DEVICE pages, migration is either
> + * handled differently or not done at all, so there is no need
> + * to clear page->mapping.
>*/
> - if (count == 1) {
> - /* Clear Active bit in case of parallel mark_page_accessed */
> -
Re: [PATCH v4 23/23] mm/gup: remove support for gup(FOLL_LONGTERM)
On Tue, Nov 12, 2019 at 08:27:10PM -0800, John Hubbard wrote:
> Now that all other kernel callers of get_user_pages(FOLL_LONGTERM)
> have been converted to pin_longterm_pages(), lock it down:
>
> 1) Add an assertion to get_user_pages(), preventing callers from
>passing FOLL_LONGTERM (in addition to the existing assertion that
>prevents FOLL_PIN).
>
> 2) Remove the associated GUP_LONGTERM_BENCHMARK test.
>
> Signed-off-by: John Hubbard
> ---
> mm/gup.c | 8
> mm/gup_benchmark.c | 9 +
> tools/testing/selftests/vm/gup_benchmark.c | 7 ++-
> 3 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 82e7e4ce5027..90f5f95ee7ac 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1756,11 +1756,11 @@ long get_user_pages(unsigned long start, unsigned
> long nr_pages,
> struct vm_area_struct **vmas)
> {
> /*
> - * FOLL_PIN must only be set internally by the pin_user_page*() and
> - * pin_longterm_*() APIs, never directly by the caller, so enforce that
> - * with an assertion:
> + * FOLL_PIN and FOLL_LONGTERM must only be set internally by the
> + * pin_user_page*() and pin_longterm_*() APIs, never directly by the
> + * caller, so enforce that with an assertion:
>*/
> - if (WARN_ON_ONCE(gup_flags & FOLL_PIN))
> + if (WARN_ON_ONCE(gup_flags & (FOLL_PIN | FOLL_LONGTERM)))
Don't we want to block FOLL_LONGTERM in get_user_pages_fast() as well after all
this?
Ira
> return -EINVAL;
>
> return __gup_longterm_locked(current, current->mm, start, nr_pages,
> diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
> index 8f980d91dbf5..679f0e6a0adb 100644
> --- a/mm/gup_benchmark.c
> +++ b/mm/gup_benchmark.c
> @@ -6,7 +6,7 @@
> #include
>
> #define GUP_FAST_BENCHMARK _IOWR('g', 1, struct gup_benchmark)
> -#define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
> +/* Command 2 has been deleted. */
> #define GUP_BENCHMARK_IOWR('g', 3, struct gup_benchmark)
> #define PIN_FAST_BENCHMARK _IOWR('g', 4, struct gup_benchmark)
> #define PIN_LONGTERM_BENCHMARK _IOWR('g', 5, struct gup_benchmark)
> @@ -28,7 +28,6 @@ static void put_back_pages(int cmd, struct page **pages,
> unsigned long nr_pages)
>
> switch (cmd) {
> case GUP_FAST_BENCHMARK:
> - case GUP_LONGTERM_BENCHMARK:
> case GUP_BENCHMARK:
> for (i = 0; i < nr_pages; i++)
> put_page(pages[i]);
> @@ -97,11 +96,6 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
> nr = get_user_pages_fast(addr, nr, gup->flags,
>pages + i);
> break;
> - case GUP_LONGTERM_BENCHMARK:
> - nr = get_user_pages(addr, nr,
> - gup->flags | FOLL_LONGTERM,
> - pages + i, NULL);
> - break;
> case GUP_BENCHMARK:
> nr = get_user_pages(addr, nr, gup->flags, pages + i,
> NULL);
> @@ -159,7 +153,6 @@ static long gup_benchmark_ioctl(struct file *filep,
> unsigned int cmd,
>
> switch (cmd) {
> case GUP_FAST_BENCHMARK:
> - case GUP_LONGTERM_BENCHMARK:
> case GUP_BENCHMARK:
> case PIN_FAST_BENCHMARK:
> case PIN_LONGTERM_BENCHMARK:
> diff --git a/tools/testing/selftests/vm/gup_benchmark.c
> b/tools/testing/selftests/vm/gup_benchmark.c
> index 03928e47a86f..836b7082db13 100644
> --- a/tools/testing/selftests/vm/gup_benchmark.c
> +++ b/tools/testing/selftests/vm/gup_benchmark.c
> @@ -15,7 +15,7 @@
> #define PAGE_SIZE sysconf(_SC_PAGESIZE)
>
> #define GUP_FAST_BENCHMARK _IOWR('g', 1, struct gup_benchmark)
> -#define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
> +/* Command 2 has been deleted. */
> #define GUP_BENCHMARK_IOWR('g', 3, struct gup_benchmark)
>
> /*
> @@ -49,7 +49,7 @@ int main(int argc, char **argv)
> char *file = "/dev/zero";
> char *p;
>
> - while ((opt = getopt(argc, argv, "m:r:n:f:abctTLUuwSH")) != -1) {
> + while ((opt = getopt(argc, argv, "m:r:n:f:abctTUuwSH")) != -1) {
> switch (opt) {
> case 'a':
> cmd = PIN_FAST_BENCHMARK;
> @@ -75,9 +75,6 @@ int main(int argc, char **argv)
> case 'T':
> thp = 0;
> break;
> - case 'L':
> - cmd = GUP_LONGTERM_BENCHMARK;
> - break;
> case 'U':
> cmd = GUP_BENCHMARK;
> break;
> --
> 2.24.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
Re: [PATCH v4 22/23] selftests/vm: run_vmtests: invoke gup_benchmark with basic FOLL_PIN coverage
On Tue, Nov 12, 2019 at 08:27:09PM -0800, John Hubbard wrote: > It's good to have basic unit test coverage of the new FOLL_PIN > behavior. Fortunately, the gup_benchmark unit test is extremely > fast (a few milliseconds), so adding it the the run_vmtests suite > is going to cause no noticeable change in running time. > > So, add two new invocations to run_vmtests: > > 1) Run gup_benchmark with normal get_user_pages(). > > 2) Run gup_benchmark with pin_user_pages(). This is much like > the first call, except that it sets FOLL_PIN. > > Running these two in quick succession also provide a visual > comparison of the running times, which is convenient. > > The new invocations are fairly early in the run_vmtests script, > because with test suites, it's usually preferable to put the > shorter, faster tests first, all other things being equal. > > Signed-off-by: John Hubbard Reviewed-by: Ira Weiny > --- > tools/testing/selftests/vm/run_vmtests | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/tools/testing/selftests/vm/run_vmtests > b/tools/testing/selftests/vm/run_vmtests > index 951c507a27f7..93e8dc9a7cad 100755 > --- a/tools/testing/selftests/vm/run_vmtests > +++ b/tools/testing/selftests/vm/run_vmtests > @@ -104,6 +104,28 @@ echo "NOTE: The above hugetlb tests provide minimal > coverage. Use" > echo " https://github.com/libhugetlbfs/libhugetlbfs.git for" > echo " hugetlb regression testing." > > +echo "" > +echo "running 'gup_benchmark -U' (normal/slow gup)" > +echo "" > +./gup_benchmark -U > +if [ $? -ne 0 ]; then > + echo "[FAIL]" > + exitcode=1 > +else > + echo "[PASS]" > +fi > + > +echo "--" > +echo "running gup_benchmark -c (pin_user_pages)" > +echo "--" > +./gup_benchmark -c > +if [ $? -ne 0 ]; then > + echo "[FAIL]" > + exitcode=1 > +else > + echo "[PASS]" > +fi > + > echo "---" > echo "running userfaultfd" > echo "---" > -- > 2.24.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 21/23] mm/gup_benchmark: support pin_user_pages() and related calls
On Tue, Nov 12, 2019 at 08:27:08PM -0800, John Hubbard wrote:
> Up until now, gup_benchmark supported testing of the
> following kernel functions:
>
> * get_user_pages(): via the '-U' command line option
> * get_user_pages_longterm(): via the '-L' command line option
> * get_user_pages_fast(): as the default (no options required)
>
> Add test coverage for the new corresponding pin_*() functions:
>
> * pin_user_pages(): via the '-c' command line option
> * pin_longterm_pages(): via the '-b' command line option
> * pin_user_pages_fast(): via the '-a' command line option
>
> Also, add an option for clarity: '-u' for what is now (still) the
> default choice: get_user_pages_fast().
>
> Also, for the three commands that set FOLL_PIN, verify that the pages
> really are dma-pinned, via the new is_dma_pinned() routine.
> Those commands are:
>
> PIN_FAST_BENCHMARK : calls pin_user_pages_fast()
> PIN_LONGTERM_BENCHMARK : calls pin_longterm_pages()
> PIN_BENCHMARK : calls pin_user_pages()
>
> In between the calls to pin_*() and put_user_pages(),
> check each page: if page_dma_pinned() returns false, then
> WARN and return.
>
> Do this outside of the benchmark timestamps, so that it doesn't
> affect reported times.
>
> Signed-off-by: John Hubbard
Reviewed-by: Ira Weiny
> ---
> mm/gup_benchmark.c | 73 --
> tools/testing/selftests/vm/gup_benchmark.c | 23 ++-
> 2 files changed, 90 insertions(+), 6 deletions(-)
>
> diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
> index 7fc44d25eca7..8f980d91dbf5 100644
> --- a/mm/gup_benchmark.c
> +++ b/mm/gup_benchmark.c
> @@ -8,6 +8,9 @@
> #define GUP_FAST_BENCHMARK _IOWR('g', 1, struct gup_benchmark)
> #define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
> #define GUP_BENCHMARK_IOWR('g', 3, struct gup_benchmark)
> +#define PIN_FAST_BENCHMARK _IOWR('g', 4, struct gup_benchmark)
> +#define PIN_LONGTERM_BENCHMARK _IOWR('g', 5, struct gup_benchmark)
> +#define PIN_BENCHMARK_IOWR('g', 6, struct gup_benchmark)
>
> struct gup_benchmark {
> __u64 get_delta_usec;
> @@ -19,6 +22,44 @@ struct gup_benchmark {
> __u64 expansion[10];/* For future use */
> };
>
> +static void put_back_pages(int cmd, struct page **pages, unsigned long
> nr_pages)
> +{
> + int i;
> +
> + switch (cmd) {
> + case GUP_FAST_BENCHMARK:
> + case GUP_LONGTERM_BENCHMARK:
> + case GUP_BENCHMARK:
> + for (i = 0; i < nr_pages; i++)
> + put_page(pages[i]);
> + break;
> +
> + case PIN_FAST_BENCHMARK:
> + case PIN_LONGTERM_BENCHMARK:
> + case PIN_BENCHMARK:
> + put_user_pages(pages, nr_pages);
> + break;
> + }
> +}
> +
> +static void verify_dma_pinned(int cmd, struct page **pages,
> + unsigned long nr_pages)
> +{
> + int i;
> +
> + switch (cmd) {
> + case PIN_FAST_BENCHMARK:
> + case PIN_LONGTERM_BENCHMARK:
> + case PIN_BENCHMARK:
> + for (i = 0; i < nr_pages; i++) {
> + if (WARN(!page_dma_pinned(pages[i]),
> + "pages[%d] is NOT dma-pinned\n", i))
> + break;
> + }
> + break;
> + }
> +}
> +
> static int __gup_benchmark_ioctl(unsigned int cmd,
> struct gup_benchmark *gup)
> {
> @@ -65,6 +106,18 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
> nr = get_user_pages(addr, nr, gup->flags, pages + i,
> NULL);
> break;
> + case PIN_FAST_BENCHMARK:
> + nr = pin_user_pages_fast(addr, nr, gup->flags,
> + pages + i);
> + break;
> + case PIN_LONGTERM_BENCHMARK:
> + nr = pin_longterm_pages(addr, nr, gup->flags, pages + i,
> + NULL);
> + break;
> + case PIN_BENCHMARK:
> + nr = pin_user_pages(addr, nr, gup->flags, pages + i,
> + NULL);
> + break;
> default:
> return -1;
> }
> @@ -75,15 +128,22 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
> }
> end_time = ktime_get();
>
> + /* Shifting the meaning of nr_pages: now it is actual number pinned: */
> + nr_pages = i;
> +
> gup->get_delta_usec = ktime_us_delta(end_time, start_time);
> gup->size = addr - gup->addr;
>
> + /*
> + * Take an un-benchmark-timed moment to verify DMA pinned
> + * state: print a warning if any non-dma-pinned pages are found:
> + */
> + verify_dma_pinned(cmd, pages, nr_pages);
> +
> start_time = ktime_get();
> - for (i = 0; i <
Re: [PATCH v4 20/23] mm/gup_benchmark: use proper FOLL_WRITE flags instead of hard-coding "1"
On Tue, Nov 12, 2019 at 08:27:07PM -0800, John Hubbard wrote:
> Fix the gup benchmark flags to use the symbolic FOLL_WRITE,
> instead of a hard-coded "1" value.
>
> Also, clean up the filtering of gup flags a little, by just doing
> it once before issuing any of the get_user_pages*() calls. This
> makes it harder to overlook, instead of having little "gup_flags & 1"
> phrases in the function calls.
>
> Signed-off-by: John Hubbard
Reviewed-by: Ira Weiny
> ---
> mm/gup_benchmark.c | 9 ++---
> tools/testing/selftests/vm/gup_benchmark.c | 6 +-
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
> index 7dd602d7f8db..7fc44d25eca7 100644
> --- a/mm/gup_benchmark.c
> +++ b/mm/gup_benchmark.c
> @@ -48,18 +48,21 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
> nr = (next - addr) / PAGE_SIZE;
> }
>
> + /* Filter out most gup flags: only allow a tiny subset here: */
> + gup->flags &= FOLL_WRITE;
> +
> switch (cmd) {
> case GUP_FAST_BENCHMARK:
> - nr = get_user_pages_fast(addr, nr, gup->flags & 1,
> + nr = get_user_pages_fast(addr, nr, gup->flags,
>pages + i);
> break;
> case GUP_LONGTERM_BENCHMARK:
> nr = get_user_pages(addr, nr,
> - (gup->flags & 1) | FOLL_LONGTERM,
> + gup->flags | FOLL_LONGTERM,
> pages + i, NULL);
> break;
> case GUP_BENCHMARK:
> - nr = get_user_pages(addr, nr, gup->flags & 1, pages + i,
> + nr = get_user_pages(addr, nr, gup->flags, pages + i,
> NULL);
> break;
> default:
> diff --git a/tools/testing/selftests/vm/gup_benchmark.c
> b/tools/testing/selftests/vm/gup_benchmark.c
> index 485cf06ef013..389327e9b30a 100644
> --- a/tools/testing/selftests/vm/gup_benchmark.c
> +++ b/tools/testing/selftests/vm/gup_benchmark.c
> @@ -18,6 +18,9 @@
> #define GUP_LONGTERM_BENCHMARK _IOWR('g', 2, struct gup_benchmark)
> #define GUP_BENCHMARK_IOWR('g', 3, struct gup_benchmark)
>
> +/* Just the flags we need, copied from mm.h: */
> +#define FOLL_WRITE 0x01/* check pte is writable */
> +
> struct gup_benchmark {
> __u64 get_delta_usec;
> __u64 put_delta_usec;
> @@ -85,7 +88,8 @@ int main(int argc, char **argv)
> }
>
> gup.nr_pages_per_call = nr_pages;
> - gup.flags = write;
> + if (write)
> + gup.flags |= FOLL_WRITE;
>
> fd = open("/sys/kernel/debug/gup_benchmark", O_RDWR);
> if (fd == -1)
> --
> 2.24.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 09/23] mm/gup: introduce pin_user_pages*() and FOLL_PIN
On Tue, Nov 12, 2019 at 08:26:56PM -0800, John Hubbard wrote: > Introduce pin_user_pages*() variations of get_user_pages*() calls, > and also pin_longterm_pages*() variations. > > These variants all set FOLL_PIN, which is also introduced, and > thoroughly documented. > > The pin_longterm*() variants also set FOLL_LONGTERM, in addition > to FOLL_PIN: > > pin_user_pages() > pin_user_pages_remote() > pin_user_pages_fast() > > pin_longterm_pages() > pin_longterm_pages_remote() > pin_longterm_pages_fast() At some point in this conversation I thought we were going to put in "unpin_*" versions of these. Is that still in the plans? Ira ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 08/23] vfio, mm: fix get_user_pages_remote() and FOLL_LONGTERM
On Tue, Nov 12, 2019 at 08:26:55PM -0800, John Hubbard wrote:
> As it says in the updated comment in gup.c: current FOLL_LONGTERM
> behavior is incompatible with FAULT_FLAG_ALLOW_RETRY because of the
> FS DAX check requirement on vmas.
>
> However, the corresponding restriction in get_user_pages_remote() was
> slightly stricter than is actually required: it forbade all
> FOLL_LONGTERM callers, but we can actually allow FOLL_LONGTERM callers
> that do not set the "locked" arg.
>
> Update the code and comments accordingly, and update the VFIO caller
> to take advantage of this, fixing a bug as a result: the VFIO caller
> is logically a FOLL_LONGTERM user.
>
> Also, remove an unnessary pair of calls that were releasing and
> reacquiring the mmap_sem. There is no need to avoid holding mmap_sem
> just in order to call page_to_pfn().
>
> Also, move the DAX check ("if a VMA is DAX, don't allow long term
> pinning") from the VFIO call site, all the way into the internals
> of get_user_pages_remote() and __gup_longterm_locked(). That is:
> get_user_pages_remote() calls __gup_longterm_locked(), which in turn
> calls check_dax_vmas(). It's lightly explained in the comments as well.
>
> Thanks to Jason Gunthorpe for pointing out a clean way to fix this,
> and to Dan Williams for helping clarify the DAX refactoring.
>
> Suggested-by: Jason Gunthorpe
> Cc: Dan Williams
> Cc: Jerome Glisse
> Cc: Ira Weiny
Reviewed-by: Ira Weiny
> Signed-off-by: John Hubbard
> ---
> drivers/vfio/vfio_iommu_type1.c | 25 ++---
> mm/gup.c| 27 ++-
> 2 files changed, 24 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index d864277ea16f..7301b710c9a4 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -340,7 +340,6 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
> {
> struct page *page[1];
> struct vm_area_struct *vma;
> - struct vm_area_struct *vmas[1];
> unsigned int flags = 0;
> int ret;
>
> @@ -348,33 +347,13 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
> flags |= FOLL_WRITE;
>
> down_read(>mmap_sem);
> - if (mm == current->mm) {
> - ret = get_user_pages(vaddr, 1, flags | FOLL_LONGTERM, page,
> - vmas);
> - } else {
> - ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags, page,
> - vmas, NULL);
> - /*
> - * The lifetime of a vaddr_get_pfn() page pin is
> - * userspace-controlled. In the fs-dax case this could
> - * lead to indefinite stalls in filesystem operations.
> - * Disallow attempts to pin fs-dax pages via this
> - * interface.
> - */
> - if (ret > 0 && vma_is_fsdax(vmas[0])) {
> - ret = -EOPNOTSUPP;
> - put_page(page[0]);
> - }
> - }
> - up_read(>mmap_sem);
> -
> + ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags | FOLL_LONGTERM,
> + page, NULL, NULL);
> if (ret == 1) {
> *pfn = page_to_pfn(page[0]);
> return 0;
> }
>
> - down_read(>mmap_sem);
> -
> vaddr = untagged_addr(vaddr);
>
> vma = find_vma_intersection(mm, vaddr, vaddr + 1);
> diff --git a/mm/gup.c b/mm/gup.c
> index 933524de6249..83702b2e86c8 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -29,6 +29,13 @@ struct follow_page_context {
> unsigned int page_mask;
> };
>
> +static __always_inline long __gup_longterm_locked(struct task_struct *tsk,
> + struct mm_struct *mm,
> + unsigned long start,
> + unsigned long nr_pages,
> + struct page **pages,
> + struct vm_area_struct **vmas,
> + unsigned int flags);
> /*
> * Return the compound head page with ref appropriately incremented,
> * or NULL if that failed.
> @@ -1167,13 +1174,23 @@ long get_user_pages_remote(struct task_struct *tsk,
> struct mm_struct *mm,
> struct vm_area_struct **vmas, int *locked)
> {
> /*
> - * FIXME: Current FOLL_LONGTERM behavior is incompatible with
> + * Parts of FOLL_LONGTERM behavior are incompatible with
>* FAULT_FLAG_ALLOW_RETRY because of the FS DAX check requirement on
> - * vmas. As there are no users of this flag in this call we simply
> - * disallow this option for now.
> + * vmas. However, this only comes up if locked is set, and there are
> + * callers that do request FOLL_LONGTERM, but do
Re: [PATCH v4 09/23] mm/gup: introduce pin_user_pages*() and FOLL_PIN
On Wed, Nov 13, 2019 at 2:43 AM Jan Kara wrote:
>
> On Tue 12-11-19 20:26:56, John Hubbard wrote:
> > Introduce pin_user_pages*() variations of get_user_pages*() calls,
> > and also pin_longterm_pages*() variations.
> >
> > These variants all set FOLL_PIN, which is also introduced, and
> > thoroughly documented.
> >
> > The pin_longterm*() variants also set FOLL_LONGTERM, in addition
> > to FOLL_PIN:
> >
> > pin_user_pages()
> > pin_user_pages_remote()
> > pin_user_pages_fast()
> >
> > pin_longterm_pages()
> > pin_longterm_pages_remote()
> > pin_longterm_pages_fast()
> >
> > All pages that are pinned via the above calls, must be unpinned via
> > put_user_page().
> >
> > The underlying rules are:
> >
> > * These are gup-internal flags, so the call sites should not directly
> > set FOLL_PIN nor FOLL_LONGTERM. That behavior is enforced with
> > assertions, for the new FOLL_PIN flag. However, for the pre-existing
> > FOLL_LONGTERM flag, which has some call sites that still directly
> > set FOLL_LONGTERM, there is no assertion yet.
> >
> > * Call sites that want to indicate that they are going to do DirectIO
> > ("DIO") or something with similar characteristics, should call a
> > get_user_pages()-like wrapper call that sets FOLL_PIN. These wrappers
> > will:
> > * Start with "pin_user_pages" instead of "get_user_pages". That
> > makes it easy to find and audit the call sites.
> > * Set FOLL_PIN
> >
> > * For pages that are received via FOLL_PIN, those pages must be returned
> > via put_user_page().
> >
> > Thanks to Jan Kara and Vlastimil Babka for explaining the 4 cases
> > in this documentation. (I've reworded it and expanded upon it.)
> >
> > Reviewed-by: Mike Rapoport # Documentation
> > Reviewed-by: Jérôme Glisse
> > Cc: Jonathan Corbet
> > Cc: Ira Weiny
> > Signed-off-by: John Hubbard
>
> Thanks for the documentation. It looks great!
>
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 83702b2e86c8..4409e84dff51 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -201,6 +201,10 @@ static struct page *follow_page_pte(struct
> > vm_area_struct *vma,
> > spinlock_t *ptl;
> > pte_t *ptep, pte;
> >
> > + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> > + if (WARN_ON_ONCE((flags & (FOLL_PIN | FOLL_GET)) ==
> > + (FOLL_PIN | FOLL_GET)))
> > + return ERR_PTR(-EINVAL);
> > retry:
> > if (unlikely(pmd_bad(*pmd)))
> > return no_page_table(vma, flags);
>
> How does FOLL_PIN result in grabbing (at least normal, for now) page
> reference?
> I didn't find that anywhere in this patch but it is a prerequisite to
> converting any user to pin_user_pages() interface, right?
>
> > +/**
> > + * pin_user_pages_fast() - pin user pages in memory without taking locks
> > + *
> > + * Nearly the same as get_user_pages_fast(), except that FOLL_PIN is set.
> > See
> > + * get_user_pages_fast() for documentation on the function arguments,
> > because
> > + * the arguments here are identical.
> > + *
> > + * FOLL_PIN means that the pages must be released via put_user_page().
> > Please
> > + * see Documentation/vm/pin_user_pages.rst for further details.
> > + *
> > + * This is intended for Case 1 (DIO) in
> > Documentation/vm/pin_user_pages.rst. It
> > + * is NOT intended for Case 2 (RDMA: long-term pins).
> > + */
> > +int pin_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages)
> > +{
> > + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> > + if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> > + return -EINVAL;
> > +
> > + gup_flags |= FOLL_PIN;
> > + return internal_get_user_pages_fast(start, nr_pages, gup_flags,
> > pages);
> > +}
> > +EXPORT_SYMBOL_GPL(pin_user_pages_fast);
>
> I was somewhat wondering about the number of functions you add here. So we
> have:
>
> pin_user_pages()
> pin_user_pages_fast()
> pin_user_pages_remote()
>
> and then longterm variants:
>
> pin_longterm_pages()
> pin_longterm_pages_fast()
> pin_longterm_pages_remote()
>
> and obviously we have gup like:
> get_user_pages()
> get_user_pages_fast()
> get_user_pages_remote()
> ... and some other gup variants ...
>
> I think we really should have pin_* vs get_* variants as they are very
> different in terms of guarantees and after conversion, any use of get_*
> variant in non-mm code should be closely scrutinized. OTOH pin_longterm_*
> don't look *that* useful to me and just using pin_* instead with
> FOLL_LONGTERM flag would look OK to me and somewhat reduce the number of
> functions which is already large enough? What do people think? I don't feel
> too strongly about this but wanted to bring this up.
I'd vote for FOLL_LONGTERM should obviate the need for
{get,pin}_user_pages_longterm(). It's a property that is passed by the
call site, not an internal flag.
___
dri-devel
Re: [PATCH] video: fbdev: atyfb: only use ioremap_uc() on i386 and ia64
On Wed, Nov 13, 2019 at 11:31:54AM +0200, Andy Shevchenko wrote: > On Wed, Nov 13, 2019 at 08:38:15AM +0100, Arnd Bergmann wrote: > > On Wed, Nov 13, 2019 at 8:27 AM Christoph Hellwig wrote: > > > > > > On Tue, Nov 12, 2019 at 10:24:23PM +, Luis Chamberlain wrote: > > > > I think this would be possible if we could flop ioremap_nocache() to UC > > > > instead of UC- on x86. Otherwise, I can't see how we can remove this by > > > > still not allowing direct MTRR calls. > > > > > > If everything goes well ioremap_nocache will be gone as of 5.5. > > > > As ioremap_nocache() just an alias for ioremap(), I suppose the idea would > > then be to make x86 ioremap be UC instead of UC-, again matching what the > > other architectures do already. > > I think it's right thing to do, i.e. assume that ioremap() always does strong > UC independently on MTRR settings. Agreed wholeheartedly. What are the blockers from making that happen? Do we have any left? Luis ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 03/23] mm/gup: move try_get_compound_head() to top, fix minor issues
On Tue, Nov 12, 2019 at 08:26:50PM -0800, John Hubbard wrote:
> An upcoming patch uses try_get_compound_head() more widely,
> so move it to the top of gup.c.
>
> Also fix a tiny spelling error and a checkpatch.pl warning.
>
> Signed-off-by: John Hubbard
Simple enough...
Reviewed-by: Ira Weiny
> ---
> mm/gup.c | 29 +++--
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 199da99e8ffc..933524de6249 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -29,6 +29,21 @@ struct follow_page_context {
> unsigned int page_mask;
> };
>
> +/*
> + * Return the compound head page with ref appropriately incremented,
> + * or NULL if that failed.
> + */
> +static inline struct page *try_get_compound_head(struct page *page, int refs)
> +{
> + struct page *head = compound_head(page);
> +
> + if (WARN_ON_ONCE(page_ref_count(head) < 0))
> + return NULL;
> + if (unlikely(!page_cache_add_speculative(head, refs)))
> + return NULL;
> + return head;
> +}
> +
> /**
> * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned
> pages
> * @pages: array of pages to be maybe marked dirty, and definitely released.
> @@ -1793,20 +1808,6 @@ static void __maybe_unused undo_dev_pagemap(int *nr,
> int nr_start,
> }
> }
>
> -/*
> - * Return the compund head page with ref appropriately incremented,
> - * or NULL if that failed.
> - */
> -static inline struct page *try_get_compound_head(struct page *page, int refs)
> -{
> - struct page *head = compound_head(page);
> - if (WARN_ON_ONCE(page_ref_count(head) < 0))
> - return NULL;
> - if (unlikely(!page_cache_add_speculative(head, refs)))
> - return NULL;
> - return head;
> -}
> -
> #ifdef CONFIG_ARCH_HAS_PTE_SPECIAL
> static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
>unsigned int flags, struct page **pages, int *nr)
> --
> 2.24.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 09/23] mm/gup: introduce pin_user_pages*() and FOLL_PIN
> > +/**
> > + * pin_user_pages_fast() - pin user pages in memory without taking locks
> > + *
> > + * Nearly the same as get_user_pages_fast(), except that FOLL_PIN is set.
> > See
> > + * get_user_pages_fast() for documentation on the function arguments,
> > because
> > + * the arguments here are identical.
> > + *
> > + * FOLL_PIN means that the pages must be released via put_user_page().
> > Please
> > + * see Documentation/vm/pin_user_pages.rst for further details.
> > + *
> > + * This is intended for Case 1 (DIO) in
> > Documentation/vm/pin_user_pages.rst. It
> > + * is NOT intended for Case 2 (RDMA: long-term pins).
> > + */
> > +int pin_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages)
> > +{
> > + /* FOLL_GET and FOLL_PIN are mutually exclusive. */
> > + if (WARN_ON_ONCE(gup_flags & FOLL_GET))
> > + return -EINVAL;
> > +
> > + gup_flags |= FOLL_PIN;
> > + return internal_get_user_pages_fast(start, nr_pages, gup_flags, pages);
> > +}
> > +EXPORT_SYMBOL_GPL(pin_user_pages_fast);
>
> I was somewhat wondering about the number of functions you add here. So we
> have:
>
> pin_user_pages()
> pin_user_pages_fast()
> pin_user_pages_remote()
>
> and then longterm variants:
>
> pin_longterm_pages()
> pin_longterm_pages_fast()
> pin_longterm_pages_remote()
>
> and obviously we have gup like:
> get_user_pages()
> get_user_pages_fast()
> get_user_pages_remote()
> ... and some other gup variants ...
>
> I think we really should have pin_* vs get_* variants as they are very
> different in terms of guarantees and after conversion, any use of get_*
> variant in non-mm code should be closely scrutinized. OTOH pin_longterm_*
> don't look *that* useful to me and just using pin_* instead with
> FOLL_LONGTERM flag would look OK to me and somewhat reduce the number of
> functions which is already large enough? What do people think? I don't feel
> too strongly about this but wanted to bring this up.
I'm a bit concerned with the function explosion myself. I think what you
suggest is a happy medium. So I'd be ok with that.
Ira
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: UDL device cannot get its own screen
2019. 11. 13. 18:25 keltezéssel, Ilia Mirkin írta:
On Wed, Nov 13, 2019 at 11:59 AM Böszörményi Zoltán wrote:
2019. 11. 12. 17:41 keltezéssel, Ilia Mirkin írta:
On Tue, Nov 12, 2019 at 9:23 AM Böszörményi Zoltán wrote:
But no, all GPU devices (now only one, the UDL device) have screen 0
(a.k.a. DISPLAY=:0.0) set when AutoBindGPU is true:
[ 2444.576] xf86AutoConfigOutputDevices: xf86NumScreens 2 xf86NumGPUScreens 1
[ 2444.576] xf86AutoConfigOutputDevices: GPU #0 driver 'modesetting' 'modeset'
scrnIndex
256 origIndex 257 pScreen->myNum 256 confScreen->screennum 0
confScreen->device->identifier 'Intel0'
confScreen->device->screen 0 confScreen->device->myScreenSection->screennum 0
confScreen->device->myScreenSection->device->screen 0
Somehow, Option "Device" should ensure that the UDL device is actually
treated as a framebuffer that can be rendered into (i.e. to be modeset(2)
instead of modeset(Gn)) and it should be woken up automatically.
This is what AutoBindGPU is supposed to do, isn't it?
But instead of assigning to screen 0, it should be assigned to whatever
screen number it is configured as.
I know it's not a common use case nowadays, but I really want separate
fullscreen apps on their independent screens, including a standalone UDL
device, instead of having the latters as a Xinerama extension to some
other device.
If you see a "G", that means it's being treated as a GPU device, which
is *not* what you want if you want separate screens. You need to try
to convince things to *not* set the devices up as GPU devices, but
instead put each device (and each one of its heads, via ZaphodHeads)
no a separate device, which in turn will have a separate screen.
I created a merge request that finally made it possible what I wanted.
https://gitlab.freedesktop.org/xorg/xserver/merge_requests/334
Now, no matter if I use the intel or modesetting drivers for the
Device sections using the Intel heads, or AutoBindGPU set to true or
false, the UDL device is correctly matched with its Option "kmsdev"
setting to the plaform device's device path.
This patch seems to be a slight layering violation, but since the
modesetting driver is built into the Xorg server sources, the patch
may get away with it.
Have you looked at setting AutoAddGPU to false? AutoBindGPU is too
late -- that's when you already have a GPU, whether to bind it to the
primary device (/screen/whatever). You need to not have a GPU in the
first place.
Yes, I tried AutoAddGPU=false. Then the UDL device was not set up at all.
What I noticed in debugging Xorg via GDB is that the UDL device was
matched to the wrong platform device in xf86platformProbeDev.
xf86_platform_devices[0] == Intel, /dev/dri/card1, primary platform device
xf86_platform_devices[1] == UDL, /dev/dri/card0
devList[0] == "Intel0"
devList[1] == "Intel1"
devList[2] == "UDL"
devList[3] == "Intel2" (GPU device)
Since the device path was not matched and the PCI ID did not match,
(after all, the UDL device is NOT PCI), this code was executed:
else {
/* for non-seat0 servers assume first device is the master */
if (ServerIsNotSeat0())
break;
if (xf86IsPrimaryPlatform(_platform_devices[j]))
break;
}
So, probeSingleDevice() was called with xf86_platform_devices[0] and
devList[2], resulting in the UDL device set up as a GPU device and not
a framebuffer on its own right.
My MR modifies this so if there is an explicit Option "kmsdev" setting,
it's matched first. The final else branch is only executed in the default
case with no explicit configuration.
With this MR, the explicit configuration for UDL works, regardless the
AutoBindGPU value.
Best regards,
Zoltán Böszörményi
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Drm: mgag200. Video adapter issue with 5.4.0-rc3 ; no graphics
Hi Thomas: See below > On Nov 13, 2019, at 2:17 AM, Thomas Zimmermann wrote: > > Hi John > > Am 12.11.19 um 20:13 schrieb John Donnelly: >> >> In short . I started with : >> >> git bisect start >> >> git bisect bad be8454afc50f >> >> git bisect good fec88ab0af97 >> >> And at the the end of bisects showed this was the offending commit : >> >> c0a74c732568 >> >> commit c0a74c732568ad347f7b3de281922808dab30504 (refs/bisect/bad) >> Author: Jani Nikula >> Date: Fri May 24 20:35:22 2019 +0300 >> >>drm/i915: Update DRIVER_DATE to 20190524 >> >>Signed-off-by: Jani Nikula >> >> That does not have any real relevance > > No, that doesn't look right. The other bad commits you list below also > don't seem to be related. Thank you for your patience ; I started over and the bisect took to me to this change that certainly reflects the behavior I am seeing : commit 81da87f63a1edebcf8cbb811d387e353d9f89c7a (refs/bisect/bad) Author: Thomas Zimmermann Date: Tue May 21 13:08:29 2019 +0200 drm: Replace drm_gem_vram_push_to_system() with kunmap + unpin The push-to-system function forces a buffer out of video RAM. This decision should rather be made by the memory manager. By replacing the function with calls to the kunmap and unpin functions, the buffer's memory becomes available, but the buffer remains in VRAM until it's evicted by a pin operation. This patch replaces the remaining instances of drm_gem_vram_push_to_system() in ast and mgag200, and removes the function from DRM. My 1st impression is we need a method that restores the previous behavior that pushes the content to the device . > > You could also bisect between your original working commit (v4.18?) and > the one you want to upgrade to (v5.3?). It's only a few more builds but > might be might give better results. > > BTW, are you also converting Gnome from X11 to Wayland. I found that > Gnome's Wayland code is so slow on mgag200 that it's unusable. They > recently added a shadow FB [1] to improve performance, but it won't be > available before Gnome 3.36. I found this issue using gnome-desktop3-3.28.2-1.el8.x86_64 If there is a more specific. RPM I can look at for guidance I will . > > Best regards > Thomas > > [1] https://gitlab.gnome.org/GNOME/mutter/merge_requests/877/diffs > >> >> >> I am not sure if I did the bisects correctly . After each test I did : >> >> >> #1 git bisect bad 827440a90146 >> >> #2 git bisect bad f5b07b04e5f0 >> >> #3 git bisect bad c0a74c732568 >> >> #4 git bisect good 818f5cb3e8fb >> >> #5 git bisect good 6cfe7ec02e85 >> >> #6 git bisect good f71e01a78bee >> >> #7 git bisect good 09a93ef3d60f >> >> #8 git bisect good f1e6b336bafa >> >> #9 git bisect good eaf20e6933dc >> >> #10 git bisect good 63e8dcdb4f8e >> >> #11 git bisect good 397049a03022 >> >> I’ve restarted the bisect without appending the after a the >> “bad|good “ , and so far git is showing the same selections. snip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: ti-tfp410: switch to using fwnode_gpiod_get_index()
On Wed, Nov 13, 2019 at 02:52:23PM +0100, Linus Walleij wrote: > On Tue, Nov 5, 2019 at 4:41 PM Daniel Vetter wrote: > > On Tue, Nov 5, 2019 at 4:29 PM Linus Walleij > > wrote: > > > On Tue, Nov 5, 2019 at 1:40 AM Dmitry Torokhov > > > wrote: > > > > I'm happy to merge it into the GPIO tree if some DRM maintainer can > > > provide an ACK. > > > > Ack. > > Thanks! > > > > Getting ACK from DRM people is problematic and a bit of friction in the > > > community, DVetter usually advice to seek mutual reviews etc, but IMO > > > it would be better if some people felt more compelled to review stuff > > > eventually. (And that has the problem that it doesn't scale.) > > > > This has a review already plus if you merge your implied review. > > Yeah I missed Laurent's review tag. I needed some kund of consent > to take it into the GPIO tree I suppose. > > > That's more than good enough imo, so not seeing the issue here? > > No issue. > > What freaked me out was the option of having to pull in an > immutable branch from my GPIO tree into drm-misc. That would > have been scary. Keeping it all in my tree works fine. For next time around, just ping one of the drm-misc (or drm maintainers) for an ack to merge it through a different tree. Or ask them what they want to do. committer model isn't 100% free-wheeling, for cross-tree stuff you still have maintainers who're supposed to do their jobs :-) But by default everyone will assume you'll just commit, so you need to poke them explicitly (like you've done here). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: UDL device cannot get its own screen
On Wed, Nov 13, 2019 at 11:59 AM Böszörményi Zoltán wrote: > > 2019. 11. 12. 17:41 keltezéssel, Ilia Mirkin írta: > > On Tue, Nov 12, 2019 at 9:23 AM Böszörményi Zoltán wrote: > >> But no, all GPU devices (now only one, the UDL device) have screen 0 > >> (a.k.a. DISPLAY=:0.0) set when AutoBindGPU is true: > >> > >> [ 2444.576] xf86AutoConfigOutputDevices: xf86NumScreens 2 > >> xf86NumGPUScreens 1 > >> [ 2444.576] xf86AutoConfigOutputDevices: GPU #0 driver 'modesetting' > >> 'modeset' scrnIndex > >> 256 origIndex 257 pScreen->myNum 256 confScreen->screennum 0 > >> confScreen->device->identifier 'Intel0' > >>confScreen->device->screen 0 > >> confScreen->device->myScreenSection->screennum 0 > >> confScreen->device->myScreenSection->device->screen 0 > >> > >> Somehow, Option "Device" should ensure that the UDL device is actually > >> treated as a framebuffer that can be rendered into (i.e. to be modeset(2) > >> instead of modeset(Gn)) and it should be woken up automatically. > >> > >> This is what AutoBindGPU is supposed to do, isn't it? > >> > >> But instead of assigning to screen 0, it should be assigned to whatever > >> screen number it is configured as. > >> > >> I know it's not a common use case nowadays, but I really want separate > >> fullscreen apps on their independent screens, including a standalone UDL > >> device, instead of having the latters as a Xinerama extension to some > >> other device. > > > > If you see a "G", that means it's being treated as a GPU device, which > > is *not* what you want if you want separate screens. You need to try > > to convince things to *not* set the devices up as GPU devices, but > > instead put each device (and each one of its heads, via ZaphodHeads) > > no a separate device, which in turn will have a separate screen. > > I created a merge request that finally made it possible what I wanted. > > https://gitlab.freedesktop.org/xorg/xserver/merge_requests/334 > > Now, no matter if I use the intel or modesetting drivers for the > Device sections using the Intel heads, or AutoBindGPU set to true or > false, the UDL device is correctly matched with its Option "kmsdev" > setting to the plaform device's device path. > > This patch seems to be a slight layering violation, but since the > modesetting driver is built into the Xorg server sources, the patch > may get away with it. Have you looked at setting AutoAddGPU to false? AutoBindGPU is too late -- that's when you already have a GPU, whether to bind it to the primary device (/screen/whatever). You need to not have a GPU in the first place. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vmwgfx: Use coherent memory if there are dma mapping size restrictions
Hi,
On 11/13/19 3:16 PM, Christoph Hellwig wrote:
On Wed, Nov 13, 2019 at 10:51:43AM +0100, Thomas Hellström (VMware) wrote:
From: Thomas Hellstrom
We're gradually moving towards using DMA coherent memory in most
situations, although TTM interactions with the DMA layers is still a
work-in-progress. Meanwhile, use coherent memory when there are size
restrictions meaning that there is a chance that streaming dma mapping
of large buffer objects may fail.
Also move DMA mask settings to the vmw_dma_select_mode function, since
it's important that we set the correct DMA masks before calling the
dma_max_mapping_size() function.
Cc: Christoph Hellwig
Signed-off-by: Thomas Hellstrom
Reviewed-by: Brian Paul
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 31 +++--
1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index fc0283659c41..1e1de83908fe 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -569,7 +569,10 @@ static int vmw_dma_select_mode(struct vmw_private
*dev_priv)
[vmw_dma_map_populate] = "Caching DMA mappings.",
[vmw_dma_map_bind] = "Giving up DMA mappings early."};
- if (vmw_force_coherent)
+ (void) dma_set_mask_and_coherent(dev_priv->dev->dev, DMA_BIT_MASK(64));
Please don't use void cast on returns. Also this genercally can't
fail, so if it fails anyway it is fatal, and you should actually
return an error.
OK. Will fix.
+ if (vmw_force_coherent ||
+ dma_max_mapping_size(dev_priv->dev->dev) != SIZE_MAX)
I don't think this is the right check to see if swiotlb would be
used. You probably want to call dma_addressing_limited(). Please
also add a comment explaining the call here.
If I understand things correctly, dma_addressing_limited() would always
return false on vmwgfx, at least if the "restrict to 44 bit" option is
removed. The "odd" modes we want to catch is someone setting
swiotlb=force, or someone enabling SEV. In both cases, vmwgfx would
break down due to limited DMA space even if streaming DMA was fixed with
appropriate sync calls.
The same holds for vmw_pvscsi (which we discussed before) where the
large queue depth will typically fill the SWIOTLB causing excessive
non-fatal error logging.
dma_max_mapping_size() currently catch these modes, but best I guess
would be dma_swiotlb_forced() function that could be used in cases like
this?
if (dev_priv->map_mode != vmw_dma_phys &&
AFAIK vmw_dma_phys can't even be set in current mainline and is dead
code. Can you check if I'm missing something? Certainly all three
branches above don't set it.
I'll remove that dead code for v2.
(sizeof(unsigned long) == 4 || vmw_restrict_dma_mask)) {
DRM_INFO("Restricting DMA addresses to 44 bits.\n");
- return dma_set_mask_and_coherent(dev->dev, DMA_BIT_MASK(44));
+ return dma_set_mask_and_coherent(dev_priv->dev->dev,
+DMA_BIT_MASK(44));
Can you keep setting the DMA mask together?
E.g. make the whole thing something like:
static int vmw_dma_select_mode(struct vmw_private *dev_priv)
{
if (dma_addressing_limited(dev_priv->dev->dev) || vmw_force_coherent) {
/*
* XXX: what about AMD iommu? virtio-iommu? Also
* swiotlb should be always available where we can
* address more than 32-bit of memory..
*/
if (!IS_ENABLED(CONFIG_SWIOTLB) &&
!IS_ENABLED(CONFIG_INTEL_IOMMU))
return -EINVAL;
The above check is only to see if coherent memory functionality is
really compiled in into TTM. We have a patchset that got lost in the
last merge window to fix this properly and avoid confusion about this.
I'll rebase on that.
dev_priv->map_mode = vmw_dma_alloc_coherent;
} else if (vmw_restrict_iommu) {
dev_priv->map_mode = vmw_dma_map_bind;
} else {
dev_priv->map_mode = vmw_dma_map_populate;
}
/*
* On 32-bit systems we can only handle 32 bit PFNs. Optionally set
* that restriction also for 64-bit systems.
*/
return dma_set_mask_and_coherent(dev->dev,
(IS_ENABLED(CONFIG_64BIT) && !vmw_restrict_dma_mask) ?
64 : 44);
}
Thanks,
Thomas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: UDL device cannot get its own screen
2019. 11. 12. 17:41 keltezéssel, Ilia Mirkin írta: On Tue, Nov 12, 2019 at 9:23 AM Böszörményi Zoltán wrote: But no, all GPU devices (now only one, the UDL device) have screen 0 (a.k.a. DISPLAY=:0.0) set when AutoBindGPU is true: [ 2444.576] xf86AutoConfigOutputDevices: xf86NumScreens 2 xf86NumGPUScreens 1 [ 2444.576] xf86AutoConfigOutputDevices: GPU #0 driver 'modesetting' 'modeset' scrnIndex 256 origIndex 257 pScreen->myNum 256 confScreen->screennum 0 confScreen->device->identifier 'Intel0' confScreen->device->screen 0 confScreen->device->myScreenSection->screennum 0 confScreen->device->myScreenSection->device->screen 0 Somehow, Option "Device" should ensure that the UDL device is actually treated as a framebuffer that can be rendered into (i.e. to be modeset(2) instead of modeset(Gn)) and it should be woken up automatically. This is what AutoBindGPU is supposed to do, isn't it? But instead of assigning to screen 0, it should be assigned to whatever screen number it is configured as. I know it's not a common use case nowadays, but I really want separate fullscreen apps on their independent screens, including a standalone UDL device, instead of having the latters as a Xinerama extension to some other device. If you see a "G", that means it's being treated as a GPU device, which is *not* what you want if you want separate screens. You need to try to convince things to *not* set the devices up as GPU devices, but instead put each device (and each one of its heads, via ZaphodHeads) no a separate device, which in turn will have a separate screen. I created a merge request that finally made it possible what I wanted. https://gitlab.freedesktop.org/xorg/xserver/merge_requests/334 Now, no matter if I use the intel or modesetting drivers for the Device sections using the Intel heads, or AutoBindGPU set to true or false, the UDL device is correctly matched with its Option "kmsdev" setting to the plaform device's device path. This patch seems to be a slight layering violation, but since the modesetting driver is built into the Xorg server sources, the patch may get away with it. Best regards, Zoltán Böszörményi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from 'gfx_v6_0.c' and 'gfx_v7_0.c'
On Wed, 2019-11-13 at 20:44 +0800, yu kuai wrote: > Fixes gcc '-Wunused-but-set-variable' warning: > > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c: In function > ‘gfx_v6_0_constants_init’: > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:1579:6: warning: variable > ‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable] [] > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c [] > @@ -1678,7 +1678,6 @@ static void gfx_v6_0_constants_init(struct > amdgpu_device *adev) > > WREG32(mmBIF_FB_EN, BIF_FB_EN__FB_READ_EN_MASK | > BIF_FB_EN__FB_WRITE_EN_MASK); > > - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP); I do not know the hardware but frequently hardware like this has read ordering requirements and various registers can not be read in a random order. Does removing this read have no effect on the hardware? > adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG); > mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg; > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c [] > @@ -4336,7 +4336,6 @@ static void gfx_v7_0_gpu_early_init(struct > amdgpu_device *adev) > break; > } > > - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP); > adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG); > mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg; Same question. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 13/13] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
If the new video=... panel_orientation option is set for a connector, honor
it and setup a matching "panel orientation" property on the connector.
BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_connector.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 40a985c411a6..591914f01cd4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct
drm_connector *connector)
connector->force = mode->force;
}
+ if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
+ DRM_INFO("setting connector %s panel_orientation to %d\n",
+connector->name, mode->panel_orientation);
+ drm_connector_set_panel_orientation(connector,
+ mode->panel_orientation);
+ }
+
DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
connector->name, mode->name,
mode->xres, mode->yres,
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 10/13] drm/modes: parse_cmdline: Remove some unnecessary code (v2)
fb_get_options() will return fb_mode_option if no video=
argument is present on the kernel commandline, so there is no need to also
do this in drm_mode_parse_command_line_for_connector() as our only caller
uses fb_get_options() to get the mode_option argument.
Changes in v2:
-Split out the changes dealing with the initialization of the mode struct
into a separate patch
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 119fed7ab815..beb764efe6b3 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1747,11 +1747,6 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
-#ifdef CONFIG_FB
- if (!mode_option)
- mode_option = fb_mode_option;
-#endif
-
if (!mode_option) {
mode->specified = false;
return false;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 12/13] drm/connector: Split out orientation quirk detection (v2)
From: Derek Basehore
Not every platform needs quirk detection for panel orientation, so
split the drm_connector_init_panel_orientation_property into two
functions. One for platforms without the need for quirks, and the
other for platforms that need quirks.
Hans de Goede (changes in v2):
Rename the function from drm_connector_init_panel_orientation_property
to drm_connector_set_panel_orientation[_with_quirk] and pass in the
panel-orientation to set.
Beside the rename, also make the function set the passed in value
only once, if the value was set before (to a value other then
DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
This change is preparation for allowing the user to override the
panel-orientation for any connector from the kernel commandline.
When the panel-orientation is overridden this way, then we must ignore
the panel-orientation detection done by the driver.
Signed-off-by: Derek Basehore
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_connector.c | 74 ++---
drivers/gpu/drm/i915/display/icl_dsi.c | 5 +-
drivers/gpu/drm/i915/display/intel_dp.c | 9 ++-
drivers/gpu/drm/i915/display/vlv_dsi.c | 5 +-
include/drm/drm_connector.h | 9 ++-
5 files changed, 71 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..40a985c411a6 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1114,7 +1114,8 @@ static const struct drm_prop_enum_list hdmi_colorspaces[]
= {
* coordinates, so if userspace rotates the picture to adjust for
* the orientation it must also apply the same transformation to the
* touchscreen input coordinates. This property is initialized by calling
- * drm_connector_init_panel_orientation_property().
+ * drm_connector_set_panel_orientation() or
+ * drm_connector_set_panel_orientation_with_quirk()
*
* scaling mode:
* This property defines how a non-native mode is upscaled to the native
@@ -1986,38 +1987,41 @@ void drm_connector_set_vrr_capable_property(
EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
/**
- * drm_connector_init_panel_orientation_property -
- * initialize the connecters panel_orientation property
- * @connector: connector for which to init the panel-orientation property.
- * @width: width in pixels of the panel, used for panel quirk detection
- * @height: height in pixels of the panel, used for panel quirk detection
+ * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
+ * @connector: connector for which to set the panel-orientation property.
+ * @panel_orientation: drm_panel_orientation value to set
+ *
+ * This function sets the connector's panel_orientation and attaches
+ * a "panel orientation" property to the connector.
*
- * This function should only be called for built-in panels, after setting
- * connector->display_info.panel_orientation first (if known).
+ * Calling this function on a connector where the panel_orientation has
+ * already been set is a no-op (e.g. the orientation has been overridden with
+ * a kernel commandline option).
*
- * This function will check for platform specific (e.g. DMI based) quirks
- * overriding display_info.panel_orientation first, then if panel_orientation
- * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
- * "panel orientation" property to the connector.
+ * It is allowed to call this function with a panel_orientation of
+ * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
*
* Returns:
* Zero on success, negative errno on failure.
*/
-int drm_connector_init_panel_orientation_property(
- struct drm_connector *connector, int width, int height)
+int drm_connector_set_panel_orientation(
+ struct drm_connector *connector,
+ enum drm_panel_orientation panel_orientation)
{
struct drm_device *dev = connector->dev;
struct drm_display_info *info = >display_info;
struct drm_property *prop;
- int orientation_quirk;
- orientation_quirk = drm_get_panel_orientation_quirk(width, height);
- if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
- info->panel_orientation = orientation_quirk;
+ /* Already set? */
+ if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+ return 0;
- if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+ /* Don't attach the property if the orientation is unknown */
+ if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
return 0;
+ info->panel_orientation = panel_orientation;
+
prop = dev->mode_config.panel_orientation_property;
if (!prop) {
prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
@@ -2034,7 +2038,37 @@ int drm_connector_init_panel_orientation_property(
[PATCH v2 08/13] drm/modes: parse_cmdline: Allow specifying stand-alone options
Some options which can be specified on the commandline, such as
margin_right=..., margin_left=..., etc. are applied not only to the
specified mode, but to all modes. As such it would be nice if the user
can simply say e.g.
video=HDMI-1:margin_right=14,margin_left=24,margin_bottom=36,margin_top=42
This commit refactors drm_mode_parse_command_line_for_connector() to
add support for this, and as a nice side effect also cleans up the
function a bit.
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 92 +++
.../gpu/drm/selftests/drm_cmdline_selftests.h | 2 +
.../drm/selftests/test-drm_cmdline_parser.c | 50 ++
3 files changed, 86 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 72828fa9fc91..2e82603f5d0a 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1677,17 +1677,6 @@ static const char * const drm_named_modes_whitelist[] = {
"PAL",
};
-static bool drm_named_mode_is_in_whitelist(const char *mode, unsigned int size)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++)
- if (!strncmp(mode, drm_named_modes_whitelist[i], size))
- return true;
-
- return false;
-}
-
/**
* drm_mode_parse_command_line_for_connector - parse command line modeline for
connector
* @mode_option: optional per connector mode option
@@ -1718,7 +1707,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
struct drm_cmdline_mode *mode)
{
const char *name;
- bool named_mode = false, parse_extras = false;
+ bool freestanding = false, parse_extras = false;
unsigned int bpp_off = 0, refresh_off = 0, options_off = 0;
unsigned int mode_end = 0;
const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
@@ -1738,49 +1727,14 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
name = mode_option;
- /*
-* This is a bit convoluted. To differentiate between the
-* named modes and poorly formatted resolutions, we need a
-* bunch of things:
-* - We need to make sure that the first character (which
-* would be our resolution in X) is a digit.
-* - If not, then it's either a named mode or a force on/off.
-* To distinguish between the two, we need to run the
-* extra parsing function, and if not, then we consider it
-* a named mode.
-*
-* If this isn't enough, we should add more heuristics here,
-* and matching unit-tests.
-*/
- if (!isdigit(name[0]) && name[0] != 'x') {
- unsigned int namelen = strlen(name);
-
- /*
-* Only the force on/off options can be in that case,
-* and they all take a single character.
-*/
- if (namelen == 1) {
- ret = drm_mode_parse_cmdline_extra(name, namelen, true,
- connector, mode);
- if (!ret)
- return true;
- }
-
- named_mode = true;
- }
-
/* Try to locate the bpp and refresh specifiers, if any */
bpp_ptr = strchr(name, '-');
if (bpp_ptr)
bpp_off = bpp_ptr - name;
refresh_ptr = strchr(name, '@');
- if (refresh_ptr) {
- if (named_mode)
- return false;
-
+ if (refresh_ptr)
refresh_off = refresh_ptr - name;
- }
/* Locate the start of named options */
options_ptr = strchr(name, ',');
@@ -1800,23 +1754,45 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
parse_extras = true;
}
- if (named_mode) {
- if (mode_end + 1 > DRM_DISPLAY_MODE_LEN)
- return false;
+ /* First check for a named mode */
+ for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
+ ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
+ if (ret == mode_end) {
+ if (refresh_ptr)
+ return false; /* named + refresh is invalid */
- if (!drm_named_mode_is_in_whitelist(name, mode_end))
- return false;
+ strcpy(mode->name, drm_named_modes_whitelist[i]);
+ mode->specified = true;
+ break;
+ }
+ }
- strscpy(mode->name, name, mode_end + 1);
- } else {
+ /* No named mode? Check for a normal mode argument, e.g. 1024x768 */
+ if (!mode->specified && isdigit(name[0])) {
ret
[PATCH v2 07/13] drm/modes: parse_cmdline: Set bpp/refresh_specified after successful parsing
drm_connector_get_cmdline_mode() calls
drm_mode_parse_command_line_for_connector() with >cmdline_mode
as mode argument, so anything which we store in the mode arguments gets
kept even if we return false.
Avoid storing a possibly false-postive bpp/refresh_specified setting
in connector->cmdline_mode by moving the setting of these to after
successful parsing of the bpp/refresh parts of the video= argument.
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 80cb247c83c7..72828fa9fc91 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1771,10 +1771,8 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
/* Try to locate the bpp and refresh specifiers, if any */
bpp_ptr = strchr(name, '-');
- if (bpp_ptr) {
+ if (bpp_ptr)
bpp_off = bpp_ptr - name;
- mode->bpp_specified = true;
- }
refresh_ptr = strchr(name, '@');
if (refresh_ptr) {
@@ -1782,7 +1780,6 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
return false;
refresh_off = refresh_ptr - name;
- mode->refresh_specified = true;
}
/* Locate the start of named options */
@@ -1825,6 +1822,8 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
ret = drm_mode_parse_cmdline_bpp(bpp_ptr, _end_ptr, mode);
if (ret)
return false;
+
+ mode->bpp_specified = true;
}
if (refresh_ptr) {
@@ -1832,6 +1831,8 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
_end_ptr, mode);
if (ret)
return false;
+
+ mode->refresh_specified = true;
}
/*
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 04/13] drm/modes: parse_cmdline: Accept extras directly after mode combined with options
Before this commit it was impossible to combine an extra mode argument
specified directly after the resolution with an option, e.g.
video=HDMI-1:720x480e,rotate=180 would not work, either the "e" to force
enable would need to be dropped or the ",rotate=180", otherwise the
mode_option would not be accepted.
This commit fixes this by setting parse_extras to true in this case, so
that drm_mode_parse_cmdline_res_mode() parses the extra arguments directly
after the resolution.
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 1 +
.../gpu/drm/selftests/drm_cmdline_selftests.h | 1 +
.../drm/selftests/test-drm_cmdline_parser.c | 24 +++
3 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index a8aa7955fd45..f49401124727 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1794,6 +1794,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
mode_end = refresh_off;
} else if (options_ptr) {
mode_end = options_off;
+ parse_extras = true;
} else {
mode_end = strlen(name);
parse_extras = true;
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
index ca1fc7a78953..003e2c3ffc39 100644
--- a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
@@ -61,3 +61,4 @@ cmdline_test(drm_cmdline_test_margin_options)
cmdline_test(drm_cmdline_test_multiple_options)
cmdline_test(drm_cmdline_test_invalid_option)
cmdline_test(drm_cmdline_test_bpp_extra_and_option)
+cmdline_test(drm_cmdline_test_extra_and_option)
diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
index 5b8dea922257..bc4db017e993 100644
--- a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
+++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
@@ -1018,6 +1018,30 @@ static int drm_cmdline_test_bpp_extra_and_option(void
*ignored)
return 0;
}
+static int drm_cmdline_test_extra_and_option(void *ignored)
+{
+ struct drm_cmdline_mode mode = { };
+
+
FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480e,rotate=180",
+ _connector,
+ ));
+ FAIL_ON(!mode.specified);
+ FAIL_ON(mode.xres != 720);
+ FAIL_ON(mode.yres != 480);
+ FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
+
+ FAIL_ON(mode.refresh_specified);
+ FAIL_ON(mode.bpp_specified);
+
+ FAIL_ON(mode.rb);
+ FAIL_ON(mode.cvt);
+ FAIL_ON(mode.interlace);
+ FAIL_ON(mode.margins);
+ FAIL_ON(mode.force != DRM_FORCE_ON);
+
+ return 0;
+}
+
#include "drm_selftest.c"
static int __init test_drm_cmdline_init(void)
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 06/13] drm/modes: parse_cmdline: Add freestanding argument to drm_mode_parse_cmdline_options()
Add a freestanding function argument to drm_mode_parse_cmdline_options()
similar to how drm_mode_parse_cmdline_extra() already has this.
This is a preparation patch for allowing parsing of stand-alone options
without a mode before them, e.g.: video=HDMI-1:margin_right=14,...
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 25e8edf4cfb8..80cb247c83c7 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1592,6 +1592,7 @@ static int drm_mode_parse_cmdline_int(const char *delim,
unsigned int *int_ret)
}
static int drm_mode_parse_cmdline_options(const char *str,
+ bool freestanding,
const struct drm_connector *connector,
struct drm_cmdline_mode *mode)
{
@@ -1663,6 +1664,9 @@ static int drm_mode_parse_cmdline_options(const char *str,
option = sep + 1;
} while (sep);
+ if (rotation && freestanding)
+ return -EINVAL;
+
mode->rotation_reflection = rotation;
return 0;
@@ -1855,6 +1859,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
if (options_ptr) {
ret = drm_mode_parse_cmdline_options(options_ptr + 1,
+false,
connector, mode);
if (ret)
return false;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 09/13] drm/modes: parse_cmdline: Add support for specifying panel_orientation (v2)
Sometimes we want to override a connector's panel_orientation from the
kernel commandline. Either for testing and for special cases, e.g. a kiosk
like setup which uses a TV mounted in portrait mode.
Users can already specify a "rotate" option through a video= kernel cmdline
option. But that only supports 0/180 degrees (see drm_client_modeset TODO)
and only works for in kernel modeset clients, not for userspace kms users.
The "panel-orientation" connector property OTOH does support 90/270 degrees
as it leaves dealing with the rotation up to userspace and this does work
for userspace kms clients (at least those which support this property).
Changes in v2:
-Add missing ':' after @panel_orientation (reported by kbuild test robot)
BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
Signed-off-by: Hans de Goede
---
Documentation/fb/modedb.rst | 3 ++
drivers/gpu/drm/drm_modes.c | 32 +++
.../gpu/drm/selftests/drm_cmdline_selftests.h | 1 +
.../drm/selftests/test-drm_cmdline_parser.c | 22 +
include/drm/drm_connector.h | 8 +
5 files changed, 66 insertions(+)
diff --git a/Documentation/fb/modedb.rst b/Documentation/fb/modedb.rst
index 9c4e3fd39e6d..624d08fd2856 100644
--- a/Documentation/fb/modedb.rst
+++ b/Documentation/fb/modedb.rst
@@ -65,6 +65,9 @@ Valid options are::
- reflect_y (boolean): Perform an axial symmetry on the Y axis
- rotate (integer): Rotate the initial framebuffer by x
degrees. Valid values are 0, 90, 180 and 270.
+ - panel_orientation, one of "normal", "upside_down", "left_side_up", or
+"right_side_up". For KMS drivers only, this sets the "panel orientation"
+property on the kms connector as hint for kms users.
-
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 2e82603f5d0a..119fed7ab815 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1591,6 +1591,33 @@ static int drm_mode_parse_cmdline_int(const char *delim,
unsigned int *int_ret)
return 0;
}
+static int drm_mode_parse_panel_orientation(const char *delim,
+ struct drm_cmdline_mode *mode)
+{
+ const char *value;
+
+ if (*delim != '=')
+ return -EINVAL;
+
+ value = delim + 1;
+ delim = strchr(value, ',');
+ if (!delim)
+ delim = value + strlen(value);
+
+ if (!strncmp(value, "normal", delim - value))
+ mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
+ else if (!strncmp(value, "upside_down", delim - value))
+ mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+ else if (!strncmp(value, "left_side_up", delim - value))
+ mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+ else if (!strncmp(value, "right_side_up", delim - value))
+ mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
static int drm_mode_parse_cmdline_options(const char *str,
bool freestanding,
const struct drm_connector *connector,
@@ -1657,6 +1684,9 @@ static int drm_mode_parse_cmdline_options(const char *str,
return -EINVAL;
mode->tv_margins.bottom = margin;
+ } else if (!strncmp(option, "panel_orientation", delim -
option)) {
+ if (drm_mode_parse_panel_orientation(delim, mode))
+ return -EINVAL;
} else {
return -EINVAL;
}
@@ -1715,6 +1745,8 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
int i, len, ret;
+ mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+
#ifdef CONFIG_FB
if (!mode_option)
mode_option = fb_mode_option;
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
index aee92ac2cc21..ceac7af9a172 100644
--- a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
@@ -64,3 +64,4 @@ cmdline_test(drm_cmdline_test_bpp_extra_and_option)
cmdline_test(drm_cmdline_test_extra_and_option)
cmdline_test(drm_cmdline_test_freestanding_options)
cmdline_test(drm_cmdline_test_freestanding_force_e_and_options)
+cmdline_test(drm_cmdline_test_panel_orientation)
diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
index 8248d4aa5aaa..520f3e66a384 100644
--- a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
+++
[PATCH v2 11/13] drm/modes: parse_cmdline: Explicitly memset the passed in drm_cmdline_mode struct
Instead of only setting mode->specified on false on an early exit and
leaving e.g. mode->bpp_specified and mode->refresh_specified as is,
lets be consistent and just zero out the entire passed in struct at
the top of drm_mode_parse_command_line_for_connector()
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index beb764efe6b3..1fee4a71eff7 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1745,12 +1745,11 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
int i, len, ret;
+ memset(mode, 0, sizeof(*mode));
mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
- if (!mode_option) {
+ if (!mode_option)
mode->specified = false;
- return false;
- }
name = mode_option;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 05/13] drm/modes: parse_cmdline: Rework drm_mode_parse_cmdline_options()
Refactor drm_mode_parse_cmdline_options() so that it takes a pointer
to the first option, rather then a pointer to the ',' before the first
option.
This is a preparation patch for allowing parsing of stand-alone options
without a mode before them, e.g.: video=HDMI-1:margin_right=14,...
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index f49401124727..25e8edf4cfb8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1591,23 +1591,21 @@ static int drm_mode_parse_cmdline_int(const char
*delim, unsigned int *int_ret)
return 0;
}
-static int drm_mode_parse_cmdline_options(const char *str, size_t len,
+static int drm_mode_parse_cmdline_options(const char *str,
const struct drm_connector *connector,
struct drm_cmdline_mode *mode)
{
unsigned int deg, margin, rotation = 0;
- const char *sep = str;
+ const char *delim, *option, *sep;
- while ((sep = strchr(sep, ','))) {
- const char *delim, *option;
-
- option = sep + 1;
+ option = str;
+ do {
delim = strchr(option, '=');
if (!delim) {
delim = strchr(option, ',');
if (!delim)
- delim = str + len;
+ delim = option + strlen(option);
}
if (!strncmp(option, "rotate", delim - option)) {
@@ -1661,8 +1659,9 @@ static int drm_mode_parse_cmdline_options(const char
*str, size_t len,
} else {
return -EINVAL;
}
- sep = delim;
- }
+ sep = strchr(delim, ',');
+ option = sep + 1;
+ } while (sep);
mode->rotation_reflection = rotation;
@@ -1855,9 +1854,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
}
if (options_ptr) {
- int len = strlen(name) - (options_ptr - name);
-
- ret = drm_mode_parse_cmdline_options(options_ptr, len,
+ ret = drm_mode_parse_cmdline_options(options_ptr + 1,
connector, mode);
if (ret)
return false;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 02/13] drm/modes: parse_cmdline: Make various char pointers const
We are not supposed to modify the passed in string, make char pointers
used in drm_mode_parse_cmdline_options() const char * where possible.
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 3c3c7435225f..654d4b6fecb3 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1591,15 +1591,15 @@ static int drm_mode_parse_cmdline_int(const char
*delim, unsigned int *int_ret)
return 0;
}
-static int drm_mode_parse_cmdline_options(char *str, size_t len,
+static int drm_mode_parse_cmdline_options(const char *str, size_t len,
const struct drm_connector *connector,
struct drm_cmdline_mode *mode)
{
unsigned int deg, margin, rotation = 0;
- char *sep = str;
+ const char *sep = str;
while ((sep = strchr(sep, ','))) {
- char *delim, *option;
+ const char *delim, *option;
option = sep + 1;
delim = strchr(option, '=');
@@ -1718,8 +1718,8 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
bool named_mode = false, parse_extras = false;
unsigned int bpp_off = 0, refresh_off = 0, options_off = 0;
unsigned int mode_end = 0;
- char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
- char *options_ptr = NULL;
+ const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
+ const char *options_ptr = NULL;
char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
int ret;
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 01/13] drm/modes: parse_cmdline: Fix possible reference past end of string
Before this commit, if the last option of a video=... option is for
example "rotate" without a "=" after it then delim will point to
the terminating 0 of the string, and value which is sets to
will point one position past the end of the string.
This commit fixes this by enforcing that the contents of delim equals '='
as it should be for options which take a value, this check is done in a
new drm_mode_parse_cmdline_int helper function which factors out the
common integer parsing code for all the options which take an int.
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 68 -
1 file changed, 30 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 88232698d7a0..3c3c7435225f 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1568,11 +1568,34 @@ static int drm_mode_parse_cmdline_res_mode(const char
*str, unsigned int length,
return 0;
}
+static int drm_mode_parse_cmdline_int(const char *delim, unsigned int *int_ret)
+{
+ const char *value;
+ char *endp;
+
+ /*
+* delim must point to the '=', otherwise it is a syntax error and
+* if delim points to the terminating zero, then delim + 1 wil point
+* past the end of the string.
+*/
+ if (*delim != '=')
+ return -EINVAL;
+
+ value = delim + 1;
+ *int_ret = simple_strtol(value, , 10);
+
+ /* Make sure we have parsed something */
+ if (endp == value)
+ return -EINVAL;
+
+ return 0;
+}
+
static int drm_mode_parse_cmdline_options(char *str, size_t len,
const struct drm_connector *connector,
struct drm_cmdline_mode *mode)
{
- unsigned int rotation = 0;
+ unsigned int deg, margin, rotation = 0;
char *sep = str;
while ((sep = strchr(sep, ','))) {
@@ -1588,13 +1611,7 @@ static int drm_mode_parse_cmdline_options(char *str,
size_t len,
}
if (!strncmp(option, "rotate", delim - option)) {
- const char *value = delim + 1;
- unsigned int deg;
-
- deg = simple_strtol(value, , 10);
-
- /* Make sure we have parsed something */
- if (sep == value)
+ if (drm_mode_parse_cmdline_int(delim, ))
return -EINVAL;
switch (deg) {
@@ -1619,57 +1636,32 @@ static int drm_mode_parse_cmdline_options(char *str,
size_t len,
}
} else if (!strncmp(option, "reflect_x", delim - option)) {
rotation |= DRM_MODE_REFLECT_X;
- sep = delim;
} else if (!strncmp(option, "reflect_y", delim - option)) {
rotation |= DRM_MODE_REFLECT_Y;
- sep = delim;
} else if (!strncmp(option, "margin_right", delim - option)) {
- const char *value = delim + 1;
- unsigned int margin;
-
- margin = simple_strtol(value, , 10);
-
- /* Make sure we have parsed something */
- if (sep == value)
+ if (drm_mode_parse_cmdline_int(delim, ))
return -EINVAL;
mode->tv_margins.right = margin;
} else if (!strncmp(option, "margin_left", delim - option)) {
- const char *value = delim + 1;
- unsigned int margin;
-
- margin = simple_strtol(value, , 10);
-
- /* Make sure we have parsed something */
- if (sep == value)
+ if (drm_mode_parse_cmdline_int(delim, ))
return -EINVAL;
mode->tv_margins.left = margin;
} else if (!strncmp(option, "margin_top", delim - option)) {
- const char *value = delim + 1;
- unsigned int margin;
-
- margin = simple_strtol(value, , 10);
-
- /* Make sure we have parsed something */
- if (sep == value)
+ if (drm_mode_parse_cmdline_int(delim, ))
return -EINVAL;
mode->tv_margins.top = margin;
} else if (!strncmp(option, "margin_bottom", delim - option)) {
- const char *value = delim + 1;
- unsigned int margin;
-
- margin = simple_strtol(value, , 10);
-
- /* Make sure we have parsed something */
- if (sep == value)
+
[PATCH v2 03/13] drm/modes: parse_cmdline: Stop parsing extras after bpp / refresh at ', '
Before this commit it was impossible to add an extra mode argument after
a bpp or refresh specifier, combined with an option, e.g.
video=HDMI-1:720x480-24e,rotate=180 would not work, either the "e" to
force enable would need to be dropped or the ",rotate=180", otherwise
the mode_option would not be accepted.
This commit fixes this by fixing the length calculation if extras_ptr
is set to stop the extra parsing at the start of the options (stop at the
',' options_ptr points to).
Acked-by: Maxime Ripard
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_modes.c | 10 ---
.../gpu/drm/selftests/drm_cmdline_selftests.h | 1 +
.../drm/selftests/test-drm_cmdline_parser.c | 26 +++
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 654d4b6fecb3..a8aa7955fd45 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1721,7 +1721,7 @@ bool drm_mode_parse_command_line_for_connector(const char
*mode_option,
const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
const char *options_ptr = NULL;
char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
- int ret;
+ int i, len, ret;
#ifdef CONFIG_FB
if (!mode_option)
@@ -1841,9 +1841,11 @@ bool drm_mode_parse_command_line_for_connector(const
char *mode_option,
else if (refresh_ptr)
extra_ptr = refresh_end_ptr;
- if (extra_ptr &&
- extra_ptr != options_ptr) {
- int len = strlen(name) - (extra_ptr - name);
+ if (extra_ptr) {
+ if (options_ptr)
+ len = options_ptr - extra_ptr;
+ else
+ len = strlen(extra_ptr);
ret = drm_mode_parse_cmdline_extra(extra_ptr, len, false,
connector, mode);
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
index 6d61a0eb5d64..ca1fc7a78953 100644
--- a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
+++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
@@ -60,3 +60,4 @@ cmdline_test(drm_cmdline_test_vmirror)
cmdline_test(drm_cmdline_test_margin_options)
cmdline_test(drm_cmdline_test_multiple_options)
cmdline_test(drm_cmdline_test_invalid_option)
+cmdline_test(drm_cmdline_test_bpp_extra_and_option)
diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
index 013de9d27c35..5b8dea922257 100644
--- a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
+++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
@@ -992,6 +992,32 @@ static int drm_cmdline_test_invalid_option(void *ignored)
return 0;
}
+static int drm_cmdline_test_bpp_extra_and_option(void *ignored)
+{
+ struct drm_cmdline_mode mode = { };
+
+
FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24e,rotate=180",
+ _connector,
+ ));
+ FAIL_ON(!mode.specified);
+ FAIL_ON(mode.xres != 720);
+ FAIL_ON(mode.yres != 480);
+ FAIL_ON(mode.rotation_reflection != DRM_MODE_ROTATE_180);
+
+ FAIL_ON(mode.refresh_specified);
+
+ FAIL_ON(!mode.bpp_specified);
+ FAIL_ON(mode.bpp != 24);
+
+ FAIL_ON(mode.rb);
+ FAIL_ON(mode.cvt);
+ FAIL_ON(mode.interlace);
+ FAIL_ON(mode.margins);
+ FAIL_ON(mode.force != DRM_FORCE_ON);
+
+ return 0;
+}
+
#include "drm_selftest.c"
static int __init test_drm_cmdline_init(void)
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/ast: Call struct drm_driver.{load, unload} before registering device
On Wed, Nov 13, 2019 at 04:58:57PM +0100, Thomas Zimmermann wrote:
> Both callbacks are deprecated. Remove them and call functions explicitly.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/ast/ast_drv.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 78c90a3c903b..9da26750a22d 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -104,17 +104,24 @@ static int ast_pci_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> dev->pdev = pdev;
> pci_set_drvdata(pdev, dev);
>
> - ret = drm_dev_register(dev, ent->driver_data);
> + ret = ast_driver_load(dev, ent->driver_data);
> if (ret)
> goto err_drm_dev_put;
>
> + ret = drm_dev_register(dev, ent->driver_data);
> + if (ret)
> + goto err_ast_driver_unload;
> +
> return 0;
>
> +err_ast_driver_unload:
> + ast_driver_unload(dev);
> err_drm_dev_put:
> drm_dev_put(dev);
> err_pci_disable_device:
> pci_disable_device(pdev);
> return ret;
> +
> }
>
> static void
> @@ -123,6 +130,7 @@ ast_pci_remove(struct pci_dev *pdev)
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> drm_dev_unregister(dev);
> + ast_driver_unload(dev);
> drm_dev_put(dev);
> }
>
> @@ -228,9 +236,6 @@ static struct drm_driver driver = {
> DRIVER_GEM |
> DRIVER_MODESET,
>
> - .load = ast_driver_load,
> - .unload = ast_driver_unload,
> -
> .fops = _fops,
> .name = DRIVER_NAME,
> .desc = DRIVER_DESC,
> --
> 2.23.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/ast: Replace drm_get_pci_device() and drm_put_dev()
On Wed, Nov 13, 2019 at 05:35:41PM +0100, Daniel Vetter wrote:
> On Wed, Nov 13, 2019 at 04:58:56PM +0100, Thomas Zimmermann wrote:
> > Both functions are deprecated. Open-code them them in preparation
> > of removing struct drm_driver.{load,unload}.
> >
> > Signed-off-by: Thomas Zimmermann
> > ---
> > drivers/gpu/drm/ast/ast_drv.c | 31 +--
> > 1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> > index d763da6f0834..78c90a3c903b 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.c
> > +++ b/drivers/gpu/drm/ast/ast_drv.c
> > @@ -86,9 +86,35 @@ static void ast_kick_out_firmware_fb(struct pci_dev
> > *pdev)
> >
> > static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id
> > *ent)
> > {
> > + struct drm_device *dev;
> > + int ret;
> > +
> > ast_kick_out_firmware_fb(pdev);
> >
> > - return drm_get_pci_dev(pdev, ent, );
> > + ret = pci_enable_device(pdev);
> > + if (ret)
> > + return ret;
> > +
> > + dev = drm_dev_alloc(, >dev);
>
> Would be nice to also switch over to embedding drm_device and
> drm_dev_init, but for another patch. This is
Even better: devm_drm_dev_init, and dropping the drm_dev_put/kfree at the
bottom of the pci_remove function.
-Daniel
>
> Reviewed-by: Daniel Vetter
>
> > + if (IS_ERR(dev)) {
> > + ret = PTR_ERR(dev);
> > + goto err_pci_disable_device;
> > + }
> > +
> > + dev->pdev = pdev;
> > + pci_set_drvdata(pdev, dev);
> > +
> > + ret = drm_dev_register(dev, ent->driver_data);
> > + if (ret)
> > + goto err_drm_dev_put;
> > +
> > + return 0;
> > +
> > +err_drm_dev_put:
> > + drm_dev_put(dev);
> > +err_pci_disable_device:
> > + pci_disable_device(pdev);
> > + return ret;
> > }
> >
> > static void
> > @@ -96,7 +122,8 @@ ast_pci_remove(struct pci_dev *pdev)
> > {
> > struct drm_device *dev = pci_get_drvdata(pdev);
> >
> > - drm_put_dev(dev);
> > + drm_dev_unregister(dev);
> > + drm_dev_put(dev);
> > }
> >
> > static int ast_drm_freeze(struct drm_device *dev)
> > --
> > 2.23.0
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm: bridge: dw_mipi_dsi: access registers via a regmap
On Wed, 13 Nov 2019, Emil Velikov wrote: On Wed, 6 Nov 2019 at 16:30, Adrian Ratiu wrote: Convert the common bridge code and the two rockchip & stm drivers which currently use it to the regmap API in anticipation for further changes to make it more generic and add older DSI host controller support as found on i.mx6 based devices. The regmap becomes an internal state of the bridge. No functional changes other than requiring the platform drivers to use the pre-configured regmap supplied by the bridge after its probe() call instead of ioremp'ing the registers themselves. In subsequent commits the bridge will become able to detect the DSI host core version and init the regmap with different register layouts. The platform drivers will continue to use the regmap without modifications or worrying about the specific layout in use (in other words the layout is abstracted away via the regmap). Suggested-by: Boris Brezillon Reviewed-by: Neil Armstrong Reviewed-by: Emil Velikov I should have been clearer earlier - I didn't quite review the patch. Is is now though. Reviewed-by: Emil Velikov Sorry about that, I got confused and thought you reviewed it all. Admittedly a couple of nitpicks (DRIVER_NAME, zero initialize of val) could have been left out. It's not a big deal, there's no need to polish those. I'll address them in v3 as well as updating your mail address. Thanks for reviewing! -Emil ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/ast: Replace drm_get_pci_device() and drm_put_dev()
On Wed, Nov 13, 2019 at 04:58:56PM +0100, Thomas Zimmermann wrote:
> Both functions are deprecated. Open-code them them in preparation
> of removing struct drm_driver.{load,unload}.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/ast/ast_drv.c | 31 +--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index d763da6f0834..78c90a3c903b 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -86,9 +86,35 @@ static void ast_kick_out_firmware_fb(struct pci_dev *pdev)
>
> static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id
> *ent)
> {
> + struct drm_device *dev;
> + int ret;
> +
> ast_kick_out_firmware_fb(pdev);
>
> - return drm_get_pci_dev(pdev, ent, );
> + ret = pci_enable_device(pdev);
> + if (ret)
> + return ret;
> +
> + dev = drm_dev_alloc(, >dev);
Would be nice to also switch over to embedding drm_device and
drm_dev_init, but for another patch. This is
Reviewed-by: Daniel Vetter
> + if (IS_ERR(dev)) {
> + ret = PTR_ERR(dev);
> + goto err_pci_disable_device;
> + }
> +
> + dev->pdev = pdev;
> + pci_set_drvdata(pdev, dev);
> +
> + ret = drm_dev_register(dev, ent->driver_data);
> + if (ret)
> + goto err_drm_dev_put;
> +
> + return 0;
> +
> +err_drm_dev_put:
> + drm_dev_put(dev);
> +err_pci_disable_device:
> + pci_disable_device(pdev);
> + return ret;
> }
>
> static void
> @@ -96,7 +122,8 @@ ast_pci_remove(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
>
> - drm_put_dev(dev);
> + drm_dev_unregister(dev);
> + drm_dev_put(dev);
> }
>
> static int ast_drm_freeze(struct drm_device *dev)
> --
> 2.23.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: fix mmap refcounting
On Wed, Nov 13, 2019 at 02:56:12PM +0100, Gerd Hoffmann wrote:
> When mapping ttm objects via drm_gem_ttm_mmap() helper
> drm_gem_mmap_obj() will take an object reference. That gets
> never released due to ttm having its own reference counting.
>
> Fix that by dropping the gem object reference once the ttm mmap
> completed (and ttm refcount got bumped).
>
> For that to work properly the drm_gem_object_get() call in
> drm_gem_ttm_mmap() must be moved so it happens before calling
> obj->funcs->mmap(), otherwise the gem refcount would go down
> to zero.
>
> Fixes: 231927d939f0 ("drm/ttm: add drm_gem_ttm_mmap()")
Since the offending patch is in drm-next and we're in the merge window
freeze past -rc6 please remember to apply this to drm-misc-next-fixes.
Otherwise it'll miss the merge window.
> Signed-off-by: Gerd Hoffmann
I was wondering whether we'd need a cc: stable, in case someone is really
fast and gets the vm_close in before we finish the mmap. But I think we
should be serialized by mmap_sem here enough ...
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_gem.c| 24 ++--
> drivers/gpu/drm/drm_gem_ttm_helper.c | 13 -
> 2 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 2f2b889096b0..000fa4a1899f 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1105,21 +1105,33 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj,
> unsigned long obj_size,
> if (obj_size < vma->vm_end - vma->vm_start)
> return -EINVAL;
>
> + /* Take a ref for this mapping of the object, so that the fault
> + * handler can dereference the mmap offset's pointer to the object.
> + * This reference is cleaned up by the corresponding vm_close
> + * (which should happen whether the vma was created by this call, or
> + * by a vm_open due to mremap or partial unmap or whatever).
> + */
> + drm_gem_object_get(obj);
> +
> if (obj->funcs && obj->funcs->mmap) {
> /* Remove the fake offset */
> vma->vm_pgoff -= drm_vma_node_start(>vma_node);
>
> ret = obj->funcs->mmap(obj, vma);
> - if (ret)
> + if (ret) {
> + drm_gem_object_put_unlocked(obj);
> return ret;
> + }
> WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
> } else {
> if (obj->funcs && obj->funcs->vm_ops)
> vma->vm_ops = obj->funcs->vm_ops;
> else if (dev->driver->gem_vm_ops)
> vma->vm_ops = dev->driver->gem_vm_ops;
> - else
> + else {
> + drm_gem_object_put_unlocked(obj);
> return -EINVAL;
> + }
>
> vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND |
> VM_DONTDUMP;
> vma->vm_page_prot =
> pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> @@ -1128,14 +1140,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj,
> unsigned long obj_size,
>
> vma->vm_private_data = obj;
>
> - /* Take a ref for this mapping of the object, so that the fault
> - * handler can dereference the mmap offset's pointer to the object.
> - * This reference is cleaned up by the corresponding vm_close
> - * (which should happen whether the vma was created by this call, or
> - * by a vm_open due to mremap or partial unmap or whatever).
> - */
> - drm_gem_object_get(obj);
> -
> return 0;
> }
> EXPORT_SYMBOL(drm_gem_mmap_obj);
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
> b/drivers/gpu/drm/drm_gem_ttm_helper.c
> index 7412bfc5c05a..605a8a3da7f9 100644
> --- a/drivers/gpu/drm/drm_gem_ttm_helper.c
> +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
> @@ -64,8 +64,19 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
>struct vm_area_struct *vma)
> {
> struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
> + int ret;
>
> - return ttm_bo_mmap_obj(vma, bo);
> + ret = ttm_bo_mmap_obj(vma, bo);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * ttm has its own object refcounting, so drop gem reference
> + * to avoid double accounting counting.
> + */
> + drm_gem_object_put_unlocked(gem);
> +
> + return 0;
> }
> EXPORT_SYMBOL(drm_gem_ttm_mmap);
>
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Wed, Nov 13, 2019 at 07:53:56AM -0600, Rob Herring wrote: > On Wed, Nov 13, 2019 at 2:18 AM Daniel Vetter wrote: > > > > On Wed, Nov 13, 2019 at 8:39 AM Gerd Hoffmann wrote: > > > > > > Hi, > > > > > > > > > >> VRAM helpers use drm_gem_ttm_mmap(), which wraps > > > > > > >> ttm_bo_mmap_obj(). > > > > > > >> These changes should be transparent. > > > > > > > > > > > > > > There's still the issue that for dma-buf mmap vs drm mmap you use > > > > > > > different f_mapping, which means ttm's pte shootdown won't work > > > > > > > correctly > > > > > > > for dma-buf mmaps. But yeah normal operation for ttm/vram helpers > > > > > > > should > > > > > > > be fine. > > > > > > > > > > > > VRAM helpers don't support dmabufs. > > > > > > > > > > It's not that simple. Even when not supporting dma-buf export and > > > > > import it is still possible to create dma-bufs, import them into the > > > > > same driver (which doesn't actually export+import but just grabs a gem > > > > > object reference instead) and also to mmap them via prime/dma-buf code > > > > > path ... > > > > > > ... but after looking again I think we are all green here. Given that > > > only self-import works we'll only see vram gem objects in the mmap code > > > path, which should have everything set up correctly. Same goes for qxl. > > > > > > All other ttm drivers still use the old mmap code path, so all green > > > there too I think. Also I somehow doubt dma-buf mmap vs. drm mmap ends > > > up using different f_mapping, ttm code has a WARN_ON in ttm_bo_vm_open() > > > which would fire should that be the case. > > > > > > Do imported dma-bufs hit the drm mmap code path in the first place? > > > Wouldn't mmap be handled by the exporting driver? > > > > drm_gem_dmabuf_mmap -> obj->funcs->mmap -> ttm_bo_mmap_obj > > > > And I'm not seeing anyone adjusting vm_file->f_mapping anywhere here at all. > > > > Note to hit this you need userspace to > > - handle2fd on a buffer to create a dma-buf fd > > - call mmap directly on that dma-buf fd > > That was exactly the vgem IGT test that prompted $subject fix. (With > vgem modified to use shmem helpers) See my other mail, this only hits the right mmap paths. For the unmap side you need even more (and that part is driver dependent, and vgem would need some debug tricks to expose that for testing). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap
On Wed, Nov 13, 2019 at 02:51:45PM +0100, Gerd Hoffmann wrote: > Hi, > > > > ... but after looking again I think we are all green here. Given that > > > only self-import works we'll only see vram gem objects in the mmap code > > > path, which should have everything set up correctly. Same goes for qxl. > > > > > > All other ttm drivers still use the old mmap code path, so all green > > > there too I think. Also I somehow doubt dma-buf mmap vs. drm mmap ends > > > up using different f_mapping, ttm code has a WARN_ON in ttm_bo_vm_open() > > > which would fire should that be the case. > > > > > > Do imported dma-bufs hit the drm mmap code path in the first place? > > > Wouldn't mmap be handled by the exporting driver? > > > > drm_gem_dmabuf_mmap -> obj->funcs->mmap -> ttm_bo_mmap_obj > > > > And I'm not seeing anyone adjusting vm_file->f_mapping anywhere here at all. > > [ some more code browsing ] > > Ok, I see. dma-bufs get their own file, their own anon inode and > thereby their own address space. So that it used when mmaping the > dma-buf. > > drm filehandle's get the shared address space instead, drm_open() sets > it. > > So, yes, I see the problem. It's not new though, as far I can see the > old dma-buf mmap code path doesn't adjust f_mapping anywhere either ... > > > Note to hit this you need userspace to > > - handle2fd on a buffer to create a dma-buf fd > > - call mmap directly on that dma-buf fd > > Hmm, seems for handle2fd I need a dummy gem_prime_get_sg_table function > wired up even when not actually exporting/importing anything. So I > think neither qxl nor any of the vram drivers allow to trigger that (and > no other ttm driver uses the new ttm mmap code yet). > > So, $subject patch should not make things worse in ttm land. > > When hacking the bochs driver to have export callbacks (without > supporting actual exports) handle2fd + mmap() callback works fine. > Didn't verify yet I actually get the correct pages mapped. But maybe > mmap() isn't the problem when the correct address space is important for > unmap only. > > Is there a good test case? You need memory pressure, to force ttm to unmap the bo, not userspace. So roughly 1. create bo 2. mmap it through drm fd, write some stuff 3. export to dma-buf, mmap it, verify stuff is there 4. create a pile more bo, mmap them write to them 5. once you've thrashed all of vram enough, recheck your original bo. If I'm right you should get the following: - drm fd mmap still show right content - dma-buf fd mmap shows random crap that you've written into other buffers Ofc you need to make sure that an mmap actually forces the buffer into vram. So might need a combo of modeset+mmap, to make that happen. Plain mmap might just give you ptes that point into system memory, which is not managed by ttm like vram. munmap called by userspace isn't a problem, since that starts from the right struct mm_struct. It's when you start with the object (i.e. in the driver), and need to figure out where all the various vma that mmap it are where this matters. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 111123] Display issues AMD RAVEN Ryzen 5 3400G APU
https://bugs.freedesktop.org/show_bug.cgi?id=23 --- Comment #4 from Leonardo da Silva --- i have the same problem, I bought 2x 3400g with A320-S2H, one machine running ubuntu 19.10 and the other Arch Linux, both with the same problem described above when using integrated graphics. -- 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 09/12] drm/modes: parse_cmdline: Add support for specifying panel_orientation
On Mon, Nov 11, 2019 at 06:25:33PM +0100, Hans de Goede wrote:
> Hi,
>
> On 11-11-2019 13:53, Maxime Ripard wrote:
> > Hi Hans,
> >
> > Thanks for this series (and thanks for bouncing the mails too).
> >
> > All the previous patches are
> > Acked-by: Maxime Ripard
>
> Thank you for the review.
>
> > On Sun, Nov 10, 2019 at 04:40:58PM +0100, Hans de Goede wrote:
> > > Sometimes we want to override a connector's panel_orientation from the
> > > kernel commandline. Either for testing and for special cases, e.g. a kiosk
> > > like setup which uses a TV mounted in portrait mode.
> > >
> > > Users can already specify a "rotate" option through a video= kernel
> > > cmdline
> > > option. But that only supports 0/180 degrees (see drm_client_modeset TODO)
> > > and only works for in kernel modeset clients, not for userspace kms users.
> > >
> > > The "panel-orientation" connector property OTOH does support 90/270
> > > degrees
> > > as it leaves dealing with the rotation up to userspace and this does work
> > > for userspace kms clients (at least those which support this property).
> > >
> > > BugLink:
> > > https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
> > > Signed-off-by: Hans de Goede
> > > ---
> > > Documentation/fb/modedb.rst | 3 ++
> > > drivers/gpu/drm/drm_modes.c | 32 +++
> > > .../gpu/drm/selftests/drm_cmdline_selftests.h | 1 +
> > > .../drm/selftests/test-drm_cmdline_parser.c | 22 +
> > > include/drm/drm_connector.h | 8 +
> > > 5 files changed, 66 insertions(+)
> > >
> > > diff --git a/Documentation/fb/modedb.rst b/Documentation/fb/modedb.rst
> > > index 9c4e3fd39e6d..624d08fd2856 100644
> > > --- a/Documentation/fb/modedb.rst
> > > +++ b/Documentation/fb/modedb.rst
> > > @@ -65,6 +65,9 @@ Valid options are::
> > > - reflect_y (boolean): Perform an axial symmetry on the Y axis
> > > - rotate (integer): Rotate the initial framebuffer by x
> > > degrees. Valid values are 0, 90, 180 and 270.
> > > + - panel_orientation, one of "normal", "upside_down", "left_side_up", or
> > > +"right_side_up". For KMS drivers only, this sets the "panel
> > > orientation"
> > > +property on the kms connector as hint for kms users.
> >
> > Even though the semantic is a bit different, I think we should remain
> > consistent and have the same argument than for rotate (ie, steps in
> > clockwise rotation in steps of 90 degrees).
>
> Well the kernel kms defines for rotation also talk about 90 / 180 / 270":
>
> #define DRM_MODE_ROTATE_0 (1<<0)
> #define DRM_MODE_ROTATE_90 (1<<1)
> #define DRM_MODE_ROTATE_180 (1<<2)
> #define DRM_MODE_ROTATE_270 (1<<3)
>
> Where as the panel orientation uses strings like right_side_up, which means
> that the side of the panel which normally is the right side of the panel
> is now pointing up as the panel is mounted 90 degrees rotated with its
> original right side now pointing up. This IMHO is much clearer then
> 90 / 270 degrees which are ambiguous and perhaps more importantly this
> matches the kernel defines for panel-orientation and matches the
> String values enumerated by the enum type panel-orientation connector
> property:
>
> static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
> { DRM_MODE_PANEL_ORIENTATION_NORMAL,"Normal"},
> { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down" },
> { DRM_MODE_PANEL_ORIENTATION_LEFT_UP, "Left Side Up" },
> { DRM_MODE_PANEL_ORIENTATION_RIGHT_UP, "Right Side Up" },
> };
>
> So I would prefer to stick to the strings.
Ok, that works for me then
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/4] dt-bindings: display: add IMX MIPI DSI host controller doc
On Wed, 6 Nov 2019 at 16:31, Adrian Ratiu wrote: > > Reviewed-by: Neil Armstrong > Reviewed-by: Emil Velikov Please drop this one. I'm not that experienced in DT to provide meaningful review. Actually, I've just noticed that respective maintainers/lists are not CC'd on the series. Please use the get_maintainer.pl script got get the correct info. Personally, I read through the output, adding only relevant people as CC in the commit message itself. In particular, I don't think adding the "maintainer: DRM DRIVER" or the "ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" are required. On the other hand "DRM DRIVERS FOR FREESCALE IMX" and "OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" seems pretty accurate for what you're doing here. HTH Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/ast: Replace drm_get_pci_device() and drm_put_dev()
Both functions are deprecated. Open-code them them in preparation
of removing struct drm_driver.{load,unload}.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_drv.c | 31 +--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index d763da6f0834..78c90a3c903b 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -86,9 +86,35 @@ static void ast_kick_out_firmware_fb(struct pci_dev *pdev)
static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
+ struct drm_device *dev;
+ int ret;
+
ast_kick_out_firmware_fb(pdev);
- return drm_get_pci_dev(pdev, ent, );
+ ret = pci_enable_device(pdev);
+ if (ret)
+ return ret;
+
+ dev = drm_dev_alloc(, >dev);
+ if (IS_ERR(dev)) {
+ ret = PTR_ERR(dev);
+ goto err_pci_disable_device;
+ }
+
+ dev->pdev = pdev;
+ pci_set_drvdata(pdev, dev);
+
+ ret = drm_dev_register(dev, ent->driver_data);
+ if (ret)
+ goto err_drm_dev_put;
+
+ return 0;
+
+err_drm_dev_put:
+ drm_dev_put(dev);
+err_pci_disable_device:
+ pci_disable_device(pdev);
+ return ret;
}
static void
@@ -96,7 +122,8 @@ ast_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- drm_put_dev(dev);
+ drm_dev_unregister(dev);
+ drm_dev_put(dev);
}
static int ast_drm_freeze(struct drm_device *dev)
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/ast: Call struct drm_driver.{load, unload} before registering device
Both callbacks are deprecated. Remove them and call functions explicitly.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ast/ast_drv.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 78c90a3c903b..9da26750a22d 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -104,17 +104,24 @@ static int ast_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
dev->pdev = pdev;
pci_set_drvdata(pdev, dev);
- ret = drm_dev_register(dev, ent->driver_data);
+ ret = ast_driver_load(dev, ent->driver_data);
if (ret)
goto err_drm_dev_put;
+ ret = drm_dev_register(dev, ent->driver_data);
+ if (ret)
+ goto err_ast_driver_unload;
+
return 0;
+err_ast_driver_unload:
+ ast_driver_unload(dev);
err_drm_dev_put:
drm_dev_put(dev);
err_pci_disable_device:
pci_disable_device(pdev);
return ret;
+
}
static void
@@ -123,6 +130,7 @@ ast_pci_remove(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
drm_dev_unregister(dev);
+ ast_driver_unload(dev);
drm_dev_put(dev);
}
@@ -228,9 +236,6 @@ static struct drm_driver driver = {
DRIVER_GEM |
DRIVER_MODESET,
- .load = ast_driver_load,
- .unload = ast_driver_unload,
-
.fops = _fops,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm/ast: Remove load/unload callbacks
This patchset removes the load/unload callback functions from ast. Tested
on AST 2100 HW.
Thomas Zimmermann (2):
drm/ast: Replace drm_get_pci_device() and drm_put_dev()
drm/ast: Call struct drm_driver.{load,unload} before registering
device
drivers/gpu/drm/ast/ast_drv.c | 42 ++-
1 file changed, 37 insertions(+), 5 deletions(-)
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/12] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
Hi,
On 12-11-2019 14:47, Daniel Vetter wrote:
On Tue, Nov 12, 2019 at 2:39 PM Hans de Goede wrote:
Hi,
On 12-11-2019 14:32, Daniel Vetter wrote:
On Tue, Nov 12, 2019 at 11:43 AM Hans de Goede wrote:
Hi,
On 12-11-2019 10:47, Daniel Vetter wrote:
On Sun, Nov 10, 2019 at 04:41:01PM +0100, Hans de Goede wrote:
If the new video=... panel_orientation option is set for a connector, honor
it and setup a matching "panel orientation" property on the connector.
BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
Signed-off-by: Hans de Goede
Don't we need one more patch here to create the panel orientation property
if the driver doesn't do it? Otherwise this won't happen, and userspace
can't look at it (only the internal fbdev stuff, which has it's own
limitations wrt rotation).
That is what patch 11/12 is for, that patch renames
drm_connector_init_panel_orientation
to drm_set_panel_orientation and makes it both set, init and attach the property
(unless called with DRM_MODE_PANEL_ORIENTATION_UNKNOWN in which case it is a
no-op).
Patch 11/12 also makes drm_set_panel_orientation do the set only once, which is
why
the orientation to set is now passed instead of stored directly in the
connector,
so that a second set call (panel_orientation of the connector already !=
DRM_MODE_PANEL_ORIENTATION_UNKNOWN) can be skipped, so that the cmdline
overrides
driver detecion code for this.
Oh, that's what I get for not reading the entire series ... Only risk
with that is that drivers set this property after driver loading is
done - we can't attach/create properties after drm_dev_register. But
I've added the corresponding WARN_ON to these, so we should be all
fine I think. So looks all good to me.
Can I take that as your Acked-by for this patch and perhaps also for 11/12 ?
Uh I didn't really read the details, ack feels a bit thin to land this ...
Ok, I will wait a bit for others to review this then. Note that Maxime has
reviewed patches 1-9, with one small remark on patch 9 which I'm still awaiting
an answer for. So most of the cmdline parsing stuff has been reviewed and
if you are ok with the non cmdline parsing changes...
Also what about your remarks to 10/12? I'm happy to do a v2 with a memset,
as said my main reason for dropping the specified=false in the early path
is that we never init bpp_specified or refresh_specified explicitly to false
I'm all for being explicit about that, but then lets just zero out the entire
passed in drm_cmdline_mode struct.
Hm yeah, clearing it all might be a good idea.
Ok I will submit a v2 with this change.
Regards,
Hans
---
drivers/gpu/drm/drm_connector.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 40a985c411a6..591914f01cd4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct
drm_connector *connector)
connector->force = mode->force;
}
+if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
+DRM_INFO("setting connector %s panel_orientation to %d\n",
+ connector->name, mode->panel_orientation);
+drm_connector_set_panel_orientation(connector,
+mode->panel_orientation);
+}
+
DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
connector->name, mode->name,
mode->xres, mode->yres,
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/3] drm/udl: Replace fbdev code with generic emulation
Am 13.11.19 um 16:41 schrieb Thomas Zimmermann: >> Either way you choose: > > Glad you like it :) > more like this >> >> Btw, nice diffstat! Glad you like it :) >> >>> if (ret) >>> goto err; >>> >>> @@ -367,6 +368,4 @@ void udl_fini(struct drm_device *dev) >>> >>> if (udl->urbs.count) >>> udl_free_urb_list(dev); >>> - >>> - udl_fbdev_cleanup(dev); >>> } >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/4] drm: imx: Add i.MX 6 MIPI DSI host driver
On Wed, 6 Nov 2019 at 16:31, Adrian Ratiu wrote:
>
> This adds support for the Synopsis DesignWare MIPI DSI v1.01 host
> controller which is embedded in i.MX 6 SoCs.
>
> Based on following patches, but updated/extended to work with existing
> support found in the kernel:
>
> - drm: imx: Support Synopsys DesignWare MIPI DSI host controller
> Signed-off-by: Liu Ying
>
> - ARM: dtsi: imx6qdl: Add support for MIPI DSI host controller
> Signed-off-by: Liu Ying
>
> Reviewed-by: Neil Armstrong
> Reviewed-by: Emil Velikov
With the const nitpick below, the patch is:
Reviewed-by: Emil Velikov
Aside, for the future consider having a change log in the patch itself.
What I tend to do is:
- v2: Keep DW version specifics in dw-mipi-dsi.c (Emil)
> +static struct dw_mipi_dsi_variant dw_mipi_dsi_v101_layout = {
Nit: make this a const.
> + .cfg_dpi_vid = REG_FIELD(DSI_DPI_CFG, 0, 1),
> + .cfg_dpi_color_coding = REG_FIELD(DSI_DPI_CFG, 2, 4),
> + .cfg_dpi_18loosely_en = REG_FIELD(DSI_DPI_CFG, 10, 10),
> + .cfg_dpi_vsync_active_low = REG_FIELD(DSI_DPI_CFG, 6, 6),
> + .cfg_dpi_hsync_active_low = REG_FIELD(DSI_DPI_CFG, 7, 7),
> + .cfg_cmd_mode_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 0,
> 0),
> + .cfg_cmd_mode_all_lp_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 1,
> 12),
> + .cfg_cmd_mode_ack_rqst_en = REG_FIELD(DSI_CMD_MODE_CFG_V101, 13,
> 13),
> + .cfg_cmd_pkt_status = REG_FIELD(DSI_CMD_PKT_STATUS_V101, 0,
> 14),
> + .cfg_vid_mode_en = REG_FIELD(DSI_VID_MODE_CFG_V101, 0,
> 0),
> + .cfg_vid_mode_type =REG_FIELD(DSI_VID_MODE_CFG_V101, 1,
> 2),
> + .cfg_vid_mode_low_power = REG_FIELD(DSI_VID_MODE_CFG_V101, 3,
> 8),
> + .cfg_vid_pkt_size = REG_FIELD(DSI_VID_PKT_CFG, 0, 10),
> + .cfg_vid_hsa_time = REG_FIELD(DSI_TMR_LINE_CFG, 0, 8),
> + .cfg_vid_hbp_time = REG_FIELD(DSI_TMR_LINE_CFG, 9, 17),
> + .cfg_vid_hline_time = REG_FIELD(DSI_TMR_LINE_CFG, 18, 31),
> + .cfg_vid_vsa_time = REG_FIELD(DSI_VTIMING_CFG, 0, 3),
> + .cfg_vid_vbp_time = REG_FIELD(DSI_VTIMING_CFG, 4, 9),
> + .cfg_vid_vfp_time = REG_FIELD(DSI_VTIMING_CFG, 10, 15),
> + .cfg_vid_vactive_time = REG_FIELD(DSI_VTIMING_CFG, 16, 26),
> + .cfg_phy_txrequestclkhs = REG_FIELD(DSI_PHY_IF_CTRL, 0, 0),
> + .cfg_phy_bta_time = REG_FIELD(DSI_PHY_TMR_CFG_V101, 0,
> 11),
> + .cfg_phy_lp2hs_time = REG_FIELD(DSI_PHY_TMR_CFG_V101, 12,
> 19),
> + .cfg_phy_hs2lp_time = REG_FIELD(DSI_PHY_TMR_CFG_V101, 20,
> 27),
> + .cfg_phy_testclr = REG_FIELD(DSI_PHY_TST_CTRL0_V101, 0,
> 0),
> + .cfg_phy_unshutdownz = REG_FIELD(DSI_PHY_RSTZ_V101, 0, 0),
> + .cfg_phy_unrstz = REG_FIELD(DSI_PHY_RSTZ_V101, 1, 1),
> + .cfg_phy_enableclk =REG_FIELD(DSI_PHY_RSTZ_V101, 2, 2),
> + .cfg_phy_nlanes = REG_FIELD(DSI_PHY_IF_CFG_V101, 0, 1),
> + .cfg_phy_stop_wait_time = REG_FIELD(DSI_PHY_IF_CFG_V101, 2, 9),
> + .cfg_phy_status = REG_FIELD(DSI_PHY_STATUS_V101, 0, 0),
> + .cfg_pckhdl_cfg = REG_FIELD(DSI_PCKHDL_CFG_V101, 0, 4),
> + .cfg_hstx_timeout_counter = REG_FIELD(DSI_TO_CNT_CFG_V101, 0, 15),
> + .cfg_lprx_timeout_counter = REG_FIELD(DSI_TO_CNT_CFG_V101, 16,
> 31),
> + .cfg_int_stat0 =REG_FIELD(DSI_ERROR_ST0_V101, 0, 20),
> + .cfg_int_stat1 =REG_FIELD(DSI_ERROR_ST1_V101, 0, 17),
> + .cfg_int_mask0 =REG_FIELD(DSI_ERROR_MSK0_V101, 0, 20),
> + .cfg_int_mask1 =REG_FIELD(DSI_ERROR_MSK1_V101, 0, 17),
> + .cfg_gen_hdr = REG_FIELD(DSI_GEN_HDR_V101, 0, 31),
> + .cfg_gen_payload = REG_FIELD(DSI_GEN_PLD_DATA_V101, 0,
> 31),
> +};
if we start getting a lot of these, one way to keep things brief is to
reuse the GEN._FEATURES approach in gpu/drm/i915/i915_pci.c
Namely:
#define 100_FEATURES \
.foo = ... \
.bar = ...
v100_layout = {
100_FEATURES,
};
... v101_layout = {
100_FEATURES,
// extra 101 changes
.foo = ...101, \
.bar = ...101
};
But that for another day.
HTH
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 1/3] drm/udl: Replace fbdev code with generic emulation
Hi Am 13.11.19 um 16:26 schrieb Noralf Trønnes: > > > Den 13.11.2019 12.52, skrev Thomas Zimmermann: >> The udl driver can use the generic fbdev implementation. Convert it. >> >> v4: >> * hardcode console bpp to 16 >> v3: >> * remove module parameter fb_bpp in favor of fbdev's video >> * call drm_fbdev_generic_setup() directly; remove udl_fbdev_init() >> * use default for struct drm_mode_config_funcs.output_poll_changed >> * use default for struct drm_driver.lastclose >> >> Signed-off-by: Thomas Zimmermann >> --- > >> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c >> index 4e854e017390..3be0c0cec49e 100644 >> --- a/drivers/gpu/drm/udl/udl_main.c >> +++ b/drivers/gpu/drm/udl/udl_main.c >> @@ -9,6 +9,7 @@ >> */ >> >> #include >> +#include >> #include >> #include >> >> @@ -338,7 +339,7 @@ int udl_init(struct udl_device *udl) >> if (ret) >> goto err; >> >> -ret = udl_fbdev_init(dev); >> +ret = drm_fbdev_generic_setup(dev, 16); > > I suggest you put this after drm_dev_register() in _probe() since fbdev > is a client, a user of the driver, not part of it as such. Good point. I'll change it. > > Either way you choose: Glad you like it :) > > Reviewed-by: Noralf Trønnes Thanks for the review. Best regards Thomas > > Btw, nice diffstat! > >> if (ret) >> goto err; >> >> @@ -367,6 +368,4 @@ void udl_fini(struct drm_device *dev) >> >> if (udl->urbs.count) >> udl_free_urb_list(dev); >> - >> -udl_fbdev_cleanup(dev); >> } > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/3] fbdev: Unexport unlink_framebuffer()
Den 13.11.2019 12.52, skrev Thomas Zimmermann: > There are no external callers of unlink_framebuffer() left. Make the > function an internal interface. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/3] drm/fb-helper: Remove drm_fb_helper_unlink_fbi()
Den 13.11.2019 12.52, skrev Thomas Zimmermann: > There are no callers of drm_fb_helper_unlink_fbi() left. Remove the > function. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Noralf Trønnes ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
