[drm-next-3.18 (-wip)] only works with b6c2b4 below evergreen (UVD)
Hello Alex and Christian, RV730 (AGP) need badly From b6c2b4faf90230ef9cf1a81f36cbccda4a606c59 Mon Sep 17 00:00:00 2001 From: Alex Deucher Date: Mon, 8 Sep 2014 13:16:39 -0400 Subject: [PATCH] drm/radeon: only use me/pfp sync on evergreen+ The packet seems to cause hangs on some 7xx asics. bug: https://bugs.freedesktop.org/show_bug.cgi?id=83616 Signed-off-by: Alex Deucher to get UVD going, again. Thanks, Dieter. PS Examination of [ 11.279944] radeon :01:00.0: (-1) pin WB bo failed [ 11.279956] radeon :01:00.0: f6124800 unpin not necessary [ 11.279975] radeon :01:00.0: disabling GPU acceleration [ 11.331227] radeon :01:00.0: f610e000 unpin not necessary is under way - I'm learning GIT...;-)
[drm-next-3.18 (-wip)] only works with b6c2b4 below evergreen (UVD)
Am 12.09.2014 00:14, schrieb Alex Deucher: > On Thu, Sep 11, 2014 at 6:12 PM, Dieter N?tzel > wrote: >> Hello Alex and Christian, >> >> RV730 (AGP) need badly >> >> From b6c2b4faf90230ef9cf1a81f36cbccda4a606c59 Mon Sep 17 00:00:00 2001 >> From: Alex Deucher >> Date: Mon, 8 Sep 2014 13:16:39 -0400 >> Subject: [PATCH] drm/radeon: only use me/pfp sync on evergreen+ >> >> The packet seems to cause hangs on some 7xx asics. >> >> bug: >> https://bugs.freedesktop.org/show_bug.cgi?id=83616 >> >> Signed-off-by: Alex Deucher >> >> to get UVD going, again. > > Dave already pulled it into drm-next: > http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-fixes&id=b6c2b4faf90230ef9cf1a81f36cbccda4a606c59 > > Alex Ah, shit... you're too fast with your nice 'Hobels'... Much appreciated! Dieter >> Thanks, >> Dieter. >> >> PS >> Examination of >> [ 11.279944] radeon :01:00.0: (-1) pin WB bo failed >> [ 11.279956] radeon :01:00.0: f6124800 unpin not necessary >> [ 11.279975] radeon :01:00.0: disabling GPU acceleration >> [ 11.331227] radeon :01:00.0: f610e000 unpin not necessary >> is under way - I'm learning GIT...;-)
[Bug 83742] [radeonsi KMS] Monitors on DP outputs not enabled
https://bugs.freedesktop.org/show_bug.cgi?id=83742 --- Comment #2 from Ralf-Peter Rohbeck --- I can but I don't think it's caused by the kernel driver alone since even older kernels like 3.14 do this now and it used to work in the past with 3.14 through 3.16. Maybe somebody can give me a hint as to what to look for and have a look at the logs. I also want to run a jessie fresh install and see if a fresh start will make a difference. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/7efd5178/attachment.html>
[Bug 75649] Glitchy output using only HDMI on laptop with AMD Mobility Radeon HD 3450/3470
https://bugs.freedesktop.org/show_bug.cgi?id=75649 --- Comment #11 from tderensis at gmail.com --- (In reply to comment #10) > If you are using dpm, make sure your ddx has this patch: > http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/commit/ > ?id=c4ae0e2cbcc0e2ebf9f13ee92d59b5120254a1dc I am using the latest ddx from git. Enabling or disabling dpm using the kernel option radeon.dpm=1 or radeon.dpm=0 does not help. It seems like the more redraws the GPU has to do, the worse things get. For example, sitting at the desktop with nothing moving is fine. Opening chromium, many horizontal lines matching the color of whatever is on the screen begin to flicker on and off. Is it possible that color tiling is the culprit as described in https://bugs.freedesktop.org/show_bug.cgi?id=42621 His symptoms sound identical (only happens on 1920x1080 resolution, smaller resolutions work). -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/32964a6d/attachment.html>
[Bug 75649] Glitchy output using only HDMI on laptop with AMD Mobility Radeon HD 3450/3470
https://bugs.freedesktop.org/show_bug.cgi?id=75649 --- Comment #12 from tderensis at gmail.com --- OK so I disabled color tiling in my xorg.conf and that worked. Disabling color tiling 2D did not work. So it seems to be an issue with 1D color tiling and single 1920x1080 resolution monitor at 60Hz refresh rate. I changed refresh rate to 50Hz using xrandr and that fixed the issue as well (pointed out by Paul previously). -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/b38c2fdc/attachment.html>
[PATCH v5] ASoC: tda998x: Add a codec to the HDMI transmitter
On 10 September 2014 19:29, Jean-Francois Moine wrote: > This patch adds a CODEC function to the NXP TDA998x HDMI transmitter. > > The CODEC handles both I2S and S/PDIF inputs. > It maintains the audio format and rate constraints according > to the HDMI device parameters (EDID) and does dynamic input > switch in the TDA998x I2C driver on start/stop audio streaming. > You should indicate on subsystem spanning patches what tree you think should merge it etc. If other tda998x ppl are okay with it, you can have my ack for merging via someone else. Dave.
[git pull] drm fixes
Hi Linus, ast, i915, radeon and msm fixes, all over the place, all fixing build issues, regressions, oopses or failure to detect cards. Dave. The following changes since commit 7ec62d421bdf29cb31101ae2689f7f3a9906289a: Merge branch 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs (2014-09-10 14:04:17 -0700) are available in the git repository at: git://people.freedesktop.org/~airlied/linux drm-fixes for you to fetch changes up to 83502a5d34386f7c6973bc70e1c423f55f5a2e3a: drm/ast: AST2000 cannot be detected correctly (2014-09-12 13:41:39 +1000) Alex Deucher (3): drm/radeon: only use me/pfp sync on evergreen+ drm/radeon: add connector quirk for fujitsu board drm/radeon/dpm: set the thermal type properly for special configs Andy Shevchenko (1): drm/radeon: reduce memory footprint for debugging Chris Wilson (2): drm/i915: Prevent recursive deadlock on releasing a busy userptr drm/i915: Evict CS TLBs between batches Christian K?nig (1): drm/radeon: fix semaphore value init Daniel Vetter (2): drm/i915: Fix EIO/wedged handling in gem fault handler drm/i915: Fix irq enable tracking in driver load Dave Airlie (3): Merge branch 'drm-fixes-3.17' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-intel-fixes-2014-09-10' of git://anongit.freedesktop.org/drm-intel into drm-fixes Merge branch 'msm-fixes-3.17-rc4' of git://people.freedesktop.org/~robclark/linux into drm-fixes Mark Charlebois (1): drm/msm: Change nested function to static function Rob Clark (2): drm/msm/hdmi: fix build break on non-CCF platforms drm/msm: don't crash if no msm.vram param Ville Syrj?l? (1): drm/i915: Wait for vblank before enabling the TV encoder Y.C. Chen (2): drm/ast: open key before detect chips drm/ast: AST2000 cannot be detected correctly drivers/gpu/drm/ast/ast_main.c| 3 +- drivers/gpu/drm/i915/i915_dma.c | 9 +- drivers/gpu/drm/i915/i915_drv.h | 10 +- drivers/gpu/drm/i915/i915_gem.c | 11 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 409 +- drivers/gpu/drm/i915/i915_reg.h | 12 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 66 +++-- drivers/gpu/drm/i915/intel_tv.c | 4 + drivers/gpu/drm/msm/hdmi/hdmi.c | 46 ++-- drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 15 +- drivers/gpu/drm/msm/msm_drv.c | 2 +- drivers/gpu/drm/radeon/atombios_dp.c | 7 +- drivers/gpu/drm/radeon/r600.c | 4 +- drivers/gpu/drm/radeon/radeon_atombios.c | 33 ++- drivers/gpu/drm/radeon/radeon_semaphore.c | 2 +- 15 files changed, 371 insertions(+), 262 deletions(-)
[PATCH] intel: make bufmgr_gem shareable from different API
On Thu, Sep 11, 2014 at 05:59:40PM +0100, Lionel Landwerlin wrote:
> When using Mesa and LibVA in the same process, one would like to be
> able bind buffers from the output of the decoder to a GL texture
> through an EGLImage.
>
> LibVA can reuse buffers allocated by Gbm through a file descriptor. It
> will then wrap it into a drm_intel_bo with
> drm_intel_bo_gem_create_from_prime().
>
> The problem at the moment is that both library get a different
> drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init()
> even though they're using the same drm file descriptor. As a result,
> instead of manipulating the same buffer object for a given file
> descriptor, they get 2 different drm_intel_bo objects and 2 different
> refcounts, leading one of the library to get errors from the kernel on
> invalid BO when one of the 2 library is done with a shared buffer.
>
> This patch modifies drm_intel_bufmgr_gem_init() so, given a file
> descriptor, it will look for an already existing drm_intel_bufmgr
> using the same file descriptor and return that object.
Almost there!
> +static void
> +drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr)
> +{
> + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
> +
> + if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
> + assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
Consider thread A destroying its bufmgr on fd, whilst thread B is
creating his.
Thread A drops the last reference and so takes the mutex.
But just before it does, thread B leaps in grabs the mutex, searches
through the cache, finds its fd, bumps the refcount and leaves with the
old bufmgr (not before dropping the lock!).
Thread A resumes with the lock and frees the bufmgr now owned by thread
B.
The usual idiom is
static inline void
drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
{
if (obj && !atomic_add_unless(&obj->refcount.refcount, -1, 1)) {
struct drm_device *dev = obj->dev;
mutex_lock(&dev->struct_mutex);
if (likely(atomic_dec_and_test(&obj->refcount.refcount)))
drm_gem_object_free(&obj->refcount);
mutex_unlock(&dev->struct_mutex);
}
}
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH] device coredump: add new device coredump class
From: Johannes Berg
Many devices run firmware and/or complex hardware, and most of that
can have bugs. When it misbehaves, however, it is often much harder
to debug than software running on the host.
Introduce a "device coredump" mechanism to allow dumping internal
device/firmware state through a generalized mechanism. As devices
are different and information needed can vary accordingly, this
doesn't prescribe a file format - it just provides mechanism to
get data to be able to capture it in a generalized way (e.g. in
distributions.)
The dumped data will be readable in sysfs in the virtual device's
data file under /sys/class/devcoredump/devcd*/. Writing to it will
free the data and remove the device, as does a 5-minute timeout.
Note that generalized capturing of such data may result in privacy
issues, so users generally need to be involved. In order to allow
certain users/system integrators/... to disable the feature at all,
introduce a Kconfig option to override the drivers that would like
to have the feature.
For now, this provides two ways of dumping data:
1) with a vmalloc'ed area, that is then given to the subsystem
and freed after retrieval or timeout
2) with a generalized reader/free function method
We could/should add more options, e.g. a list of pages, since the
vmalloc area is very limited on some architectures.
Signed-off-by: Johannes Berg
---
MAINTAINERS | 7 ++
drivers/base/Kconfig| 21
drivers/base/Makefile | 1 +
drivers/base/devcoredump.c | 265
include/linux/devcoredump.h | 35 ++
5 files changed, 329 insertions(+)
create mode 100644 drivers/base/devcoredump.c
create mode 100644 include/linux/devcoredump.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 2f85f55c8fb8..8e31d053fb14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2847,6 +2847,13 @@ T: git
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
S: Maintained
F: drivers/usb/dwc3/
+DEVICE COREDUMP (DEV_COREDUMP)
+M: Johannes Berg
+L: linux-kernel at vger.kernel.org
+S: Maintained
+F: drivers/base/devcoredump.c
+F: include/linux/devcoredump.h
+
DEVICE FREQUENCY (DEVFREQ)
M: MyungJoo Ham
M: Kyungmin Park
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 4e7f0ff83ae7..134f763d90fd 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -165,6 +165,27 @@ config FW_LOADER_USER_HELPER_FALLBACK
If you are unsure about this, say N here.
+config WANT_DEV_COREDUMP
+ bool
+ help
+ Drivers should "select" this option if they desire to use the
+ device coredump mechanism.
+
+config DISABLE_DEV_COREDUMP
+ bool "Disable device coredump" if EXPERT
+ help
+ Disable the device coredump mechanism despite drivers wanting to
+ use it; this allows for more sensitive systems or systems that
+ don't want to ever access the information to not have the code,
+ nor keep any data.
+
+ If unsure, say N.
+
+config DEV_COREDUMP
+ bool
+ default y if WANT_DEV_COREDUMP
+ depends on !DISABLE_DEV_COREDUMP
+
config DEBUG_DRIVER
bool "Driver Core verbose debug messages"
depends on DEBUG_KERNEL
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 4aab26ec0292..6922cd6850a2 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
obj-$(CONFIG_REGMAP) += regmap/
obj-$(CONFIG_SOC_BUS) += soc.o
obj-$(CONFIG_PINCTRL) += pinctrl.o
+obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
new file mode 100644
index ..96614b04544c
--- /dev/null
+++ b/drivers/base/devcoredump.c
@@ -0,0 +1,265 @@
+/*
+ * This file is provided under the GPLv2 license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2014 Intel Mobile Communications GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ *
+ * Contact Information:
+ * Intel Linux Wireless
+ * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+ *
+ * Author: Johannes Berg
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* if data isn't read by userspace after 5 minutes then delete it */
+#define DEVCD_TIMEOUT (HZ * 60 * 5)
+
+struct devcd_entry {
+ struct dev
[PATCH] drm/i915: Fix regression in the sprite plane update split
On Thu, 11 Sep 2014, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> 7e4bf45dbd99a965c7b5d5944c6dc4246f171eb5 introduced the regression.
> We fix it by doing the right assignment of crtc_y
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83747
> Signed-off-by: Gustavo Padovan
Thanks for the quick fix.
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 90bb45f..78044bb 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1080,7 +1080,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
>
> if (state->visible) {
> crtc_x = state->dst.x1;
> - crtc_y = state->dst.x2;
> + crtc_y = state->dst.y1;
> crtc_w = drm_rect_width(&state->dst);
> crtc_h = drm_rect_height(&state->dst);
> src_x = state->src.x1;
> --
> 1.9.3
>
--
Jani Nikula, Intel Open Source Technology Center
[PULL] topic/vblank-rework, take 2
Hi Dave, So updated vblank-rework pull request, now with the polish that Mario requested applied (and reviewed by him). Also with backmerge like you've requested for easier merging. The neat thing this finally allows is to immediately disable the vblank interrupt on the last drm_vblank_put if the hardware has perfectly accurate vblank counter and timestamp readout support. On i915 that required piles of small adjustements from Ville since depending upon the platform and port the vblank happens at different scanout lines. Of course this is fully opt-in and per-device (we need that since gen2 doesn't have a hw vblank counter). Cheers, Daniel The following changes since commit fdcaa1dbb7c6ed419b10fb8cdb5001ab0a00538f: Merge tag 'ipu-3.18' of git://git.pengutronix.de/git/pza/linux into drm-next (2014-09-10 19:43:29 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/vblank-rework-2014-09-12 for you to fetch changes up to 336879b1da97fffc097f77c6d6f818660f2826f0: Merge remote-tracking branch 'airlied/drm-next' into topic/vblank-rework (2014-09-11 14:46:53 +0200) Daniel Vetter (5): drm: Really never disable vblank irqs for offdelay==0 drm: Only update final vblank count when precise ts is available drm: Simplify return value of drm_get_last_vbltimestamp drm: Clarify vblank ts/scanoutpos sampling #defines Merge remote-tracking branch 'airlied/drm-next' into topic/vblank-rework Mario Kleiner (2): drm: Remove drm_vblank_cleanup from drm_vblank_init error path. drm: Use vblank_disable_and_save in drm_vblank_cleanup() Ville Syrj?l? (16): drm: Always reject drm_vblank_get() after drm_vblank_off() drm/i915: Warn if drm_vblank_get() still works after drm_vblank_off() drm: Don't clear vblank timestamps when vblank interrupt is disabled drm: Move drm_update_vblank_count() drm: Have the vblank counter account for the time between vblank irq disable and drm_vblank_off() drm: Avoid random vblank counter jumps if the hardware counter has been reset drm: Reduce the amount of dev->vblank[crtc] in the code drm: Fix deadlock between event_lock and vbl_lock/vblank_time_lock drm: Fix race between drm_vblank_off() and drm_queue_vblank_event() drm: Disable vblank interrupt immediately when drm_vblank_offdelay<0 drm: Add dev->vblank_disable_immediate flag drm/i915: Opt out of vblank disable timer on >gen2 drm: Kick start vblank interrupts at drm_vblank_on() drm/i915: Update scanline_offset only for active crtcs drm: Fix confusing debug message in drm_update_vblank_count() drm: Store the vblank timestamp when adjusting the counter during disable Documentation/DocBook/drm.tmpl| 7 + drivers/gpu/drm/drm_drv.c | 4 +- drivers/gpu/drm/drm_irq.c | 379 +++--- drivers/gpu/drm/i915/i915_irq.c | 10 +- drivers/gpu/drm/i915/intel_display.c | 17 +- drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_pm.c| 2 +- include/drm/drmP.h| 18 +- 9 files changed, 286 insertions(+), 155 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 83792] New: Kernel hangs on boot without nomodeset option
https://bugs.freedesktop.org/show_bug.cgi?id=83792 Priority: medium Bug ID: 83792 Assignee: dri-devel at lists.freedesktop.org Summary: Kernel hangs on boot without nomodeset option Severity: normal Classification: Unclassified OS: All Reporter: wayland at wayland.id.au Hardware: Other Status: NEW Version: XOrg CVS Component: DRM/Radeon Product: DRI Hi all. I have a Radeon FireMV 2400. I'm attempting to upgrade from Fedora 13 (kernel 2.6.33) to Fedora 20. More recent kernels have not worked for me; I recently discovered that if I use the nomodeset option, then I can book, but xorg no longer functions. Since it seems that using the nomodeset option is no longer going to be supported, I need to make this work without the nomodeset option. Unfortunately, the computer hangs (no ping via network) during boot. I tried to give it the netconsole option, but this did not seem to produce any results. What else can I try to help resolve this problem? Thanks, -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/cdcf3e55/attachment.html>
[Intel-gfx] [PATCH] drm/i915: Fix regression in the sprite plane update split
On Fri, Sep 12, 2014 at 10:12:37AM +0300, Jani Nikula wrote: > On Thu, 11 Sep 2014, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > 7e4bf45dbd99a965c7b5d5944c6dc4246f171eb5 introduced the regression. > > We fix it by doing the right assignment of crtc_y > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83747 > > Signed-off-by: Gustavo Padovan > > Thanks for the quick fix. > > Reviewed-by: Jani Nikula Yeah, thanks a lot for the quick fix. Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/ttm: make sure format string cannot leak in
Hi
On Thu, Sep 11, 2014 at 10:53 PM, Kees Cook wrote:
> While zone->name is currently hard coded, the call to kobject_init_and_add()
> should follow the more defensive argument list usage (as already done in
> other places in ttm_memory.c) where "%s" is used instead of directly passing
> in a variable as a format string.
>
> Signed-off-by: Kees Cook
Reviewed-by: David Herrmann
Thanks
David
> ---
> drivers/gpu/drm/ttm/ttm_memory.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
> b/drivers/gpu/drm/ttm/ttm_memory.c
> index fa53df487875..1e688b603e46 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -300,7 +300,8 @@ static int ttm_mem_init_highmem_zone(struct
> ttm_mem_global *glob,
> zone->glob = glob;
> glob->zone_highmem = zone;
> ret = kobject_init_and_add(
> - &zone->kobj, &ttm_mem_zone_kobj_type, &glob->kobj,
> zone->name);
> + &zone->kobj, &ttm_mem_zone_kobj_type, &glob->kobj, "%s",
> + zone->name);
> if (unlikely(ret != 0)) {
> kobject_put(&zone->kobj);
> return ret;
> --
> 1.9.1
>
>
> --
> Kees Cook
> Chrome OS Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
Hi Andrzej, On 2014? 09? 09? 22:16, Andrzej Hajda wrote: > Adding reference to framebuffer should be accompanied with removing > reference to old framebuffer assigned to the plane. > This patch removes following warning: > > [ 95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 > drm_mode_config_cleanup+0x258/0x268() > [ 95.048086] Modules linked in: > [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW > 3.16.0-11355-g7a6eca5-dirty #3015 > [ 95.060058] [] (unwind_backtrace) from [] > (show_stack+0x10/0x14) > [ 95.067766] [] (show_stack) from [] > (dump_stack+0x70/0xbc) > [ 95.074953] [] (dump_stack) from [] > (warn_slowpath_common+0x64/0x88) > [ 95.083005] [] (warn_slowpath_common) from [] > (warn_slowpath_null+0x1c/0x24) > [ 95.091780] [] (warn_slowpath_null) from [] > (drm_mode_config_cleanup+0x258/0x268) > [ 95.100983] [] (drm_mode_config_cleanup) from [] > (exynos_drm_unload+0x38/0x50) > [ 95.109915] [] (exynos_drm_unload) from [] > (drm_dev_unregister+0x24/0x98) > [ 95.118414] [] (drm_dev_unregister) from [] > (drm_put_dev+0x28/0x64) > [ 95.126412] [] (drm_put_dev) from [] > (take_down_master+0x24/0x44) > [ 95.134218] [] (take_down_master) from [] > (component_del+0x8c/0xc8) > [ 95.142201] [] (component_del) from [] > (exynos_dsi_remove+0x18/0x2c) > [ 95.150294] [] (exynos_dsi_remove) from [] > (platform_drv_remove+0x18/0x1c) > [ 95.158872] [] (platform_drv_remove) from [] > (__device_release_driver+0x70/0xc4) > [ 95.167981] [] (__device_release_driver) from [] > (device_release_driver+0x20/0x2c) > [ 95.177268] [] (device_release_driver) from [] > (unbind_store+0x5c/0x94) > [ 95.185597] [] (unbind_store) from [] > (drv_attr_store+0x20/0x2c) > [ 95.193323] [] (drv_attr_store) from [] > (sysfs_kf_write+0x4c/0x50) > [ 95.201224] [] (sysfs_kf_write) from [] > (kernfs_fop_write+0xc4/0x184) > [ 95.209393] [] (kernfs_fop_write) from [] > (vfs_write+0xa0/0x1a8) > [ 95.217111] [] (vfs_write) from [] > (SyS_write+0x40/0x8c) > [ 95.224146] [] (SyS_write) from [] > (ret_fast_syscall+0x0/0x48) > > Signed-off-by: Andrzej Hajda > --- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > index b68e58f..bde19f4 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct > drm_display_mode *mode, > if (ret) > return ret; > > + /* we need to unreference current fb after replacing it with new one */ > + old_fb = plane->fb; > + > plane->crtc = crtc; > plane->fb = crtc->primary->fb; > drm_framebuffer_reference(plane->fb); > > + if (old_fb) > + drm_framebuffer_unreference(old_fb); This time would be a good chance that we can consider drm flip queue to make sure that whole memory region to old_fb is scanned out completely before dropping a reference of old_fb. the reference of old_fb should be dropped at irq handler of each crtc devices, fimd and mixer. Thanks, Inki Dae > + > return 0; > } > >
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: > Hi Andrzej, > > On 2014? 09? 09? 22:16, Andrzej Hajda wrote: > > Adding reference to framebuffer should be accompanied with removing > > reference to old framebuffer assigned to the plane. > > This patch removes following warning: > > > > [ 95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 > > drm_mode_config_cleanup+0x258/0x268() > > [ 95.048086] Modules linked in: > > [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW > > 3.16.0-11355-g7a6eca5-dirty #3015 > > [ 95.060058] [] (unwind_backtrace) from [] > > (show_stack+0x10/0x14) > > [ 95.067766] [] (show_stack) from [] > > (dump_stack+0x70/0xbc) > > [ 95.074953] [] (dump_stack) from [] > > (warn_slowpath_common+0x64/0x88) > > [ 95.083005] [] (warn_slowpath_common) from [] > > (warn_slowpath_null+0x1c/0x24) > > [ 95.091780] [] (warn_slowpath_null) from [] > > (drm_mode_config_cleanup+0x258/0x268) > > [ 95.100983] [] (drm_mode_config_cleanup) from [] > > (exynos_drm_unload+0x38/0x50) > > [ 95.109915] [] (exynos_drm_unload) from [] > > (drm_dev_unregister+0x24/0x98) > > [ 95.118414] [] (drm_dev_unregister) from [] > > (drm_put_dev+0x28/0x64) > > [ 95.126412] [] (drm_put_dev) from [] > > (take_down_master+0x24/0x44) > > [ 95.134218] [] (take_down_master) from [] > > (component_del+0x8c/0xc8) > > [ 95.142201] [] (component_del) from [] > > (exynos_dsi_remove+0x18/0x2c) > > [ 95.150294] [] (exynos_dsi_remove) from [] > > (platform_drv_remove+0x18/0x1c) > > [ 95.158872] [] (platform_drv_remove) from [] > > (__device_release_driver+0x70/0xc4) > > [ 95.167981] [] (__device_release_driver) from [] > > (device_release_driver+0x20/0x2c) > > [ 95.177268] [] (device_release_driver) from [] > > (unbind_store+0x5c/0x94) > > [ 95.185597] [] (unbind_store) from [] > > (drv_attr_store+0x20/0x2c) > > [ 95.193323] [] (drv_attr_store) from [] > > (sysfs_kf_write+0x4c/0x50) > > [ 95.201224] [] (sysfs_kf_write) from [] > > (kernfs_fop_write+0xc4/0x184) > > [ 95.209393] [] (kernfs_fop_write) from [] > > (vfs_write+0xa0/0x1a8) > > [ 95.217111] [] (vfs_write) from [] > > (SyS_write+0x40/0x8c) > > [ 95.224146] [] (SyS_write) from [] > > (ret_fast_syscall+0x0/0x48) > > > > Signed-off-by: Andrzej Hajda > > --- > > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > > b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > > index b68e58f..bde19f4 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > > @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, > > struct drm_display_mode *mode, > > if (ret) > > return ret; > > > > + /* we need to unreference current fb after replacing it with new one */ > > + old_fb = plane->fb; > > + > > plane->crtc = crtc; > > plane->fb = crtc->primary->fb; > > drm_framebuffer_reference(plane->fb); > > > > + if (old_fb) > > + drm_framebuffer_unreference(old_fb); > > This time would be a good chance that we can consider drm flip queue to > make sure that whole memory region to old_fb is scanned out completely > before dropping a reference of old_fb. the reference of old_fb should be > dropped at irq handler of each crtc devices, fimd and mixer. Generally it's not a good idea to drop fb references from irq context, since if you actually drop the last reference it'll blow up: fb cleanup needs a bunch of mutexes. Also the drm core really should be taking care of this for you, you only need to grab references yourself for async flips if you want the buffer to survive a bit. crtc_mode_set has not need for this. I expect that the refcounting bug is somewhere else, at least from my experience chasing such issues in i915 ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
On 2014? 09? 12? 17:57, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: >> Hi Andrzej, >> >> On 2014? 09? 09? 22:16, Andrzej Hajda wrote: >>> Adding reference to framebuffer should be accompanied with removing >>> reference to old framebuffer assigned to the plane. >>> This patch removes following warning: >>> >>> [ 95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 >>> drm_mode_config_cleanup+0x258/0x268() >>> [ 95.048086] Modules linked in: >>> [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW >>> 3.16.0-11355-g7a6eca5-dirty #3015 >>> [ 95.060058] [] (unwind_backtrace) from [] >>> (show_stack+0x10/0x14) >>> [ 95.067766] [] (show_stack) from [] >>> (dump_stack+0x70/0xbc) >>> [ 95.074953] [] (dump_stack) from [] >>> (warn_slowpath_common+0x64/0x88) >>> [ 95.083005] [] (warn_slowpath_common) from [] >>> (warn_slowpath_null+0x1c/0x24) >>> [ 95.091780] [] (warn_slowpath_null) from [] >>> (drm_mode_config_cleanup+0x258/0x268) >>> [ 95.100983] [] (drm_mode_config_cleanup) from [] >>> (exynos_drm_unload+0x38/0x50) >>> [ 95.109915] [] (exynos_drm_unload) from [] >>> (drm_dev_unregister+0x24/0x98) >>> [ 95.118414] [] (drm_dev_unregister) from [] >>> (drm_put_dev+0x28/0x64) >>> [ 95.126412] [] (drm_put_dev) from [] >>> (take_down_master+0x24/0x44) >>> [ 95.134218] [] (take_down_master) from [] >>> (component_del+0x8c/0xc8) >>> [ 95.142201] [] (component_del) from [] >>> (exynos_dsi_remove+0x18/0x2c) >>> [ 95.150294] [] (exynos_dsi_remove) from [] >>> (platform_drv_remove+0x18/0x1c) >>> [ 95.158872] [] (platform_drv_remove) from [] >>> (__device_release_driver+0x70/0xc4) >>> [ 95.167981] [] (__device_release_driver) from [] >>> (device_release_driver+0x20/0x2c) >>> [ 95.177268] [] (device_release_driver) from [] >>> (unbind_store+0x5c/0x94) >>> [ 95.185597] [] (unbind_store) from [] >>> (drv_attr_store+0x20/0x2c) >>> [ 95.193323] [] (drv_attr_store) from [] >>> (sysfs_kf_write+0x4c/0x50) >>> [ 95.201224] [] (sysfs_kf_write) from [] >>> (kernfs_fop_write+0xc4/0x184) >>> [ 95.209393] [] (kernfs_fop_write) from [] >>> (vfs_write+0xa0/0x1a8) >>> [ 95.217111] [] (vfs_write) from [] >>> (SyS_write+0x40/0x8c) >>> [ 95.224146] [] (SyS_write) from [] >>> (ret_fast_syscall+0x0/0x48) >>> >>> Signed-off-by: Andrzej Hajda >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> index b68e58f..bde19f4 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, >>> struct drm_display_mode *mode, >>> if (ret) >>> return ret; >>> >>> + /* we need to unreference current fb after replacing it with new one */ >>> + old_fb = plane->fb; >>> + >>> plane->crtc = crtc; >>> plane->fb = crtc->primary->fb; >>> drm_framebuffer_reference(plane->fb); >>> >>> + if (old_fb) >>> + drm_framebuffer_unreference(old_fb); >> >> This time would be a good chance that we can consider drm flip queue to >> make sure that whole memory region to old_fb is scanned out completely >> before dropping a reference of old_fb. the reference of old_fb should be >> dropped at irq handler of each crtc devices, fimd and mixer. > > Generally it's not a good idea to drop fb references from irq context, > since if you actually drop the last reference it'll blow up: fb cleanup > needs a bunch of mutexes. Actually, drm_flip_work_commit function will be called at irq context so the reference will be dropped by work queue handler so mutex lock would be required before dropping the reference. My concern was that gem memory may be freed before the memory region is scanned out completely so I thought we need to make sure to scan out completely somehow. > > Also the drm core really should be taking care of this for you, you only > need to grab references yourself for async flips if you want the buffer to > survive a bit. crtc_mode_set has not need for this. I expect that the > refcounting bug is somewhere else, at least from my experience chasing > such issues in i915 ;-) Thanks for comments. Yes, there may be refcounting bug somewhere else if drm core makes sure to take care of this. It seems to need more checking. Thanks, Inki Dae > -Daniel >
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
On 09/12/2014 10:57 AM, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: >> Hi Andrzej, >> >> On 2014? 09? 09? 22:16, Andrzej Hajda wrote: >>> Adding reference to framebuffer should be accompanied with removing >>> reference to old framebuffer assigned to the plane. >>> This patch removes following warning: >>> >>> [ 95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 >>> drm_mode_config_cleanup+0x258/0x268() >>> [ 95.048086] Modules linked in: >>> [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW >>> 3.16.0-11355-g7a6eca5-dirty #3015 >>> [ 95.060058] [] (unwind_backtrace) from [] >>> (show_stack+0x10/0x14) >>> [ 95.067766] [] (show_stack) from [] >>> (dump_stack+0x70/0xbc) >>> [ 95.074953] [] (dump_stack) from [] >>> (warn_slowpath_common+0x64/0x88) >>> [ 95.083005] [] (warn_slowpath_common) from [] >>> (warn_slowpath_null+0x1c/0x24) >>> [ 95.091780] [] (warn_slowpath_null) from [] >>> (drm_mode_config_cleanup+0x258/0x268) >>> [ 95.100983] [] (drm_mode_config_cleanup) from [] >>> (exynos_drm_unload+0x38/0x50) >>> [ 95.109915] [] (exynos_drm_unload) from [] >>> (drm_dev_unregister+0x24/0x98) >>> [ 95.118414] [] (drm_dev_unregister) from [] >>> (drm_put_dev+0x28/0x64) >>> [ 95.126412] [] (drm_put_dev) from [] >>> (take_down_master+0x24/0x44) >>> [ 95.134218] [] (take_down_master) from [] >>> (component_del+0x8c/0xc8) >>> [ 95.142201] [] (component_del) from [] >>> (exynos_dsi_remove+0x18/0x2c) >>> [ 95.150294] [] (exynos_dsi_remove) from [] >>> (platform_drv_remove+0x18/0x1c) >>> [ 95.158872] [] (platform_drv_remove) from [] >>> (__device_release_driver+0x70/0xc4) >>> [ 95.167981] [] (__device_release_driver) from [] >>> (device_release_driver+0x20/0x2c) >>> [ 95.177268] [] (device_release_driver) from [] >>> (unbind_store+0x5c/0x94) >>> [ 95.185597] [] (unbind_store) from [] >>> (drv_attr_store+0x20/0x2c) >>> [ 95.193323] [] (drv_attr_store) from [] >>> (sysfs_kf_write+0x4c/0x50) >>> [ 95.201224] [] (sysfs_kf_write) from [] >>> (kernfs_fop_write+0xc4/0x184) >>> [ 95.209393] [] (kernfs_fop_write) from [] >>> (vfs_write+0xa0/0x1a8) >>> [ 95.217111] [] (vfs_write) from [] >>> (SyS_write+0x40/0x8c) >>> [ 95.224146] [] (SyS_write) from [] >>> (ret_fast_syscall+0x0/0x48) >>> >>> Signed-off-by: Andrzej Hajda >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> index b68e58f..bde19f4 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >>> @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, >>> struct drm_display_mode *mode, >>> if (ret) >>> return ret; >>> >>> + /* we need to unreference current fb after replacing it with new one */ >>> + old_fb = plane->fb; >>> + >>> plane->crtc = crtc; >>> plane->fb = crtc->primary->fb; >>> drm_framebuffer_reference(plane->fb); >>> >>> + if (old_fb) >>> + drm_framebuffer_unreference(old_fb); >> This time would be a good chance that we can consider drm flip queue to >> make sure that whole memory region to old_fb is scanned out completely >> before dropping a reference of old_fb. the reference of old_fb should be >> dropped at irq handler of each crtc devices, fimd and mixer. > Generally it's not a good idea to drop fb references from irq context, > since if you actually drop the last reference it'll blow up: fb cleanup > needs a bunch of mutexes. I agree with that. > > Also the drm core really should be taking care of this for you, you only > need to grab references yourself for async flips if you want the buffer to > survive a bit. crtc_mode_set has not need for this. I expect that the > refcounting bug is somewhere else, at least from my experience chasing > such issues in i915 ;-) Hmm, maybe I miss something but I do not see the core grabbing fb reference on plane->fb update. On the other side drm_framebuffer_remove calls drm_plane_force_disable which drops plane->fb reference. I am not yet familiar with this code so maybe there is better solution. If not I guess it would be better to move this code to exynos_plane_mode_set. At least it is done this way in omap and msm, in fact it seems better place for such things. What do you think? Regards Andrzej > -Daniel
[Bug 82828] Regression: Crash in 3Dmark2001
https://bugs.freedesktop.org/show_bug.cgi?id=82828 Fabio Pedretti changed: What|Removed |Added Severity|normal |blocker Priority|medium |high --- Comment #22 from Fabio Pedretti --- Can someone push Connor patches and backport the fix in time for 10.3? r300 is seriously broken without this fix, with many apps crashing, and it would be nice to have it fixed in time for 10.3. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/c96f585a/attachment-0001.html>
[Bug 73841] Audio broken on HDMI (ATI)
https://bugzilla.kernel.org/show_bug.cgi?id=73841 Jani Nikula changed: What|Removed |Added Status|NEEDINFO|ASSIGNED Component|Video(DRI - Intel) |Video(DRI - non Intel) Assignee|rodrigo.vivi at gmail.com |drivers_video-dri at kernel-bu ||gs.osdl.org --- Comment #10 from Jani Nikula --- Oops, we've totally failed to notice the HDMI is connected to the discrete GPU. Reassigning. -- You are receiving this mail because: You are watching the assignee of the bug.
Shareable bufmgr objects v3
Hopefully I got this right this time :) - Lionel
[PATCH] intel: make bufmgr_gem shareable from different API
When using Mesa and LibVA in the same process, one would like to be
able bind buffers from the output of the decoder to a GL texture
through an EGLImage.
LibVA can reuse buffers allocated by Gbm through a file descriptor. It
will then wrap it into a drm_intel_bo with
drm_intel_bo_gem_create_from_prime().
The problem at the moment is that both library get a different
drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init()
even though they're using the same drm file descriptor. As a result,
instead of manipulating the same buffer object for a given file
descriptor, they get 2 different drm_intel_bo objects and 2 different
refcounts, leading one of the library to get errors from the kernel on
invalid BO when one of the 2 library is done with a shared buffer.
This patch modifies drm_intel_bufmgr_gem_init() so, given a file
descriptor, it will look for an already existing drm_intel_bufmgr
using the same file descriptor and return that object.
Signed-off-by: Lionel Landwerlin
---
intel/intel_bufmgr_gem.c | 63
1 file changed, 58 insertions(+), 5 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 0e1cb0d..73f46a1 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
typedef struct _drm_intel_bufmgr_gem {
drm_intel_bufmgr bufmgr;
+ atomic_t refcount;
+
int fd;
int max_relocs;
@@ -111,6 +113,8 @@ typedef struct _drm_intel_bufmgr_gem {
int num_buckets;
time_t time;
+ drmMMListHead managers;
+
drmMMListHead named;
drmMMListHead vma_cache;
int vma_count, vma_open, vma_max;
@@ -3186,6 +3190,41 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
*bo,
bo_gem->aub_annotation_count = count;
}
+static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+static drmMMListHead bufmgr_list = { &bufmgr_list, &bufmgr_list };
+
+static drm_intel_bufmgr_gem *
+drm_intel_bufmgr_gem_find(int fd)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem;
+
+ DRMLISTFOREACHENTRY(bufmgr_gem, &bufmgr_list, managers) {
+ if (bufmgr_gem->fd == fd) {
+ atomic_inc(&bufmgr_gem->refcount);
+ return bufmgr_gem;
+ }
+ }
+
+ return NULL;
+}
+
+static void
+drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
+
+ if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
+ assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
+
+ if (atomic_read(&bufmgr_gem->refcount)) {
+ DRMLISTDEL(&bufmgr_gem->managers);
+ drm_intel_bufmgr_gem_destroy(bufmgr);
+ }
+
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+ }
+}
+
/**
* Initializes the GEM buffer manager, which uses the kernel to allocate, map,
* and manage map buffer objections.
@@ -3201,15 +3240,23 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
int ret, tmp;
bool exec2 = false;
+ pthread_mutex_lock(&bufmgr_list_mutex);
+
+ bufmgr_gem = drm_intel_bufmgr_gem_find(fd);
+ if (bufmgr_gem)
+ goto exit;
+
bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
if (bufmgr_gem == NULL)
- return NULL;
+ goto exit;
bufmgr_gem->fd = fd;
+ atomic_set(&bufmgr_gem->refcount, 1);
if (pthread_mutex_init(&bufmgr_gem->lock, NULL) != 0) {
free(bufmgr_gem);
- return NULL;
+ bufmgr_gem = NULL;
+ goto exit;
}
ret = drmIoctl(bufmgr_gem->fd,
@@ -3246,7 +3293,8 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->gen = 8;
else {
free(bufmgr_gem);
- return NULL;
+ bufmgr_gem = NULL;
+ goto exit;
}
if (IS_GEN3(bufmgr_gem->pci_device) &&
@@ -3357,7 +3405,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->bufmgr.bo_exec = drm_intel_gem_bo_exec;
bufmgr_gem->bufmgr.bo_busy = drm_intel_gem_bo_busy;
bufmgr_gem->bufmgr.bo_madvise = drm_intel_gem_bo_madvise;
- bufmgr_gem->bufmgr.destroy = drm_intel_bufmgr_gem_destroy;
+ bufmgr_gem->bufmgr.destroy = drm_intel_bufmgr_gem_unref;
bufmgr_gem->bufmgr.debug = 0;
bufmgr_gem->bufmgr.check_aperture_space =
drm_intel_gem_check_aperture_space;
@@ -3373,5 +3421,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
DRMINITLISTHEAD(&bufmgr_gem->vma_cache);
bufmgr_gem->vma_max = -1; /* unlimited by default */
- return &bufmgr_gem->bufmgr;
+ DRMLISTADD(&bufmgr_gem->managers, &bufmgr_list);
+
+exit:
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+
+ return bufmgr_gem
[Bug 73841] Audio broken on HDMI (ATI)
https://bugzilla.kernel.org/show_bug.cgi?id=73841 Christian K?nig changed: What|Removed |Added CC||deathsimple at vodafone.de --- Comment #11 from Christian K?nig --- (In reply to Jani Nikula from comment #10) > Oops, we've totally failed to notice the HDMI is connected to the discrete > GPU. Reassigning. But audio now works for the user again: > audio now works (3.14.1-1) but i still get the error. What he still complains about is: "drm:I915_stolen_to_physical" If that isn't critical or already fixed I think we can close the bug now. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] intel: make bufmgr_gem shareable from different API
On Fri, Sep 12, 2014 at 11:27:07AM +0100, Lionel Landwerlin wrote:
> When using Mesa and LibVA in the same process, one would like to be
> able bind buffers from the output of the decoder to a GL texture
> through an EGLImage.
>
> LibVA can reuse buffers allocated by Gbm through a file descriptor. It
> will then wrap it into a drm_intel_bo with
> drm_intel_bo_gem_create_from_prime().
>
> The problem at the moment is that both library get a different
> drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init()
> even though they're using the same drm file descriptor. As a result,
> instead of manipulating the same buffer object for a given file
> descriptor, they get 2 different drm_intel_bo objects and 2 different
> refcounts, leading one of the library to get errors from the kernel on
> invalid BO when one of the 2 library is done with a shared buffer.
>
> This patch modifies drm_intel_bufmgr_gem_init() so, given a file
> descriptor, it will look for an already existing drm_intel_bufmgr
> using the same file descriptor and return that object.
>
> Signed-off-by: Lionel Landwerlin
So close.
> +static void
> +drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr)
> +{
> + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
> +
> + if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
> + assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
(Oh, don't use assert for expressions with side-effects)
> +
> + if (atomic_read(&bufmgr_gem->refcount)) {
As a thought exercise, now consider what happens if thread B grabs a
reference to this bufmgr and frees it all before this thread wakes up?
Double free, kaboom.
This does have to be an atomic_add_unless, which does not yet exist in
libdrm:
static inline int atomic_add_unless(atomic_t *v, int add, int unless)
{
int c, old;
c = atomic_read(v);
while (c != unless && (old = atomic_cmpxchg(v, c, c + add)) != c)
c = old;
return c == unless;
}
static void
drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr)
{
drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
if (atomic_add_unless(&bufmgr_gem->refcount, -1, 1)) {
pthread_mutex_lock(&bufmgr_list_mutex);
if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
free stuff;
}
pthread_mutex_unlock(&bufmgr_list_mutex);
}
}
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
On 2014? 09? 12? 18:27, Andrzej Hajda wrote: > On 09/12/2014 10:57 AM, Daniel Vetter wrote: >> On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: >>> Hi Andrzej, >>> >>> On 2014? 09? 09? 22:16, Andrzej Hajda wrote: Adding reference to framebuffer should be accompanied with removing reference to old framebuffer assigned to the plane. This patch removes following warning: [ 95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 drm_mode_config_cleanup+0x258/0x268() [ 95.048086] Modules linked in: [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW 3.16.0-11355-g7a6eca5-dirty #3015 [ 95.060058] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 95.067766] [] (show_stack) from [] (dump_stack+0x70/0xbc) [ 95.074953] [] (dump_stack) from [] (warn_slowpath_common+0x64/0x88) [ 95.083005] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24) [ 95.091780] [] (warn_slowpath_null) from [] (drm_mode_config_cleanup+0x258/0x268) [ 95.100983] [] (drm_mode_config_cleanup) from [] (exynos_drm_unload+0x38/0x50) [ 95.109915] [] (exynos_drm_unload) from [] (drm_dev_unregister+0x24/0x98) [ 95.118414] [] (drm_dev_unregister) from [] (drm_put_dev+0x28/0x64) [ 95.126412] [] (drm_put_dev) from [] (take_down_master+0x24/0x44) [ 95.134218] [] (take_down_master) from [] (component_del+0x8c/0xc8) [ 95.142201] [] (component_del) from [] (exynos_dsi_remove+0x18/0x2c) [ 95.150294] [] (exynos_dsi_remove) from [] (platform_drv_remove+0x18/0x1c) [ 95.158872] [] (platform_drv_remove) from [] (__device_release_driver+0x70/0xc4) [ 95.167981] [] (__device_release_driver) from [] (device_release_driver+0x20/0x2c) [ 95.177268] [] (device_release_driver) from [] (unbind_store+0x5c/0x94) [ 95.185597] [] (unbind_store) from [] (drv_attr_store+0x20/0x2c) [ 95.193323] [] (drv_attr_store) from [] (sysfs_kf_write+0x4c/0x50) [ 95.201224] [] (sysfs_kf_write) from [] (kernfs_fop_write+0xc4/0x184) [ 95.209393] [] (kernfs_fop_write) from [] (vfs_write+0xa0/0x1a8) [ 95.217111] [] (vfs_write) from [] (SyS_write+0x40/0x8c) [ 95.224146] [] (SyS_write) from [] (ret_fast_syscall+0x0/0x48) Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index b68e58f..bde19f4 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, if (ret) return ret; + /* we need to unreference current fb after replacing it with new one */ + old_fb = plane->fb; + plane->crtc = crtc; plane->fb = crtc->primary->fb; drm_framebuffer_reference(plane->fb); + if (old_fb) + drm_framebuffer_unreference(old_fb); >>> This time would be a good chance that we can consider drm flip queue to >>> make sure that whole memory region to old_fb is scanned out completely >>> before dropping a reference of old_fb. the reference of old_fb should be >>> dropped at irq handler of each crtc devices, fimd and mixer. >> Generally it's not a good idea to drop fb references from irq context, >> since if you actually drop the last reference it'll blow up: fb cleanup >> needs a bunch of mutexes. > > I agree with that. > >> >> Also the drm core really should be taking care of this for you, you only >> need to grab references yourself for async flips if you want the buffer to >> survive a bit. crtc_mode_set has not need for this. I expect that the >> refcounting bug is somewhere else, at least from my experience chasing >> such issues in i915 ;-) > > Hmm, maybe I miss something but I do not see the core grabbing fb reference > on plane->fb update. On the other side drm_framebuffer_remove calls > drm_plane_force_disable which drops plane->fb reference. > I am not yet familiar with this code so maybe there is better solution. > > If not I guess it would be better to move this code to > exynos_plane_mode_set. > At least it is done this way in omap and msm, in fact it seems better place I cannot see it in msm. In case of msm, drm flip queue is used in mdp4/5_crtc_mode_set function. See update_fb function there. > for such things. What do you think? > > Regards > Andrzej > >> -Daniel > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordo
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
On 09/12/2014 12:45 PM, Inki Dae wrote: > On 2014? 09? 12? 18:27, Andrzej Hajda wrote: >> On 09/12/2014 10:57 AM, Daniel Vetter wrote: >>> On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: Hi Andrzej, On 2014? 09? 09? 22:16, Andrzej Hajda wrote: > Adding reference to framebuffer should be accompanied with removing > reference to old framebuffer assigned to the plane. > This patch removes following warning: > > [ 95.038017] WARNING: CPU: 1 PID: 3067 at > drivers/gpu/drm/drm_crtc.c:5115 drm_mode_config_cleanup+0x258/0x268() > [ 95.048086] Modules linked in: > [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW > 3.16.0-11355-g7a6eca5-dirty #3015 > [ 95.060058] [] (unwind_backtrace) from [] > (show_stack+0x10/0x14) > [ 95.067766] [] (show_stack) from [] > (dump_stack+0x70/0xbc) > [ 95.074953] [] (dump_stack) from [] > (warn_slowpath_common+0x64/0x88) > [ 95.083005] [] (warn_slowpath_common) from [] > (warn_slowpath_null+0x1c/0x24) > [ 95.091780] [] (warn_slowpath_null) from [] > (drm_mode_config_cleanup+0x258/0x268) > [ 95.100983] [] (drm_mode_config_cleanup) from [] > (exynos_drm_unload+0x38/0x50) > [ 95.109915] [] (exynos_drm_unload) from [] > (drm_dev_unregister+0x24/0x98) > [ 95.118414] [] (drm_dev_unregister) from [] > (drm_put_dev+0x28/0x64) > [ 95.126412] [] (drm_put_dev) from [] > (take_down_master+0x24/0x44) > [ 95.134218] [] (take_down_master) from [] > (component_del+0x8c/0xc8) > [ 95.142201] [] (component_del) from [] > (exynos_dsi_remove+0x18/0x2c) > [ 95.150294] [] (exynos_dsi_remove) from [] > (platform_drv_remove+0x18/0x1c) > [ 95.158872] [] (platform_drv_remove) from [] > (__device_release_driver+0x70/0xc4) > [ 95.167981] [] (__device_release_driver) from [] > (device_release_driver+0x20/0x2c) > [ 95.177268] [] (device_release_driver) from [] > (unbind_store+0x5c/0x94) > [ 95.185597] [] (unbind_store) from [] > (drv_attr_store+0x20/0x2c) > [ 95.193323] [] (drv_attr_store) from [] > (sysfs_kf_write+0x4c/0x50) > [ 95.201224] [] (sysfs_kf_write) from [] > (kernfs_fop_write+0xc4/0x184) > [ 95.209393] [] (kernfs_fop_write) from [] > (vfs_write+0xa0/0x1a8) > [ 95.217111] [] (vfs_write) from [] > (SyS_write+0x40/0x8c) > [ 95.224146] [] (SyS_write) from [] > (ret_fast_syscall+0x0/0x48) > > Signed-off-by: Andrzej Hajda > --- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > index b68e58f..bde19f4 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, > struct drm_display_mode *mode, > if (ret) > return ret; > > + /* we need to unreference current fb after replacing it with new one */ > + old_fb = plane->fb; > + > plane->crtc = crtc; > plane->fb = crtc->primary->fb; > drm_framebuffer_reference(plane->fb); > > + if (old_fb) > + drm_framebuffer_unreference(old_fb); This time would be a good chance that we can consider drm flip queue to make sure that whole memory region to old_fb is scanned out completely before dropping a reference of old_fb. the reference of old_fb should be dropped at irq handler of each crtc devices, fimd and mixer. >>> Generally it's not a good idea to drop fb references from irq context, >>> since if you actually drop the last reference it'll blow up: fb cleanup >>> needs a bunch of mutexes. >> I agree with that. >> >>> Also the drm core really should be taking care of this for you, you only >>> need to grab references yourself for async flips if you want the buffer to >>> survive a bit. crtc_mode_set has not need for this. I expect that the >>> refcounting bug is somewhere else, at least from my experience chasing >>> such issues in i915 ;-) >> Hmm, maybe I miss something but I do not see the core grabbing fb reference >> on plane->fb update. On the other side drm_framebuffer_remove calls >> drm_plane_force_disable which drops plane->fb reference. >> I am not yet familiar with this code so maybe there is better solution. >> >> If not I guess it would be better to move this code to >> exynos_plane_mode_set. >> At least it is done this way in omap and msm, in fact it seems better place > I cannot see it in msm. In case of msm, drm flip queue is used in > mdp4/5_crtc_mode_set function. See update_fb function there. fb reference dance for planes is in mdp*_plane_update. Regarding drm flip in msm old fb unreferencing is done via drm_flip_work_queue. Regards Andrzej
[Bug 73841] Audio broken on HDMI (ATI)
https://bugzilla.kernel.org/show_bug.cgi?id=73841 Jani Nikula changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |CODE_FIX --- Comment #12 from Jani Nikula --- (In reply to Christian K?nig from comment #11) > What he still complains about is: "drm:I915_stolen_to_physical" If that > isn't critical or already fixed I think we can close the bug now. Right. Sorry for the noise. Thanks for the report anyway. Please file a new bug against DRM/Intel at https://bugs.freedesktop.org/enter_bug.cgi?product=DRI if the problem with drm:I915_stolen_to_physical persists with recent kernels. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
On 2014? 09? 12? 20:04, Andrzej Hajda wrote: > On 09/12/2014 12:45 PM, Inki Dae wrote: >> On 2014? 09? 12? 18:27, Andrzej Hajda wrote: >>> On 09/12/2014 10:57 AM, Daniel Vetter wrote: On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: > Hi Andrzej, > > On 2014? 09? 09? 22:16, Andrzej Hajda wrote: >> Adding reference to framebuffer should be accompanied with removing >> reference to old framebuffer assigned to the plane. >> This patch removes following warning: >> >> [ 95.038017] WARNING: CPU: 1 PID: 3067 at >> drivers/gpu/drm/drm_crtc.c:5115 drm_mode_config_cleanup+0x258/0x268() >> [ 95.048086] Modules linked in: >> [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW >> 3.16.0-11355-g7a6eca5-dirty #3015 >> [ 95.060058] [] (unwind_backtrace) from [] >> (show_stack+0x10/0x14) >> [ 95.067766] [] (show_stack) from [] >> (dump_stack+0x70/0xbc) >> [ 95.074953] [] (dump_stack) from [] >> (warn_slowpath_common+0x64/0x88) >> [ 95.083005] [] (warn_slowpath_common) from [] >> (warn_slowpath_null+0x1c/0x24) >> [ 95.091780] [] (warn_slowpath_null) from [] >> (drm_mode_config_cleanup+0x258/0x268) >> [ 95.100983] [] (drm_mode_config_cleanup) from [] >> (exynos_drm_unload+0x38/0x50) >> [ 95.109915] [] (exynos_drm_unload) from [] >> (drm_dev_unregister+0x24/0x98) >> [ 95.118414] [] (drm_dev_unregister) from [] >> (drm_put_dev+0x28/0x64) >> [ 95.126412] [] (drm_put_dev) from [] >> (take_down_master+0x24/0x44) >> [ 95.134218] [] (take_down_master) from [] >> (component_del+0x8c/0xc8) >> [ 95.142201] [] (component_del) from [] >> (exynos_dsi_remove+0x18/0x2c) >> [ 95.150294] [] (exynos_dsi_remove) from [] >> (platform_drv_remove+0x18/0x1c) >> [ 95.158872] [] (platform_drv_remove) from [] >> (__device_release_driver+0x70/0xc4) >> [ 95.167981] [] (__device_release_driver) from [] >> (device_release_driver+0x20/0x2c) >> [ 95.177268] [] (device_release_driver) from [] >> (unbind_store+0x5c/0x94) >> [ 95.185597] [] (unbind_store) from [] >> (drv_attr_store+0x20/0x2c) >> [ 95.193323] [] (drv_attr_store) from [] >> (sysfs_kf_write+0x4c/0x50) >> [ 95.201224] [] (sysfs_kf_write) from [] >> (kernfs_fop_write+0xc4/0x184) >> [ 95.209393] [] (kernfs_fop_write) from [] >> (vfs_write+0xa0/0x1a8) >> [ 95.217111] [] (vfs_write) from [] >> (SyS_write+0x40/0x8c) >> [ 95.224146] [] (SyS_write) from [] >> (ret_fast_syscall+0x0/0x48) >> >> Signed-off-by: Andrzej Hajda >> --- >> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> index b68e58f..bde19f4 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, >> struct drm_display_mode *mode, >> if (ret) >> return ret; >> >> +/* we need to unreference current fb after replacing it with >> new one */ >> +old_fb = plane->fb; >> + >> plane->crtc = crtc; >> plane->fb = crtc->primary->fb; >> drm_framebuffer_reference(plane->fb); >> >> +if (old_fb) >> +drm_framebuffer_unreference(old_fb); > This time would be a good chance that we can consider drm flip queue to > make sure that whole memory region to old_fb is scanned out completely > before dropping a reference of old_fb. the reference of old_fb should be > dropped at irq handler of each crtc devices, fimd and mixer. Generally it's not a good idea to drop fb references from irq context, since if you actually drop the last reference it'll blow up: fb cleanup needs a bunch of mutexes. >>> I agree with that. >>> Also the drm core really should be taking care of this for you, you only need to grab references yourself for async flips if you want the buffer to survive a bit. crtc_mode_set has not need for this. I expect that the refcounting bug is somewhere else, at least from my experience chasing such issues in i915 ;-) >>> Hmm, maybe I miss something but I do not see the core grabbing fb reference >>> on plane->fb update. On the other side drm_framebuffer_remove calls >>> drm_plane_force_disable which drops plane->fb reference. >>> I am not yet familiar with this code so maybe there is better solution. >>> >>> If not I guess it would be better to move this code to >>> exynos_plane_mode_set. >>> At least it is done this way in omap and msm, in fact it seems better place >> I cannot see it in msm. In case of msm, drm flip queue is u
[PATCH 1/2 v2] vgaarb: Don't default exclusively to first video device with mem+io
Bjorn, What is missing to get these two patches pushed to Linus? Bruno On Thu, 28 Aug 2014 22:47:50 +0200 Bruno Pr?mont wrote: > On Tue, 26 August 2014 Andreas Noever wrote: > > On Sun, Aug 24, 2014 at 11:09 PM, Bruno Pr?mont wrote: > > > With commit 20cde694027e boot video device detection was moved from > > > efifb to x86 and ia64 pci/fixup.c. > > > > > > For dual-GPU Apple computers above change represents a regression as > > > code in efifb did forcefully override vga_default_device while the > > > merge did not (vgaarb happens prior to PCI fixup). > > > > > > To improve on initial device selection by vgaarb (it cannot know if > > > PCI device not behind bridges see/decode legacy VGA I/O or not), move > > > the screen_info based check from pci_video_fixup to vgaarb's init > > > function and use it to refine/override decision taken while adding > > > the individual PCI VGA devices. > > > This way PCI fixup has no reason to adjust vga_default_device > > > anymore but can depend on its value for flagging shadowed VBIOS. > > > > > > This has the nice benefit of removing duplicated code but does > > > introduce a #if defined() block in vgaarb. > > > Not all architectures have screen_info and would cause compile to > > > fail without it. > > > > > > Reported-By: Andreas Noever > > > CC: Matthew Garrett > > > CC: stable at vger.kernel.org # v3.5+ > > > Signed-off-by: Bruno Pr?mont > > > --- > > > Andreas, does this work properly for you, including the improvement > > > on i915 complaint about VBIOS going from KERN_ERR to KERN_INFO? > > Yep, thanks! > > > > > > > > Other arches using PCI and vgaarb that have screen_info may want > > > to be added to the #if defined() block or even introduce a new > > > CONFIG_HAVE_SCREEN_INFO or similar... > > Bjorn, can you queue these two patches, probably going through -next > for a week and passing them to Linus for -rc4, adding Andreas's Tested-by > to Patch 1/2 v2? > > Thanks, > Bruno
[PATCH 03/11] drm/radeon: remove unnecessary VM syncs
From: Christian K?nig
The PD/PTs reservation object now contains everything needed.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon_vm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
b/drivers/gpu/drm/radeon/radeon_vm.c
index b53576e..f684c18 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -700,7 +700,6 @@ int radeon_vm_update_page_directory(struct radeon_device
*rdev,
radeon_asic_vm_pad_ib(rdev, &ib);
radeon_semaphore_sync_resv(ib.semaphore, pd->tbo.resv, false);
- radeon_semaphore_sync_fence(ib.semaphore, vm->last_id_use);
WARN_ON(ib.length_dw > ndw);
r = radeon_ib_schedule(rdev, &ib, NULL, false);
if (r) {
@@ -968,7 +967,6 @@ int radeon_vm_bo_update(struct radeon_device *rdev,
radeon_asic_vm_pad_ib(rdev, &ib);
WARN_ON(ib.length_dw > ndw);
- radeon_semaphore_sync_fence(ib.semaphore, vm->fence);
r = radeon_ib_schedule(rdev, &ib, NULL, false);
if (r) {
radeon_ib_free(rdev, &ib);
--
1.9.1
[PATCH 02/11] drm/radeon: stop re-reserving the BO in radeon_vm_bo_set_addr
From: Christian K?nig
That's useless when all callers drop the reservation
immediately after calling the function.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon_gem.c | 2 +-
drivers/gpu/drm/radeon/radeon_kms.c | 2 --
drivers/gpu/drm/radeon/radeon_vm.c | 4 ++--
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
b/drivers/gpu/drm/radeon/radeon_gem.c
index 4b7c8ec..cbd7244 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -601,6 +601,7 @@ int radeon_gem_va_ioctl(struct drm_device *dev, void *data,
if (bo_va->it.start) {
args->operation = RADEON_VA_RESULT_VA_EXIST;
args->offset = bo_va->it.start * RADEON_GPU_PAGE_SIZE;
+ radeon_bo_unreserve(rbo);
goto out;
}
r = radeon_vm_bo_set_addr(rdev, bo_va, args->offset,
args->flags);
@@ -616,7 +617,6 @@ int radeon_gem_va_ioctl(struct drm_device *dev, void *data,
args->operation = RADEON_VA_RESULT_ERROR;
}
out:
- radeon_bo_unreserve(rbo);
drm_gem_object_unreference_unlocked(gobj);
return r;
}
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
b/drivers/gpu/drm/radeon/radeon_kms.c
index 8309b11..85ee6f7 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -621,8 +621,6 @@ int radeon_driver_open_kms(struct drm_device *dev, struct
drm_file *file_priv)
RADEON_VA_IB_OFFSET,
RADEON_VM_PAGE_READABLE |
RADEON_VM_PAGE_SNOOPED);
-
- radeon_bo_unreserve(rdev->ring_tmp_bo.bo);
if (r) {
radeon_vm_fini(rdev, vm);
kfree(fpriv);
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
b/drivers/gpu/drm/radeon/radeon_vm.c
index 29f2f5e..b53576e 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -450,7 +450,7 @@ error:
* Validate and set the offset requested within the vm address space.
* Returns 0 for success, error for failure.
*
- * Object has to be reserved!
+ * Object has to be reserved and gets unreserved by this function!
*/
int radeon_vm_bo_set_addr(struct radeon_device *rdev,
struct radeon_bo_va *bo_va,
@@ -575,7 +575,7 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
}
mutex_unlock(&vm->mutex);
- return radeon_bo_reserve(bo_va->bo, false);
+ return 0;
}
/**
--
1.9.1
drm/radeon: allow concurrent VM use
Similar to the already pushed patches for allowing concurrent buffers use from different engines this set of patches allows concurrent VM use from different engines at the same time. Please review and comment, Christian.
[PATCH 01/11] drm/radeon: rework vm_flush parameters
From: Christian K?nig
Use ring structure instead of index and provide vm_id and pd_addr separately.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/cik.c | 23 ++-
drivers/gpu/drm/radeon/cik_sdma.c| 22 +-
drivers/gpu/drm/radeon/ni.c | 14 +-
drivers/gpu/drm/radeon/ni_dma.c | 14 +-
drivers/gpu/drm/radeon/radeon.h | 5 +++--
drivers/gpu/drm/radeon/radeon_asic.h | 18 --
drivers/gpu/drm/radeon/radeon_vm.c | 3 ++-
drivers/gpu/drm/radeon/si.c | 18 +++---
drivers/gpu/drm/radeon/si_dma.c | 19 ---
9 files changed, 61 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 0b5a230..16a861d 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -5953,26 +5953,23 @@ static void cik_vm_decode_fault(struct radeon_device
*rdev,
* Update the page table base and flush the VM TLB
* using the CP (CIK).
*/
-void cik_vm_flush(struct radeon_device *rdev, int ridx, struct radeon_vm *vm)
+void cik_vm_flush(struct radeon_device *rdev, struct radeon_ring *ring,
+ unsigned vm_id, uint64_t pd_addr)
{
- struct radeon_ring *ring = &rdev->ring[ridx];
- int usepfp = (ridx == RADEON_RING_TYPE_GFX_INDEX);
-
- if (vm == NULL)
- return;
+ int usepfp = (ring->idx == RADEON_RING_TYPE_GFX_INDEX);
radeon_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
radeon_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
WRITE_DATA_DST_SEL(0)));
- if (vm->id < 8) {
+ if (vm_id < 8) {
radeon_ring_write(ring,
- (VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (vm->id
<< 2)) >> 2);
+ (VM_CONTEXT0_PAGE_TABLE_BASE_ADDR + (vm_id <<
2)) >> 2);
} else {
radeon_ring_write(ring,
- (VM_CONTEXT8_PAGE_TABLE_BASE_ADDR + ((vm->id
- 8) << 2)) >> 2);
+ (VM_CONTEXT8_PAGE_TABLE_BASE_ADDR + ((vm_id -
8) << 2)) >> 2);
}
radeon_ring_write(ring, 0);
- radeon_ring_write(ring, vm->pd_gpu_addr >> 12);
+ radeon_ring_write(ring, pd_addr >> 12);
/* update SH_MEM_* regs */
radeon_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
@@ -5980,7 +5977,7 @@ void cik_vm_flush(struct radeon_device *rdev, int ridx,
struct radeon_vm *vm)
WRITE_DATA_DST_SEL(0)));
radeon_ring_write(ring, SRBM_GFX_CNTL >> 2);
radeon_ring_write(ring, 0);
- radeon_ring_write(ring, VMID(vm->id));
+ radeon_ring_write(ring, VMID(vm_id));
radeon_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 6));
radeon_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
@@ -6001,7 +5998,7 @@ void cik_vm_flush(struct radeon_device *rdev, int ridx,
struct radeon_vm *vm)
radeon_ring_write(ring, VMID(0));
/* HDP flush */
- cik_hdp_flush_cp_ring_emit(rdev, ridx);
+ cik_hdp_flush_cp_ring_emit(rdev, ring->idx);
/* bits 0-15 are the VM contexts0-15 */
radeon_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
@@ -6009,7 +6006,7 @@ void cik_vm_flush(struct radeon_device *rdev, int ridx,
struct radeon_vm *vm)
WRITE_DATA_DST_SEL(0)));
radeon_ring_write(ring, VM_INVALIDATE_REQUEST >> 2);
radeon_ring_write(ring, 0);
- radeon_ring_write(ring, 1 << vm->id);
+ radeon_ring_write(ring, 1 << vm_id);
/* compute doesn't have PFP */
if (usepfp) {
diff --git a/drivers/gpu/drm/radeon/cik_sdma.c
b/drivers/gpu/drm/radeon/cik_sdma.c
index c01a610..dd73246 100644
--- a/drivers/gpu/drm/radeon/cik_sdma.c
+++ b/drivers/gpu/drm/radeon/cik_sdma.c
@@ -905,25 +905,21 @@ void cik_sdma_vm_pad_ib(struct radeon_ib *ib)
* Update the page table base and flush the VM TLB
* using sDMA (CIK).
*/
-void cik_dma_vm_flush(struct radeon_device *rdev, int ridx, struct radeon_vm
*vm)
+void cik_dma_vm_flush(struct radeon_device *rdev, struct radeon_ring *ring,
+ unsigned vm_id, uint64_t pd_addr)
{
- struct radeon_ring *ring = &rdev->ring[ridx];
-
- if (vm == NULL)
- return;
-
radeon_ring_write(ring, SDMA_PACKET(SDMA_OPCODE_SRBM_WRITE, 0, 0xf000));
- if (vm->id < 8) {
- radeon_ring_write(ring, (VM_CONTEXT0_PAGE_TABLE_BASE_ADDR +
(vm->id << 2)) >> 2);
+ if (vm_id < 8) {
+ radeon_ring_write(ring, (VM_CONTEXT0_PAGE_TABLE_BASE_ADDR +
(vm_id << 2)) >> 2);
} else {
- radeon_ring_write(ring, (VM_CONTEXT8_PAGE_TABLE_BASE_ADDR +
((vm->id - 8) << 2)) >> 2);
+ radeon_ring_write(ring, (VM_CONTEXT8_PAGE_TABLE_BASE_ADDR +
((vm_id - 8) << 2)) >> 2);
}
- radeon_ring_write(ring, vm->
[PATCH 05/11] drm/radeon: fence PT updates manually
From: Christian K?nig
This allows us to add the real execution fence as shared.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon_object.c | 19 ++
drivers/gpu/drm/radeon/radeon_object.h | 2 ++
drivers/gpu/drm/radeon/radeon_vm.c | 65 +-
3 files changed, 62 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c
b/drivers/gpu/drm/radeon/radeon_object.c
index 8abee5f..8714baf 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -790,3 +790,22 @@ int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
bool no_wait)
ttm_bo_unreserve(&bo->tbo);
return r;
}
+
+/**
+ * radeon_bo_fence - add fence to buffer object
+ *
+ * @bo: buffer object in question
+ * @fence: fence to add
+ * @shared: true if fence should be added shared
+ *
+ */
+void radeon_bo_fence(struct radeon_bo *bo, struct radeon_fence *fence,
+ bool shared)
+{
+ struct reservation_object *resv = bo->tbo.resv;
+
+ if (shared)
+ reservation_object_add_shared_fence(resv, &fence->base);
+ else
+ reservation_object_add_excl_fence(resv, &fence->base);
+}
diff --git a/drivers/gpu/drm/radeon/radeon_object.h
b/drivers/gpu/drm/radeon/radeon_object.h
index 98a47fd..ec80b28 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -154,6 +154,8 @@ extern void radeon_bo_move_notify(struct ttm_buffer_object
*bo,
struct ttm_mem_reg *new_mem);
extern int radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
extern int radeon_bo_get_surface_reg(struct radeon_bo *bo);
+extern void radeon_bo_fence(struct radeon_bo *bo, struct radeon_fence *fence,
+ bool shared);
/*
* sub allocation
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
b/drivers/gpu/drm/radeon/radeon_vm.c
index 411146a..84b2735 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -143,7 +143,7 @@ struct radeon_cs_reloc *radeon_vm_get_bos(struct
radeon_device *rdev,
list[0].prefered_domains = RADEON_GEM_DOMAIN_VRAM;
list[0].allowed_domains = RADEON_GEM_DOMAIN_VRAM;
list[0].tv.bo = &vm->page_directory->tbo;
- list[0].tv.shared = false;
+ list[0].tv.shared = true;
list[0].tiling_flags = 0;
list[0].handle = 0;
list_add(&list[0].tv.head, head);
@@ -157,7 +157,7 @@ struct radeon_cs_reloc *radeon_vm_get_bos(struct
radeon_device *rdev,
list[idx].prefered_domains = RADEON_GEM_DOMAIN_VRAM;
list[idx].allowed_domains = RADEON_GEM_DOMAIN_VRAM;
list[idx].tv.bo = &list[idx].robj->tbo;
- list[idx].tv.shared = false;
+ list[idx].tv.shared = true;
list[idx].tiling_flags = 0;
list[idx].handle = 0;
list_add(&list[idx++].tv.head, head);
@@ -388,35 +388,25 @@ static void radeon_vm_set_pages(struct radeon_device
*rdev,
static int radeon_vm_clear_bo(struct radeon_device *rdev,
struct radeon_bo *bo)
{
-struct ttm_validate_buffer tv;
-struct ww_acquire_ctx ticket;
-struct list_head head;
struct radeon_ib ib;
unsigned entries;
uint64_t addr;
int r;
-memset(&tv, 0, sizeof(tv));
-tv.bo = &bo->tbo;
- tv.shared = false;
-
-INIT_LIST_HEAD(&head);
-list_add(&tv.head, &head);
-
-r = ttm_eu_reserve_buffers(&ticket, &head, true);
-if (r)
+ r = radeon_bo_reserve(bo, false);
+ if (r)
return r;
-r = ttm_bo_validate(&bo->tbo, &bo->placement, true, false);
-if (r)
-goto error;
+ r = ttm_bo_validate(&bo->tbo, &bo->placement, true, false);
+ if (r)
+ goto error_unreserve;
addr = radeon_bo_gpu_offset(bo);
entries = radeon_bo_size(bo) / 8;
r = radeon_ib_get(rdev, R600_RING_TYPE_DMA_INDEX, &ib, NULL, 256);
if (r)
-goto error;
+ goto error_unreserve;
ib.length_dw = 0;
@@ -426,15 +416,15 @@ static int radeon_vm_clear_bo(struct radeon_device *rdev,
r = radeon_ib_schedule(rdev, &ib, NULL, false);
if (r)
-goto error;
+ goto error_free;
- ttm_eu_fence_buffer_objects(&ticket, &head, &ib.fence->base);
- radeon_ib_free(rdev, &ib);
+ radeon_bo_fence(bo, ib.fence, false);
- return 0;
+error_free:
+ radeon_ib_free(rdev, &ib);
-error:
- ttm_eu_backoff_reservation(&ticket, &head);
+error_unreserve:
+ radeon_bo_unreserve(bo);
return r;
}
@@ -706,6 +696,7 @@ int radeon_vm_update_page_directory(struct radeon_device
*rdev,
radeon_ib_free(rdev, &ib);
return r;
[PATCH 11/11] drm/radeon: update the VM after setting BO address
From: Christian K?nig
This way the necessary VM update is kicked off immediately
if all BOs involved are in GPU accessible memory.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon_gem.c | 60 +
1 file changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
b/drivers/gpu/drm/radeon/radeon_gem.c
index cbd7244..c100aa6 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -518,6 +518,63 @@ out:
return r;
}
+/**
+ * radeon_gem_va_update_vm -update the bo_va in its VM
+ *
+ * @rdev: radeon_device pointer
+ * @bo_va: bo_va to update
+ *
+ * Update the bo_va directly after setting it's address. Errors are not
+ * vital here, so they are not reported back to userspace.
+ */
+static void radeon_gem_va_update_vm(struct radeon_device *rdev,
+ struct radeon_bo_va *bo_va)
+{
+ struct ttm_validate_buffer tv, *entry;
+ struct radeon_cs_reloc *vm_bos;
+ struct ww_acquire_ctx ticket;
+ struct list_head list;
+ unsigned domain;
+ int r;
+
+ INIT_LIST_HEAD(&list);
+
+ tv.bo = &bo_va->bo->tbo;
+ tv.shared = true;
+ list_add(&tv.head, &list);
+
+ vm_bos = radeon_vm_get_bos(rdev, bo_va->vm, &list);
+ if (!vm_bos)
+ return;
+
+ r = ttm_eu_reserve_buffers(&ticket, &list, true);
+ if (r)
+ goto error_free;
+
+ list_for_each_entry(entry, &list, head) {
+ domain = radeon_mem_type_to_domain(entry->bo->mem.mem_type);
+ /* if anything is swapped out don't swap it in here,
+ just abort and wait for the next CS */
+ if (domain == RADEON_GEM_DOMAIN_CPU)
+ goto error_unreserve;
+ }
+
+ r = radeon_vm_clear_freed(rdev, bo_va->vm);
+ if (r)
+ goto error_unreserve;
+
+ r = radeon_vm_bo_update(rdev, bo_va, &bo_va->bo->tbo.mem);
+
+error_unreserve:
+ ttm_eu_backoff_reservation(&ticket, &list);
+
+error_free:
+ kfree(vm_bos);
+
+ if (r)
+ DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
+}
+
int radeon_gem_va_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
@@ -605,6 +662,9 @@ int radeon_gem_va_ioctl(struct drm_device *dev, void *data,
goto out;
}
r = radeon_vm_bo_set_addr(rdev, bo_va, args->offset,
args->flags);
+ if (!r)
+ radeon_gem_va_update_vm(rdev, bo_va);
+
break;
case RADEON_VA_UNMAP:
r = radeon_vm_bo_set_addr(rdev, bo_va, 0, 0);
--
1.9.1
[PATCH 04/11] drm/radeon: split semaphore and sync object handling
From: Christian K?nig
Previously we just allocated space for four hardware semaphores
in each software semaphore object. Make software semaphore objects
represent only one hardware semaphore address again by splitting
the sync code into it's own object.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/Makefile | 3 +-
drivers/gpu/drm/radeon/cik.c | 18 ++-
drivers/gpu/drm/radeon/cik_sdma.c | 18 ++-
drivers/gpu/drm/radeon/evergreen_dma.c| 18 ++-
drivers/gpu/drm/radeon/r600.c | 18 ++-
drivers/gpu/drm/radeon/r600_dma.c | 18 ++-
drivers/gpu/drm/radeon/radeon.h | 40 +++---
drivers/gpu/drm/radeon/radeon_cs.c| 7 +-
drivers/gpu/drm/radeon/radeon_ib.c| 13 +-
drivers/gpu/drm/radeon/radeon_semaphore.c | 139 +
drivers/gpu/drm/radeon/radeon_sync.c | 198 ++
drivers/gpu/drm/radeon/radeon_vm.c| 4 +-
drivers/gpu/drm/radeon/rv770_dma.c| 18 ++-
drivers/gpu/drm/radeon/si_dma.c | 18 ++-
14 files changed, 286 insertions(+), 244 deletions(-)
create mode 100644 drivers/gpu/drm/radeon/radeon_sync.c
diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile
index 7d7aed5..8cc9f68 100644
--- a/drivers/gpu/drm/radeon/Makefile
+++ b/drivers/gpu/drm/radeon/Makefile
@@ -80,7 +80,8 @@ radeon-y += radeon_device.o radeon_asic.o radeon_kms.o \
r600_dpm.o rs780_dpm.o rv6xx_dpm.o rv770_dpm.o rv730_dpm.o rv740_dpm.o \
rv770_smc.o cypress_dpm.o btc_dpm.o sumo_dpm.o sumo_smc.o trinity_dpm.o
\
trinity_smc.o ni_dpm.o si_smc.o si_dpm.o kv_smc.o kv_dpm.o ci_smc.o \
- ci_dpm.o dce6_afmt.o radeon_vm.o radeon_ucode.o radeon_ib.o radeon_mn.o
+ ci_dpm.o dce6_afmt.o radeon_vm.o radeon_ucode.o radeon_ib.o radeon_mn.o
\
+ radeon_sync.o
# add async DMA block
radeon-y += \
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 16a861d..6dd55ea 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -3970,31 +3970,27 @@ struct radeon_fence *cik_copy_cpdma(struct
radeon_device *rdev,
unsigned num_gpu_pages,
struct reservation_object *resv)
{
- struct radeon_semaphore *sem = NULL;
struct radeon_fence *fence;
+ struct radeon_sync sync;
int ring_index = rdev->asic->copy.blit_ring_index;
struct radeon_ring *ring = &rdev->ring[ring_index];
u32 size_in_bytes, cur_size_in_bytes, control;
int i, num_loops;
int r = 0;
- r = radeon_semaphore_create(rdev, &sem);
- if (r) {
- DRM_ERROR("radeon: moving bo (%d).\n", r);
- return ERR_PTR(r);
- }
+ radeon_sync_create(&sync);
size_in_bytes = (num_gpu_pages << RADEON_GPU_PAGE_SHIFT);
num_loops = DIV_ROUND_UP(size_in_bytes, 0x1f);
r = radeon_ring_lock(rdev, ring, num_loops * 7 + 18);
if (r) {
DRM_ERROR("radeon: moving bo (%d).\n", r);
- radeon_semaphore_free(rdev, &sem, NULL);
+ radeon_sync_free(rdev, &sync, NULL);
return ERR_PTR(r);
}
- radeon_semaphore_sync_resv(sem, resv, false);
- radeon_semaphore_sync_rings(rdev, sem, ring->idx);
+ radeon_sync_resv(&sync, resv, false);
+ radeon_sync_rings(rdev, &sync, ring->idx);
for (i = 0; i < num_loops; i++) {
cur_size_in_bytes = size_in_bytes;
@@ -4018,12 +4014,12 @@ struct radeon_fence *cik_copy_cpdma(struct
radeon_device *rdev,
r = radeon_fence_emit(rdev, &fence, ring->idx);
if (r) {
radeon_ring_unlock_undo(rdev, ring);
- radeon_semaphore_free(rdev, &sem, NULL);
+ radeon_sync_free(rdev, &sync, NULL);
return ERR_PTR(r);
}
radeon_ring_unlock_commit(rdev, ring, false);
- radeon_semaphore_free(rdev, &sem, fence);
+ radeon_sync_free(rdev, &sync, fence);
return fence;
}
diff --git a/drivers/gpu/drm/radeon/cik_sdma.c
b/drivers/gpu/drm/radeon/cik_sdma.c
index dd73246..06602e4 100644
--- a/drivers/gpu/drm/radeon/cik_sdma.c
+++ b/drivers/gpu/drm/radeon/cik_sdma.c
@@ -548,31 +548,27 @@ struct radeon_fence *cik_copy_dma(struct radeon_device
*rdev,
unsigned num_gpu_pages,
struct reservation_object *resv)
{
- struct radeon_semaphore *sem = NULL;
struct radeon_fence *fence;
+ struct radeon_sync sync;
int ring_index = rdev->asic->copy.dma_ring_index;
struct radeon_ring *ring = &rdev->ring[ring_index];
u32 size_in_bytes, cur_size_in_bytes;
int i, num_loops;
int r = 0;
- r = radeon_semaphore_create(rdev, &sem);
- if (r) {
- DRM_ERROR("radeon: moving bo (%d).\n", r);
- retur
[PATCH 06/11] drm/radeon: track VM update fences separately
From: Christian K?nig
Note for each fence if it's a VM page table update or not. This allows
us to determine the last VM update in a sync object and so to figure
out if we need to flush the TLB or not.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon.h | 18 ++
drivers/gpu/drm/radeon/radeon_fence.c | 1 +
drivers/gpu/drm/radeon/radeon_ib.c| 3 ++-
drivers/gpu/drm/radeon/radeon_sync.c | 7 +++
drivers/gpu/drm/radeon/radeon_vm.c| 25 +
5 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index c326fae..fddb19e 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -357,14 +357,15 @@ struct radeon_fence_driver {
};
struct radeon_fence {
- struct fence base;
+ struct fencebase;
- struct radeon_device*rdev;
- uint64_tseq;
+ struct radeon_device*rdev;
+ uint64_tseq;
/* RB, DMA, etc. */
- unsignedring;
+ unsignedring;
+ boolis_vm_update;
- wait_queue_tfence_wake;
+ wait_queue_tfence_wake;
};
int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring);
@@ -591,6 +592,7 @@ void radeon_semaphore_free(struct radeon_device *rdev,
struct radeon_sync {
struct radeon_semaphore *semaphores[RADEON_NUM_SYNCS];
struct radeon_fence *sync_to[RADEON_NUM_RINGS];
+ struct radeon_fence *last_vm_update;
};
void radeon_sync_create(struct radeon_sync *sync);
@@ -918,8 +920,8 @@ struct radeon_vm {
struct mutexmutex;
/* last fence for cs using this vm */
struct radeon_fence *fence;
- /* last flush or NULL if we still need to flush */
- struct radeon_fence *last_flush;
+ /* last flushed PD/PT update */
+ struct radeon_fence *flushed_updates;
/* last use of vmid */
struct radeon_fence *last_id_use;
};
@@ -2948,7 +2950,7 @@ struct radeon_fence *radeon_vm_grab_id(struct
radeon_device *rdev,
struct radeon_vm *vm, int ring);
void radeon_vm_flush(struct radeon_device *rdev,
struct radeon_vm *vm,
- int ring);
+int ring, struct radeon_fence *fence);
void radeon_vm_fence(struct radeon_device *rdev,
struct radeon_vm *vm,
struct radeon_fence *fence);
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index af9f2d6..71aa198 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -140,6 +140,7 @@ int radeon_fence_emit(struct radeon_device *rdev,
(*fence)->rdev = rdev;
(*fence)->seq = seq;
(*fence)->ring = ring;
+ (*fence)->is_vm_update = false;
fence_init(&(*fence)->base, &radeon_fence_ops,
&rdev->fence_queue.lock, rdev->fence_context + ring, seq);
radeon_fence_ring_emit(rdev, ring, *fence);
diff --git a/drivers/gpu/drm/radeon/radeon_ib.c
b/drivers/gpu/drm/radeon/radeon_ib.c
index 56a1704..c39ce1f 100644
--- a/drivers/gpu/drm/radeon/radeon_ib.c
+++ b/drivers/gpu/drm/radeon/radeon_ib.c
@@ -154,7 +154,8 @@ int radeon_ib_schedule(struct radeon_device *rdev, struct
radeon_ib *ib,
}
if (ib->vm)
- radeon_vm_flush(rdev, ib->vm, ib->ring);
+ radeon_vm_flush(rdev, ib->vm, ib->ring,
+ ib->sync.last_vm_update);
if (const_ib) {
radeon_ring_ib_execute(rdev, const_ib->ring, const_ib);
diff --git a/drivers/gpu/drm/radeon/radeon_sync.c
b/drivers/gpu/drm/radeon/radeon_sync.c
index 77b7401..4e7a7f7 100644
--- a/drivers/gpu/drm/radeon/radeon_sync.c
+++ b/drivers/gpu/drm/radeon/radeon_sync.c
@@ -48,6 +48,8 @@ void radeon_sync_create(struct radeon_sync *sync)
for (i = 0; i < RADEON_NUM_RINGS; ++i)
sync->sync_to[i] = NULL;
+
+ sync->last_vm_update = NULL;
}
/**
@@ -68,6 +70,11 @@ void radeon_sync_fence(struct radeon_sync *sync,
other = sync->sync_to[fence->ring];
sync->sync_to[fence->ring] = radeon_fence_later(fence, other);
+
+ if (fence->is_vm_update) {
+ other = sync->last_vm_update;
+ sync->last_vm_update = radeon_fence_later(fence, other);
+ }
}
/**
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
b/drivers/gpu/drm/radeon/radeon_vm.c
index 84b2735..ff104a5 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -190,7 +190,7 @@ struct radeon_fence *radeon_vm_grab_id(struct radeon_device
*rdev,
return NULL;
/* we definatel
[PATCH 07/11] drm/radeon: use one VMID for each ring
From: Christian K?nig
Use multiple VMIDs for each VM, one for each ring. That allows
us to execute flushes separately on each ring, still not ideal
cause in a lot of cases rings can share IDs.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/cik.c | 4 +--
drivers/gpu/drm/radeon/cik_sdma.c | 2 +-
drivers/gpu/drm/radeon/ni.c| 6 ++--
drivers/gpu/drm/radeon/ni_dma.c| 3 +-
drivers/gpu/drm/radeon/radeon.h| 36 +--
drivers/gpu/drm/radeon/radeon_vm.c | 59 +++---
drivers/gpu/drm/radeon/si.c| 6 ++--
7 files changed, 68 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 6dd55ea..fae5a8c 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -4042,6 +4042,7 @@ struct radeon_fence *cik_copy_cpdma(struct radeon_device
*rdev,
void cik_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
{
struct radeon_ring *ring = &rdev->ring[ib->ring];
+ unsigned vm_id = ib->vm ? ib->vm->ids[ib->ring].id : 0;
u32 header, control = INDIRECT_BUFFER_VALID;
if (ib->is_const_ib) {
@@ -4070,8 +4071,7 @@ void cik_ring_ib_execute(struct radeon_device *rdev,
struct radeon_ib *ib)
header = PACKET3(PACKET3_INDIRECT_BUFFER, 2);
}
- control |= ib->length_dw |
- (ib->vm ? (ib->vm->id << 24) : 0);
+ control |= ib->length_dw | (vm_id << 24);
radeon_ring_write(ring, header);
radeon_ring_write(ring,
diff --git a/drivers/gpu/drm/radeon/cik_sdma.c
b/drivers/gpu/drm/radeon/cik_sdma.c
index 06602e4..1374ecf 100644
--- a/drivers/gpu/drm/radeon/cik_sdma.c
+++ b/drivers/gpu/drm/radeon/cik_sdma.c
@@ -134,7 +134,7 @@ void cik_sdma_ring_ib_execute(struct radeon_device *rdev,
struct radeon_ib *ib)
{
struct radeon_ring *ring = &rdev->ring[ib->ring];
- u32 extra_bits = (ib->vm ? ib->vm->id : 0) & 0xf;
+ u32 extra_bits = (ib->vm ? ib->vm->ids[ib->ring].id : 0) & 0xf;
if (rdev->wb.enabled) {
u32 next_rptr = ring->wptr + 5;
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 7f451aa..aca3e91 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1366,6 +1366,7 @@ void cayman_fence_ring_emit(struct radeon_device *rdev,
void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
{
struct radeon_ring *ring = &rdev->ring[ib->ring];
+ unsigned vm_id = ib->vm ? ib->vm->ids[ib->ring].id : 0;
u32 cp_coher_cntl = PACKET3_FULL_CACHE_ENA | PACKET3_TC_ACTION_ENA |
PACKET3_SH_ACTION_ENA;
@@ -1388,15 +1389,14 @@ void cayman_ring_ib_execute(struct radeon_device *rdev,
struct radeon_ib *ib)
#endif
(ib->gpu_addr & 0xFFFC));
radeon_ring_write(ring, upper_32_bits(ib->gpu_addr) & 0xFF);
- radeon_ring_write(ring, ib->length_dw |
- (ib->vm ? (ib->vm->id << 24) : 0));
+ radeon_ring_write(ring, ib->length_dw | (vm_id << 24));
/* flush read cache over gart for this vmid */
radeon_ring_write(ring, PACKET3(PACKET3_SURFACE_SYNC, 3));
radeon_ring_write(ring, PACKET3_ENGINE_ME | cp_coher_cntl);
radeon_ring_write(ring, 0x);
radeon_ring_write(ring, 0);
- radeon_ring_write(ring, ((ib->vm ? ib->vm->id : 0) << 24) | 10); /*
poll interval */
+ radeon_ring_write(ring, (vm_id << 24) | 10); /* poll interval */
}
static void cayman_cp_enable(struct radeon_device *rdev, bool enable)
diff --git a/drivers/gpu/drm/radeon/ni_dma.c b/drivers/gpu/drm/radeon/ni_dma.c
index 1d15f6b..efe98b1 100644
--- a/drivers/gpu/drm/radeon/ni_dma.c
+++ b/drivers/gpu/drm/radeon/ni_dma.c
@@ -123,6 +123,7 @@ void cayman_dma_ring_ib_execute(struct radeon_device *rdev,
struct radeon_ib *ib)
{
struct radeon_ring *ring = &rdev->ring[ib->ring];
+ unsigned vm_id = ib->vm ? ib->vm->ids[ib->ring].id : 0;
if (rdev->wb.enabled) {
u32 next_rptr = ring->wptr + 4;
@@ -140,7 +141,7 @@ void cayman_dma_ring_ib_execute(struct radeon_device *rdev,
*/
while ((ring->wptr & 7) != 5)
radeon_ring_write(ring, DMA_PACKET(DMA_PACKET_NOP, 0, 0, 0));
- radeon_ring_write(ring, DMA_IB_PACKET(DMA_PACKET_INDIRECT_BUFFER,
ib->vm ? ib->vm->id : 0, 0));
+ radeon_ring_write(ring, DMA_IB_PACKET(DMA_PACKET_INDIRECT_BUFFER,
vm_id, 0));
radeon_ring_write(ring, (ib->gpu_addr & 0xFFE0));
radeon_ring_write(ring, (ib->length_dw << 12) |
(upper_32_bits(ib->gpu_addr) & 0xFF));
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index fddb19e..bee8934 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -897,33 +897,39 @@ struct radeon_vm_pt {
uint64_
[PATCH 08/11] drm/radeon: fence BO_VAs manually
From: Christian K?nig
This allows us to finally remove the VM fence and
so allow concurrent use of it from different engines.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon.h| 7 +++
drivers/gpu/drm/radeon/radeon_cs.c | 5 -
drivers/gpu/drm/radeon/radeon_vm.c | 17 ++---
3 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index bee8934..ab71050 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -453,6 +453,7 @@ struct radeon_bo_va {
struct list_headbo_list;
uint32_tflags;
uint64_taddr;
+ struct radeon_fence *last_pt_update;
unsignedref_count;
/* protected by vm mutex */
@@ -907,6 +908,8 @@ struct radeon_vm_id {
};
struct radeon_vm {
+ struct mutexmutex;
+
struct rb_root va;
/* BOs moved, but not yet updated in the PT */
@@ -924,10 +927,6 @@ struct radeon_vm {
struct radeon_bo_va *ib_bo_va;
- struct mutexmutex;
- /* last fence for cs using this vm */
- struct radeon_fence *fence;
-
/* for id and flush management per ring */
struct radeon_vm_id ids[RADEON_NUM_RINGS];
};
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c
b/drivers/gpu/drm/radeon/radeon_cs.c
index 0619c32..9e6714b 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -494,6 +494,8 @@ static int radeon_bo_vm_update_pte(struct radeon_cs_parser
*p,
if (r)
return r;
+ radeon_sync_resv(&p->ib.sync, vm->page_directory->tbo.resv, true);
+
r = radeon_vm_clear_freed(rdev, vm);
if (r)
return r;
@@ -525,6 +527,8 @@ static int radeon_bo_vm_update_pte(struct radeon_cs_parser
*p,
r = radeon_vm_bo_update(rdev, bo_va, &bo->tbo.mem);
if (r)
return r;
+
+ radeon_sync_fence(&p->ib.sync, bo_va->last_pt_update);
}
return radeon_vm_clear_invalids(rdev, vm);
@@ -563,7 +567,6 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev,
goto out;
}
radeon_cs_sync_rings(parser);
- radeon_sync_fence(&parser->ib.sync, vm->fence);
if ((rdev->family >= CHIP_TAHITI) &&
(parser->chunk_const_ib_idx != -1)) {
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
b/drivers/gpu/drm/radeon/radeon_vm.c
index 2688d49..61e697f 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -275,9 +275,6 @@ void radeon_vm_fence(struct radeon_device *rdev,
{
unsigned vm_id = vm->ids[fence->ring].id;
- radeon_fence_unref(&vm->fence);
- vm->fence = radeon_fence_ref(fence);
-
radeon_fence_unref(&rdev->vm_manager.active[vm_id]);
rdev->vm_manager.active[vm_id] = radeon_fence_ref(fence);
@@ -706,8 +703,6 @@ int radeon_vm_update_page_directory(struct radeon_device
*rdev,
}
ib.fence->is_vm_update = true;
radeon_bo_fence(pd, ib.fence, false);
- radeon_fence_unref(&vm->fence);
- vm->fence = radeon_fence_ref(ib.fence);
}
radeon_ib_free(rdev, &ib);
@@ -998,8 +993,8 @@ int radeon_vm_bo_update(struct radeon_device *rdev,
}
ib.fence->is_vm_update = true;
radeon_vm_fence_pts(vm, bo_va->it.start, bo_va->it.last + 1, ib.fence);
- radeon_fence_unref(&vm->fence);
- vm->fence = radeon_fence_ref(ib.fence);
+ radeon_fence_unref(&bo_va->last_pt_update);
+ bo_va->last_pt_update = radeon_fence_ref(ib.fence);
radeon_ib_free(rdev, &ib);
return 0;
@@ -1025,6 +1020,7 @@ int radeon_vm_clear_freed(struct radeon_device *rdev,
list_for_each_entry_safe(bo_va, tmp, &vm->freed, vm_status) {
r = radeon_vm_bo_update(rdev, bo_va, NULL);
radeon_bo_unref(&bo_va->bo);
+ radeon_fence_unref(&bo_va->last_pt_update);
kfree(bo_va);
if (r)
return r;
@@ -1083,6 +1079,7 @@ void radeon_vm_bo_rmv(struct radeon_device *rdev,
bo_va->bo = radeon_bo_ref(bo_va->bo);
list_add(&bo_va->vm_status, &vm->freed);
} else {
+ radeon_fence_unref(&bo_va->last_pt_update);
kfree(bo_va);
}
@@ -1129,8 +1126,6 @@ int radeon_vm_init(struct radeon_device *rdev, struct
radeon_vm *vm)
int i, r;
vm->ib_bo_va = NULL;
- vm->fence = NULL;
-
for (i = 0; i < RADEON_NUM_RINGS; ++i) {
vm->ids[i].id = 0;
vm->ids[i].flushed_updates = NULL;
@@ -1191,11 +1186,13 @@ void radeon_vm_fini(struct radeon_device *rdev, struct
radeon_vm *vm)
[PATCH 09/11] drm/radeon: sync PD updates as shared
From: Christian K?nig
We never invalidate PD entries and making them valid can
run with other users in parallel.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon_vm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_vm.c
b/drivers/gpu/drm/radeon/radeon_vm.c
index 61e697f..80262c6 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -694,7 +694,7 @@ int radeon_vm_update_page_directory(struct radeon_device
*rdev,
if (ib.length_dw != 0) {
radeon_asic_vm_pad_ib(rdev, &ib);
- radeon_sync_resv(&ib.sync, pd->tbo.resv, false);
+ radeon_sync_resv(&ib.sync, pd->tbo.resv, true);
WARN_ON(ib.length_dw > ndw);
r = radeon_ib_schedule(rdev, &ib, NULL, false);
if (r) {
--
1.9.1
[PATCH 10/11] drm/radeon: sync PT updates as shared if they are validating
From: Christian K?nig Only invalidating PTEs needs to be executed synchronized to using the PT. Signed-off-by: Christian K?nig --- drivers/gpu/drm/radeon/radeon_vm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c index 80262c6..55afb84 100644 --- a/drivers/gpu/drm/radeon/radeon_vm.c +++ b/drivers/gpu/drm/radeon/radeon_vm.c @@ -819,7 +819,8 @@ static void radeon_vm_update_ptes(struct radeon_device *rdev, unsigned nptes; uint64_t pte; - radeon_sync_resv(&ib->sync, pt->tbo.resv, false); + radeon_sync_resv(&ib->sync, pt->tbo.resv, +!!(flags & R600_PTE_VALID)); if ((addr & ~mask) == (end & ~mask)) nptes = end - addr; -- 1.9.1
[Bug 83436] Sudden framerate drops in multiple games
https://bugs.freedesktop.org/show_bug.cgi?id=83436 Fabio Pedretti changed: What|Removed |Added CC||fabio.ped at libero.it --- Comment #28 from Fabio Pedretti --- Just to let you know that oibaf PPA now disable -mtune=generic to avoid this issue. So don't trust the PPA, as it is now, to check if this issue is fixed or not. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/d59dea2c/attachment.html>
[Bug 83792] Kernel hangs on boot without nomodeset option
https://bugs.freedesktop.org/show_bug.cgi?id=83792 --- Comment #1 from Alex Deucher --- Can you elaborate on what "not worked" means? Blank screen, corruption, hangs? Are you able to access the machine remotely? If so, can you attach your xorg log and dmesg output with the radeon driver loaded? Can you bisect the kernel to determine what commit broke your card? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/c92ea113/attachment.html>
[PATCH 1/4] atomic: add atomic_add_unless()
Signed-off-by: Lionel Landwerlin
---
xf86atomic.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/xf86atomic.h b/xf86atomic.h
index db2f619..bc482c9 100644
--- a/xf86atomic.h
+++ b/xf86atomic.h
@@ -96,4 +96,13 @@ typedef struct { uint_t atomic; } atomic_t;
#error libdrm requires atomic operations, please define them for your
CPU/compiler.
#endif
+static inline int atomic_add_unless(atomic_t *v, int add, int unless)
+{
+ int c, old;
+ c = atomic_read(v);
+ while (c != unless && (old = atomic_cmpxchg(v, c, c + add)) != c)
+ c = old;
+ return c == unless;
+}
+
#endif
--
2.0.1
[PATCH 2/4] intel: make bufmgr_gem shareable from different API
When using Mesa and LibVA in the same process, one would like to be
able bind buffers from the output of the decoder to a GL texture
through an EGLImage.
LibVA can reuse buffers allocated by Gbm through a file descriptor. It
will then wrap it into a drm_intel_bo with
drm_intel_bo_gem_create_from_prime().
The problem at the moment is that both library get a different
drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init()
even though they're using the same drm file descriptor. As a result,
instead of manipulating the same buffer object for a given file
descriptor, they get 2 different drm_intel_bo objects and 2 different
refcounts, leading one of the library to get errors from the kernel on
invalid BO when one of the 2 library is done with a shared buffer.
This patch modifies drm_intel_bufmgr_gem_init() so, given a file
descriptor, it will look for an already existing drm_intel_bufmgr
using the same file descriptor and return that object.
Signed-off-by: Lionel Landwerlin
---
intel/intel_bufmgr_gem.c | 63
1 file changed, 58 insertions(+), 5 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 0e1cb0d..1e2dd77 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
typedef struct _drm_intel_bufmgr_gem {
drm_intel_bufmgr bufmgr;
+ atomic_t refcount;
+
int fd;
int max_relocs;
@@ -111,6 +113,8 @@ typedef struct _drm_intel_bufmgr_gem {
int num_buckets;
time_t time;
+ drmMMListHead managers;
+
drmMMListHead named;
drmMMListHead vma_cache;
int vma_count, vma_open, vma_max;
@@ -3186,6 +3190,41 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
*bo,
bo_gem->aub_annotation_count = count;
}
+static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+static drmMMListHead bufmgr_list = { &bufmgr_list, &bufmgr_list };
+
+static drm_intel_bufmgr_gem *
+drm_intel_bufmgr_gem_find(int fd)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem;
+
+ DRMLISTFOREACHENTRY(bufmgr_gem, &bufmgr_list, managers) {
+ if (bufmgr_gem->fd == fd) {
+ atomic_inc(&bufmgr_gem->refcount);
+ return bufmgr_gem;
+ }
+ }
+
+ return NULL;
+}
+
+static void
+drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
+
+ if (atomic_add_unless(&bufmgr_gem->refcount, -1, 1)) {
+ pthread_mutex_lock(&bufmgr_list_mutex);
+
+ if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
+ DRMLISTDEL(&bufmgr_gem->managers);
+ drm_intel_bufmgr_gem_destroy(bufmgr);
+ }
+
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+ }
+}
+
/**
* Initializes the GEM buffer manager, which uses the kernel to allocate, map,
* and manage map buffer objections.
@@ -3201,15 +3240,23 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
int ret, tmp;
bool exec2 = false;
+ pthread_mutex_lock(&bufmgr_list_mutex);
+
+ bufmgr_gem = drm_intel_bufmgr_gem_find(fd);
+ if (bufmgr_gem)
+ goto exit;
+
bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
if (bufmgr_gem == NULL)
- return NULL;
+ goto exit;
bufmgr_gem->fd = fd;
+ atomic_set(&bufmgr_gem->refcount, 1);
if (pthread_mutex_init(&bufmgr_gem->lock, NULL) != 0) {
free(bufmgr_gem);
- return NULL;
+ bufmgr_gem = NULL;
+ goto exit;
}
ret = drmIoctl(bufmgr_gem->fd,
@@ -3246,7 +3293,8 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->gen = 8;
else {
free(bufmgr_gem);
- return NULL;
+ bufmgr_gem = NULL;
+ goto exit;
}
if (IS_GEN3(bufmgr_gem->pci_device) &&
@@ -3357,7 +3405,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->bufmgr.bo_exec = drm_intel_gem_bo_exec;
bufmgr_gem->bufmgr.bo_busy = drm_intel_gem_bo_busy;
bufmgr_gem->bufmgr.bo_madvise = drm_intel_gem_bo_madvise;
- bufmgr_gem->bufmgr.destroy = drm_intel_bufmgr_gem_destroy;
+ bufmgr_gem->bufmgr.destroy = drm_intel_bufmgr_gem_unref;
bufmgr_gem->bufmgr.debug = 0;
bufmgr_gem->bufmgr.check_aperture_space =
drm_intel_gem_check_aperture_space;
@@ -3373,5 +3421,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
DRMINITLISTHEAD(&bufmgr_gem->vma_cache);
bufmgr_gem->vma_max = -1; /* unlimited by default */
- return &bufmgr_gem->bufmgr;
+ DRMLISTADD(&bufmgr_gem->managers, &bufmgr_list);
+
+exit:
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+
+ return bufmgr_gem
[PATCH 3/4] intel: make bo_unreference() thread safe
Signed-off-by: Lionel Landwerlin
---
intel/intel_bufmgr_gem.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 1e2dd77..bf6745d 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -1157,7 +1157,8 @@ static void drm_intel_gem_bo_unreference(drm_intel_bo *bo)
drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
assert(atomic_read(&bo_gem->refcount) > 0);
- if (atomic_dec_and_test(&bo_gem->refcount)) {
+
+ if (atomic_add_unless(&bo_gem->refcount, -1, 1)) {
drm_intel_bufmgr_gem *bufmgr_gem =
(drm_intel_bufmgr_gem *) bo->bufmgr;
struct timespec time;
@@ -1165,8 +1166,12 @@ static void drm_intel_gem_bo_unreference(drm_intel_bo
*bo)
clock_gettime(CLOCK_MONOTONIC, &time);
pthread_mutex_lock(&bufmgr_gem->lock);
- drm_intel_gem_bo_unreference_final(bo, time.tv_sec);
- drm_intel_gem_cleanup_bo_cache(bufmgr_gem, time.tv_sec);
+
+ if (atomic_dec_and_test(&bo_gem->refcount)) {
+ drm_intel_gem_bo_unreference_final(bo, time.tv_sec);
+ drm_intel_gem_cleanup_bo_cache(bufmgr_gem, time.tv_sec);
+ }
+
pthread_mutex_unlock(&bufmgr_gem->lock);
}
}
--
2.0.1
Shareable bufmgr objects v4
This is getting bigger than expected. Adding the locking that Chris suggested on IRC. Thanks for taking time to review Chris. - Lionel
[PATCH 4/4] intel: make drm_intel_gem_bo_get_reloc_count() thread safe
Signed-off-by: Lionel Landwerlin
---
intel/intel_bufmgr_gem.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index bf6745d..4e34f75 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -1783,6 +1783,7 @@ drm_intel_gem_bo_get_reloc_count(drm_intel_bo *bo)
drm_public void
drm_intel_gem_bo_clear_relocs(drm_intel_bo *bo, int start)
{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
int i;
struct timespec time;
@@ -1790,7 +1791,10 @@ drm_intel_gem_bo_clear_relocs(drm_intel_bo *bo, int
start)
clock_gettime(CLOCK_MONOTONIC, &time);
assert(bo_gem->reloc_count >= start);
+
/* Unreference the cleared target buffers */
+ pthread_mutex_lock(&bufmgr_gem->lock);
+
for (i = start; i < bo_gem->reloc_count; i++) {
drm_intel_bo_gem *target_bo_gem = (drm_intel_bo_gem *)
bo_gem->reloc_target_info[i].bo;
if (&target_bo_gem->bo != bo) {
@@ -1800,6 +1804,9 @@ drm_intel_gem_bo_clear_relocs(drm_intel_bo *bo, int start)
}
}
bo_gem->reloc_count = start;
+
+ pthread_mutex_unlock(&bufmgr_gem->lock);
+
}
/**
--
2.0.1
[Bug 75649] Glitchy output using only HDMI on laptop with AMD Mobility Radeon HD 3450/3470
https://bugs.freedesktop.org/show_bug.cgi?id=75649 --- Comment #13 from Alex Deucher --- Does booting with radeon.disp_priority=2 on the kernel command line in grub help? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/ee0c3c8b/attachment-0001.html>
[Bug 83792] Kernel hangs on boot without nomodeset option
https://bugs.freedesktop.org/show_bug.cgi?id=83792 --- Comment #2 from Tim Nelson --- Regarding "not worked", these kernels have always hung on boot with a blank screen; most of these are from my attempts to upgrade to various different versions of Fedora between 13 (currently working for me) and 20 (the one I'm attempting to install now). I suspect that these kernels weren't being passed the "nomodeset" option by default, but I have no evidence of that, and I don't plan to go back and look unless it becomes necessary, since I want to get the Fedora 20 kernel working, rather than the intermediate ones. When I boot the machine (after removing the "quiet rhgb" options, and adding "debug"), the machine spits out a bunch of log messages that scroll too quickly for me to see, and then the screens go blank (the LCD one displaying the message "No Signal"), and I can't access the machine remotely at that point; unfortunately this means that I also can't post the relevant dmesg and xorg information. I think the machine is hanging before the network card is enabled, although I will try to double-check that. As for bisecting the kernel, I can have a go, but I suspect it will be the original KMS commit. Thanks for the time you've put into this. I'll have some more time to put into it later, but probably not for another week or two, unfortunately. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/60369604/attachment.html>
[Bug 83792] Kernel hangs on boot without nomodeset option
https://bugs.freedesktop.org/show_bug.cgi?id=83792 --- Comment #3 from Christian K?nig --- Well the nomodeset option only makes sense with the UMS support and that was deprecated in January 2013. So as long as Fedora 20 doesn't compile their kernels with a deprecated feature they probably won't support nomodeset any more. Without any logs it would be rather hard to figure out what's going wrong here. So is there a possibility to attach for example a serial or network console to grab the last few lines of system log before the machine goes into nirvana? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/eb4c9997/attachment.html>
[Bug 83792] Kernel hangs on boot without nomodeset option
https://bugs.freedesktop.org/show_bug.cgi?id=83792 --- Comment #4 from Alex Deucher --- Can you also try blacklisting radeon and loading it after the box has booted and you have remote access? E.g., append modprobe.blacklist=radeon 3 to the kernel command line in grub (the 3 is to boot into runlevel 3 so X doesn't start). Then once the system is booted and you have remote access, as root, run: modprobe radeon and see if you can get the dmesg output. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/85292237/attachment.html>
Question on UAPI for fences
Hello everyone, to allow concurrent buffer access by different engines beyond the multiple readers/single writer model that we currently use in radeon and other drivers we need some kind of synchonization object exposed to userspace. My initial patch set for this used (or rather abused) zero sized GEM buffers as fence handles. This is obviously isn't the best way of doing this (to much overhead, rather ugly etc...), Jerome commented on this accordingly. So what should a driver expose instead? Android sync points? Something else? Please discuss and/or advise, Christian.
[Bug 83800] New: 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine
https://bugs.freedesktop.org/show_bug.cgi?id=83800 Priority: medium Bug ID: 83800 Assignee: dri-devel at lists.freedesktop.org Summary: 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine Severity: normal Classification: Unclassified OS: All Reporter: darkbasic at linuxsystems.it Hardware: Other Status: NEW Version: XOrg CVS Component: DRM/Radeon Product: DRI HD 7950 Linux 3.17.0-rc2-core-avx-i-drm-next-3.18 20140912 llvm 3.6 git xorg-server 1.15.1 + glamor git mesa git + gallium nine wine 1.7.26 + gallium nine -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/10385111/attachment.html>
[Bug 83800] 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine
https://bugs.freedesktop.org/show_bug.cgi?id=83800 --- Comment #1 from darkbasic --- Created attachment 106183 --> https://bugs.freedesktop.org/attachment.cgi?id=106183&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/cc89afc0/attachment.html>
[Bug 83800] 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine
https://bugs.freedesktop.org/show_bug.cgi?id=83800 --- Comment #2 from darkbasic --- Created attachment 106184 --> https://bugs.freedesktop.org/attachment.cgi?id=106184&action=edit Xorg.0.log -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/65fbad41/attachment-0001.html>
[PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
On Fri, Sep 12, 2014 at 11:27:41AM +0200, Andrzej Hajda wrote: > On 09/12/2014 10:57 AM, Daniel Vetter wrote: > > On Fri, Sep 12, 2014 at 05:34:50PM +0900, Inki Dae wrote: > >> Hi Andrzej, > >> > >> On 2014? 09? 09? 22:16, Andrzej Hajda wrote: > >>> Adding reference to framebuffer should be accompanied with removing > >>> reference to old framebuffer assigned to the plane. > >>> This patch removes following warning: > >>> > >>> [ 95.038017] WARNING: CPU: 1 PID: 3067 at > >>> drivers/gpu/drm/drm_crtc.c:5115 drm_mode_config_cleanup+0x258/0x268() > >>> [ 95.048086] Modules linked in: > >>> [ 95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: GW > >>> 3.16.0-11355-g7a6eca5-dirty #3015 > >>> [ 95.060058] [] (unwind_backtrace) from [] > >>> (show_stack+0x10/0x14) > >>> [ 95.067766] [] (show_stack) from [] > >>> (dump_stack+0x70/0xbc) > >>> [ 95.074953] [] (dump_stack) from [] > >>> (warn_slowpath_common+0x64/0x88) > >>> [ 95.083005] [] (warn_slowpath_common) from [] > >>> (warn_slowpath_null+0x1c/0x24) > >>> [ 95.091780] [] (warn_slowpath_null) from [] > >>> (drm_mode_config_cleanup+0x258/0x268) > >>> [ 95.100983] [] (drm_mode_config_cleanup) from [] > >>> (exynos_drm_unload+0x38/0x50) > >>> [ 95.109915] [] (exynos_drm_unload) from [] > >>> (drm_dev_unregister+0x24/0x98) > >>> [ 95.118414] [] (drm_dev_unregister) from [] > >>> (drm_put_dev+0x28/0x64) > >>> [ 95.126412] [] (drm_put_dev) from [] > >>> (take_down_master+0x24/0x44) > >>> [ 95.134218] [] (take_down_master) from [] > >>> (component_del+0x8c/0xc8) > >>> [ 95.142201] [] (component_del) from [] > >>> (exynos_dsi_remove+0x18/0x2c) > >>> [ 95.150294] [] (exynos_dsi_remove) from [] > >>> (platform_drv_remove+0x18/0x1c) > >>> [ 95.158872] [] (platform_drv_remove) from [] > >>> (__device_release_driver+0x70/0xc4) > >>> [ 95.167981] [] (__device_release_driver) from [] > >>> (device_release_driver+0x20/0x2c) > >>> [ 95.177268] [] (device_release_driver) from [] > >>> (unbind_store+0x5c/0x94) > >>> [ 95.185597] [] (unbind_store) from [] > >>> (drv_attr_store+0x20/0x2c) > >>> [ 95.193323] [] (drv_attr_store) from [] > >>> (sysfs_kf_write+0x4c/0x50) > >>> [ 95.201224] [] (sysfs_kf_write) from [] > >>> (kernfs_fop_write+0xc4/0x184) > >>> [ 95.209393] [] (kernfs_fop_write) from [] > >>> (vfs_write+0xa0/0x1a8) > >>> [ 95.217111] [] (vfs_write) from [] > >>> (SyS_write+0x40/0x8c) > >>> [ 95.224146] [] (SyS_write) from [] > >>> (ret_fast_syscall+0x0/0x48) > >>> > >>> Signed-off-by: Andrzej Hajda > >>> --- > >>> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > >>> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > >>> index b68e58f..bde19f4 100644 > >>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > >>> @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, > >>> struct drm_display_mode *mode, > >>> if (ret) > >>> return ret; > >>> > >>> + /* we need to unreference current fb after replacing it with new one */ > >>> + old_fb = plane->fb; > >>> + > >>> plane->crtc = crtc; > >>> plane->fb = crtc->primary->fb; > >>> drm_framebuffer_reference(plane->fb); > >>> > >>> + if (old_fb) > >>> + drm_framebuffer_unreference(old_fb); > >> This time would be a good chance that we can consider drm flip queue to > >> make sure that whole memory region to old_fb is scanned out completely > >> before dropping a reference of old_fb. the reference of old_fb should be > >> dropped at irq handler of each crtc devices, fimd and mixer. > > Generally it's not a good idea to drop fb references from irq context, > > since if you actually drop the last reference it'll blow up: fb cleanup > > needs a bunch of mutexes. > > I agree with that. > > > > > Also the drm core really should be taking care of this for you, you only > > need to grab references yourself for async flips if you want the buffer to > > survive a bit. crtc_mode_set has not need for this. I expect that the > > refcounting bug is somewhere else, at least from my experience chasing > > such issues in i915 ;-) > > Hmm, maybe I miss something but I do not see the core grabbing fb reference > on plane->fb update. On the other side drm_framebuffer_remove calls > drm_plane_force_disable which drops plane->fb reference. > I am not yet familiar with this code so maybe there is better solution. It does, see e.g. drm_mode_set_config_internal. All the other places also do it. > If not I guess it would be better to move this code to > exynos_plane_mode_set. > At least it is done this way in omap and msm, in fact it seems better place > for such things. What do you think? Drivers _only_ need to do their own refcounting if they do asynchronous updates (pageflips or asynchronous modesets). Which omap does (and iirc msm for pageflips). But if that need is comm
[Bug 83800] 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine
https://bugs.freedesktop.org/show_bug.cgi?id=83800 --- Comment #3 from darkbasic --- This is 100% reproducible: just start 3DMark2003 with a gallium nine enabled wine (and mesa of course) and it will crash your whole system. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/48f7d3d3/attachment.html>
[Bug 79980] Random radeonsi crashes
https://bugs.freedesktop.org/show_bug.cgi?id=79980 --- Comment #132 from darkbasic --- This is 100% reproducible: just start 3DMark2003 with a gallium nine enabled wine (and mesa of course) and it will crash your whole system: https://bugs.freedesktop.org/show_bug.cgi?id=83800 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/4f4daccd/attachment.html>
[Bug 83800] 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine
https://bugs.freedesktop.org/show_bug.cgi?id=83800 --- Comment #4 from Christian K?nig --- (In reply to comment #3) > This is 100% reproducible: just start 3DMark2003 with a gallium nine enabled > wine (and mesa of course) and it will crash your whole system. ^^ very interesting. Would it be possible that I get SSH access to the crashed system? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/b4cf1d78/attachment.html>
Question on UAPI for fences
On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: > Hello everyone, > > to allow concurrent buffer access by different engines beyond the multiple > readers/single writer model that we currently use in radeon and other > drivers we need some kind of synchonization object exposed to userspace. > > My initial patch set for this used (or rather abused) zero sized GEM buffers > as fence handles. This is obviously isn't the best way of doing this (to > much overhead, rather ugly etc...), Jerome commented on this accordingly. > > So what should a driver expose instead? Android sync points? Something else? I think actually exposing the struct fence objects as a fd, using android syncpts (or at least something compatible to it) is the way to go. Problem is that it's super-hard to get the android guys out of hiding for this :( Adding a bunch of people in the hopes that something sticks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 82828] Regression: Crash in 3Dmark2001
https://bugs.freedesktop.org/show_bug.cgi?id=82828 Andreas Boll changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #23 from Andreas Boll --- Fixed with commit afd82dcad127b64381ca6d80d0e499368074f474 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/1c7819b9/attachment-0001.html>
[PATCH 1/2 v2] vgaarb: Don't default exclusively to first video device with mem+io
On Fri, Sep 12, 2014 at 5:19 AM, Bruno Pr?mont wrote: > Bjorn, > > What is missing to get these two patches pushed to Linus? Sorry, I've been working on some other regressions and overlooked this one. If you open a bugzilla report and mark it as a regression, that will help keep this on my radar. Bjorn > On Thu, 28 Aug 2014 22:47:50 +0200 Bruno Pr?mont wrote: >> On Tue, 26 August 2014 Andreas Noever wrote: >> > On Sun, Aug 24, 2014 at 11:09 PM, Bruno Pr?mont wrote: >> > > With commit 20cde694027e boot video device detection was moved from >> > > efifb to x86 and ia64 pci/fixup.c. >> > > >> > > For dual-GPU Apple computers above change represents a regression as >> > > code in efifb did forcefully override vga_default_device while the >> > > merge did not (vgaarb happens prior to PCI fixup). >> > > >> > > To improve on initial device selection by vgaarb (it cannot know if >> > > PCI device not behind bridges see/decode legacy VGA I/O or not), move >> > > the screen_info based check from pci_video_fixup to vgaarb's init >> > > function and use it to refine/override decision taken while adding >> > > the individual PCI VGA devices. >> > > This way PCI fixup has no reason to adjust vga_default_device >> > > anymore but can depend on its value for flagging shadowed VBIOS. >> > > >> > > This has the nice benefit of removing duplicated code but does >> > > introduce a #if defined() block in vgaarb. >> > > Not all architectures have screen_info and would cause compile to >> > > fail without it. >> > > >> > > Reported-By: Andreas Noever >> > > CC: Matthew Garrett >> > > CC: stable at vger.kernel.org # v3.5+ >> > > Signed-off-by: Bruno Pr?mont >> > > --- >> > > Andreas, does this work properly for you, including the improvement >> > > on i915 complaint about VBIOS going from KERN_ERR to KERN_INFO? >> > Yep, thanks! >> > >> > > >> > > Other arches using PCI and vgaarb that have screen_info may want >> > > to be added to the #if defined() block or even introduce a new >> > > CONFIG_HAVE_SCREEN_INFO or similar... >> >> Bjorn, can you queue these two patches, probably going through -next >> for a week and passing them to Linus for -rc4, adding Andreas's Tested-by >> to Patch 1/2 v2? >> >> Thanks, >> Bruno
Question on UAPI for fences
On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: >> Hello everyone, >> >> to allow concurrent buffer access by different engines beyond the multiple >> readers/single writer model that we currently use in radeon and other >> drivers we need some kind of synchonization object exposed to userspace. >> >> My initial patch set for this used (or rather abused) zero sized GEM buffers >> as fence handles. This is obviously isn't the best way of doing this (to >> much overhead, rather ugly etc...), Jerome commented on this accordingly. >> >> So what should a driver expose instead? Android sync points? Something else? > > I think actually exposing the struct fence objects as a fd, using android > syncpts (or at least something compatible to it) is the way to go. Problem > is that it's super-hard to get the android guys out of hiding for this :( > > Adding a bunch of people in the hopes that something sticks. More people. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 83800] 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine
https://bugs.freedesktop.org/show_bug.cgi?id=83800 --- Comment #5 from darkbasic --- Of course, just give me a few days to setup a test system on another hard disk ;) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/a2f0c5c5/attachment.html>
Question on UAPI for fences
On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: > > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: > >> Hello everyone, > >> > >> to allow concurrent buffer access by different engines beyond the multiple > >> readers/single writer model that we currently use in radeon and other > >> drivers we need some kind of synchonization object exposed to userspace. > >> > >> My initial patch set for this used (or rather abused) zero sized GEM > >> buffers > >> as fence handles. This is obviously isn't the best way of doing this (to > >> much overhead, rather ugly etc...), Jerome commented on this accordingly. > >> > >> So what should a driver expose instead? Android sync points? Something > >> else? > > > > I think actually exposing the struct fence objects as a fd, using android > > syncpts (or at least something compatible to it) is the way to go. Problem > > is that it's super-hard to get the android guys out of hiding for this :( > > > > Adding a bunch of people in the hopes that something sticks. > > More people. Just to re-iterate, exposing such thing while still using command stream ioctl that use implicit synchronization is a waste and you can only get the lowest common denominator which is implicit synchronization. So i do not see the point of such api if you are not also adding a new cs ioctl with explicit contract that it does not do any kind of synchronization (it could be almost the exact same code modulo the do not wait for previous cmd to complete). Also one thing that the Android sync point does not have, AFAICT, is a way to schedule synchronization as part of a cs ioctl so cpu never have to be involve for cmd stream that deal only one gpu (assuming the driver and hw can do such trick). Cheers, J?r?me > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm: Assert correct locking for drm_send_vblank_event
The comment says that the caller must hold the dev->event_lock
spinlock, so let's enforce this.
A quick audit over all driver shows that except for the one place in
i915 which motivated this all callers fullfill this requirement
already.
Cc: Chris Wilson
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 80ff94ada75e..bf248eb9ffb2 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -907,6 +907,9 @@ void drm_send_vblank_event(struct drm_device *dev, int crtc,
{
struct timeval now;
unsigned int seq;
+
+ assert_spin_locked(&dev->event_lock);
+
if (crtc >= 0) {
seq = drm_vblank_count_and_time(dev, crtc, &now);
} else {
--
1.9.3
Question on UAPI for fences
On Fri, Sep 12, 2014 at 10:50:49AM -0400, Jerome Glisse wrote: > On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: > > On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: > > > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: > > >> Hello everyone, > > >> > > >> to allow concurrent buffer access by different engines beyond the > > >> multiple > > >> readers/single writer model that we currently use in radeon and other > > >> drivers we need some kind of synchonization object exposed to userspace. > > >> > > >> My initial patch set for this used (or rather abused) zero sized GEM > > >> buffers > > >> as fence handles. This is obviously isn't the best way of doing this (to > > >> much overhead, rather ugly etc...), Jerome commented on this accordingly. > > >> > > >> So what should a driver expose instead? Android sync points? Something > > >> else? > > > > > > I think actually exposing the struct fence objects as a fd, using android > > > syncpts (or at least something compatible to it) is the way to go. Problem > > > is that it's super-hard to get the android guys out of hiding for this :( > > > > > > Adding a bunch of people in the hopes that something sticks. > > > > More people. > > Just to re-iterate, exposing such thing while still using command stream > ioctl that use implicit synchronization is a waste and you can only get > the lowest common denominator which is implicit synchronization. So i do > not see the point of such api if you are not also adding a new cs ioctl > with explicit contract that it does not do any kind of synchronization > (it could be almost the exact same code modulo the do not wait for > previous cmd to complete). I don't think we should cathegorically exclude this, since without some partial implicit/explicit world we'll never convert over to fences. Of course adding fences without any way to at least partially forgoe the implicit syncing is pointless. But that might be some other user (e.g. camera capture device) which needs explicit fences. > Also one thing that the Android sync point does not have, AFAICT, is a > way to schedule synchronization as part of a cs ioctl so cpu never have > to be involve for cmd stream that deal only one gpu (assuming the driver > and hw can do such trick). You need to integrate the android stuff with your (new) cs ioctl, with a input parameter for the fence fd to wait on before executing the cs and one that gets created to signal when it's all done. Same goes for all the other places android wants sync objects, e.g. for synchronization before atomic flips and for signalling completion of the same. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm: Assert correct locking for drm_send_vblank_event
On Fri, Sep 12, 2014 at 03:40:56PM +0200, Daniel Vetter wrote: > The comment says that the caller must hold the dev->event_lock > spinlock, so let's enforce this. > > A quick audit over all driver shows that except for the one place in > i915 which motivated this all callers fullfill this requirement > already. Replace the rogue WARN_ON_SMP(!spin_is_locked(&dev->event_lock)) in send_vblank_event() as well then. -Chris -- Chris Wilson, Intel Open Source Technology Centre
Question on UAPI for fences
On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse wrote: > On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: >> On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: >> > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: >> >> Hello everyone, >> >> >> >> to allow concurrent buffer access by different engines beyond the multiple >> >> readers/single writer model that we currently use in radeon and other >> >> drivers we need some kind of synchonization object exposed to userspace. >> >> >> >> My initial patch set for this used (or rather abused) zero sized GEM >> >> buffers >> >> as fence handles. This is obviously isn't the best way of doing this (to >> >> much overhead, rather ugly etc...), Jerome commented on this accordingly. >> >> >> >> So what should a driver expose instead? Android sync points? Something >> >> else? >> > >> > I think actually exposing the struct fence objects as a fd, using android >> > syncpts (or at least something compatible to it) is the way to go. Problem >> > is that it's super-hard to get the android guys out of hiding for this :( >> > >> > Adding a bunch of people in the hopes that something sticks. >> >> More people. > > Just to re-iterate, exposing such thing while still using command stream > ioctl that use implicit synchronization is a waste and you can only get > the lowest common denominator which is implicit synchronization. So i do > not see the point of such api if you are not also adding a new cs ioctl > with explicit contract that it does not do any kind of synchronization > (it could be almost the exact same code modulo the do not wait for > previous cmd to complete). Our thinking was to allow explicit sync from a single process, but implicitly sync between processes. Alex > > Also one thing that the Android sync point does not have, AFAICT, is a > way to schedule synchronization as part of a cs ioctl so cpu never have > to be involve for cmd stream that deal only one gpu (assuming the driver > and hw can do such trick). > > Cheers, > J?r?me > >> -Daniel >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Question on UAPI for fences
On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: > On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse wrote: > > On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: > >> On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: > >> > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: > >> >> Hello everyone, > >> >> > >> >> to allow concurrent buffer access by different engines beyond the > >> >> multiple > >> >> readers/single writer model that we currently use in radeon and other > >> >> drivers we need some kind of synchonization object exposed to userspace. > >> >> > >> >> My initial patch set for this used (or rather abused) zero sized GEM > >> >> buffers > >> >> as fence handles. This is obviously isn't the best way of doing this (to > >> >> much overhead, rather ugly etc...), Jerome commented on this > >> >> accordingly. > >> >> > >> >> So what should a driver expose instead? Android sync points? Something > >> >> else? > >> > > >> > I think actually exposing the struct fence objects as a fd, using android > >> > syncpts (or at least something compatible to it) is the way to go. > >> > Problem > >> > is that it's super-hard to get the android guys out of hiding for this :( > >> > > >> > Adding a bunch of people in the hopes that something sticks. > >> > >> More people. > > > > Just to re-iterate, exposing such thing while still using command stream > > ioctl that use implicit synchronization is a waste and you can only get > > the lowest common denominator which is implicit synchronization. So i do > > not see the point of such api if you are not also adding a new cs ioctl > > with explicit contract that it does not do any kind of synchronization > > (it could be almost the exact same code modulo the do not wait for > > previous cmd to complete). > > Our thinking was to allow explicit sync from a single process, but > implicitly sync between processes. This is a BIG NAK if you are using the same ioctl as it would mean you are changing userspace API, well at least userspace expectation. Adding a new cs flag might do the trick but it should not be about inter-process, or any thing special, it's just implicit sync or no synchronization. Converting userspace is not that much of a big deal either, it can be broken into several step. Like mesa use explicit synchronization all time but ddx use implicit. Cheers, J?r?me > > Alex > > > > > Also one thing that the Android sync point does not have, AFAICT, is a > > way to schedule synchronization as part of a cs ioctl so cpu never have > > to be involve for cmd stream that deal only one gpu (assuming the driver > > and hw can do such trick). > > > > Cheers, > > J?r?me > > > >> -Daniel > >> -- > >> Daniel Vetter > >> Software Engineer, Intel Corporation > >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > ___ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH] drm: Assert correct locking for drm_send_vblank_event
On Fri, Sep 12, 2014 at 04:23:29PM +0100, Chris Wilson wrote: > On Fri, Sep 12, 2014 at 03:40:56PM +0200, Daniel Vetter wrote: > > The comment says that the caller must hold the dev->event_lock > > spinlock, so let's enforce this. > > > > A quick audit over all driver shows that except for the one place in > > i915 which motivated this all callers fullfill this requirement > > already. > > Replace the rogue WARN_ON_SMP(!spin_is_locked(&dev->event_lock)) in > send_vblank_event() as well then. Meh, I've missed that one, that's actually better I think. I'll drop my patch here. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 83800] 3DMark2003 crashes radeon with wine 1.7.26 + gallium nine
https://bugs.freedesktop.org/show_bug.cgi?id=83800 --- Comment #6 from David Heidelberger (okias) --- HD 6550D (r600g) Linux 3.16.1 llvm 3.5 git xorg-server-git mesa git + gallium nine wine 1.7.26 + gallium nine just getting spammed by ... EE r600_state_common.c:751 r600_shader_select - Failed to build shader variant (type=0) -22 EE r600_shader.c:2293 tgsi_unsupported - CLAMP tgsi opcode unsupported EE r600_shader.c:157 r600_pipe_shader_create - translation from TGSI failed ! EE r600_state_common.c:751 r600_shader_select - Failed to build shader variant (type=0) -22 EE r600_shader.c:2293 tgsi_unsupported - CLAMP tgsi opcode unsupported EE r600_shader.c:157 r600_pipe_shader_create - translation from TGSI failed ! ... otherwise OK, i'll try investigate but maybe it's somehow not handled on radeonsi. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/8add15b1/attachment.html>
Question on UAPI for fences
On Fri, Sep 12, 2014 at 11:33 AM, Jerome Glisse wrote: > On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: >> On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse >> wrote: >> > On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: >> >> On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: >> >> > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: >> >> >> Hello everyone, >> >> >> >> >> >> to allow concurrent buffer access by different engines beyond the >> >> >> multiple >> >> >> readers/single writer model that we currently use in radeon and other >> >> >> drivers we need some kind of synchonization object exposed to >> >> >> userspace. >> >> >> >> >> >> My initial patch set for this used (or rather abused) zero sized GEM >> >> >> buffers >> >> >> as fence handles. This is obviously isn't the best way of doing this >> >> >> (to >> >> >> much overhead, rather ugly etc...), Jerome commented on this >> >> >> accordingly. >> >> >> >> >> >> So what should a driver expose instead? Android sync points? Something >> >> >> else? >> >> > >> >> > I think actually exposing the struct fence objects as a fd, using >> >> > android >> >> > syncpts (or at least something compatible to it) is the way to go. >> >> > Problem >> >> > is that it's super-hard to get the android guys out of hiding for this >> >> > :( >> >> > >> >> > Adding a bunch of people in the hopes that something sticks. >> >> >> >> More people. >> > >> > Just to re-iterate, exposing such thing while still using command stream >> > ioctl that use implicit synchronization is a waste and you can only get >> > the lowest common denominator which is implicit synchronization. So i do >> > not see the point of such api if you are not also adding a new cs ioctl >> > with explicit contract that it does not do any kind of synchronization >> > (it could be almost the exact same code modulo the do not wait for >> > previous cmd to complete). >> >> Our thinking was to allow explicit sync from a single process, but >> implicitly sync between processes. > > This is a BIG NAK if you are using the same ioctl as it would mean you are > changing userspace API, well at least userspace expectation. Adding a new > cs flag might do the trick but it should not be about inter-process, or any > thing special, it's just implicit sync or no synchronization. Converting > userspace is not that much of a big deal either, it can be broken into > several step. Like mesa use explicit synchronization all time but ddx use > implicit. Right, you'd have to explicitly ask for it to avoid breaking old userspace. My point was just that within a single process, it's quite easy to know exactly what you are doing and handle the synchronization yourself, while for inter-process there is an assumed implicit sync. Alex > > Cheers, > J?r?me > >> >> Alex >> >> > >> > Also one thing that the Android sync point does not have, AFAICT, is a >> > way to schedule synchronization as part of a cs ioctl so cpu never have >> > to be involve for cmd stream that deal only one gpu (assuming the driver >> > and hw can do such trick). >> > >> > Cheers, >> > J?r?me >> > >> >> -Daniel >> >> -- >> >> Daniel Vetter >> >> Software Engineer, Intel Corporation >> >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> > ___ >> > dri-devel mailing list >> > dri-devel at lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] topic/drm-header-rework
Hi Dave, So here's the header cleanup, rebased on top of drm-next. Two new header files are created here: - drivers/gpu/drm/drm_internal.h for non-legacy drm.ko private declarations. - include/drm/drm_legacy.h for legacy interfaces used by non-kms drivers. And of course lots fo stuff gets shuffled into the already existing drivers/gpu/drm/drm_legacy.h for drm.ko internal stuff. topic branch smoke-tested in drm-intel-nightly for a bit. And the 0day tester also worked through it (and found a few places I didn't add a static to functions). There's still two pull requests besides that one: - topic/core-stuff, as usual - I'll send you that soon - Last i915 feature pull for 3.18 which will happen only in a week. As discussed on irc QA needs that to test it, but otherwise it's frozen and already in linux-next anyway. Cheers, Daniel The following changes since commit edbaae5a5cab89de0e64b8c03ebd9a8d5d266550: Merge tag 'topic/vblank-rework-2014-09-12' of git://anongit.freedesktop.org/drm-intel into drm-next (2014-09-12 19:04:53 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/drm-header-rework-2014-09-12 for you to fetch changes up to 6865b20ad354548a045c74a388eb37afe1ad5174: drm: Move DRM_MAGIC_HASH_ORDER into drm_drv.c (2014-09-12 15:28:14 +0200) Daniel Vetter (15): drm: Move dma functions into drm_legacy.h drm: Move sg functions into drm_legacy.h drm: Move drm_legacy_vma_flush into drm_legacy.h drm: Create drm legacy driver header drm: Move __drm_pci_free to drm_legacy.h drm: Drop drm_sysfs_class from drmP.h drm: Move vblank related module options into drm_irq.c drm: Move piles of functions from drmP.h to drm_internal.h drm: unexport drm_global_mutex drm: Purge ioctl forward declarations from drmP.h drm: Move drm_memory.c map support declarations to drm: Move legacy buffer structures to drm: Move LOCK_TEST_WITH_RETURN to drm: Move drm_class to drm_internal.h drm: Move DRM_MAGIC_HASH_ORDER into drm_drv.c drivers/gpu/drm/drm_auth.c| 1 + drivers/gpu/drm/drm_bufs.c| 4 +- drivers/gpu/drm/drm_crtc.c| 1 + drivers/gpu/drm/drm_debugfs.c | 1 + drivers/gpu/drm/drm_dma.c | 11 +- drivers/gpu/drm/drm_drv.c | 16 +- drivers/gpu/drm/drm_fops.c| 4 +- drivers/gpu/drm/drm_gem.c | 1 + drivers/gpu/drm/drm_info.c| 1 + drivers/gpu/drm/drm_internal.h| 95 drivers/gpu/drm/drm_ioctl.c | 247 +++--- drivers/gpu/drm/drm_irq.c | 15 ++ drivers/gpu/drm/drm_legacy.h | 18 +++ drivers/gpu/drm/drm_lock.c| 1 + drivers/gpu/drm/drm_memory.c | 12 +- drivers/gpu/drm/drm_pci.c | 5 +- drivers/gpu/drm/drm_prime.c | 1 + drivers/gpu/drm/drm_scatter.c | 9 +- drivers/gpu/drm/drm_sysfs.c | 1 + drivers/gpu/drm/drm_vm.c | 2 +- drivers/gpu/drm/i810/i810_dma.c | 12 +- drivers/gpu/drm/i810/i810_drv.h | 2 + drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 9 +- drivers/gpu/drm/mga/mga_dma.c | 28 ++-- drivers/gpu/drm/mga/mga_drv.h | 2 + drivers/gpu/drm/mgag200/mgag200_drv.h | 2 - drivers/gpu/drm/r128/r128_cce.c | 22 +-- drivers/gpu/drm/r128/r128_drv.h | 2 + drivers/gpu/drm/radeon/r600_cp.c | 24 +-- drivers/gpu/drm/radeon/radeon_cp.c| 24 +-- drivers/gpu/drm/radeon/radeon_drv.h | 1 + drivers/gpu/drm/savage/savage_bci.c | 16 +- drivers/gpu/drm/savage/savage_drv.h | 2 + drivers/gpu/drm/sis/sis_drv.h | 2 + drivers/gpu/drm/via/via_dma.c | 4 +- drivers/gpu/drm/via/via_drv.h | 2 + drivers/gpu/drm/via/via_map.c | 4 +- drivers/gpu/drm/via/via_verifier.c| 1 + include/drm/ati_pcigart.h | 2 + include/drm/drmP.h| 279 ++ include/drm/drm_legacy.h | 202 42 files changed, 588 insertions(+), 501 deletions(-) create mode 100644 drivers/gpu/drm/drm_internal.h create mode 100644 include/drm/drm_legacy.h -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Question on UAPI for fences
Am 12.09.2014 um 17:33 schrieb Jerome Glisse: > On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: >> On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse >> wrote: >>> On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: >> Hello everyone, >> >> to allow concurrent buffer access by different engines beyond the >> multiple >> readers/single writer model that we currently use in radeon and other >> drivers we need some kind of synchonization object exposed to userspace. >> >> My initial patch set for this used (or rather abused) zero sized GEM >> buffers >> as fence handles. This is obviously isn't the best way of doing this (to >> much overhead, rather ugly etc...), Jerome commented on this accordingly. >> >> So what should a driver expose instead? Android sync points? Something >> else? > I think actually exposing the struct fence objects as a fd, using android > syncpts (or at least something compatible to it) is the way to go. Problem > is that it's super-hard to get the android guys out of hiding for this :( > > Adding a bunch of people in the hopes that something sticks. More people. >>> Just to re-iterate, exposing such thing while still using command stream >>> ioctl that use implicit synchronization is a waste and you can only get >>> the lowest common denominator which is implicit synchronization. So i do >>> not see the point of such api if you are not also adding a new cs ioctl >>> with explicit contract that it does not do any kind of synchronization >>> (it could be almost the exact same code modulo the do not wait for >>> previous cmd to complete). >> Our thinking was to allow explicit sync from a single process, but >> implicitly sync between processes. > This is a BIG NAK if you are using the same ioctl as it would mean you are > changing userspace API, well at least userspace expectation. Adding a new > cs flag might do the trick but it should not be about inter-process, or any > thing special, it's just implicit sync or no synchronization. Converting > userspace is not that much of a big deal either, it can be broken into > several step. Like mesa use explicit synchronization all time but ddx use > implicit. The thinking here is that we need to be backward compatible for DRI2/3 and support all kind of different use cases like old DDX and new Mesa, or old Mesa and new DDX etc... So for my prototype if the kernel sees any access of a BO from two different clients it falls back to the old behavior of implicit synchronization of access to the same buffer object. That might not be the fastest approach, but is as far as I can see conservative and so should work under all conditions. Apart from that the planning so far was that we just hide this feature behind a couple of command submission flags and new chunks. Regards, Christian. > > Cheers, > J?r?me > >> Alex >> >>> Also one thing that the Android sync point does not have, AFAICT, is a >>> way to schedule synchronization as part of a cs ioctl so cpu never have >>> to be involve for cmd stream that deal only one gpu (assuming the driver >>> and hw can do such trick). >>> >>> Cheers, >>> J?r?me >>> -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch >>> ___ >>> dri-devel mailing list >>> dri-devel at lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Question on UAPI for fences
On Fri, Sep 12, 2014 at 05:42:57PM +0200, Christian K?nig wrote: > Am 12.09.2014 um 17:33 schrieb Jerome Glisse: > >On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: > >>On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse > >>wrote: > >>>On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: > >On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: > >>Hello everyone, > >> > >>to allow concurrent buffer access by different engines beyond the > >>multiple > >>readers/single writer model that we currently use in radeon and other > >>drivers we need some kind of synchonization object exposed to userspace. > >> > >>My initial patch set for this used (or rather abused) zero sized GEM > >>buffers > >>as fence handles. This is obviously isn't the best way of doing this (to > >>much overhead, rather ugly etc...), Jerome commented on this > >>accordingly. > >> > >>So what should a driver expose instead? Android sync points? Something > >>else? > >I think actually exposing the struct fence objects as a fd, using android > >syncpts (or at least something compatible to it) is the way to go. > >Problem > >is that it's super-hard to get the android guys out of hiding for this :( > > > >Adding a bunch of people in the hopes that something sticks. > More people. > >>>Just to re-iterate, exposing such thing while still using command stream > >>>ioctl that use implicit synchronization is a waste and you can only get > >>>the lowest common denominator which is implicit synchronization. So i do > >>>not see the point of such api if you are not also adding a new cs ioctl > >>>with explicit contract that it does not do any kind of synchronization > >>>(it could be almost the exact same code modulo the do not wait for > >>>previous cmd to complete). > >>Our thinking was to allow explicit sync from a single process, but > >>implicitly sync between processes. > >This is a BIG NAK if you are using the same ioctl as it would mean you are > >changing userspace API, well at least userspace expectation. Adding a new > >cs flag might do the trick but it should not be about inter-process, or any > >thing special, it's just implicit sync or no synchronization. Converting > >userspace is not that much of a big deal either, it can be broken into > >several step. Like mesa use explicit synchronization all time but ddx use > >implicit. > > The thinking here is that we need to be backward compatible for DRI2/3 and > support all kind of different use cases like old DDX and new Mesa, or old > Mesa and new DDX etc... > > So for my prototype if the kernel sees any access of a BO from two different > clients it falls back to the old behavior of implicit synchronization of > access to the same buffer object. That might not be the fastest approach, > but is as far as I can see conservative and so should work under all > conditions. > > Apart from that the planning so far was that we just hide this feature > behind a couple of command submission flags and new chunks. Just to reproduce IRC discussion, i think it's a lot simpler and not that complex. For explicit cs ioctl you do not wait for any previous fence of any of the buffer referenced in the cs ioctl, but you still associate a new fence with all the buffer object referenced in the cs ioctl. So if the next ioctl is an implicit sync ioctl it will wait properly and synchronize properly with previous explicit cs ioctl. Hence you can easily have a mix in userspace thing is you only get benefit once enough of your userspace is using explicit. Note that you still need a way to have explicit cs ioctl to wait on a previos "explicit" fence so you need some api to expose fence per cs submission. Cheers, J?r?me > > Regards, > Christian. > > > > >Cheers, > >J?r?me > > > >>Alex > >> > >>>Also one thing that the Android sync point does not have, AFAICT, is a > >>>way to schedule synchronization as part of a cs ioctl so cpu never have > >>>to be involve for cmd stream that deal only one gpu (assuming the driver > >>>and hw can do such trick). > >>> > >>>Cheers, > >>>J?r?me > >>> > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > >>>___ > >>>dri-devel mailing list > >>>dri-devel at lists.freedesktop.org > >>>http://lists.freedesktop.org/mailman/listinfo/dri-devel >
Question on UAPI for fences
On Fri, Sep 12, 2014 at 05:58:09PM +0200, Christian K?nig wrote: > Am 12.09.2014 um 17:48 schrieb Jerome Glisse: > >On Fri, Sep 12, 2014 at 05:42:57PM +0200, Christian K?nig wrote: > >>Am 12.09.2014 um 17:33 schrieb Jerome Glisse: > >>>On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: > On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse > wrote: > >On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: > >>On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter > >>wrote: > >>>On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: > Hello everyone, > > to allow concurrent buffer access by different engines beyond the > multiple > readers/single writer model that we currently use in radeon and other > drivers we need some kind of synchonization object exposed to > userspace. > > My initial patch set for this used (or rather abused) zero sized GEM > buffers > as fence handles. This is obviously isn't the best way of doing this > (to > much overhead, rather ugly etc...), Jerome commented on this > accordingly. > > So what should a driver expose instead? Android sync points? > Something else? > >>>I think actually exposing the struct fence objects as a fd, using > >>>android > >>>syncpts (or at least something compatible to it) is the way to go. > >>>Problem > >>>is that it's super-hard to get the android guys out of hiding for this > >>>:( > >>> > >>>Adding a bunch of people in the hopes that something sticks. > >>More people. > >Just to re-iterate, exposing such thing while still using command stream > >ioctl that use implicit synchronization is a waste and you can only get > >the lowest common denominator which is implicit synchronization. So i do > >not see the point of such api if you are not also adding a new cs ioctl > >with explicit contract that it does not do any kind of synchronization > >(it could be almost the exact same code modulo the do not wait for > >previous cmd to complete). > Our thinking was to allow explicit sync from a single process, but > implicitly sync between processes. > >>>This is a BIG NAK if you are using the same ioctl as it would mean you are > >>>changing userspace API, well at least userspace expectation. Adding a new > >>>cs flag might do the trick but it should not be about inter-process, or any > >>>thing special, it's just implicit sync or no synchronization. Converting > >>>userspace is not that much of a big deal either, it can be broken into > >>>several step. Like mesa use explicit synchronization all time but ddx use > >>>implicit. > >>The thinking here is that we need to be backward compatible for DRI2/3 and > >>support all kind of different use cases like old DDX and new Mesa, or old > >>Mesa and new DDX etc... > >> > >>So for my prototype if the kernel sees any access of a BO from two different > >>clients it falls back to the old behavior of implicit synchronization of > >>access to the same buffer object. That might not be the fastest approach, > >>but is as far as I can see conservative and so should work under all > >>conditions. > >> > >>Apart from that the planning so far was that we just hide this feature > >>behind a couple of command submission flags and new chunks. > >Just to reproduce IRC discussion, i think it's a lot simpler and not that > >complex. For explicit cs ioctl you do not wait for any previous fence of > >any of the buffer referenced in the cs ioctl, but you still associate a > >new fence with all the buffer object referenced in the cs ioctl. So if the > >next ioctl is an implicit sync ioctl it will wait properly and synchronize > >properly with previous explicit cs ioctl. Hence you can easily have a mix > >in userspace thing is you only get benefit once enough of your userspace > >is using explicit. > > Yes, that's exactly what my patches currently implement. > > The only difference is that by current planning I implemented it as a per BO > flag for the command submission, but that was just for testing. Having a > single flag to switch between implicit and explicit synchronization for > whole CS IOCTL would do equally well. Doing it per BO sounds bogus to me. But otherwise yes we are in agreement. As Daniel said using fd is most likely the way we want to do it but this remains vague. > > >Note that you still need a way to have explicit cs ioctl to wait on a > >previos "explicit" fence so you need some api to expose fence per cs > >submission. > > Exactly, that's what this mail thread is all about. > > As Daniel correctly noted you need something like a functionality to get a > fence as the result of a command submission as well as pass in a list of > fences to wait for before beginning a command submission. > > At least it looks like we are all on the same general line
[Intel-gfx] [PATCH] drm: Assert correct locking for drm_send_vblank_event
On Fri, Sep 12, 2014 at 05:34:56PM +0200, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 04:23:29PM +0100, Chris Wilson wrote: > > On Fri, Sep 12, 2014 at 03:40:56PM +0200, Daniel Vetter wrote: > > > The comment says that the caller must hold the dev->event_lock > > > spinlock, so let's enforce this. > > > > > > A quick audit over all driver shows that except for the one place in > > > i915 which motivated this all callers fullfill this requirement > > > already. > > > > Replace the rogue WARN_ON_SMP(!spin_is_locked(&dev->event_lock)) in > > send_vblank_event() as well then. > > Meh, I've missed that one, that's actually better I think. I'll drop my > patch here. I thought assert_spin_lock was the preferred form? -Chris -- Chris Wilson, Intel Open Source Technology Centre
Question on UAPI for fences
> As Daniel said using fd is most likely the way we want to do it but this > remains vague. Separating the discussion if it should be an fd or not. Using an fd sounds fine to me in general, but I have some concerns as well. For example what was the maximum number of opened FDs per process again? Could that become a problem? etc... Please comment, Christian. Am 12.09.2014 um 18:03 schrieb Jerome Glisse: > On Fri, Sep 12, 2014 at 05:58:09PM +0200, Christian K?nig wrote: >> Am 12.09.2014 um 17:48 schrieb Jerome Glisse: >>> On Fri, Sep 12, 2014 at 05:42:57PM +0200, Christian K?nig wrote: Am 12.09.2014 um 17:33 schrieb Jerome Glisse: > On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: >> On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse >> wrote: >>> On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: >> Hello everyone, >> >> to allow concurrent buffer access by different engines beyond the >> multiple >> readers/single writer model that we currently use in radeon and other >> drivers we need some kind of synchonization object exposed to >> userspace. >> >> My initial patch set for this used (or rather abused) zero sized GEM >> buffers >> as fence handles. This is obviously isn't the best way of doing this >> (to >> much overhead, rather ugly etc...), Jerome commented on this >> accordingly. >> >> So what should a driver expose instead? Android sync points? >> Something else? > I think actually exposing the struct fence objects as a fd, using > android > syncpts (or at least something compatible to it) is the way to go. > Problem > is that it's super-hard to get the android guys out of hiding for > this :( > > Adding a bunch of people in the hopes that something sticks. More people. >>> Just to re-iterate, exposing such thing while still using command stream >>> ioctl that use implicit synchronization is a waste and you can only get >>> the lowest common denominator which is implicit synchronization. So i do >>> not see the point of such api if you are not also adding a new cs ioctl >>> with explicit contract that it does not do any kind of synchronization >>> (it could be almost the exact same code modulo the do not wait for >>> previous cmd to complete). >> Our thinking was to allow explicit sync from a single process, but >> implicitly sync between processes. > This is a BIG NAK if you are using the same ioctl as it would mean you are > changing userspace API, well at least userspace expectation. Adding a new > cs flag might do the trick but it should not be about inter-process, or > any > thing special, it's just implicit sync or no synchronization. Converting > userspace is not that much of a big deal either, it can be broken into > several step. Like mesa use explicit synchronization all time but ddx use > implicit. The thinking here is that we need to be backward compatible for DRI2/3 and support all kind of different use cases like old DDX and new Mesa, or old Mesa and new DDX etc... So for my prototype if the kernel sees any access of a BO from two different clients it falls back to the old behavior of implicit synchronization of access to the same buffer object. That might not be the fastest approach, but is as far as I can see conservative and so should work under all conditions. Apart from that the planning so far was that we just hide this feature behind a couple of command submission flags and new chunks. >>> Just to reproduce IRC discussion, i think it's a lot simpler and not that >>> complex. For explicit cs ioctl you do not wait for any previous fence of >>> any of the buffer referenced in the cs ioctl, but you still associate a >>> new fence with all the buffer object referenced in the cs ioctl. So if the >>> next ioctl is an implicit sync ioctl it will wait properly and synchronize >>> properly with previous explicit cs ioctl. Hence you can easily have a mix >>> in userspace thing is you only get benefit once enough of your userspace >>> is using explicit. >> Yes, that's exactly what my patches currently implement. >> >> The only difference is that by current planning I implemented it as a per BO >> flag for the command submission, but that was just for testing. Having a >> single flag to switch between implicit and explicit synchronization for >> whole CS IOCTL would do equally well. > Doing it per BO sounds bogus to me. But otherwise yes we are in agreement. > As Daniel said using fd is most likely the way we want to do it but this > remains vague. >
Question on UAPI for fences
Am 12.09.2014 um 17:48 schrieb Jerome Glisse: > On Fri, Sep 12, 2014 at 05:42:57PM +0200, Christian K?nig wrote: >> Am 12.09.2014 um 17:33 schrieb Jerome Glisse: >>> On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse wrote: > On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: >> On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter >> wrote: >>> On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: Hello everyone, to allow concurrent buffer access by different engines beyond the multiple readers/single writer model that we currently use in radeon and other drivers we need some kind of synchonization object exposed to userspace. My initial patch set for this used (or rather abused) zero sized GEM buffers as fence handles. This is obviously isn't the best way of doing this (to much overhead, rather ugly etc...), Jerome commented on this accordingly. So what should a driver expose instead? Android sync points? Something else? >>> I think actually exposing the struct fence objects as a fd, using >>> android >>> syncpts (or at least something compatible to it) is the way to go. >>> Problem >>> is that it's super-hard to get the android guys out of hiding for this >>> :( >>> >>> Adding a bunch of people in the hopes that something sticks. >> More people. > Just to re-iterate, exposing such thing while still using command stream > ioctl that use implicit synchronization is a waste and you can only get > the lowest common denominator which is implicit synchronization. So i do > not see the point of such api if you are not also adding a new cs ioctl > with explicit contract that it does not do any kind of synchronization > (it could be almost the exact same code modulo the do not wait for > previous cmd to complete). Our thinking was to allow explicit sync from a single process, but implicitly sync between processes. >>> This is a BIG NAK if you are using the same ioctl as it would mean you are >>> changing userspace API, well at least userspace expectation. Adding a new >>> cs flag might do the trick but it should not be about inter-process, or any >>> thing special, it's just implicit sync or no synchronization. Converting >>> userspace is not that much of a big deal either, it can be broken into >>> several step. Like mesa use explicit synchronization all time but ddx use >>> implicit. >> The thinking here is that we need to be backward compatible for DRI2/3 and >> support all kind of different use cases like old DDX and new Mesa, or old >> Mesa and new DDX etc... >> >> So for my prototype if the kernel sees any access of a BO from two different >> clients it falls back to the old behavior of implicit synchronization of >> access to the same buffer object. That might not be the fastest approach, >> but is as far as I can see conservative and so should work under all >> conditions. >> >> Apart from that the planning so far was that we just hide this feature >> behind a couple of command submission flags and new chunks. > Just to reproduce IRC discussion, i think it's a lot simpler and not that > complex. For explicit cs ioctl you do not wait for any previous fence of > any of the buffer referenced in the cs ioctl, but you still associate a > new fence with all the buffer object referenced in the cs ioctl. So if the > next ioctl is an implicit sync ioctl it will wait properly and synchronize > properly with previous explicit cs ioctl. Hence you can easily have a mix > in userspace thing is you only get benefit once enough of your userspace > is using explicit. Yes, that's exactly what my patches currently implement. The only difference is that by current planning I implemented it as a per BO flag for the command submission, but that was just for testing. Having a single flag to switch between implicit and explicit synchronization for whole CS IOCTL would do equally well. > Note that you still need a way to have explicit cs ioctl to wait on a > previos "explicit" fence so you need some api to expose fence per cs > submission. Exactly, that's what this mail thread is all about. As Daniel correctly noted you need something like a functionality to get a fence as the result of a command submission as well as pass in a list of fences to wait for before beginning a command submission. At least it looks like we are all on the same general line here, its just nobody has a good idea how the details should look like. Regards, Christian. > > Cheers, > J?r?me > >> Regards, >> Christian. >> >>> Cheers, >>> J?r?me >>> Alex > Also one thing that the Android sync point does not have, AFAICT, is a > way to schedule synchronization as part of a cs ioctl so cpu never
[RFC v2] device coredump: add new device coredump class
On Mon, Sep 08, 2014 at 10:38:07AM +0200, Johannes Berg wrote: > On Fri, 2014-09-05 at 15:13 -0700, Greg Kroah-Hartman wrote: > > > > + /* > > > + * this seems racy, but I don't see a notifier or such on > > > + * a struct device to know when it goes away? > > > + */ > > > + if (devcd->failing_dev->kobj.sd) > > > + sysfs_delete_link(&devcd->failing_dev->kobj, &dev->kobj, > > > + "dev_coredump"); > > > > What is this link? It should "just go away" if this: > > > > > + put_device(devcd->failing_dev); > > > > was the last put_device() call on the failing_dev, right? So you > > shouldn't need to make this call to sysfs_delete_link(). > > I looked at this again, and it's the other way around. This is the link > that Daniel requested, from the original device to the one that's being > freed. For whatever reason though, symlinks don't automatically go away > when freed: > > (with a test patch that makes "mac80211-hwsim" crash whenever we have > radar) > > # echo 1 > /sys/kernel/debug/ieee80211/phy0/hwsim/dfs_simulate_radar > # ls /sys/class/devcoredump/ > devcd1 > # ls /sys/class/devcoredump/devcd1/ > datafailing_device/ power/ subsystem/ uevent > # ls -l /sys/class/mac80211_hwsim/hwsim0/dev_coredump > lrwxrwxrwx 1 root root 0 Sep 8 08:34 > /sys/class/mac80211_hwsim/hwsim0/dev_coredump -> ../../devcoredump/devcd1 > # echo > /sys/class/devcoredump/devcd1/data > # ls /sys/class/devcoredump/ > # ls -l /sys/class/mac80211_hwsim/hwsim0/dev_coredump > lrwxrwxrwx 1 root root 0 Sep 8 08:34 > /sys/class/mac80211_hwsim/hwsim0/dev_coredump -> ../../devcoredump/devcd1 > > (but the link is now dead) > > Maybe I'm creating the links in the wrong way so they don't > automatically get removed when the struct device is released? No, you are correct, sorry for the noise, I thought the symlink was the other way around for some reason. greg k-h
Question on UAPI for fences
On Fri, 12 Sep 2014 18:08:23 +0200 Christian K?nig wrote: > > As Daniel said using fd is most likely the way we want to do it but this > > remains vague. > Separating the discussion if it should be an fd or not. Using an fd > sounds fine to me in general, but I have some concerns as well. > > For example what was the maximum number of opened FDs per process again? > Could that become a problem? etc... You can check out the i915 patches I posted if you want to see examples. Max fds may be an issue if userspace doesn't clean up its fences. The implementation is pretty easy with the stuff Maarten has done recently. The changes I still need to make to mine: - sit on top of Chris's request/seqno changes (driver internals really) - switch over to execbuf as the main API on the render side (like you're doing) - add support for display and other timelines As far as compat goes, I don't think it should be too hard. Even with GPU scheduling, a given context's buffers should all be in-order with respect to one another, so we ought to be able to mix & match clients using explicit fencing and implicit fencing. Though in Mesa I still haven't looked at how to handle server vs client side arb_sync with the scheduler and explicit fencing in place; might need some extra work there... -- Jesse Barnes, Intel Open Source Technology Center
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #95 from Maciej --- (In reply to comment #94) > Maybe the crash actually happens because of glamor rendering - setting > R600_DEBUG won't do anything in that case. > > Does this patch to Mesa make any difference? > > https://bugs.freedesktop.org/attachment.cgi?id=105745 I got a feeling that You guys always expect everyone here to have some sort of skills, but a lot of Linux users have no idea how to compile Mesa, just like me. I can test this if it lands in something like Oibaf PPA, but that's it :/ -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/84571884/attachment.html>
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #96 from Aaron B --- Just a while ago, I was the same way, sort. The main thing is to get away from the not-as-good package manager in Ubuntu, and go with Arch like I did. Package support is amazing because of the AUR, which build source for you, and when you have to do it yourself like us, most of the time you can just grab the config from the auto-installer, which should have 99% of the config options in-place. Like I said, it's screwed ATM, but usually it's pretty painless to compile anything. You just have to try it out. If you email me, I can get you set up with a Mesa build configure for RadeonSI. Then all you have to do is learn how to clone, pull, and patch in git. It's all not too bad. I mean, most LInux people are devs of some sort, so if you want to have a good time, you should put a little more time in and learn how the back-end of Linux and how programs are compiled to have a good time. I'll keep testing, as soon as I find a way to compile LLVM 3.6. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/6429a5db/attachment.html>
Question on UAPI for fences
On Fri, Sep 12, 2014 at 05:58:09PM +0200, Christian K?nig wrote: > pass in a list of fences to wait for before beginning a command submission. The Android implementation has a mechanism for combining multiple sync points into a brand new single sync pt. Thus APIs only ever need to take in a single fd not a list of them. If the user wants an operation to wait for multiple events to occur then it is up to them to request the combined version first. They can then happily close the individual fds that have been combined and only keep the one big one around. Indeed, even that fd can be closed once it has been passed on to some other API. Doing such combining and cleaning up fds as soon as they have been passed on should keep each application's fd usage fairly small. On 12/09/2014 17:08, Christian K?nig wrote: >> As Daniel said using fd is most likely the way we want to do it but this >> remains vague. > Separating the discussion if it should be an fd or not. Using an fd > sounds fine to me in general, but I have some concerns as well. > > For example what was the maximum number of opened FDs per process > again? Could that become a problem? etc... > > Please comment, > Christian. > > Am 12.09.2014 um 18:03 schrieb Jerome Glisse: >> On Fri, Sep 12, 2014 at 05:58:09PM +0200, Christian K?nig wrote: >>> Am 12.09.2014 um 17:48 schrieb Jerome Glisse: On Fri, Sep 12, 2014 at 05:42:57PM +0200, Christian K?nig wrote: > Am 12.09.2014 um 17:33 schrieb Jerome Glisse: >> On Fri, Sep 12, 2014 at 11:25:12AM -0400, Alex Deucher wrote: >>> On Fri, Sep 12, 2014 at 10:50 AM, Jerome Glisse >>> wrote: On Fri, Sep 12, 2014 at 04:43:44PM +0200, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 4:09 PM, Daniel Vetter > wrote: >> On Fri, Sep 12, 2014 at 03:23:22PM +0200, Christian K?nig wrote: >>> Hello everyone, >>> >>> to allow concurrent buffer access by different engines >>> beyond the multiple >>> readers/single writer model that we currently use in radeon >>> and other >>> drivers we need some kind of synchonization object exposed >>> to userspace. >>> >>> My initial patch set for this used (or rather abused) zero >>> sized GEM buffers >>> as fence handles. This is obviously isn't the best way of >>> doing this (to >>> much overhead, rather ugly etc...), Jerome commented on this >>> accordingly. >>> >>> So what should a driver expose instead? Android sync points? >>> Something else? >> I think actually exposing the struct fence objects as a fd, >> using android >> syncpts (or at least something compatible to it) is the way >> to go. Problem >> is that it's super-hard to get the android guys out of hiding >> for this :( >> >> Adding a bunch of people in the hopes that something sticks. > More people. Just to re-iterate, exposing such thing while still using command stream ioctl that use implicit synchronization is a waste and you can only get the lowest common denominator which is implicit synchronization. So i do not see the point of such api if you are not also adding a new cs ioctl with explicit contract that it does not do any kind of synchronization (it could be almost the exact same code modulo the do not wait for previous cmd to complete). >>> Our thinking was to allow explicit sync from a single process, but >>> implicitly sync between processes. >> This is a BIG NAK if you are using the same ioctl as it would >> mean you are >> changing userspace API, well at least userspace expectation. >> Adding a new >> cs flag might do the trick but it should not be about >> inter-process, or any >> thing special, it's just implicit sync or no synchronization. >> Converting >> userspace is not that much of a big deal either, it can be broken >> into >> several step. Like mesa use explicit synchronization all time but >> ddx use >> implicit. > The thinking here is that we need to be backward compatible for > DRI2/3 and > support all kind of different use cases like old DDX and new Mesa, > or old > Mesa and new DDX etc... > > So for my prototype if the kernel sees any access of a BO from two > different > clients it falls back to the old behavior of implicit > synchronization of > access to the same buffer object. That might not be the fastest > approach, > but is as far as I can see conservative and so should work under all > conditions. > > Apart from that the planning so far was that we just hide this > feature > behind a couple of command submission flags and new chunks. Just to reprod
[Intel-gfx] [PATCH] drm: Assert correct locking for drm_send_vblank_event
On Fri, Sep 12, 2014 at 01:03:51PM -0400, Peter Hurley wrote: > On 09/12/2014 12:04 PM, Chris Wilson wrote: > > On Fri, Sep 12, 2014 at 05:34:56PM +0200, Daniel Vetter wrote: > >> On Fri, Sep 12, 2014 at 04:23:29PM +0100, Chris Wilson wrote: > >>> On Fri, Sep 12, 2014 at 03:40:56PM +0200, Daniel Vetter wrote: > The comment says that the caller must hold the dev->event_lock > spinlock, so let's enforce this. > > A quick audit over all driver shows that except for the one place in > i915 which motivated this all callers fullfill this requirement > already. > >>> > >>> Replace the rogue WARN_ON_SMP(!spin_is_locked(&dev->event_lock)) in > >>> send_vblank_event() as well then. > >> > >> Meh, I've missed that one, that's actually better I think. I'll drop my > >> patch here. > > > > I thought assert_spin_lock was the preferred form? > > Actually, lockdep_assert_held() is the preferred form. > > See https://lkml.org/lkml/2014/9/3/171 Which unfortunately doesn't warn for all the normal users which are not insane enough to enable lockdep and so is totally useless to validate a driver that runs on metric piles of different chips (with a resulting combinatorial explosion of code-paths because hw designers are creative). And we rely a lot on random drive-by testers to report such issues. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PULL] topic/core-stuff
Hi Dave, Usual pile of random drm patches all over. One i915 one in here for the hdmi doubleclock stuff since I've put both of Clint's patches into this branch. Rebased just now only to check what you've merged already, otherwise this has been hanging out in -nightly for quite a while. Cheers, Daniel The following changes since commit edbaae5a5cab89de0e64b8c03ebd9a8d5d266550: Merge tag 'topic/vblank-rework-2014-09-12' of git://anongit.freedesktop.org/drm-intel into drm-next (2014-09-12 19:04:53 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/core-stuff-2014-09-12 for you to fetch changes up to 3bd33023f122c16ecbf8a2a74903df52dbce5690: drm: Drop modeset locking from crtc init function (2014-09-12 17:09:24 +0200) Chris Wilson (1): drm: Include task->name and master status in debugfs clients info Clint Taylor (2): drm/edid: Reduce horizontal timings for pixel replicated modes drm/i915/hdmi: Enable pipe pixel replication for SD interlaced modes Daniel Vetter (1): drm: Drop modeset locking from crtc init function David Herrmann (1): drm: fix division-by-zero on dumb_create() Julia Lawall (1): drm: use c99 initializers in structures Laurent Pinchart (1): drm/gem: Fix kerneldoc typo Randy Dunlap (1): drm: fix drm_modeset_lock.h kernel-doc notation Rob Clark (1): ww-mutex: clarify help text for DEBUG_WW_MUTEX_SLOWPATH drivers/gpu/drm/drm_crtc.c| 9 +-- drivers/gpu/drm/drm_edid.c| 117 +++--- drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_info.c| 27 +++-- drivers/gpu/drm/i915/intel_hdmi.c | 15 - drivers/gpu/drm/sti/sti_vtac.c| 12 +++- include/drm/drm_modeset_lock.h| 4 +- lib/Kconfig.debug | 4 ++ 8 files changed, 113 insertions(+), 77 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Intel-gfx] [PATCH] drm: Assert correct locking for drm_send_vblank_event
On 09/12/2014 01:25 PM, Daniel Vetter wrote: > On Fri, Sep 12, 2014 at 01:03:51PM -0400, Peter Hurley wrote: >> On 09/12/2014 12:04 PM, Chris Wilson wrote: >>> On Fri, Sep 12, 2014 at 05:34:56PM +0200, Daniel Vetter wrote: On Fri, Sep 12, 2014 at 04:23:29PM +0100, Chris Wilson wrote: > On Fri, Sep 12, 2014 at 03:40:56PM +0200, Daniel Vetter wrote: >> The comment says that the caller must hold the dev->event_lock >> spinlock, so let's enforce this. >> >> A quick audit over all driver shows that except for the one place in >> i915 which motivated this all callers fullfill this requirement >> already. > > Replace the rogue WARN_ON_SMP(!spin_is_locked(&dev->event_lock)) in > send_vblank_event() as well then. Meh, I've missed that one, that's actually better I think. I'll drop my patch here. >>> >>> I thought assert_spin_lock was the preferred form? >> >> Actually, lockdep_assert_held() is the preferred form. >> >> See https://lkml.org/lkml/2014/9/3/171 > > Which unfortunately doesn't warn for all the normal users which are not > insane enough to enable lockdep and so is totally useless to validate a > driver that runs on metric piles of different chips (with a resulting > combinatorial explosion of code-paths because hw designers are creative). > And we rely a lot on random drive-by testers to report such issues. I know. When I wrote [in that thread linked above]: On Wed, Sep 03, 2014 at 10:50:01AM -0400, Peter Hurley wrote: > So a lockdep-only assert is unlikely to draw attention to existing bugs, > especially in established drivers. here's the replies I got: Peter Zijlstra wrote: > By the same logic lockdep will not find locking errors in established > drivers. and On 09/04/2014 01:14 AM, Ingo Molnar wrote: > Indeed, this patch is ill-advised in several ways: > > - it extends an API variant that we want to phase > > - emits a warning even if say lockdep has already emitted a > warning and locking state is not guaranteed to be consistent. > > - makes the kernel more expensive once fully debugged, in that > non-fatal checks are unconditional. :/ Regards, Peter Hurley
[Intel-gfx] [PATCH] drm: Assert correct locking for drm_send_vblank_event
On 09/12/2014 12:04 PM, Chris Wilson wrote: > On Fri, Sep 12, 2014 at 05:34:56PM +0200, Daniel Vetter wrote: >> On Fri, Sep 12, 2014 at 04:23:29PM +0100, Chris Wilson wrote: >>> On Fri, Sep 12, 2014 at 03:40:56PM +0200, Daniel Vetter wrote: The comment says that the caller must hold the dev->event_lock spinlock, so let's enforce this. A quick audit over all driver shows that except for the one place in i915 which motivated this all callers fullfill this requirement already. >>> >>> Replace the rogue WARN_ON_SMP(!spin_is_locked(&dev->event_lock)) in >>> send_vblank_event() as well then. >> >> Meh, I've missed that one, that's actually better I think. I'll drop my >> patch here. > > I thought assert_spin_lock was the preferred form? Actually, lockdep_assert_held() is the preferred form. See https://lkml.org/lkml/2014/9/3/171 Regards, Peter Hurley
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #97 from Maciej --- (In reply to comment #96) > Just a while ago, I was the same way, sort. The main thing is to get away > from the not-as-good package manager in Ubuntu, and go with Arch like I did. > Package support is amazing because of the AUR, which build source for you, > and when you have to do it yourself like us, most of the time you can just > grab the config from the auto-installer, which should have 99% of the config > options in-place. Like I said, it's screwed ATM, but usually it's pretty > painless to compile anything. You just have to try it out. If you email me, > I can get you set up with a Mesa build configure for RadeonSI. Then all you > have to do is learn how to clone, pull, and patch in git. It's all not too > bad. I mean, most LInux people are devs of some sort, so if you want to have > a good time, you should put a little more time in and learn how the back-end > of Linux and how programs are compiled to have a good time. I'll keep > testing, as soon as I find a way to compile LLVM 3.6. I'd rather have my system to just work. Adding PPA takes no effort, setting up something like Arch or compiling Mesa does and I have no time to deal with it. Some of Linux users (I assume mostly those on *buntu distros) are not developers, sysadmins or kernel hackers, we got other jobs and things to do, but we like open source solutions and other benefits of Linux world. I suppose I should just change my GPU to Nvidia and stop giving a fuck, but I prefer open source solutions and AMD is the only choice when it comes to high performance foss drivers. I'm not bitching or whining here, just clarifying few facts. I can reports bugs, provide logs, test some PPAs, but please don't patronize me with your linux hacker attitude, I'm not one of You, I'm just a user with different job description who happens to like open source for various reason (messing with code is not one of them). -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/e2fd4ca7/attachment.html>
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #98 from Aaron B --- I'm giving you details on how to more help us, and if you can run Linux in any way, and understand how the command line works even remotely, and how packages work, going to a better system/learning one little thing like code compiling isn't hard is my point, if you don't want to, that is fine, but at the same time, even I'm a programmer, and the DRM infrastructure and hardware isn't in my league so I can't help that way, but to help, yeah it's good to get confirmation of bugs, but unless we can patch together and such, it's a long shot from finding the actual problem, since, like I said, it's hard to track down, it's so random. But still, whoever said AMD has the best FOSS drivers was dead wrong, as they're last AFAIK. Intel and Nvidia patches to Mesa even are 10x more. Our guys on here do good work, it's not their fault AMD as a company won't cater to us, though. They don't have much help, it's 4-5 guys, most of them watching this. I mean, if they ditched their Proprietary drivers made in Hong Kong, which aren't bad when they compile on 2 year old kernels, and contributed to Mesa/DRM, we'd of had good drivers years ago. But they don't, so we're here now, and we gotta be patient. You should have known and understood this before you went to a newer technology card. I mean, I'm mad too, I also want to go with a GTX 770ti, but I'm sticking with this to help get it stable, as without users it's also harder to get software into a usable state over all. If you want complete usability on your hardware, you should just go windows then, FWIW. As bad as that sounds, if you need it to work, use what does for your hardware, as new hardware in Linux takes time, especially since there's always new hardware. It works for me, just a few bugs, I'm sorry yours is worse, but there's not much any of us can do. Also, I haven't had a crash in 3 or so days now since I went to LLVM 3.6, even though it doesn't build Mesa with it on 32-bit. Maybe it's fixed, just have to wait for LLVM to update. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/72b390e8/attachment.html>
[Bug 84431] New: Kernel crash when unloading radeon module for switcheroo card
https://bugzilla.kernel.org/show_bug.cgi?id=84431 Bug ID: 84431 Summary: Kernel crash when unloading radeon module for switcheroo card Product: Drivers Version: 2.5 Kernel Version: all Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: pali.rohar at gmail.com Regression: No Created attachment 149991 --> https://bugzilla.kernel.org/attachment.cgi?id=149991&action=edit Fix crash after rmmod radeon on PX systems. Calling rmmod radeon on PX system cause kernel crash. Reason is function vga_switcheroo_init_domain_pm_ops() which setting dev->pm_domain function of PCI device. When radeon module is unloaded pointer dev->pm_domain is set to vga_switcheroo function which try to call radeon function (which does not exists in memory after rmmod radeon). I bet that nouveau has same problem. I'm attaching simple patch which set dev->pm_domain of PCI device back to NULL when removing radeon device so vga_switcheroo will not be called. But I think that proper way for fixing this bug - which is in vga_switcheroo - should be to add function like "vga_switcheroo_exit_domain_pm_ops()" which will set pm_domain back to origin value (which is in my case NULL). With my patch on PX system I can call rmmod radeon, modprobe radeon, rmmod radeon, ... many times without no crash. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 84431] Kernel crash when unloading radeon module for switcheroo card
https://bugzilla.kernel.org/show_bug.cgi?id=84431 Pali Roh?r changed: What|Removed |Added Attachment #149991|0 |1 is patch|| Attachment #149991|application/octet-stream|text/plain mime type|| -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #99 from Emil Velikov --- Maciej Just a friendly note from a guy not involved with the radeon drivers or the AMD team. There is software development and distribution. As the latter differs greatly it is somewhat unexpected for developers to know every distro, how they build their software, how its packaged, distributed etc. If you are willing to help, apart from reporting issues here, file them with the distribution and obviously link the two. This way they can prepare packages for you (and other affected people) to test. I fear that explaining over and over again that you don't have experience in building and/or bisecting does not really help. If you have the time to learn - great, otherwise seek assistance from your distribution. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/a3978fba/attachment.html>
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #100 from Aaron B --- I also would like to note that, I just had 2 crashes. Chromium. So it still exists, just seems to happen rarer and rarer for me. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/a12b19b6/attachment-0001.html>
[Bug 75649] Glitchy output using only HDMI on laptop with AMD Mobility Radeon HD 3450/3470
https://bugs.freedesktop.org/show_bug.cgi?id=75649 --- Comment #14 from tderensis at gmail.com --- (In reply to comment #13) > Does booting with radeon.disp_priority=2 on the kernel command line in grub > help? That fixes it. Thanks. Do you have any idea why this happens with a single monitor but not a duel setup? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140912/ba21889f/attachment.html>
[Bug 84431] Kernel crash when unloading radeon module for switcheroo card
https://bugzilla.kernel.org/show_bug.cgi?id=84431 Alex Deucher changed: What|Removed |Added CC||alexdeucher at gmail.com --- Comment #1 from Alex Deucher --- Care to generate a git patch and sign-off on it? -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 84431] Kernel crash when unloading radeon module for switcheroo card
https://bugzilla.kernel.org/show_bug.cgi?id=84431 --- Comment #2 from Pali Roh?r --- I can, but I do not know if this is proper way how to fix it. I still think that root of bug is in function vga_switcheroo_init_domain_pm_ops() which overwrite dev->pm_domain, but does not restore it when driver/device unregister. -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 84431] Kernel crash when unloading radeon module for switcheroo card
https://bugzilla.kernel.org/show_bug.cgi?id=84431 --- Comment #3 from Alex Deucher --- Created attachment 150001 --> https://bugzilla.kernel.org/attachment.cgi?id=150001&action=edit patch 1/3 How about this patch set? -- You are receiving this mail because: You are watching the assignee of the bug.
