[Bug 111415] BUG: kernel NULL pointer dereference - supervisor read access in kernel mode
https://bugs.freedesktop.org/show_bug.cgi?id=111415 Bug ID: 111415 Summary: BUG: kernel NULL pointer dereference - supervisor read access in kernel mode Product: DRI Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: tseew...@gmail.com Created attachment 145088 --> https://bugs.freedesktop.org/attachment.cgi?id=145088=edit dmesg of freeze Distribution: Fedora 30 Kernel: 5.2.8 Mesa: 19.1.4 GPU: RX 560 Problem: While browsing the internet using Chromium my entire machine froze, it was completely unresponsive to input but audio continued to play. Snippet of BUG: [173497.820810] BUG: kernel NULL pointer dereference, address: 02b4 [173497.820815] #PF: supervisor read access in kernel mode [173497.820817] #PF: error_code(0x) - not-present page [173497.820818] PGD 0 P4D 0 [173497.820822] Oops: [#1] SMP PTI [173497.820825] CPU: 2 PID: 21197 Comm: kworker/u8:2 Not tainted 5.2.8-200.fc30.x86_64 #1 [173497.820826] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77 Extreme4, BIOS P2.90 07/11/2013 [173497.820839] Workqueue: events_unbound commit_work [drm_kms_helper] [173497.820953] RIP: 0010:dc_stream_log+0x6/0xb0 [amdgpu] See attached dmesg for full message. Let me know if you need any additional information. -- 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 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 Andrew Shark changed: What|Removed |Added Resolution|WORKSFORME |--- Status|VERIFIED|REOPENED -- 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 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 Andrew Shark changed: What|Removed |Added Attachment #144555|0 |1 is obsolete|| --- Comment #29 from Andrew Shark --- Created attachment 145087 --> https://bugs.freedesktop.org/attachment.cgi?id=145087=edit Script to modify packages to be able to use with ubuntu 19.04 Updated script to work with Ubuntu 19.04 and driver version 19.30-855429 Set status to REOPENED, because the problem is still actual. -- 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 111122] 2500U: Graphics corruption on kernel 5.2
https://bugs.freedesktop.org/show_bug.cgi?id=22 --- Comment #20 from Brian Schott --- The following applies to the graphics corruption seen in Dolphin: ea5b7de138bb7e9a4e7e4f0c39c4ceb16acae923 is the first bad commit commit ea5b7de138bb7e9a4e7e4f0c39c4ceb16acae923 Author: Pierre-Eric Pelloux-Prayer Date: Wed Jul 3 19:27:12 2019 +0200 radeonsi: make gl_SampleMaskIn = 0x1 when MSAA is disabled gl_SampleMaskIn is 1 when R_028BE0_PA_SC_AA_CONFIG is 0, so this commit rework the conditions controlling this register. Before it was set if the sctx->framebuffer had a sample count > 1. Now we still require this condition, but we also need either: - GL_MULTISAMPLE to be enabled - to be executing an operation that doesn't depends on GL state using u_blitter. This fixes the arb_sample_shading/sample_mask piglit tests on radeonsi. Signed-off-by: Marek Olšák src/gallium/drivers/radeonsi/si_state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 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 AUTOSEL 5.2 51/59] drm/exynos: fix missing decrement of retry counter
On Wed, Aug 07, 2019 at 08:49:52AM +, David Laight wrote: From: Sasha Levin Sent: 06 August 2019 22:33 From: Colin Ian King [ Upstream commit 1bbbab097a05276e312dd2462791d32b21ceb1ee ] Currently the retry counter is not being decremented, leading to a potential infinite spin if the scalar_reads don't change state. Addresses-Coverity: ("Infinite loop") Fixes: 280e54c9f614 ("drm/exynos: scaler: Reset hardware before starting the operation") Signed-off-by: Colin Ian King Signed-off-by: Inki Dae Signed-off-by: Sasha Levin --- drivers/gpu/drm/exynos/exynos_drm_scaler.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c b/drivers/gpu/drm/exynos/exynos_drm_scaler.c index ec9c1b7d31033..8989f8af716b7 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c +++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c @@ -94,12 +94,12 @@ static inline int scaler_reset(struct scaler_context *scaler) scaler_write(SCALER_CFG_SOFT_RESET, SCALER_CFG); do { cpu_relax(); - } while (retry > 1 && + } while (--retry > 1 && scaler_read(SCALER_CFG) & SCALER_CFG_SOFT_RESET); do { cpu_relax(); scaler_write(1, SCALER_INT_EN); - } while (retry > 0 && scaler_read(SCALER_INT_EN) != 1); + } while (--retry > 0 && scaler_read(SCALER_INT_EN) != 1); return retry ? 0 : -EIO; If the first loop hits the retry limit the second loop won't be right and the final return value will be 0. This looks like an upstream problem as well, no? -- Thanks, Sasha ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH AUTOSEL 5.2 37/59] drm/vgem: fix cache synchronization on arm/arm64
On Tue, Aug 06, 2019 at 03:45:48PM -0700, Rob Clark wrote: please don't queue this one for stable branches.. it was causing problems in intel CI Now dropped, thank you. -- Thanks, Sasha
[Bug 203879] hard freeze on high single threaded load (AMD Ryzen 7 2700X CPU)
https://bugzilla.kernel.org/show_bug.cgi?id=203879 --- Comment #9 from Sam Bazley (sambaz...@fastmail.com) --- I've realised that I am actually being affected by this bug: https://bugzilla.kernel.org/show_bug.cgi?id=204181 Please disregard my previous comments. -- 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
[PATCH] radeon: add option so DVI always respect HPD over DDC
From: Dave Airlie Purelink FX-D120 (DVI over fibre extendeders) drive the HPD line low on the GPU side when the monitor side device is unplugged or loses the connection. However the GPU side device seems to cache EDID in this case. Per DVI spec the HPD line must be driven in order for EDID to be done, but we've met enough broken devices (mainly VGA->DVI convertors) that do the wrong thing with HPD that we ignore it if a DDC probe succeeds. This patch adds an option to the radeon driver to always respect HPD on DVI connectors such that if the HPD line isn't driven then EDID isn't probed. Signed-off-by: Dave Airlie --- drivers/gpu/drm/radeon/radeon.h| 1 + drivers/gpu/drm/radeon/radeon_connectors.c | 7 +++ drivers/gpu/drm/radeon/radeon_drv.c| 4 3 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 32808e50be12..d572e8ded9b9 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -117,6 +117,7 @@ extern int radeon_uvd; extern int radeon_vce; extern int radeon_si_support; extern int radeon_cik_support; +extern int radeon_respect_hpd; /* * Copy from radeon_drv.h so we don't have to include both and have conflicting diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index c60d1a44d22a..e9b3924df06e 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -1265,6 +1265,13 @@ radeon_dvi_detect(struct drm_connector *connector, bool force) goto exit; } + if (radeon_respect_hpd && radeon_connector->hpd.hpd != RADEON_HPD_NONE) { + if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd)) { + ret = connector_status_disconnected; + goto exit; + } + } + if (radeon_connector->ddc_bus) { dret = radeon_ddc_probe(radeon_connector, false); diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index a6cbe11f79c6..556ae381ea86 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -207,6 +207,7 @@ int radeon_auxch = -1; int radeon_mst = 0; int radeon_uvd = 1; int radeon_vce = 1; +int radeon_respect_hpd = 0; MODULE_PARM_DESC(no_wb, "Disable AGP writeback for scratch registers"); module_param_named(no_wb, radeon_no_wb, int, 0444); @@ -312,6 +313,9 @@ int radeon_cik_support = 1; MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled (default), 0 = disabled)"); module_param_named(cik_support, radeon_cik_support, int, 0444); +MODULE_PARM_DESC(respect_hpd, "For DVI always believe HPD"); +module_param_named(respect_hpd, radeon_respect_hpd, int, 0644); + static struct pci_device_id pciidlist[] = { radeon_PCI_IDS }; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #28 from Sebastian --- There is a new Version of the Radeon Software (ver 19.30 from Aug 12.) https://www.amd.com/en/support/kb/release-notes/rn-amdgpu-unified-linux Does the script work for the new version? -- 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 110886] After S3 resume, kernel: [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:57:crtc-0] flip_done timed out
https://bugs.freedesktop.org/show_bug.cgi?id=110886 --- Comment #15 from Samantha McVey --- I have uploaded my dmesg log and xorg log from amd-staging-drm-next -- 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 110886] After S3 resume, kernel: [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:57:crtc-0] flip_done timed out
https://bugs.freedesktop.org/show_bug.cgi?id=110886 --- Comment #13 from Samantha McVey --- Created attachment 145085 --> https://bugs.freedesktop.org/attachment.cgi?id=145085=edit amd-staging-drm-net dmesg log -- 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 110886] After S3 resume, kernel: [drm:drm_atomic_helper_wait_for_flip_done [drm_kms_helper]] *ERROR* [CRTC:57:crtc-0] flip_done timed out
https://bugs.freedesktop.org/show_bug.cgi?id=110886 --- Comment #14 from Samantha McVey --- Created attachment 145086 --> https://bugs.freedesktop.org/attachment.cgi?id=145086=edit amd-staging-drm-next xorg log -- 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 111414] [REGRESSION] [BISECTED] Segmentation fault in si_bind_blend_state after removal of the blend state NULL check
https://bugs.freedesktop.org/show_bug.cgi?id=111414 Bug ID: 111414 Summary: [REGRESSION] [BISECTED] Segmentation fault in si_bind_blend_state after removal of the blend state NULL check Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: edmondo.tommas...@gmail.com QA Contact: dri-devel@lists.freedesktop.org Created attachment 145084 --> https://bugs.freedesktop.org/attachment.cgi?id=145084=edit Backtrace vlc using vdpau received signal SIGSEGV after the removal of the blend state NULL check. Bisected to: https://gitlab.freedesktop.org/mesa/mesa/commit/b758eed9c373db14a5acc04d9522ec9d74e51f1b commit b758eed9c373db14a5acc04d9522ec9d74e51f1b (HEAD, refs/bisect/bad) Author: Marek Olšák Date: Tue Jul 30 17:43:41 2019 -0400 radeonsi: make sure that blend state != NULL and remove all NULL checking Reviewed-by: Pierre-Eric Pelloux-Prayer The backtrace shows old_blend = 0x0 where the new code doesn't check old_blend anymore: + if (old_blend->cb_target_mask != blend->cb_target_mask || Full backtrace attached, here an extract: 0x7fff60e6497b in si_bind_blend_state (ctx=0x7fff58468f20, state=) at ../mesa-/src/gallium/drivers/radeonsi/si_state.c:686 686 ../mesa-/src/gallium/drivers/radeonsi/si_state.c: No such file or directory. (gdb) bt full #0 0x7fff60e6497b in si_bind_blend_state (ctx=0x7fff58468f20, state=) at ../mesa-/src/gallium/drivers/radeonsi/si_state.c:686 sctx = 0x7fff58468f20 old_blend = 0x0 blend = 0x7fff584f2220 #1 0x7fff60cea216 in draw_layers (dirty=0x7fff584099b8, s=0x7fff584fbf60, c=0x7fff58409788) at ../mesa-/src/gallium/auxiliary/vl/vl_compositor_gfx.c:662 layer = 0x7fff584fbf98 samplers = 0x7fff584fbfe8 num_sampler_views = 1 blend = vb_index = 0 i = 0 vb_index = i = layer = samplers = num_sampler_views = blend = drawn = #2 vl_compositor_gfx_render (s=s@entry=0x7fff584fbf60, c=c@entry=0x7fff58409788, dst_surface=dst_surface@entry=0x7fff58530980, dirty_area=0x7fff584099b8, dirty_area@entry=0x1, clear_dirty=) at ../mesa-/src/gallium/auxiliary/vl/vl_compositor_gfx.c:725 No locals. #3 0x7fff60ce462c in vl_compositor_render (clear_dirty=true, dirty_area=0x1, dst_surface=0x7fff58530980, c=0x7fff58409788, s=0x7fff584fbf60) at ../mesa-/src/gallium/auxiliary/vl/vl_compositor.c:755 No locals. #4 vl_compositor_render (s=s@entry=0x7fff584fbf60, c=c@entry=0x7fff58409788, dst_surface=dst_surface@entry=0x7fff5866b060, dirty_area=dirty_area@entry=0x7fff584099b8, clear_dirty=clear_dirty@entry=true) at ../mesa-/src/gallium/auxiliary/vl/vl_compositor.c:744 No locals. #5 0x7fff60cda6ae in vlVdpPresentationQueueDisplay (presentation_queue=, surface=4, clip_width=, clip_height=0, earliest_presentation_time=36232721127000) at ../mesa-/src/gallium/state_trackers/vdpau/presentation.c:262 dump_window = -1 pq = 0x7fff584fbf50 surf = 0x7fff585043d0 pipe = 0x7fff58468f20 tex = 0x7fff58530980 surf_templ = {reference = {count = 0}, format = PIPE_FORMAT_B8G8R8X8_UNORM, writable = 0, texture = 0x0, context = 0x0, width = 0, height = 0, nr_samples = 0, u = {tex = {level = 0, first_layer = 0, last_layer = 0}, buf = { first_element = 0, last_element = 0}}} surf_draw = 0x7fff5866b060 src_rect = {x0 = 0, x1 = 1658, y0 = 0, y1 = 933} dst_clip = {x0 = 0, x1 = 1658, y0 = 0, y1 = 933} dirty_area = 0x7fff584099b8 compositor = 0x7fff58409788 cstate = 0x7fff584fbf60 vscreen = 0x7fff58409900 -- 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] drm/arm: drop use of drmP.h
Hi Sidong On Sat, Aug 17, 2019 at 08:41:15AM +0100, Sidong Yang wrote: > Drop use of deprecated drmP.h header file. > Remove drmP.h includes and add some include headers for function or > struct that used in code. Thanks for your patch. We already have a similiar patch in drm-misc-next, that drop the use of drmP.h from arm so this patch is obsoleted. But keep up the spirit and send us other good stuff. Sam
Re: [PATCH 3/5] mm, notifier: Catch sleeping/blocking for !blockable
On Sat, Aug 17, 2019 at 5:26 PM Jason Gunthorpe wrote: > > On Thu, Aug 15, 2019 at 09:02:49AM +0200, Daniel Vetter wrote: > > On Wed, Aug 14, 2019 at 09:00:29PM -0300, Jason Gunthorpe wrote: > > > On Wed, Aug 14, 2019 at 10:20:25PM +0200, Daniel Vetter wrote: > > > > We need to make sure implementations don't cheat and don't have a > > > > possible schedule/blocking point deeply burried where review can't > > > > catch it. > > > > > > > > I'm not sure whether this is the best way to make sure all the > > > > might_sleep() callsites trigger, and it's a bit ugly in the code flow. > > > > But it gets the job done. > > > > > > > > Inspired by an i915 patch series which did exactly that, because the > > > > rules haven't been entirely clear to us. > > > > > > I thought lockdep already was able to detect: > > > > > > spin_lock() > > > might_sleep(); > > > spin_unlock() > > > > > > Am I mistaken? If yes, couldn't this patch just inject a dummy lockdep > > > spinlock? > > > > Hm ... assuming I didn't get lost in the maze I think might_sleep (well > > ___might_sleep) doesn't do any lockdep checking at all. And we want > > might_sleep, since that catches a lot more than lockdep. > > Don't know how it works, but it sure looks like it does: > > This: > spin_lock(>uobjects_lock); > down_read(>hw_destroy_rwsem); > up_read(>hw_destroy_rwsem); > spin_unlock(>uobjects_lock); > > Causes: > > [ 33.324729] BUG: sleeping function called from invalid context at > kernel/locking/rwsem.c:1444 > [ 33.325599] in_atomic(): 1, irqs_disabled(): 0, pid: 247, name: ibv_devinfo > [ 33.326115] 3 locks held by ibv_devinfo/247: > [ 33.326556] #0: 9edf8379 (_dev->disassociate_srcu){}, > at: ib_uverbs_open+0xff/0x5f0 [ib_uverbs] > [ 33.327657] #1: 5e0eddf1 (_dev->lists_mutex){+.+.}, at: > ib_uverbs_open+0x16c/0x5f0 [ib_uverbs] > [ 33.328682] #2: 505f509e (&(>uobjects_lock)->rlock){+.+.}, > at: ib_uverbs_open+0x31a/0x5f0 [ib_uverbs] > > And this: > > spin_lock(>uobjects_lock); > might_sleep(); > spin_unlock(>uobjects_lock); > > Causes: > > [ 16.867211] BUG: sleeping function called from invalid context at > drivers/infiniband/core/uverbs_main.c:1095 > [ 16.867776] in_atomic(): 1, irqs_disabled(): 0, pid: 245, name: ibv_devinfo > [ 16.868098] 3 locks held by ibv_devinfo/245: > [ 16.868383] #0: 4c5954ff (_dev->disassociate_srcu){}, > at: ib_uverbs_open+0xf8/0x600 [ib_uverbs] > [ 16.868938] #1: 20a6fae2 (_dev->lists_mutex){+.+.}, at: > ib_uverbs_open+0x16c/0x600 [ib_uverbs] > [ 16.869568] #2: 036e6a97 (&(>uobjects_lock)->rlock){+.+.}, > at: ib_uverbs_open+0x317/0x600 [ib_uverbs] > > I think this is done in some very expensive way, so it probably only > works when lockdep is enabled.. This is the might_sleep debug infrastructure (both of them), not lockdep. Disable CONFIG_PROVE_LOCKING and you should still get these. -Daniel -- 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: [PATCH v2] dma-fence: Store the timestamp in the same union as the cb_list
Am 17.08.19 um 17:30 schrieb Chris Wilson: > The timestamp and the cb_list are mutually exclusive, the cb_list can > only be added to prior to being signaled (and once signaled we drain), > while the timestamp is only valid upon being signaled. Both the > timestamp and the cb_list are only valid while the fence is alive, and > as soon as no references are held can be replaced by the rcu_head. > > By reusing the union for the timestamp, we squeeze the base dma_fence > struct to 64 bytes on x86-64. > > v2: Sort the union chronologically > > Suggested-by: Christian König > Signed-off-by: Chris Wilson > Cc: Christian König I can't judge about the correctness of the vmw and Intel stuff, so only Acked-by: Christian König . > --- > drivers/dma-buf/dma-fence.c | 16 +++--- > drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 13 ++-- > drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 3 +++ > include/linux/dma-fence.h | 23 - > 4 files changed, 37 insertions(+), 18 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 8a6d0250285d..2c136aee3e79 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -129,6 +129,7 @@ EXPORT_SYMBOL(dma_fence_context_alloc); > int dma_fence_signal_locked(struct dma_fence *fence) > { > struct dma_fence_cb *cur, *tmp; > + struct list_head cb_list; > > lockdep_assert_held(fence->lock); > > @@ -136,16 +137,16 @@ int dma_fence_signal_locked(struct dma_fence *fence) > >flags))) > return -EINVAL; > > + /* Stash the cb_list before replacing it with the timestamp */ > + list_replace(>cb_list, _list); > + > fence->timestamp = ktime_get(); > set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > trace_dma_fence_signaled(fence); > > - if (!list_empty(>cb_list)) { > - list_for_each_entry_safe(cur, tmp, >cb_list, node) { > - INIT_LIST_HEAD(>node); > - cur->func(fence, cur); > - } > - INIT_LIST_HEAD(>cb_list); > + list_for_each_entry_safe(cur, tmp, _list, node) { > + INIT_LIST_HEAD(>node); > + cur->func(fence, cur); > } > > return 0; > @@ -231,7 +232,8 @@ void dma_fence_release(struct kref *kref) > > trace_dma_fence_destroy(fence); > > - if (WARN(!list_empty(>cb_list), > + if (WARN(!list_empty(>cb_list) && > + !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags), >"Fence %s:%s:%llx:%llx released with pending signals!\n", >fence->ops->get_driver_name(fence), >fence->ops->get_timeline_name(fence), > diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > index 2bc9c460e78d..09c68dda2098 100644 > --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > @@ -114,18 +114,18 @@ __dma_fence_signal__timestamp(struct dma_fence *fence, > ktime_t timestamp) > } > > static void > -__dma_fence_signal__notify(struct dma_fence *fence) > +__dma_fence_signal__notify(struct dma_fence *fence, > +const struct list_head *list) > { > struct dma_fence_cb *cur, *tmp; > > lockdep_assert_held(fence->lock); > lockdep_assert_irqs_disabled(); > > - list_for_each_entry_safe(cur, tmp, >cb_list, node) { > + list_for_each_entry_safe(cur, tmp, list, node) { > INIT_LIST_HEAD(>node); > cur->func(fence, cur); > } > - INIT_LIST_HEAD(>cb_list); > } > > void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) > @@ -187,11 +187,12 @@ void intel_engine_breadcrumbs_irq(struct > intel_engine_cs *engine) > list_for_each_safe(pos, next, ) { > struct i915_request *rq = > list_entry(pos, typeof(*rq), signal_link); > - > - __dma_fence_signal__timestamp(>fence, timestamp); > + struct list_head cb_list; > > spin_lock(>lock); > - __dma_fence_signal__notify(>fence); > + list_replace(>fence.cb_list, _list); > + __dma_fence_signal__timestamp(>fence, timestamp); > + __dma_fence_signal__notify(>fence, _list); > spin_unlock(>lock); > > i915_request_put(rq); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > index 434dfadb0e52..178a6cd1a06f 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > @@ -185,6 +185,9 @@ static long vmw_fence_wait(struct dma_fence *f, bool > intr, signed long timeout) > > spin_lock(f->lock); > > + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) > + goto out; > + > if (intr &&
Re: [PATCH 6/6] dma-fence: Store the timestamp in the same union as the cb_list
Am 17.08.19 um 17:27 schrieb Chris Wilson: > Quoting Koenig, Christian (2019-08-17 16:20:12) >> Am 17.08.19 um 16:47 schrieb Chris Wilson: >>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c >>> index 89d96e3e6df6..2c21115b1a37 100644 >>> --- a/drivers/dma-buf/dma-fence.c >>> +++ b/drivers/dma-buf/dma-fence.c >>> @@ -129,6 +129,7 @@ EXPORT_SYMBOL(dma_fence_context_alloc); >>>int dma_fence_signal_locked(struct dma_fence *fence) >>>{ >>>struct dma_fence_cb *cur, *tmp; >>> + struct list_head cb_list; >>> >>>lockdep_assert_held(fence->lock); >>> >>> @@ -136,16 +137,16 @@ int dma_fence_signal_locked(struct dma_fence *fence) >>> >flags))) >>>return -EINVAL; >>> >>> + /* Stash the cb_list before replacing it with the timestamp */ >>> + list_replace(>cb_list, _list); >> Stashing the timestamp instead is probably less bytes to modify. > My thinking was to pass the timestamp to the notify callbacks, we need > to stash the list and set the timestamp first. I don't see much of a reason for callbacks to use the timestamp, they could just call ktime_get() and would most likely get the same or at least a very close by value. > Nothing that I'm aware of uses the timestamp (just the sync file debug > which weston was considering using at one point)... So I guess we don't > care? But I would say we should do that as a separate step in case > someone does. Yeah, agree. Christian. > -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] dma-fence: Store the timestamp in the same union as the cb_list
The timestamp and the cb_list are mutually exclusive, the cb_list can only be added to prior to being signaled (and once signaled we drain), while the timestamp is only valid upon being signaled. Both the timestamp and the cb_list are only valid while the fence is alive, and as soon as no references are held can be replaced by the rcu_head. By reusing the union for the timestamp, we squeeze the base dma_fence struct to 64 bytes on x86-64. v2: Sort the union chronologically Suggested-by: Christian König Signed-off-by: Chris Wilson Cc: Christian König --- drivers/dma-buf/dma-fence.c | 16 +++--- drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 13 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 3 +++ include/linux/dma-fence.h | 23 - 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 8a6d0250285d..2c136aee3e79 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -129,6 +129,7 @@ EXPORT_SYMBOL(dma_fence_context_alloc); int dma_fence_signal_locked(struct dma_fence *fence) { struct dma_fence_cb *cur, *tmp; + struct list_head cb_list; lockdep_assert_held(fence->lock); @@ -136,16 +137,16 @@ int dma_fence_signal_locked(struct dma_fence *fence) >flags))) return -EINVAL; + /* Stash the cb_list before replacing it with the timestamp */ + list_replace(>cb_list, _list); + fence->timestamp = ktime_get(); set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); trace_dma_fence_signaled(fence); - if (!list_empty(>cb_list)) { - list_for_each_entry_safe(cur, tmp, >cb_list, node) { - INIT_LIST_HEAD(>node); - cur->func(fence, cur); - } - INIT_LIST_HEAD(>cb_list); + list_for_each_entry_safe(cur, tmp, _list, node) { + INIT_LIST_HEAD(>node); + cur->func(fence, cur); } return 0; @@ -231,7 +232,8 @@ void dma_fence_release(struct kref *kref) trace_dma_fence_destroy(fence); - if (WARN(!list_empty(>cb_list), + if (WARN(!list_empty(>cb_list) && +!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags), "Fence %s:%s:%llx:%llx released with pending signals!\n", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c index 2bc9c460e78d..09c68dda2098 100644 --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c @@ -114,18 +114,18 @@ __dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp) } static void -__dma_fence_signal__notify(struct dma_fence *fence) +__dma_fence_signal__notify(struct dma_fence *fence, + const struct list_head *list) { struct dma_fence_cb *cur, *tmp; lockdep_assert_held(fence->lock); lockdep_assert_irqs_disabled(); - list_for_each_entry_safe(cur, tmp, >cb_list, node) { + list_for_each_entry_safe(cur, tmp, list, node) { INIT_LIST_HEAD(>node); cur->func(fence, cur); } - INIT_LIST_HEAD(>cb_list); } void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) @@ -187,11 +187,12 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) list_for_each_safe(pos, next, ) { struct i915_request *rq = list_entry(pos, typeof(*rq), signal_link); - - __dma_fence_signal__timestamp(>fence, timestamp); + struct list_head cb_list; spin_lock(>lock); - __dma_fence_signal__notify(>fence); + list_replace(>fence.cb_list, _list); + __dma_fence_signal__timestamp(>fence, timestamp); + __dma_fence_signal__notify(>fence, _list); spin_unlock(>lock); i915_request_put(rq); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c index 434dfadb0e52..178a6cd1a06f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c @@ -185,6 +185,9 @@ static long vmw_fence_wait(struct dma_fence *f, bool intr, signed long timeout) spin_lock(f->lock); + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) + goto out; + if (intr && signal_pending(current)) { ret = -ERESTARTSYS; goto out; diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 2ce4d877d33e..8b4a5aaa6848 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -65,17 +65,30 @@ struct dma_fence_cb; struct dma_fence {
[Bug 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #17 from Matt Turner --- The build failure is in Clover, the OpenCL implementation. If the application that triggers the huge amount of RAM problem is not using OpenCL, disable OpenCL in meson configure and try to get past that. -- 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 v3] dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal
Am 17.08.19 um 17:23 schrieb Chris Wilson: > Currently dma_fence_signal() tries to avoid the spinlock and only takes > it if absolutely required to walk the callback list. However, to allow > for some users to surreptitiously insert lazy signal callbacks that > do not depend on enabling the signaling mechanism around every fence, > we always need to notify the callbacks on signaling. As such, we will > always need to take the spinlock and dma_fence_signal() effectively > becomes a clone of dma_fence_signal_locked(). > > v2: Update the test_and_set_bit() before entering the spinlock. > v3: Drop the test_[and_set]_bit() before the spinlock, it's a caller > error so expected to be very unlikely. > > Signed-off-by: Chris Wilson > Cc: Christian König > Cc: Daniel Vetter Reviewed-by: Christian König > --- > drivers/dma-buf/dma-fence.c | 44 ++--- > 1 file changed, 12 insertions(+), 32 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index ff0cd6eae766..8a6d0250285d 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -129,25 +129,16 @@ EXPORT_SYMBOL(dma_fence_context_alloc); > int dma_fence_signal_locked(struct dma_fence *fence) > { > struct dma_fence_cb *cur, *tmp; > - int ret = 0; > > lockdep_assert_held(fence->lock); > > - if (WARN_ON(!fence)) > + if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, > + >flags))) > return -EINVAL; > > - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) { > - ret = -EINVAL; > - > - /* > - * we might have raced with the unlocked dma_fence_signal, > - * still run through all callbacks > - */ > - } else { > - fence->timestamp = ktime_get(); > - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > - trace_dma_fence_signaled(fence); > - } > + fence->timestamp = ktime_get(); > + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > + trace_dma_fence_signaled(fence); > > if (!list_empty(>cb_list)) { > list_for_each_entry_safe(cur, tmp, >cb_list, node) { > @@ -156,7 +147,8 @@ int dma_fence_signal_locked(struct dma_fence *fence) > } > INIT_LIST_HEAD(>cb_list); > } > - return ret; > + > + return 0; > } > EXPORT_SYMBOL(dma_fence_signal_locked); > > @@ -176,28 +168,16 @@ EXPORT_SYMBOL(dma_fence_signal_locked); > int dma_fence_signal(struct dma_fence *fence) > { > unsigned long flags; > + int ret; > > if (!fence) > return -EINVAL; > > - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) > - return -EINVAL; > - > - fence->timestamp = ktime_get(); > - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > - trace_dma_fence_signaled(fence); > - > - if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) { > - struct dma_fence_cb *cur, *tmp; > + spin_lock_irqsave(fence->lock, flags); > + ret = dma_fence_signal_locked(fence); > + spin_unlock_irqrestore(fence->lock, flags); > > - spin_lock_irqsave(fence->lock, flags); > - list_for_each_entry_safe(cur, tmp, >cb_list, node) { > - list_del_init(>node); > - cur->func(fence, cur); > - } > - spin_unlock_irqrestore(fence->lock, flags); > - } > - return 0; > + return ret; > } > EXPORT_SYMBOL(dma_fence_signal); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/6] dma-fence: Store the timestamp in the same union as the cb_list
Quoting Koenig, Christian (2019-08-17 16:20:12) > Am 17.08.19 um 16:47 schrieb Chris Wilson: > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > > index 89d96e3e6df6..2c21115b1a37 100644 > > --- a/drivers/dma-buf/dma-fence.c > > +++ b/drivers/dma-buf/dma-fence.c > > @@ -129,6 +129,7 @@ EXPORT_SYMBOL(dma_fence_context_alloc); > > int dma_fence_signal_locked(struct dma_fence *fence) > > { > > struct dma_fence_cb *cur, *tmp; > > + struct list_head cb_list; > > > > lockdep_assert_held(fence->lock); > > > > @@ -136,16 +137,16 @@ int dma_fence_signal_locked(struct dma_fence *fence) > > >flags))) > > return -EINVAL; > > > > + /* Stash the cb_list before replacing it with the timestamp */ > > + list_replace(>cb_list, _list); > > Stashing the timestamp instead is probably less bytes to modify. My thinking was to pass the timestamp to the notify callbacks, we need to stash the list and set the timestamp first. Nothing that I'm aware of uses the timestamp (just the sync file debug which weston was considering using at one point)... So I guess we don't care? But I would say we should do that as a separate step in case someone does. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 204407] New: Bad page state in process Xorg
Vlastimil Babka wrote on 8/16/2019 5:47 AM: On 8/15/19 9:13 PM, Petr Vandrovec wrote: With iommu=off disks are visible, but USB keyboard (and other USB devices) does not work: I've been told iommu=soft might help. Thanks. I've rebuilt kernel without IOMMU. Unfortunately I was not able to reproduce original problem with latest kernel - neither with CMA nor without CMA. System seems stable as a rock. This is the change on which I've tried to reproduce it with your debugging patches: commit 41de59634046b19cd53a1983594a95135c656997 (HEAD -> master, origin/master, origin/HEAD) Merge: e22a97a2a85d 1ee1119d184b Author: Linus Torvalds Date: Wed Aug 14 15:29:53 2019 -0700 Merge tag 'Wimplicit-fallthrough-5.3-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux Petr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 hmm 00/11] Add mmu_notifier_get/put for managing mmu notifier registrations
On Tue, Aug 06, 2019 at 08:15:37PM -0300, Jason Gunthorpe wrote: > This series is already entangled with patches in the hmm & RDMA tree and > will require some git topic branches for the RDMA ODP stuff. I intend for > it to go through the hmm tree. > Jason Gunthorpe (11): > mm/mmu_notifiers: hoist do_mmu_notifier_register down_write to the > caller > mm/mmu_notifiers: do not speculatively allocate a mmu_notifier_mm > mm/mmu_notifiers: add a get/put scheme for the registration > misc/sgi-gru: use mmu_notifier_get/put for struct gru_mm_struct > hmm: use mmu_notifier_get/put for 'struct hmm' > drm/radeon: use mmu_notifier_get/put for struct radeon_mn > drm/amdkfd: fix a use after free race with mmu_notifer unregister > drm/amdkfd: use mmu_notifier_put Other than these patches: > RDMA/odp: use mmu_notifier_get/put for 'struct ib_ucontext_per_mm' > RDMA/odp: remove ib_ucontext from ib_umem > mm/mmu_notifiers: remove unregister_no_release This series has been applied. I will apply the ODP patches when the series they depend on is merged to the RDMA tree Any further acks/remarks I will annotate, thanks in advance Thanks to all reviewers, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: turn hmm migrate_vma upside down v3
On Fri, Aug 16, 2019 at 08:51:41AM +0200, Christoph Hellwig wrote: > Jason, > > are you going to look into picking this up? Unfortunately there is > a hole pile in this area still pending, including the kvmppc secure > memory driver from Bharata that depends on the work. Done, Lets see if Dan will comment on the pagemap part (looks straightforward to me), and then after we grab that I will declare hmm.git non-rebasing and Bharata can build his series upon it. As a reminder, please do not send hmm.git inside another pull request to Linus without making it very clear in that cover letter and Cc'ing me. Thanks. Regards, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 06/11] drm/bridge: analogix-anx78xx: add support for avdd33 regulator
On Thu, Aug 15, 2019 at 10:22:45AM +0200, Linus Walleij wrote: > On Thu, Aug 15, 2019 at 2:49 AM Brian Masney wrote: > > > Add support for the avdd33 regulator to the analogix-anx78xx driver. > > Note that the regulator is currently enabled during driver probe and > > disabled when the driver is removed. This is currently how the > > downstream MSM kernel sources do this. > > > > Let's not merge this upstream for the mean time until I get the external > > display fully working on the Nexus 5 and then I can submit proper > > support then that powers down this regulator in the power off function. > > > > Signed-off-by: Brian Masney > > > +static void anx78xx_disable_regulator_action(void *_data) > > +{ > > + struct anx78xx_platform_data *pdata = _data; > > + > > + regulator_disable(pdata->avdd33); > > +} > (...) > > + err = devm_add_action(dev, anx78xx_disable_regulator_action, > > + pdata); > > Clever idea. Good for initial support, probably later on it would > need to be reworked using runtime PM so it's not constantly > powered up. Yes, that's my plan. I suspect that I may have a regulator disabled somewhere so I was planning to leave this on all the time for the time being to match the downstream behavior until I get the hot plug detect GPIO working. > Reviewed-by: Linus Walleij Thanks, Brian ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v3 1/2] dt/bindings: display: Add optional property node for Mali DP500
Add optional property node 'arm,malidp-arqos-value' for the Mali DP500. This property describe the ARQoS levels of DP500's QoS signaling. Signed-off-by: Wen He --- change in v3: - correction the describe of the node Documentation/devicetree/bindings/display/arm,malidp.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/display/arm,malidp.txt b/Documentation/devicetree/bindings/display/arm,malidp.txt index 2f7870983ef1..1f711d32f235 100644 --- a/Documentation/devicetree/bindings/display/arm,malidp.txt +++ b/Documentation/devicetree/bindings/display/arm,malidp.txt @@ -37,6 +37,8 @@ Optional properties: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt) to be used for the framebuffer; if not present, the framebuffer may be located anywhere in memory. + - arm,malidp-arqos-high-level: phandle to describing the ARQoS levels of DP500's +QoS signaling. Example: @@ -54,6 +56,7 @@ Example: clocks = <>, <>, <>, <>; clock-names = "pxlclk", "mclk", "aclk", "pclk"; arm,malidp-output-port-lines = /bits/ 8 <8 8 8>; + arm,malidp-arqos-high-level = <>; port { dp0_output: endpoint { remote-endpoint = <_2_input>; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/10] mm: turn migrate_vma upside down
On Wed, Aug 14, 2019 at 09:59:19AM +0200, Christoph Hellwig wrote: > There isn't any good reason to pass callbacks to migrate_vma. Instead > we can just export the three steps done by this function to drivers and > let them sequence the operation without callbacks. This removes a lot > of boilerplate code as-is, and will allow the drivers to drastically > improve code flow and error handling further on. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Ralph Campbell > --- > Documentation/vm/hmm.rst | 55 +- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 122 +++-- > include/linux/migrate.h| 118 ++-- > mm/migrate.c | 244 +++-- > 4 files changed, 195 insertions(+), 344 deletions(-) The mechanical transformation looks OK, I squashed the below nits in, let me know if I got it wrong Reviewed-by: Jason Gunthorpe diff --git a/Documentation/vm/hmm.rst b/Documentation/vm/hmm.rst index 4f81c77059e305..0a5960beccf76d 100644 --- a/Documentation/vm/hmm.rst +++ b/Documentation/vm/hmm.rst @@ -339,9 +339,8 @@ Migration to and from device memory === Because the CPU cannot access device memory, migration must use the device DMA -engine to perform copy from and to device memory. For this we need a new to -use migrate_vma_setup(), migrate_vma_pages(), and migrate_vma_finalize() -helpers. +engine to perform copy from and to device memory. For this we need to use +migrate_vma_setup(), migrate_vma_pages(), and migrate_vma_finalize() helpers. Memory cgroup (memcg) and rss accounting diff --git a/mm/migrate.c b/mm/migrate.c index 993386cb53358d..0e78ebd720c0e4 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2622,10 +2622,9 @@ static void migrate_vma_unmap(struct migrate_vma *migrate) * successfully migrated, and which were not. Successfully migrated pages will * have the MIGRATE_PFN_MIGRATE flag set for their src array entry. * - * It is safe to update device page table from within the finalize_and_map() - * callback because both destination and source page are still locked, and the - * mmap_sem is held in read mode (hence no one can unmap the range being - * migrated). + * It is safe to update device page table after migrate_vma_pages() because + * both destination and source page are still locked, and the mmap_sem is held + * in read mode (hence no one can unmap the range being migrated). * * Once the caller is done cleaning up things and updating its page table (if it * chose to do so, this is not an obligation) it finally calls @@ -2657,10 +2656,11 @@ int migrate_vma_setup(struct migrate_vma *args) args->npages = 0; migrate_vma_collect(args); - if (args->cpages) - migrate_vma_prepare(args); - if (args->cpages) - migrate_vma_unmap(args); + if (!args->cpages) + return 0; + + migrate_vma_prepare(args); + migrate_vma_unmap(args); /* * At this point pages are locked and unmapped, and thus they have ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/10] mm: remove CONFIG_MIGRATE_VMA_HELPER
On Wed, Aug 14, 2019 at 09:59:28AM +0200, Christoph Hellwig wrote: > CONFIG_MIGRATE_VMA_HELPER guards helpers that are required for proper > devic private memory support. Remove the option and just check for > CONFIG_DEVICE_PRIVATE instead. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/nouveau/Kconfig | 1 - > mm/Kconfig | 3 --- > mm/migrate.c| 4 ++-- > 3 files changed, 2 insertions(+), 6 deletions(-) Reviewed-by: Jason Gunthorpe Aside from it making sense, I checked no other driver enables DEVICE_PRIVATE, so no change in kernel build. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Fri, Aug 16, 2019 at 10:21:41AM -0700, Dan Williams wrote: > > We can do a get_dev_pagemap inside the page_walk and touch the pgmap, > > or we can do the 'device mutex && retry' pattern and touch the pgmap > > in the driver, under that lock. > > > > However in all cases the current get_dev_pagemap()'s in the page walk > > are not necessary, and we can delete them. > > Yes, as long as 'struct page' instances resulting from that lookup are > not passed outside of that lock. Indeed. Also, I was reflecting over lunch that the hmm_range_fault should only return DEVICE_PRIVATE pages for the caller's device (see other thread with HCH), and in this case, the caller should also be responsible to ensure that the driver is not calling hmm_range_fault at the same time it is deleting it's own DEVICE_PRIVATE mapping - ie by fencing its page fault handler. This does not apply to PCI_P2PDMA, but, lets see how that looks when we get there. So the whole thing seems pretty safe. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] drm+dma: cache support for arm, etc
On Thu, Aug 15, 2019 at 10:53 AM Christoph Hellwig wrote: > > On Thu, Aug 15, 2019 at 06:54:39AM -0700, Rob Clark wrote: > > On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig wrote: > > > > > > As said before I don't think these low-level helpers are the > > > right API to export, but even if they did you'd just cover a tiny > > > subset of the architectures. > > > > Are you thinking instead something like: > > > > void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist > > *sgl, > > int nents, enum dma_data_direction dir) > > { > > for_each_sg(sgl, sg, nents, i) { > > arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir); > > } > > } > > EXPORT_SYMBOL_GPL(dma_sync_sg_for_..) > > > > or did you have something else in mind? > > No. We really need an interface thay says please give me uncached > memory (for some definition of uncached that includes that grapics > drivers call write combine), and then let the architecture do the right > thing. Basically dma_alloc_coherent with DMA_ATTR_NO_KERNEL_MAPPING > is superficially close to what you want, except that the way the drm > drivers work you can't actually use it. I don't disagree about needing an API to get uncached memory (or ideally just something outside of the linear map). But I think this is a separate problem. What I was hoping for, for v5.4, is a way to stop abusing dma_map/sync for cache ops to get rid of the hack I had to make for v5.3. And also to fix vgem on non-x86. (Unfortunately changing vgem to used cached mappings breaks x86 CI, but fixes CI on arm/arm64..) We can do that without any changes in allocation. There is still the possibility for problems due to cached alias, but that has been a problem this whole time, it isn't something new. BR, -R > The reason for that is if we can we really need to not create another > uncachable alias, but instead change the page attributes in place. > On x86 we can and must do that for example, and based on the > conversation with Will arm64 could do that fairly easily. arm32 can > right now only do that for CMA, though. > > The big question is what API do we want. I had a pretty similar > discussion with Christian on doing such an allocation for amdgpu, > where the device normally is cache coherent, but they actually want > to turn it into non-coherent by using PCIe unsnooped transactions. > > Here is my high level plan, which still has a few lose end: > > (1) provide a new API: > > struct page *dma_alloc_pages(struct device *dev, unsigned nr_pages, > gfp_t gfp, unsigned long flags); > void dma_free_pages(struct device *dev, unsigned nr_pages, > unsigned long flags); > > These give you back page backed memory that is guaranteed to be > addressable by the device (no swiotlb or similar). The memory can > then be mapped using dma_map*, including unmap and dma_sync to > bounce ownership around. This is the replacement for the current > dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT API, that is rather > badly defined. > > (2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead > of dma_alloc_attrs. The initial difference with that flag is just > that we allow highmem, but in the future we could also unmap this > memory from the kernel linear mapping entirely on architectures > where we can easily do that. > > (3) Add a dma_pages_map/dma_pages_unmap or similar API that allows you > to get a kernel mapping for parts or all of a > DMA_ATTR_NO_KERNEL_MAPPING allocation. This is to replace things > like your open-coded vmap in msm (or similarly elsewhere in dma-buf > providers). > > (4) Add support for a DMA_ATTR_UNCACHABLE flags (or similar) to the new > API, that maps the pages as uncachable iff they have a kernel > mapping, including invalidating the caches at time of this page > attribute change (or creation of a new mapping). This API will fail > if the architecture does not allow in-place remapping. Note that for > arm32 we could always dip into the CMA pool if one is present to not > fail. We'll also need some helper to map from the DMA_ATTR_* flags > to a pgprot for mapping the page to userspace. There is also a few > other weird bits here, e.g. on architectures like mips that use an > uncached segment we'll have to fail use with the plain > DMA_ATTR_UNCACHABLE flag, but it could be supported with > DMA_ATTR_UNCACHABLE | DMA_ATTR_NO_KERNEL_MAPPING. > > I was hoping to get most of this done for this merge window, but I'm > probably lucky if I get at least parts done. Too much distraction. > > > Hmm, not entirely sure why.. you should be on the cc list for each > > individual patch. > > They finally made it, although even with the delay they only ended up > in the spam mailbox. I still can't see them on the
Re: [PATCH] drm/amdgpu: Apply flags after amdgpu_device_ip_init()
at 21:33, Deucher, Alexander wrote: Thanks for finding this! I think the attached patch should fix the issue and it's much less invasive. Yes it also fix the issue, please add by tested-by: Tested-by: Kai-Heng Feng I took this more or less future proof approach because I think this won’t be the last chip that needs firmware information, which isn’t available in early init, to decides its flags. Yes it’s intrusive to carve out all flags from early init callbacks, but I don’t think it’s that ugly. Kai-Heng Alex From: Kai-Heng Feng Sent: Thursday, August 15, 2019 1:11 AM To: Deucher, Alexander ; Koenig, Christian ; Zhou, David(ChunMing) Cc: Huang, Ray ; anthony.w...@canonical.com ; amd-...@lists.freedesktop.org ; dri-devel@lists.freedesktop.org ; linux-ker...@vger.kernel.org ; Kai-Heng Feng Subject: [PATCH] drm/amdgpu: Apply flags after amdgpu_device_ip_init() After commit 005440066f92 ("drm/amdgpu: enable gfxoff again on raven series (v2)"), some Raven Ridge systems start to have rendering corruption in browser [1]. Chip specific flags like cg_flags and pg_flags are applied in amdgpu_device_ip_early_init(). For Raven Ridge, the flags may depend on pp_feature's PP_GFXOFF_MASK bit, which can be negated in amdgpu_device_ip_init() based on firmware information. At that time it's already too late, since cg_flags and pg_flags are already set. Apply flags after amdgpu_device_ip_init() and consolidate all flags to one place, to solve the issue. [1] https://lore.kernel.org/lkml/3eb0e920-31d7-4c91-a360-dbfb4417a...@canonical.com/ Fixes: 005440066f92 ("drm/amdgpu: enable gfxoff again on raven series (v2)") Signed-off-by: Kai-Heng Feng --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 589 + drivers/gpu/drm/amd/amdgpu/cik.c | 87 --- drivers/gpu/drm/amd/amdgpu/nv.c| 50 -- drivers/gpu/drm/amd/amdgpu/si.c| 83 --- drivers/gpu/drm/amd/amdgpu/soc15.c | 140 - drivers/gpu/drm/amd/amdgpu/vi.c| 162 -- 6 files changed, 589 insertions(+), 522 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 275277364a8a..10ea4899c338 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1852,6 +1852,591 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev) return r; } +#define CZ_REV_BRISTOL(rev) \ + ((rev >= 0xC8 && rev <= 0xCE) || (rev >= 0xE1 && rev <= 0xE6)) + +static int amdgpu_device_apply_flags(struct amdgpu_device *adev) +{ + switch (adev->asic_type) { + case CHIP_TAHITI: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + /*AMD_CG_SUPPORT_GFX_CGCG |*/ + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | + AMD_CG_SUPPORT_HDP_MGCG; + adev->pg_flags = 0; + break; + case CHIP_PITCAIRN: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + /*AMD_CG_SUPPORT_GFX_CGCG |*/ + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_GFX_RLC_LS | + AMD_CG_SUPPORT_MC_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | + AMD_CG_SUPPORT_UVD_MGCG | + AMD_CG_SUPPORT_HDP_LS | + AMD_CG_SUPPORT_HDP_MGCG; + adev->pg_flags = 0; + break; + case CHIP_VERDE: + adev->cg_flags = + AMD_CG_SUPPORT_GFX_MGCG | + AMD_CG_SUPPORT_GFX_MGLS | + AMD_CG_SUPPORT_GFX_CGLS | + AMD_CG_SUPPORT_GFX_CGTS | + AMD_CG_SUPPORT_GFX_CGTS_LS | + AMD_CG_SUPPORT_GFX_CP_LS | + AMD_CG_SUPPORT_MC_LS | + AMD_CG_SUPPORT_MC_MGCG | + AMD_CG_SUPPORT_SDMA_MGCG | + AMD_CG_SUPPORT_SDMA_LS | + AMD_CG_SUPPORT_BIF_LS | + AMD_CG_SUPPORT_VCE_MGCG | +
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> -Original Message- > From: Takashi Iwai > Sent: Thursday, August 15, 2019 9:57 AM > To: Alex Deucher > Cc: Karol Herbst; Limonciello, Mario; nouveau; Rafael J . Wysocki; LKML; > dri-devel; > Linux ACPI Mailing List; Alex Hung; Ben Skeggs; David Airlie > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to > enable dGPU direct output" > > > [EXTERNAL EMAIL] > > On Thu, 15 Aug 2019 16:37:05 +0200, > Alex Deucher wrote: > > > > On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst wrote: > > > > > > On Thu, Aug 15, 2019 at 4:20 PM wrote: > > > > > > > > > > There are definitely going to be regressions on machines in the > > > > > > field > with the > > > > > > in tree drivers by reverting this. I think we should have an > > > > > > answer for all > of > > > > > those > > > > > > before this revert is accepted. > > > > > > > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with > > > > > > partners > to > > > > > collect > > > > > > some information on the impact of reverting this. > > > > > > > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD > GPU to > > > > > use > > > > > > "Hybrid Graphics" when on Windows and "Power Express" and > "Switchable > > > > > Graphics" > > > > > > when on Linux. > > > > > > > > > > and what's exactly the difference between those? And what's the actual > > > > > issue here? > > > > > > > > DP/HDMI is not detected unless plugged in at bootup. It's due to > > > > missing > HPD > > > > events. > > > > > > > > > > afaik Lyude was working on fixing all that, at least for some drivers. > > > If there is something wrong, we still should fix the drivers, not > > > adding ACPI workarounds. > > > > > > Alex: do you know if there are remaining issues regarding that with > > > amdgpu? > > > > There was an issue with hpd events not making it to the audio side > > when things were powered down that was fixed with this patch set: > > https://patchwork.freedesktop.org/patch/316793/ > > Those patches depended on a bunch of alsa changes as well which may > > have not been available in the distro used for a particular OEM > > program. > > FYI, the corresponding commit for ALSA part is destined for 5.4 > kernel: > > https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=ade > 49db337a9d44ac5835cfce1ee873549011b27 > > BTW, Nouveau should suffer from the same problem. The patch to add > the audio component support is found at: > https://patchwork.freedesktop.org/patch/319131/ > > It sounds like 5.3rcX won't be a useful check then. So am I correct to understand that everything related to the AMD failures described in this thread should be in linux-next at this point? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Thu, Aug 15, 2019 at 08:54:46PM -0700, Dan Williams wrote: > > However, this means we cannot do any processing of ZONE_DEVICE pages > > outside the driver lock, so eg, doing any DMA map that might rely on > > MEMORY_DEVICE_PCI_P2PDMA has to be done in the driver lock, which is > > a bit unfortunate. > > Wouldn't P2PDMA use page pins? Not needing to hold a lock over > ZONE_DEVICE page operations was one of the motivations for plumbing > get_dev_pagemap() with a percpu-ref. hmm_range_fault() doesn't use page pins at all, so if a ZONE_DEVICE page comes out of it then it needs to use another locking pattern. If I follow it all right: We can do a get_dev_pagemap inside the page_walk and touch the pgmap, or we can do the 'device mutex && retry' pattern and touch the pgmap in the driver, under that lock. However in all cases the current get_dev_pagemap()'s in the page walk are not necessary, and we can delete them. ? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: turn hmm migrate_vma upside down v3
On Fri, Aug 16, 2019 at 08:51:41AM +0200, Christoph Hellwig wrote: > Jason, > > are you going to look into picking this up? Unfortunately there is > a hole pile in this area still pending, including the kvmppc secure > memory driver from Bharata that depends on the work. > > mm folks: migrate.c is mostly a classic MM file except for the hmm > additions. Do you want to also look over this or just let it pass? Yes, after you explained the functions were hmm ones, it seems OK to go to hmm.git. I was waiting for the dust to settle, I see Ralph tested-by, are we good now? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: turn hmm migrate_vma upside down v3
On Wed, Aug 14, 2019 at 09:59:18AM +0200, Christoph Hellwig wrote: > Hi Jérôme, Ben and Jason, > > below is a series against the hmm tree which starts revamping the > migrate_vma functionality. The prime idea is to export three slightly > lower level functions and thus avoid the need for migrate_vma_ops > callbacks. > > Diffstat: > > 7 files changed, 282 insertions(+), 614 deletions(-) Yay, another big wack of code gone Applied to hmm.git Thanks, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/5] kernel.h: Add non_block_start/end()
On Fri, Aug 16, 2019 at 06:36:52PM +0200, Daniel Vetter wrote: > On Fri, Aug 16, 2019 at 4:38 PM Jason Gunthorpe wrote: > > > > On Fri, Aug 16, 2019 at 04:11:34PM +0200, Daniel Vetter wrote: > > > Also, aside from this patch (which is prep for the next) and some > > > simple reordering conflicts they're all independent. So if there's no > > > way to paint this bikeshed here (technicolor perhaps?) then I'd like > > > to get at least the others considered. > > > > Sure, I think for conflict avoidance reasons I'm probably taking > > mmu_notifier stuff via hmm.git, so: > > > > - Andrew had a minor remark on #1, I am ambivalent and would take it > > as-is. Your decision if you want to respin. > > I like mine better, see also the reply from Ralph Campbell. Sure > > - #2/#3 is this issue, I would stand by the preempt_disable/etc path > > Our situation matches yours, debug tests run lockdep/etc. > > Since Michal requested the current flavour I think we need spin a bit > more on these here. I guess I'll just rebase them to the end so > they're not holding up the others. > > > - #4 I like a lot, except the map should enclose range_end too, > > this can be done after the mm_has_notifiers inside the > > __mmu_notifier function > > To make sure I get this right: The same lockdep context, but also > wrapped around invalidate_range_end? Yes, the locking context of _range_start and _range_end should be identical, last time I checked callers this was the case. So, just add it to __mmu_notifier_invalidate_range_end() outside the SRCU as there is no reason to burden debug kernel callers twice when mmu notifiers are not enabled Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> -Original Message- > From: Karol Herbst > Sent: Thursday, August 15, 2019 9:25 AM > To: Limonciello, Mario > Cc: Dave Airlie; LKML; Linux ACPI Mailing List; dri-devel; nouveau; Rafael J . > Wysocki; Alex Hung; Ben Skeggs; David Airlie > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to > enable dGPU direct output" > > > [EXTERNAL EMAIL] > > On Thu, Aug 15, 2019 at 4:20 PM wrote: > > > > > > There are definitely going to be regressions on machines in the field > > > > with > the > > > > in tree drivers by reverting this. I think we should have an answer > > > > for all of > > > those > > > > before this revert is accepted. > > > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to > > > collect > > > > some information on the impact of reverting this. > > > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD > GPU to > > > use > > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable > > > Graphics" > > > > when on Linux. > > > > > > and what's exactly the difference between those? And what's the actual > > > issue here? > > > > DP/HDMI is not detected unless plugged in at bootup. It's due to missing > > HPD > > events. > > > > afaik Lyude was working on fixing all that, at least for some drivers. > If there is something wrong, we still should fix the drivers, not > adding ACPI workarounds. I don't disagree, but timing is frequently a limitation if you want the hardware to work when you put it on the market. The whole idea behind the OSI string was that it could be reverted when the time was right. From this discussion it very well may be for systems with AMD GPU, but it needs to be checked again. > > Alex: do you know if there are remaining issues regarding that with amdgpu? > > > > > > > We already have the PRIME offloading in place and if that's not > > > enough, we should work on extending it, not adding some ACPI based > > > workarounds, because that's exactly how that looks like. > > > > > > Also, was this discussed with anybody involved in the drm subsystem? > > > > > > > > > > > I feel we need a knob and/or DMI detection to affect the changes that > > > > the > ASL > > > > normally performs. > > > > > > Why do we have to do that on a firmware level at all? > > > > Folks from AMD Graphics team recommended this approach. I should clarify this is from the folks on AMD graphics team that interact to OEMs like Dell which are not necessarily the same folks who work on the drivers directly. > From their perspective > > it's not a workaround. They view this as a different architecture for AMD > graphics driver on > > Windows and AMD graphics w/ amdgpu driver. They have different ASL paths > used for > > each. > > @alex: is this true? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] mm/hmm: hmm_range_fault handle pages swapped out
On Thu, Aug 15, 2019 at 08:52:56PM +, Yang, Philip wrote: > hmm_range_fault may return NULL pages because some of pfns are equal to > HMM_PFN_NONE. This happens randomly under memory pressure. The reason is > for swapped out page pte path, hmm_vma_handle_pte doesn't update fault > variable from cpu_flags, so it failed to call hmm_vam_do_fault to swap > the page in. > > The fix is to call hmm_pte_need_fault to update fault variable. > Change-Id: I2e8611485563d11d938881c18b7935fa1e7c91ee I'll fix it for you but please be careful not to send Change-id's to the public lists. Also what is the Fixes line for this? > Signed-off-by: Philip Yang > mm/hmm.c | 3 +++ > 1 file changed, 3 insertions(+) Ralph has also been looking at this area also so I'll give him a bit to chime in, otherwise with Jerome's review this looks OK to go to linux-next Thanks, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 hmm 08/11] drm/radeon: use mmu_notifier_get/put for struct radeon_mn
On Thu, Aug 15, 2019 at 10:28:21AM +0200, Christian König wrote: > Am 07.08.19 um 01:15 schrieb Jason Gunthorpe: > > From: Jason Gunthorpe > > > > radeon is using a device global hash table to track what mmu_notifiers > > have been registered on struct mm. This is better served with the new > > get/put scheme instead. > > > > radeon has a bug where it was not blocking notifier release() until all > > the BO's had been invalidated. This could result in a use after free of > > pages the BOs. This is tied into a second bug where radeon left the > > notifiers running endlessly even once the interval tree became > > empty. This could result in a use after free with module unload. > > > > Both are fixed by changing the lifetime model, the BOs exist in the > > interval tree with their natural lifetimes independent of the mm_struct > > lifetime using the get/put scheme. The release runs synchronously and just > > does invalidate_start across the entire interval tree to create the > > required DMA fence. > > > > Additions to the interval tree after release are already impossible as > > only current->mm is used during the add. > > > > Signed-off-by: Jason Gunthorpe > > Acked-by: Christian König Thanks! > But I'm wondering if we shouldn't completely drop radeon userptr support. > It's just to buggy, I would not object :) Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Thu, Aug 15, 2019 at 04:51:33PM -0400, Jerome Glisse wrote: > struct page. In this case any way we can update the > nouveau_dmem_page() to check that page page->pgmap == the > expected pgmap. I was also wondering if that is a problem.. just blindly doing a container_of on the page->pgmap does seem like it assumes that only this driver is using DEVICE_PRIVATE. It seems like something missing in hmm_range_fault, it should be told what DEVICE_PRIVATE is acceptable to trigger HMM_PFN_DEVICE_PRIVATE and fault all others? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Thu, Aug 15, 2019 at 01:47:12PM -0700, Dan Williams wrote: > On Thu, Aug 15, 2019 at 1:41 PM Jason Gunthorpe wrote: > > > > On Thu, Aug 15, 2019 at 04:33:06PM -0400, Jerome Glisse wrote: > > > > > So nor HMM nor driver should dereference the struct page (i do not > > > think any iommu driver would either), > > > > Er, they do technically deref the struct page: > > > > nouveau_dmem_convert_pfn(struct nouveau_drm *drm, > > struct hmm_range *range) > > struct page *page; > > page = hmm_pfn_to_page(range, range->pfns[i]); > > if (!nouveau_dmem_page(drm, page)) { > > > > > > nouveau_dmem_page(struct nouveau_drm *drm, struct page *page) > > { > > return is_device_private_page(page) && drm->dmem == > > page_to_dmem(page) > > > > > > Which does touch 'page->pgmap' > > > > Is this OK without having a get_dev_pagemap() ? > > > > Noting that the collision-retry scheme doesn't protect anything here > > as we can have a concurrent invalidation while doing the above deref. > > As long take_driver_page_table_lock() in Jerome's flow can replace > percpu_ref_tryget_live() on the pagemap reference. It seems > nouveau_dmem_convert_pfn() happens after: > > mutex_lock(>mutex); > if (!nouveau_range_done()) { > > ...so I would expect that to be functionally equivalent to validating > the reference count. Yes, OK, that makes sense, I was mostly surprised by the statement the driver doesn't touch the struct page.. I suppose "doesn't touch the struct page out of the driver lock" is the case. However, this means we cannot do any processing of ZONE_DEVICE pages outside the driver lock, so eg, doing any DMA map that might rely on MEMORY_DEVICE_PCI_P2PDMA has to be done in the driver lock, which is a bit unfortunate. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 204407] New: Bad page state in process Xorg
On Fri, Aug 16, 2019 at 02:47:53PM +0200, Vlastimil Babka wrote: > On 8/15/19 9:13 PM, Petr Vandrovec wrote: > > [ 18.110985] DMAR: [DMA Write] Request device [07:00.1] fault addr > > fffe [fault reason 02] Present bit in context entry is clear > > Worth reporting as well, not nice regression. Is that a regression between 5.3-rc3 and 5.3-rc4 or is it already broken since -rc1? The 5.3-rc5 kernel will contains some VT-d fixes that are worth a try here too. If you can test latest linus/master branch that would be great, otherwise -rc5 is fine too. Regards, Joerg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] kernel.h: Add non_block_start/end()
On Thu, Aug 15, 2019 at 07:42:07PM +0200, Michal Hocko wrote: > On Thu 15-08-19 13:56:31, Jason Gunthorpe wrote: > > On Thu, Aug 15, 2019 at 06:00:41PM +0200, Michal Hocko wrote: > > > > > > AFAIK 'GFP_NOWAIT' is characterized by the lack of __GFP_FS and > > > > __GFP_DIRECT_RECLAIM.. > > > > > > > > This matches the existing test in __need_fs_reclaim() - so if you are > > > > OK with GFP_NOFS, aka __GFP_IO which triggers try_to_compact_pages(), > > > > allocations during OOM, then I think fs_reclaim already matches what > > > > you described? > > > > > > No GFP_NOFS is equally bad. Please read my other email explaining what > > > the oom_reaper actually requires. In short no blocking on direct or > > > indirect dependecy on memory allocation that might sleep. > > > > It is much easier to follow with some hints on code, so the true > > requirement is that the OOM repear not block on GFP_FS and GFP_IO > > allocations, great, that constraint is now clear. > > I still do not get why do you put FS/IO into the picture. This is really > about __GFP_DIRECT_RECLAIM. Like I said this is complicated, translating "no blocking on direct or indirect dependecy on memory allocation that might sleep" into GFP flags is hard for us outside the mm community. So the contraint here is no __GFP_DIRECT_RECLAIM? I bring up FS/IO because that is what Tejun mentioned when I asked him about reclaim restrictions, and is what fs_reclaim_acquire() is already sensitive too. It is pretty confusing if we have places using the word 'reclaim' with different restrictions. :( > >CPU0 CPU1 > > mutex_lock() > > kmalloc(GFP_KERNEL) > > no I mean __GFP_DIRECT_RECLAIM here. > > > mutex_unlock() > > fs_reclaim_acquire() > > mutex_lock() <- illegal: lock dep assertion > > I cannot really comment on how that is achieveable by lockdep. ??? I am trying to explain this is already done and working today. The above example will already fault with lockdep enabled. This is existing debugging we can use and improve upon rather that invent new debugging. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 204407] New: Bad page state in process Xorg
Vlastimil Babka wrote on 8/15/2019 7:32 AM: Does the issue still happen with rc4? Could you apply the 3 attached patches (work in progress), configure-enable CONFIG_DEBUG_PAGEALLOC and CONFIG_PAGE_OWNER and boot kernel with debug_pagealloc=on page_owner=on parameters? That should print stacktraces of allocation and first freeing (assuming this is a double free). Unfortunately -rc4 does not find any my SATA disks due to some misunderstanding between AHCI driver and HPT642L adapter (there is no device 07:00.1, HPT is single-function device at 07:00.0): [ 18.003015] scsi host6: ahci [ 18.006605] DMAR: DRHD: handling fault status reg 2 [ 18.006619] DMAR: [DMA Write] Request device [07:00.1] fault addr fffe [fault reason 02] Present bit in context entry is clear [ 18.076616] DMAR: DRHD: handling fault status reg 102 [ 18.085910] DMAR: [DMA Write] Request device [07:00.1] fault addr fffa [fault reason 02] Present bit in context entry is clear [ 18.100989] DMAR: DRHD: handling fault status reg 202 [ 18.110985] DMAR: [DMA Write] Request device [07:00.1] fault addr fffe [fault reason 02] Present bit in context entry is clear With iommu=off disks are visible, but USB keyboard (and other USB devices) does not work: [ 18.174802] ehci-pci :00:1a.0: swiotlb buffer is full (sz: 8 bytes), total 0 (slots), used 0 (slots) [ 18.174804] ehci-pci :00:1a.0: overflow 0x000ffdc75ae8+8 of DMA mask bus mask 0 [ 18.174815] WARNING: CPU: 2 PID: 508 at kernel/dma/direct.c:35 report_addr+0x2e/0x50 [ 18.174816] Modules linked in: [ 18.174818] CPU: 2 PID: 508 Comm: kworker/2:1 Tainted: G T 5.3.0-rc4-64-00058-gd717b092e0b2 #77 [ 18.174819] Hardware name: Dell Inc. Precision T3610/09M8Y8, BIOS A16 02/05/2018 [ 18.174822] Workqueue: usb_hub_wq hub_event I'll try to find -rc4 configuration that has enabled debugging and can boot. Petr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> -Original Message- > From: linux-acpi-ow...@vger.kernel.org On > Behalf Of Dave Airlie > Sent: Wednesday, August 14, 2019 5:48 PM > To: Karol Herbst > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung; Ben > Skeggs; Dave Airlie > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to > enable dGPU direct output" > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst wrote: > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c. > > > > The original commit message didn't even make sense. AMD _does_ support it > > and > > it works with Nouveau as well. > > > > Also what was the issue being solved here? No references to any bugs and not > > even explaining any issue at all isn't the way we do things. > > > > And even if it means a muxed design, then the fix is to make it work inside > > the > > driver, not adding some hacky workaround through ACPI tricks. > > > > And what out of tree drivers do or do not support we don't care one bit > > anyway. > > > > I think the reverts should be merged via Rafael's tree as the original > patches went in via there, and we should get them in asap. > > Acked-by: Dave Airlie > Dave. There are definitely going to be regressions on machines in the field with the in tree drivers by reverting this. I think we should have an answer for all of those before this revert is accepted. Regarding systems with Intel+NVIDIA, we'll have to work with partners to collect some information on the impact of reverting this. When this is used on a system with Intel+AMD the ASL configures AMD GPU to use "Hybrid Graphics" when on Windows and "Power Express" and "Switchable Graphics" when on Linux. I feel we need a knob and/or DMI detection to affect the changes that the ASL normally performs. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Fri, Aug 16, 2019 at 06:44:48AM +0200, Christoph Hellwig wrote: > On Fri, Aug 16, 2019 at 12:43:07AM +, Jason Gunthorpe wrote: > > On Thu, Aug 15, 2019 at 04:51:33PM -0400, Jerome Glisse wrote: > > > > > struct page. In this case any way we can update the > > > nouveau_dmem_page() to check that page page->pgmap == the > > > expected pgmap. > > > > I was also wondering if that is a problem.. just blindly doing a > > container_of on the page->pgmap does seem like it assumes that only > > this driver is using DEVICE_PRIVATE. > > > > It seems like something missing in hmm_range_fault, it should be told > > what DEVICE_PRIVATE is acceptable to trigger HMM_PFN_DEVICE_PRIVATE > > and fault all others? > > The whole device private handling in hmm and migrate_vma seems pretty > broken as far as I can tell, and I have some WIP patches. Basically we > should not touch (or possibly eventually call migrate to ram eventually > in the future) device private pages not owned by the caller, where I > try to defined the caller by the dev_pagemap_ops instance. I think it needs to be more elaborate. For instance, a system may have multiple DEVICE_PRIVATE map's owned by the same driver - but multiple physical devices using that driver. Each physical device's driver should only ever get DEVICE_PRIVATE pages for it's own on-device memory. Never a DEVICE_PRIVATE for another device's memory. The dev_pagemap_ops would not be unique enough, right? Probably also clusters of same-driver struct device can share a DEVICE_PRIVATE, at least high end GPU's now have private memory coherency busses between their devices. Since we want to trigger migration to CPU on incompatible DEVICE_PRIVATE pages, it seems best to sort this out in the hmm_range_fault? Maybe some sort of unique ID inside the page->pgmap and passed as input? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Thu, Aug 15, 2019 at 02:03:25PM -0400, Jerome Glisse wrote: > On Wed, Aug 14, 2019 at 07:48:28AM -0700, Dan Williams wrote: > > On Wed, Aug 14, 2019 at 6:28 AM Jason Gunthorpe wrote: > > > > > > On Wed, Aug 14, 2019 at 09:38:54AM +0200, Christoph Hellwig wrote: > > > > On Tue, Aug 13, 2019 at 06:36:33PM -0700, Dan Williams wrote: > > > > > Section alignment constraints somewhat save us here. The only example > > > > > I can think of a PMD not containing a uniform pgmap association for > > > > > each pte is the case when the pgmap overlaps normal dram, i.e. shares > > > > > the same 'struct memory_section' for a given span. Otherwise, distinct > > > > > pgmaps arrange to manage their own exclusive sections (and now > > > > > subsections as of v5.3). Otherwise the implementation could not > > > > > guarantee different mapping lifetimes. > > > > > > > > > > That said, this seems to want a better mechanism to determine "pfn is > > > > > ZONE_DEVICE". > > > > > > > > So I guess this patch is fine for now, and once you provide a better > > > > mechanism we can switch over to it? > > > > > > What about the version I sent to just get rid of all the strange > > > put_dev_pagemaps while scanning? Odds are good we will work with only > > > a single pagemap, so it makes some sense to cache it once we find it? > > > > Yes, if the scan is over a single pmd then caching it makes sense. > > Quite frankly an easier an better solution is to remove the pagemap > lookup as HMM user abide by mmu notifier it means we will not make > use or dereference the struct page so that we are safe from any > racing hotunplug of dax memory (as long as device driver using hmm > do not have a bug). Yes, I also would prefer to drop the confusing checks entirely - Christoph can you resend this patch? Thanks, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
nouveau: System crashes with NVIDIA GeForce 8600 GT
Hi all, I'm getting frequent system crashes (every few hours or so) and it seems that the nouveau driver is causing the issue (dmesg output below). I see it with both v5.2.8 and the v4.19 LTS kernel. Sometimes the system completely freezes and sometimes seemingly just the nouveau driver goes down. The screen freezes and colours stream across it. Often after I reboot the BIOS logo is mangled too until the first modeset. The crash seems to be happening in nv50_fb_intr() in nv50.c. I'm not sure if this is related, but the system now often freezes on suspend or resume since I switched from using the old (recently abandoned) proprietry NVIDIA drivers, again both with 5.2 and 4.19 kernels. Blacklisting the nouveau driver doesn't seem to fix it however, though I guess the graphics card could still be causing issues in some other way? I never had problems with suspend and resume before. Any suggestions about how I could debug this further? Best, Alex dmesg output: [0.00] BIOS-e820: [mem 0x0010-0xcfe8] usable [0.00] BIOS-e820: [mem 0xcfe9-0xcfee2fff] ACPI NVS [0.00] BIOS-e820: [mem 0xcfee3000-0xcfee] ACPI data [0.00] BIOS-e820: [mem 0xcfef-0xcfef] reserved [0.00] BIOS-e820: [mem 0xe000-0xefff] reserved [0.00] BIOS-e820: [mem 0xfec0-0x] reserved [0.00] BIOS-e820: [mem 0x0001-0x00014fff] usable [0.00] NX (Execute Disable) protection: active [0.00] SMBIOS 2.5 present. [0.00] DMI: MEDIONPC MS-7502/MS-7502, BIOS 6.00 PG 01/13/2010 [0.00] tsc: Fast TSC calibration using PIT [0.00] tsc: Detected 2394.193 MHz processor [0.003907] e820: update [mem 0x-0x0fff] usable ==> reserved [0.003909] e820: remove [mem 0x000a-0x000f] usable [0.003914] last_pfn = 0x15 max_arch_pfn = 0x4 [0.003920] MTRR default type: uncachable [0.003921] MTRR fixed ranges enabled: [0.003923] 0-9 write-back [0.003924] A-B uncachable [0.003925] C-CBFFF write-protect [0.003926] CC000-F write-through [0.003927] MTRR variable ranges enabled: [0.003929] 0 base 1 mask FC000 write-back [0.003931] 1 base 14000 mask FF000 write-back [0.003932] 2 base 0 mask F8000 write-back [0.003934] 3 base 08000 mask FC000 write-back [0.003935] 4 base 0C000 mask FF000 write-back [0.003936] 5 base 0CFF0 mask 0 uncachable [0.003937] 6 disabled [0.003938] 7 disabled [0.004864] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WP UC- WT [0.005025] total RAM covered: 4607M [0.005436] Found optimal setting for mtrr clean up [0.005438] gran_size: 64K chunk_size: 2M num_reg: 6 lose cover RAM: 0G [0.006325] e820: update [mem 0xcff0-0x] usable ==> reserved [0.006331] last_pfn = 0xcfe90 max_arch_pfn = 0x4 [0.011041] found SMP MP-table at [mem 0x000f3e50-0x000f3e5f] [0.034915] check: Scanning 1 areas for low memory corruption [0.034920] Kernel/User page tables isolation: disabled on command line. [0.034923] BRK [0x6ae01000, 0x6ae01fff] PGTABLE [0.034926] BRK [0x6ae02000, 0x6ae02fff] PGTABLE [0.034928] BRK [0x6ae03000, 0x6ae03fff] PGTABLE [0.034958] BRK [0x6ae04000, 0x6ae04fff] PGTABLE [0.034960] BRK [0x6ae05000, 0x6ae05fff] PGTABLE [0.035037] BRK [0x6ae06000, 0x6ae06fff] PGTABLE [0.035287] RAMDISK: [mem 0x36443000-0x37218fff] [0.035295] ACPI: Early table checksum verification disabled [0.035367] ACPI: RSDP 0x000F8050 14 (v00 MEDION) [0.035371] ACPI: RSDT 0xCFEE3000 3C (v01 MEDION MEDIONAG 42302E31 AWRD ) [0.035378] ACPI: FACP 0xCFEE30C0 74 (v01 MEDION MEDIONAG 42302E31 AWRD ) [0.035385] ACPI: DSDT 0xCFEE3140 004AC3 (v01 MEDION AWRDACPI 1000 MSFT 0300) [0.035390] ACPI: FACS 0xCFE9 40 [0.035393] ACPI: HPET 0xCFEE7E80 38 (v01 MEDION MEDIONAG 42302E31 AWRD 0098) [0.035397] ACPI: MCFG 0xCFEE7EC0 3C (v01 MEDION MEDIONAG 42302E31 AWRD ) [0.035401] ACPI: SLIC 0xCFEE7C40 000176 (v01 MEDION MEDIONAG 42302E31 AWRD ) [0.035405] ACPI: APIC 0xCFEE7DC0 84 (v01 MEDION MEDIONAG 42302E31 AWRD ) [0.035409] ACPI: SSDT 0xCFEE8820 000380 (v01 PmRef CpuPm 3000 INTL 20041203) [0.035419] ACPI: Local APIC address 0xfee0 [0.035497] No NUMA configuration found [0.035499] Faking a node at [mem 0x-0x00014fff] [0.035502] NODE_DATA(0) allocated [mem 0x14fff8000-0x14fffbfff] [0.035530] Zone ranges: [0.035531] DMA [mem 0x1000-0x00ff] [0.035532] DMA32[mem
Re: [PATCH 01/10] mm: turn migrate_vma upside down
On Sat, Aug 17, 2019 at 01:31:28PM +0200, Christoph Hellwig wrote: > On Fri, Aug 16, 2019 at 05:11:07PM +, Jason Gunthorpe wrote: > > - if (args->cpages) > > - migrate_vma_prepare(args); > > - if (args->cpages) > > - migrate_vma_unmap(args); > > + if (!args->cpages) > > + return 0; > > + > > + migrate_vma_prepare(args); > > + migrate_vma_unmap(args); > > I don't think this is ok. Both migrate_vma_prepare and migrate_vma_unmap > can reduce args->cpages, including possibly to 0. Oh, yes, that was far too hasty on my part, I had assumed collect set the cpages. Thank you for checking Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/1] drm/hisilicon/hibmc: Make CONFIG_DRM_HISI_HIBMC depend on ARM64
On 2019/8/15 12:26, Matthew Ruffell wrote: Hi, amd64 based Huawei servers have problems where the display output of their iBMC chips is broken, resulting in a "blurry" screen when viewed from their in house remote kvm-like console. Example: https://launchpadlibrarian.net/365907668/creen_picture_for_blur.png The issue is caused by the hibmc_drm kernel module being loaded. The PCI ID for the iBMC chips on amd64 hardware is the same as arm64 hardware, but the hibmc_drm driver was developed only for use on arm64 hardware, most notably for the Huawei D05 development board. The impact to Huawei is that their customers cannot use Ubuntu server install media as the screen goes "blurry" when the d-i install media or subuqity installer loads the hibmc_drm kernel module after language selection. The only workaround for their customers is to press the "E" key during the very first installer menu and adding "modprobe.blacklist=hibmc_drm" to the kernel command line in the grub menu. This is not good for customer experience with their servers. Huawei help site on the matter: https://support.huawei.com/enterprise/en/knowledge/EKB1100015521 Other problems exist for the hibmc_drm driver on amd64, such as working on bios, but not uefi, and not being wayland compatible, making the screen "blurry" whenever a desktop session is started and gdm loaded. More information about the above can be found on Launchpad: https://bugs.launchpad.net/bugs/1762940 Huawei have asked us to remove hibmc_drm from all architectures except arm64, and this aligns with advice from Hisilicon. Hibmc maintainers, can you please review the status of hibmc_drm on amd64 and confirm that these issues exist, and consider merging the patch to update Kconfig to set CONFIG_DRM_HISI_HIBMC arm64 only. Matthew Ruffell (1): drm/hisilicon/hibmc: Make CONFIG_DRM_HISI_HIBMC depend on ARM64 drivers/gpu/drm/hisilicon/hibmc/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Thanks Matthew for the patch. For the patch, Acked-by: Xinliang Liu And applied and will push to drm maintainer soon. Thanks, Xinliang ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/10] mm: remove the unused MIGRATE_PFN_DEVICE flag
On Wed, Aug 14, 2019 at 09:59:27AM +0200, Christoph Hellwig wrote: > No one ever checks this flag, and we could easily get that information > from the page if needed. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Ralph Campbell > --- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 3 +-- > include/linux/migrate.h| 1 - > mm/migrate.c | 4 ++-- > 3 files changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Jason Gunthorpe Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Thu, Aug 15, 2019 at 04:33:06PM -0400, Jerome Glisse wrote: > So nor HMM nor driver should dereference the struct page (i do not > think any iommu driver would either), Er, they do technically deref the struct page: nouveau_dmem_convert_pfn(struct nouveau_drm *drm, struct hmm_range *range) struct page *page; page = hmm_pfn_to_page(range, range->pfns[i]); if (!nouveau_dmem_page(drm, page)) { nouveau_dmem_page(struct nouveau_drm *drm, struct page *page) { return is_device_private_page(page) && drm->dmem == page_to_dmem(page) Which does touch 'page->pgmap' Is this OK without having a get_dev_pagemap() ? Noting that the collision-retry scheme doesn't protect anything here as we can have a concurrent invalidation while doing the above deref. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> > There are definitely going to be regressions on machines in the field with > > the > > in tree drivers by reverting this. I think we should have an answer for > > all of > those > > before this revert is accepted. > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to > collect > > some information on the impact of reverting this. > > > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to > use > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable > Graphics" > > when on Linux. > > and what's exactly the difference between those? And what's the actual > issue here? DP/HDMI is not detected unless plugged in at bootup. It's due to missing HPD events. > > We already have the PRIME offloading in place and if that's not > enough, we should work on extending it, not adding some ACPI based > workarounds, because that's exactly how that looks like. > > Also, was this discussed with anybody involved in the drm subsystem? > > > > > I feel we need a knob and/or DMI detection to affect the changes that the > > ASL > > normally performs. > > Why do we have to do that on a firmware level at all? Folks from AMD Graphics team recommended this approach. From their perspective it's not a workaround. They view this as a different architecture for AMD graphics driver on Windows and AMD graphics w/ amdgpu driver. They have different ASL paths used for each. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> On Thu, Aug 15, 2019 at 10:15 AM Karol Herbst wrote: > > > > On Thu, Aug 15, 2019 at 4:13 PM Alex Deucher > wrote: > > > > > > On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst wrote: > > > > > > > > On Thu, Aug 15, 2019 at 3:56 PM wrote: > > > > > > > > > > > -Original Message- > > > > > > From: linux-acpi-ow...@vger.kernel.org ow...@vger.kernel.org> On > > > > > > Behalf Of Dave Airlie > > > > > > Sent: Wednesday, August 14, 2019 5:48 PM > > > > > > To: Karol Herbst > > > > > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex > > > > > > Hung; > Ben > > > > > > Skeggs; Dave Airlie > > > > > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI > string to > > > > > > enable dGPU direct output" > > > > > > > > > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst > wrote: > > > > > > > > > > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c. > > > > > > > > > > > > > > The original commit message didn't even make sense. AMD _does_ > support it and > > > > > > > it works with Nouveau as well. > > > > > > > > > > > > > > Also what was the issue being solved here? No references to any > > > > > > > bugs > and not > > > > > > > even explaining any issue at all isn't the way we do things. > > > > > > > > > > > > > > And even if it means a muxed design, then the fix is to make it > > > > > > > work > inside the > > > > > > > driver, not adding some hacky workaround through ACPI tricks. > > > > > > > > > > > > > > And what out of tree drivers do or do not support we don't care > > > > > > > one > bit anyway. > > > > > > > > > > > > > > > > > > > I think the reverts should be merged via Rafael's tree as the > > > > > > original > > > > > > patches went in via there, and we should get them in asap. > > > > > > > > > > > > Acked-by: Dave Airlie > > > > > > Dave. > > > > > > > > > > There are definitely going to be regressions on machines in the field > > > > > with > the > > > > > in tree drivers by reverting this. I think we should have an answer > > > > > for all > of those > > > > > before this revert is accepted. > > > > > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners > > > > > to > collect > > > > > some information on the impact of reverting this. > > > > > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD > GPU to use > > > > > "Hybrid Graphics" when on Windows and "Power Express" and > "Switchable Graphics" > > > > > when on Linux. > > > > > > > > and what's exactly the difference between those? And what's the actual > > > > issue here? > > > > > > Hybrid Graphics is the new "standard" way of handling these laptops. > > > It uses the standard _PR3 APCI method to handle dGPU power. Support > > > for this was added to Linux relatively later compared to when the > > > laptops were launched. "Power Express" used the other AMD specific > > > ATPX ACPI method to handle dGPU power. The driver supports both so > > > either method will work. > > > > > > Alex > > > > > > > thanks for clarifying. But that still means that we won't need such > > workarounds for AMD users, right? amdgpu handles hybrid graphics just > > fine, right? > > Yeah it should, assuming you have a new enough kernel which supports > HG, which has been several years at this point IIRC. > > Alex > Can you define how new of a kernel is a new enough kernel? Looking on my side these problems were on new hardware (Precision 7740) and are checked as recently as start of this summer, w/ kernel 4.15. We can arrange to have it checked again on 5.3rcX w/ the OSI disabled. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] drm+dma: cache support for arm, etc
On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig wrote: > > As said before I don't think these low-level helpers are the > right API to export, but even if they did you'd just cover a tiny > subset of the architectures. Are you thinking instead something like: void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist *sgl, int nents, enum dma_data_direction dir) { for_each_sg(sgl, sg, nents, i) { arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir); } } EXPORT_SYMBOL_GPL(dma_sync_sg_for_..) or did you have something else in mind? I guess something like this would avoid figuring out *which* archs actually build drm.. > Also to distil the previous thread - if you remap memory to uncached > the helper to use is arch_dma_prep_coherent, which does a writeback+ > invalidate everywhere, and there is no need to clean up after a > long-term uncached mapping. We might still get speculations into > that area, if we don't remap the direct mapping, but it isn't like > invalidting that just before freeing the memory is going to help > anyone. hmm, IIUC the aarch64 cache instructions, what I'm doing now is equiv to what I would get with dma_map_sg(DMA_BIDIRECTIONAL) and arch_dma_prep_coherent() is equiv to what I'd get w/ DMA_FROM_DEVICE (but a single pass instead of separate inv+clean passes).. but I can respin this with a single dma_prep_coherent_sg() which uses arch_dma_prep_coherent().. > Also it seems like patches 5 and 6 are missing in my inbox. Hmm, not entirely sure why.. you should be on the cc list for each individual patch. But here is the patchwork link if you want to see them: https://patchwork.freedesktop.org/series/65211/ BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal
Currently dma_fence_signal() tries to avoid the spinlock and only takes it if absolutely required to walk the callback list. However, to allow for some users to surreptitiously insert lazy signal callbacks that do not depend on enabling the signaling mechanism around every fence, we always need to notify the callbacks on signaling. As such, we will always need to take the spinlock and dma_fence_signal() effectively becomes a clone of dma_fence_signal_locked(). v2: Update the test_and_set_bit() before entering the spinlock. v3: Drop the test_[and_set]_bit() before the spinlock, it's a caller error so expected to be very unlikely. Signed-off-by: Chris Wilson Cc: Christian König Cc: Daniel Vetter --- drivers/dma-buf/dma-fence.c | 44 ++--- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index ff0cd6eae766..8a6d0250285d 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -129,25 +129,16 @@ EXPORT_SYMBOL(dma_fence_context_alloc); int dma_fence_signal_locked(struct dma_fence *fence) { struct dma_fence_cb *cur, *tmp; - int ret = 0; lockdep_assert_held(fence->lock); - if (WARN_ON(!fence)) + if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, + >flags))) return -EINVAL; - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) { - ret = -EINVAL; - - /* -* we might have raced with the unlocked dma_fence_signal, -* still run through all callbacks -*/ - } else { - fence->timestamp = ktime_get(); - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); - trace_dma_fence_signaled(fence); - } + fence->timestamp = ktime_get(); + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); + trace_dma_fence_signaled(fence); if (!list_empty(>cb_list)) { list_for_each_entry_safe(cur, tmp, >cb_list, node) { @@ -156,7 +147,8 @@ int dma_fence_signal_locked(struct dma_fence *fence) } INIT_LIST_HEAD(>cb_list); } - return ret; + + return 0; } EXPORT_SYMBOL(dma_fence_signal_locked); @@ -176,28 +168,16 @@ EXPORT_SYMBOL(dma_fence_signal_locked); int dma_fence_signal(struct dma_fence *fence) { unsigned long flags; + int ret; if (!fence) return -EINVAL; - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) - return -EINVAL; - - fence->timestamp = ktime_get(); - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); - trace_dma_fence_signaled(fence); - - if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) { - struct dma_fence_cb *cur, *tmp; + spin_lock_irqsave(fence->lock, flags); + ret = dma_fence_signal_locked(fence); + spin_unlock_irqrestore(fence->lock, flags); - spin_lock_irqsave(fence->lock, flags); - list_for_each_entry_safe(cur, tmp, >cb_list, node) { - list_del_init(>node); - cur->func(fence, cur); - } - spin_unlock_irqrestore(fence->lock, flags); - } - return 0; + return ret; } EXPORT_SYMBOL(dma_fence_signal); -- 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/6] dma-fence: Store the timestamp in the same union as the cb_list
Am 17.08.19 um 16:47 schrieb Chris Wilson: > The timestamp and the cb_list are mutually exclusive, the cb_list can > only be added to prior to being signaled (and once signaled we drain), > while the timestamp is only valid upon being signaled. Both the > timestamp and the cb_list are only valid while the fence is alive, and > as soon as no references are held can be replaced by the rcu_head. > > By reusing the union for the timestamp, we squeeze the base dma_fence > struct to 64 bytes on x86-64. > > Suggested-by: Christian König > Signed-off-by: Chris Wilson > Cc: Christian König > --- > drivers/dma-buf/dma-fence.c | 16 +--- > drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 13 +++-- > drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 3 +++ > include/linux/dma-fence.h | 17 + > 4 files changed, 32 insertions(+), 17 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 89d96e3e6df6..2c21115b1a37 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -129,6 +129,7 @@ EXPORT_SYMBOL(dma_fence_context_alloc); > int dma_fence_signal_locked(struct dma_fence *fence) > { > struct dma_fence_cb *cur, *tmp; > + struct list_head cb_list; > > lockdep_assert_held(fence->lock); > > @@ -136,16 +137,16 @@ int dma_fence_signal_locked(struct dma_fence *fence) > >flags))) > return -EINVAL; > > + /* Stash the cb_list before replacing it with the timestamp */ > + list_replace(>cb_list, _list); Stashing the timestamp instead is probably less bytes to modify. > + > fence->timestamp = ktime_get(); > set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > trace_dma_fence_signaled(fence); > > - if (!list_empty(>cb_list)) { > - list_for_each_entry_safe(cur, tmp, >cb_list, node) { > - INIT_LIST_HEAD(>node); > - cur->func(fence, cur); > - } > - INIT_LIST_HEAD(>cb_list); > + list_for_each_entry_safe(cur, tmp, _list, node) { > + INIT_LIST_HEAD(>node); > + cur->func(fence, cur); > } > > return 0; > @@ -234,7 +235,8 @@ void dma_fence_release(struct kref *kref) > > trace_dma_fence_destroy(fence); > > - if (WARN(!list_empty(>cb_list), > + if (WARN(!list_empty(>cb_list) && > + !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags), >"Fence %s:%s:%llx:%llx released with pending signals!\n", >fence->ops->get_driver_name(fence), >fence->ops->get_timeline_name(fence), > diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > index 2bc9c460e78d..09c68dda2098 100644 > --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > @@ -114,18 +114,18 @@ __dma_fence_signal__timestamp(struct dma_fence *fence, > ktime_t timestamp) > } > > static void > -__dma_fence_signal__notify(struct dma_fence *fence) > +__dma_fence_signal__notify(struct dma_fence *fence, > +const struct list_head *list) > { > struct dma_fence_cb *cur, *tmp; > > lockdep_assert_held(fence->lock); > lockdep_assert_irqs_disabled(); > > - list_for_each_entry_safe(cur, tmp, >cb_list, node) { > + list_for_each_entry_safe(cur, tmp, list, node) { > INIT_LIST_HEAD(>node); > cur->func(fence, cur); > } > - INIT_LIST_HEAD(>cb_list); > } > > void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) > @@ -187,11 +187,12 @@ void intel_engine_breadcrumbs_irq(struct > intel_engine_cs *engine) > list_for_each_safe(pos, next, ) { > struct i915_request *rq = > list_entry(pos, typeof(*rq), signal_link); > - > - __dma_fence_signal__timestamp(>fence, timestamp); > + struct list_head cb_list; > > spin_lock(>lock); > - __dma_fence_signal__notify(>fence); > + list_replace(>fence.cb_list, _list); > + __dma_fence_signal__timestamp(>fence, timestamp); > + __dma_fence_signal__notify(>fence, _list); > spin_unlock(>lock); > > i915_request_put(rq); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > index 434dfadb0e52..178a6cd1a06f 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c > @@ -185,6 +185,9 @@ static long vmw_fence_wait(struct dma_fence *f, bool > intr, signed long timeout) > > spin_lock(f->lock); > > + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) > + goto out; > + > if (intr && signal_pending(current)) { > ret = -ERESTARTSYS; > goto out; >
Re: [PATCH 5/6] dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal
Am 17.08.19 um 16:47 schrieb Chris Wilson: > Currently dma_fence_signal() tries to avoid the spinlock and only takes > it if absolutely required to walk the callback list. However, to allow > for some users to surreptitiously insert lazy signal callbacks that > do not depend on enabling the signaling mechanism around every fence, > we always need to notify the callbacks on signaling. As such, we will > always need to take the spinlock and dma_fence_signal() effectively > becomes a clone of dma_fence_signal_locked(). > > v2: Update the test_and_set_bit() before entering the spinlock. > > Signed-off-by: Chris Wilson > Cc: Christian König > Cc: Daniel Vetter > --- > drivers/dma-buf/dma-fence.c | 43 +++-- > 1 file changed, 13 insertions(+), 30 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index ff0cd6eae766..89d96e3e6df6 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -129,25 +129,16 @@ EXPORT_SYMBOL(dma_fence_context_alloc); > int dma_fence_signal_locked(struct dma_fence *fence) > { > struct dma_fence_cb *cur, *tmp; > - int ret = 0; > > lockdep_assert_held(fence->lock); > > - if (WARN_ON(!fence)) > + if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, > + >flags))) > return -EINVAL; > > - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) { > - ret = -EINVAL; > - > - /* > - * we might have raced with the unlocked dma_fence_signal, > - * still run through all callbacks > - */ > - } else { > - fence->timestamp = ktime_get(); > - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > - trace_dma_fence_signaled(fence); > - } > + fence->timestamp = ktime_get(); > + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > + trace_dma_fence_signaled(fence); > > if (!list_empty(>cb_list)) { > list_for_each_entry_safe(cur, tmp, >cb_list, node) { > @@ -156,7 +147,8 @@ int dma_fence_signal_locked(struct dma_fence *fence) > } > INIT_LIST_HEAD(>cb_list); > } > - return ret; > + > + return 0; > } > EXPORT_SYMBOL(dma_fence_signal_locked); > > @@ -176,28 +168,19 @@ EXPORT_SYMBOL(dma_fence_signal_locked); > int dma_fence_signal(struct dma_fence *fence) > { > unsigned long flags; > + int ret; > > if (!fence) > return -EINVAL; > > - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) > + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) > return -EINVAL; Actually I think we can completely drop this extra test. Signaling a fence twice shouldn't be the fast path we should optimize for. Apart from that it looks good to me, Christian. > > - fence->timestamp = ktime_get(); > - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); > - trace_dma_fence_signaled(fence); > - > - if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) { > - struct dma_fence_cb *cur, *tmp; > + spin_lock_irqsave(fence->lock, flags); > + ret = dma_fence_signal_locked(fence); > + spin_unlock_irqrestore(fence->lock, flags); > > - spin_lock_irqsave(fence->lock, flags); > - list_for_each_entry_safe(cur, tmp, >cb_list, node) { > - list_del_init(>node); > - cur->func(fence, cur); > - } > - spin_unlock_irqrestore(fence->lock, flags); > - } > - return 0; > + return ret; > } > EXPORT_SYMBOL(dma_fence_signal); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][drm-next] drm/panel: remove redundant assignment to val
On Sat, Aug 17, 2019 at 01:21:24PM +0100, Colin King wrote: > From: Colin Ian King > > Variable val is initialized to a value in a for-loop that is > never read and hence it is redundant. Remove it. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King Thanks. Applied and pushed to drm-misc-next. Sam > --- > drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > index 3b4f30c0fdae..84370562910f 100644 > --- a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c > @@ -116,7 +116,7 @@ static void td043mtea1_write_gamma(struct > td043mtea1_panel *lcd) > td043mtea1_write(lcd, 0x13, val); > > /* gamma bits [7:0] */ > - for (val = i = 0; i < 12; i++) > + for (i = 0; i < 12; i++) > td043mtea1_write(lcd, 0x14 + i, gamma[i] & 0xff); > } > > -- > 2.20.1
[PATCH 3/6] dma-fence: Shrink size of struct dma_fence
Rearrange the couple of 32-bit atomics hidden amongst the field of pointers that unnecessarily caused the compiler to insert some padding, shrinks the size of the base struct dma_fence from 80 to 72 bytes on x86-64. Signed-off-by: Chris Wilson Cc: Christian König Reviewed-by: Christian König --- include/linux/dma-fence.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 404aa748eda6..2ce4d877d33e 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -63,7 +63,7 @@ struct dma_fence_cb; * been completed, or never called at all. */ struct dma_fence { - struct kref refcount; + spinlock_t *lock; const struct dma_fence_ops *ops; /* We clear the callback list on kref_put so that by the time we * release the fence it is unused. No one should be adding to the cb_list @@ -73,11 +73,11 @@ struct dma_fence { struct rcu_head rcu; struct list_head cb_list; }; - spinlock_t *lock; u64 context; u64 seqno; - unsigned long flags; ktime_t timestamp; + unsigned long flags; + struct kref refcount; int error; }; -- 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] dma-buf: Introduce selftesting framework
In light of recent review slip ups, the absence of a suite of tests for dma-buf became apparent. Given the current plethora of testing frameworks, opt for one already in use by Intel's CI and so allow easy hook up into igt. We introduce a new module that when loaded will execute the list of selftests and their subtest. The names of the selftests are put into the modinfo as parameters so that igt can identify each, and run them independently, principally for ease of error reporting. Signed-off-by: Chris Wilson Cc: Daniel Vetter --- drivers/dma-buf/Kconfig | 5 ++ drivers/dma-buf/Makefile| 3 + drivers/dma-buf/selftest.c | 167 drivers/dma-buf/selftest.h | 30 +++ drivers/dma-buf/selftests.h | 12 +++ 5 files changed, 217 insertions(+) create mode 100644 drivers/dma-buf/selftest.c create mode 100644 drivers/dma-buf/selftest.h create mode 100644 drivers/dma-buf/selftests.h diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig index b6a9c2f1bc41..a23b6752d11a 100644 --- a/drivers/dma-buf/Kconfig +++ b/drivers/dma-buf/Kconfig @@ -39,4 +39,9 @@ config UDMABUF A driver to let userspace turn memfd regions into dma-bufs. Qemu can use this to create host dmabufs for guest framebuffers. +config DMABUF_SELFTESTS + tristate "Selftests for the dma-buf interfaces" + default n + depends on DMA_SHARED_BUFFER + endmenu diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index dcfb01e7c6f4..b5ae122a9349 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -4,3 +4,6 @@ obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ obj-$(CONFIG_SYNC_FILE)+= sync_file.o obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o obj-$(CONFIG_UDMABUF) += udmabuf.o + +dmabuf_selftests-y := selftest.o +obj-$(CONFIG_DMABUF_SELFTESTS) += dmabuf_selftests.o diff --git a/drivers/dma-buf/selftest.c b/drivers/dma-buf/selftest.c new file mode 100644 index ..c60b6944b4bd --- /dev/null +++ b/drivers/dma-buf/selftest.c @@ -0,0 +1,167 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2019 Intel Corporation + */ + +#include +#include +#include +#include +#include + +#include "selftest.h" + +enum { +#define selftest(n, func) __idx_##n, +#include "selftests.h" +#undef selftest +}; + +#define selftest(n, f) [__idx_##n] = { .name = #n, .func = f }, +static struct selftest { + bool enabled; + const char *name; + int (*func)(void); +} selftests[] = { +#include "selftests.h" +}; +#undef selftest + +/* Embed the line number into the parameter name so that we can order tests */ +#define param(n) __PASTE(igt__, __PASTE(__PASTE(__LINE__, __), n)) +#define selftest_0(n, func, id) \ +module_param_named(id, selftests[__idx_##n].enabled, bool, 0400); +#define selftest(n, func) selftest_0(n, func, param(n)) +#include "selftests.h" +#undef selftest + +int __sanitycheck__(void) +{ + pr_debug("Hello World!\n"); + return 0; +} + +static char *__st_filter; + +static bool apply_subtest_filter(const char *caller, const char *name) +{ + char *filter, *sep, *tok; + bool result = true; + + filter = kstrdup(__st_filter, GFP_KERNEL); + for (sep = filter; (tok = strsep(, ","));) { + bool allow = true; + char *sl; + + if (*tok == '!') { + allow = false; + tok++; + } + + if (*tok == '\0') + continue; + + sl = strchr(tok, '/'); + if (sl) { + *sl++ = '\0'; + if (strcmp(tok, caller)) { + if (allow) + result = false; + continue; + } + tok = sl; + } + + if (strcmp(tok, name)) { + if (allow) + result = false; + continue; + } + + result = allow; + break; + } + kfree(filter); + + return result; +} + +int +__subtests(const char *caller, const struct subtest *st, int count, void *data) +{ + int err; + + for (; count--; st++) { + cond_resched(); + if (signal_pending(current)) + return -EINTR; + + if (!apply_subtest_filter(caller, st->name)) + continue; + + pr_info("dma-buf: Running %s/%s\n", caller, st->name); + + err = st->func(data); + if (err && err != -EINTR) { + pr_err("dma-buf/%s: %s failed with error %d\n", + caller, st->name, err); + return err; + } + } + + return 0; +} + +static
[PATCH 4/6] dma-fence: Avoid list_del during fence->cb_list iteration
Before we notify the fence signal callback, we remove the cb from the list. However, since we are processing the entire list from underneath the spinlock, we do not need to individual delete each element, but can simply reset the link and the entire list. Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Christian König Reviewed-by: Christian König --- drivers/dma-buf/dma-fence.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 8025a891d3e9..ff0cd6eae766 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -149,9 +149,12 @@ int dma_fence_signal_locked(struct dma_fence *fence) trace_dma_fence_signaled(fence); } - list_for_each_entry_safe(cur, tmp, >cb_list, node) { - list_del_init(>node); - cur->func(fence, cur); + if (!list_empty(>cb_list)) { + list_for_each_entry_safe(cur, tmp, >cb_list, node) { + INIT_LIST_HEAD(>node); + cur->func(fence, cur); + } + INIT_LIST_HEAD(>cb_list); } return ret; } -- 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal
Currently dma_fence_signal() tries to avoid the spinlock and only takes it if absolutely required to walk the callback list. However, to allow for some users to surreptitiously insert lazy signal callbacks that do not depend on enabling the signaling mechanism around every fence, we always need to notify the callbacks on signaling. As such, we will always need to take the spinlock and dma_fence_signal() effectively becomes a clone of dma_fence_signal_locked(). v2: Update the test_and_set_bit() before entering the spinlock. Signed-off-by: Chris Wilson Cc: Christian König Cc: Daniel Vetter --- drivers/dma-buf/dma-fence.c | 43 +++-- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index ff0cd6eae766..89d96e3e6df6 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -129,25 +129,16 @@ EXPORT_SYMBOL(dma_fence_context_alloc); int dma_fence_signal_locked(struct dma_fence *fence) { struct dma_fence_cb *cur, *tmp; - int ret = 0; lockdep_assert_held(fence->lock); - if (WARN_ON(!fence)) + if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, + >flags))) return -EINVAL; - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) { - ret = -EINVAL; - - /* -* we might have raced with the unlocked dma_fence_signal, -* still run through all callbacks -*/ - } else { - fence->timestamp = ktime_get(); - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); - trace_dma_fence_signaled(fence); - } + fence->timestamp = ktime_get(); + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); + trace_dma_fence_signaled(fence); if (!list_empty(>cb_list)) { list_for_each_entry_safe(cur, tmp, >cb_list, node) { @@ -156,7 +147,8 @@ int dma_fence_signal_locked(struct dma_fence *fence) } INIT_LIST_HEAD(>cb_list); } - return ret; + + return 0; } EXPORT_SYMBOL(dma_fence_signal_locked); @@ -176,28 +168,19 @@ EXPORT_SYMBOL(dma_fence_signal_locked); int dma_fence_signal(struct dma_fence *fence) { unsigned long flags; + int ret; if (!fence) return -EINVAL; - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) return -EINVAL; - fence->timestamp = ktime_get(); - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); - trace_dma_fence_signaled(fence); - - if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, >flags)) { - struct dma_fence_cb *cur, *tmp; + spin_lock_irqsave(fence->lock, flags); + ret = dma_fence_signal_locked(fence); + spin_unlock_irqrestore(fence->lock, flags); - spin_lock_irqsave(fence->lock, flags); - list_for_each_entry_safe(cur, tmp, >cb_list, node) { - list_del_init(>node); - cur->func(fence, cur); - } - spin_unlock_irqrestore(fence->lock, flags); - } - return 0; + return ret; } EXPORT_SYMBOL(dma_fence_signal); -- 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] dma-buf: Add selftests for dma-fence
Exercise the dma-fence API exported to drivers. Signed-off-by: Chris Wilson Cc: Daniel Vetter --- drivers/dma-buf/Makefile | 5 +- drivers/dma-buf/selftests.h| 1 + drivers/dma-buf/st-dma-fence.c | 571 + 3 files changed, 576 insertions(+), 1 deletion(-) create mode 100644 drivers/dma-buf/st-dma-fence.c diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index b5ae122a9349..03479da06422 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -5,5 +5,8 @@ obj-$(CONFIG_SYNC_FILE) += sync_file.o obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o obj-$(CONFIG_UDMABUF) += udmabuf.o -dmabuf_selftests-y := selftest.o +dmabuf_selftests-y := \ + selftest.o \ + st-dma-fence.o + obj-$(CONFIG_DMABUF_SELFTESTS) += dmabuf_selftests.o diff --git a/drivers/dma-buf/selftests.h b/drivers/dma-buf/selftests.h index 44b44390d23a..5320386f02e5 100644 --- a/drivers/dma-buf/selftests.h +++ b/drivers/dma-buf/selftests.h @@ -10,3 +10,4 @@ * Tests are executed in order by igt/dmabuf_selftest */ selftest(sanitycheck, __sanitycheck__) /* keep first (igt selfcheck) */ +selftest(dma_fence, dma_fence) diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c new file mode 100644 index ..d052bea4fb9f --- /dev/null +++ b/drivers/dma-buf/st-dma-fence.c @@ -0,0 +1,571 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2019 Intel Corporation + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "selftest.h" + +static struct kmem_cache *slab_fences; + +static struct mock_fence { + struct dma_fence base; + struct spinlock lock; +} *to_mock_fence(struct dma_fence *f) { + return container_of(f, struct mock_fence, base); +} + +static const char *mock_name(struct dma_fence *f) +{ + return "mock"; +} + +static void mock_fence_release(struct dma_fence *f) +{ + kmem_cache_free(slab_fences, to_mock_fence(f)); +} + +struct wait_cb { + struct dma_fence_cb cb; + struct task_struct *task; +}; + +static void mock_wakeup(struct dma_fence *f, struct dma_fence_cb *cb) +{ + wake_up_process(container_of(cb, struct wait_cb, cb)->task); +} + +static long mock_wait(struct dma_fence *f, bool intr, long timeout) +{ + const int state = intr ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; + struct wait_cb cb = { .task = current }; + + if (dma_fence_add_callback(f, , mock_wakeup)) + return timeout; + + while (timeout) { + set_current_state(state); + + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) + break; + + if (signal_pending_state(state, current)) + break; + + timeout = schedule_timeout(timeout); + } + __set_current_state(TASK_RUNNING); + + if (!dma_fence_remove_callback(f, )) + return timeout; + + if (signal_pending_state(state, current)) + return -ERESTARTSYS; + + return -ETIME; +} + +static const struct dma_fence_ops mock_ops = { + .get_driver_name = mock_name, + .get_timeline_name = mock_name, + .wait = mock_wait, + .release = mock_fence_release, +}; + +static struct dma_fence *mock_fence(void) +{ + struct mock_fence *f; + + f = kmem_cache_alloc(slab_fences, GFP_KERNEL); + if (!f) + return NULL; + + spin_lock_init(>lock); + dma_fence_init(>base, _ops, >lock, 0, 0); + + return >base; +} + +static int sanitycheck(void *arg) +{ + struct dma_fence *f; + + f = mock_fence(); + if (!f) + return -ENOMEM; + + dma_fence_signal(f); + dma_fence_put(f); + + return 0; +} + +static int test_signaling(void *arg) +{ + struct dma_fence *f; + int err = -EINVAL; + + f = mock_fence(); + if (!f) + return -ENOMEM; + + if (dma_fence_is_signaled(f)) { + pr_err("Fence unexpectedly signaled on creation\n"); + goto err_free; + } + + if (dma_fence_signal(f)) { + pr_err("Fence reported being already signaled\n"); + goto err_free; + } + + if (!dma_fence_is_signaled(f)) { + pr_err("Fence not reporting signaled\n"); + goto err_free; + } + + if (!dma_fence_signal(f)) { + pr_err("Fence reported not being already signaled\n"); + goto err_free; + } + + err = 0; +err_free: + dma_fence_put(f); + return err; +} + +struct simple_cb { + struct dma_fence_cb cb; + bool seen; +}; + +static void simple_callback(struct dma_fence *f, struct dma_fence_cb *cb) +{ + smp_store_mb(container_of(cb, struct simple_cb, cb)->seen, true); +} + +static int test_add_callback(void *arg) +{ + struct simple_cb cb = {}; +
[PATCH 6/6] dma-fence: Store the timestamp in the same union as the cb_list
The timestamp and the cb_list are mutually exclusive, the cb_list can only be added to prior to being signaled (and once signaled we drain), while the timestamp is only valid upon being signaled. Both the timestamp and the cb_list are only valid while the fence is alive, and as soon as no references are held can be replaced by the rcu_head. By reusing the union for the timestamp, we squeeze the base dma_fence struct to 64 bytes on x86-64. Suggested-by: Christian König Signed-off-by: Chris Wilson Cc: Christian König --- drivers/dma-buf/dma-fence.c | 16 +--- drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 13 +++-- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 3 +++ include/linux/dma-fence.h | 17 + 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 89d96e3e6df6..2c21115b1a37 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -129,6 +129,7 @@ EXPORT_SYMBOL(dma_fence_context_alloc); int dma_fence_signal_locked(struct dma_fence *fence) { struct dma_fence_cb *cur, *tmp; + struct list_head cb_list; lockdep_assert_held(fence->lock); @@ -136,16 +137,16 @@ int dma_fence_signal_locked(struct dma_fence *fence) >flags))) return -EINVAL; + /* Stash the cb_list before replacing it with the timestamp */ + list_replace(>cb_list, _list); + fence->timestamp = ktime_get(); set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, >flags); trace_dma_fence_signaled(fence); - if (!list_empty(>cb_list)) { - list_for_each_entry_safe(cur, tmp, >cb_list, node) { - INIT_LIST_HEAD(>node); - cur->func(fence, cur); - } - INIT_LIST_HEAD(>cb_list); + list_for_each_entry_safe(cur, tmp, _list, node) { + INIT_LIST_HEAD(>node); + cur->func(fence, cur); } return 0; @@ -234,7 +235,8 @@ void dma_fence_release(struct kref *kref) trace_dma_fence_destroy(fence); - if (WARN(!list_empty(>cb_list), + if (WARN(!list_empty(>cb_list) && +!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags), "Fence %s:%s:%llx:%llx released with pending signals!\n", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c index 2bc9c460e78d..09c68dda2098 100644 --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c @@ -114,18 +114,18 @@ __dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp) } static void -__dma_fence_signal__notify(struct dma_fence *fence) +__dma_fence_signal__notify(struct dma_fence *fence, + const struct list_head *list) { struct dma_fence_cb *cur, *tmp; lockdep_assert_held(fence->lock); lockdep_assert_irqs_disabled(); - list_for_each_entry_safe(cur, tmp, >cb_list, node) { + list_for_each_entry_safe(cur, tmp, list, node) { INIT_LIST_HEAD(>node); cur->func(fence, cur); } - INIT_LIST_HEAD(>cb_list); } void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) @@ -187,11 +187,12 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) list_for_each_safe(pos, next, ) { struct i915_request *rq = list_entry(pos, typeof(*rq), signal_link); - - __dma_fence_signal__timestamp(>fence, timestamp); + struct list_head cb_list; spin_lock(>lock); - __dma_fence_signal__notify(>fence); + list_replace(>fence.cb_list, _list); + __dma_fence_signal__timestamp(>fence, timestamp); + __dma_fence_signal__notify(>fence, _list); spin_unlock(>lock); i915_request_put(rq); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c index 434dfadb0e52..178a6cd1a06f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c @@ -185,6 +185,9 @@ static long vmw_fence_wait(struct dma_fence *f, bool intr, signed long timeout) spin_lock(f->lock); + if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags)) + goto out; + if (intr && signal_pending(current)) { ret = -ERESTARTSYS; goto out; diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 2ce4d877d33e..6c36502a0562 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -65,17 +65,26 @@ struct dma_fence_cb; struct dma_fence { spinlock_t *lock; const
[Bug 111413] Linux's DRI interface
https://bugs.freedesktop.org/show_bug.cgi?id=111413 Andre Klapper changed: What|Removed |Added Component|IGT |Two Group||spam Status|NEW |RESOLVED Version|XOrg git|unspecified Resolution|--- |INVALID Product|DRI |Spam --- 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 110674] Crashes / Resets From AMDGPU / Radeon VII
https://bugs.freedesktop.org/show_bug.cgi?id=110674 --- Comment #115 from Tom B --- I should have noted it earlier, but I had already tried reverting both "golden values" commits. I've no idea what it does but it didn't fix this crash. One thing that would be insightful would be logging every call to smum_send_msg_to_smc_with_parameter and printing out message/parameter: int smum_send_msg_to_smc_with_parameter(struct pp_hwmgr *hwmgr, uint16_t msg, uint32_t parameter) { This would cause a very busy log but we could see the last successful message that was sent and with the same log in 5.0.13 see if there are any obvious differences. It might be that the previous message causes the invalid state so knowing what that is could lead us towards the solution. I don't think I have time to try it today but if anyone is recompiling the code adding pr_err("msg: %d / parameter: %d\n", msg, parameter); to this function in smumgr.c would be a useful addition. Also, wants to try re-compiling, here's a quick guide for arch: 1. Get the kernel sources using asp as described here: https://wiki.archlinux.org/index.php/Kernel/Arch_Build_System navigate to the created linux/repos/core-x86_64 directory. 2. You will need to run makepkg -s once to get it to download the sources. 3. You can set the kernel version in PKGBUILD: e.g. _srcver=5.2.7-arch1 or _srcver=5.0.13-arch1 4. If you want to revert one or more commits put it in the prepare() block before local src: echo "$_kernelname" > localversion.20-pkgname git revert db64a2f43c1bc22c5ff2d22606000b8c3587d0ec --no-edit git revert f5e79735cab448981e245a41ee6cbebf0e334f61 --no-edit local src It will open your editor, if you don't want to use vi set: 5. For making changes to the code you need to make a patch. Open the src/archlinux-linux directory. The files you're interested in are in drivers/drm/gpu/drm/amd/powerplay likely hwmgr/vega20_hwmgr.c Make your changes to the code. You can't just re-run makepkg as it checks out the original version of the code. After making changes, navigate to the archlinux-linux directory and run git diff > ../../vii.patch 6. Add your patch to PKGBUILD source: source=( "$_srcname::git+https://git.archlinux.org/linux.git?signed#tag=v$_srcver; config # the main kernel config file 60-linux.hook # pacman hook for depmod 90-linux.hook # pacman hook for initramfs regeneration linux.preset # standard config files for mkinitcpio ramdisk vii.patch ) 7. I've been cheating with makepkg and getting it to skip hash checks as otherwise you have to generate the sha256sums for each patch you create. This is an extra step that only slows down testing. To compile/install run makepkg -si --skipinteg Because of the way makepkg works, it keeps the compiled code in the src directory. That means that although the first compile will take a few minutes, subsequent compiles will be a lot faster as it'll probably only be recompiling vega20_hwmgr.c -- 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-next] drm/panel: remove redundant assignment to val
From: Colin Ian King Variable val is initialized to a value in a for-loop that is never read and hence it is redundant. Remove it. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King --- drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c index 3b4f30c0fdae..84370562910f 100644 --- a/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c +++ b/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c @@ -116,7 +116,7 @@ static void td043mtea1_write_gamma(struct td043mtea1_panel *lcd) td043mtea1_write(lcd, 0x13, val); /* gamma bits [7:0] */ - for (val = i = 0; i < 12; i++) + for (i = 0; i < 12; i++) td043mtea1_write(lcd, 0x14 + i, gamma[i] & 0xff); } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: Shrink size of struct dma_fence
Quoting Koenig, Christian (2019-08-17 12:42:48) > Am 17.08.19 um 13:39 schrieb Chris Wilson: > > Rearrange the couple of 32-bit atomics hidden amongst the field of > > pointers that unnecessarily caused the compiler to insert some padding, > > shrinks the size of the base struct dma_fence from 80 to 72 bytes on > > x86-64. > > > > Signed-off-by: Chris Wilson > > Cc: Christian König > > Reviewed-by: Christian König > > BTW: We could also put the timestamp in the union if we want. > > E.g. the cb_list should only be used while the fence is unsignaled, the > timestamp while it is signaled and the rcu while it is freed. > > Would save another 8 bytes, bringing us down to 64. I was looking at packing the error into the flags and shrinking that to 32b to fit inside the magical 64 bytes. You are right about the timestamp being mutually exclusive with the cb_list. The only caveat being that no reader would be allowed access to the timestamp unless they hold a reference (which I think covers all current users). -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dma-buf: Shrink size of struct dma_fence
Am 17.08.19 um 13:39 schrieb Chris Wilson: > Rearrange the couple of 32-bit atomics hidden amongst the field of > pointers that unnecessarily caused the compiler to insert some padding, > shrinks the size of the base struct dma_fence from 80 to 72 bytes on > x86-64. > > Signed-off-by: Chris Wilson > Cc: Christian König Reviewed-by: Christian König BTW: We could also put the timestamp in the union if we want. E.g. the cb_list should only be used while the fence is unsignaled, the timestamp while it is signaled and the rcu while it is freed. Would save another 8 bytes, bringing us down to 64. Christian. > --- > include/linux/dma-fence.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index 404aa748eda6..2ce4d877d33e 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -63,7 +63,7 @@ struct dma_fence_cb; >* been completed, or never called at all. >*/ > struct dma_fence { > - struct kref refcount; > + spinlock_t *lock; > const struct dma_fence_ops *ops; > /* We clear the callback list on kref_put so that by the time we >* release the fence it is unused. No one should be adding to the > cb_list > @@ -73,11 +73,11 @@ struct dma_fence { > struct rcu_head rcu; > struct list_head cb_list; > }; > - spinlock_t *lock; > u64 context; > u64 seqno; > - unsigned long flags; > ktime_t timestamp; > + unsigned long flags; > + struct kref refcount; > int error; > }; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dma-buf: Shrink size of struct dma_fence
Rearrange the couple of 32-bit atomics hidden amongst the field of pointers that unnecessarily caused the compiler to insert some padding, shrinks the size of the base struct dma_fence from 80 to 72 bytes on x86-64. Signed-off-by: Chris Wilson Cc: Christian König --- include/linux/dma-fence.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 404aa748eda6..2ce4d877d33e 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -63,7 +63,7 @@ struct dma_fence_cb; * been completed, or never called at all. */ struct dma_fence { - struct kref refcount; + spinlock_t *lock; const struct dma_fence_ops *ops; /* We clear the callback list on kref_put so that by the time we * release the fence it is unused. No one should be adding to the cb_list @@ -73,11 +73,11 @@ struct dma_fence { struct rcu_head rcu; struct list_head cb_list; }; - spinlock_t *lock; u64 context; u64 seqno; - unsigned long flags; ktime_t timestamp; + unsigned long flags; + struct kref refcount; int error; }; -- 2.23.0.rc1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] efifb: BGRT: Improve efifb_bgrt_sanity_check
Hi, On 21-07-19 15:19, Hans de Goede wrote: For various reasons, at least with x86 EFI firmwares, the xoffset and yoffset in the BGRT info are not always reliable. Extensive testing has shown that when the info is correct, the BGRT image is always exactly centered horizontally (the yoffset variable is more variable and not always predictable). This commit simplifies / improves the bgrt_sanity_check to simply check that the BGRT image is exactly centered horizontally and skips (re)drawing it when it is not. This fixes the BGRT image sometimes being drawn in the wrong place. Cc: sta...@vger.kernel.org Fixes: 88fe4ceb2447 ("efifb: BGRT: Do not copy the boot graphics for non native resolutions") Signed-off-by: Hans de Goede ping? I do not see this one in -next yet, what is the status of this patch? Regards, Hans --- drivers/video/fbdev/efifb.c | 27 ++- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index dfa8dd47d19d..5b3cef9bf794 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -122,28 +122,13 @@ static void efifb_copy_bmp(u8 *src, u32 *dst, int width, struct screen_info *si) */ static bool efifb_bgrt_sanity_check(struct screen_info *si, u32 bmp_width) { - static const int default_resolutions[][2] = { - { 800, 600 }, - { 1024, 768 }, - { 1280, 1024 }, - }; - u32 i, right_margin; - - for (i = 0; i < ARRAY_SIZE(default_resolutions); i++) { - if (default_resolutions[i][0] == si->lfb_width && - default_resolutions[i][1] == si->lfb_height) - break; - } - /* If not a default resolution used for textmode, this should be fine */ - if (i >= ARRAY_SIZE(default_resolutions)) - return true; - - /* If the right margin is 5 times smaller then the left one, reject */ - right_margin = si->lfb_width - (bgrt_tab.image_offset_x + bmp_width); - if (right_margin < (bgrt_tab.image_offset_x / 5)) - return false; + /* +* All x86 firmwares horizontally center the image (the yoffset +* calculations differ between boards, but xoffset is predictable). +*/ + u32 expected_xoffset = (si->lfb_width - bmp_width) / 2; - return true; + return bgrt_tab.image_offset_x == expected_xoffset; } #else static bool efifb_bgrt_sanity_check(struct screen_info *si, u32 bmp_width) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/arm: drop use of drmP.h
Drop use of deprecated drmP.h header file. Remove drmP.h includes and add some include headers for function or struct that used in code. --- drivers/gpu/drm/arm/hdlcd_crtc.c| 2 +- drivers/gpu/drm/arm/hdlcd_drv.c | 6 +- drivers/gpu/drm/arm/malidp_crtc.c | 4 +++- drivers/gpu/drm/arm/malidp_drv.c| 4 +++- drivers/gpu/drm/arm/malidp_drv.h| 1 - drivers/gpu/drm/arm/malidp_hw.c | 7 ++- drivers/gpu/drm/arm/malidp_mw.c | 2 +- drivers/gpu/drm/arm/malidp_planes.c | 4 +++- 8 files changed, 22 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c index a3efa28436ea..8285ff3e9991 100644 --- a/drivers/gpu/drm/arm/hdlcd_crtc.c +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c @@ -9,7 +9,6 @@ * Implementation of a CRTC class for the HDLCD driver. */ -#include #include #include #include @@ -19,6 +18,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index 27c46a2838c5..c2a59712cf46 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -9,6 +9,7 @@ * ARM HDLCD Driver */ +#include #include #include #include @@ -17,18 +18,21 @@ #include #include #include +#include #include -#include #include #include +#include #include #include #include #include +#include #include #include #include +#include #include "hdlcd_drv.h" #include "hdlcd_regs.h" diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c index db4451260fff..3735554f61bf 100644 --- a/drivers/gpu/drm/arm/malidp_crtc.c +++ b/drivers/gpu/drm/arm/malidp_crtc.c @@ -6,11 +6,13 @@ * ARM Mali DP500/DP550/DP650 driver (crtc operations) */ -#include #include #include #include +#include #include +#include + #include #include #include diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index c27ff456eddc..2a13490e5dbb 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -15,17 +15,19 @@ #include #include -#include #include #include #include +#include #include #include #include +#include #include #include #include #include +#include #include "malidp_drv.h" #include "malidp_mw.h" diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h index 0a639af8337e..a57edff55f2c 100644 --- a/drivers/gpu/drm/arm/malidp_drv.h +++ b/drivers/gpu/drm/arm/malidp_drv.h @@ -14,7 +14,6 @@ #include #include #include -#include #include "malidp_hw.h" #define MALIDP_CONFIG_VALID_INIT 0 diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c index 380be66d4c6e..f66d6b4bdaab 100644 --- a/drivers/gpu/drm/arm/malidp_hw.c +++ b/drivers/gpu/drm/arm/malidp_hw.c @@ -9,9 +9,14 @@ */ #include +#include #include #include -#include + +#include +#include +#include + #include #include diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c index 2e812525025d..c2f5ba52c8aa 100644 --- a/drivers/gpu/drm/arm/malidp_mw.c +++ b/drivers/gpu/drm/arm/malidp_mw.c @@ -10,8 +10,8 @@ #include #include #include +#include #include -#include #include #include "malidp_drv.h" diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c index 488375bd133d..3c70a53813bf 100644 --- a/drivers/gpu/drm/arm/malidp_planes.c +++ b/drivers/gpu/drm/arm/malidp_planes.c @@ -7,11 +7,13 @@ */ #include +#include -#include #include #include +#include #include +#include #include #include #include -- 2.20.1
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #106 from reject5...@naver.com --- (In reply to magist3r from comment #105) > (In reply to reject5514 from comment #103) > > I have this problem on Archlinux 5.2.8-arch1-1-ARCH when connected 2 > > monitors(1920x1080 @ 60Hz) and amdgpu.ppfeaturemask=0x option > > enabled. Patch didn't work for me. > > > > My GPU is RX570. > > Try this patch: > https://lists.freedesktop.org/archives/amd-gfx/2019-June/036022.html That patch solved the issue but memory clock is fixed to maximum state(1750MHz). Normally it should change dynamically. -- 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 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #16 from rol...@rptd.ch --- Created attachment 145083 --> https://bugs.freedesktop.org/attachment.cgi?id=145083=edit Linking fail log -- 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 111077] link_shader and deserialize_glsl_program suddenly consume huge amount of RAM
https://bugs.freedesktop.org/show_bug.cgi?id=111077 --- Comment #15 from rol...@rptd.ch --- I've tried now the patching approach. I had to patch in total three files: src/amd/common/ac_nir_to_llvm.c src/gallium/auxiliary/gallivm/lp_bld_init.c src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c Nevertheless the build still fails at linking stage (see fail_log attachment). -- 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][drm-next] drm/amd/display: fix a potential null pointer dereference
On Fri, Aug 16, 2019 at 11:10:11PM +0100, Colin King wrote: > From: Colin Ian King > > Currently the pointer init_data is dereferenced on the assignment > of fw_info before init_data is sanity checked to see if it is null. > Fix te potential null pointer dereference on init_data by only > performing dereference after it is null checked. > > Addresses-Coverity: ("Dereference before null check") > Fixes: 9adc8050bf3c ("drm/amd/display: make firmware info only load once > during dc_bios create") > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > index bee81bf288be..926954c804a6 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > @@ -1235,7 +1235,7 @@ static bool calc_pll_max_vco_construct( > struct calc_pll_clock_source_init_data *init_data) > { > uint32_t i; > - struct dc_firmware_info *fw_info = _data->bp->fw_info; > + struct dc_firmware_info *fw_info; > if (calc_pll_cs == NULL || > init_data == NULL || > init_data->bp == NULL) init_data can't be NULL. I'm mostly pointing this out because that NULL check is written so higgledy-piggledy. At first I thought this was staging code so I was planning to ignore the patch. :P regards, dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel