[Intel-gfx] [PATCH] [v2] drm/i915: Exec flag to force non IA-Coherent cache for Gen9+
Starting from Gen9 we can use IA-Coherent caches. Generally, coherency can be programmed using RENDER_SURFACE_STATE or BTI 255, depending if surface state model or stateless model is used. It is important to control whether IA or GPU cache coherency should be used, especially for non-LLC devices. However this control is complicated when stateless memory access model is in action. It would require dedicated ISA code depending on coherency requirement. By setting HDC_FORCE_NON_COHERENT we *Force* data port to ignore these attributes and all caches are GPU-Coherent. This register is part of HW context, however it is private and cannot be programmed from non-privileged batch buffer. Default operation mode is as programmed by workaround. When WaForceEnableNonCoherent is in place caches are GPU-Coherent and we should not change it back to IA-Coherent because this can lead to GPU hangs (as workaround description says). A new device parameter is to inform user space about kernel capability. It tells if can request to disable IA-Coherency. Exec flag is to allow UMD to decide whether IA-Coherency is not needed for submitted batch buffer. Exec flag behavior: 1. flag is not set - use system default 2. flag is set but WaForceEnableNonCoherent is a) not programmed - *Force* GPU-Coherent cache by setting HDC_FORCE_NON_COHERENT prior to bb_start and clearing after b) programmed - do nothing, GPU-Coherent is already in place v2: Ringbufer handling fixes (Chris) Moved workarounds to common place (Chris) Removed flag cleanup (Dave) Updated commit message to reflect comments (Chris,Dave) Signed-off-by: Artur Harasimiuk --- drivers/gpu/drm/i915/i915_dma.c| 4 drivers/gpu/drm/i915/i915_drv.h| 4 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 drivers/gpu/drm/i915/intel_lrc.c | 38 ++ drivers/gpu/drm/i915/intel_ringbuffer.c| 2 ++ include/uapi/drm/i915_drm.h| 8 ++- 6 files changed, 59 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 44a896c..f735e56 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -172,6 +172,10 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_HAS_EXEC_SOFTPIN: value = 1; break; + case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT: + value = !dev_priv->workarounds.WaForceEnableNonCoherent && + INTEL_INFO(dev)->gen >= 9; + break; default: DRM_DEBUG("Unknown parameter %d\n", param->param); return -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 104bd18..793be854 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1658,6 +1658,10 @@ struct i915_wa_reg { struct i915_workarounds { struct i915_wa_reg reg[I915_MAX_WA_REGS]; u32 count; + + struct { + unsigned int WaForceEnableNonCoherent:1; + }; }; struct i915_virtual_gpu { diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d469c47..5db3806 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, if (!i915_gem_check_execbuffer(args)) return -EINVAL; + if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) && + INTEL_INFO(dev)->gen < 9) + return -EINVAL; + ret = validate_exec_list(dev, exec, args->buffer_count); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index ab344e0..f37d12f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -879,6 +879,36 @@ int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request) return intel_logical_ring_begin(request, 0); } +static inline int +intel_lr_emit_force_non_coherent(struct i915_execbuffer_params *params, + struct drm_i915_gem_execbuffer2 *args, bool force) +{ + struct drm_device *dev = params->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + int ret; + + if (dev_priv->workarounds.WaForceEnableNonCoherent) + return 0; + + if (args->flags & I915_EXEC_FORCE_NON_COHERENT) { + struct intel_ringbuffer *ringbuf = params->request->ringbuf; + + ret = intel_logical_ring_begin(params->request, 4); + if (ret) + return ret; + + intel_logical_ring_emit(ringbuf, MI_NOOP); + intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1)); + intel_l
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ (rev2)
== Summary == Built on 4efb7ea717f8478860c4cb829fc0956f195c0b50 drm-intel-nightly: 2016y-01m-15d-02h-35m-27s UTC integration manifest Test gem_ctx_basic: pass -> FAIL (bdw-ultra) Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (skl-i5k-2) UNSTABLE pass -> DMESG-WARN (bdw-nuci7) dmesg-warn -> PASS (skl-i7k-2) UNSTABLE Test kms_pipe_crc_basic: Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (byt-nuc) bdw-nuci7total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:131 dwarn:0 dfail:0 fail:1 skip:6 byt-nuc total:141 pass:121 dwarn:5 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1193/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tools/intel_bios_reader: Fix typo on dump info
Fix typo on intel_bios_reader.c Signed-off-by: Mika Kahola --- tools/intel_bios_reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/intel_bios_reader.c b/tools/intel_bios_reader.c index b31f648..5ca50ff 100644 --- a/tools/intel_bios_reader.c +++ b/tools/intel_bios_reader.c @@ -843,7 +843,7 @@ static void dump_mipi_config(const struct bdb_block *block) printf("\tMIPI PPS\n"); printf("\t\tPanel power ON delay: %d\n", pps->panel_on_delay); - printf("\t\tPanel power on to Baklight enable delay: %d\n", pps->bl_enable_delay); + printf("\t\tPanel power on to Backlight enable delay: %d\n", pps->bl_enable_delay); printf("\t\tBacklight disable to Panel power OFF delay: %d\n", pps->bl_disable_delay); printf("\t\tPanel power OFF delay: %d\n", pps->panel_off_delay); printf("\t\tPanel power cycle delay: %d\n", pps->panel_power_cycle_delay); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] lib/igt_kms: Add COMIT_ATOMIC to igt_display_commit2()
So far, we had only COMMIT_UNIVERSAL and COMMIT_LEGACY, using drmModeSetPlane()/drmSetCrtc(). This patch adds COMMIT_ATOMIC to igt_display_commit2() that makes use of drmModeAtomicCommit(). Signed-off-by: Marius Vlad --- lib/igt_kms.c | 190 +- lib/igt_kms.h | 33 +- 2 files changed, 221 insertions(+), 2 deletions(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 497118a..61f7a39 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -1306,6 +1306,191 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane) igt_assert(r == 0); \ } +static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = { + "SRC_X", + "SRC_Y", + "SRC_W", + "SRC_H", + "CRTC_X", + "CRTC_Y", + "CRTC_W", + "CRTC_H", + "FB_ID", + "CRTC_ID", + "type" +}; + +/* + * Retrieve all the properies specified in props_name and store them into + * plane->atomic_props_plane. + */ +static void +igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane, + int type, int num_props, const char **prop_names) +{ + drmModeObjectPropertiesPtr props; + int i, j, fd; + + fd = display->drm_fd; + + props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, type); + igt_assert(props); + + for (i = 0; i < props->count_props; i++) { + drmModePropertyPtr prop = + drmModeGetProperty(fd, props->props[i]); + + for (j = 0; j < num_props; j++) { + if (strcmp(prop->name, prop_names[j]) != 0) + continue; + plane->atomic_props_plane[j] = props->props[i]; + break; + } + + drmModeFreeProperty(prop); + } + + drmModeFreeObjectProperties(props); +} + +/* + * Commit position and fb changes to a DRM plane via the AtomicCommit() + * ioctl; if the DRM call to program the plane fails, we'll either fail + * immediately (for tests that expect the commit to succeed) or return the + * failure code (for tests that expect a specific error code). + */ +static int +igt_atomic_plane_commit(igt_plane_t *plane, igt_output_t *output, + bool fail_on_error) +{ + igt_display_t *display = output->display; + + uint32_t fb_id, crtc_id; + int ret; + uint32_t src_x; + uint32_t src_y; + uint32_t src_w; + uint32_t src_h; + int32_t crtc_x; + int32_t crtc_y; + uint32_t crtc_w; + uint32_t crtc_h; + drmModeAtomicReq *req; + + igt_assert(plane->drm_plane); + + do_or_die(drmSetClientCap(display->drm_fd, DRM_CLIENT_CAP_ATOMIC, 1)); + + /* it's an error to try an unsupported feature */ + igt_assert(igt_plane_supports_rotation(plane) || + !plane->rotation_changed); + + fb_id = igt_plane_get_fb_id(plane); + crtc_id = output->config.crtc->crtc_id; + + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) { + + LOG(display, + "%s: drmModeAtomicCommit pipe %s, plane %d, disabling\n", +igt_output_name(output), +kmstest_pipe_name(output->config.pipe), +plane->index); + + req = drmModeAtomicAlloc(); + igt_atomic_fill_plane_props(display, plane, + DRM_MODE_OBJECT_PLANE, + IGT_NUM_PLANE_PROPS, + igt_plane_prop_names); + + drmModeAtomicSetCursor(req, 0); + + /* populate plane req */ + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id); + + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0)); + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0)); + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0)); + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0)); + + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0); + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0); + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0); + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0); + + ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL); + drmModeAtomicFree(req); + + CHECK_RETURN(ret, fail_on_error); + + } else if (plane->fb_changed || plane->position_changed || + plane->size_changed) { + + src_x = IGT_FIXED(plane->fb->src_x,0); /* src_x */ + src_y = IGT_FIXED(plane->fb->src_y,0); /* src_y */ +
[Intel-gfx] [PATCH i-g-t 1/2 v2] lib/igt_kms: Various fixes and add remaining documentation
lib/igt_kms: Fix minor spelling/gtk-doc links. Style changes -- trimmed several calls to asprintf(). Placed SECTION header at the beginning of the file. Add documentation for remaining functions and macros. v2: Initially, I wanted to aggregate the documentation in header file(s). This version keeps the initial format of the documentation. Signed-off-by: Marius Vlad --- lib/igt_kms.c | 160 +++--- lib/igt_kms.h | 123 ++-- 2 files changed, 236 insertions(+), 47 deletions(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 5d5a95c..c7a0b77 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -49,6 +49,30 @@ #include "intel_chipset.h" #include "igt_debugfs.h" +/** + * SECTION:igt_kms + * @short_description: Kernel modesetting support library + * @title: KMS + * @include: igt.h + * + * This library provides support to enumerate and set modeset configurations. + * + * There are two parts in this library: First the low level helper function + * which directly build on top of raw ioctls or the interfaces provided by + * libdrm. Those functions all have a kmstest_ prefix. + * + * The second part is a high-level library to manage modeset configurations + * which abstracts away some of the low-level details like the difference + * between legacy and universal plane support for setting cursors or in the + * future the difference between legacy and atomic commit. These high-level + * functions have all igt_ prefixes. This part is still very much work in + * progress and so also lacks a bit documentation for the individual functions. + * + * Note that this library's header pulls in the [i-g-t + * framebuffer](intel-gpu-tools-Framebuffer.html) library as a + * dependency. + */ + /* list of connectors that need resetting on exit */ #define MAX_CONNECTORS 32 static char *forced_connectors[MAX_CONNECTORS + 1]; @@ -104,7 +128,7 @@ static void update_edid_csum(unsigned char *edid) * * Returns: a basic edid block */ -const unsigned char* igt_kms_get_base_edid(void) +const unsigned char *igt_kms_get_base_edid(void) { update_edid_csum(base_edid); @@ -145,7 +169,7 @@ const unsigned char* igt_kms_get_base_edid(void) * * Returns: an alternate edid block */ -const unsigned char* igt_kms_get_alt_edid(void) +const unsigned char *igt_kms_get_alt_edid(void) { update_edid_csum(alt_edid); @@ -153,33 +177,10 @@ const unsigned char* igt_kms_get_alt_edid(void) } /** - * SECTION:igt_kms - * @short_description: Kernel modesetting support library - * @title: KMS - * @include: igt.h - * - * This library provides support to enumerate and set modeset configurations. - * - * There are two parts in this library: First the low level helper function - * which directly build on top of raw ioctls or the interfaces provided by - * libdrm. Those functions all have a kmstest_ prefix. - * - * The second part is a high-level library to manage modeset configurations - * which abstracts away some of the low-level details like the difference - * between legacy and universal plane support for setting cursors or in the - * future the difference between legacy and atomic commit. These high-level - * functions have all igt_ prefixes. This part is still very much work in - * progress and so also lacks a bit documentation for the individual functions. - * - * Note that this library's header pulls in the [i-g-t framebuffer](intel-gpu-tools-i-g-t-framebuffer.html) - * library as a dependency. - */ - -/** * kmstest_pipe_name: * @pipe: display pipe * - * Returns: String represnting @pipe, e.g. "A". + * Returns: String representing @pipe, e.g. "A". */ const char *kmstest_pipe_name(enum pipe pipe) { @@ -195,7 +196,7 @@ const char *kmstest_pipe_name(enum pipe pipe) * kmstest_plane_name: * @plane: display plane * - * Returns: String represnting @pipe, e.g. "plane1". + * Returns: String representing @pipe, e.g. "plane1". */ const char *kmstest_plane_name(enum igt_plane plane) { @@ -239,7 +240,7 @@ static const char *mode_stereo_name(const drmModeModeInfo *mode) * kmstest_dump_mode: * @mode: libdrm mode structure * - * Prints @mode to stdout in a huma-readable form. + * Prints @mode to stdout in a human-readable form. */ void kmstest_dump_mode(drmModeModeInfo *mode) { @@ -413,8 +414,9 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector, break; } - igt_assert_neq(asprintf(&path, "%s-%d/force", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id), - -1); + igt_assert_neq(asprintf(&path, "%s-%d/force", + kmstest_connector_type_str(connector->connector_type), + connector->connector_type_id), -1); debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC); if (debugfs_fd == -1) { @@ -478,8 +480,9 @@ void kmstest_force_edid(int drm_fd, drmModeCo
[Intel-gfx] [PATCH i-g-t 2/2] lib/igt_kms: Short description between Intel/DRM terminology.
lib/igt_kms: Briefly describe Intel-to-DRM mapping between pipes, encoders and connectors. Signed-off-by: Marius Vlad --- lib/igt_kms.c | 82 +++ 1 file changed, 82 insertions(+) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index c7a0b77..caa8837 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -68,6 +68,88 @@ * functions have all igt_ prefixes. This part is still very much work in * progress and so also lacks a bit documentation for the individual functions. * + * Intel/DRM terminology and display connections: + * + * Intel documentation describes the road from memory to an output connector as + * follows: + * + * |[!<-- language="C" --> + * .. .---. .-. .-. + * | Memory |>| Pipes |>| Transcoders |>| DDI | + * '' '---' '-' '-' + * ]| + * + * Pipes represent the front-end of the display contain the planes, blending, + * and color correction, while the transcoders contain timing generators, + * encoders, A/V mixers and PSR (Panel-Self Refresh) controllers. Finally the + * DDI represent the connectors attached to the display. + * + * + * In DRM we have the following: + * + * |[!<-- language="C" --> + * .---. .---. .---. .. + * | Framebuff |>| Pipes |>| Encoders |>| Connectors | + * '---' '---' '---' '' + * ]| + * + * + * The frame buffer ties a reference to a memory object and provides a pointer + * to the actual data. + * + * The pipe is used to set the display mode, timings and gamma tables. On some + * hardware models this is tied with the transcoder. In DRM-parlance this is + * referred as a CRTC. + * + * Each pipe has multiple planes. On older hardware these planes where known as + * primary plane, overlay/sprite plane, and cursor plane. From GEN9 (SKL/BXT) + * each pipe has three planes and a cursor plane. Each plane can be used as a + * primary, as a sprite or as an overlay plane. The planes are the ones that + * retrieve the pixels from memory and pushes them to the encoder. + * + * A pipe prior to GEN9: + * + * |[!<-- language="C" --> + * .. + * | Memory | .. + * ||>| Cursor |-->... + * || '' + * || + * || .. + * ||>| Sprite |-->... + * || '' + * || + * || .-. + * ||->| Primary |>... + * || '-' + * '' + * ]| + * + * A pipe with universal planes: + * + * |[!<-- language="C" --> + * .. + * | Memory | .. + * ||>| Cursor |-->... + * || '' + * || + * || .---. + * ||>| Plane |-->... + * || '---' + * || + * || .---. + * ||->| Plane |>... + * || '---' + * '' + * ]| + * + * The encoder is used to convert, depending on the output, pixels from pipes + * to signals understood by the connector. + * + * The connector will connect to the output display. This contains information + * about the attached display such as EDID, DPMS and information about modes + * supported by the display. + * * Note that this library's header pulls in the [i-g-t * framebuffer](intel-gpu-tools-Framebuffer.html) library as a * dependency. -- 2.6.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] igt/gem_softpin: Remove false dependencies on esoteric features
Hi, On 14/01/16 11:02, Chris Wilson wrote: For softpinning, we do not require either userptr or extended ppgtt, so remove those requirements and make the tests work universally. (Certain ABI tests require large GTT, or per-process GTT.) In the process, make the tests more extensive - validate overlapping handling more careful, explicitly test no-relocation support, validate more ABI handling. And for fun, cause a kernel GPF. Signed-off-by: Chris Wilson --- tests/gem_softpin.c | 1313 +-- 1 file changed, 324 insertions(+), 989 deletions(-) Adding some people to Cc who could be potential reviewers. We were tracking blanket improvements agreed during code review of the initial version in VIZ-6951. I've put a reference to this patch in there. Regards, Tvrtko diff --git a/tests/gem_softpin.c b/tests/gem_softpin.c index 1cbde4e..f188559 100644 --- a/tests/gem_softpin.c +++ b/tests/gem_softpin.c @@ -26,80 +26,10 @@ * */ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include "drm.h" -#include "ioctl_wrappers.h" -#include "drmtest.h" -#include "intel_chipset.h" -#include "intel_io.h" -#include "i915_drm.h" -#include -#include -#include -#include -#include "igt_kms.h" -#include -#include -#include - -#define BO_SIZE 4096 -#define MULTIPAGE_BO_SIZE 2 * BO_SIZE -#define STORE_BATCH_BUFFER_SIZE 4 +#include "igt.h" + #define EXEC_OBJECT_PINNED(1<<4) #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) -#define SHARED_BUFFER_SIZE 4096 - -typedef struct drm_i915_gem_userptr i915_gem_userptr; - -static uint32_t init_userptr(int fd, i915_gem_userptr *, void *ptr, uint64_t size); -static void *create_mem_buffer(uint64_t size); -static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr); -static void gem_pin_userptr_test(void); -static void gem_pin_bo_test(void); -static void gem_pin_invalid_vma_test(bool test_decouple_flags, bool test_canonical_offset); -static void gem_pin_overlap_test(void); -static void gem_pin_high_address_test(void); - -#define NO_PPGTT 0 -#define ALIASING_PPGTT 1 -#define FULL_32_BIT_PPGTT 2 -#define FULL_48_BIT_PPGTT 3 -/* uses_full_ppgtt - * Finds supported PPGTT details. - * @fd DRM fd - * @min can be - * 0 - No PPGTT - * 1 - Aliasing PPGTT - * 2 - Full PPGTT (32b) - * 3 - Full PPGTT (48b) - * RETURNS true/false if min support is present -*/ -static bool uses_full_ppgtt(int fd, int min) -{ - struct drm_i915_getparam gp; - int val = 0; - - memset(&gp, 0, sizeof(gp)); - gp.param = 18; /* HAS_ALIASING_PPGTT */ - gp.value = &val; - - if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp)) - return 0; - - errno = 0; - return val >= min; -} /* has_softpin_support * Finds if softpin feature is supported @@ -121,83 +51,6 @@ static bool has_softpin_support(int fd) return (val == 1); } -/* gem_call_userptr_ioctl - * Helper to call ioctl - TODO: move to lib - * @fd - drm fd - * @userptr - pointer to initialised userptr - * RETURNS status of ioctl call -*/ -static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr) -{ - int ret; - - ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_USERPTR, userptr); - - if (ret) - ret = errno; - - return ret; -} - -/* init_userptr - * Helper that inits userptr an returns handle - * @fd - drm fd - * @userptr - pointer to empty userptr - * @ptr - buffer to be shared - * @size - size of buffer - * @ro - read only flag - * RETURNS handle to shared buffer -*/ -static uint32_t init_userptr(int fd, i915_gem_userptr *userptr, void *ptr, -uint64_t size) -{ - int ret; - - memset((void*)userptr, 0, sizeof(i915_gem_userptr)); - - userptr->user_ptr = (unsigned long)ptr; /* Need the cast to overcome compiler warning */ - userptr->user_size = size; - userptr->flags = 0; /* use synchronized operation */ - - ret = gem_call_userptr_ioctl(fd, userptr); - igt_assert_eq(ret, 0); - - return userptr->handle; -} - -/* create_mem_buffer - * Creates a 4K aligned CPU buffer - * @size - size of buffer - * RETURNS pointer to buffer of @size -*/ -static void *create_mem_buffer(uint64_t size) -{ - void *addr; - int ret; - - ret = posix_memalign(&addr, 4096, size); - igt_assert(ret == 0); - - return addr; -} - -/* setup_exec_obj - * populate exec object - * @exec - exec object - * @handle - handle to gem buffer - * @flags - any flags - * @offset - requested VMA -*/ -static void setup_exec_obj(struct drm_i915_gem_exec_object2 *exec, - uint32_t handle, uint32_t flags, - uint64_t offset) -{ - memset(exec, 0, sizeof(struct drm_i915_gem_exec_object2)); - exec->handle = handle; - exec->flags = flags; - exec->offset = offset; -} - /* gen8_canonical_ad
Re: [Intel-gfx] [PATCH] drm/i915/dp: fall back to 18 bpp when sink capability is unknown
On Wed, 13 Jan 2016, Ville Syrjälä wrote: > On Wed, Jan 13, 2016 at 04:35:20PM +0200, Jani Nikula wrote: >> Per DP spec, the source device should fall back to 18 bpp, VESA range >> RGB when the sink capability is unknown. Fix the color depth >> clamping. 18 bpp color depth should ensure full color range in automatic >> mode. >> >> The clamping has been HDMI specific since its introduction in >> >> commit 996a2239f93b03c5972923f04b097f65565c5bed >> Author: Daniel Vetter >> Date: Fri Apr 19 11:24:34 2013 +0200 >> >> drm/i915: Disable high-bpc on pre-1.4 EDID screens >> >> Cc: sta...@vger.kernel.org >> Reported-by: Dihan Wickremasuriya >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105331 >> Signed-off-by: Jani Nikula > > Makes sense to me as far as the spec is concerned. > Reviewed-by: Ville Syrjälä Also got Tested-by in the bug. Pushed to drm-intel-next-queued, thanks for the review. BR, Jani. > >> --- >> drivers/gpu/drm/i915/intel_display.c | 20 +++- >> 1 file changed, 15 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 07ca19b0ec17..6eaecd9385ab 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12171,11 +12171,21 @@ connected_sink_compute_bpp(struct intel_connector >> *connector, >> pipe_config->pipe_bpp = connector->base.display_info.bpc*3; >> } >> >> -/* Clamp bpp to 8 on screens without EDID 1.4 */ >> -if (connector->base.display_info.bpc == 0 && bpp > 24) { >> -DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit >> of 24\n", >> - bpp); >> -pipe_config->pipe_bpp = 24; >> +/* Clamp bpp to default limit on screens without EDID 1.4 */ >> +if (connector->base.display_info.bpc == 0) { >> +int type = connector->base.connector_type; >> +int clamp_bpp = 24; >> + >> +/* Fall back to 18 bpp when DP sink capability is unknown. */ >> +if (type == DRM_MODE_CONNECTOR_DisplayPort || >> +type == DRM_MODE_CONNECTOR_eDP) >> +clamp_bpp = 18; >> + >> +if (bpp > clamp_bpp) { >> +DRM_DEBUG_KMS("clamping display bpp (was %d) to default >> limit of %d\n", >> + bpp, clamp_bpp); >> +pipe_config->pipe_bpp = clamp_bpp; >> +} >> } >> } >> >> -- >> 2.1.4 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bios: Fix the sequence size calculations for MIPI seq v3
On Thu, 14 Jan 2016, Ville Syrjälä wrote: > On Thu, Jan 14, 2016 at 05:12:07PM +0200, Jani Nikula wrote: >> Two errors in a single line. The size was read from the wrong offset, >> and the end index didn't take the five bytes for sequence byte and size >> of sequence into account. Fix it all, and break up the calculations a >> bit to make it clearer. >> >> Cc: Ville Syrjälä >> Reported-by: Mika Kahola >> Fixes: 2a33d93486f2 ("drm/i915/bios: add support for MIPI sequence block v3") >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/i915/intel_bios.c | 17 ++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c >> b/drivers/gpu/drm/i915/intel_bios.c >> index 12e2f8b8bf9c..bf62a19c8f69 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -842,6 +842,7 @@ static int goto_next_sequence_v3(const u8 *data, int >> index, int total) >> { >> int seq_end; >> u16 len; >> +u32 size_of_sequence; >> >> /* >> * Could skip sequence based on Size of Sequence alone, but also do some >> @@ -852,14 +853,24 @@ static int goto_next_sequence_v3(const u8 *data, int >> index, int total) >> return 0; >> } >> >> -seq_end = index + *((const u32 *)(data + 1)); >> +/* Skip Sequence Byte. */ >> +index++; >> + >> +/* >> + * Size of Sequence. Excludes the Sequence Byte and the size itself, >> + * includes MIPI_SEQ_ELEM_END byte, excludes the final MIPI_SEQ_END >> + * byte. >> + */ >> +size_of_sequence = *((const uint32_t *)(data + index)); > > Hmm. So it was reading from 'data+1' and now it's basically 'data+index+1'. > So it was correct for the first sequence, and busted for later ones I > suppose. > >> +index += 4; >> + >> +seq_end = index + size_of_sequence; > > And now we count the size of the sequence starting from the operation > byte, before we counted it from the sequence byte. "Fortunately" the spec > doesn't even tell us which is correct. If it works, it works. > > Reviewed-by: Ville Syrjälä Pushed to drm-intel-next-queued, thanks for the review and testing. BR, Jani. > > BTW I was thinking that we could maybe add some kind of > "read the thing at index, and and increment the index past it" helpers. > > Eg. > int get_u8(const void *data, int index, int size, u8 *ret); > int get_u32(const void *data, int index, int size, u32 *ret); > > they could also do the index vs. size check and just return an error if > we try to go too far. > >> if (seq_end > total) { >> DRM_ERROR("Invalid sequence size\n"); >> return 0; >> } >> >> -/* Skip Sequence Byte and Size of Sequence. */ >> -for (index = index + 5; index < total; index += len) { >> +for (; index < total; index += len) { >> u8 operation_byte = *(data + index); >> index++; >> >> -- >> 2.1.4 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make sure DC writes are coherent on flush.
On Thu, 14 Jan 2016, Francisco Jerez wrote: > Jani Nikula writes: > >> On Thu, 14 Jan 2016, Francisco Jerez wrote: >>> We need to set the DC FLUSH PIPE_CONTROL bit on Gen7+ to guarantee >>> that writes performed via the HDC are visible in memory. Fixes an >>> intermittent failure in a Piglit test that writes to a BO from a >>> shader using GL atomic counters (implemented as HDC untyped atomics) >>> and then expects the memory to read back the same value after mapping >>> it on the CPU. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91298 >>> Tested-by: Mark Janes >>> Cc: Rodrigo Vivi >> >> Francisco, this is missing your Signed-off-by i.e. developer certificate >> of origin http://developercertificate.org/ - can't push without. Please >> reply with that. >> > Oops, sorry for that -- And yeah it shouldn't hurt to CC stable too. > > Signed-off-by: Francisco Jerez Pushed to drm-intel-next-queued, thanks for the patch and review. BR, Jani. > >> BR, >> Jani. >> >> >>> --- >>> drivers/gpu/drm/i915/intel_lrc.c| 1 + >>> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ >>> 2 files changed, 3 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c >>> b/drivers/gpu/drm/i915/intel_lrc.c >>> index ab344e0..02213c6 100644 >>> --- a/drivers/gpu/drm/i915/intel_lrc.c >>> +++ b/drivers/gpu/drm/i915/intel_lrc.c >>> @@ -1735,6 +1735,7 @@ static int gen8_emit_flush_render(struct >>> drm_i915_gem_request *request, >>> if (flush_domains) { >>> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; >>> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; >>> + flags |= PIPE_CONTROL_DC_FLUSH_ENABLE; >>> flags |= PIPE_CONTROL_FLUSH_ENABLE; >>> } >>> >>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c >>> b/drivers/gpu/drm/i915/intel_ringbuffer.c >>> index 4060acf..8cd8aab 100644 >>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >>> @@ -331,6 +331,7 @@ gen7_render_ring_flush(struct drm_i915_gem_request *req, >>> if (flush_domains) { >>> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; >>> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; >>> + flags |= PIPE_CONTROL_DC_FLUSH_ENABLE; >>> flags |= PIPE_CONTROL_FLUSH_ENABLE; >>> } >>> if (invalidate_domains) { >>> @@ -403,6 +404,7 @@ gen8_render_ring_flush(struct drm_i915_gem_request *req, >>> if (flush_domains) { >>> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; >>> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; >>> + flags |= PIPE_CONTROL_DC_FLUSH_ENABLE; >>> flags |= PIPE_CONTROL_FLUSH_ENABLE; >>> } >>> if (invalidate_domains) { >> >> -- >> Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/11] drm/i915: Add support for mapping an object page by page
On 14/01/16 13:34, Chris Wilson wrote: On Thu, Jan 14, 2016 at 10:32:11AM +, Tvrtko Ursulin wrote: diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index b448ad8..5f86596 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -317,6 +317,11 @@ struct i915_address_space { uint64_t start, uint64_t length, bool use_scratch); + void (*insert_page)(struct i915_address_space *vm, + dma_addr_t addr, + uint64_t offset, + enum i915_cache_level cache_level, + u32 flags); void (*insert_entries)(struct i915_address_space *vm, struct sg_table *st, uint64_t start, Why not extend the current API to support start page offset and number of pages? Could default to full object like today if zero. Eg: void (*insert_entries)(struct i915_address_space *vm, struct sg_table *st, + unsigned page_offset, + unsigned num_pages, Ouch. That would be quite slow for the insert_page() use case of page-by-page looping. It could use the same last page caching trick so why would be be so slow? uint64_t start, enum i915_cache_level cache_level, u32 flags); That way we would not have two functions for effectively the same thing operating on different type of input parameters. If extending insert_entries is not preferable, then still we could add another compatible one, like insert_entries_range or something, and then both could share the same underlying implementation for less code. Like this, this patch already does not match current codebase - see assert_rpm_atomic_begin in insert_entries. Also if API between the two was compatible there would be no need for i915_gem_object_get_dma_address() and i915_gem_object_get_page() could be used instead. The point was to write a lowlevel analog to provide a complementary API to insert_entries that could be used for everywhere the we wanted to peek through the GTT without even touching an object, i.e. for cases where we might allocate a scratch page and temporarily put it into the GTT. Well maybe it is not that bad on the API front, but code duplication would still not be my first choice, as demonstrated by the divergence which already happened. Need to think about it some more. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915: Make display gtt offsets u32
On Thu, Jan 14, 2016 at 10:36:53PM +, Chris Wilson wrote: > On Thu, Jan 14, 2016 at 03:22:15PM +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Using 'unsigned long' for ggtt offsets doesn't make much sense. Use > > 'u32' instead since we've not yet seen a >4GiB ggtt. > > We should do: > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 2e460b369e82..2ce16acceb75 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev) > if (ret) > return ret; > > + if (gtt->base.total >> 32) { > + DRM_ERROR("The code does not expect a Global GTT with more > that 32bits of address space! Found %lldM!\n", > + gtt->base.total >> 20); > + gtt->base.total = 1ull << 32; > + gtt->mappable_end = min(gtt->mappable_end, gtt->base.total); > + } > + > /* GMADR is the PCI mmio aperture into the global GTT. */ > DRM_INFO("Memory usable by graphics device = %lluM\n", > gtt->base.total >> 20); > > To get that early warning in. Seems reasonable. Feel free to slap on Reviewed-by: Ville Syrjälä > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Splitting intel_dp_detect
On Thu, 2016-01-14 at 19:20 +0530, Shubhangi Shrivastava wrote: > > On Wednesday 13 January 2016 07:03 PM, Ander Conselvan De Oliveira wrote: > > On Wed, 2016-01-13 at 13:20 +0200, Ander Conselvan De Oliveira wrote: > > > On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote: > > > > intel_dp_detect() is called for not just detection but > > > > during modes enumeration as well. Repeating the whole > > > > sequence during each of these calls is wasteful and > > > > time consuming. > > > > This patch moves probing for panel, DPCD read etc done in > > > > intel_dp_detect() to a new function intel_dp_long_pulse(). > > > > Note that the behavior of intel_dp_detect() is changed to > > > > report connected or disconnected depending on whether the > > > > EDID is available or not. > > > > This change will be required by further patches in the series > > > > to avoid performing duplicated DPCD operations on hotplug. > > > > > > > > v2: Moved a hunk to next patch of the series. > > > > Moved intel_dp_unset_edid to out. (Ander) > > > > v3: Rephrased commit message and intel_dp_unset_dp() is called > > > > within intel_dp_set_dp() to free the previous EDID. (Ander) > > > > > > > > Tested-by: Nathan D Ciobanu > > > > Signed-off-by: Sivakumar Thulasimani > > > > Signed-off-by: Shubhangi Shrivastava > > > > --- > > > > drivers/gpu/drm/i915/intel_dp.c | 56 + > > > > - > > > > -- > > > > - > > > > 1 file changed, 35 insertions(+), 21 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > > b/drivers/gpu/drm/i915/intel_dp.c > > > > index 796e3d3..e3b4208 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -129,6 +129,7 @@ static void edp_panel_vdd_off(struct intel_dp > > > > *intel_dp, > > > > bool sync); > > > > static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp); > > > > static void vlv_steal_power_sequencer(struct drm_device *dev, > > > > enum pipe pipe); > > > > +static void intel_dp_unset_edid(struct intel_dp *intel_dp); > > > > > > > > static unsigned int intel_dp_unused_lane_mask(int lane_count) > > > > { > > > > @@ -4587,6 +4588,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp) > > > > struct intel_connector *intel_connector = intel_dp > > > > ->attached_connector; > > > > struct edid *edid; > > > > > > > > + intel_dp_unset_edid(intel_dp); > > > > edid = intel_dp_get_edid(intel_dp); > > > > intel_connector->detect_edid = edid; > > > > > > > > @@ -4607,9 +4609,10 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) > > > > intel_dp->has_audio = false; > > > > } > > > > > > > > -static enum drm_connector_status > > > > -intel_dp_detect(struct drm_connector *connector, bool force) > > > > +static void > > > > +intel_dp_long_pulse(struct intel_connector *intel_connector) > > > > { > > > > + struct drm_connector *connector = &intel_connector->base; > > > > struct intel_dp *intel_dp = intel_attached_dp(connector); > > > > struct intel_digital_port *intel_dig_port = > > > > dp_to_dig_port(intel_dp); > > > > struct intel_encoder *intel_encoder = &intel_dig_port->base; > > > > @@ -4619,17 +4622,6 @@ intel_dp_detect(struct drm_connector *connector, > > > > bool > > > > force) > > > > bool ret; > > > > u8 sink_irq_vector; > > > > > > > > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > > > > - connector->base.id, connector->name); > > > > - intel_dp_unset_edid(intel_dp); > > > > - > > > > - if (intel_dp->is_mst) { > > > > - /* MST devices are disconnected from a monitor POV */ > > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > > - return connector_status_disconnected; > > > > - } > > > > - > > > > power_domain = > > > > intel_display_port_aux_power_domain(intel_encoder); > > > > intel_display_power_get(to_i915(dev), power_domain); > > > > > > > > @@ -4653,14 +4645,8 @@ intel_dp_detect(struct drm_connector *connector, > > > > bool > > > > force) > > > > intel_dp_probe_oui(intel_dp); > > > > > > > > ret = intel_dp_probe_mst(intel_dp); > > > > - if (ret) { > > > > - /* if we are in MST mode then this connector > > > > - won't appear connected or have anything with EDID on > > > > it > > > > */ > > > > - if (intel_encoder->type != INTEL_OUTPUT_EDP) > > > > - intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT; > > > This deletion is new in this version of the patch. I think we still need > > > the > > > hunk above, otherwise we might not properly update the encoder type when > > > we > > > switch from an HDMI sink connected through a level shifte
[Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
Throughout the code base, we use u32 for offsets into the global GTT. If we ever see any hardware with a larger GGTT, then we run the real risk of silent corruption. So test for our assumption up front so that we have a nice reminder should the time come when it fails. Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 2e460b369e82..0d910638972c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev) if (ret) return ret; + if ((gtt->base.total - 1) >> 32) { + DRM_ERROR("We never expected a Global GTT with more than 32bits of address space! Found %lldM!\n", + gtt->base.total >> 20); + gtt->base.total = 1ull << 32; + gtt->mappable_end = min(gtt->mappable_end, gtt->base.total); + } + /* GMADR is the PCI mmio aperture into the global GTT. */ DRM_INFO("Memory usable by graphics device = %lluM\n", gtt->base.total >> 20); -- 2.7.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Decouple execbuf uAPI from internal implementation
On 14/01/16 22:09, Chris Wilson wrote: On Thu, Jan 14, 2016 at 03:02:59PM +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin At the moment execbuf ring selection is fully coupled to internal ring ids which is not a good thing on its own. This dependency is also spread between two source files and not spelled out at either side which makes it hidden and fragile. This patch decouples this dependency by introducing an explicit translation table of execbuf uAPI to ring id close to the only call site (i915_gem_do_execbuffer). This way we are free to change driver internal implementation details without breaking userspace. All state relating to the uAPI is now contained in, or next to, i915_gem_do_execbuffer. I was hesistant at first, since "why?" isn't fully developed, but the code won me over. +#define I915_USER_RINGS (4) + +static const enum intel_ring_id user_ring_map[I915_USER_RINGS + 1] = { + [I915_EXEC_DEFAULT] = RCS, + [I915_EXEC_RENDER] = RCS, + [I915_EXEC_BLT] = BCS, + [I915_EXEC_BSD] = VCS, + [I915_EXEC_VEBOX] = VECS +}; I was wondering whether packing here mattered at all, but we're under a cacheline so highly unlikely. static int i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct drm_file *file, @@ -1393,6 +1393,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct i915_execbuffer_params params_master; /* XXX: will be removed later */ struct i915_execbuffer_params *params = ¶ms_master; const u32 ctx_id = i915_execbuffer2_get_context_id(*args); + unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; u32 dispatch_flags; int ret; bool need_relocs; @@ -1414,49 +1415,39 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, if (args->flags & I915_EXEC_IS_PINNED) dispatch_flags |= I915_DISPATCH_PINNED; - if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) { - DRM_DEBUG("execbuf with unknown ring: %d\n", - (int)(args->flags & I915_EXEC_RING_MASK)); + if (user_ring_id > I915_USER_RINGS) { + DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); return -EINVAL; } - if (((args->flags & I915_EXEC_RING_MASK) != I915_EXEC_BSD) && + if ((user_ring_id != I915_EXEC_BSD) && ((args->flags & I915_EXEC_BSD_MASK) != 0)) { DRM_DEBUG("execbuf with non bsd ring but with invalid " "bsd dispatch flags: %d\n", (int)(args->flags)); return -EINVAL; - } - - if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT) - ring = &dev_priv->ring[RCS]; - else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) { - if (HAS_BSD2(dev)) { - int ring_id; - - switch (args->flags & I915_EXEC_BSD_MASK) { - case I915_EXEC_BSD_DEFAULT: - ring_id = gen8_dispatch_bsd_ring(dev, file); - ring = &dev_priv->ring[ring_id]; - break; - case I915_EXEC_BSD_RING1: - ring = &dev_priv->ring[VCS]; - break; - case I915_EXEC_BSD_RING2: - ring = &dev_priv->ring[VCS2]; - break; - default: - DRM_DEBUG("execbuf with unknown bsd ring: %d\n", - (int)(args->flags & I915_EXEC_BSD_MASK)); - return -EINVAL; - } - } else - ring = &dev_priv->ring[VCS]; - } else - ring = &dev_priv->ring[(args->flags & I915_EXEC_RING_MASK) - 1]; + } + + if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev)) { HAS_BSD2(dev_priv) + unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK; + + if (bsd_idx == I915_EXEC_BSD_DEFAULT) { + bsd_idx = gen8_dispatch_bsd_ring(dev_priv, file); + } else if (bsd_idx >= I915_EXEC_BSD_RING1 && + bsd_idx <= I915_EXEC_BSD_RING2) { + bsd_idx--; + } else { + DRM_DEBUG("execbuf with unknown bsd ring: %u\n", + bsd_idx); + return -EINVAL; + } + + ring = &dev_priv->ring[_VCS(bsd_idx)]; + } else { + ring = &dev_priv->ring[user_ring_map[user_ring_id]]; + } if (!intel_ring_initialized(ring)) { - DRM_DEBUG("execbuf with invalid ring: %d\n", - (int)(args->flags & I915_EXEC_RING_MASK)); +
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Codify our assumption that the Global GTT is <= 4GiB
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (bdw-nuci7) pass -> DMESG-WARN (bdw-ultra) Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) bdw-nuci7total:138 pass:129 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:131 dwarn:1 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:123 dwarn:3 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1194/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10] drm/i915: Extend LRC pinning to cover GPU context writeback
On 14/01/2016 12:37, Nick Hoath wrote: On 14/01/2016 12:31, Chris Wilson wrote: On Thu, Jan 14, 2016 at 11:56:07AM +, Nick Hoath wrote: On 14/01/2016 11:36, Chris Wilson wrote: On Wed, Jan 13, 2016 at 04:19:45PM +, Nick Hoath wrote: + if (ctx->engine[ring->id].dirty) { + struct drm_i915_gem_request *req = NULL; + + /** +* If there is already a request pending on +* this ring, wait for that to complete, +* otherwise create a switch to idle request +*/ + if (list_empty(&ring->request_list)) { + int ret; + + ret = i915_gem_request_alloc( + ring, + ring->default_context, + &req); + if (!ret) + i915_add_request(req); + else + DRM_DEBUG("Failed to ensure context saved"); + } else { + req = list_first_entry( + &ring->request_list, + typeof(*req), list); + } + if (req) { + ret = i915_wait_request(req); + if (ret != 0) { + /** +* If we get here, there's probably been a ring +* reset, so we just clean up the dirty flag.& +* pin count. +*/ + ctx->engine[ring->id].dirty = false; + __intel_lr_context_unpin( + ring, + ctx); + } + } If you were to take a lr_context_pin on the last_context, and only release that pin when you change to a new context, you do not need to That what this patch does. introduce a blocking context-close, nor do you need to introduce the usage of default_context. The use of default_context here is to stop a context hanging around after it is no longer needed. By blocking, which is not acceptable. Also we can eliminate the default_context and so pinning that opposed to the last_context serves no purpose other than by chance having a more preferrable position when it comes to defragmentation. But you don't enable that anyway and we Enabling the shrinker on execlists is something I'm working on which is predicated on this patch. Also why is blocking on closing a context not acceptable? As a clarification: Without rewriting the execlist code to not submit or cleanup from an interrupt handler, we can't use refcounting to allow non blocking closing. have alternative strategies now that avoid the issue with fragmentation of the mappable aperture. (lr_context_pin should take a reference on the ctx to prevent early freeeing ofc). You can't clear the reference on the ctx in an interrupt context. The execlists submission should moved out of the interrupt context, for the very simple reason that it is causing machine panics. userspace submits a workload, machine lockups Create a jira, and I'm sure we'll look at making that change. -Chris ___ 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] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
Tvrtko was looking through the execbuffer-ioctl and noticed that the uABI was tightly coupled to our internal engine identifiers. Close inspection also revealed that we leak those internal engine identifiers through the busy-ioctl, and those internal identifiers already do not match the user identifiers. Fortuitiously, there is only one user of the set of busy rings from the busy-ioctl, and they only wish to choose between the RENDER and the BLT engines. Let's fix the userspace ABI while we still can. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 18 ++ drivers/gpu/drm/i915/intel_lrc.c| 5 + drivers/gpu/drm/i915/intel_ringbuffer.c | 5 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bb44bad15403..85797813a3de 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, if (ret) goto unref; - BUILD_BUG_ON(I915_NUM_RINGS > 16); - args->busy = obj->active << 16; - if (obj->last_write_req) - args->busy |= obj->last_write_req->ring->id; + args->busy = 0; + if (obj->active) { + int i; + + for (i = 0; i < I915_NUM_RINGS; i++) { + struct drm_i915_gem_request *req; + + req = obj->last_read_req[i]; + if (req) + args->busy |= 1 << (16 + req->ring->exec_id); + } + if (obj->last_write_req) + args->busy |= obj->last_write_req->ring->exec_id; + } unref: drm_gem_object_unreference(&obj->base); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f5d89c845ede..4aa209483237 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device *dev) ring->name = "render ring"; ring->id = RCS; + ring->exec_id = I915_EXEC_RENDER; ring->mmio_base = RENDER_RING_BASE; logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT); @@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev) ring->name = "bsd ring"; ring->id = VCS; + ring->exec_id = I915_EXEC_BSD; ring->mmio_base = GEN6_BSD_RING_BASE; logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT); @@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev) ring->name = "bsd2 ring"; ring->id = VCS2; + ring->exec_id = I915_EXEC_BSD; ring->mmio_base = GEN8_BSD2_RING_BASE; logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT); @@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev) ring->name = "blitter ring"; ring->id = BCS; + ring->exec_id = I915_EXEC_BLT; ring->mmio_base = BLT_RING_BASE; logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT); @@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device *dev) ring->name = "video enhancement ring"; ring->id = VECS; + ring->exec_id = I915_EXEC_VEBOX; ring->mmio_base = VEBOX_RING_BASE; logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8cd8aabcc3ff..310d151c0db2 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring->name = "render ring"; ring->id = RCS; + ring->exec_id = I915_EXEC_RENDER; ring->mmio_base = RENDER_RING_BASE; if (INTEL_INFO(dev)->gen >= 8) { @@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) ring->name = "bsd ring"; ring->id = VCS; + ring->exec_id = I915_EXEC_BSD; ring->write_tail = ring_write_tail; if (INTEL_INFO(dev)->gen >= 6) { @@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev) ring->name = "bsd2 ring"; ring->id = VCS2; + ring->exec_id = I915_EXEC_BSD; ring->write_tail = ring_write_tail; ring->mmio_base = GEN8_BSD2_RING_BASE; @@ -2934,6 +2937,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) ring->name = "blitter ring"; ring->id = BCS; + ring->exec_id = I915_EXEC_BLT; ring->mmio_base = BLT_RING_BASE; ring->write_tail = ring_write_tail; @@ -2991,6 +2995,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev) ring->name = "video enhancement ring"; r
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (bdw-nuci7) pass -> DMESG-WARN (skl-i7k-2) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: pass -> DMESG-WARN (byt-nuc) bdw-nuci7total:138 pass:129 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:132 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1195/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
On 15/01/16 10:20, Chris Wilson wrote: Throughout the code base, we use u32 for offsets into the global GTT. If we ever see any hardware with a larger GGTT, then we run the real risk of silent corruption. So test for our assumption up front so that we have a nice reminder should the time come when it fails. Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 2e460b369e82..0d910638972c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev) if (ret) return ret; + if ((gtt->base.total - 1) >> 32) { + DRM_ERROR("We never expected a Global GTT with more than 32bits of address space! Found %lldM!\n", + gtt->base.total >> 20); + gtt->base.total = 1ull << 32; + gtt->mappable_end = min(gtt->mappable_end, gtt->base.total); Assuming Mika's comment on 'struct i915_address_space' is correct: ... u64 start; /* Start offset always 0 for dri2 */ ... otherwise this calculation would need to be adjusted. + /* GMADR is the PCI mmio aperture into the global GTT. */ DRM_INFO("Memory usable by graphics device = %lluM\n", gtt->base.total >> 20); LGTM. Reviewed-by: Dave Gordon ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
On Fri, Jan 15, 2016 at 10:20:11AM +, Chris Wilson wrote: > Throughout the code base, we use u32 for offsets into the global GTT. If > we ever see any hardware with a larger GGTT, then we run the real risk > of silent corruption. So test for our assumption up front so that we > have a nice reminder should the time come when it fails. > > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > Cc: Daniel Vetter From the earlier cut-n-paste, Reviewed-by: Ville Syrjälä (so I hope it still holds with the minor correction applied :) -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] drm/i915: Decouple execbuf uAPI from internal implementation
On Fri, Jan 15, 2016 at 10:29:05AM +, Tvrtko Ursulin wrote: > >One question, how does this look if we move this section to its own > >function: > > > >ring = eb_select_ring(dev_priv, file, args); > >if (IS_ERR(ring)) > > return PTR_ERR(ring); > > > >Do you keep your code size reduction? > > No, perhaps because of all the ERR_PTR business. But decoupling the > ring and return value keeps it: > > static int > eb_select_ring(struct drm_i915_private *dev_priv, > struct drm_file *file, > struct drm_i915_gem_execbuffer2 *args, > struct intel_engine_cs **ring) Hmm, I wonder why. ERR_PTR(ret) should be a no-op, so the only thing that I would have thought changed would be the test afterwards. I guess a foray into the assembly would be a good learning experience for me then! > Saves ~100 bytes out of ~4600 on my build. > > >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > >>b/drivers/gpu/drm/i915/intel_ringbuffer.h > >>index 7349d9258191..fdc220fc2b18 100644 > >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h > >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > >>@@ -148,14 +148,14 @@ struct i915_ctx_workarounds { > >> struct intel_engine_cs { > >>const char *name; > >>enum intel_ring_id { > >>- RCS = 0x0, > >>- VCS, > >>+ RCS = 0, > >>BCS, > >>- VECS, > >>- VCS2 > >>+ VCS, > >>+ VCS2, /* Keep instances of the same type engine together. */ > >>+ VECS > > > >Technically, this breaks userspace ABI elsewhere. :| Though the only > >user of the id mask is only looking for RCS==0 vs the reset. > > > >So I think we would cope, but to be extra safe we could just avoid > >reshuffling the ids. Let me sleep on the implications. We may say just > >break that ABI whilst we still can and do a reverse-map to EXEC bit. > >Hmm. > > What are the other places it breaks the ABI? I'd like to understand it. The busy-ioctl is strong abi, i915_trace.h has a few weak abi uses of ring->id, and the use in debugfs is no abi barrier at all. ring->id also crops up in the guc_context_desc, which is intriguing. -Cris -- 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 01/11] drm/i915: Add support for mapping an object page by page
On Fri, Jan 15, 2016 at 09:53:46AM +, Tvrtko Ursulin wrote: > > On 14/01/16 13:34, Chris Wilson wrote: > >On Thu, Jan 14, 2016 at 10:32:11AM +, Tvrtko Ursulin wrote: > >>>diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h > >>>b/drivers/gpu/drm/i915/i915_gem_gtt.h > >>>index b448ad8..5f86596 100644 > >>>--- a/drivers/gpu/drm/i915/i915_gem_gtt.h > >>>+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > >>>@@ -317,6 +317,11 @@ struct i915_address_space { > >>> uint64_t start, > >>> uint64_t length, > >>> bool use_scratch); > >>>+ void (*insert_page)(struct i915_address_space *vm, > >>>+ dma_addr_t addr, > >>>+ uint64_t offset, > >>>+ enum i915_cache_level cache_level, > >>>+ u32 flags); > >>> void (*insert_entries)(struct i915_address_space *vm, > >>> struct sg_table *st, > >>> uint64_t start, > >> > >>Why not extend the current API to support start page offset and > >>number of pages? Could default to full object like today if zero. > >>Eg: > >> > >> void (*insert_entries)(struct i915_address_space *vm, > >>struct sg_table *st, > >>+ unsigned page_offset, > >>+ unsigned num_pages, > > > >Ouch. That would be quite slow for the insert_page() use case of > >page-by-page looping. > > It could use the same last page caching trick so why would be be so slow? We don't have the convenient struct? I presumed you were thinking of passing the offset through to sg_page_iter for_each_sg_page(st->sgl, &sg_iter, st->nents, page_offset) { if (--num_pages) break; /* original pt insertion code */ } Is it worth expanding struct sg_table for this shortcut? There are some games where we spend more time in sg_page_iter_next() than anything else in the kernel. The games have more textures than GTT space, they aren't playable right now nor will they ever likely to be. :( Improving these loops would definitely be a boon nevertheless. -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] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
On Fri, Jan 15, 2016 at 11:25:38AM +, Dave Gordon wrote: > On 15/01/16 10:20, Chris Wilson wrote: > >Throughout the code base, we use u32 for offsets into the global GTT. If > >we ever see any hardware with a larger GGTT, then we run the real risk > >of silent corruption. So test for our assumption up front so that we > >have a nice reminder should the time come when it fails. > > > >Signed-off-by: Chris Wilson > >Cc: Ville Syrjälä > >Cc: Daniel Vetter > >--- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ > > 1 file changed, 7 insertions(+) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > >b/drivers/gpu/drm/i915/i915_gem_gtt.c > >index 2e460b369e82..0d910638972c 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >@@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev) > > if (ret) > > return ret; > > > >+if ((gtt->base.total - 1) >> 32) { > >+DRM_ERROR("We never expected a Global GTT with more than 32bits > >of address space! Found %lldM!\n", > >+ gtt->base.total >> 20); > >+gtt->base.total = 1ull << 32; > >+gtt->mappable_end = min(gtt->mappable_end, gtt->base.total); > > Assuming Mika's comment on 'struct i915_address_space' is correct: > ... > u64 start; /* Start offset always 0 for dri2 */ > ... > otherwise this calculation would need to be adjusted. The address_space start was for the obsolete UMS call where the xserver would tell the kernel the range of the (then only global) GTT to use for itself. The vgpu plugin comes later? Though if I remember correctly, it reserves ranges of the GGTT for itself rather than alter the drm_mm. So I don't think it has been revitalised since. Removing the start value from the struct i915_address_space confirms it is obsolete. -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] drm/i915: Remove obsolete starting offset from struct i915_address_space
Since the removal of UMS, we have always had sole ownership of the GTTs. We no longer have to track the subsection we are allowed to use (denoted by start + total). vGPU does restrict the range of the global GTT we can use (as it has to share it amongst all the clients on the host), but that is achieved by ballooning reserved node within the whole (so that it could adjust available space dynamically). As such i915_address_space.start is always 0 and we can remove having to worry about such complications. Signed-off-by: Chris Wilson Cc: Dave Gordon Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 24 drivers/gpu/drm/i915/i915_gem_gtt.h | 1 - drivers/gpu/drm/i915/i915_vgpu.c| 17 +++-- 4 files changed, 16 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e3377abc0d4d..b1bd710f645a 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -495,7 +495,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data) seq_printf(m, "%llu [%llu] gtt total\n", dev_priv->gtt.base.total, - (u64)dev_priv->gtt.mappable_end - dev_priv->gtt.base.start); + dev_priv->gtt.mappable_end); seq_putc(m, '\n'); print_batch_pool_stats(m, dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 56f4f2e58d53..11080f00bda1 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1443,7 +1443,7 @@ static void gen8_dump_pdp(struct i915_page_directory_pointer *pdp, static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m) { struct i915_address_space *vm = &ppgtt->base; - uint64_t start = ppgtt->base.start; + uint64_t start = 0; uint64_t length = ppgtt->base.total; gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page), I915_CACHE_LLC, true); @@ -1507,7 +1507,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt) if (ret) return ret; - ppgtt->base.start = 0; ppgtt->base.cleanup = gen8_ppgtt_cleanup; ppgtt->base.allocate_va_range = gen8_alloc_va_range; ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; @@ -1560,7 +1559,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m) gen6_pte_t scratch_pte; uint32_t pd_entry; uint32_t pte, pde, temp; - uint32_t start = ppgtt->base.start, length = ppgtt->base.total; + uint32_t start = 0, length = ppgtt->base.total; scratch_pte = vm->pte_encode(px_dma(vm->scratch_page), I915_CACHE_LLC, true, 0); @@ -2086,7 +2085,6 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) ppgtt->base.unbind_vma = ppgtt_unbind_vma; ppgtt->base.bind_vma = ppgtt_bind_vma; ppgtt->base.cleanup = gen6_ppgtt_cleanup; - ppgtt->base.start = 0; ppgtt->base.total = I915_PDES * GEN6_PTES * PAGE_SIZE; ppgtt->debug_dump = gen6_dump_ppgtt; @@ -2123,7 +2121,7 @@ static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) static void i915_address_space_init(struct i915_address_space *vm, struct drm_i915_private *dev_priv) { - drm_mm_init(&vm->mm, vm->start, vm->total); + drm_mm_init(&vm->mm, 0, vm->total); vm->dev = dev_priv->dev; INIT_LIST_HEAD(&vm->active_list); INIT_LIST_HEAD(&vm->inactive_list); @@ -2312,8 +2310,7 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev) i915_check_and_clear_faults(dev); dev_priv->gtt.base.clear_range(&dev_priv->gtt.base, - dev_priv->gtt.base.start, - dev_priv->gtt.base.total, + 0, dev_priv->gtt.base.total, true); i915_ggtt_flush(dev_priv); @@ -2681,7 +2678,6 @@ static void i915_gtt_color_adjust(struct drm_mm_node *node, } static int i915_gem_setup_global_gtt(struct drm_device *dev, -u64 start, u64 mappable_end, u64 end) { @@ -2703,11 +2699,9 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev, BUG_ON(mappable_end > end); - ggtt_vm->start = start; - /* Subtract the guard page before address space initialization to * shrink the range used by drm_mm */ - ggtt_vm->total = end - start - PAGE_SIZE; + ggtt_vm->total = end - PAGE_SIZE; i915_address_space_init(ggtt_vm, dev_priv); ggtt_vm->total += PAGE_SIZE; @@ -2773,8 +2767,
Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On 15/01/16 11:06, Chris Wilson wrote: Tvrtko was looking through the execbuffer-ioctl and noticed that the uABI was tightly coupled to our internal engine identifiers. Close inspection also revealed that we leak those internal engine identifiers through the busy-ioctl, and those internal identifiers already do not match the user identifiers. Fortuitiously, there is only one user of the set of busy rings from the busy-ioctl, and they only wish to choose between the RENDER and the BLT engines. Let's fix the userspace ABI while we still can. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 18 ++ drivers/gpu/drm/i915/intel_lrc.c| 5 + drivers/gpu/drm/i915/intel_ringbuffer.c | 5 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bb44bad15403..85797813a3de 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, if (ret) goto unref; - BUILD_BUG_ON(I915_NUM_RINGS > 16); - args->busy = obj->active << 16; - if (obj->last_write_req) - args->busy |= obj->last_write_req->ring->id; + args->busy = 0; + if (obj->active) { + int i; + + for (i = 0; i < I915_NUM_RINGS; i++) { + struct drm_i915_gem_request *req; + + req = obj->last_read_req[i]; + if (req) + args->busy |= 1 << (16 + req->ring->exec_id); If I got it right bit 16 was RCS, now will always be clear. And blitter was bit 17 and now is 19. Regards, Tvrtko + } + if (obj->last_write_req) + args->busy |= obj->last_write_req->ring->exec_id; + } unref: drm_gem_object_unreference(&obj->base); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f5d89c845ede..4aa209483237 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device *dev) ring->name = "render ring"; ring->id = RCS; + ring->exec_id = I915_EXEC_RENDER; ring->mmio_base = RENDER_RING_BASE; logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT); @@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev) ring->name = "bsd ring"; ring->id = VCS; + ring->exec_id = I915_EXEC_BSD; ring->mmio_base = GEN6_BSD_RING_BASE; logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT); @@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev) ring->name = "bsd2 ring"; ring->id = VCS2; + ring->exec_id = I915_EXEC_BSD; ring->mmio_base = GEN8_BSD2_RING_BASE; logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT); @@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev) ring->name = "blitter ring"; ring->id = BCS; + ring->exec_id = I915_EXEC_BLT; ring->mmio_base = BLT_RING_BASE; logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT); @@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device *dev) ring->name = "video enhancement ring"; ring->id = VECS; + ring->exec_id = I915_EXEC_VEBOX; ring->mmio_base = VEBOX_RING_BASE; logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8cd8aabcc3ff..310d151c0db2 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring->name = "render ring"; ring->id = RCS; + ring->exec_id = I915_EXEC_RENDER; ring->mmio_base = RENDER_RING_BASE; if (INTEL_INFO(dev)->gen >= 8) { @@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) ring->name = "bsd ring"; ring->id = VCS; + ring->exec_id = I915_EXEC_BSD; ring->write_tail = ring_write_tail; if (INTEL_INFO(dev)->gen >= 6) { @@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev) ring->name = "bsd2 ring"; ring->id = VCS2; + ring->exec_id = I915_EXEC_BSD; ring->write_tail = ring_write_tail; ring->mmio_base = GEN8_BSD2_RING_BASE; @@ -2934,6 +2937,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) ring->name = "blitter ring"; ring->id = BCS; + ring->exec_id = I915_EXEC_BLT; ring->mmio_base = BLT_RING_BASE; ring->write_tail = ring
Re: [Intel-gfx] ✗ failure: Fi.CI.BAT
On Thu, Jan 14, 2016 at 05:00:02PM +0200, Ville Syrjälä wrote: > On Thu, Jan 14, 2016 at 02:49:45PM -, Patchwork wrote: > > == Summary == > > > > Built on 8fb2feecca499d11e104264071ac55e273e23af5 drm-intel-nightly: > > 2016y-01m-14d-13h-06m-44s UTC integration manifest > > > > Test gem_ctx_basic: > > pass -> FAIL (bdw-ultra) > > "Returncode -15" and nothing more. Weird. Strikes me as a bug in the testrunner. That's not one of ours. -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 079/190] drm/i915: Reduce the pointer dance of i915_is_ggtt()
On 11/01/16 09:17, Chris Wilson wrote: The multiple levels of indirect do nothing but hinder the compiler and the pointer chasing turns to be quite painful but painless to fix. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_debugfs.c| 13 ++--- drivers/gpu/drm/i915/i915_drv.h| 7 --- drivers/gpu/drm/i915/i915_gem.c| 18 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++--- drivers/gpu/drm/i915/i915_gem_gtt.c| 12 +--- drivers/gpu/drm/i915/i915_gem_gtt.h| 5 + drivers/gpu/drm/i915/i915_trace.h | 27 --- 7 files changed, 33 insertions(+), 54 deletions(-) [snip] diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c9c1a5cdc1e5..f840cc55f1ab 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2905,18 +2905,11 @@ bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj); /* Some GGTT VM helpers */ #define i915_obj_to_ggtt(obj) \ (&((struct drm_i915_private *)(obj)->base.dev->dev_private)->gtt.base) -static inline bool i915_is_ggtt(struct i915_address_space *vm) -{ - struct i915_address_space *ggtt = - &((struct drm_i915_private *)(vm)->dev->dev_private)->gtt.base; - return vm == ggtt; -} diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index b5c3bbe6dc2a..06117bd0fc00 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3150,6 +3150,7 @@ int i915_gem_gtt_init(struct drm_device *dev) } gtt->base.dev = dev; + gtt->base.is_ggtt = true; So, it looks like the plan here is that when we need to determine whether something is the special distinguished instance of a type, then instead of comparing its address against the global pointer to the distinguished instance, we'll just look for a flag /inside/ the object itself, which is set /only/ on the distinguished instance. Now why didn't I think of that? That looks like such a good idea, we should apply it in other CONTEXTs! Reviewed-by: Dave Gordon ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Remove obsolete starting offset from struct i915_address_space
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (skl-i5k-2) UNSTABLE dmesg-warn -> PASS (bdw-nuci7) pass -> DMESG-WARN (skl-i7k-2) UNSTABLE pass -> DMESG-WARN (bdw-ultra) Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) bdw-nuci7total:138 pass:129 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:131 dwarn:1 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:123 dwarn:3 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1196/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 079/190] drm/i915: Reduce the pointer dance of i915_is_ggtt()
On Fri, Jan 15, 2016 at 12:12:15PM +, Dave Gordon wrote: > On 11/01/16 09:17, Chris Wilson wrote: > >The multiple levels of indirect do nothing but hinder the compiler and > >the pointer chasing turns to be quite painful but painless to fix. > > > >Signed-off-by: Chris Wilson > >--- > > drivers/gpu/drm/i915/i915_debugfs.c| 13 ++--- > > drivers/gpu/drm/i915/i915_drv.h| 7 --- > > drivers/gpu/drm/i915/i915_gem.c| 18 +++--- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++--- > > drivers/gpu/drm/i915/i915_gem_gtt.c| 12 +--- > > drivers/gpu/drm/i915/i915_gem_gtt.h| 5 + > > drivers/gpu/drm/i915/i915_trace.h | 27 --- > > 7 files changed, 33 insertions(+), 54 deletions(-) > > [snip] > > >diff --git a/drivers/gpu/drm/i915/i915_drv.h > >b/drivers/gpu/drm/i915/i915_drv.h > >index c9c1a5cdc1e5..f840cc55f1ab 100644 > >--- a/drivers/gpu/drm/i915/i915_drv.h > >+++ b/drivers/gpu/drm/i915/i915_drv.h > >@@ -2905,18 +2905,11 @@ bool i915_gem_obj_is_pinned(struct > >drm_i915_gem_object *obj); > > /* Some GGTT VM helpers */ > > #define i915_obj_to_ggtt(obj) \ > > (&((struct drm_i915_private *)(obj)->base.dev->dev_private)->gtt.base) > >-static inline bool i915_is_ggtt(struct i915_address_space *vm) > >-{ > >-struct i915_address_space *ggtt = > >-&((struct drm_i915_private *)(vm)->dev->dev_private)->gtt.base; > >-return vm == ggtt; > >-} > > >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > >b/drivers/gpu/drm/i915/i915_gem_gtt.c > >index b5c3bbe6dc2a..06117bd0fc00 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >@@ -3150,6 +3150,7 @@ int i915_gem_gtt_init(struct drm_device *dev) > > } > > > > gtt->base.dev = dev; > >+gtt->base.is_ggtt = true; > > So, it looks like the plan here is that when we need to determine > whether something is the special distinguished instance of a type, > then instead of comparing its address against the global pointer to > the distinguished instance, we'll just look for a flag /inside/ the > object itself, which is set /only/ on the distinguished instance. > > Now why didn't I think of that? That looks like such a good idea, we > should apply it in other CONTEXTs! But we already have that flag in contexts! It also happens to be useful for other tracking as well. And we demonstrated that we didn't even need the checks for the kernel context anyway. You will also note this is a small stepping patch after which we transition away from i915_address_space.is_ggtt to using the owner. -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] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On Fri, Jan 15, 2016 at 11:58:32AM +, Tvrtko Ursulin wrote: > > On 15/01/16 11:06, Chris Wilson wrote: > >Tvrtko was looking through the execbuffer-ioctl and noticed that the > >uABI was tightly coupled to our internal engine identifiers. Close > >inspection also revealed that we leak those internal engine identifiers > >through the busy-ioctl, and those internal identifiers already do not > >match the user identifiers. Fortuitiously, there is only one user of the > >set of busy rings from the busy-ioctl, and they only wish to choose > >between the RENDER and the BLT engines. > > > >Let's fix the userspace ABI while we still can. > > > >Signed-off-by: Chris Wilson > >Cc: Tvrtko Ursulin > >Cc: Daniel Vetter > >--- > > drivers/gpu/drm/i915/i915_gem.c | 18 ++ > > drivers/gpu/drm/i915/intel_lrc.c| 5 + > > drivers/gpu/drm/i915/intel_ringbuffer.c | 5 + > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + > > 4 files changed, 25 insertions(+), 4 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem.c > >b/drivers/gpu/drm/i915/i915_gem.c > >index bb44bad15403..85797813a3de 100644 > >--- a/drivers/gpu/drm/i915/i915_gem.c > >+++ b/drivers/gpu/drm/i915/i915_gem.c > >@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void > >*data, > > if (ret) > > goto unref; > > > >-BUILD_BUG_ON(I915_NUM_RINGS > 16); > >-args->busy = obj->active << 16; > >-if (obj->last_write_req) > >-args->busy |= obj->last_write_req->ring->id; > >+args->busy = 0; > >+if (obj->active) { > >+int i; > >+ > >+for (i = 0; i < I915_NUM_RINGS; i++) { > >+struct drm_i915_gem_request *req; > >+ > >+req = obj->last_read_req[i]; > >+if (req) > >+args->busy |= 1 << (16 + req->ring->exec_id); > > If I got it right bit 16 was RCS, now will always be clear. And > blitter was bit 17 and now is 19. Sssh. You weren't meant to point that out. I am willing to take the hit in order to decouple the uABI from internals. I am also willing to codify this busy-ioctl ABI into igt! -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 2/2] lib/igt_kms: Short description between Intel/DRM terminology.
On Fri, Jan 15, 2016 at 11:06:42AM +0200, Marius Vlad wrote: > lib/igt_kms: Briefly describe Intel-to-DRM mapping between pipes, encoders and > connectors. > > Signed-off-by: Marius Vlad > --- > lib/igt_kms.c | 82 > +++ > 1 file changed, 82 insertions(+) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index c7a0b77..caa8837 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -68,6 +68,88 @@ > * functions have all igt_ prefixes. This part is still very much work in > * progress and so also lacks a bit documentation for the individual > functions. > * > + * Intel/DRM terminology and display connections: > + * > + * Intel documentation describes the road from memory to an output connector > as > + * follows: > + * > + * |[!<-- language="C" --> > + * .. .---. .-. .-. > + * | Memory |>| Pipes |>| Transcoders |>| DDI | > + * '' '---' '-' '-' > + * ]| > + * > + * Pipes represent the front-end of the display contain the planes, blending, > + * and color correction, while the transcoders contain timing generators, > + * encoders, A/V mixers and PSR (Panel-Self Refresh) controllers. Finally the > + * DDI represent the connectors attached to the display. > + * > + * > + * In DRM we have the following: > + * > + * |[!<-- language="C" --> > + * .---. .---. .---. .. > + * | Framebuff |>| Pipes |>| Encoders |>| Connectors | > + * '---' '---' '---' '' "Framebuff"? Also should this say crtc instead of pipe? > + * ]| > + * > + * > + * The frame buffer ties a reference to a memory object and provides a > pointer > + * to the actual data. > + * > + * The pipe is used to set the display mode, timings and gamma tables. On > some > + * hardware models this is tied with the transcoder. In DRM-parlance this is > + * referred as a CRTC. > + * > + * Each pipe has multiple planes. On older hardware these planes where known > as > + * primary plane, overlay/sprite plane, and cursor plane. From GEN9 (SKL/BXT) > + * each pipe has three planes and a cursor plane. Not quite true. > Each plane can be used as a > + * primary, as a sprite or as an overlay plane. The planes are the ones that > + * retrieve the pixels from memory and pushes them to the encoder. > + * > + * A pipe prior to GEN9: Really more like g4x-bdw. Before g4x it was totally different, and gen4 was sort of mix of both the old and the new. And vlv/chv have two sprites on each pipe. So given all the variations in the hardware, and the fact that it keeps changing all the time, I'm not convinced there's any point in documenting this in igt. It'll get stale real quick, and there are efforts to decouple igt from i915 as much as possible, so next thing you know someone else will want to docuemnt their favorite hardware here as well. So if we do want to document this stuff, then I think it should be somewhere in the kernel modeset code (around crtc/plane init I suppose). > + * > + * |[!<-- language="C" --> > + * .. > + * | Memory | .. > + * ||>| Cursor |-->... > + * || '' > + * || > + * || .. > + * ||>| Sprite |-->... > + * || '' > + * || > + * || .-. > + * ||->| Primary |>... > + * || '-' > + * '' > + * ]| > + * > + * A pipe with universal planes: > + * > + * |[!<-- language="C" --> > + * .. > + * | Memory | .. > + * ||>| Cursor |-->... > + * || '' > + * || > + * || .---. > + * ||>| Plane |-->... > + * || '---' > + * || > + * || .---. > + * ||->| Plane |>... > + * || '---' > + * '' > + * ]| > + * > + * The encoder is used to convert, depending on the output, pixels from pipes > + * to signals understood by the connector. > + * > + * The connector will connect to the output display. This contains > information > + * about the attached display such as EDID, DPMS and information about modes > + * supported by the display. > + * > * Note that this library's header pulls in the [i-g-t > * framebuffer](intel-gpu-tools-Framebuffer.html) library as a > * dependency. > -- > 2.6.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
On Fri, Jan 15, 2016 at 11:28:16AM +, Chris Wilson wrote: > On Fri, Jan 15, 2016 at 10:20:11AM +, Chris Wilson wrote: > > Throughout the code base, we use u32 for offsets into the global GTT. If > > we ever see any hardware with a larger GGTT, then we run the real risk > > of silent corruption. So test for our assumption up front so that we > > have a nice reminder should the time come when it fails. > > > > Signed-off-by: Chris Wilson > > Cc: Ville Syrjälä > > Cc: Daniel Vetter > > >From the earlier cut-n-paste, > > Reviewed-by: Ville Syrjälä > > (so I hope it still holds with the minor correction applied :) Yes. Please excuse the sucky review I gave to the original. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On 15/01/16 12:27, Chris Wilson wrote: On Fri, Jan 15, 2016 at 11:58:32AM +, Tvrtko Ursulin wrote: On 15/01/16 11:06, Chris Wilson wrote: Tvrtko was looking through the execbuffer-ioctl and noticed that the uABI was tightly coupled to our internal engine identifiers. Close inspection also revealed that we leak those internal engine identifiers through the busy-ioctl, and those internal identifiers already do not match the user identifiers. Fortuitiously, there is only one user of the set of busy rings from the busy-ioctl, and they only wish to choose between the RENDER and the BLT engines. Let's fix the userspace ABI while we still can. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 18 ++ drivers/gpu/drm/i915/intel_lrc.c| 5 + drivers/gpu/drm/i915/intel_ringbuffer.c | 5 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bb44bad15403..85797813a3de 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, if (ret) goto unref; - BUILD_BUG_ON(I915_NUM_RINGS > 16); - args->busy = obj->active << 16; - if (obj->last_write_req) - args->busy |= obj->last_write_req->ring->id; + args->busy = 0; + if (obj->active) { + int i; + + for (i = 0; i < I915_NUM_RINGS; i++) { + struct drm_i915_gem_request *req; + + req = obj->last_read_req[i]; + if (req) + args->busy |= 1 << (16 + req->ring->exec_id); If I got it right bit 16 was RCS, now will always be clear. And blitter was bit 17 and now is 19. Sssh. You weren't meant to point that out. I am willing to take the hit in order to decouple the uABI from internals. I am also willing to codify this busy-ioctl ABI into igt! Looks like your DDX is the only user not using it in the boolean mode? And libdrm is a bit confused in its return statements: ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); if (ret == 0) { bo_gem->idle = !busy.busy; return busy.busy; } else { return false; } return (ret == 0 && busy.busy); Looks like it was a boolean as well until commit 02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident started exposing the bits. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tools/intel_bios_reader: Fix typo on dump info
On Fri, 15 Jan 2016, Mika Kahola wrote: > Fix typo on intel_bios_reader.c > > Signed-off-by: Mika Kahola Pushed, thanks for the patch. BR, Jani. > --- > tools/intel_bios_reader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/intel_bios_reader.c b/tools/intel_bios_reader.c > index b31f648..5ca50ff 100644 > --- a/tools/intel_bios_reader.c > +++ b/tools/intel_bios_reader.c > @@ -843,7 +843,7 @@ static void dump_mipi_config(const struct bdb_block > *block) > > printf("\tMIPI PPS\n"); > printf("\t\tPanel power ON delay: %d\n", pps->panel_on_delay); > - printf("\t\tPanel power on to Baklight enable delay: %d\n", > pps->bl_enable_delay); > + printf("\t\tPanel power on to Backlight enable delay: %d\n", > pps->bl_enable_delay); > printf("\t\tBacklight disable to Panel power OFF delay: %d\n", > pps->bl_disable_delay); > printf("\t\tPanel power OFF delay: %d\n", pps->panel_off_delay); > printf("\t\tPanel power cycle delay: %d\n", > pps->panel_power_cycle_delay); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [i-g-t PATCH 0/6] intel_bios_reader: support MIPI sequence block v3
On Thu, 14 Jan 2016, Jani Nikula wrote: > Support MIPI sequence block v3 in the intel_bios_reader tool. This is > mostly copied from the kernel. It makes some of the parts a bit > artifical for an userspace tool, but hey, this pattern has been followed > all around in IGT and it makes debugging kernel issues much easier that > the code is similar. I don't really expect anyone else to care, so I pushed the patches. Any objections will be considered volunteering for bios/opregion tools work. ;) BR, Jani. > > BR, > Jani. > > > Jani Nikula (6): > intel_bios_reader: pass bdb pointer around instead of having as global > intel_bios_reader: fix size handling for 32-bit block size > intel_bios_reader: make the VBT pointers more const > intel_bios_reader: port find_panel_sequence_block from kernel > intel_bios_reader: port the sequence block parsing from kernel > intel_bios_reader: dump MIPI sequence block v3 > > tools/intel_bios_reader.c | 412 > +- > 1 file changed, 296 insertions(+), 116 deletions(-) -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] PROBLEM: displayport MST external monitors don't return from sleep mode
Kind Regards, Derek Yerger Computer Engineering 10171358 / djy24 [1.] displayport MST external monitors don't return from sleep mode [2.] When displays go to sleep, upon return from sleep the two monitors attached to a belkin mini displayport to 2x HDMI, do not return from sleep. This results in part of the desktop being inaccessible, until the external displays are unplugged, and then plugged back in. Sometimes, Xorg will also back down to one monitor. The data accompanying this report is for an instance where the power save mode caused Xorg to drop to the primary monitor only. This is on a Dell XPS 13 (9333) using intel i915. There are two external monitors connected by HDMI to a DP-MST adapter which connects to the single mini-displayport. A single-HDMI-to-mini-displayport adapter works as expected (coming out of power save). The bug can be reproduced by either letting the screen power save timeout elapse, or by running "xset dpms force off" [3.] [4.] Linux version 4.4.0-999-generic (kernel@tangerine) (gcc version 5.2.1 20151010 (Ubuntu 5.2.1-22ubuntu2) ) #201601122100 SMP Wed Jan 13 02:02:49 UTC 2016 [5.] [6.] [7.] Description:Ubuntu 15.10 Release:15.10 [7.1.] Linux exa 4.4.0-999-generic #201601122100 SMP Wed Jan 13 02:02:49 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux GNU C5.2.1 GNU Make4.0 Binutils2.25.1 Util-linux2.26.2 Mount2.26.2 Module-init-tools21 E2fsprogs1.42.12 Pcmciautils018 PPP2.4.6 Linux C Library2.21 Dynamic linker (ldd)2.21 Linux C++ Library6.0.21 Procps3.3.9 Net-tools1.60 Kbd1.15.5 Console-tools1.15.5 Sh-utils8.23 Udev225 Wireless-tools30 Modules Loaded8250_dw ablk_helper ac97_bus acpi_als aesni_intel aes_x86_64 ahci ansi_cprng arc4 autofs4 bluetooth bnep btbcm btintel btrtl btusb ccm cfg80211 coretemp crc32_pclmul crct10dif_pclmul cryptd ctr dcdbas dell_laptop dell_rbtn dell_smm_hwmon dell_wmi drbg drm drm_kms_helper dw_dmac dw_dmac_core elan_i2c fb_sys_fops fjes gf128mul glue_helper hid hid_generic hid_multitouch hid_rmi i2c_algo_bit i2c_designware_core i2c_designware_platform i2c_hid i915 industrialio input_leds intel_powerclamp intel_rapl intel_smartconnect ip6table_filter ip6_tables ip6t_REJECT ip6t_rt iptable_filter ip_tables ipt_REJECT irqbypass iwlmvm iwlwifi joydev kfifo_buf kvm kvm_intel libahci lp lpc_ich lrw mac80211 mac_hid media mei mei_me nf_conntrack nf_conntrack_broadcast nf_conntrack_ftp nf_conntrack_ipv4 nf_conntrack_ipv6 nf_conntrack_netbios_ns nf_defrag_ipv4 nf_defrag_ipv6 nf_log_common nf_log_ipv4 nf_log_ipv6 nf_nat nf_nat_ftp nf_reject_ipv4 nf_reject_ipv6 parport parport_pc ppdev psmouse rfcomm sdhci sdhci_acpi serio_raw shpchp snd snd_compress snd_hda_codec snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_core snd_hda_intel snd_hwdep snd_pcm snd_pcm_dmaengine snd_rawmidi snd_seq snd_seq_device snd_seq_midi snd_seq_midi_event snd_soc_core snd_soc_rl6231 snd_soc_rt5640 snd_soc_sst_acpi snd_timer soundcore sparse_keymap spi_pxa2xx_platform syscopyarea sysfillrect sysimgblt uas usbhid usb_storage uvcvideo video videobuf2_core videobuf2_memops videobuf2_v4l2 videobuf2_vmalloc videodev vmw_vmci vmw_vsock_vmci_transport vsock wmi x86_pkg_temp_thermal x_tables xt_addrtype xt_conntrack xt_hl xt_limit xt_LOG xt_tcpudp [7.2.] processor: 0 vendor_id: GenuineIntel cpu family: 6 model: 69 model name: Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz stepping: 1 microcode: 0x1c cpu MHz: 1800.000 cache size: 4096 KB physical id: 0 siblings: 4 core id: 0 cpu cores: 2 apicid: 0 initial apicid: 0 fpu: yes fpu_exception: yes cpuid level: 13 wp: yes flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm epb tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid xsaveopt dtherm ida arat pln pts bugs: bogomips: 4788.86 clflush size: 64 cache_alignment: 64 address sizes: 39 bits physical, 48 bits virtual power management: processor: 1 vendor_id: GenuineIntel cpu family: 6 model: 69 model name: Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz stepping: 1 microcode: 0x1c cpu MHz: 1905.843 cache size: 4096 KB physical id: 0 siblings: 4 core id: 0 cpu cores: 2 apicid: 1 initial apicid: 1 fpu: yes fpu_exception: yes cpuid level: 13 wp: yes
[Intel-gfx] [PATCH igt] tests: Add gem_busy
Exercise the busy-ioctl and verify it reports the right active engines using the execbuffer notation. Signed-off-by: Chris Wilson --- tests/Makefile.sources | 1 + tests/gem_busy.c | 233 + 2 files changed, 234 insertions(+) create mode 100644 tests/gem_busy.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index d2c7d6f..545aca0 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -11,6 +11,7 @@ TESTS_progs_M = \ drv_hangman \ gem_bad_reloc \ gem_basic \ + gem_busy \ gem_caching \ gem_close_race \ gem_concurrent_blit \ diff --git a/tests/gem_busy.c b/tests/gem_busy.c new file mode 100644 index 000..c6b8a8b --- /dev/null +++ b/tests/gem_busy.c @@ -0,0 +1,233 @@ +/* + * Copyright © 2015 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 "igt.h" + +/* Exercise the busy-ioctl, ensuring the ABI is never broken */ +IGT_TEST_DESCRIPTION("Basic check of busy-ioctl ABI."); + +enum { TEST = 0, BUSY, BATCH }; + +static void __gem_busy(int fd, + uint32_t handle, + uint32_t *read, + uint32_t *write) +{ + struct drm_i915_gem_busy busy; + + memset(&busy, 0, sizeof(busy)); + busy.handle = handle; + + do_ioctl(fd, DRM_IOCTL_I915_GEM_BUSY, &busy); + + *write = busy.busy & 0x; + *read = busy.busy >> 16; +} + +static uint32_t busy_blt(int fd) +{ + const int gen = intel_gen(intel_get_drm_devid(fd)); + const int has_64bit_reloc = gen >= 8; + struct drm_i915_gem_execbuffer2 execbuf; + struct drm_i915_gem_exec_object2 object[2]; + struct drm_i915_gem_relocation_entry reloc[20], *r; + uint32_t read, write; + uint32_t *map; + int factor = 10; + int i = 0; + + memset(object, 0, sizeof(object)); + object[0].handle = gem_create(fd, 1024*1024); + object[1].handle = gem_create(fd, 4096); + + r = memset(reloc, 0, sizeof(reloc)); + map = gem_mmap__cpu(fd, object[1].handle, 0, 4096, PROT_WRITE); + gem_set_domain(fd, object[1].handle, + I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU); + +#define COPY_BLT_CMD (2<<29|0x53<<22|0x6) +#define BLT_WRITE_ALPHA(1<<21) +#define BLT_WRITE_RGB (1<<20) + while (factor--) { + /* XY_SRC_COPY */ + map[i++] = COPY_BLT_CMD | BLT_WRITE_ALPHA | BLT_WRITE_RGB; + if (has_64bit_reloc) + map[i-1] += 2; + map[i++] = 0xcc << 16 | 1 << 25 | 1 << 24 | (4*1024); + map[i++] = 0; + map[i++] = 256 << 16 | 1024; + + r->offset = i * sizeof(uint32_t); + r->target_handle = object[0].handle; + r->read_domains = I915_GEM_DOMAIN_RENDER; + r->write_domain = I915_GEM_DOMAIN_RENDER; + r++; + map[i++] = 0; + if (has_64bit_reloc) + map[i++] = 0; + + map[i++] = 0; + map[i++] = 4096; + + r->offset = i * sizeof(uint32_t); + r->target_handle = object[0].handle; + r->read_domains = I915_GEM_DOMAIN_RENDER; + r->write_domain = 0; + r++; + map[i++] = 0; + if (has_64bit_reloc) + map[i++] = 0; + } + map[i++] = MI_BATCH_BUFFER_END; + munmap(map, 4096); + + object[1].relocs_ptr = (uintptr_t)reloc; + object[1].relocation_count = r - reloc; + + memset(&execbuf, 0, sizeof(execbuf)); + execbuf.buffers_ptr = (unsigned long)object; + execbuf.buffer_count = 2; + if (gen >= 6) + execbuf.flags = I915_EXEC_BLT; + gem_execbuf(fd, &exec
Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote: > Looks like your DDX is the only user not using it in the boolean mode? As far as I am aware, that is the only user that worries about which engine the object is currently active on. > And libdrm is a bit confused in its return statements: > > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); > if (ret == 0) { > bo_gem->idle = !busy.busy; > return busy.busy; > } else { > return false; > } > return (ret == 0 && busy.busy); > > Looks like it was a boolean as well until commit > 02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident > started exposing the bits. Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch postdates when we started storing read/write bits in the return value. So definitely an unintentional leakage. -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] "missed-interrupt" syndrome on Broadwell+
Mika wanted to bump these patches as they should help reduce both real and false declarations of missed interrupts plaguing Broadwell/Skylake CI. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+
In order to ensure seqno/irq coherency, we current read a ring register. We are not sure quite how it works, only that is does. Experiments show that e.g. doing a clflush(seqno) instead is not sufficient, but we can remove the forcewake dance from the mmio access. v2: Baytrail wants a clflush too. Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Mika Kuoppala --- drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8cd8aabcc3ff..935add1422ae 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1492,10 +1492,21 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring, bool lazy_coherency) { /* Workaround to force correct ordering between irq and seqno writes on * ivb (and maybe also on snb) by reading from a CS register (like -* ACTHD) before reading the status page. */ +* ACTHD) before reading the status page. +* +* Note that this effectively effectively stalls the read by the time +* it takes to do a memory transaction, which more or less ensures +* that the write from the GPU has sufficient time to invalidate +* the CPU cacheline. Alternatively we could delay the interrupt from +* the CS ring to give the write time to land, but that would incur +* a delay after every batch i.e. much more frequent than a delay +* when waiting for the interrupt (with the same net latency). +*/ if (!lazy_coherency) { struct drm_i915_private *dev_priv = ring->dev->dev_private; - POSTING_READ(RING_ACTHD(ring->mmio_base)); + POSTING_READ_FW(RING_ACTHD(ring->mmio_base)); + + intel_flush_status_page(ring, I915_GEM_HWS_INDEX); } return intel_read_status_page(ring, I915_GEM_HWS_INDEX); -- 2.7.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: Separate out the seqno-barrier from engine->get_seqno
In order to simplify future patches, extract the lazy_coherency optimisation our of the engine->get_seqno() vfunc into its own callback. v2: Rename the barrier to engine->irq_seqno_barrier to try and better reflect that the barrier is only required after the user interrupt before reading the seqno (to ensure that the seqno update lands in time as we do not have strict seqno-irq ordering on all platforms). Reviewed-by: Dave Gordon [#v2] v3: Comments for hangcheck paranoia. Mika wanted to keep the extra barrier inside the hangcheck, just in case. I can argue that it doesn't provide a barrier against anything, but the side-effects of applying the barrier may prevent a false declaration of a hung GPU. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Dave Gordon --- drivers/gpu/drm/i915/i915_debugfs.c | 6 +++--- drivers/gpu/drm/i915/i915_drv.h | 12 +++ drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 14 +++-- drivers/gpu/drm/i915/i915_trace.h | 2 +- drivers/gpu/drm/i915/intel_lrc.c| 18 ++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 36 + drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++-- 8 files changed, 52 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e3377abc0d4d..b421b53ca128 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -600,7 +600,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) ring->name, i915_gem_request_get_seqno(work->flip_queued_req), dev_priv->next_seqno, - ring->get_seqno(ring, true), + ring->get_seqno(ring), i915_gem_request_completed(work->flip_queued_req, true)); } else seq_printf(m, "Flip not associated with any ring\n"); @@ -732,7 +732,7 @@ static void i915_ring_seqno_info(struct seq_file *m, { if (ring->get_seqno) { seq_printf(m, "Current sequence (%s): %x\n", - ring->name, ring->get_seqno(ring, false)); + ring->name, ring->get_seqno(ring)); } } @@ -1342,8 +1342,8 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) intel_runtime_pm_get(dev_priv); for_each_ring(ring, dev_priv, i) { - seqno[i] = ring->get_seqno(ring, false); acthd[i] = intel_ring_get_active_head(ring); + seqno[i] = ring->get_seqno(ring); } i915_get_extra_instdone(dev, instdone); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index eb7bb97f7316..0cdaed66b786 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2961,15 +2961,19 @@ i915_seqno_passed(uint32_t seq1, uint32_t seq2) static inline bool i915_gem_request_started(struct drm_i915_gem_request *req, bool lazy_coherency) { - u32 seqno = req->ring->get_seqno(req->ring, lazy_coherency); - return i915_seqno_passed(seqno, req->previous_seqno); + if (!lazy_coherency && req->ring->irq_seqno_barrier) + req->ring->irq_seqno_barrier(req->ring); + return i915_seqno_passed(req->ring->get_seqno(req->ring), +req->previous_seqno); } static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req, bool lazy_coherency) { - u32 seqno = req->ring->get_seqno(req->ring, lazy_coherency); - return i915_seqno_passed(seqno, req->seqno); + if (!lazy_coherency && req->ring->irq_seqno_barrier) + req->ring->irq_seqno_barrier(req->ring); + return i915_seqno_passed(req->ring->get_seqno(req->ring), +req->seqno); } int __must_check i915_gem_get_seqno(struct drm_device *dev, u32 *seqno); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 06ca4082735b..b978febf67a0 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -902,8 +902,8 @@ static void i915_record_ring_state(struct drm_device *dev, ering->waiting = waitqueue_active(&ring->irq_queue); ering->instpm = I915_READ(RING_INSTPM(ring->mmio_base)); - ering->seqno = ring->get_seqno(ring, false); ering->acthd = intel_ring_get_active_head(ring); + ering->seqno = ring->get_seqno(ring); ering->start = I915_READ_START(ring); ering->head = I915_READ_HEAD(ring); ering->tail = I915_READ_TAIL(ring); diff --git a/drivers/gpu/
[Intel-gfx] [PATCH 5/6] drm/i915: Use simplest form for flushing the single cacheline in the HWS
Rather than call a function to compute the matching cachelines and clflush them, just call the clflush *instruction* directly. We also know that we can use the unpatched plain clflush rather than the clflushopt alternative. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Imre Deak --- drivers/gpu/drm/i915/intel_ringbuffer.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index b22573561669..1603cb1af12f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -389,8 +389,9 @@ intel_ring_sync_index(struct intel_engine_cs *ring, static inline void intel_flush_status_page(struct intel_engine_cs *ring, int reg) { - drm_clflush_virt_range(&ring->status_page.page_addr[reg], - sizeof(uint32_t)); + mb(); + clflush(&ring->status_page.page_addr[reg]); + mb(); } static inline u32 -- 2.7.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: Replace manual barrier() with READ_ONCE() in HWS accessor
When reading from the HWS page, we use barrier() to prevent the compiler optimising away the read from the volatile (may be updated by the GPU) memory address. This is more suited to READ_ONCE(); make it so. Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 1603cb1af12f..78d5192ecf9c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -399,8 +399,7 @@ intel_read_status_page(struct intel_engine_cs *ring, int reg) { /* Ensure that the compiler doesn't optimize away the load. */ - barrier(); - return ring->status_page.page_addr[reg]; + return READ_ONCE(ring->status_page.page_addr[reg]); } static inline void -- 2.7.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: Harden detection of missed interrupts
Only declare a missed interrupt if we find that the GPU is idle with waiters and a hangcheck interval has passed in which no new user interrupts have been raised. Signed-off-by: Chris Wilson Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_debugfs.c | 11 +++ drivers/gpu/drm/i915/i915_irq.c | 7 ++- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index b421b53ca128..966fc022418c 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -730,10 +730,10 @@ static int i915_gem_request_info(struct seq_file *m, void *data) static void i915_ring_seqno_info(struct seq_file *m, struct intel_engine_cs *ring) { - if (ring->get_seqno) { - seq_printf(m, "Current sequence (%s): %x\n", - ring->name, ring->get_seqno(ring)); - } + seq_printf(m, "Current sequence (%s): %x\n", + ring->name, ring->get_seqno(ring)); + seq_printf(m, "Current user interrupts (%s): %x\n", + ring->name, READ_ONCE(ring->user_interrupts)); } static int i915_gem_seqno_info(struct seq_file *m, void *data) @@ -1361,6 +1361,9 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused) seq_printf(m, "%s:\n", ring->name); seq_printf(m, "\tseqno = %x [current %x]\n", ring->hangcheck.seqno, seqno[i]); + seq_printf(m, "\tuser interrupts = %x [current %x]\n", + ring->hangcheck.user_interrupts, + ring->user_interrupts); seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n", (long long)ring->hangcheck.acthd, (long long)acthd[i]); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 07bc2cdd6252..978eebcf4594 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1000,6 +1000,7 @@ static void notify_ring(struct intel_engine_cs *ring) return; trace_i915_gem_request_notify(ring); + ring->user_interrupts++; wake_up_all(&ring->irq_queue); } @@ -3097,6 +3098,7 @@ static void i915_hangcheck_elapsed(struct work_struct *work) for_each_ring(ring, dev_priv, i) { u64 acthd; u32 seqno; + unsigned user_interrupts; bool busy = true; semaphore_clear_deadlocks(dev_priv); @@ -3113,6 +3115,7 @@ static void i915_hangcheck_elapsed(struct work_struct *work) acthd = intel_ring_get_active_head(ring); seqno = ring->get_seqno(ring); + user_interrupts = READ_ONCE(ring->user_interrupts); if (ring->hangcheck.seqno == seqno) { if (ring_idle(ring, seqno)) { @@ -3120,7 +3123,8 @@ static void i915_hangcheck_elapsed(struct work_struct *work) if (waitqueue_active(&ring->irq_queue)) { /* Issue a wake-up to catch stuck h/w. */ - if (!test_and_set_bit(ring->id, &dev_priv->gpu_error.missed_irq_rings)) { + if (ring->hangcheck.user_interrupts == user_interrupts && + !test_and_set_bit(ring->id, &dev_priv->gpu_error.missed_irq_rings)) { if (!(dev_priv->gpu_error.test_irq_rings & intel_ring_flag(ring))) DRM_ERROR("Hangcheck timer elapsed... %s idle\n", ring->name); @@ -3187,6 +3191,7 @@ static void i915_hangcheck_elapsed(struct work_struct *work) ring->hangcheck.seqno = seqno; ring->hangcheck.acthd = acthd; + ring->hangcheck.user_interrupts = user_interrupts; busy_count += busy; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 8fb02b21e75d..b22573561669 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -90,6 +90,7 @@ struct intel_ring_hangcheck { u64 acthd; u64 max_acthd; u32 seqno; + unsigned user_interrupts; int score; enum intel_ring_hangcheck_action action; int deadlock; @@ -301,6 +302,7 @@ struct intel_engine_cs { * inspecting request list. */ u32 last_submitted_seqno; + unsigned user_interrupts; bool gpu_caches_dirty; -- 2.7.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lis
[Intel-gfx] [PATCH 3/6] drm/i915: Broadwell execlists needs exactly the same seqno w/a as legacy
In legacy mode, we use the gen6 seqno barrier to insert a delay after the interrupt before reading the seqno (as the seqno write is not flushed before the interrupt is sent, the interrupt arrives before the seqno is visible). Execlists ignored the evidence of igt. Note that is harder, but not impossible, to reproduce the missed interrupt syndrome with execlists. This is primarily because execlists itself being interrupt driven helps mask the issue. v2: Rebase and unsquash! I kept it as gen6_seqno_barrier() for I have a long term plan to merge the two implementations. Signed-off-by: Chris Wilson Cc: Mika Kuoppala dev); + POSTING_READ_FW(RING_ACTHD(ring->mmio_base)); + intel_flush_status_page(ring, I915_GEM_HWS_INDEX); } @@ -1949,12 +1959,11 @@ logical_ring_default_vfuncs(struct drm_device *dev, ring->irq_get = gen8_logical_ring_get_irq; ring->irq_put = gen8_logical_ring_put_irq; ring->emit_bb_start = gen8_emit_bb_start; + ring->irq_seqno_barrier = gen6_seqno_barrier; ring->get_seqno = gen8_get_seqno; ring->set_seqno = gen8_set_seqno; - if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) { - ring->irq_seqno_barrier = bxt_seqno_barrier; + if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) ring->set_seqno = bxt_a_set_seqno; - } } static inline void -- 2.7.0.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
Adding Chris. We would need stolen_base and size early, could you check if moving i915_gem_init_stolen() earlier based on the diff at the end is ok? On to, 2016-01-14 at 21:26 +0530, Sagar Arun Kamble wrote: > RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6 > setup registers. If those are not setup Driver should not enable RC6. > For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values > to know if BIOS has enabled HW/SW RC6. > This will also enable user to control RC6 using BIOS settings alone. > RC6 related instability can be avoided by disabling via BIOS settings > till driver fixes it. > > v2: Had placed logic in gen8 function by mistake. Fixed it. > Ensuring RPM is not enabled in case BIOS disabled RC6. > > v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel) > Runtime PM enabling happens before gen9_enable_rc6. > Moved the updation of enable_rc6 parameter in intel_uncore_sanitize. > > v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. > (Imre) > > v5: Caching reserved stolen base and size in the driver private data. > Reorganized RC6 setup check. Moved from gen9_enable_rc6 to > intel_uncore_sanitize. (Imre) > > Cc: Imre Deak > Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87 > Signed-off-by: Sagar Arun Kamble > --- > drivers/gpu/drm/i915/i915_drv.h| 1 + > drivers/gpu/drm/i915/i915_gem_gtt.h| 2 ++ > drivers/gpu/drm/i915/i915_gem_stolen.c | 39 ++ > drivers/gpu/drm/i915/i915_reg.h| 11 +++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c| 59 > -- > drivers/gpu/drm/i915/intel_uncore.c| 4 +++ > 7 files changed, 109 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index cf7e0fc..0c8e61c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3223,6 +3223,7 @@ int i915_gem_stolen_insert_node_in_range(struct > drm_i915_private *dev_priv, > u64 end); > void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv, > struct drm_mm_node *node); > +int i915_gem_init_stolen_reserved(struct drm_device *dev); > int i915_gem_init_stolen(struct drm_device *dev); > void i915_gem_cleanup_stolen(struct drm_device *dev); > struct drm_i915_gem_object * > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h > b/drivers/gpu/drm/i915/i915_gem_gtt.h > index b448ad8..20ff6ca 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -343,6 +343,8 @@ struct i915_gtt { > > size_t stolen_size; /* Total size of stolen memory */ > size_t stolen_usable_size; /* Total size minus BIOS reserved */ > + size_t stolen_reserved_base; > + size_t stolen_reserved_size; > u64 mappable_end; /* End offset that we can CPU map */ > struct io_mapping *mappable;/* Mapping to our CPU mappable region */ > phys_addr_t mappable_base; /* PA of our GMADR */ > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c > b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 3476877..d5a65d9 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -386,14 +386,12 @@ static void bdw_get_stolen_reserved(struct > drm_i915_private *dev_priv, > *size = stolen_top - *base; > } > > -int i915_gem_init_stolen(struct drm_device *dev) > +int i915_gem_init_stolen_reserved(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - unsigned long reserved_total, reserved_base = 0, reserved_size; > + unsigned long reserved_base = 0, reserved_size; > unsigned long stolen_top; > > - mutex_init(&dev_priv->mm.stolen_lock); > - > #ifdef CONFIG_INTEL_IOMMU > if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) { > DRM_INFO("DMAR active, disabling use of stolen memory\n"); > @@ -458,9 +456,38 @@ int i915_gem_init_stolen(struct drm_device *dev) > return 0; > } > > + dev_priv->gtt.stolen_reserved_base = reserved_base; > + dev_priv->gtt.stolen_reserved_size = reserved_size; > + > + return 0; > +} > + > +int i915_gem_init_stolen(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + unsigned long reserved_total; > + unsigned long stolen_top; > + > + mutex_init(&dev_priv->mm.stolen_lock); > + > +#ifdef CONFIG_INTEL_IOMMU > + if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) { > + DRM_INFO("DMAR active, disabling use of stolen memory\n"); > + return 0; > + } > +#endif > + > + if (dev_priv->gtt.stolen_size == 0) > + return 0; > + > + if (dev_priv->mm.stolen_base == 0) > + return 0;
[Intel-gfx] [PATCH] MAINTAINERS: Add "B:" preferred bug reporting method
Different subsystems and drivers have different preferred ways of receiving bug reports; mailing list or bugzillas at various locations. Add "B:" entry for specifying the preference to guide bug reporters at the right location. While at it, document the freedesktop.org bugzilla as the preferred location for drm/i915 bug reports. We have more reassignment of bugs between the kernel and userspace components than with other kernel components, and we've thus consolidated our bug tracking there. Cc: Daniel Vetter Cc: Joe Perches Cc: Andrew Morton Signed-off-by: Jani Nikula --- MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 32eda9d0be0c..a803066a9422 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -75,6 +75,8 @@ Descriptions of section entries: L: Mailing list that is relevant to this area W: Web-page with status/info Q: Patchwork web based patch tracking system site + B: Preferred method for reporting bugs; bug tracking system site + or "List" for mailing list. T: SCM tree type and location. Type is one of: git, hg, quilt, stgit, topgit S: Status, one of the following: @@ -3655,6 +3657,7 @@ L:intel-gfx@lists.freedesktop.org L: dri-de...@lists.freedesktop.org W: https://01.org/linuxgraphics/ Q: http://patchwork.freedesktop.org/project/intel-gfx/ +B: https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel T: git git://anongit.freedesktop.org/drm-intel S: Supported F: drivers/gpu/drm/i915/ -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] tests: Add gem_busy
On 15/01/16 13:53, Chris Wilson wrote: Exercise the busy-ioctl and verify it reports the right active engines using the execbuffer notation. Signed-off-by: Chris Wilson --- tests/Makefile.sources | 1 + tests/gem_busy.c | 233 + 2 files changed, 234 insertions(+) create mode 100644 tests/gem_busy.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index d2c7d6f..545aca0 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -11,6 +11,7 @@ TESTS_progs_M = \ drv_hangman \ gem_bad_reloc \ gem_basic \ + gem_busy \ gem_caching \ gem_close_race \ gem_concurrent_blit \ diff --git a/tests/gem_busy.c b/tests/gem_busy.c new file mode 100644 index 000..c6b8a8b --- /dev/null +++ b/tests/gem_busy.c @@ -0,0 +1,233 @@ +/* + * Copyright © 2015 Intel Corporation 2016 + * + * 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 "igt.h" + +/* Exercise the busy-ioctl, ensuring the ABI is never broken */ +IGT_TEST_DESCRIPTION("Basic check of busy-ioctl ABI."); + +enum { TEST = 0, BUSY, BATCH }; + +static void __gem_busy(int fd, + uint32_t handle, + uint32_t *read, + uint32_t *write) +{ + struct drm_i915_gem_busy busy; + + memset(&busy, 0, sizeof(busy)); + busy.handle = handle; + + do_ioctl(fd, DRM_IOCTL_I915_GEM_BUSY, &busy); + + *write = busy.busy & 0x; + *read = busy.busy >> 16; +} + +static uint32_t busy_blt(int fd) +{ + const int gen = intel_gen(intel_get_drm_devid(fd)); + const int has_64bit_reloc = gen >= 8; + struct drm_i915_gem_execbuffer2 execbuf; + struct drm_i915_gem_exec_object2 object[2]; + struct drm_i915_gem_relocation_entry reloc[20], *r; + uint32_t read, write; + uint32_t *map; + int factor = 10; + int i = 0; + + memset(object, 0, sizeof(object)); + object[0].handle = gem_create(fd, 1024*1024); + object[1].handle = gem_create(fd, 4096); + + r = memset(reloc, 0, sizeof(reloc)); + map = gem_mmap__cpu(fd, object[1].handle, 0, 4096, PROT_WRITE); + gem_set_domain(fd, object[1].handle, + I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU); + +#define COPY_BLT_CMD (2<<29|0x53<<22|0x6) +#define BLT_WRITE_ALPHA(1<<21) +#define BLT_WRITE_RGB (1<<20) + while (factor--) { + /* XY_SRC_COPY */ + map[i++] = COPY_BLT_CMD | BLT_WRITE_ALPHA | BLT_WRITE_RGB; + if (has_64bit_reloc) + map[i-1] += 2; + map[i++] = 0xcc << 16 | 1 << 25 | 1 << 24 | (4*1024); + map[i++] = 0; + map[i++] = 256 << 16 | 1024; + + r->offset = i * sizeof(uint32_t); + r->target_handle = object[0].handle; + r->read_domains = I915_GEM_DOMAIN_RENDER; + r->write_domain = I915_GEM_DOMAIN_RENDER; + r++; + map[i++] = 0; + if (has_64bit_reloc) + map[i++] = 0; + + map[i++] = 0; + map[i++] = 4096; + + r->offset = i * sizeof(uint32_t); + r->target_handle = object[0].handle; + r->read_domains = I915_GEM_DOMAIN_RENDER; + r->write_domain = 0; + r++; + map[i++] = 0; + if (has_64bit_reloc) + map[i++] = 0; + } + map[i++] = MI_BATCH_BUFFER_END; + munmap(map, 4096); + + object[1].relocs_ptr = (uintptr_t)reloc; + object[1].relocation_count = r - reloc; + + memset(&execbuf, 0, sizeof(execbuf)); + execbuf.buffers_ptr = (unsigned long)object; + execbuf.buffer_count = 2; + if (gen >= 6) + execb
Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
On pe, 2016-01-15 at 16:44 +0200, Imre Deak wrote: > Adding Chris. We would need stolen_base and size early, could you > check > if moving i915_gem_init_stolen() earlier based on the diff at the end > is ok? Oops, I meant stolen reserved_base and reserved_size. > On to, 2016-01-14 at 21:26 +0530, Sagar Arun Kamble wrote: > > RC6 setup is shared between BIOS and Driver. BIOS sets up subset of > > RC6 > > setup registers. If those are not setup Driver should not enable > > RC6. > > For implementing this, driver can check RC_CTRL0 and RC_CTRL1 > > values > > to know if BIOS has enabled HW/SW RC6. > > This will also enable user to control RC6 using BIOS settings > > alone. > > RC6 related instability can be avoided by disabling via BIOS > > settings > > till driver fixes it. > > > > v2: Had placed logic in gen8 function by mistake. Fixed it. > > Ensuring RPM is not enabled in case BIOS disabled RC6. > > > > v3: Need to disable RPM if RC6 is disabled due to BIOS settings. > > (Daniel) > > Runtime PM enabling happens before gen9_enable_rc6. > > Moved the updation of enable_rc6 parameter in > > intel_uncore_sanitize. > > > > v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx > > for bxt. (Imre) > > > > v5: Caching reserved stolen base and size in the driver private > > data. > > Reorganized RC6 setup check. Moved from gen9_enable_rc6 to > > intel_uncore_sanitize. (Imre) > > > > Cc: Imre Deak > > Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87 > > Signed-off-by: Sagar Arun Kamble > > --- > > drivers/gpu/drm/i915/i915_drv.h| 1 + > > drivers/gpu/drm/i915/i915_gem_gtt.h| 2 ++ > > drivers/gpu/drm/i915/i915_gem_stolen.c | 39 ++ > > drivers/gpu/drm/i915/i915_reg.h| 11 +++ > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > drivers/gpu/drm/i915/intel_pm.c| 59 > > -- > > drivers/gpu/drm/i915/intel_uncore.c| 4 +++ > > 7 files changed, 109 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index cf7e0fc..0c8e61c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -3223,6 +3223,7 @@ int > > i915_gem_stolen_insert_node_in_range(struct drm_i915_private > > *dev_priv, > > u64 end); > > void i915_gem_stolen_remove_node(struct drm_i915_private > > *dev_priv, > > struct drm_mm_node *node); > > +int i915_gem_init_stolen_reserved(struct drm_device *dev); > > int i915_gem_init_stolen(struct drm_device *dev); > > void i915_gem_cleanup_stolen(struct drm_device *dev); > > struct drm_i915_gem_object * > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h > > b/drivers/gpu/drm/i915/i915_gem_gtt.h > > index b448ad8..20ff6ca 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > > @@ -343,6 +343,8 @@ struct i915_gtt { > > > > size_t stolen_size; /* Total size of stolen > > memory */ > > size_t stolen_usable_size; /* Total size minus BIOS > > reserved */ > > + size_t stolen_reserved_base; > > + size_t stolen_reserved_size; > > u64 mappable_end; /* End offset that we can > > CPU map */ > > struct io_mapping *mappable;/* Mapping to our CPU > > mappable region */ > > phys_addr_t mappable_base; /* PA of our GMADR */ > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c > > b/drivers/gpu/drm/i915/i915_gem_stolen.c > > index 3476877..d5a65d9 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > > @@ -386,14 +386,12 @@ static void bdw_get_stolen_reserved(struct > > drm_i915_private *dev_priv, > > *size = stolen_top - *base; > > } > > > > -int i915_gem_init_stolen(struct drm_device *dev) > > +int i915_gem_init_stolen_reserved(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > - unsigned long reserved_total, reserved_base = 0, > > reserved_size; > > + unsigned long reserved_base = 0, reserved_size; > > unsigned long stolen_top; > > > > - mutex_init(&dev_priv->mm.stolen_lock); > > - > > #ifdef CONFIG_INTEL_IOMMU > > if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) { > > DRM_INFO("DMAR active, disabling use of stolen > > memory\n"); > > @@ -458,9 +456,38 @@ int i915_gem_init_stolen(struct drm_device > > *dev) > > return 0; > > } > > > > + dev_priv->gtt.stolen_reserved_base = reserved_base; > > + dev_priv->gtt.stolen_reserved_size = reserved_size; > > + > > + return 0; > > +} > > + > > +int i915_gem_init_stolen(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + unsigned long reserved_total; > > + unsigned long stolen_top; > > + > > + mutex_init(&dev_priv->mm.stolen_lock); > > + > > +#ifdef CONFIG_INTEL_I
Re: [Intel-gfx] [PATCH 10/11] acpi: Export acpi_bus_type
On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sha...@intel.com wrote: > From: Ankitprasad Sharma > > Some modules, like i915.ko, needs to detect when certain ACPI features > are active inorder to prevent corruption on contended resources. > In particular, use of BIOS RapidStart Technology may corrupt the contents > of the reserved graphics memory, due to unalarmed hibernation. In which > case i915.ko cannot assume that it (reserved gfx memory) remains > unmodified and must recreate teh contents and importantly not use it to > store unrecoverable user data. > > Signed-off-by: Ankitprasad Sharma > Cc: "Rafael J. Wysocki" > Cc: Len Brown > Cc: linux-a...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > --- > drivers/acpi/bus.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index a212cef..69509c7 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = { > .remove = acpi_device_remove, > .uevent = acpi_device_uevent, > }; > +EXPORT_SYMBOL_GPL(acpi_bus_type); > > /* -- > Initialization/Cleanup > No. I see no reason whatsoever for doing this. Thanks, Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Cache LRC state page in the context
From: Tvrtko Ursulin LRC lifetime is well defined so we can cache the page pointing to the object backing store in the context in order to avoid walking over the object SG page list from the interrupt context without the big lock held. v2: Also cache the mapping. (Chris Wilson) v3: Unmap on the error path. v4: No need to cache the page. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson Cc: Dave Gordon --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_lrc.c | 53 ++-- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index acff98b9c148..af301482e6f8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -890,6 +890,7 @@ struct intel_context { int pin_count; struct i915_vma *lrc_vma; u64 lrc_desc; + uint32_t *lrc_reg_state; } engine[I915_NUM_RINGS]; struct list_head link; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 545173c1b5c5..70bd28cc8887 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -390,14 +390,7 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) { struct intel_engine_cs *ring = rq->ring; struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt; - struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; - struct page *page; - uint32_t *reg_state; - - BUG_ON(!ctx_obj); - - page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN); - reg_state = kmap_atomic(page); + uint32_t *reg_state = rq->ctx->engine[ring->id].lrc_reg_state; reg_state[CTX_RING_TAIL+1] = rq->tail; reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start; @@ -414,8 +407,6 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) ASSIGN_CTX_PDP(ppgtt, reg_state, 0); } - kunmap_atomic(reg_state); - return 0; } @@ -1067,6 +1058,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state; struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; + struct page *lrc_state_page; + uint32_t *lrc_reg_state; int ret; WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); @@ -1076,12 +1069,25 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, if (ret) return ret; + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN); + if (WARN_ON(!lrc_state_page)) { + ret = -ENODEV; + goto unpin_ctx_obj; + } + + lrc_reg_state = kmap(lrc_state_page); + if (!lrc_reg_state) { + ret = -ENOMEM; + goto unpin_ctx_obj; + } + ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf); if (ret) - goto unpin_ctx_obj; + goto unmap_state_page; ctx->engine[ring->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj); intel_lr_context_descriptor_update(ctx, ring); + ctx->engine[ring->id].lrc_reg_state = lrc_reg_state; ctx_obj->dirty = true; /* Invalidate GuC TLB. */ @@ -1090,6 +1096,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, return ret; +unmap_state_page: + kunmap(lrc_state_page); unpin_ctx_obj: i915_gem_object_ggtt_unpin(ctx_obj); @@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct drm_i915_gem_request *rq) struct intel_engine_cs *ring = rq->ring; struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; struct intel_ringbuffer *ringbuf = rq->ringbuf; + struct page *lrc_state_page; - if (ctx_obj) { - WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); - if (--rq->ctx->engine[ring->id].pin_count == 0) { - intel_unpin_ringbuffer_obj(ringbuf); - i915_gem_object_ggtt_unpin(ctx_obj); - rq->ctx->engine[ring->id].lrc_vma = NULL; - rq->ctx->engine[ring->id].lrc_desc = 0; - } + WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); + + if (!ctx_obj) + return; + + if (--rq->ctx->engine[ring->id].pin_count == 0) { + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, + LRC_STATE_PN); + kunmap(lrc_state_page); + intel_unpin_ringbuffer_obj(ringbuf); + i915_gem_object_ggtt_unpin(ctx_obj); + rq->ctx->engine[ring->id].lrc_vma = NULL; + rq->ctx->
[Intel-gfx] [PATCH] drm/i915: Only grab timestamps when needed
From: Tvrtko Ursulin No need to call ktime_get_raw_ns twice per unlimited wait and can also elimate a local variable. v2: Added comment about silencing the compiler warning. (Daniel Vetter) Signed-off-by: Tvrtko Ursulin Reviewed-by: Dave Gordon Acked-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ddc21d4b388d..6b0102da859c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1251,7 +1251,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req, int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; DEFINE_WAIT(wait); unsigned long timeout_expire; - s64 before, now; + s64 before = 0; /* Only to silence a compiler warning. */ int ret; WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled"); @@ -1271,14 +1271,17 @@ int __i915_wait_request(struct drm_i915_gem_request *req, return -ETIME; timeout_expire = jiffies + nsecs_to_jiffies_timeout(*timeout); + + /* +* Record current time in case interrupted by signal, or wedged. +*/ + before = ktime_get_raw_ns(); } if (INTEL_INFO(dev_priv)->gen >= 6) gen6_rps_boost(dev_priv, rps, req->emitted_jiffies); - /* Record current time in case interrupted by signal, or wedged */ trace_i915_gem_request_wait_begin(req); - before = ktime_get_raw_ns(); /* Optimistic spin for the next jiffie before touching IRQs */ ret = __i915_spin_request(req, state); @@ -1343,11 +1346,10 @@ int __i915_wait_request(struct drm_i915_gem_request *req, finish_wait(&ring->irq_queue, &wait); out: - now = ktime_get_raw_ns(); trace_i915_gem_request_wait_end(req); if (timeout) { - s64 tres = *timeout - (now - before); + s64 tres = *timeout - (ktime_get_raw_ns() - before); *timeout = tres < 0 ? 0 : tres; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Cache ringbuffer GTT VMA
From: Tvrtko Ursulin Purpose is to avoid calling i915_gem_obj_ggtt_offset from the interrupt context without the big lock held. v2: Renamed gtt_start to gtt_offset. (Daniel Vetter) v3: Cache the VMA instead of address. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Cc: Daniel Vetter Cc: Chris Wilson --- drivers/gpu/drm/i915/intel_lrc.c| 3 +-- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 1a0d85325072..545173c1b5c5 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -391,7 +391,6 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) struct intel_engine_cs *ring = rq->ring; struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt; struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; - struct drm_i915_gem_object *rb_obj = rq->ringbuf->obj; struct page *page; uint32_t *reg_state; @@ -401,7 +400,7 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) reg_state = kmap_atomic(page); reg_state[CTX_RING_TAIL+1] = rq->tail; - reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(rb_obj); + reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start; if (ppgtt && !USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) { /* True 32b PPGTT with dynamic page allocation: update PDP diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 4060acf0601a..4150a240d47d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1997,6 +1997,7 @@ void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf) else iounmap(ringbuf->virtual_start); ringbuf->virtual_start = NULL; + ringbuf->vma = NULL; i915_gem_object_ggtt_unpin(ringbuf->obj); } @@ -2063,6 +2064,8 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev, } } + ringbuf->vma = i915_gem_obj_to_ggtt(obj); + return 0; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 85ce2272f92c..ede57954080e 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -99,6 +99,7 @@ struct intel_ring_hangcheck { struct intel_ringbuffer { struct drm_i915_gem_object *obj; void __iomem *virtual_start; + struct i915_vma *vma; struct intel_engine_cs *ring; struct list_head link; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Do not call API requiring struct_mutex where it is not available
From: Tvrtko Ursulin LRC code was calling GEM API like i915_gem_obj_ggtt_offset from places where the struct_mutex cannot be grabbed (irq handlers). To avoid that this patch caches some interesting bits and values in the engine and context structures. Some usages are also removed where they are not needed like a few asserts which are either impossible or have been checked already during engine initialization. Side benefit is also that interrupt handlers and command submission stop evaluating invariant conditionals, like what Gen we are running on, on every interrupt and every command submitted. This patch deals with logical ring context id and descriptors while subsequent patches will deal with the remaining issues. v2: * Cache the VMA instead of the address. (Chris Wilson) * Incorporate Dave Gordon's good comments and function name. v3: * Extract ctx descriptor template to a function and group functions dealing with ctx descriptor & co together near top of the file. (Dave Gordon) Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Cc: Daniel Vetter Cc: Dave Gordon --- drivers/gpu/drm/i915/i915_debugfs.c | 15 ++-- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_gem_gtt.h | 1 - drivers/gpu/drm/i915/intel_lrc.c| 151 +++- drivers/gpu/drm/i915/intel_lrc.h| 4 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 6 files changed, 103 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index e3377abc0d4d..0b3550f05026 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1994,12 +1994,13 @@ static int i915_context_status(struct seq_file *m, void *unused) } static void i915_dump_lrc_obj(struct seq_file *m, - struct intel_engine_cs *ring, - struct drm_i915_gem_object *ctx_obj) + struct intel_context *ctx, + struct intel_engine_cs *ring) { struct page *page; uint32_t *reg_state; int j; + struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state; unsigned long ggtt_offset = 0; if (ctx_obj == NULL) { @@ -2009,7 +2010,7 @@ static void i915_dump_lrc_obj(struct seq_file *m, } seq_printf(m, "CONTEXT: %s %u\n", ring->name, - intel_execlists_ctx_id(ctx_obj)); + intel_execlists_ctx_id(ctx, ring)); if (!i915_gem_obj_ggtt_bound(ctx_obj)) seq_puts(m, "\tNot bound in GGTT\n"); @@ -2058,8 +2059,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused) list_for_each_entry(ctx, &dev_priv->context_list, link) { for_each_ring(ring, dev_priv, i) { if (ring->default_context != ctx) - i915_dump_lrc_obj(m, ring, - ctx->engine[i].state); + i915_dump_lrc_obj(m, ctx, ring); } } @@ -2133,11 +2133,8 @@ static int i915_execlists(struct seq_file *m, void *data) seq_printf(m, "\t%d requests in queue\n", count); if (head_req) { - struct drm_i915_gem_object *ctx_obj; - - ctx_obj = head_req->ctx->engine[ring_id].state; seq_printf(m, "\tHead request id: %u\n", - intel_execlists_ctx_id(ctx_obj)); + intel_execlists_ctx_id(head_req->ctx, ring)); seq_printf(m, "\tHead request tail: %u\n", head_req->tail); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index eb7bb97f7316..acff98b9c148 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -888,6 +888,8 @@ struct intel_context { struct drm_i915_gem_object *state; struct intel_ringbuffer *ringbuf; int pin_count; + struct i915_vma *lrc_vma; + u64 lrc_desc; } engine[I915_NUM_RINGS]; struct list_head link; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index b448ad832dcf..e5737963ab79 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -44,7 +44,6 @@ typedef uint64_t gen8_ppgtt_pml4e_t; #define gtt_total_entries(gtt) ((gtt).base.total >> PAGE_SHIFT) - /* gen6-hsw has bit 11-4 for physical addr bit 39-32 */ #define GEN6_GTT_ADDR_ENCODE(addr) ((addr) | (((addr) >> 28) & 0xff0)) #define GEN6_PTE_ADDR_ENCODE(addr) GEN6_GTT_ADDR_ENCODE(addr) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 5027699c5291..1a0d85325072 100644 --- a/drivers
[Intel-gfx] [PATCH v2] drm/i915: Decouple execbuf uAPI from internal implementation
From: Tvrtko Ursulin At the moment execbuf ring selection is fully coupled to internal ring ids which is not a good thing on its own. This dependency is also spread between two source files and not spelled out at either side which makes it hidden and fragile. This patch decouples this dependency by introducing an explicit translation table of execbuf uAPI to ring id close to the only call site (i915_gem_do_execbuffer). This way we are free to change driver internal implementation details without breaking userspace. All state relating to the uAPI is now contained in, or next to, i915_gem_do_execbuffer. As a side benefit, this patch decreases the compiled size of i915_gem_do_execbuffer. v2: Extract ring selection into eb_select_ring. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Cc: Daniel Vetter Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h| 4 +- drivers/gpu/drm/i915/i915_gem.c| 2 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 139 +++-- drivers/gpu/drm/i915/intel_ringbuffer.h| 10 +-- 4 files changed, 81 insertions(+), 74 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index eb7bb97f7316..35d5d6099a44 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -334,7 +334,7 @@ struct drm_i915_file_private { unsigned boosts; } rps; - struct intel_engine_cs *bsd_ring; + unsigned int bsd_ring; }; enum intel_dpll_id { @@ -1300,7 +1300,7 @@ struct i915_gem_mm { bool busy; /* the indicator for dispatch video commands on two BSD rings */ - int bsd_ring_dispatch_index; + unsigned int bsd_ring_dispatch_index; /** Bit 6 swizzling required for X tiling */ uint32_t bit_6_swizzle_x; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ddc21d4b388d..26e6842f2df3 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5112,6 +5112,8 @@ int i915_gem_open(struct drm_device *dev, struct drm_file *file) spin_lock_init(&file_priv->mm.lock); INIT_LIST_HEAD(&file_priv->mm.request_list); + file_priv->bsd_ring = -1; + ret = i915_gem_context_open(dev, file); if (ret) kfree(file_priv); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d469c4779ff5..497a2f87836c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1328,33 +1328,23 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, /** * Find one BSD ring to dispatch the corresponding BSD command. - * The Ring ID is returned. + * The ring index is returned. */ -static int gen8_dispatch_bsd_ring(struct drm_device *dev, - struct drm_file *file) +static unsigned int +gen8_dispatch_bsd_ring(struct drm_i915_private *dev_priv, struct drm_file *file) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_file_private *file_priv = file->driver_priv; - /* Check whether the file_priv is using one ring */ - if (file_priv->bsd_ring) - return file_priv->bsd_ring->id; - else { - /* If no, use the ping-pong mechanism to select one ring */ - int ring_id; - - mutex_lock(&dev->struct_mutex); - if (dev_priv->mm.bsd_ring_dispatch_index == 0) { - ring_id = VCS; - dev_priv->mm.bsd_ring_dispatch_index = 1; - } else { - ring_id = VCS2; - dev_priv->mm.bsd_ring_dispatch_index = 0; - } - file_priv->bsd_ring = &dev_priv->ring[ring_id]; - mutex_unlock(&dev->struct_mutex); - return ring_id; + /* Check whether the file_priv has already selected one ring. */ + if ((int)file_priv->bsd_ring < 0) { + /* If not, use the ping-pong mechanism to select one. */ + mutex_lock(&dev_priv->dev->struct_mutex); + file_priv->bsd_ring = dev_priv->mm.bsd_ring_dispatch_index; + dev_priv->mm.bsd_ring_dispatch_index ^= 1; + mutex_unlock(&dev_priv->dev->struct_mutex); } + + return file_priv->bsd_ring; } static struct drm_i915_gem_object * @@ -1377,6 +1367,63 @@ eb_get_batch(struct eb_vmas *eb) return vma->obj; } +#define I915_USER_RINGS (4) + +static const enum intel_ring_id user_ring_map[I915_USER_RINGS + 1] = { + [I915_EXEC_DEFAULT] = RCS, + [I915_EXEC_RENDER] = RCS, + [I915_EXEC_BLT] = BCS, + [I915_EXEC_BSD] = VCS, + [I915_EXEC_VEBOX] = VECS +}; + +static int +eb_select_ring(struct drm_i915_private *dev_priv, + struct drm_file *file, + st
Re: [Intel-gfx] [PATCH 01/11] drm/i915: Add support for mapping an object page by page
On 15/01/16 11:44, Chris Wilson wrote: On Fri, Jan 15, 2016 at 09:53:46AM +, Tvrtko Ursulin wrote: On 14/01/16 13:34, Chris Wilson wrote: On Thu, Jan 14, 2016 at 10:32:11AM +, Tvrtko Ursulin wrote: diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index b448ad8..5f86596 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -317,6 +317,11 @@ struct i915_address_space { uint64_t start, uint64_t length, bool use_scratch); + void (*insert_page)(struct i915_address_space *vm, + dma_addr_t addr, + uint64_t offset, + enum i915_cache_level cache_level, + u32 flags); void (*insert_entries)(struct i915_address_space *vm, struct sg_table *st, uint64_t start, Why not extend the current API to support start page offset and number of pages? Could default to full object like today if zero. Eg: void (*insert_entries)(struct i915_address_space *vm, struct sg_table *st, + unsigned page_offset, + unsigned num_pages, Ouch. That would be quite slow for the insert_page() use case of page-by-page looping. It could use the same last page caching trick so why would be be so slow? We don't have the convenient struct? I presumed you were thinking of passing the offset through to sg_page_iter for_each_sg_page(st->sgl, &sg_iter, st->nents, page_offset) { if (--num_pages) break; /* original pt insertion code */ } Is it worth expanding struct sg_table for this shortcut? There are some games where we spend more time in sg_page_iter_next() than anything else in the kernel. The games have more textures than GTT space, they aren't playable right now nor will they ever likely to be. :( Improving these loops would definitely be a boon nevertheless. Okay, dead end, mission abort. :) Ankit please just sync the patch in next respin with the RPM changes which happened in the meantime. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/6] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (bdw-nuci7) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (bdw-ultra) Subgroup suspend-read-crc-pipe-a: pass -> SKIP (hsw-gt2) bdw-nuci7total:138 pass:129 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:131 dwarn:1 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:123 dwarn:3 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:136 dwarn:0 dfail:0 fail:0 skip:5 hsw-xps12total:138 pass:133 dwarn:1 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1197/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] tests: Add gem_busy
On Fri, Jan 15, 2016 at 02:45:49PM +, Tvrtko Ursulin wrote: > > On 15/01/16 13:53, Chris Wilson wrote: > >Exercise the busy-ioctl and verify it reports the right active engines > >using the execbuffer notation. > > > >Signed-off-by: Chris Wilson > >--- > > tests/Makefile.sources | 1 + > > tests/gem_busy.c | 233 > > + > > 2 files changed, 234 insertions(+) > > create mode 100644 tests/gem_busy.c > > > >diff --git a/tests/Makefile.sources b/tests/Makefile.sources > >index d2c7d6f..545aca0 100644 > >--- a/tests/Makefile.sources > >+++ b/tests/Makefile.sources > >@@ -11,6 +11,7 @@ TESTS_progs_M = \ > > drv_hangman \ > > gem_bad_reloc \ > > gem_basic \ > >+gem_busy \ > > gem_caching \ > > gem_close_race \ > > gem_concurrent_blit \ > >diff --git a/tests/gem_busy.c b/tests/gem_busy.c > >new file mode 100644 > >index 000..c6b8a8b > >--- /dev/null > >+++ b/tests/gem_busy.c > >@@ -0,0 +1,233 @@ > >+/* > >+ * Copyright © 2015 Intel Corporation > > 2016 Already? > >+__gem_busy(fd, object[0].handle, &read, &write); > >+igt_assert_eq(read, 1 << write); > >+igt_assert_eq(write, gen >= 6 ? I915_EXEC_BLT : I915_EXEC_RENDER); > > Will the blits be long enough for this to be stable? I can bump it by a factor of 5 easily enough, that should put it in the range of a few milliseconds even on a fast system. If we get rescheduled, we might miss the window for the test. I did think of asserting that the blit was still busy eash time. > >+/* Queue a batch after the busy, it should block and remain "busy" */ > >+igt_require(exec_noop(fd, handle, ring | flags, false)); > > Too bad we don't get ENODEV but a boring old EINVAL on !HAS_BSD2 - > this was this can be too skip happy if something else gets broken. > :( True, but it would be better to have a test that checked which rings the kernel said were on the hardware and that execbuffer supported them. > >+igt_main > >+{ > >+int fd = -1; > >+ > >+igt_skip_on_simulation(); > >+ > >+igt_fixture { > >+fd = drm_open_driver(DRIVER_INTEL); > >+igt_require(has_semaphores(fd)); > > Copy&paste, or? A requirement. At least on the current infrastructure. It uses the sync with the write on the BLT ring to block the GPU on the other rings, so that the nops don't complete too early. We would need a busy workload on each engine - code that we don't have yet. Without semaphores, and without deferred scheduling, we current wait for the busy workload to complete before scheduling on the next... If I lie about the busywork being a write target, we will get parallel execution rather than blocking and so not be able to set the right flags. I couldn't see a way around semaphores without gen specific new code. > Maybe mark as basic since it is ABI? Can't until the kernel abides by the abi we just concocted :) I definitely don't trust my judgement as to what is basic. Surviving weeks of stress testing is a basic requirement for us :) -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 v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6
On Fri, Jan 15, 2016 at 04:44:46PM +0200, Imre Deak wrote: > Adding Chris. We would need stolen_base and size early, could you check > if moving i915_gem_init_stolen() earlier based on the diff at the end > is ok? Had a quick discussion with Imre on irc, and yes this is fine. Though we could move gem_init_stolen even earlier, into i915_gem_gtt_init() or just after. I favour gtt_init since the stolen arena is closely coupled with the gtt probing in i915_gem_gtt_init(). Also intel_setup_mchbar() is semantically related to our dev_priv->regs = pci_iomap(), and I would suggest a new intel_setup_mmio() to do both, nice and early. -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 i-g-t] tests/kms_fbc_crc.c: No longer dependant on Cairo A setup function that used to use Cairo to draw 2 rectangles covering the whole screen has been changed to use igt_draw
From: Devon Davies tests/kms_frontbuffer_tracking.c: Now builds with DRM_PRIMARY_DISABLE Each call to the function drmModeSetPlane now has an addtional NULL in the arguments if DRM_PRIMARY_DISABLE is set. tests/Android.mk: Allow the above tests to be built without Cairo Simply removed them from the tests the be skipped. libs/igt_kms.c: Now builds with DRM_PRIMARY_DISABLE I had to define ffs as __builtin_fss due to compiler issues. Each call to the function drmModeSetPlane now has an addtional NULL in the arguments if DRM_PRIMARY_DISABLE is set. libs/igt_fb.c: Will now build some functions without Cairo Functions which aren't used by the framebuffer compression tests are now behind an #if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO libs/Android.mk igt_fb and igt_kms are no longer ignored if we don't have Cairo. The tests kms_fbc_crc and kms_frontbuffer_tracking had an unnecessary dependance on the Cairo graphics engine. Also, drmModeSetPlane may have an additional argument if DRM_PRIMARY_DISABLE is set (as it was for me), I have fixed that issue. Signed-off-by: Devon Davies --- lib/Android.mk | 4 -- lib/igt_fb.c | 26 - lib/igt_kms.c| 15 ++-- tests/Android.mk | 5 +++ tests/kms_fbc_crc.c | 20 ++ tests/kms_frontbuffer_tracking.c | 79 +--- 6 files changed, 119 insertions(+), 30 deletions(-) diff --git a/lib/Android.mk b/lib/Android.mk index badec1e..bbdb051 100644 --- a/lib/Android.mk +++ b/lib/Android.mk @@ -37,10 +37,6 @@ ifeq ("${ANDROID_HAS_CAIRO}", "1") LOCAL_C_INCLUDES += $(ANDROID_BUILD_TOP)/external/cairo-1.12.16/src LOCAL_CFLAGS += -DANDROID_HAS_CAIRO=1 -DIGT_DATADIR=\".\" -DIGT_SRCDIR=\".\" else -skip_lib_list := \ -igt_kms.c \ -igt_kms.h \ -igt_fb.c -DANDROID_HAS_CAIRO=0 endif diff --git a/lib/igt_fb.c b/lib/igt_fb.c index c985824..5acdaa7 100644 --- a/lib/igt_fb.c +++ b/lib/igt_fb.c @@ -33,6 +33,7 @@ #include "igt_fb.h" #include "ioctl_wrappers.h" + /** * SECTION:igt_fb * @short_description: Framebuffer handling and drawing library @@ -52,11 +53,23 @@ */ /* drm fourcc/cairo format maps */ +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO) + #define DF(did, cid, _bpp, _depth) \ { DRM_FORMAT_##did, CAIRO_FORMAT_##cid, # did, _bpp, _depth } + +#else + +#define DF(did, cid, _bpp, _depth) \ + { DRM_FORMAT_##did, # did, _bpp, _depth } + +#endif + static struct format_desc_struct { uint32_t drm_id; +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO) cairo_format_t cairo_id; +#endif const char *name; int bpp; int depth; @@ -72,7 +85,6 @@ static struct format_desc_struct { #define for_each_format(f) \ for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++) - /* helpers to create nice-looking framebuffers */ static int create_bo_for_fb(int fd, int width, int height, int bpp, uint64_t tiling, unsigned bo_size, @@ -125,6 +137,8 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp, return ret; } +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO) + /** * igt_paint_color: * @cr: cairo drawing context @@ -394,6 +408,7 @@ void igt_paint_image(cairo_t *cr, const char *filename, fclose(f); } +#endif /** * igt_create_fb_with_bo_size: @@ -494,6 +509,7 @@ unsigned int igt_create_fb(int fd, int width, int height, uint32_t format, return igt_create_fb_with_bo_size(fd, width, height, format, tiling, fb, 0, 0); } +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO) /** * igt_create_color_fb: @@ -985,6 +1001,7 @@ void igt_write_fb_to_png(int fd, struct igt_fb *fb, const char *filename) igt_assert(status == CAIRO_STATUS_SUCCESS); } +#endif /** * igt_remove_fb: @@ -997,10 +1014,13 @@ void igt_write_fb_to_png(int fd, struct igt_fb *fb, const char *filename) */ void igt_remove_fb(int fd, struct igt_fb *fb) { +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO) cairo_surface_destroy(fb->cairo_surface); +#endif do_or_die(drmModeRmFB(fd, fb->fb_id)); gem_close(fd, fb->gem_handle); } +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO) /** * igt_bpp_depth_to_drm_format: @@ -1024,6 +1044,8 @@ uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth) depth); } +#endif + /** * igt_drm_format_to_bpp: * @drm_format: drm fourcc pixel format code @@ -1062,6 +1084,7 @@ const char *igt_format_str(uint32_t drm_format) return "invalid"; } +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO) /** * igt_get_all_formats: @@ -1089,3 +1112,4 @@ void igt_get_
[Intel-gfx] ✗ Fi.CI.BAT: failure for MAINTAINERS: Add "B:" preferred bug reporting method
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_ctx_basic: pass -> FAIL (bdw-ultra) Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (skl-i7k-2) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_force_connector_basic: Subgroup prune-stale-modes: pass -> SKIP (hsw-gt2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-c: pass -> DMESG-WARN (bdw-ultra) bdw-nuci7total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:130 dwarn:1 dfail:0 fail:1 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:123 dwarn:3 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:136 dwarn:0 dfail:0 fail:0 skip:5 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1198/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tools/Android.mk: Add zlib support
Can this be merged so IGT on Android builds? No one has raise any objections since Monday about this patch. //Derek -Original Message- From: Morton, Derek J Sent: Monday, January 11, 2016 1:32 PM To: intel-gfx@lists.freedesktop.org Cc: Wood, Thomas; Gore, Tim; ch...@chris-wilson.co.uk; Morton, Derek J Subject: [PATCH] tools/Android.mk: Add zlib support IGT does not build for Android due to a zlib dependency being added to intel_error_decode.c in a recent patch. This patch fixes the error by updating the Android makefile to add the path to the zlib library and using any LDFLAGS specified in Makefile.sources. Signed-off-by: Derek Morton --- tools/Android.mk | 5 + 1 file changed, 5 insertions(+) diff --git a/tools/Android.mk b/tools/Android.mk index 934d3a1..da4f3c0 100644 --- a/tools/Android.mk +++ b/tools/Android.mk @@ -23,9 +23,14 @@ define add_tool LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. LOCAL_CFLAGS += -Wno-sign-compare +ifeq ($($(1)_LDFLAGS),) +else +LOCAL_LDFLAGS += $($(1)_LDFLAGS) +endif LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/PRIVATE/drm/include/drm +LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/zlib LOCAL_MODULE := $1_tool LOCAL_MODULE_TAGS := optional -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Decouple execbuf uAPI from internal implementation
On Fri, Jan 15, 2016 at 03:12:50PM +, Tvrtko Ursulin wrote: > +static int > +eb_select_ring(struct drm_i915_private *dev_priv, > +struct drm_file *file, > +struct drm_i915_gem_execbuffer2 *args, > +struct intel_engine_cs **ring) > +{ > + unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; > + > + if (user_ring_id > I915_USER_RINGS) { > + DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); > + return -EINVAL; > + } > + > + if ((user_ring_id != I915_EXEC_BSD) && > + ((args->flags & I915_EXEC_BSD_MASK) != 0)) { > + DRM_DEBUG("execbuf with non bsd ring but with invalid " > + "bsd dispatch flags: %d\n", (int)(args->flags)); Not your bug, but we should limit the flags reported here to (int)(args->flags & I915_EXEC_BSD_MASK). Though actually just nuke the test. At the moment, we complain for !BSD, then allow them even if we don't have BSD2 (and ignore the setting). A little inconsistent. If we just document that these flags only provide extra selection criteria for the I915_EXEC_BSD ring, we would be done. I'll pretend that it is adequately documented... Looks good and we completed our review of ABI impact for reordering the ring ids, so Reviewed-by: Chris Wilson -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] tools/Android.mk: Add zlib support
On Fri, Jan 15, 2016 at 03:49:00PM +, Morton, Derek J wrote: > Can this be merged so IGT on Android builds? No one has raise any > objections since Monday about this patch. Merged, thanks for the patch. -- Damien ___ 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: Cache LRC state page in the context
On Fri, Jan 15, 2016 at 03:10:29PM +, Tvrtko Ursulin wrote: > @@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct > drm_i915_gem_request *rq) > + if (--rq->ctx->engine[ring->id].pin_count == 0) { > + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, > + LRC_STATE_PN); Interesting choice. We called set_page_dirty() when we took the mapping. Should that page flag be preserved whilst we hold the kmap - I think so, i.e. the mm cannot flush the page whilst it has an elevated mapcount. So calling set_page_dirty() again is redundant, right? -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] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On 15/01/16 13:57, Chris Wilson wrote: On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote: Looks like your DDX is the only user not using it in the boolean mode? As far as I am aware, that is the only user that worries about which engine the object is currently active on. And libdrm is a bit confused in its return statements: ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); if (ret == 0) { bo_gem->idle = !busy.busy; return busy.busy; } else { return false; } return (ret == 0 && busy.busy); Looks like it was a boolean as well until commit 02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident started exposing the bits. Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch postdates when we started storing read/write bits in the return value. So definitely an unintentional leakage. In that case I think just respin with comment corrections in uapi header for drm_i915_gem_busy? Regards, Tvrtko ___ 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: Cache LRC state page in the context
On 15/01/16 16:01, Chris Wilson wrote: On Fri, Jan 15, 2016 at 03:10:29PM +, Tvrtko Ursulin wrote: @@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct drm_i915_gem_request *rq) + if (--rq->ctx->engine[ring->id].pin_count == 0) { + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, + LRC_STATE_PN); Interesting choice. We called set_page_dirty() when we took the mapping. Should that page flag be preserved whilst we hold the kmap - I think so, i.e. the mm cannot flush the page whilst it has an elevated mapcount. So calling set_page_dirty() again is redundant, right? If you call mindless copy & paste interesting. :D Any other concerns or I can respin with that only? Regards, Tvrtko ___ 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: Cache LRC state page in the context
On Fri, Jan 15, 2016 at 04:05:24PM +, Tvrtko Ursulin wrote: > > On 15/01/16 16:01, Chris Wilson wrote: > >On Fri, Jan 15, 2016 at 03:10:29PM +, Tvrtko Ursulin wrote: > >>@@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct > >>drm_i915_gem_request *rq) > >>+ if (--rq->ctx->engine[ring->id].pin_count == 0) { > >>+ lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, > >>+ LRC_STATE_PN); > > > >Interesting choice. We called set_page_dirty() when we took the mapping. > >Should that page flag be preserved whilst we hold the kmap - I think so, > >i.e. the mm cannot flush the page whilst it has an elevated mapcount. So > >calling set_page_dirty() again is redundant, right? > > If you call mindless copy & paste interesting. :D > > Any other concerns or I can respin with that only? No. I was quibbling over the excess clearing of state on unpinning :) Pity we have to respin even for innoculous changes just to get a CI tick. -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] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Do not call API requiring struct_mutex where it is not available
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (skl-i7k-2) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-b: pass -> DMESG-WARN (ilk-hp8440p) bdw-nuci7total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:132 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:123 dwarn:3 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 ilk-hp8440p total:141 pass:100 dwarn:4 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a Results at /archive/results/CI_IGT_test/Patchwork_1199/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On 15/01/16 16:19, Chris Wilson wrote: On Fri, Jan 15, 2016 at 04:02:52PM +, Tvrtko Ursulin wrote: On 15/01/16 13:57, Chris Wilson wrote: On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote: Looks like your DDX is the only user not using it in the boolean mode? As far as I am aware, that is the only user that worries about which engine the object is currently active on. And libdrm is a bit confused in its return statements: ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); if (ret == 0) { bo_gem->idle = !busy.busy; return busy.busy; } else { return false; } return (ret == 0 && busy.busy); Looks like it was a boolean as well until commit 02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident started exposing the bits. Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch postdates when we started storing read/write bits in the return value. So definitely an unintentional leakage. In that case I think just respin with comment corrections in uapi header for drm_i915_gem_busy? /** Return busy status * * A return of 0 implies that the object is idle (after * having flushed any pending activity), and a non-zero return that * the object is still in-flight on the GPU. (The GPU has not yet * signaled completion for all pending requests that reference the * object.) * * The returned dword is split into two fields to indicate both * the engines on which the object is being read, and the * engine on which is currently being writtern to (if any). * * The low word (bits 0:15) indicate if the object is being written * to by any engine (there can only be one, as the GEM implicit * synchronisation rules force writes to be serialised.) Only the * engine for last write is reported. * * The high word (bits 16:31) are a bitmask of which engines are * currently reading from the object. * * The value of each engine is the same as specified in the * EXECBUFFER2 ioctl, i.e. I915_EXEC_RENDER, I915_EXEC_BSD etc. * (Note I915_EXEC_DEFAULT is a symbolic value and is mapped to * the I915_EXEC_RENDER engine for execution, and so never reported * as active itself.) */ Very good, r-b on the resulting patch. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On Fri, Jan 15, 2016 at 04:02:52PM +, Tvrtko Ursulin wrote: > > On 15/01/16 13:57, Chris Wilson wrote: > >On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote: > >>Looks like your DDX is the only user not using it in the boolean mode? > > > >As far as I am aware, that is the only user that worries about which > >engine the object is currently active on. > > > >>And libdrm is a bit confused in its return statements: > >> > >> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); > >> if (ret == 0) { > >> bo_gem->idle = !busy.busy; > >> return busy.busy; > >> } else { > >> return false; > >> } > >> return (ret == 0 && busy.busy); > >> > >>Looks like it was a boolean as well until commit > >>02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident > >>started exposing the bits. > > > >Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch > >postdates when we started storing read/write bits in the return value. > >So definitely an unintentional leakage. > > In that case I think just respin with comment corrections in uapi > header for drm_i915_gem_busy? /** Return busy status * * A return of 0 implies that the object is idle (after * having flushed any pending activity), and a non-zero return that * the object is still in-flight on the GPU. (The GPU has not yet * signaled completion for all pending requests that reference the * object.) * * The returned dword is split into two fields to indicate both * the engines on which the object is being read, and the * engine on which is currently being writtern to (if any). * * The low word (bits 0:15) indicate if the object is being written * to by any engine (there can only be one, as the GEM implicit * synchronisation rules force writes to be serialised.) Only the * engine for last write is reported. * * The high word (bits 16:31) are a bitmask of which engines are * currently reading from the object. * * The value of each engine is the same as specified in the * EXECBUFFER2 ioctl, i.e. I915_EXEC_RENDER, I915_EXEC_BSD etc. * (Note I915_EXEC_DEFAULT is a symbolic value and is mapped to * the I915_EXEC_RENDER engine for execution, and so never reported * as active itself.) */ -- 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 v5] drm/i915: Cache LRC state page in the context
From: Tvrtko Ursulin LRC lifetime is well defined so we can cache the page pointing to the object backing store in the context in order to avoid walking over the object SG page list from the interrupt context without the big lock held. v2: Also cache the mapping. (Chris Wilson) v3: Unmap on the error path. v4: No need to cache the page. (Chris Wilson) v5: No need to dirty the page on unpin. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson Cc: Dave Gordon --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_lrc.c | 53 ++-- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index acff98b9c148..af301482e6f8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -890,6 +890,7 @@ struct intel_context { int pin_count; struct i915_vma *lrc_vma; u64 lrc_desc; + uint32_t *lrc_reg_state; } engine[I915_NUM_RINGS]; struct list_head link; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 545173c1b5c5..dc3ea03a887d 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -390,14 +390,7 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) { struct intel_engine_cs *ring = rq->ring; struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt; - struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; - struct page *page; - uint32_t *reg_state; - - BUG_ON(!ctx_obj); - - page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN); - reg_state = kmap_atomic(page); + uint32_t *reg_state = rq->ctx->engine[ring->id].lrc_reg_state; reg_state[CTX_RING_TAIL+1] = rq->tail; reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start; @@ -414,8 +407,6 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) ASSIGN_CTX_PDP(ppgtt, reg_state, 0); } - kunmap_atomic(reg_state); - return 0; } @@ -1067,6 +1058,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state; struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; + struct page *lrc_state_page; + uint32_t *lrc_reg_state; int ret; WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); @@ -1076,12 +1069,25 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, if (ret) return ret; + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN); + if (WARN_ON(!lrc_state_page)) { + ret = -ENODEV; + goto unpin_ctx_obj; + } + + lrc_reg_state = kmap(lrc_state_page); + if (!lrc_reg_state) { + ret = -ENOMEM; + goto unpin_ctx_obj; + } + ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf); if (ret) - goto unpin_ctx_obj; + goto unmap_state_page; ctx->engine[ring->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj); intel_lr_context_descriptor_update(ctx, ring); + ctx->engine[ring->id].lrc_reg_state = lrc_reg_state; ctx_obj->dirty = true; /* Invalidate GuC TLB. */ @@ -1090,6 +1096,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, return ret; +unmap_state_page: + kunmap(lrc_state_page); unpin_ctx_obj: i915_gem_object_ggtt_unpin(ctx_obj); @@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct drm_i915_gem_request *rq) struct intel_engine_cs *ring = rq->ring; struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; struct intel_ringbuffer *ringbuf = rq->ringbuf; + struct page *lrc_state_page; - if (ctx_obj) { - WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); - if (--rq->ctx->engine[ring->id].pin_count == 0) { - intel_unpin_ringbuffer_obj(ringbuf); - i915_gem_object_ggtt_unpin(ctx_obj); - rq->ctx->engine[ring->id].lrc_vma = NULL; - rq->ctx->engine[ring->id].lrc_desc = 0; - } + WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); + + if (!ctx_obj) + return; + + if (--rq->ctx->engine[ring->id].pin_count == 0) { + lrc_state_page = i915_gem_object_get_page(ctx_obj, + LRC_STATE_PN); + kunmap(lrc_state_page); + intel_unpin_ringbuffer_obj(ringbuf); + i915_gem_object_ggtt_unpin(ctx_obj); + rq->ctx->engine[ring->id
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Only grab timestamps when needed
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (bdw-ultra) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (byt-nuc) bdw-nuci7total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:131 dwarn:1 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1200/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
Tvrtko was looking through the execbuffer-ioctl and noticed that the uABI was tightly coupled to our internal engine identifiers. Close inspection also revealed that we leak those internal engine identifiers through the busy-ioctl, and those internal identifiers already do not match the user identifiers. Fortuitiously, there is only one user of the set of busy rings from the busy-ioctl, and they only wish to choose between the RENDER and the BLT engines. Let's fix the userspace ABI while we still can. v2: Update the uAPI documentation to explain the identifiers. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c | 18 ++ drivers/gpu/drm/i915/intel_lrc.c| 5 + drivers/gpu/drm/i915/intel_ringbuffer.c | 5 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + include/uapi/drm/i915_drm.h | 33 + 5 files changed, 54 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bb44bad15403..85797813a3de 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, if (ret) goto unref; - BUILD_BUG_ON(I915_NUM_RINGS > 16); - args->busy = obj->active << 16; - if (obj->last_write_req) - args->busy |= obj->last_write_req->ring->id; + args->busy = 0; + if (obj->active) { + int i; + + for (i = 0; i < I915_NUM_RINGS; i++) { + struct drm_i915_gem_request *req; + + req = obj->last_read_req[i]; + if (req) + args->busy |= 1 << (16 + req->ring->exec_id); + } + if (obj->last_write_req) + args->busy |= obj->last_write_req->ring->exec_id; + } unref: drm_gem_object_unreference(&obj->base); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f5d89c845ede..4aa209483237 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device *dev) ring->name = "render ring"; ring->id = RCS; + ring->exec_id = I915_EXEC_RENDER; ring->mmio_base = RENDER_RING_BASE; logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT); @@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev) ring->name = "bsd ring"; ring->id = VCS; + ring->exec_id = I915_EXEC_BSD; ring->mmio_base = GEN6_BSD_RING_BASE; logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT); @@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev) ring->name = "bsd2 ring"; ring->id = VCS2; + ring->exec_id = I915_EXEC_BSD; ring->mmio_base = GEN8_BSD2_RING_BASE; logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT); @@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev) ring->name = "blitter ring"; ring->id = BCS; + ring->exec_id = I915_EXEC_BLT; ring->mmio_base = BLT_RING_BASE; logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT); @@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device *dev) ring->name = "video enhancement ring"; ring->id = VECS; + ring->exec_id = I915_EXEC_VEBOX; ring->mmio_base = VEBOX_RING_BASE; logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 8cd8aabcc3ff..310d151c0db2 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring->name = "render ring"; ring->id = RCS; + ring->exec_id = I915_EXEC_RENDER; ring->mmio_base = RENDER_RING_BASE; if (INTEL_INFO(dev)->gen >= 8) { @@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) ring->name = "bsd ring"; ring->id = VCS; + ring->exec_id = I915_EXEC_BSD; ring->write_tail = ring_write_tail; if (INTEL_INFO(dev)->gen >= 6) { @@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev) ring->name = "bsd2 ring"; ring->id = VCS2; + ring->exec_id = I915_EXEC_BSD; ring->write_tail = ring_write_tail; ring->mmio_base = GEN8_BSD2_RING_BASE; @@ -2934,6 +2937,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) ring->name = "blitter ring"; ring->id = BCS; + ring->exec_id = I915_EXEC_BLT; ring->mmio_base = BLT_RING_BASE; r
Re: [Intel-gfx] [PATCH v5] drm/i915: Cache LRC state page in the context
On Fri, Jan 15, 2016 at 04:39:29PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > LRC lifetime is well defined so we can cache the page pointing > to the object backing store in the context in order to avoid > walking over the object SG page list from the interrupt context > without the big lock held. > > v2: Also cache the mapping. (Chris Wilson) > v3: Unmap on the error path. > v4: No need to cache the page. (Chris Wilson) > v5: No need to dirty the page on unpin. (Chris Wilson) Sorry. > @@ -1076,12 +1069,25 @@ static int intel_lr_context_do_pin(struct > intel_engine_cs *ring, > if (ret) > return ret; > > + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN); > + if (WARN_ON(!lrc_state_page)) { > + ret = -ENODEV; > + goto unpin_ctx_obj; > + } > + > + lrc_reg_state = kmap(lrc_state_page); kmap() cannot fail. > + if (!lrc_reg_state) { > + ret = -ENOMEM; > + goto unpin_ctx_obj; > + } > + if (--rq->ctx->engine[ring->id].pin_count == 0) { I was wondering if we should just use kunmap(kmap_to_page(ce->lrc_reg_state)); Except until we get struct intel_context_engine that line will exceed 80cols! -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 v2] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On Fri, Jan 15, 2016 at 04:51:46PM +, Chris Wilson wrote: > Tvrtko was looking through the execbuffer-ioctl and noticed that the > uABI was tightly coupled to our internal engine identifiers. Close > inspection also revealed that we leak those internal engine identifiers > through the busy-ioctl, and those internal identifiers already do not > match the user identifiers. Fortuitiously, there is only one user of the > set of busy rings from the busy-ioctl, and they only wish to choose > between the RENDER and the BLT engines. > > Let's fix the userspace ABI while we still can. > > v2: Update the uAPI documentation to explain the identifiers. Testcase: igt/gem_busy > Signed-off-by: Chris Wilson > Cc: Tvrtko Ursulin > Cc: Daniel Vetter > Reviewed-by: Tvrtko Ursulin -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 v6] drm/i915: Cache LRC state page in the context
From: Tvrtko Ursulin LRC lifetime is well defined so we can cache the page pointing to the object backing store in the context in order to avoid walking over the object SG page list from the interrupt context without the big lock held. v2: Also cache the mapping. (Chris Wilson) v3: Unmap on the error path. v4: No need to cache the page. (Chris Wilson) v5: No need to dirty the page on unpin. (Chris Wilson) v6: kmap() cannot fail and use kmap_to_page to simplify unpin. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson Cc: Dave Gordon --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_lrc.c | 39 +-- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index acff98b9c148..af301482e6f8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -890,6 +890,7 @@ struct intel_context { int pin_count; struct i915_vma *lrc_vma; u64 lrc_desc; + uint32_t *lrc_reg_state; } engine[I915_NUM_RINGS]; struct list_head link; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 545173c1b5c5..285e7f26760c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -390,14 +390,7 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) { struct intel_engine_cs *ring = rq->ring; struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt; - struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; - struct page *page; - uint32_t *reg_state; - - BUG_ON(!ctx_obj); - - page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN); - reg_state = kmap_atomic(page); + uint32_t *reg_state = rq->ctx->engine[ring->id].lrc_reg_state; reg_state[CTX_RING_TAIL+1] = rq->tail; reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start; @@ -414,8 +407,6 @@ static int execlists_update_context(struct drm_i915_gem_request *rq) ASSIGN_CTX_PDP(ppgtt, reg_state, 0); } - kunmap_atomic(reg_state); - return 0; } @@ -1067,6 +1058,7 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state; struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; + struct page *lrc_state_page; int ret; WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); @@ -1076,12 +1068,19 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring, if (ret) return ret; + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN); + if (WARN_ON(!lrc_state_page)) { + ret = -ENODEV; + goto unpin_ctx_obj; + } + ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf); if (ret) goto unpin_ctx_obj; ctx->engine[ring->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj); intel_lr_context_descriptor_update(ctx, ring); + ctx->engine[ring->id].lrc_reg_state = kmap(lrc_state_page); ctx_obj->dirty = true; /* Invalidate GuC TLB. */ @@ -1119,14 +1118,18 @@ void intel_lr_context_unpin(struct drm_i915_gem_request *rq) struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state; struct intel_ringbuffer *ringbuf = rq->ringbuf; - if (ctx_obj) { - WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); - if (--rq->ctx->engine[ring->id].pin_count == 0) { - intel_unpin_ringbuffer_obj(ringbuf); - i915_gem_object_ggtt_unpin(ctx_obj); - rq->ctx->engine[ring->id].lrc_vma = NULL; - rq->ctx->engine[ring->id].lrc_desc = 0; - } + WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex)); + + if (!ctx_obj) + return; + + if (--rq->ctx->engine[ring->id].pin_count == 0) { + kunmap(kmap_to_page(rq->ctx->engine[ring->id].lrc_reg_state)); + intel_unpin_ringbuffer_obj(ringbuf); + i915_gem_object_ggtt_unpin(ctx_obj); + rq->ctx->engine[ring->id].lrc_vma = NULL; + rq->ctx->engine[ring->id].lrc_desc = 0; + rq->ctx->engine[ring->id].lrc_reg_state = NULL; } } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Decouple execbuf uAPI from internal implementation (rev2)
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (skl-i5k-2) UNSTABLE pass -> DMESG-WARN (skl-i7k-2) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (byt-nuc) bdw-nuci7total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:132 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a Results at /archive/results/CI_IGT_test/Patchwork_1201/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids (rev2)
== Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (skl-i5k-2) UNSTABLE dmesg-warn -> PASS (bdw-nuci7) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a-frame-sequence: pass -> FAIL (snb-x220t) Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (byt-nuc) bdw-nuci7total:138 pass:129 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:132 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:121 dwarn:5 dfail:0 fail:2 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1202/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+
Chris Wilson writes: > In order to ensure seqno/irq coherency, we current read a ring register. > We are not sure quite how it works, only that is does. Experiments show > that e.g. doing a clflush(seqno) instead is not sufficient, but we can > remove the forcewake dance from the mmio access. > > v2: Baytrail wants a clflush too. > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Mika Kuoppala > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 8cd8aabcc3ff..935add1422ae 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1492,10 +1492,21 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring, > bool lazy_coherency) > { > /* Workaround to force correct ordering between irq and seqno writes on >* ivb (and maybe also on snb) by reading from a CS register (like > - * ACTHD) before reading the status page. */ > + * ACTHD) before reading the status page. > + * > + * Note that this effectively effectively stalls the read by the > time effectively twice there. Reviewed-by: Mika Kuoppala > + * it takes to do a memory transaction, which more or less ensures > + * that the write from the GPU has sufficient time to invalidate > + * the CPU cacheline. Alternatively we could delay the interrupt from > + * the CS ring to give the write time to land, but that would incur > + * a delay after every batch i.e. much more frequent than a delay > + * when waiting for the interrupt (with the same net latency). > + */ > if (!lazy_coherency) { > struct drm_i915_private *dev_priv = ring->dev->dev_private; > - POSTING_READ(RING_ACTHD(ring->mmio_base)); > + POSTING_READ_FW(RING_ACTHD(ring->mmio_base)); > + > + intel_flush_status_page(ring, I915_GEM_HWS_INDEX); > } > > return intel_read_status_page(ring, I915_GEM_HWS_INDEX); > -- > 2.7.0.rc3 > > ___ > 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] drm/i915: Don't reject primayr plane windowing with color keying enabled on SKL+
From: Ville Syrjälä On SKL+ plane scaling is mutually exclusive with color keying. The code check for this, but during some refactoring the code got changed to also reject primary plane windowing when color keying is used. There is no such restriction in the hardware, so restore the original logic. Cc: Maarten Lankhorst Fixes: 061e4b8d650a ("drm/i915: clean up atomic plane check functions, v2.") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a726c5e91220..7bc56e217ecc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14019,11 +14019,12 @@ intel_check_primary_plane(struct drm_plane *plane, int max_scale = DRM_PLANE_HELPER_NO_SCALING; bool can_position = false; - /* use scaler when colorkey is not required */ - if (INTEL_INFO(plane->dev)->gen >= 9 && - state->ckey.flags == I915_SET_COLORKEY_NONE) { - min_scale = 1; - max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state); + if (INTEL_INFO(plane->dev)->gen >= 9) { + /* use scaler when colorkey is not required */ + if (state->ckey.flags == I915_SET_COLORKEY_NONE) { + min_scale = 1; + max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state); + } can_position = true; } -- 2.4.10 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: skl_update_scaler() wants a rotation bitmask instead of bit number
On Thu, Oct 15, 2015 at 05:01:58PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Pass BIT(DRM_ROTATE_0) instead of DRM_ROTATE_0 to skl_update_scaler(). > The former is a mask, the latter just the bit number. > > Fortunately the only thing skl_update_scaler() does with the rotation > is check if it's 90/270 degrees or not, and so in this case it would > still do the right thing. > > Cc: Chandra Konduru > Signed-off-by: Ville Syrjälä Ping, anyone care to r-b this one? > --- > 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 7498c9d..02316d0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4673,7 +4673,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state > *state) > intel_crtc->base.base.id, intel_crtc->pipe, > SKL_CRTC_INDEX); > > return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX, > - &state->scaler_state.scaler_id, DRM_ROTATE_0, > + &state->scaler_state.scaler_id, BIT(DRM_ROTATE_0), > state->pipe_src_w, state->pipe_src_h, > adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay); > } > -- > 2.4.9 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use the active wm config for merging on ILK-BDW
On Thu, Jan 14, 2016 at 06:37:32PM -0800, Matt Roper wrote: > On Thu, Jan 14, 2016 at 02:53:35PM +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > ilk_program_watermarks() is supposed to merge the active watermarks from > > all pipes. Thus we need to use the active config too instead of some > > precomputed stuff. > > So to clarify, the bug you're fixing here would be if we have racing > commits that operate on disjoint sets of CRTC's; in that case the second > one that actually gets into wm_mutex will fail to see the config changes > made by the first commit, right? That, or I suppose cases where the intermediate and optimal watermarks have a different idea about the sprite enabled/scaled flags. I assume what it was doing is picking those flags always based on the optimal wms even when programming the intermedidate wms. > > Seems like we could go ahead and remove dev_priv->wm.config (and > calc_watermark_data() that builds it) since it's not actually doing us > any good. Although it's probably fine to hold that off to a subsequent > patch. IIRC I was a reference to that stuff in the SKL code too, so I didn't bother trying to kill it without knowing what it was doing. And I was too lazy to take a deeper look at the code right now. > > Both of your patches are > > Reviewed-by: Matt Roper > > CI results do report SKL failures, but those are clearly bogus since SKL > doesn't even run any of the functions that you're changing in these two > patches; as you noted, it looks more like the machine had some kind of > bizarre hardware failure that was unrelated to the patchset. The other > results look clean. Given that, I've gone ahead and pushed your patches > to dinq. Thanks! Cheers. > > > Matt > > > > > > Fixes: aa363136866c ("drm/i915: Calculate watermark configuration during > > atomic check (v2)") > > Cc: Matt Roper > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_pm.c | 32 ++-- > > 1 file changed, 26 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index e9f4e6e7b736..f44a961183d7 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3680,23 +3680,43 @@ static void skl_update_wm(struct drm_crtc *crtc) > > dev_priv->wm.skl_hw = *results; > > } > > > > +static void ilk_compute_wm_config(struct drm_device *dev, > > + struct intel_wm_config *config) > > +{ > > + struct intel_crtc *crtc; > > + > > + /* Compute the currently _active_ config */ > > + for_each_intel_crtc(dev, crtc) { > > + const struct intel_pipe_wm *wm = &crtc->wm.active.ilk; > > + > > + if (!wm->pipe_enabled) > > + continue; > > + > > + config->sprites_enabled |= wm->sprites_enabled; > > + config->sprites_scaled |= wm->sprites_scaled; > > + config->num_pipes_active++; > > + } > > +} > > + > > static void ilk_program_watermarks(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = dev_priv->dev; > > struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm; > > struct ilk_wm_maximums max; > > - struct intel_wm_config *config = &dev_priv->wm.config; > > + struct intel_wm_config config = {}; > > struct ilk_wm_values results = {}; > > enum intel_ddb_partitioning partitioning; > > > > - ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max); > > - ilk_wm_merge(dev, config, &max, &lp_wm_1_2); > > + ilk_compute_wm_config(dev, &config); > > + > > + ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2, &max); > > + ilk_wm_merge(dev, &config, &max, &lp_wm_1_2); > > > > /* 5/6 split only in single pipe config on IVB+ */ > > if (INTEL_INFO(dev)->gen >= 7 && > > - config->num_pipes_active == 1 && config->sprites_enabled) { > > - ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_5_6, > > &max); > > - ilk_wm_merge(dev, config, &max, &lp_wm_5_6); > > + config.num_pipes_active == 1 && config.sprites_enabled) { > > + ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_5_6, > > &max); > > + ilk_wm_merge(dev, &config, &max, &lp_wm_5_6); > > > > best_lp_wm = ilk_find_best_result(dev, &lp_wm_1_2, &lp_wm_5_6); > > } else { > > -- > > 2.4.10 > > > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/8] drm/i915: Some more fb offsets[] prep stuff
On Thu, Jan 14, 2016 at 03:22:08PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Yanked a few more prep patches from my earlier fb offsets[] work [1]. First > few have r-bs, the rest don't. > > The last two patches are new. Things just tickled my OCD a bit too much so > had to deal with them. > > [1] http://lists.freedesktop.org/archives/intel-gfx/2015-October/078050.html > > Ville Syrjälä (8): > drm/i915: Don't leak framebuffer_references if drm_framebuffer_init() > fails > drm/i915: Set i915_ggtt_view_normal type explicitly > drm/i915: Pass the dma_addr_t array as const to rotate_pages() These first three had r-bs already, so I went and pushed them to dinq. Thanks for the reviews. > drm/i915: Rename the rotated gtt view member to 'rotated' > drm/i915: Pass stride to rotate_pages() > drm/i915: Pass rotation_info to intel_rotate_fb_obj_pages() > drm/i915: Make display gtt offsets u32 > drm/i915: Standardize on 'cpp' for bytes per pixel > > drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +++--- > drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 49 +++--- > drivers/gpu/drm/i915/intel_drv.h | 12 ++-- > drivers/gpu/drm/i915/intel_pm.c | 128 > +-- > drivers/gpu/drm/i915/intel_sprite.c | 40 +-- > 6 files changed, 123 insertions(+), 130 deletions(-) > > -- > 2.4.10 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6] drm/i915: Cache LRC state page in the context
On Fri, Jan 15, 2016 at 05:12:45PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > LRC lifetime is well defined so we can cache the page pointing > to the object backing store in the context in order to avoid > walking over the object SG page list from the interrupt context > without the big lock held. > > v2: Also cache the mapping. (Chris Wilson) > v3: Unmap on the error path. > v4: No need to cache the page. (Chris Wilson) > v5: No need to dirty the page on unpin. (Chris Wilson) > v6: kmap() cannot fail and use kmap_to_page to simplify unpin. > (Chris Wilson) > > Signed-off-by: Tvrtko Ursulin > Cc: Chris Wilson > Cc: Dave Gordon Reviewed-by: Chris Wilson -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] drm/i915: Don't do pre plane update on disabled crtcs
On Thu, Jan 14, 2016 at 06:32:10PM +0200, Mika Kuoppala wrote: > CI/Bat got following (shortened) trace on byt and also > on bsw: > > [ cut here ]--- > Unclaimed register detected before reading register 0x186500 > Call Trace: > __unclaimed_reg_debug+0x68/0x80 [i915] > vlv_read32+0x2de/0x370 [i915] > intel_set_memory_cxsr+0x87/0x1a0 [i915] > intel_pre_plane_update+0xb3/0xf0 [i915] > intel_atomic_commit+0x3b5/0x17c0 [i915] > ... > ---[ end trace 6387a0ad001bb39f ]--- > > Fix this by limiting pre plane update only to active crtcs. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=93698 > Cc: Ville Syrjälä > Cc: Maarten Lankhorst > Signed-off-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/intel_display.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index aa24f79d85bf..a134a698d97d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13580,9 +13580,8 @@ static int intel_atomic_commit(struct drm_device *dev, > if (!needs_modeset(crtc->state)) > continue; > > - intel_pre_plane_update(intel_crtc); > - > if (crtc_state->active) { > + intel_pre_plane_update(intel_crtc); Won't this change prevent us from setting up watermarks before turning on the CRTC in a disabled->enabled transition? Matt > intel_crtc_disable_planes(crtc, crtc_state->plane_mask); > dev_priv->display.crtc_disable(crtc); > intel_crtc->active = false; > -- > 2.5.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: skl_update_scaler() wants a rotation bitmask instead of bit number
On Fri, Jan 15, 2016 at 08:48:26PM +0200, Ville Syrjälä wrote: > On Thu, Oct 15, 2015 at 05:01:58PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Pass BIT(DRM_ROTATE_0) instead of DRM_ROTATE_0 to skl_update_scaler(). > > The former is a mask, the latter just the bit number. > > > > Fortunately the only thing skl_update_scaler() does with the rotation > > is check if it's 90/270 degrees or not, and so in this case it would > > still do the right thing. > > > > Cc: Chandra Konduru > > Signed-off-by: Ville Syrjälä > > Ping, anyone care to r-b this one? Reviewed-by: Matt Roper Looks like this bug has been present since scalers were first added in 6156a45602f9 ("drm/i915: skylake primary plane scaling using shared scalers") Matt > > > --- > > 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 7498c9d..02316d0 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -4673,7 +4673,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state > > *state) > > intel_crtc->base.base.id, intel_crtc->pipe, > > SKL_CRTC_INDEX); > > > > return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX, > > - &state->scaler_state.scaler_id, DRM_ROTATE_0, > > + &state->scaler_state.scaler_id, BIT(DRM_ROTATE_0), > > state->pipe_src_w, state->pipe_src_h, > > adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay); > > } > > -- > > 2.4.9 > > -- > Ville Syrjälä > Intel OTC > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't reject primayr plane windowing with color keying enabled on SKL+
On Fri, Jan 15, 2016 at 08:46:53PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > On SKL+ plane scaling is mutually exclusive with color keying. The code > check for this, but during some refactoring the code got changed to > also reject primary plane windowing when color keying is used. There is > no such restriction in the hardware, so restore the original logic. > > Cc: Maarten Lankhorst > Fixes: 061e4b8d650a ("drm/i915: clean up atomic plane check functions, v2.") > Signed-off-by: Ville Syrjälä s/primayr/primary in your headline, but otherwise Reviewed-by: Matt Roper Still waiting for CI to run the BAT on this... Matt > --- > drivers/gpu/drm/i915/intel_display.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index a726c5e91220..7bc56e217ecc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14019,11 +14019,12 @@ intel_check_primary_plane(struct drm_plane *plane, > int max_scale = DRM_PLANE_HELPER_NO_SCALING; > bool can_position = false; > > - /* use scaler when colorkey is not required */ > - if (INTEL_INFO(plane->dev)->gen >= 9 && > - state->ckey.flags == I915_SET_COLORKEY_NONE) { > - min_scale = 1; > - max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state); > + if (INTEL_INFO(plane->dev)->gen >= 9) { > + /* use scaler when colorkey is not required */ > + if (state->ckey.flags == I915_SET_COLORKEY_NONE) { > + min_scale = 1; > + max_scale = skl_max_scale(to_intel_crtc(crtc), > crtc_state); > + } > can_position = true; > } > > -- > 2.4.10 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] MAINTAINERS: Add "B:" preferred bug reporting method
On Fri, 2016-01-15 at 16:45 +0200, Jani Nikula wrote: > Different subsystems and drivers have different preferred ways of > receiving bug reports; mailing list or bugzillas at various > locations. Add "B:" entry for specifying the preference to guide bug > reporters at the right location. [] > diff --git a/MAINTAINERS b/MAINTAINERS [] > @@ -75,6 +75,8 @@ Descriptions of section entries: > L: Mailing list that is relevant to this area > W: Web-page with status/info > Q: Patchwork web based patch tracking system site > + B: Preferred method for reporting bugs; bug tracking system site > + or "List" for mailing list. > T: SCM tree type and location. > Type is one of: git, hg, quilt, stgit, topgit > S: Status, one of the following: > @@ -3655,6 +3657,7 @@ L: intel-gfx@lists.freedesktop.org > L: dri-de...@lists.freedesktop.org > W: https://01.org/linuxgraphics/ > Q: http://patchwork.freedesktop.org/project/intel-gfx/ > +B: > https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel This requires a LoginID & password Maybe useful to show the open bugs too: https://bugs.freedesktop.org/buglist.cgi?component=DRM%2FIntel&product=DRI&resolution=--- Maybe the get_maintainer script should be updated. Something like: (untested) --- scripts/get_maintainer.pl | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 1873421..bbe5337 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -47,6 +47,7 @@ my $output_rolestats = 1; my $output_section_maxlen = 50; my $scm = 0; my $web = 0; +my $bug = 0; my $subsystem = 0; my $status = 0; my $keywords = 1; @@ -239,6 +240,7 @@ if (!GetOptions( 'status!' => \$status, 'scm!' => \$scm, 'web!' => \$web, + 'bug!' => \$bug, 'pattern-depth=i' => \$pattern_depth, 'k|keywords!' => \$keywords, 'sections!' => \$sections, @@ -276,12 +278,13 @@ if ($sections) { $status = 0; $subsystem = 0; $web = 0; +$bug = 0; $keywords = 0; $interactive = 0; } else { -my $selections = $email + $scm + $status + $subsystem + $web; +my $selections = $email + $scm + $status + $subsystem + $web + $bug; if ($selections == 0) { - die "$P: Missing required option: email, scm, status, subsystem or web\n"; + die "$P: Missing required option: email, scm, status, subsystem, web or bug\n"; } } @@ -505,6 +508,7 @@ my %hash_list_to; my @list_to = (); my @scm = (); my @web = (); +my @bug = (); my @subsystem = (); my @status = (); my %deduplicate_name_hash = (); @@ -537,6 +541,11 @@ if ($web) { output(@web); } +if ($bug) { +@bug = uniq(@bug); +output(@bug); +} + exit($exit); sub ignore_email_address { @@ -593,6 +602,7 @@ sub get_maintainers { @list_to = (); @scm = (); @web = (); +@bug = (); @subsystem = (); @status = (); %deduplicate_name_hash = (); @@ -802,6 +812,7 @@ MAINTAINER field selection options: --status => print status if any --subsystem => print subsystem name if any --web => print website(s) if any + --bug => print bug reporting mechanism(s) if any Output type options: --separator [, ] => separator for multiple entries on 1 line @@ -1129,6 +1140,8 @@ sub add_categories { push(@scm, $pvalue); } elsif ($ptype eq "W") { push(@web, $pvalue); + } elsif ($ptype eq "B") { + push(@bug, $pvalue); } elsif ($ptype eq "S") { push(@status, $pvalue); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Do not call API requiring struct_mutex where it is not available (rev3)
== Summary == Built on 0ea9000bcb6f394edde5111494a92b0607214cfa drm-intel-nightly: 2016y-01m-15d-19h-09m-45s UTC integration manifest Test gem_ctx_basic: pass -> FAIL (bdw-ultra) Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (bdw-ultra) UNSTABLE Test kms_flip: Subgroup basic-flip-vs-wf_vblank: dmesg-warn -> PASS (snb-dellxps) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (snb-dellxps) Test pm_rpm: Subgroup basic-rte: dmesg-warn -> PASS (byt-nuc) UNSTABLE bdw-nuci7total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:131 dwarn:0 dfail:0 fail:1 skip:6 byt-nuc total:141 pass:124 dwarn:2 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1203/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx