Re: [Intel-gfx] [PATCH v3 1/5] drm: Add config for detecting libdrm
On 01 Jul 2015 14:52, Patrik Jakobsson wrote: Use pkg-config to try to find libdrm. If that fails use the standard include directory for kernel drm headers in /usr/include/drm. * configure.ac: Use pkg-config to find libdrm Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com --- configure.ac | 4 1 file changed, 4 insertions(+) diff --git a/configure.ac b/configure.ac index bb8bf46..aa63af7 100644 --- a/configure.ac +++ b/configure.ac @@ -844,6 +844,10 @@ fi AM_CONDITIONAL([USE_LIBUNWIND], [test x$use_libunwind = xyes]) AC_MSG_RESULT([$use_libunwind]) +PKG_CHECK_MODULES([libdrm], [libdrm], + [CPPFLAGS=$CPPFLAGS $libdrm_CFLAGS], + [CPPFLAGS=$CPPFLAGS -I/usr/include/drm]) yikes, no, this is a really really bad idea. it should read: PKG_CHECK_MODULES([LIBDRM], [libdrm], [CPPFLAGS=$CPPFLAGS $LIBDRM_CFLAGS], [:]) -mike signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/5] drm: Add config for detecting libdrm
On 23 Jul 2015 13:44, Dmitry V. Levin wrote: On Thu, Jul 23, 2015 at 05:48:21AM -0400, Mike Frysinger wrote: On 01 Jul 2015 14:52, Patrik Jakobsson wrote: Use pkg-config to try to find libdrm. If that fails use the standard include directory for kernel drm headers in /usr/include/drm. * configure.ac: Use pkg-config to find libdrm Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com --- configure.ac | 4 1 file changed, 4 insertions(+) diff --git a/configure.ac b/configure.ac index bb8bf46..aa63af7 100644 --- a/configure.ac +++ b/configure.ac @@ -844,6 +844,10 @@ fi AM_CONDITIONAL([USE_LIBUNWIND], [test x$use_libunwind = xyes]) AC_MSG_RESULT([$use_libunwind]) +PKG_CHECK_MODULES([libdrm], [libdrm], + [CPPFLAGS=$CPPFLAGS $libdrm_CFLAGS], + [CPPFLAGS=$CPPFLAGS -I/usr/include/drm]) yikes, no, this is a really really bad idea. it should read: PKG_CHECK_MODULES([LIBDRM], [libdrm], [CPPFLAGS=$CPPFLAGS $LIBDRM_CFLAGS], [:]) Why [:] ? Wouldn't [] suffice? probably ... force of habit after being bitten by m4 macros that did not expect to expand empty code and thus lead to shell errors (include macros by autotools projects). i.e. if the m4 looked something like: if ...check if pkg is available...; then $3 else $4 fi then the generated configure script would have syntax errors. -mike signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Parsing LFP brightness control from VBT
Any inputs on this patch ? - Vandana On 7/6/2015 4:35 PM, Vandana Kannan wrote: From: Deepak M m.dee...@intel.com LFP brighness control from the VBT block 43 indicates which controller is used for brightness. LFP1 brightness control method: Bit 7-4 = This field controller number of the brightnes controller. 0 = Controller 0 1 = Controller 1 2 = Controller 2 3 = Controller 3 Others = Reserved Bits 3-0 are for Control pin 0 = PMIC pin is used for brightness control 1 = LPSS PWM is used for brightness control 2 = Display DDI is used for brightness control 3 = CABC method to control brightness Others = Reserved History: This patch was submitted earlier including a check for control pin. http://lists.freedesktop.org/archives/intel-gfx/2014-December/057530.html Since it caused the issue, https://bugs.freedesktop.org/show_bug.cgi?id=87671, it was reverted in http://lists.freedesktop.org/archives/intel-gfx/2015-January/058110.html The current patch reads controller and control pin from VBT (version = 191) From VBT version = 197, default value of control pin is set to DDI, so the corresponding check during backlight setup will be made in a future patch Signed-off-by: Deepak M m.dee...@intel.com Signed-off-by: Vandana Kannan vandana.kan...@intel.com Cc: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_bios.c | 9 + drivers/gpu/drm/i915/intel_bios.h | 11 +++ 3 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 950a981..a89e9a9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1461,6 +1461,8 @@ struct intel_vbt_data { bool present; bool active_low_pwm; u8 min_brightness; /* min_brightness/255 of max */ + u8 controller; /* brightness controller number */ + u8 control_pin; /* brightness control pin */ } backlight; /* MIPI DSI */ diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 2ff9eb0..32c1ef2 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -256,6 +256,7 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, { const struct bdb_lfp_backlight_data *backlight_data; const struct bdb_lfp_backlight_data_entry *entry; + const struct bdb_lfp_backlight_control_data *bl_ctrl_data; backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT); if (!backlight_data) @@ -268,6 +269,7 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, } entry = backlight_data-data[panel_type]; + bl_ctrl_data = backlight_data-blc_ctl[panel_type]; dev_priv-vbt.backlight.present = entry-type == BDB_BACKLIGHT_TYPE_PWM; if (!dev_priv-vbt.backlight.present) { @@ -279,12 +281,19 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv, dev_priv-vbt.backlight.pwm_freq_hz = entry-pwm_freq_hz; dev_priv-vbt.backlight.active_low_pwm = entry-active_low_pwm; dev_priv-vbt.backlight.min_brightness = entry-min_brightness; + dev_priv-vbt.backlight.controller = bl_ctrl_data-controller; + dev_priv-vbt.backlight.control_pin = bl_ctrl_data-pin; + DRM_DEBUG_KMS(VBT backlight PWM modulation frequency %u Hz, active %s, min brightness %u, level %u\n, dev_priv-vbt.backlight.pwm_freq_hz, dev_priv-vbt.backlight.active_low_pwm ? low : high, dev_priv-vbt.backlight.min_brightness, backlight_data-level[panel_type]); + + DRM_DEBUG_KMS(VBT BL controller %u, BL control pin %u\n, + dev_priv-vbt.backlight.controller, + dev_priv-vbt.backlight.control_pin); } /* Try to find sdvo panel data */ diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h index af0b476..e97c1c0 100644 --- a/drivers/gpu/drm/i915/intel_bios.h +++ b/drivers/gpu/drm/i915/intel_bios.h @@ -402,10 +402,21 @@ struct bdb_lfp_backlight_data_entry { u8 obsolete3; } __packed; +#define BLC_CONTROL_PIN_PMIC 0 +#define BLC_CONTROL_PIN_LPSS_PWM 1 +#define BLC_CONTROL_PIN_DDI2 +#define BLC_CONTROL_PIN_CABC 3 + +struct bdb_lfp_backlight_control_data { + u8 controller:4; + u8 pin:4; +} __packed; + struct bdb_lfp_backlight_data { u8 entry_size; struct bdb_lfp_backlight_data_entry data[16]; u8 level[16]; + struct bdb_lfp_backlight_control_data blc_ctl[16]; } __packed; struct aimdb_header { ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/drm_import_export: Add tests for prime/flink sharing races
It is possible to race between unreference of the underlying BO and importing it from prime_fd/name. Verify that the behaviour of libdrm is consistent for prime/flink. Signed-off-by: Michał Winiarski michal.winiar...@intel.com --- tests/drm_import_export.c | 103 ++ 1 file changed, 103 insertions(+) diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c index 57b13dd..db11c18 100644 --- a/tests/drm_import_export.c +++ b/tests/drm_import_export.c @@ -131,6 +131,98 @@ static void * test_thread(void * par) return NULL; } +#define IMPORT_RACE_LOOPS 10 + +struct import_race_thread_data { + int prime_fd; + uint32_t flink_name; + unsigned int stop; + pthread_mutex_t mutex; +}; + +static void *import_close_thread(void *data) +{ + struct import_race_thread_data *t = (struct import_race_thread_data *)data; + drm_intel_bo *bo; + pthread_mutex_lock(t-mutex); + while (!t-stop) { + pthread_mutex_unlock(t-mutex); + bo = NULL; + if (use_flink) + bo = drm_intel_bo_gem_create_from_name(bufmgr, buf-shared, t-flink_name); + else { + pthread_mutex_lock(t-mutex); + if (t-prime_fd != -1) { + bo = drm_intel_bo_gem_create_from_prime(bufmgr, t-prime_fd, 4096); + pthread_mutex_unlock(t-mutex); + } + else + /* We take the lock right after entering the loop */ + continue; + } + if (bo == NULL) { + /* +* If the bo is NULL it means that we've unreferenced in other +* thread - therefore we should expect ENOENT +*/ + igt_assert_eq(errno, ENOENT); + continue; + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t-mutex); + } + pthread_mutex_unlock(t-mutex); + + return NULL; +} + +static void test_import_close_race(void) +{ + pthread_t t; + unsigned int loops = IMPORT_RACE_LOOPS; + drm_intel_bo *bo; + struct import_race_thread_data t_data; + + memset(t_data, 0, sizeof(t_data)); + pthread_mutex_init(t_data.mutex, NULL); + t_data.prime_fd = -1; + + igt_assert_eq(pthread_create(t, NULL, import_close_thread , t_data), 0); + + while (loops--) { + bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096); + igt_assert(bo != NULL); + /* +* We setup the test in such way, that create_from_* can race between +* unreference. If we're using prime, prime_fd is always a valid fd. +*/ + if (use_flink) + igt_assert_eq(drm_intel_bo_flink(bo, (t_data.flink_name)), 0); + else { + pthread_mutex_lock(t_data.mutex); + igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, (t_data.prime_fd)), 0); + igt_assert(t_data.prime_fd != -1); + pthread_mutex_unlock(t_data.mutex); + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t_data.mutex); + close(t_data.prime_fd); + t_data.prime_fd = -1; + pthread_mutex_unlock(t_data.mutex); + } + + pthread_mutex_lock(t_data.mutex); + t_data.stop = 1; + pthread_mutex_unlock(t_data.mutex); + + pthread_join(t, NULL); + pthread_mutex_destroy(t_data.mutex); +} + pthread_t test_thread_id1; pthread_t test_thread_id2; pthread_t test_thread_id3; @@ -153,6 +245,16 @@ igt_main { drm_intel_bufmgr_gem_enable_reuse(bufmgr); } + igt_subtest(import-close-race-flink) { + use_flink = true; + test_import_close_race(); + } + + igt_subtest(import-close-race-prime) { + use_flink = false; + test_import_close_race(); + } + igt_subtest(flink) { use_flink = true; @@ -180,4 +282,5 @@ igt_main { pthread_join(test_thread_id3, NULL); pthread_join(test_thread_id4, NULL); } + } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex
From: Rafał Sapała rafal.a.sap...@intel.com It is possible to hit a race condition in create_from_prime, when trying to import a BO that's currently being freed. In case of prime sharing we'll succesfully get a handle, but fail on get_tiling call, potentially confusing the caller (and requiring different locking scheme than with sharing using flink). Wrap fd_to_handle with struct_mutex to force a more consistent behaviour between prime/flink, convert fprintf to DBG when handling errors. Testcase: igt/drm_import_export/import-close-race-prime Signed-off-by: Rafał Sapała rafal.a.sap...@intel.com Signed-off-by: Michał Winiarski michal.winiar...@intel.com --- intel/intel_bufmgr_gem.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index b1c3b3a..ed4ffd2 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -2728,14 +2728,19 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, int prime_fd, int s struct drm_i915_gem_get_tiling get_tiling; drmMMListHead *list; + pthread_mutex_lock(bufmgr_gem-lock); ret = drmPrimeFDToHandle(bufmgr_gem-fd, prime_fd, handle); + if (ret) { + DBG(create_from_prime: failed to obtain handle from fd: %s\n, strerror(errno)); + pthread_mutex_unlock(bufmgr_gem-lock); + return NULL; + } /* * See if the kernel has already returned this buffer to us. Just as * for named buffers, we must not create two bo's pointing at the same * kernel object */ - pthread_mutex_lock(bufmgr_gem-lock); for (list = bufmgr_gem-named.next; list != bufmgr_gem-named; list = list-next) { @@ -2747,12 +2752,6 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, int prime_fd, int s } } - if (ret) { - fprintf(stderr,ret is %d %d\n, ret, errno); - pthread_mutex_unlock(bufmgr_gem-lock); - return NULL; - } - bo_gem = calloc(1, sizeof(*bo_gem)); if (!bo_gem) { pthread_mutex_unlock(bufmgr_gem-lock); @@ -2793,6 +2792,7 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, int prime_fd, int s DRM_IOCTL_I915_GEM_GET_TILING, get_tiling); if (ret != 0) { + DBG(create_from_prime: failed to get tiling: %s\n, strerror(errno)); drm_intel_gem_bo_unreference(bo_gem-bo); return NULL; } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex
On Fri, Jul 24, 2015 at 11:22:34AM +0200, Michał Winiarski wrote: From: Rafał Sapała rafal.a.sap...@intel.com It is possible to hit a race condition in create_from_prime, when trying to import a BO that's currently being freed. In case of prime sharing we'll succesfully get a handle, but fail on get_tiling call, potentially confusing the caller (and requiring different locking scheme than with sharing using flink). Wrap fd_to_handle with struct_mutex to force a more consistent behaviour between prime/flink, convert fprintf to DBG when handling errors. The race is that the kernel returns us the same file-private handle as the first thread, but that first thread is about to call gem_close (thereby removing the handle from the file completely) and does so between us acquiring the handle and taking the mutex. If we take the mutex, then we acquire the refcnt on the bo prior to the first thread completing its unref (and so preventing the early close). Or we acquire the handle after the earlier close, in which case we are the new owner. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFCv2 00/12] TDR/watchdog timeout support for gen8 (edit: fixed coverletter)
On 21/07/2015 15:51, Tomas Elf wrote: This patch series introduces the following features: * Feature 1: TDR (Timeout Detection and Recovery) for gen8 execlist mode. TDR is an umbrella term for anything that goes into detecting and recovering from GPU hangs and is a term more widely used outside of the upstream driver. This feature introduces an extensible framework that currently supports gen8 but that can be easily extended to support gen7 as well (which is already the case in GMIN but unfortunately in a not quite upstreamable form). The code contained in this submission represents the essentials of what is currently in GMIN merged with what is currently in upstream (as of the time when this work commenced a few months back). This feature adds a new hang recovery path alongside the legacy GPU reset path, which takes care of engine recovery only. Aside from adding support for per-engine recovery this feature also introduces rules for how to promote a potential per-engine reset to a legacy, full GPU reset. The hang checker now integrates with the error handler in a slightly different way in that it allows hang recovery on multiple engines at the same time by passing an engine flag mask to the error handler where flags representing all of the hung engines are set. This allows us to schedule hang recovery once for all currently hung engines instead of one hang recovery per detected engine hang. Previously, when only full GPU reset was supported this was all the same since it wouldn't matter if one or four engines were hung at any given point since it would all amount to the same thing - the GPU getting reset. As it stands now the behaviour is different depending on which engine is hung since each engine is reset separately from all the other engines, therefore we have to think about this in terms of scheduling cost and recovery latency. (see open question below) OPEN QUESTIONS: 1. Do we want to investigate the possibility of per-engine hang detection? In the current upstream driver there is only one work queue that handles the hang checker and everything from initial hang detection to final hang recovery runs in this thread. This makes sense if you're only supporting one form of hang recovery - using full GPU reset and nothing tied to any particular engine. However, as part of this patch series we're changing that by introducing per-engine hang recovery. It could make sense to introduce multiple work queues - one per engine - to run multiple hang checking threads in parallel. This would potentially allow savings in terms of recovery latency since we don't have to scan all engines every time the hang checker is scheduled and the error handler does not have to scan all engines every time it is scheduled. Instead, we could implement one work queue per engine that would invoke the hang checker that only checks _that_ particular engine and then the error handler is invoked for _that_ particular engine. If one engine has hung the latency for getting to the hang recovery path for that particular engine would be (Time For Hang Checking One Engine) + (Time For Error Handling One Engine) rather than the time it takes to do hang checking for all engines + the time it takes to do error handling for all engines that have been detected as hung (which in the worst case would be all engines). There would potentially be as many hang checker and error handling threads going on concurrently as there are engines in the hardware but they would all be running in parallel without any significant locking. The first time where any thread needs exclusive access to the driver is at the point of the actual hang recovery but the time it takes to get there would theoretically be lower and the time it actually takes to do per-engine hang recovery is quite a lot lower than the time it takes to actually detect a hang reliably. How much we would save by such a change still needs to be analysed and compared against the current single-thread model but it makes sense from a theoretical design point of view. * Feature 2: Watchdog Timeout (a.k.a media engine reset) for gen8. This feature allows userland applications to control whether or not individual batch buffers should have a first-level, fine-grained, hardware-based hang detection mechanism on top of the ordinary, software-based periodic hang checker that is already in the driver. The advantage over relying solely on the current software-based hang checker is that the watchdog timeout mechanism is about 1000x quicker and more precise. Since it's not a full driver-level hang detection mechanism but only targetting one individual batch buffer at a time it can afford to be that quick without
[Intel-gfx] [PATCH i-g-t v2] tests/drm_import_export: Add tests for prime/flink sharing races
It is possible to race between unreference of the underlying BO and importing it from prime_fd/name. Verify that the behaviour of libdrm is consistent for prime/flink. v2: more comments in source file, dropped extra whitespace Signed-off-by: Michał Winiarski michal.winiar...@intel.com Cc: Thomas Wood thomas.w...@intel.com --- tests/drm_import_export.c | 112 ++ 1 file changed, 112 insertions(+) diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c index 57b13dd..e24e0df 100644 --- a/tests/drm_import_export.c +++ b/tests/drm_import_export.c @@ -131,6 +131,108 @@ static void * test_thread(void * par) return NULL; } +#define IMPORT_RACE_LOOPS 10 + +struct import_race_thread_data { + int prime_fd; + uint32_t flink_name; + unsigned int stop; + pthread_mutex_t mutex; +}; + +/* + * Attempt to import the bo. It is possible that GEM_CLOSE was already called + * in different thread and from i915 point of view the handle is no longer + * valid (thus create_from_prime/name should fail). + */ +static void *import_close_thread(void *data) +{ + struct import_race_thread_data *t = (struct import_race_thread_data *)data; + drm_intel_bo *bo; + pthread_mutex_lock(t-mutex); + while (!t-stop) { + pthread_mutex_unlock(t-mutex); + bo = NULL; + if (use_flink) + bo = drm_intel_bo_gem_create_from_name(bufmgr, buf-shared, t-flink_name); + else { + pthread_mutex_lock(t-mutex); + if (t-prime_fd != -1) { + bo = drm_intel_bo_gem_create_from_prime(bufmgr, t-prime_fd, 4096); + pthread_mutex_unlock(t-mutex); + } + else + /* We take the lock right after entering the loop */ + continue; + } + if (bo == NULL) { + /* +* If the bo is NULL it means that we've unreferenced in other +* thread - therefore we should expect ENOENT +*/ + igt_assert_eq(errno, ENOENT); + continue; + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t-mutex); + } + pthread_mutex_unlock(t-mutex); + + return NULL; +} + +/* + * It is possible to race between unreference of the underlying BO and importing + * it from prime_fd/name. Verify that the behaviour of libdrm is consistent for + * prime/flink. + */ +static void test_import_close_race(void) +{ + pthread_t t; + unsigned int loops = IMPORT_RACE_LOOPS; + drm_intel_bo *bo; + struct import_race_thread_data t_data; + + memset(t_data, 0, sizeof(t_data)); + pthread_mutex_init(t_data.mutex, NULL); + t_data.prime_fd = -1; + + igt_assert_eq(pthread_create(t, NULL, import_close_thread , t_data), 0); + + while (loops--) { + bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096); + igt_assert(bo != NULL); + /* +* We setup the test in such way, that create_from_* can race between +* unreference. If we're using prime, prime_fd is always a valid fd. +*/ + if (use_flink) + igt_assert_eq(drm_intel_bo_flink(bo, (t_data.flink_name)), 0); + else { + pthread_mutex_lock(t_data.mutex); + igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, (t_data.prime_fd)), 0); + igt_assert(t_data.prime_fd != -1); + pthread_mutex_unlock(t_data.mutex); + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t_data.mutex); + close(t_data.prime_fd); + t_data.prime_fd = -1; + pthread_mutex_unlock(t_data.mutex); + } + + pthread_mutex_lock(t_data.mutex); + t_data.stop = 1; + pthread_mutex_unlock(t_data.mutex); + + pthread_join(t, NULL); + pthread_mutex_destroy(t_data.mutex); +} + pthread_t test_thread_id1; pthread_t test_thread_id2; pthread_t test_thread_id3; @@ -153,6 +255,16 @@ igt_main { drm_intel_bufmgr_gem_enable_reuse(bufmgr); } + igt_subtest(import-close-race-flink) { + use_flink = true; + test_import_close_race(); + } + + igt_subtest(import-close-race-prime) { + use_flink = false; + test_import_close_race(); + } + igt_subtest(flink) { use_flink = true; -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH i-g-t] tools/Makefile.sources: fix igt_stats complie error on android
Found a simpler way of doing this using 'LOCAL_MODULE'. Will prepare a replacement patch. //Derek -Original Message- From: Morton, Derek J Sent: Thursday, July 23, 2015 1:59 PM To: intel-gfx@lists.freedesktop.org Cc: Wood, Thomas; Gore, Tim; Morton, Derek J Subject: [PATCH i-g-t] tools/Makefile.sources: fix igt_stats complie error on android There are two versions of igt_stats.c in tools and lib\tests which causes the build system to have two build modules with the same name. This patch specifies a different target name for the tool so there is no clash. Signed-off-by: Derek Morton derek.j.mor...@intel.com --- tools/Makefile.sources | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/Makefile.sources b/tools/Makefile.sources index 8ca9351..d19ef96 100644 --- a/tools/Makefile.sources +++ b/tools/Makefile.sources @@ -5,7 +5,7 @@ noinst_PROGRAMS = \ $(NULL) bin_PROGRAMS =\ - igt_stats \ + igt_statistics \ intel_audio_dump\ intel_reg \ intel_backlight \ @@ -63,3 +63,9 @@ intel_l3_parity_SOURCES =\ intel_l3_parity.h \ intel_l3_udev_listener.c +# Android does not like building multiple targets with the same name # +tools/igt_stats.c clashes with lib/tests/igt_stats.c. Simplest fix is +to # specify a different target name here. +igt_statistics_SOURCES = \ + igt_stats.c + -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android
-Original Message- From: Thomas Wood [mailto:thomas.w...@intel.com] Sent: Friday, July 24, 2015 4:33 PM To: Morton, Derek J Cc: Intel Graphics Development; Gore, Tim Subject: Re: [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android On 24 July 2015 at 14:35, Derek Morton derek.j.mor...@intel.com wrote: There are two versions of gem_exec_nop.c in benchmarks and tests which causes the build system to have two build modules with the same name. This patch renames benchmarks/gem_exec_nop.c to benchmarks/gem_exec_nop_benchmark.c using the existing gem_userptr_benchmark.c as a naming convention. Would using LOCAL_MODULE_FILENAME help here, allowing an alternative output name for Android? https://developer.android.com/ndk/guides/android_mk.html#mdv Not really, but changing LOCAL_MODULE to be $1_benchmarks instead of $1 to 'namespace' the directory does, and will prevent the same problem reoccuring when more files are added in the future. I will prepare a new patch. v2: Also rename gem_mmap to gem_mmap_benchmark. Another file which breaks android which was added after this patch was 1st submitted. Signed-off-by: Derek Morton derek.j.mor...@intel.com --- benchmarks/Makefile.sources | 9 +- benchmarks/gem_exec_nop.c | 153 - benchmarks/gem_exec_nop_benchmark.c | 153 + benchmarks/gem_mmap.c | 165 benchmarks/gem_mmap_benchmark.c | 165 5 files changed, 325 insertions(+), 320 deletions(-) delete mode 100644 benchmarks/gem_exec_nop.c create mode 100644 benchmarks/gem_exec_nop_benchmark.c delete mode 100644 benchmarks/gem_mmap.c create mode 100644 benchmarks/gem_mmap_benchmark.c diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources index 7ad95a5..35722b5 100644 --- a/benchmarks/Makefile.sources +++ b/benchmarks/Makefile.sources @@ -1,11 +1,16 @@ +# If you copy a test to benckmarks, rename it _benchmark # The +andriod build will fail when trying to build multiple binaries with # +the same name. + bin_PROGRAMS = \ intel_upload_blit_large \ intel_upload_blit_large_gtt \ intel_upload_blit_large_map \ intel_upload_blit_small \ - gem_exec_nop\ - gem_mmap\ + gem_exec_nop_benchmark \ + gem_mmap_benchmark \ gem_prw \ gem_userptr_benchmark \ kms_vblank \ $(NULL) + diff --git a/benchmarks/gem_exec_nop.c b/benchmarks/gem_exec_nop.c deleted file mode 100644 index 2a3abd2..000 --- a/benchmarks/gem_exec_nop.c +++ /dev/null @@ -1,153 +0,0 @@ -/* - * Copyright © 2011 Intel Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the Software), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - * - * Authors: - *Chris Wilson ch...@chris-wilson.co.uk - * - */ - -#include unistd.h -#include stdlib.h -#include stdint.h -#include stdio.h -#include string.h -#include fcntl.h -#include inttypes.h -#include errno.h -#include sys/stat.h -#include sys/ioctl.h -#include sys/time.h -#include time.h - -#include drm.h -#include ioctl_wrappers.h -#include drmtest.h -#include intel_io.h -#include igt_stats.h - -#define LOCAL_I915_EXEC_NO_RELOC (111) -#define LOCAL_I915_EXEC_HANDLE_LUT (112) - -static uint64_t elapsed(const struct timespec *start, - const struct timespec *end, - int loop) -{ - return (10ULL*(end-tv_sec - start-tv_sec) + (end-tv_nsec - start-tv_nsec))/loop; -} - -static int __gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf) -{ - int
Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android
On 24 July 2015 at 14:43, Chris Wilson ch...@chris-wilson.co.uk wrote: On Fri, Jul 24, 2015 at 02:35:29PM +0100, Derek Morton wrote: There are two versions of gem_exec_nop.c in benchmarks and tests which causes the build system to have two build modules with the same name. This patch renames benchmarks/gem_exec_nop.c to benchmarks/gem_exec_nop_benchmark.c using the existing gem_userptr_benchmark.c as a naming convention. Surely a simpler fix than breaking external tools would be to fix the Makefile? If the binaries do have to be renamed, the .gitignore files will also need updating. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Remove bogus kerneldoc include directive
On Fri, Jul 24, 2015 at 05:40:13PM +0200, Daniel Vetter wrote: Afaict intel_irq_fini never existed. No idea how that one came about. It's notable by its absence. We should write it! There are a few steps in intel_irq_init() that would be best undone intel_irq_fini(). The work handlers could be dealt with by a comment that the core does a flush_wq() - that at leasts says we have done due diligence and checked that the work will be complete before module unload. But we should move the pm_qos teardown here for instance. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] tests/drm_import_export: Add tests for prime/flink sharing races
On 24 July 2015 at 15:43, Michał Winiarski michal.winiar...@intel.com wrote: It is possible to race between unreference of the underlying BO and importing it from prime_fd/name. Verify that the behaviour of libdrm is consistent for prime/flink. v2: more comments in source file, dropped extra whitespace Thanks, patch pushed. Signed-off-by: Michał Winiarski michal.winiar...@intel.com Cc: Thomas Wood thomas.w...@intel.com --- tests/drm_import_export.c | 112 ++ 1 file changed, 112 insertions(+) diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c index 57b13dd..e24e0df 100644 --- a/tests/drm_import_export.c +++ b/tests/drm_import_export.c @@ -131,6 +131,108 @@ static void * test_thread(void * par) return NULL; } +#define IMPORT_RACE_LOOPS 10 + +struct import_race_thread_data { + int prime_fd; + uint32_t flink_name; + unsigned int stop; + pthread_mutex_t mutex; +}; + +/* + * Attempt to import the bo. It is possible that GEM_CLOSE was already called + * in different thread and from i915 point of view the handle is no longer + * valid (thus create_from_prime/name should fail). + */ +static void *import_close_thread(void *data) +{ + struct import_race_thread_data *t = (struct import_race_thread_data *)data; + drm_intel_bo *bo; + pthread_mutex_lock(t-mutex); + while (!t-stop) { + pthread_mutex_unlock(t-mutex); + bo = NULL; + if (use_flink) + bo = drm_intel_bo_gem_create_from_name(bufmgr, buf-shared, t-flink_name); + else { + pthread_mutex_lock(t-mutex); + if (t-prime_fd != -1) { + bo = drm_intel_bo_gem_create_from_prime(bufmgr, t-prime_fd, 4096); + pthread_mutex_unlock(t-mutex); + } + else + /* We take the lock right after entering the loop */ + continue; + } + if (bo == NULL) { + /* +* If the bo is NULL it means that we've unreferenced in other +* thread - therefore we should expect ENOENT +*/ + igt_assert_eq(errno, ENOENT); + continue; + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t-mutex); + } + pthread_mutex_unlock(t-mutex); + + return NULL; +} + +/* + * It is possible to race between unreference of the underlying BO and importing + * it from prime_fd/name. Verify that the behaviour of libdrm is consistent for + * prime/flink. + */ +static void test_import_close_race(void) +{ + pthread_t t; + unsigned int loops = IMPORT_RACE_LOOPS; + drm_intel_bo *bo; + struct import_race_thread_data t_data; + + memset(t_data, 0, sizeof(t_data)); + pthread_mutex_init(t_data.mutex, NULL); + t_data.prime_fd = -1; + + igt_assert_eq(pthread_create(t, NULL, import_close_thread , t_data), 0); + + while (loops--) { + bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096); + igt_assert(bo != NULL); + /* +* We setup the test in such way, that create_from_* can race between +* unreference. If we're using prime, prime_fd is always a valid fd. +*/ + if (use_flink) + igt_assert_eq(drm_intel_bo_flink(bo, (t_data.flink_name)), 0); + else { + pthread_mutex_lock(t_data.mutex); + igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, (t_data.prime_fd)), 0); + igt_assert(t_data.prime_fd != -1); + pthread_mutex_unlock(t_data.mutex); + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t_data.mutex); + close(t_data.prime_fd); + t_data.prime_fd = -1; + pthread_mutex_unlock(t_data.mutex); + } + + pthread_mutex_lock(t_data.mutex); + t_data.stop = 1; + pthread_mutex_unlock(t_data.mutex); + + pthread_join(t, NULL); + pthread_mutex_destroy(t_data.mutex); +} + pthread_t test_thread_id1; pthread_t test_thread_id2; pthread_t test_thread_id3; @@ -153,6 +255,16 @@ igt_main { drm_intel_bufmgr_gem_enable_reuse(bufmgr); } + igt_subtest(import-close-race-flink) { + use_flink = true; + test_import_close_race(); + } + + igt_subtest(import-close-race-prime) { + use_flink = false; +
Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android
On 24 July 2015 at 14:35, Derek Morton derek.j.mor...@intel.com wrote: There are two versions of gem_exec_nop.c in benchmarks and tests which causes the build system to have two build modules with the same name. This patch renames benchmarks/gem_exec_nop.c to benchmarks/gem_exec_nop_benchmark.c using the existing gem_userptr_benchmark.c as a naming convention. Would using LOCAL_MODULE_FILENAME help here, allowing an alternative output name for Android? https://developer.android.com/ndk/guides/android_mk.html#mdv v2: Also rename gem_mmap to gem_mmap_benchmark. Another file which breaks android which was added after this patch was 1st submitted. Signed-off-by: Derek Morton derek.j.mor...@intel.com --- benchmarks/Makefile.sources | 9 +- benchmarks/gem_exec_nop.c | 153 - benchmarks/gem_exec_nop_benchmark.c | 153 + benchmarks/gem_mmap.c | 165 benchmarks/gem_mmap_benchmark.c | 165 5 files changed, 325 insertions(+), 320 deletions(-) delete mode 100644 benchmarks/gem_exec_nop.c create mode 100644 benchmarks/gem_exec_nop_benchmark.c delete mode 100644 benchmarks/gem_mmap.c create mode 100644 benchmarks/gem_mmap_benchmark.c diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources index 7ad95a5..35722b5 100644 --- a/benchmarks/Makefile.sources +++ b/benchmarks/Makefile.sources @@ -1,11 +1,16 @@ +# If you copy a test to benckmarks, rename it _benchmark +# The andriod build will fail when trying to build multiple binaries with +# the same name. + bin_PROGRAMS = \ intel_upload_blit_large \ intel_upload_blit_large_gtt \ intel_upload_blit_large_map \ intel_upload_blit_small \ - gem_exec_nop\ - gem_mmap\ + gem_exec_nop_benchmark \ + gem_mmap_benchmark \ gem_prw \ gem_userptr_benchmark \ kms_vblank \ $(NULL) + diff --git a/benchmarks/gem_exec_nop.c b/benchmarks/gem_exec_nop.c deleted file mode 100644 index 2a3abd2..000 --- a/benchmarks/gem_exec_nop.c +++ /dev/null @@ -1,153 +0,0 @@ -/* - * Copyright © 2011 Intel Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the Software), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - * - * Authors: - *Chris Wilson ch...@chris-wilson.co.uk - * - */ - -#include unistd.h -#include stdlib.h -#include stdint.h -#include stdio.h -#include string.h -#include fcntl.h -#include inttypes.h -#include errno.h -#include sys/stat.h -#include sys/ioctl.h -#include sys/time.h -#include time.h - -#include drm.h -#include ioctl_wrappers.h -#include drmtest.h -#include intel_io.h -#include igt_stats.h - -#define LOCAL_I915_EXEC_NO_RELOC (111) -#define LOCAL_I915_EXEC_HANDLE_LUT (112) - -static uint64_t elapsed(const struct timespec *start, - const struct timespec *end, - int loop) -{ - return (10ULL*(end-tv_sec - start-tv_sec) + (end-tv_nsec - start-tv_nsec))/loop; -} - -static int __gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf) -{ - int err = 0; - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, execbuf)) - err = -errno; - return err; -} - -static uint32_t batch(int fd) -{ - const uint32_t buf[] = {MI_BATCH_BUFFER_END}; - uint32_t handle = gem_create(fd, 4096); - gem_write(fd, handle, 0, buf, sizeof(buf)); - return handle; -} - -static int loop(unsigned ring, int reps) -{ - struct drm_i915_gem_execbuffer2 execbuf; - struct drm_i915_gem_exec_object2 gem_exec; - int
Re: [Intel-gfx] [PATCH 1/4] drm/i915: kerneldoc for fences
On Fri, Jul 24, 2015 at 05:40:12PM +0200, Daniel Vetter wrote: v2: Clarify that this is about fence _registers_. Also clarify that the fence code revokes cpu ptes and not gtt ptes. Both suggested by Chris. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@intel.com Quibble over 2, but 1,3,4 are Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Remove bogus kerneldoc include directive
Afaict intel_irq_fini never existed. No idea how that one came about. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- Documentation/DocBook/drm.tmpl | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index d7dc91b7e98c..e56bc0d01fdd 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3920,7 +3920,6 @@ int num_ioctls;/synopsis titleInterrupt Handling/title !Pdrivers/gpu/drm/i915/i915_irq.c interrupt handling !Fdrivers/gpu/drm/i915/i915_irq.c intel_irq_init intel_irq_init_hw intel_hpd_init -!Fdrivers/gpu/drm/i915/i915_irq.c intel_irq_fini !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_disable_interrupts !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_enable_interrupts /sect2 -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: kerneldoc for tiling IOCTL and swizzle functions
Chris rightfully suggested that documenting fences without documenting the BO tiling tracking doesn't make much sense, so fix that. The important bit to stress here (since it lead to some confusion) is the GEM doesn't really care about tiling. Except for a few select cases where the kernel needs to manage something that userspace can't take care of: Namely the limited number of fences and fixing up swizzling, although we still fail at the later. v2: Move the low-level tiling/swizzling functions and kerneldoc to i915_gem_fence.c and leave only the userspace interface here. Suggested by Chris. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- Documentation/DocBook/drm.tmpl | 16 ++- drivers/gpu/drm/i915/i915_gem_fence.c | 28 ++-- drivers/gpu/drm/i915/i915_gem_tiling.c | 84 +- 3 files changed, 80 insertions(+), 48 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index e56bc0d01fdd..f1884038b90f 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4050,9 +4050,21 @@ int num_ioctls;/synopsis !Idrivers/gpu/drm/i915/i915_gem_gtt.c /sect2 sect2 -titleGlobal GTT Fence Handling/title -!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence register handling +titleGTT Fences and Swizzling/title !Idrivers/gpu/drm/i915/i915_gem_fence.c +sect3 + titleGlobal GTT Fence Handling/title +!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence register handling +/sect3 +sect3 + titleHardware Tiling and Swizzling Details/title +!Pdrivers/gpu/drm/i915/i915_gem_fence.c tiling swizzling details +/sect3 + /sect2 + sect2 +titleObject Tiling IOCTLs/title +!Idrivers/gpu/drm/i915/i915_gem_tiling.c +!Pdrivers/gpu/drm/i915/i915_gem_tiling.c buffer object tiling /sect2 sect2 titleBuffer Object Eviction/title diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c index c643260a90c5..af1f8c461060 100644 --- a/drivers/gpu/drm/i915/i915_gem_fence.c +++ b/drivers/gpu/drm/i915/i915_gem_fence.c @@ -497,8 +497,7 @@ void i915_gem_restore_fences(struct drm_device *dev) } /** - * - * Support for managing tiling state of buffer objects. + * DOC: tiling swizzling details * * The idea behind tiling is to increase cache hit rates by rearranging * pixel data so that a group of pixel accesses are in the same cacheline. @@ -546,6 +545,9 @@ void i915_gem_restore_fences(struct drm_device *dev) */ /** + * i915_gem_detect_bit_6_swizzle - detect bit 6 swizzling pattern + * @dev: DRM device + * * Detects bit 6 swizzling of address lookup between IGD access and CPU * access through main memory. */ @@ -692,7 +694,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev) dev_priv-mm.bit_6_swizzle_y = swizzle_y; } -/** +/* * Swap every 64 bytes of this page around, to account for it having a new * bit 17 of its physical address and therefore being interpreted differently * by the GPU. @@ -715,6 +717,18 @@ i915_gem_swizzle_page(struct page *page) kunmap(page); } +/** + * i915_gem_object_do_bit_17_swizzle - fixup bit 17 swizzling + * @obj: i915 GEM buffer object + * + * This function fixes up the swizzling in case any page frame number for this + * object has changed in bit 17 since that state has been saved with + * i915_gem_object_save_bit_17_swizzle(). + * + * This is called when pinning backing storage again, since the kernel is free + * to move unpinned backing storage around (either by directly moving pages or + * by swapping them out and back in again). + */ void i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj) { @@ -737,6 +751,14 @@ i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj) } } +/** + * i915_gem_object_save_bit_17_swizzle - save bit 17 swizzling + * @obj: i915 GEM buffer object + * + * This function saves the bit 17 of each page frame number so that swizzling + * can be fixed up later on with i915_gem_object_do_bit_17_swizzle(). This must + * be called before the backing storage can be unpinned. + */ void i915_gem_object_save_bit_17_swizzle(struct drm_i915_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index fa7a8d7a24e0..ac3eb566c9d2 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -31,53 +31,31 @@ #include drm/i915_drm.h #include i915_drv.h -/** @file i915_gem_tiling.c - * - * Support for managing tiling state of buffer objects. - * - * The idea behind tiling is to increase cache hit rates by rearranging - * pixel data so that a group of pixel accesses are in the same cacheline. - * Performance improvement from doing this on the back/depth buffer are on - * the order of 30%. - * - * Intel architectures make this
[Intel-gfx] [PATCH 3/4] drm/i915: Move low-level swizzling code to i915_gem_fence.c
It fits more with the low-level fence code, and this move leaves only the userspace tiling ioctl handling in i915_gem_tiling.c. Suggested-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- drivers/gpu/drm/i915/i915_drv.h| 8 +- drivers/gpu/drm/i915/i915_gem_fence.c | 268 + drivers/gpu/drm/i915/i915_gem_tiling.c | 219 --- 3 files changed, 272 insertions(+), 223 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4c34fb15d0df..9aebf92132a5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3058,6 +3058,10 @@ void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj); void i915_gem_restore_fences(struct drm_device *dev); +void i915_gem_detect_bit_6_swizzle(struct drm_device *dev); +void i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj); +void i915_gem_object_save_bit_17_swizzle(struct drm_i915_gem_object *obj); + /* i915_gem_context.c */ int __must_check i915_gem_context_init(struct drm_device *dev); void i915_gem_context_fini(struct drm_device *dev); @@ -3150,10 +3154,6 @@ static inline bool i915_gem_object_needs_bit17_swizzle(struct drm_i915_gem_objec obj-tiling_mode != I915_TILING_NONE; } -void i915_gem_detect_bit_6_swizzle(struct drm_device *dev); -void i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj); -void i915_gem_object_save_bit_17_swizzle(struct drm_i915_gem_object *obj); - /* i915_gem_debug.c */ #if WATCH_LISTS int i915_verify_lists(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c index 0434c42d8c11..c643260a90c5 100644 --- a/drivers/gpu/drm/i915/i915_gem_fence.c +++ b/drivers/gpu/drm/i915/i915_gem_fence.c @@ -495,3 +495,271 @@ void i915_gem_restore_fences(struct drm_device *dev) } } } + +/** + * + * Support for managing tiling state of buffer objects. + * + * The idea behind tiling is to increase cache hit rates by rearranging + * pixel data so that a group of pixel accesses are in the same cacheline. + * Performance improvement from doing this on the back/depth buffer are on + * the order of 30%. + * + * Intel architectures make this somewhat more complicated, though, by + * adjustments made to addressing of data when the memory is in interleaved + * mode (matched pairs of DIMMS) to improve memory bandwidth. + * For interleaved memory, the CPU sends every sequential 64 bytes + * to an alternate memory channel so it can get the bandwidth from both. + * + * The GPU also rearranges its accesses for increased bandwidth to interleaved + * memory, and it matches what the CPU does for non-tiled. However, when tiled + * it does it a little differently, since one walks addresses not just in the + * X direction but also Y. So, along with alternating channels when bit + * 6 of the address flips, it also alternates when other bits flip -- Bits 9 + * (every 512 bytes, an X tile scanline) and 10 (every two X tile scanlines) + * are common to both the 915 and 965-class hardware. + * + * The CPU also sometimes XORs in higher bits as well, to improve + * bandwidth doing strided access like we do so frequently in graphics. This + * is called Channel XOR Randomization in the MCH documentation. The result + * is that the CPU is XORing in either bit 11 or bit 17 to bit 6 of its address + * decode. + * + * All of this bit 6 XORing has an effect on our memory management, + * as we need to make sure that the 3d driver can correctly address object + * contents. + * + * If we don't have interleaved memory, all tiling is safe and no swizzling is + * required. + * + * When bit 17 is XORed in, we simply refuse to tile at all. Bit + * 17 is not just a page offset, so as we page an objet out and back in, + * individual pages in it will have different bit 17 addresses, resulting in + * each 64 bytes being swapped with its neighbor! + * + * Otherwise, if interleaved, we have to tell the 3d driver what the address + * swizzling it needs to do is, since it's writing with the CPU to the pages + * (bit 6 and potentially bit 11 XORed in), and the GPU is reading from the + * pages (bit 6, 9, and 10 XORed in), resulting in a cumulative bit swizzling + * required by the CPU of XORing in bit 6, 9, 10, and potentially 11, in order + * to match what the GPU expects. + */ + +/** + * Detects bit 6 swizzling of address lookup between IGD access and CPU + * access through main memory. + */ +void +i915_gem_detect_bit_6_swizzle(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; + uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; + + if (INTEL_INFO(dev)-gen = 8 || IS_VALLEYVIEW(dev)) { + /* +* On BDW+,
[Intel-gfx] [PATCH 1/4] drm/i915: kerneldoc for fences
v2: Clarify that this is about fence _registers_. Also clarify that the fence code revokes cpu ptes and not gtt ptes. Both suggested by Chris. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- Documentation/DocBook/drm.tmpl| 5 +++ drivers/gpu/drm/i915/i915_gem_fence.c | 75 +++ 2 files changed, 80 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 86680f074111..d7dc91b7e98c 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4051,6 +4051,11 @@ int num_ioctls;/synopsis !Idrivers/gpu/drm/i915/i915_gem_gtt.c /sect2 sect2 +titleGlobal GTT Fence Handling/title +!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence register handling +!Idrivers/gpu/drm/i915/i915_gem_fence.c + /sect2 + sect2 titleBuffer Object Eviction/title para This section documents the interface functions for evicting buffer diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c index d3284ee04794..0434c42d8c11 100644 --- a/drivers/gpu/drm/i915/i915_gem_fence.c +++ b/drivers/gpu/drm/i915/i915_gem_fence.c @@ -25,6 +25,36 @@ #include drm/i915_drm.h #include i915_drv.h +/** + * DOC: fence register handling + * + * Important to avoid confusions: fences in the i915 driver are not execution + * fences used to track command completion but hardware detiler objects which + * wrap a given range of the global GTT. Each platform has only a fairly limited + * set of these objects. + * + * Fences are used to detile GTT memory mappings. They're also connected to the + * hardware frontbuffer render tracking and hence interract with frontbuffer + * conmpression. Furthermore on older platforms fences are required for tiled + * objects used by the display engine. They can also be used by the render + * engine - they're required for blitter commands and are optional for render + * commands. But on gen4+ both display (with the exception of fbc) and rendering + * have their own tiling state bits and don't need fences. + * + * Also note that fences only support X and Y tiling and hence can't be used for + * the fancier new tiling formats like W, Ys and Yf. + * + * Finally note that because fences are such a restricted resource they're + * dynamically associated with objects. Furthermore fence state is committed to + * the hardware lazily to avoid unecessary stalls on gen2/3. Therefore code must + * explictly call i915_gem_object_get_fence() to synchronize fencing status + * for cpu access. Also note that some code wants an unfenced view, for those + * cases the fence can be removed forcefully with i915_gem_object_put_fence(). + * + * Internally these functions will synchronize with userspace access by removing + * CPU ptes into GTT mmaps (not the GTT ptes themselves) as needed. + */ + static void i965_write_fence_reg(struct drm_device *dev, int reg, struct drm_i915_gem_object *obj) { @@ -247,6 +277,17 @@ i915_gem_object_wait_fence(struct drm_i915_gem_object *obj) return 0; } +/** + * i915_gem_object_put_fence - force-remove fence for an object + * @obj: object to map through a fence reg + * + * This function force-removes any fence from the given object, which is useful + * if the kernel wants to do untiled GTT access. + * + * Returns: + * + * 0 on success, negative error code on failure. + */ int i915_gem_object_put_fence(struct drm_i915_gem_object *obj) { @@ -322,6 +363,10 @@ deadlock: * and tiling format. * * For an untiled surface, this removes any existing fence. + * + * Returns: + * + * 0 on success, negative error code on failure. */ int i915_gem_object_get_fence(struct drm_i915_gem_object *obj) @@ -374,6 +419,21 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj) return 0; } +/** + * i915_gem_object_pin_fence - pin fencing state + * @obj: object to pin fencing for + * + * This pins the fencing state (whether tiled or untiled) to make sure the + * object is ready to be used as a scanout target. Fencing status must be + * synchronize first by calling i915_gem_object_get_fence(): + * + * The resulting fence pin reference must be released again with + * i915_gem_object_unpin_fence(). + * + * Returns: + * + * True if the object has a fence, false otherwise. + */ bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj) { @@ -390,6 +450,14 @@ i915_gem_object_pin_fence(struct drm_i915_gem_object *obj) return false; } +/** + * i915_gem_object_unpin_fence - unpin fencing state + * @obj: object to unpin fencing for + * + * This releases the fence pin reference acquired through + * i915_gem_object_pin_fence. It will handle both objects with and without an + * attached fence correctly, callers do not need to distinguish this. + */ void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj)
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Try to stop sink crc calculation on error.
On Thu, Jul 23, 2015 at 04:35:45PM -0700, Rodrigo Vivi wrote: Right now if we face any kind of error sink crc calculation stays enabled. So, let's give a shot and try to stop it anyway if it got enabled. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f1b9f93..70a4a37 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3994,7 +3994,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK_MISC, buf) 0) { ret = -EIO; - goto out; + goto stop; } test_crc_count = buf DP_TEST_COUNT_MASK; @@ -4003,7 +4003,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK_MISC, buf) 0) { ret = -EIO; - goto out; + goto stop; } intel_wait_for_vblank(dev, intel_crtc-pipe); } while (--attempts (buf DP_TEST_COUNT_MASK) == test_crc_count); @@ -4011,14 +4011,15 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) if (attempts == 0) { DRM_DEBUG_KMS(Panel is unable to calculate CRC after 6 vblanks\n); ret = -ETIMEDOUT; - goto out; + goto stop; } if (drm_dp_dpcd_read(intel_dp-aux, DP_TEST_CRC_R_CR, crc, 6) 0) { ret = -EIO; - goto out; + goto stop; } +stop: if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf) 0) { ret = -EIO; goto out; Nice cleanup on exit. Reviewed-by: Rafael Antognolli rafael.antogno...@intel.com -- Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Don't return error on sink crc stop.
On Thu, Jul 23, 2015 at 04:35:46PM -0700, Rodrigo Vivi wrote: If we got to the point where we are trying to stop sink CRC the main output of this function was already gotten properly, so don't return the error and let userspace use the crc data. Let's replace the errnos returns with some log messages. So, up to this commit, there's no way to know that an error has ocurred and the next CRC calculation can go wrong. I know that in a follow up patch this is fixed by trying to stop the calculation at the beginning too, but just pointing out that this one specifically has this problem. Not sure if this is a problem though, since the patches are submitted together. If not, then Reviewed-by: Rafael Antognolli rafael.antogno...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 70a4a37..44f8a32 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4021,12 +4021,12 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) stop: if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf) 0) { - ret = -EIO; + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n); goto out; } if (drm_dp_dpcd_writeb(intel_dp-aux, DP_TEST_SINK, buf ~DP_TEST_SINK_START) 0) { - ret = -EIO; + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n); goto out; } out: -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Don't return error on sink crc stop.
On Fri, Jul 24, 2015 at 11:25 AM Rafael Antognolli rafael.antogno...@intel.com wrote: On Thu, Jul 23, 2015 at 04:35:46PM -0700, Rodrigo Vivi wrote: If we got to the point where we are trying to stop sink CRC the main output of this function was already gotten properly, so don't return the error and let userspace use the crc data. Let's replace the errnos returns with some log messages. So, up to this commit, there's no way to know that an error has ocurred and the next CRC calculation can go wrong. I know that in a follow up patch this is fixed by trying to stop the calculation at the beginning too, but just pointing out that this one specifically has this problem. Actually it isn't a problem if crc calculation continue enabled. Mainly with the mask s/0x7/0xf fix. But it is good to disable and force counter reset anyway. (the following patch you mentioned) This patch here is just to highlight that errors during read has more priority than errors on stopping crc. Another way would be to create a stop_ret and if (!ret and stop_ret) ret = stop_ret; But I decided this cleaned version would be better. Not sure if this is a problem though, since the patches are submitted together. If not, then Reviewed-by: Rafael Antognolli rafael.antogno...@intel.com Thanks! Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 70a4a37..44f8a32 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4021,12 +4021,12 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) stop: if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf) 0) { - ret = -EIO; + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n); goto out; } if (drm_dp_dpcd_writeb(intel_dp-aux, DP_TEST_SINK, buf ~DP_TEST_SINK_START) 0) { - ret = -EIO; + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n); goto out; } out: -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 0/4] i915 to call hda driver on HDMI plug/unplug
On Thu, 23 Jul 2015 17:26:15 +0200, David Henningsson wrote: Changes since v2 is that the patch set has diminished to not transfer connector/eld information, since it might be better that such information to be transferred by a separate call in the ordinary direction. That could be added in a later patch set. The audio_ptr is now a void* (instead of a hdac_bus*) so that the audio side can easily refactor the ptr to point to the codec instead of the bus if we would find that beneficial. OK, that leaves some room if we want to change in future. I still consider whether it's better to register from the codec driver or from the bus driver. The codec driver might be a bit cleaner, but then it'd be a call like snd_hda_i915_register_notifier() instead of adding a callback in the codec ops. What I see a bit ugly in the current patchset is the addition of i915 specific callback to a generic hda codec ops. If the codec registers the callback dynamically like above, the pointer will be stored in component directly, so we don't need it in codec ops. (But we'd need to unregister at codec removal, too.) Or, if we keep it as a codec ops callback, as already mentioned, it might make more sense to define it as a generic notifier, i.e. drop i915 prefix and pass some event type to identify the event. thanks, Takashi David Henningsson (4): drm/i915: Add audio hotplug info callback drm/i915: Call audio hotplug notify function ALSA: hda - Dispatch incoming HDMI hotplug i915 callback ALSA: hda - Wake the codec up on hotplug notify events drivers/gpu/drm/i915/i915_drv.h|1 + drivers/gpu/drm/i915/intel_audio.c | 23 --- include/drm/i915_component.h |5 + include/sound/hdaudio.h|4 sound/hda/hdac_i915.c | 23 +++ sound/pci/hda/patch_hdmi.c | 13 - 6 files changed, 65 insertions(+), 4 deletions(-) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: kerneldoc for fences
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6855 -Summary- Platform Delta drm-intel-nightly Series Applied ILK 295/295 295/295 SNB 315/315 315/315 IVB 342/342 342/342 BYT -2 285/285 283/285 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *BYT igt@gem_partial_pwrite_pread@reads PASS(1) FAIL(1) *BYT igt@gem_partial_pwrite_pread@reads-display PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/13 v4] drm/i915: Expose two LRC functions for GuC submission mode
On Thu, Jul 09, 2015 at 07:29:07PM +0100, Dave Gordon wrote: GuC submission is basically execlist submission, but with the GuC handling the actual writes to the ELSP and the resulting context switch interrupts. So to prepare a context for submission via the GuC, we need some of the same functions used in execlist mode. This commit exposes two such functions, changing their names to better describe what they do (they're related to logical ring contexts rather than to execlists per se). v2: Replaces previous drm/i915: Move execlists defines from .c to .h v3: Incorporates a change to one of the functions exposed here that was previously part of an internal patch, but which was omitted from the version recently committed to drm-intel-nightly: 7a01a0a drm/i915/lrc: Update PDPx registers with lri commands So we reinstate this change here. v4: Drop v3 change, update function parameters due to collision with 8ee3615 drm/i915: Convert execlists_ctx_descriptor() for requests Issue: VIZ-4884 Signed-off-by: Dave Gordon david.s.gor...@intel.com Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com --- drivers/gpu/drm/i915/intel_lrc.c | 21 ++--- drivers/gpu/drm/i915/intel_lrc.h | 3 +++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d4f8b43..9e121d3 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -261,11 +261,11 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj) return lrca 12; } -static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request *rq) +uint64_t intel_lr_context_descriptor(struct intel_context *ctx, + struct intel_engine_cs *ring) { - struct intel_engine_cs *ring = rq-ring; struct drm_device *dev = ring-dev; - struct drm_i915_gem_object *ctx_obj = rq-ctx-engine[ring-id].state; + struct drm_i915_gem_object *ctx_obj = ctx-engine[ring-id].state; uint64_t desc; uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj); @@ -303,13 +303,13 @@ static void execlists_elsp_write(struct drm_i915_gem_request *rq0, uint64_t desc[2]; if (rq1) { - desc[1] = execlists_ctx_descriptor(rq1); + desc[1] = intel_lr_context_descriptor(rq1-ctx, rq1-ring); rq1-elsp_submitted++; } else { desc[1] = 0; } - desc[0] = execlists_ctx_descriptor(rq0); + desc[0] = intel_lr_context_descriptor(rq0-ctx, rq0-ring); rq0-elsp_submitted++; /* You must always write both descriptors in the order below. */ @@ -328,7 +328,8 @@ static void execlists_elsp_write(struct drm_i915_gem_request *rq0, spin_unlock(dev_priv-uncore.lock); } -static int execlists_update_context(struct drm_i915_gem_request *rq) +/* Update the ringbuffer pointer and tail offset in a saved context image */ +void intel_lr_context_update(struct drm_i915_gem_request *rq) { struct intel_engine_cs *ring = rq-ring; struct i915_hw_ppgtt *ppgtt = rq-ctx-ppgtt; @@ -358,17 +359,15 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) } kunmap_atomic(reg_state); - - return 0; } static void execlists_submit_requests(struct drm_i915_gem_request *rq0, struct drm_i915_gem_request *rq1) { - execlists_update_context(rq0); + intel_lr_context_update(rq0); if (rq1) - execlists_update_context(rq1); + intel_lr_context_update(rq1); execlists_elsp_write(rq0, rq1); } @@ -2051,7 +2050,7 @@ populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object *ctx_o reg_state[CTX_RING_TAIL+1] = 0; reg_state[CTX_RING_BUFFER_START] = RING_START(ring-mmio_base); /* Ring buffer start address is not known until the buffer is pinned. - * It is written to the context image in execlists_update_context() + * It is written to the context image in intel_lr_context_update() */ reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring-mmio_base); reg_state[CTX_RING_BUFFER_CONTROL+1] = diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index e0299fb..6ecc0b3 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -73,6 +73,9 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, void intel_lr_context_unpin(struct drm_i915_gem_request *req); void intel_lr_context_reset(struct drm_device *dev, struct intel_context *ctx); +void intel_lr_context_update(struct drm_i915_gem_request *rq); +uint64_t intel_lr_context_descriptor(struct intel_context *ctx, + struct intel_engine_cs *ring); /* Execlists */ int
[Intel-gfx] [PATCH 1/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.
Since active function on VLV immediately activate PSR let's give more time for idleness. Different from core platforms where we have idle_frames count. Also kms_psr_sink_crc now is automated and always get this: [drm:intel_enable_pipe] enabling pipe A [drm:intel_edp_backlight_on] [drm:intel_panel_enable_backlight] pipe [drm:intel_panel_enable_backlight] pipe A [drm:intel_panel_actually_set_backlight] set backlight PWM = 7812 PSR gets enabled somewhere here after backlight. [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x0 [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511 [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp PSR gets flushed around here by intel_atomic_commit [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511 [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp [drm:intel_set_memory_cxsr] memory self-refresh is enabled [drm:intel_connector_check_state] [CONNECTOR:39:eDP-1] [drm:check_encoder_state] [ENCODER:30:DAC-30] [drm:check_encoder_state] [ENCODER:31:TMDS-31] [drm:check_encoder_state] [ENCODER:36:TMDS-36] [drm:check_encoder_state] [ENCODER:38:TMDS-38] [drm:check_crtc_state] [CRTC:21] [drm:check_crtc_state] [CRTC:26] [drm:intel_psr_activate [i915]] *ERROR* PSR Active [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x [drm:intel_set_cpu_fifo_underrun_reporting [i915]] *ERROR* pipe A underrun [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO Underrun. It is true that in a product we won't keep disabling and enabling planes so frequently, but for safeness let's stay conservative. It is also true that 500ms is an etternity. But PSR is anyway a power saving feature for idle scenario. So if it is idle feature stays on and 500ms to get it reanabled is not that insane. v2: Rebase over intel_psr.c and fix typo. v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge risk of getting blank screens when planes are being enabled. v4: Revival 2 with reasonable delay. 1/2 sec instead of 5. VBT is 10 sec but actually time for link training what we aren't doing, but with only 100 sec in some cases kms_psr_sink_crc manual was showing blank screen, so let's use this for now. Also changed comment by a FIXME. v5: Rebase after a long time, remove FIXME and update comment above. Reviewed-by: Durgadoss R durgados...@intel.com (v4) Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index acd8ec8..0f446b7 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -698,6 +698,7 @@ void intel_psr_flush(struct drm_device *dev, struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc; enum pipe pipe; + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 500); mutex_lock(dev_priv-psr.lock); if (!dev_priv-psr.enabled) { @@ -733,7 +734,7 @@ void intel_psr_flush(struct drm_device *dev, if (!dev_priv-psr.active !dev_priv-psr.busy_frontbuffer_bits) schedule_delayed_work(dev_priv-psr.work, - msecs_to_jiffies(100)); + msecs_to_jiffies(delay)); mutex_unlock(dev_priv-psr.lock); } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Also call frontbuffer flip when disabling planes.
We also need to call the frontbuffer flip to trigger proper invalidations when disabling planes. Otherwise we will miss screen updates when disabling sprites or cursor. It was caught with kms_psr_sink_crc sprite_plane_onoff and cursor_plane_onoff subtests. This is probably a regression since I can also get this with the manual test case, but with so many changes on atomic modeset I couldn't track exactly when this was introduced. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfe..bb124cc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11716,7 +11716,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, intel_crtc-atomic.update_wm_pre = true; } - if (visible) + if (visible || was_visible) intel_crtc-atomic.fb_bits |= to_intel_plane(plane)-frontbuffer_bit; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.
Since active function on VLV immediately activate PSR let's give more time for idleness. Different from core platforms where we have idle_frames count. Also kms_psr_sink_crc now is automated and always get this: [drm:intel_enable_pipe] enabling pipe A [drm:intel_edp_backlight_on] [drm:intel_panel_enable_backlight] pipe [drm:intel_panel_enable_backlight] pipe A [drm:intel_panel_actually_set_backlight] set backlight PWM = 7812 PSR gets enabled somewhere here after backlight. [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x0 [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511 [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp PSR gets flushed around here by intel_atomic_commit [drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511 [drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp [drm:intel_set_memory_cxsr] memory self-refresh is enabled [drm:intel_connector_check_state] [CONNECTOR:39:eDP-1] [drm:check_encoder_state] [ENCODER:30:DAC-30] [drm:check_encoder_state] [ENCODER:31:TMDS-31] [drm:check_encoder_state] [ENCODER:36:TMDS-36] [drm:check_encoder_state] [ENCODER:38:TMDS-38] [drm:check_crtc_state] [CRTC:21] [drm:check_crtc_state] [CRTC:26] [drm:intel_psr_activate [i915]] *ERROR* PSR Active [drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x [drm:intel_set_cpu_fifo_underrun_reporting [i915]] *ERROR* pipe A underrun [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO Underrun. It is true that in a product we won't keep disabling and enabling planes so frequently, but for safeness let's stay conservative. It is also true that 500ms is an etternity. But PSR is anyway a power saving feature for idle scenario. So if it is idle feature stays on and 500ms to get it reanabled is not that insane. v2: Rebase over intel_psr.c and fix typo. v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge risk of getting blank screens when planes are being enabled. v4: Revival 2 with reasonable delay. 1/2 sec instead of 5. VBT is 10 sec but actually time for link training what we aren't doing, but with only 100 sec in some cases kms_psr_sink_crc manual was showing blank screen, so let's use this for now. Also changed comment by a FIXME. v5: Rebase after a long time, remove FIXME and update comment above. v6: msecs_to_jiffies is already on delay. remove duplication. Reviewed-by: Durgadoss R durgados...@intel.com (v4) Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index acd8ec8..bec13b8 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -698,6 +698,7 @@ void intel_psr_flush(struct drm_device *dev, struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc; enum pipe pipe; + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 500); mutex_lock(dev_priv-psr.lock); if (!dev_priv-psr.enabled) { @@ -732,8 +733,7 @@ void intel_psr_flush(struct drm_device *dev, } if (!dev_priv-psr.active !dev_priv-psr.busy_frontbuffer_bits) - schedule_delayed_work(dev_priv-psr.work, - msecs_to_jiffies(100)); + schedule_delayed_work(dev_priv-psr.work, delay); mutex_unlock(dev_priv-psr.lock); } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/13 v4] drm/i915: Enable GuC firmware log
On Thu, Jul 09, 2015 at 07:29:09PM +0100, Dave Gordon wrote: From: Alex Dai yu@intel.com Allocate a GEM object to hold GuC log data. A debugfs interface (i915_guc_log_dump) is provided to print out the log content. v2: Add struct members at point of use [Chris Wilson] v4: Rebased Issue: VIZ-4884 Signed-off-by: Alex Dai yu@intel.com Signed-off-by: Dave Gordon david.s.gor...@intel.com Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c| 29 +++ drivers/gpu/drm/i915/i915_guc_submission.c | 46 ++ drivers/gpu/drm/i915/intel_guc.h | 1 + 3 files changed, 76 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9ff5f17..13e37d1 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2397,6 +2397,34 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data) return 0; } +static int i915_guc_log_dump(struct seq_file *m, void *data) +{ + struct drm_info_node *node = m-private; + struct drm_device *dev = node-minor-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_i915_gem_object *log_obj = dev_priv-guc.log_obj; + u32 *log; + int i = 0, pg; + + if (!log_obj) + return 0; + + for (pg = 0; pg log_obj-base.size / PAGE_SIZE; pg++) { + log = kmap_atomic(i915_gem_object_get_page(log_obj, pg)); + + for (i = 0; i PAGE_SIZE / sizeof(u32); i += 4) + seq_printf(m, 0x%08x 0x%08x 0x%08x 0x%08x\n, +*(log + i), *(log + i + 1), +*(log + i + 2), *(log + i + 3)); + + kunmap_atomic(log); + } + + seq_putc(m, '\n'); + + return 0; +} + static int i915_edp_psr_status(struct seq_file *m, void *data) { struct drm_info_node *node = m-private; @@ -5112,6 +5140,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {i915_gem_hws_vebox, i915_hws_info, 0, (void *)VECS}, {i915_gem_batch_pool, i915_gem_batch_pool_info, 0}, {i915_guc_load_status, i915_guc_load_status_info, 0}, + {i915_guc_log_dump, i915_guc_log_dump, 0}, {i915_frequency_info, i915_frequency_info, 0}, {i915_hangcheck_info, i915_hangcheck_info, 0}, {i915_drpc_info, i915_drpc_info, 0}, diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 70a0405..e9d46d6 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -75,6 +75,47 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj) drm_gem_object_unreference(obj-base); } +static void guc_create_log(struct intel_guc *guc) +{ + struct drm_i915_private *dev_priv = guc_to_i915(guc); + struct drm_i915_gem_object *obj; + unsigned long offset; + uint32_t size, flags; + + if (i915.guc_log_level GUC_LOG_VERBOSITY_MIN) + return; + + if (i915.guc_log_level GUC_LOG_VERBOSITY_MAX) + i915.guc_log_level = GUC_LOG_VERBOSITY_MAX; + + /* The first page is to save log buffer state. Allocate one + * extra page for others in case for overlap */ + size = (1 + GUC_LOG_DPC_PAGES + 1 + + GUC_LOG_ISR_PAGES + 1 + + GUC_LOG_CRASH_PAGES + 1) PAGE_SHIFT; + + obj = guc-log_obj; + if (!obj) { + obj = gem_allocate_guc_obj(dev_priv-dev, size); + if (!obj) { + /* logging will be off */ + i915.guc_log_level = -1; + return; + } + + guc-log_obj = obj; + } + + /* each allocated unit is a page */ + flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL | + (GUC_LOG_DPC_PAGES GUC_LOG_DPC_SHIFT) | + (GUC_LOG_ISR_PAGES GUC_LOG_ISR_SHIFT) | + (GUC_LOG_CRASH_PAGES GUC_LOG_CRASH_SHIFT); [TOR:] How does the log half full interrupt get handled? + + offset = i915_gem_obj_ggtt_offset(obj) PAGE_SHIFT; /* in pages */ + guc-log_flags = (offset GUC_LOG_BUF_ADDR_SHIFT) | flags; +} + /* * Set up the memory resources to be shared with the GuC. At this point, * we require just one object that can be mapped through the GGTT. @@ -99,6 +140,8 @@ int i915_guc_submission_init(struct drm_device *dev) ida_init(guc-ctx_ids); + guc_create_log(guc); + return 0; } @@ -107,6 +150,9 @@ void i915_guc_submission_fini(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; struct intel_guc *guc = dev_priv-guc; + gem_release_guc_obj(dev_priv-guc.log_obj); + guc-log_obj = NULL; + if (guc-ctx_pool_obj)
Re: [Intel-gfx] [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1
[TOR:] When I see phase 1 I also look for phase 2. A subject that better describes the change in this patch would help. On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote: From: Alex Dai yu@intel.com This adds the first of the data structures used to communicate with the GuC (the pool of guc_context structures). We create a GuC-specific wrapper round the GEM object allocator as all GEM objects shared with the GuC must be pinned into GGTT space at an address that is NOT in the range [0..WOPCM_SIZE), as that range of GGTT addresses is not accessible to the GuC (from the GuC's point of view, it's permanently reserved for other objects such as the BootROM SRAM). [TOR:] I would like a clarfication on the excluded range. The excluded range should be 0 to size for guc within WOPCM area and not 0 to size of WOPCM area. Later, we will need to allocate additional GuC-sharable objects for the submission client(s) and the GuC's debug log. v2: Remove redundant initialisation [Chris Wilson] Defer adding struct members until needed [Chris Wilson] Local functions should pass dev_priv rather than dev [Chris Wilson] v4: Rebased Issue: VIZ-4884 Signed-off-by: Alex Dai yu@intel.com Signed-off-by: Dave Gordon david.s.gor...@intel.com --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/i915_guc_submission.c | 114 + drivers/gpu/drm/i915/intel_guc.h | 7 ++ drivers/gpu/drm/i915/intel_guc_loader.c| 19 + 4 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index e604cfe..23f5612 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -40,7 +40,8 @@ i915-y += i915_cmd_parser.o \ intel_uncore.o # general-purpose microcontroller (GuC) support -i915-y += intel_guc_loader.o +i915-y += intel_guc_loader.o \ + i915_guc_submission.o # autogenerated null render state i915-y += intel_renderstate_gen6.o \ diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c new file mode 100644 index 000..70a0405 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -0,0 +1,114 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ +#include linux/firmware.h +#include linux/circ_buf.h +#include i915_drv.h +#include intel_guc.h + +/** + * gem_allocate_guc_obj() - Allocate gem object for GuC usage + * @dev: drm device + * @size:size of object + * + * This is a wrapper to create a gem obj. In order to use it inside GuC, the + * object needs to be pinned lifetime. Also we must pin it to gtt space other + * than [0, GUC_WOPCM_SIZE] because this range is reserved inside GuC. + * + * Return: A drm_i915_gem_object if successful, otherwise NULL. + */ +static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev, + u32 size) +{ + struct drm_i915_gem_object *obj; + + obj = i915_gem_alloc_object(dev, size); + if (!obj) + return NULL; + + if (i915_gem_object_get_pages(obj)) { + drm_gem_object_unreference(obj-base); + return NULL; + } + + if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, + PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE)) { + drm_gem_object_unreference(obj-base); + return NULL; + } + + return obj; +} + +/** + * gem_release_guc_obj() - Release gem object allocated for GuC usage + * @obj: gem obj to be released + */ +static void gem_release_guc_obj(struct drm_i915_gem_object *obj) +{ +
Re: [Intel-gfx] [PATCH v1.1 1/5] drm/i915: Handle return value in intel_pin_and_fence_fb_obj, v2.
On Fri, Jul 24, 2015 at 04:26:27PM +0300, Ander Conselvan De Oliveira wrote: On Tue, 2015-07-21 at 16:09 +0200, Maarten Lankhorst wrote: -EDEADLK has special meaning in atomic, but get_fence may call i915_find_fence_reg which can return -EDEADLK. This has special meaning in the atomic world, so convert the error to -EBUSY for this case. Doesn't this change the behavior of intel_crtc_page_flip() slightly? It now would return -EBUSY to user space if it can't find a fence instead of -EDEADLK. Not sure if that is a problem though. I don't expect user space would actually check for -EDEADLK. It's extreme, but we could fix the -EDEADLK by dropping the struct_mutex and flushing the unpin workqueue (there is no other conceivable way of triggering EDEADLK here other than uncompleted flips except for a kernel bug ofc). I can confidently state that no one has or ever will see an EDEADLK here - and even if it did userspace is best to treat it exactly the same as any other error path, cancel the flip and try again later. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/13 v4] drm/i915: Add GuC-related header files
On Tue, Jul 21, 2015 at 08:38:35AM +0200, Daniel Vetter wrote: On Fri, Jul 17, 2015 at 05:38:05PM -0700, O'Rourke, Tom wrote: On Thu, Jul 09, 2015 at 07:29:04PM +0100, Dave Gordon wrote: intel_guc_fwif.h contains the subset of the GuC interface that we will need for submission of commands through the GuC. These MUST be kept in sync with the definitions used by the GuC firmware, and updates to this file will (or should) be autogenerated from the source files used to build the firmware. Editing this file is therefore not recommended. i915_guc_reg.h contains definitions of GuC-related hardware: registers, bitmasks, etc. These should match the BSpec. v2: Files renamed resliced per review comments by Chris Wilson v4: Added DON'T-EDIT-ME warning [Tom O'Rourke] Issue: VIZ-4884 Signed-off-by: Alex Dai yu@intel.com Signed-off-by: Dave Gordon david.s.gor...@intel.com --- Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com Merged up to this patch, thanks. -Daniel [TOR:] Thank you. Please hold merging remaining patches in this series until guc firmware v3 is available on 01.org. Tom ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 00/25] replace ioremap_{cache|wt} with memremap
Changes since v1 [1]: 1/ Drop the attempt at unifying ioremap() prototypes, just focus on converting ioremap_cache and ioremap_wt over to memremap (Christoph) 2/ Drop the unrelated cleanups to use %pa in __ioremap_caller (Thomas) 3/ Add support for memremap() attempts on System RAM to simply return the kernel virtual address for that range. ARM depends on this functionality in ioremap_cache() and ACPI was open coding a similar solution. (Mark) 4/ Split the conversions of ioremap_{cache|wt} into separate patches per driver / arch. 5/ Fix bisection breakage and other reports from 0day-kbuild --- While developing the pmem driver we noticed that the __iomem annotation on the return value from ioremap_cache() was being mishandled by several callers. We also observed that all of the call sites expected to be able to treat the return value from ioremap_cache() as normal (non-__iomem) pointer to memory. This patchset takes the opportunity to clean up the above confusion as well as a few issues with the ioremap_{cache|wt} interface, including: 1/ Eliminating the possibility of function prototypes differing between architectures by defining a central memremap() prototype that takes flags to determine the mapping type. 2/ Returning NULL rather than falling back silently to a different mapping-type. This allows drivers to be stricter about the mapping-type fallbacks that are permissible. [1]: http://marc.info/?l=linux-arm-kernelm=143735199029255w=2 --- Dan Williams (22): mm: enhance region_is_ram() to distinguish 'unknown' vs 'mixed' arch, drivers: don't include asm/io.h directly, use linux/io.h instead cleanup IORESOURCE_CACHEABLE vs ioremap() intel_iommu: fix leaked ioremap mapping arch: introduce memremap() arm: switch from ioremap_cache to memremap x86: switch from ioremap_cache to memremap gma500: switch from acpi_os_ioremap to ioremap i915: switch from acpi_os_ioremap to ioremap acpi: switch from ioremap_cache to memremap toshiba laptop: replace ioremap_cache with ioremap memconsole: fix __iomem mishandling, switch to memremap visorbus: switch from ioremap_cache to memremap intel-iommu: switch from ioremap_cache to memremap libnvdimm, pmem: switch from ioremap_cache to memremap pxa2xx-flash: switch from ioremap_cache to memremap sfi: switch from ioremap_cache to memremap fbdev: switch from ioremap_wt to memremap pmem: switch from ioremap_wt to memremap arch: remove ioremap_cache, replace with arch_memremap arch: remove ioremap_wt, replace with arch_memremap pmem: convert to generic memremap Toshi Kani (3): mm, x86: Fix warning in ioremap RAM check mm, x86: Remove region_is_ram() call from ioremap mm: Fix bugs in region_is_ram() arch/arc/include/asm/io.h |1 arch/arm/Kconfig |1 arch/arm/include/asm/io.h | 13 +++- arch/arm/include/asm/xen/page.h|4 + arch/arm/mach-clps711x/board-cdb89712.c|2 - arch/arm/mach-shmobile/pm-rcar.c |2 - arch/arm/mm/ioremap.c | 12 +++- arch/arm/mm/nommu.c| 11 ++- arch/arm64/Kconfig |1 arch/arm64/include/asm/acpi.h | 10 +-- arch/arm64/include/asm/dmi.h |8 +-- arch/arm64/include/asm/io.h|8 ++- arch/arm64/kernel/efi.c|9 ++- arch/arm64/kernel/smp_spin_table.c | 19 +++--- arch/arm64/mm/ioremap.c| 20 ++ arch/avr32/include/asm/io.h|1 arch/frv/Kconfig |1 arch/frv/include/asm/io.h | 17 ++--- arch/frv/mm/kmap.c |6 ++ arch/ia64/Kconfig |1 arch/ia64/include/asm/io.h | 11 +++ arch/ia64/kernel/cyclone.c |2 - arch/m32r/include/asm/io.h |1 arch/m68k/Kconfig |1 arch/m68k/include/asm/io_mm.h | 14 +--- arch/m68k/include/asm/io_no.h | 12 ++-- arch/m68k/include/asm/raw_io.h |4 + arch/m68k/mm/kmap.c| 17 + arch/m68k/mm/sun3kmap.c|6 ++ arch/metag/include/asm/io.h|3 - arch/microblaze/include/asm/io.h |1 arch/mn10300/include/asm/io.h |1 arch/nios2/include/asm/io.h|1 arch/powerpc/kernel/pci_of_scan.c |2 - arch/s390/include/asm/io.h |1 arch/sh/Kconfig|1 arch/sh/include/asm/io.h |
[Intel-gfx] [PATCH] benchmarks: Do not install to system-wide bin/
These benchmarks are first-and-foremost development tools, not aimed at general users. As such they should not be installed into the system-wide bin/ directory, but installing into libexec/ is a possibility when we adpot a benchmarking system. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- benchmarks/Makefile.sources | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources index 7ad95a5..90dae18 100644 --- a/benchmarks/Makefile.sources +++ b/benchmarks/Makefile.sources @@ -1,4 +1,4 @@ -bin_PROGRAMS = \ +noinst_PROGRAMS = \ intel_upload_blit_large \ intel_upload_blit_large_gtt \ intel_upload_blit_large_map \ -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1.1 1/5] drm/i915: Handle return value in intel_pin_and_fence_fb_obj, v2.
On Tue, 2015-07-21 at 16:09 +0200, Maarten Lankhorst wrote: -EDEADLK has special meaning in atomic, but get_fence may call i915_find_fence_reg which can return -EDEADLK. This has special meaning in the atomic world, so convert the error to -EBUSY for this case. Doesn't this change the behavior of intel_crtc_page_flip() slightly? It now would return -EBUSY to user space if it can't find a fence instead of -EDEADLK. Not sure if that is a problem though. I don't expect user space would actually check for -EDEADLK. Ander Changes since v1: - Add comment in the code. Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com --- Like this? diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfee4771..11387f5ed681 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2395,8 +2395,20 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, * a fence as the cost is not that onerous. */ ret = i915_gem_object_get_fence(obj); - if (ret) + if (ret) { + if (ret == -EDEADLK) { + /* + * -EDEADLK means there are no free fences + * and no pending flips. + * + * This is propagated to atomic, but it uses + * -EDEADLK to force a locking recovery, so + * change the returned error to -EBUSY. + */ + ret = -EBUSY; + } goto err_unpin; + } i915_gem_object_pin_fence(obj); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android
There are two versions of gem_exec_nop.c in benchmarks and tests which causes the build system to have two build modules with the same name. This patch renames benchmarks/gem_exec_nop.c to benchmarks/gem_exec_nop_benchmark.c using the existing gem_userptr_benchmark.c as a naming convention. v2: Also rename gem_mmap to gem_mmap_benchmark. Another file which breaks android which was added after this patch was 1st submitted. Signed-off-by: Derek Morton derek.j.mor...@intel.com --- benchmarks/Makefile.sources | 9 +- benchmarks/gem_exec_nop.c | 153 - benchmarks/gem_exec_nop_benchmark.c | 153 + benchmarks/gem_mmap.c | 165 benchmarks/gem_mmap_benchmark.c | 165 5 files changed, 325 insertions(+), 320 deletions(-) delete mode 100644 benchmarks/gem_exec_nop.c create mode 100644 benchmarks/gem_exec_nop_benchmark.c delete mode 100644 benchmarks/gem_mmap.c create mode 100644 benchmarks/gem_mmap_benchmark.c diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources index 7ad95a5..35722b5 100644 --- a/benchmarks/Makefile.sources +++ b/benchmarks/Makefile.sources @@ -1,11 +1,16 @@ +# If you copy a test to benckmarks, rename it _benchmark +# The andriod build will fail when trying to build multiple binaries with +# the same name. + bin_PROGRAMS = \ intel_upload_blit_large \ intel_upload_blit_large_gtt \ intel_upload_blit_large_map \ intel_upload_blit_small \ - gem_exec_nop\ - gem_mmap\ + gem_exec_nop_benchmark \ + gem_mmap_benchmark \ gem_prw \ gem_userptr_benchmark \ kms_vblank \ $(NULL) + diff --git a/benchmarks/gem_exec_nop.c b/benchmarks/gem_exec_nop.c deleted file mode 100644 index 2a3abd2..000 --- a/benchmarks/gem_exec_nop.c +++ /dev/null @@ -1,153 +0,0 @@ -/* - * Copyright © 2011 Intel Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the Software), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - * - * Authors: - *Chris Wilson ch...@chris-wilson.co.uk - * - */ - -#include unistd.h -#include stdlib.h -#include stdint.h -#include stdio.h -#include string.h -#include fcntl.h -#include inttypes.h -#include errno.h -#include sys/stat.h -#include sys/ioctl.h -#include sys/time.h -#include time.h - -#include drm.h -#include ioctl_wrappers.h -#include drmtest.h -#include intel_io.h -#include igt_stats.h - -#define LOCAL_I915_EXEC_NO_RELOC (111) -#define LOCAL_I915_EXEC_HANDLE_LUT (112) - -static uint64_t elapsed(const struct timespec *start, - const struct timespec *end, - int loop) -{ - return (10ULL*(end-tv_sec - start-tv_sec) + (end-tv_nsec - start-tv_nsec))/loop; -} - -static int __gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf) -{ - int err = 0; - if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, execbuf)) - err = -errno; - return err; -} - -static uint32_t batch(int fd) -{ - const uint32_t buf[] = {MI_BATCH_BUFFER_END}; - uint32_t handle = gem_create(fd, 4096); - gem_write(fd, handle, 0, buf, sizeof(buf)); - return handle; -} - -static int loop(unsigned ring, int reps) -{ - struct drm_i915_gem_execbuffer2 execbuf; - struct drm_i915_gem_exec_object2 gem_exec; - int count, fd; - - fd = drm_open_any(); - - memset(gem_exec, 0, sizeof(gem_exec)); - gem_exec.handle = batch(fd); - - memset(execbuf, 0, sizeof(execbuf)); - execbuf.buffers_ptr = (uintptr_t)gem_exec; - execbuf.buffer_count = 1; - execbuf.flags = ring; - execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT; - execbuf.flags |=
Re: [Intel-gfx] [PATCH i-g-t] tests/drm_import_export: Add tests for prime/flink sharing races
On 24 July 2015 at 10:24, Michał Winiarski michal.winiar...@intel.com wrote: It is possible to race between unreference of the underlying BO and importing it from prime_fd/name. Verify that the behaviour of libdrm is consistent for prime/flink. Could you add this description into the source file as a comment? There also appears to be an extra white space change at the end of your patch. Signed-off-by: Michał Winiarski michal.winiar...@intel.com --- tests/drm_import_export.c | 103 ++ 1 file changed, 103 insertions(+) diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c index 57b13dd..db11c18 100644 --- a/tests/drm_import_export.c +++ b/tests/drm_import_export.c @@ -131,6 +131,98 @@ static void * test_thread(void * par) return NULL; } +#define IMPORT_RACE_LOOPS 10 + +struct import_race_thread_data { + int prime_fd; + uint32_t flink_name; + unsigned int stop; + pthread_mutex_t mutex; +}; + +static void *import_close_thread(void *data) +{ + struct import_race_thread_data *t = (struct import_race_thread_data *)data; + drm_intel_bo *bo; + pthread_mutex_lock(t-mutex); + while (!t-stop) { + pthread_mutex_unlock(t-mutex); + bo = NULL; + if (use_flink) + bo = drm_intel_bo_gem_create_from_name(bufmgr, buf-shared, t-flink_name); + else { + pthread_mutex_lock(t-mutex); + if (t-prime_fd != -1) { + bo = drm_intel_bo_gem_create_from_prime(bufmgr, t-prime_fd, 4096); + pthread_mutex_unlock(t-mutex); + } + else + /* We take the lock right after entering the loop */ + continue; + } + if (bo == NULL) { + /* +* If the bo is NULL it means that we've unreferenced in other +* thread - therefore we should expect ENOENT +*/ + igt_assert_eq(errno, ENOENT); + continue; + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t-mutex); + } + pthread_mutex_unlock(t-mutex); + + return NULL; +} + +static void test_import_close_race(void) +{ + pthread_t t; + unsigned int loops = IMPORT_RACE_LOOPS; + drm_intel_bo *bo; + struct import_race_thread_data t_data; + + memset(t_data, 0, sizeof(t_data)); + pthread_mutex_init(t_data.mutex, NULL); + t_data.prime_fd = -1; + + igt_assert_eq(pthread_create(t, NULL, import_close_thread , t_data), 0); + + while (loops--) { + bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096); + igt_assert(bo != NULL); + /* +* We setup the test in such way, that create_from_* can race between +* unreference. If we're using prime, prime_fd is always a valid fd. +*/ + if (use_flink) + igt_assert_eq(drm_intel_bo_flink(bo, (t_data.flink_name)), 0); + else { + pthread_mutex_lock(t_data.mutex); + igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, (t_data.prime_fd)), 0); + igt_assert(t_data.prime_fd != -1); + pthread_mutex_unlock(t_data.mutex); + } + + drm_intel_bo_unreference(bo); + + pthread_mutex_lock(t_data.mutex); + close(t_data.prime_fd); + t_data.prime_fd = -1; + pthread_mutex_unlock(t_data.mutex); + } + + pthread_mutex_lock(t_data.mutex); + t_data.stop = 1; + pthread_mutex_unlock(t_data.mutex); + + pthread_join(t, NULL); + pthread_mutex_destroy(t_data.mutex); +} + pthread_t test_thread_id1; pthread_t test_thread_id2; pthread_t test_thread_id3; @@ -153,6 +245,16 @@ igt_main { drm_intel_bufmgr_gem_enable_reuse(bufmgr); } + igt_subtest(import-close-race-flink) { + use_flink = true; + test_import_close_race(); + } + + igt_subtest(import-close-race-prime) { + use_flink = false; + test_import_close_race(); + } + igt_subtest(flink) { use_flink = true; @@ -180,4 +282,5 @@ igt_main { pthread_join(test_thread_id3, NULL); pthread_join(test_thread_id4, NULL); } + } -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android
On Fri, Jul 24, 2015 at 02:35:29PM +0100, Derek Morton wrote: There are two versions of gem_exec_nop.c in benchmarks and tests which causes the build system to have two build modules with the same name. This patch renames benchmarks/gem_exec_nop.c to benchmarks/gem_exec_nop_benchmark.c using the existing gem_userptr_benchmark.c as a naming convention. Surely a simpler fix than breaking external tools would be to fix the Makefile? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/13 v4] drm/i915: Implementation of GuC client
On Thu, Jul 09, 2015 at 07:29:10PM +0100, Dave Gordon wrote: A GuC client has its own doorbell and workqueue. It maintains the doorbell cache line, process description object and work queue item. A default guc_client is created for the i915 driver to use for normal-priority in-order submission. Note that the created client is not yet ready for use; doorbell allocation will fail as we haven't yet linked the GuC's context descriptor to the default contexts for each ring (see later patch). v2: Defer adding structure members until needed [Chris Wilson] Rationalise type declarations [Chris Wilson] v4: Rebased Issue: VIZ-4884 Signed-off-by: Alex Dai yu@intel.com Signed-off-by: Dave Gordon david.s.gor...@intel.com [TOR:] I had some non-critical questions below. Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com --- drivers/gpu/drm/i915/i915_guc_submission.c | 649 + drivers/gpu/drm/i915/intel_guc.h | 42 ++ drivers/gpu/drm/i915/intel_guc_loader.c| 12 + 3 files changed, 703 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index e9d46d6..25d8807 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -27,6 +27,512 @@ #include intel_guc.h /** + * DOC: GuC Client + * + * i915_guc_client: + * We use the term client to avoid confusion with contexts. A i915_guc_client is + * equivalent to GuC object guc_context_desc. This context descriptor is + * allocated from a pool of 1024 entries. Kernel driver will allocate doorbell + * and workqueue for it. Also the process descriptor (guc_process_desc), which + * is mapped to client space. So the client can write Work Item then ring the + * doorbell. + * + * To simplify the implementation, we allocate one gem object that contains all + * pages for doorbell, process descriptor and workqueue. + * + * The Scratch registers: + * There are 16 MMIO-based registers start from 0xC180. The kernel driver writes + * a value to the action register (SOFT_SCRATCH_0) along with any data. It then + * triggers an interrupt on the GuC via another register write (0xC4C8). + * Firmware writes a success/fail code back to the action register after + * processes the request. The kernel driver polls waiting for this update and + * then proceeds. + * See host2guc_action() + * + * Doorbells: + * Doorbells are interrupts to uKernel. A doorbell is a single cache line (QW) + * mapped into process space. + * + * Work Items: + * There are several types of work items that the host may place into a + * workqueue, each with its own requirements and limitations. Currently only + * WQ_TYPE_INORDER is needed to support legacy submission via GuC, which + * represents in-order queue. The kernel driver packs ring tail pointer and an + * ELSP context descriptor dword into Work Item. + * See guc_add_workqueue_item() + * + */ + +/* + * Read GuC command/status register (SOFT_SCRATCH_0) + * Return true if it contains a response rather than a command + */ +static inline bool host2guc_action_response(struct drm_i915_private *dev_priv, + u32 *status) +{ + u32 val = I915_READ(SOFT_SCRATCH(0)); + *status = val; + return GUC2HOST_IS_RESPONSE(val); +} + +static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) +{ + struct drm_i915_private *dev_priv = guc_to_i915(guc); + u32 status; + int i; + int ret; + + if (WARN_ON(len 1 || len 15)) + return -EINVAL; + [TOR:] Would it be good for host2guc_action to take a forcewake? There are several writes and polling reads for completion. Taking a forcewake could avoid surplus forcewakes for each register access. + spin_lock(dev_priv-guc.host2guc_lock); + + dev_priv-guc.action_count += 1; + dev_priv-guc.action_cmd = data[0]; + + for (i = 0; i len; i++) + I915_WRITE(SOFT_SCRATCH(i), data[i]); + + POSTING_READ(SOFT_SCRATCH(i - 1)); + + I915_WRITE(HOST2GUC_INTERRUPT, HOST2GUC_TRIGGER); + + ret = wait_for_atomic(host2guc_action_response(dev_priv, status), 10); [TOR:] Why 10? + if (status != GUC2HOST_STATUS_SUCCESS) { + /* either GuC doesn't respond, which is a TIMEOUT, + * or a failure code is returned. */ + if (ret != -ETIMEDOUT) + ret = -EIO; + + DRM_ERROR(GUC: host2guc action 0x%X failed. ret=%d + status=0x%08X response=0x%08X\n, + data[0], ret, status, + I915_READ(SOFT_SCRATCH(15))); + + dev_priv-guc.action_fail += 1; + dev_priv-guc.action_err = ret; + } + dev_priv-guc.action_status = status; + +
[Intel-gfx] [PATCH v2 12/25] i915: switch from acpi_os_ioremap to ioremap
acpi_os_ioremap uses cached mappings, however it appears that i915 wants to read dynamic platform state. Switch to ioremap() to prevent it reading stale state from cache. Cc: Daniel Vetter daniel.vet...@intel.com Cc: Jani Nikula jani.nik...@linux.intel.com Cc: intel-gfx@lists.freedesktop.org Cc: David Airlie airl...@linux.ie Cc: dri-de...@lists.freedesktop.org Signed-off-by: Dan Williams dan.j.willi...@intel.com --- drivers/gpu/drm/i915/intel_opregion.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 481337436f72..16ba7c67410d 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -863,7 +863,7 @@ int intel_opregion_setup(struct drm_device *dev) INIT_WORK(opregion-asle_work, asle_work); #endif - base = acpi_os_ioremap(asls, OPREGION_SIZE); + base = ioremap(asls, OPREGION_SIZE); if (!base) return -ENOMEM; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Extract i915_gem_fence.c
On Fri, Jul 24, 2015 at 01:55:11PM +0200, Daniel Vetter wrote: No code changes, just moving all the fence related code into a separate file (and avoiding a bunch of forward declarations while at it). As well as i915_gem_tiling? i915_gem_fence is a better name, so can we move the tiling ioctl to i915_gem_fence.c as well? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: kerneldoc for fences
On Fri, Jul 24, 2015 at 01:55:12PM +0200, Daniel Vetter wrote: Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- Documentation/DocBook/drm.tmpl| 5 +++ drivers/gpu/drm/i915/i915_gem_fence.c | 75 +++ 2 files changed, 80 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 458772e6ce08..86c9c453b218 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4137,6 +4137,11 @@ int num_ioctls;/synopsis !Idrivers/gpu/drm/i915/i915_gem_gtt.c /sect2 sect2 +titleGlobal GTT Fence Handling/title +!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence handling +!Idrivers/gpu/drm/i915/i915_gem_fence.c + /sect2 + sect2 titleBuffer Object Eviction/title para This section documents the interface functions for evicting buffer diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c index d3284ee04794..63d8d0d8a9cb 100644 --- a/drivers/gpu/drm/i915/i915_gem_fence.c +++ b/drivers/gpu/drm/i915/i915_gem_fence.c @@ -25,6 +25,36 @@ #include drm/i915_drm.h #include i915_drv.h +/** + * DOC: fence handling DOC: fence register handling Might as well aim not to be ambiguous when you start out by pointing out the confusion between the GTT detiling registers and dma-fence. + * + * Important to avoid confusions: fences in the i915 driver are not execution + * fences used to track command completion but hardware detiler objects which + * wrap a given range of the global GTT. Each platform has only a fairly limited + * set of these objects. Hmm, good point. Maybe i915_gem_tiling was the better name after all! Otoh, i915_gem_fence is better as it prevents us from wondering whether this is suitable for Yf etc. We really should emphasize that again in the set-tiling documentation - that is not about declaring the tiling for the buffer per-se, but about fence allocation. + * + * Fences are used to detile GTT memory mappings. They're also connected to the + * hardware frontbuffer render tracking and hence interract with frontbuffer + * conmpression. Furthermore on older platforms fences are required for tiled + * objects used by the display engine. They can also be used by the render + * engine - they're required for blitter commands and are optional for render + * commands. But on gen4+ both display (with the exception of fbc) and rendering + * have their own tiling state bits and don't need fences. + * + * Also note that fences only support X and Y tiling and hence can't be used for + * the fancier new tiling formats like W, Ys and Yf. + * + * Finally note that because fences are such a restricted resource they're + * dynamically associated with objects. Furthermore fence state is committed to + * the hardware lazily to avoid unecessary stalls on gen2/3. Therefore code must + * explictly call i915_gem_object_get_fence() to synchronize fencing status + * for cpu access. Also note that some code wants an unfenced view, for those + * cases the fence can be removed forcefully with i915_gem_object_put_fence(). + * + * Internally these functions will synchronize with userspace access by removing + * ptes as needed. Revoking CPU ptes into GTT mmaps. Important when discussing PTEs to be clear when we don't mean the GTT PTEs (which I think is the default for the driver). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC CABC PATCH v2 3/3] drm/i915: CABC support for backlight control
In CABC (Content Adaptive Brightness Control) content grey level scale can be increased while simultaneously decreasing brightness of the backlight to achieve same perceived brightness. The CABC is not standardized and panel vendors are free to follow their implementation. The CABC implementaion here assumes that the panels use standard SW register for control. In this design there will be no PWM signal from the SoC and DCS commands are sent to enable and control the backlight brightness. v2: - Created a new backlight driver for cabc, which will be registered only when it cabc is supported by panel. (Addressed comment from Daniel Vetter) Signed-off-by: Deepak M m.dee...@intel.com --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/intel_drv.h |3 +- drivers/gpu/drm/i915/intel_dsi.c | 13 ++ drivers/gpu/drm/i915/intel_dsi_cabc.c | 349 + drivers/gpu/drm/i915/intel_dsi_cabc.h | 45 + drivers/gpu/drm/i915/intel_panel.c| 22 ++- include/video/mipi_display.h |8 + 7 files changed, 436 insertions(+), 5 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.h diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index de21965..a73953c 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -75,6 +75,7 @@ i915-y += dvo_ch7017.o \ intel_dp_mst.o \ intel_dsi.o \ intel_dsi_pll.o \ + intel_dsi_cabc.o \ intel_dsi_panel_vbt.o \ intel_dvo.o \ intel_hdmi.o \ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3f0a890..9f806dd5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1319,7 +1319,8 @@ extern struct drm_display_mode *intel_find_panel_downclock( struct drm_connector *connector); void intel_backlight_register(struct drm_device *dev); void intel_backlight_unregister(struct drm_device *dev); - +extern int cabc_backlight_device_register(struct intel_connector *connector); +extern void cabc_backlight_device_unregister(struct intel_connector *connector); /* intel_psr.c */ void intel_psr_enable(struct intel_dp *intel_dp); diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 98998e9..8f006f2 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -34,6 +34,7 @@ #include i915_drv.h #include intel_drv.h #include intel_dsi.h +#include intel_dsi_cabc.h static const struct { u16 panel_id; @@ -376,6 +377,7 @@ static void intel_dsi_enable(struct intel_encoder *encoder) struct drm_device *dev = encoder-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder-base); + struct intel_connector *connector = intel_dsi-attached_connector; enum port port; DRM_DEBUG_KMS(\n); @@ -396,6 +398,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder) intel_dsi_port_enable(encoder); } + + if (dev_priv-vbt.dsi.config-cabc_supported) + cabc_enable_backlight(connector); } static void intel_dsi_pre_enable(struct intel_encoder *encoder) @@ -469,11 +474,15 @@ static void intel_dsi_disable(struct intel_encoder *encoder) struct drm_device *dev = encoder-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder-base); + struct intel_connector *connector = intel_dsi-attached_connector; enum port port; u32 temp; DRM_DEBUG_KMS(\n); + if (dev_priv-vbt.dsi.config-cabc_supported) + cabc_disable_backlight(connector); + if (is_vid_mode(intel_dsi)) { for_each_dsi_port(port, intel_dsi-ports) wait_for_dsi_fifo_empty(intel_dsi, port); @@ -1099,6 +1108,10 @@ void intel_dsi_init(struct drm_device *dev) intel_panel_init(intel_connector-panel, fixed_mode, NULL); + if (dev_priv-vbt.dsi.config-cabc_supported) + cabc_setup_backlight(connector, + intel_encoder-crtc_mask == + (1 PIPE_A) ? PIPE_A : PIPE_B); return; err: diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c b/drivers/gpu/drm/i915/intel_dsi_cabc.c new file mode 100644 index 000..2a78326 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c @@ -0,0 +1,349 @@ +/* + * Copyright © 2006-2010 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish,
[Intel-gfx] [PATCH 1/3] drm/i915: Clean up Makefile
Sorting became confused and a few new files ended up in strange places. Also move i915_irq.c to core since with the recent-ish extraction of i915_gpu_error.c and intel_hotplug.c it's more and more really just basic irq handling code. When adding new files please don't put them somewhere randomly. Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- drivers/gpu/drm/i915/Makefile | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index e52e01251644..bf91482e14a3 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -6,12 +6,13 @@ # core driver code i915-y := i915_drv.o \ + i915_irq.o \ i915_params.o \ i915_suspend.o \ i915_sysfs.o \ + intel_csr.o \ intel_pm.o \ - intel_runtime_pm.o \ - intel_csr.o + intel_runtime_pm.o i915-$(CONFIG_COMPAT) += i915_ioc32.o i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o @@ -20,21 +21,19 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o i915-y += i915_cmd_parser.o \ i915_gem_batch_pool.o \ i915_gem_context.o \ - i915_gem_render_state.o \ i915_gem_debug.o \ i915_gem_dmabuf.o \ i915_gem_evict.o \ i915_gem_execbuffer.o \ i915_gem_gtt.o \ i915_gem.o \ + i915_gem_render_state.o \ i915_gem_shrinker.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ i915_gem_userptr.o \ i915_gpu_error.o \ - i915_irq.o \ i915_trace_points.o \ - intel_hotplug.o \ intel_lrc.o \ intel_mocs.o \ intel_ringbuffer.o \ @@ -48,11 +47,14 @@ i915-y += intel_renderstate_gen6.o \ # modesetting core code i915-y += intel_audio.o \ + intel_atomic.o \ + intel_atomic_plane.o \ intel_bios.o \ intel_display.o \ intel_fbc.o \ intel_fifo_underrun.o \ intel_frontbuffer.o \ + intel_hotplug.o \ intel_modes.o \ intel_overlay.o \ intel_psr.o \ @@ -68,15 +70,13 @@ i915-y += dvo_ch7017.o \ dvo_ns2501.o \ dvo_sil164.o \ dvo_tfp410.o \ - intel_atomic.o \ - intel_atomic_plane.o \ intel_crt.o \ intel_ddi.o \ - intel_dp.o \ intel_dp_mst.o \ + intel_dp.o \ intel_dsi.o \ - intel_dsi_pll.o \ intel_dsi_panel_vbt.o \ + intel_dsi_pll.o \ intel_dvo.o \ intel_hdmi.o \ intel_i2c.o \ -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Extract i915_gem_fence.c
No code changes, just moving all the fence related code into a separate file (and avoiding a bunch of forward declarations while at it). Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.h | 16 +- drivers/gpu/drm/i915/i915_gem.c | 401 drivers/gpu/drm/i915/i915_gem_fence.c | 422 ++ 4 files changed, 432 insertions(+), 408 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_fence.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index bf91482e14a3..41fb8a9c5bef 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -25,6 +25,7 @@ i915-y += i915_cmd_parser.o \ i915_gem_dmabuf.o \ i915_gem_evict.o \ i915_gem_execbuffer.o \ + i915_gem_fence.o \ i915_gem_gtt.o \ i915_gem.o \ i915_gem_render_state.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 773883d54b28..4c34fb15d0df 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2858,11 +2858,6 @@ static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req, int __must_check i915_gem_get_seqno(struct drm_device *dev, u32 *seqno); int __must_check i915_gem_set_seqno(struct drm_device *dev, u32 seqno); -int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj); -int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj); - -bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj); -void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj); struct drm_i915_gem_request * i915_gem_find_active_request(struct intel_engine_cs *ring); @@ -2960,8 +2955,6 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev, struct dma_buf *i915_gem_prime_export(struct drm_device *dev, struct drm_gem_object *gem_obj, int flags); -void i915_gem_restore_fences(struct drm_device *dev); - unsigned long i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o, const struct i915_ggtt_view *view); @@ -3056,6 +3049,15 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj) i915_gem_object_ggtt_unpin_view(obj, i915_ggtt_view_normal); } +/* i915_gem_fence.c */ +int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj); +int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj); + +bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj); +void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj); + +void i915_gem_restore_fences(struct drm_device *dev); + /* i915_gem_context.c */ int __must_check i915_gem_context_init(struct drm_device *dev); void i915_gem_context_fini(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 322bbefafbc3..5d685789b1f9 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -46,11 +46,6 @@ static void i915_gem_object_retire__write(struct drm_i915_gem_object *obj); static void i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring); -static void i915_gem_write_fence(struct drm_device *dev, int reg, -struct drm_i915_gem_object *obj); -static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj, -struct drm_i915_fence_reg *fence, -bool enable); static bool cpu_cache_is_coherent(struct drm_device *dev, enum i915_cache_level level) @@ -66,18 +61,6 @@ static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj) return obj-pin_display; } -static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj) -{ - if (obj-tiling_mode) - i915_gem_release_mmap(obj); - - /* As we do not have an associated fence register, we will force -* a tiling change if we ever need to acquire one. -*/ - obj-fence_dirty = false; - obj-fence_reg = I915_FENCE_REG_NONE; -} - /* some bookkeeping */ static void i915_gem_info_add_obj(struct drm_i915_private *dev_priv, size_t size) @@ -2793,27 +2776,6 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv, } } -void i915_gem_restore_fences(struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev-dev_private; - int i; - - for (i = 0; i dev_priv-num_fence_regs; i++) { - struct drm_i915_fence_reg *reg = dev_priv-fence_regs[i]; - - /* -* Commit delayed tiling changes if we have an object still -* attached to the fence, otherwise just clear the fence. -
[Intel-gfx] [PATCH 3/3] drm/i915: kerneldoc for fences
Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- Documentation/DocBook/drm.tmpl| 5 +++ drivers/gpu/drm/i915/i915_gem_fence.c | 75 +++ 2 files changed, 80 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 458772e6ce08..86c9c453b218 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4137,6 +4137,11 @@ int num_ioctls;/synopsis !Idrivers/gpu/drm/i915/i915_gem_gtt.c /sect2 sect2 +titleGlobal GTT Fence Handling/title +!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence handling +!Idrivers/gpu/drm/i915/i915_gem_fence.c + /sect2 + sect2 titleBuffer Object Eviction/title para This section documents the interface functions for evicting buffer diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c index d3284ee04794..63d8d0d8a9cb 100644 --- a/drivers/gpu/drm/i915/i915_gem_fence.c +++ b/drivers/gpu/drm/i915/i915_gem_fence.c @@ -25,6 +25,36 @@ #include drm/i915_drm.h #include i915_drv.h +/** + * DOC: fence handling + * + * Important to avoid confusions: fences in the i915 driver are not execution + * fences used to track command completion but hardware detiler objects which + * wrap a given range of the global GTT. Each platform has only a fairly limited + * set of these objects. + * + * Fences are used to detile GTT memory mappings. They're also connected to the + * hardware frontbuffer render tracking and hence interract with frontbuffer + * conmpression. Furthermore on older platforms fences are required for tiled + * objects used by the display engine. They can also be used by the render + * engine - they're required for blitter commands and are optional for render + * commands. But on gen4+ both display (with the exception of fbc) and rendering + * have their own tiling state bits and don't need fences. + * + * Also note that fences only support X and Y tiling and hence can't be used for + * the fancier new tiling formats like W, Ys and Yf. + * + * Finally note that because fences are such a restricted resource they're + * dynamically associated with objects. Furthermore fence state is committed to + * the hardware lazily to avoid unecessary stalls on gen2/3. Therefore code must + * explictly call i915_gem_object_get_fence() to synchronize fencing status + * for cpu access. Also note that some code wants an unfenced view, for those + * cases the fence can be removed forcefully with i915_gem_object_put_fence(). + * + * Internally these functions will synchronize with userspace access by removing + * ptes as needed. + */ + static void i965_write_fence_reg(struct drm_device *dev, int reg, struct drm_i915_gem_object *obj) { @@ -247,6 +277,17 @@ i915_gem_object_wait_fence(struct drm_i915_gem_object *obj) return 0; } +/** + * i915_gem_object_put_fence - force-remove fence for an object + * @obj: object to map through a fence reg + * + * This function force-removes any fence from the given object, which is useful + * if the kernel wants to do untiled GTT access. + * + * Returns: + * + * 0 on success, negative error code on failure. + */ int i915_gem_object_put_fence(struct drm_i915_gem_object *obj) { @@ -322,6 +363,10 @@ deadlock: * and tiling format. * * For an untiled surface, this removes any existing fence. + * + * Returns: + * + * 0 on success, negative error code on failure. */ int i915_gem_object_get_fence(struct drm_i915_gem_object *obj) @@ -374,6 +419,21 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj) return 0; } +/** + * i915_gem_object_pin_fence - pin fencing state + * @obj: object to pin fencing for + * + * This pins the fencing state (whether tiled or untiled) to make sure the + * object is ready to be used as a scanout target. Fencing status must be + * synchronize first by calling i915_gem_object_get_fence(): + * + * The resulting fence pin reference must be released again with + * i915_gem_object_unpin_fence(). + * + * Returns: + * + * True if the object has a fence, false otherwise. + */ bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj) { @@ -390,6 +450,14 @@ i915_gem_object_pin_fence(struct drm_i915_gem_object *obj) return false; } +/** + * i915_gem_object_unpin_fence - unpin fencing state + * @obj: object to unpin fencing for + * + * This releases the fence pin reference acquired through + * i915_gem_object_pin_fence. It will handle both objects with and without an + * attached fence correctly, callers do not need to distinguish this. + */ void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj) { @@ -400,6 +468,13 @@ i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj) } } +/** + * i915_gem_restore_fences - restore fence state + * @dev: DRM device + * + * Restore the hw fence state to match the software tracking again, to