Re: [Intel-gfx] [PATCH] drm/i915: Fix inconsistent naming of i915_guc_client parameter
On to, 2016-12-15 at 19:53 +, Michal Wajdeczko wrote: > We usually use 'client' as identifier for the i915_guc_client. > For unknown reason, few functions were using 'gc' name. > > Signed-off-by: Michal Wajdeczko Looks good, I'll merge this. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [bug report] drm/i915: Small compaction of the engine init code
On 15/12/2016 20:54, Chris Wilson wrote: On Thu, Dec 15, 2016 at 11:44:13PM +0300, Dan Carpenter wrote: Hello Tvrtko Ursulin, The patch a19d6ff29a82: "drm/i915: Small compaction of the engine init code" from Jun 23, 2016, leads to the following static checker warning: drivers/gpu/drm/i915/intel_lrc.c:1973 logical_render_ring_init() warn: passing freed memory 'engine' drivers/gpu/drm/i915/intel_lrc.c 1970 1971 ret = logical_ring_init(engine); 1972 if (ret) { 1973 lrc_destroy_wa_ctx_obj(engine); The problem is that logical_ring_init() frees "engine" on the error path so this is a use after free. And calls lrc_destroy_wa_ctx_obj() in the process. So we can diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 067394b0a769..1c1bad8ae7b0 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1940,12 +1940,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine) ret); } - ret = logical_ring_init(engine); - if (ret) { - lrc_destroy_wa_ctx_obj(engine); - } - - return ret; + return logical_ring_init(engine); } int logical_xcs_ring_init(struct intel_engine_cs *engine) I've marked this as TODO for later. Interesting that it was detected only now. Even more so, the referenced commit just moved the code around. I think that the real issue was introduced around "drm/i915/gen8: Re-order init pipe_control in lrc mode". Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
On 2016-12-14 11:13 AM, Brian Starkey wrote: Hi, On Wed, Dec 14, 2016 at 04:05:04AM -0500, Robert Foss wrote: From: Gustavo Padovan Add support for the OUT_FENCE_PTR property to enable setting out fences for atomic commits. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- lib/igt_kms.c | 21 - lib/igt_kms.h | 3 +++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 8ca49d86..fe1b356d 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -175,7 +175,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = { "DEGAMMA_LUT", "GAMMA_LUT", "MODE_ID", -"ACTIVE" +"ACTIVE", +"OUT_FENCE_PTR" }; const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = { @@ -2103,6 +2104,10 @@ static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE, !!output); } +if (pipe_obj->out_fence_ptr) +igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_OUT_FENCE_PTR, +(uint64_t)(uintptr_t) pipe_obj->out_fence_ptr); + /* *TODO: Add all crtc level properties here */ @@ -2683,6 +2688,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length) } /** + * igt_pipe_set_out_fence_ptr: + * @pipe: pipe pointer to which background color to be set + * @fence_ptr: out fence pointer + * + * Sets the out fence pointer that will be passed to the kernel in + * the atomic ioctl. When the kernel returns the out fence pointer + * will contain the fd number of the out fence created by KMS. + */ +void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr) +{ +pipe->out_fence_ptr = fence_ptr; +} + +/** * igt_crtc_set_background: * @pipe: pipe pointer to which background color to be set * @background: background color value in BGR 16bpc diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 9766807c..8eb611c0 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties { IGT_CRTC_GAMMA_LUT, IGT_CRTC_MODE_ID, IGT_CRTC_ACTIVE, + IGT_CRTC_OUT_FENCE_PTR, IGT_NUM_CRTC_PROPS }; @@ -316,6 +317,7 @@ struct igt_pipe { uint64_t mode_blob; bool mode_changed; +int64_t *out_fence_ptr; I prefer the interface that got suggested before - igt_pipe gets an "int64_t out_fence;" member which tests can query to get the fence value: int igt_pipe_get_last_out_fence(igt_pipe_t *pipe); ..and the kernel only ever receives a pointer to pipe->out_fence. The only reason I can see for a test to want to provide its own fence pointer is to test invalid pointer values - and I think it's OK for that test to set the property directly instead of making setting a custom fence pointer the common case for all users. If we don't want to get a fence for every commit then I guess there could be a way for a test to request an out-fence for just the next commit on a pipe (or the inverse - disable fencing for a particular commit): void igt_pipe_request_out_fence(igt_pipe_t *pipe); -Brian Now I see what you meant in the v1 discussion, I'll amend the implementation in v3 to be the one mentioned above. The only question I have is about igt_pipe_get_last_out_fence(), is it really necessary? I don't foresee a function like that ever doing more than just returning a struct member. Is it not a bit redundant? Rob. }; typedef struct { @@ -369,6 +371,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t *plane) void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length); void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length); void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length); +void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr); void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb); void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2 08/12] tests/kms_atomic: stress possible fence settings
On 2016-12-14 11:39 AM, Brian Starkey wrote: Hi, On Wed, Dec 14, 2016 at 04:05:05AM -0500, Robert Foss wrote: From: Gustavo Padovan Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- tests/kms_atomic.c | 186 ++--- 1 file changed, 176 insertions(+), 10 deletions(-) diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c index 8b648eba..a557ac60 100644 --- a/tests/kms_atomic.c +++ b/tests/kms_atomic.c @@ -44,6 +44,7 @@ #include "drmtest.h" #include "igt.h" #include "igt_aux.h" +#include "sw_sync.h" #ifndef DRM_CLIENT_CAP_ATOMIC #define DRM_CLIENT_CAP_ATOMIC 3 @@ -126,6 +127,7 @@ struct kms_atomic_plane_state { uint32_t fb_id; /* 0 to disable */ uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */ uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */ +int32_t fence_fd; }; struct kms_atomic_crtc_state { @@ -133,6 +135,7 @@ struct kms_atomic_crtc_state { uint32_t obj; int idx; bool active; +uint64_t out_fence_ptr; struct kms_atomic_blob mode; }; @@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig) crtc_populate_req(crtc, req); \ plane_populate_req(plane, req); \ do_atomic_commit((crtc)->state->desc->fd, req, flags); \ -crtc_check_current_state(crtc, plane, relax); \ -plane_check_current_state(plane, relax); \ +if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \ +crtc_check_current_state(crtc, plane, relax); \ +plane_check_current_state(plane, relax); \ +} \ } -#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, relax, e) { \ +#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, flags, relax, e) { \ drmModeAtomicSetCursor(req, 0); \ crtc_populate_req(crtc, req); \ plane_populate_req(plane, req); \ @@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state, static void plane_populate_req(struct kms_atomic_plane_state *plane, drmModeAtomicReq *req) { +if (plane->fence_fd) +plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD, plane->fence_fd); + plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id); plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id); plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x); @@ -424,6 +432,10 @@ find_plane(struct kms_atomic_state *state, enum plane_type type, static void crtc_populate_req(struct kms_atomic_crtc_state *crtc, drmModeAtomicReq *req) { +if (crtc->out_fence_ptr) +crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR, + crtc->out_fence_ptr); + crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id); crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active); } @@ -1052,6 +1064,37 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc, drmModeAtomicFree(req); } +static void plane_invalid_params_fence(struct kms_atomic_crtc_state *crtc, +struct kms_atomic_plane_state *plane_old, +struct kms_atomic_connector_state *conn) +{ +struct kms_atomic_plane_state plane = *plane_old; +drmModeAtomicReq *req = drmModeAtomicAlloc(); +int timeline, fence_fd; + +igt_require_sw_sync(); + +/* invalid fence fd */ +plane.fence_fd = plane.state->desc->fd; +plane.crtc_id = plane_old->crtc_id; +plane_commit_atomic_err(&plane, plane_old, req, +ATOMIC_RELAX_NONE, EINVAL); + +/* Valid fence_fd but invalid CRTC */ +timeline = sw_sync_timeline_create(); +fence_fd = sw_sync_fence_create(timeline, 1); +plane.fence_fd = fence_fd; +plane.crtc_id = ~0U; +plane_commit_atomic_err(&plane, plane_old, req, +ATOMIC_RELAX_NONE, EINVAL); + +plane.fence_fd = -1; +close(fence_fd); +close(timeline); + +drmModeAtomicFree(req); +} + static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, struct kms_atomic_plane_state *plane, struct kms_atomic_connector_state *conn) @@ -1063,30 +1106,32 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, /* Pass a series of invalid object IDs for the mode ID. */ crtc.mode.id = plane->obj; -crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, +crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); crtc.mode.id = crtc.obj; -crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, +crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); crtc.mode.id = conn->obj; -crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, +crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); crtc.mode.id = plane->fb_id; -crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req
Re: [Intel-gfx] [bug report] drm/i915: Small compaction of the engine init code
On 16/12/2016 08:02, Tvrtko Ursulin wrote: On 15/12/2016 20:54, Chris Wilson wrote: On Thu, Dec 15, 2016 at 11:44:13PM +0300, Dan Carpenter wrote: Hello Tvrtko Ursulin, The patch a19d6ff29a82: "drm/i915: Small compaction of the engine init code" from Jun 23, 2016, leads to the following static checker warning: drivers/gpu/drm/i915/intel_lrc.c:1973 logical_render_ring_init() warn: passing freed memory 'engine' drivers/gpu/drm/i915/intel_lrc.c 1970 1971 ret = logical_ring_init(engine); 1972 if (ret) { 1973 lrc_destroy_wa_ctx_obj(engine); The problem is that logical_ring_init() frees "engine" on the error path so this is a use after free. And calls lrc_destroy_wa_ctx_obj() in the process. So we can diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 067394b0a769..1c1bad8ae7b0 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1940,12 +1940,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine) ret); } - ret = logical_ring_init(engine); - if (ret) { - lrc_destroy_wa_ctx_obj(engine); - } - - return ret; + return logical_ring_init(engine); } int logical_xcs_ring_init(struct intel_engine_cs *engine) I've marked this as TODO for later. Interesting that it was detected only now. Even more so, the referenced commit just moved the code around. I think that the real issue was introduced around "drm/i915/gen8: Re-order init pipe_control in lrc mode". Actually it was probably the dynamic engine allocation patch. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v3] tests/kms_plane_lowres: Plane visibility after atomic modesets
Testcase for plane visibility after atomic modesets. The idea of the test is the following: - draw a blue screen with high resolution - enable a yellow plane, visible, in lower-left corner - set a new lower resolution mode (1024x768) that makes plane invisible - check from debugfs 'i915_display_info' that the plane is invisible - switch back to higher resolution mode - check from debugfs 'i915_display_info' that the plane is visible again - repeat number of iterations, default 64 v2: allow test to be run on non-Intel drivers (Daniel) moved test for plane visibility to as helper function (Daniel) moved get_vblank() function to be part of helper functions (Daniel) rename 'tiling' parameter as 'modifier' (Daniel) select a mode from a list so that the plane should be invisible. use default 1024x768 mode only as a fallback if decent mode has not been found (Daniel) add tiling MODE_NONE (Daniel) v3: draw as many overlay planes as the platform supports + cursor plane on top of each other on lower-left corner skip the test if i915_display_info file is not available test plane visibility with igt_assert_plane_visibility() function drop option for multiple test iterations (Daniel Vetter) Cc: Daniel Stone Signed-off-by: Mika Kahola --- lib/igt_kms.c| 161 ++ lib/igt_kms.h| 23 tests/Makefile.sources | 1 + tests/kms_plane_lowres.c | 344 +++ 4 files changed, 529 insertions(+) create mode 100644 tests/kms_plane_lowres.c diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 989704e..1ef74dc 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -324,6 +324,24 @@ const char *kmstest_pipe_name(enum pipe pipe) } /** + * kmstest_pipe_to_index: + *@pipe: display pipe in string format + * + * Returns: index to corresponding pipe + */ +int kmstest_pipe_to_index(char pipe) +{ + if (pipe == 'A') + return 0; + else if (pipe == 'B') + return 1; + else if (pipe == 'C') + return 2; + else + return -EINVAL; +} + +/** * kmstest_plane_name: * @plane: display plane * @@ -1176,6 +1194,149 @@ int kmstest_get_crtc_idx(drmModeRes *res, uint32_t crtc_id) igt_assert(false); } +static inline uint32_t pipe_select(int pipe) +{ + if (pipe > 1) + return pipe << DRM_VBLANK_HIGH_CRTC_SHIFT; + else if (pipe > 0) + return DRM_VBLANK_SECONDARY; + else + return 0; +} + +unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int flags) +{ + union drm_wait_vblank vbl; + + memset(&vbl, 0, sizeof(vbl)); + vbl.request.type = DRM_VBLANK_RELATIVE | pipe_select(pipe) | flags; + if (drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl)) + return 0; + + return vbl.reply.sequence; +} + +static void get_plane(char *str, int type, struct kmstest_plane *plane) +{ + int ret; + char buf[256]; + + plane->plane = type; + ret = sscanf(str + 12, "%d%*c %*s %[^n]s", +&plane->id, +buf); + igt_assert_eq(ret, 2); + + ret = sscanf(buf + 9, "%4d%*c%4d%*c", &plane->pos_x, &plane->pos_y); + igt_assert_eq(ret, 2); + + ret = sscanf(buf + 30, "%4d%*c%4d%*c", &plane->width, &plane->height); + igt_assert_eq(ret, 2); +} + +static int parse_planes(FILE *fid, struct kmstest_plane *plane) +{ + char tmp[256]; + int nplanes; + int ovl; + + ovl = 0; + nplanes = 0; + while (fgets(tmp, 256, fid) != NULL) { + igt_assert_neq(nplanes, IGT_MAX_PLANES); + if (strstr(tmp, "type=PRI") != NULL) { + get_plane(tmp, IGT_PLANE_PRIMARY, &plane[nplanes]); + nplanes++; + } else if (strstr(tmp, "type=OVL") != NULL) { + get_plane(tmp, IGT_PLANE_2 + ovl, &plane[nplanes]); + ovl++; + nplanes++; + } else if (strstr(tmp, "type=CUR") != NULL) { + get_plane(tmp, IGT_PLANE_CURSOR, &plane[nplanes]); + nplanes++; + break; + } + } + + return nplanes; +} + +static void parse_crtc(char *info, struct kmstest_crtc *crtc) +{ + char buf[256]; + int ret; + char pipe; + + ret = sscanf(info + 4, "%d%*c %*s %c%*c %*s %s%*c", +&crtc->id, &pipe, buf); + igt_assert_eq(ret, 3); + + crtc->pipe = kmstest_pipe_to_index(pipe); + igt_assert(crtc->pipe >= 0); + + ret = sscanf(buf + 6, "%d%*c%d%*c", +&crtc->width, &crtc->height); + igt_assert_eq(ret, 2); +} + +void kmstest_get_crtc(enum pipe pipe, struct kmstest_crtc *crtc) +{ + char tmp[256]; + FILE *fid; + const char *mode = "r"; + int ncrtc; + in
Re: [Intel-gfx] [PATCH i-g-t v2 10/12] tests/kms_atomic_transition: add out_fences tests
On 2016-12-14 11:57 AM, Brian Starkey wrote: On Wed, Dec 14, 2016 at 04:05:07AM -0500, Robert Foss wrote: From: Gustavo Padovan Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- lib/igt_kms.c | 22 ++ tests/kms_atomic_transition.c | 32 ++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index fe1b356d..eb59ab36 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -49,6 +49,7 @@ #include "intel_chipset.h" #include "igt_debugfs.h" #include "igt_sysfs.h" +#include "sw_sync.h" /** * SECTION:igt_kms @@ -2184,6 +2185,27 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_ } ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data); +if (!ret) { +int64_t *fence_ptr; + +for_each_pipe(display, pipe) { +igt_pipe_t *pipe_obj = &display->pipes[pipe]; + +fence_ptr = pipe_obj->out_fence_ptr; +if (!fence_ptr) +continue; + +if (flags & DRM_MODE_ATOMIC_TEST_ONLY) { +igt_assert(*fence_ptr == -1); +} else { +igt_assert(*fence_ptr >= 0); +ret = sw_sync_wait(*fence_ptr, 1000); +igt_assert(ret == 0); +close(*fence_ptr); +} +} +} + drmModeAtomicFree(req); return ret; diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c index b7d99975..08f9499f 100644 --- a/tests/kms_atomic_transition.c +++ b/tests/kms_atomic_transition.c @@ -132,6 +132,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output struct plane_parms parms[IGT_MAX_PLANES]; bool skip_test = false; unsigned flags = DRM_MODE_PAGE_FLIP_EVENT; +int out_fence, ret; out_fence needs to be 64-bit. Ack, fixed in v3. if (nonblocking) flags |= DRM_MODE_ATOMIC_NONBLOCK; @@ -198,9 +199,11 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output * planes to fix this */ while (1) { -int ret; - wm_setup_plane(display, pipe, iter_max - 1, parms); + +if (fencing) +igt_pipe_set_out_fence_ptr(&display->pipes[pipe], +(int64_t *) &out_fence); ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL); if (ret != -EINVAL || n_planes < 3) @@ -231,6 +234,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output wm_setup_plane(display, pipe, i, parms); +if (fencing) +igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); + igt_display_commit_atomic(display, flags, (void *)(unsigned long)i); drmHandleEvent(display->drm_fd, &drm_events); @@ -239,6 +245,10 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output wm_setup_plane(display, pipe, 0, parms); +if (fencing) +igt_pipe_set_out_fence_ptr(&display->pipes[pipe], +(int64_t *) &out_fence); + igt_display_commit_atomic(display, flags, (void *)0UL); drmHandleEvent(display->drm_fd, &drm_events); @@ -252,6 +262,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output if (type == TRANSITION_MODESET) igt_output_override_mode(output, &override_mode); +if (fencing) +igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); + igt_display_commit_atomic(display, flags, (void *)(unsigned long)j); drmHandleEvent(display->drm_fd, &drm_events); @@ -259,6 +272,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output if (type == TRANSITION_MODESET) igt_output_override_mode(output, NULL); +if (fencing) +igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence); + igt_display_commit_atomic(display, flags, (void *)(unsigned long)i); drmHandleEvent(display->drm_fd, &drm_events); } @@ -588,14 +604,26 @@ igt_main for_each_pipe_with_valid_output(&display, pipe, output) run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false); +igt_subtest("plane-all-transition-fencing") +for_each_pipe_with_valid_output(&display, pipe, output) +run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, true); + igt_subtest("plane-all-transition-nonblocking") for_each_pipe_with_valid_output(&display, pipe, output) run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false); +igt_subtest("plane-all-transition-nonblocking-fencing") +for_each_pipe_with_valid_output
Re: [Intel-gfx] [PATCH v2 01/40] drm/i915: Use the MRU stack search after evicting
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > When we evict from the GTT to make room for an object, the hole we > create is put onto the MRU stack inside the drm_mm range manager. On the > next search pass, we can speed up a PIN_HIGH allocation by referencing > that stack for the new hole. > > v2: Pull together the 3 identical implements (ahem, a couple were > outdated) into a common routine for allocating a node and evicting as > necessary. > > Signed-off-by: Chris Wilson > Reviewed-by: Joonas Lahtinen #v1 Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 03/40] drm: Add drm_mm_for_each_node_safe()
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > A complement to drm_mm_for_each_node(), wraps list_for_each_entry_safe() > for walking the list of nodes safe against removal. > Most of the diff is about __drm_mm_nodes(mm), which could be split into own patch and keep the R-b's. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,01/40] drm/i915: Use the MRU stack search after evicting
== Series Details == Series: series starting with [v2,01/40] drm/i915: Use the MRU stack search after evicting URL : https://patchwork.freedesktop.org/series/16906/ State : success == Summary == Series 16906v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/16906/revisions/1/mbox/ fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:247 pass:222 dwarn:0 dfail:0 fail:0 skip:25 fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20 fi-skl-6770hqtotal:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 428e9f939ad72727abcd02be76fdf41be3672a5b drm-tip: 2016y-12m-16d-08h-25m-46s UTC integration manifest 5bb157c drm: kselftest for drm_mm and bottom-up allocation 4e89e38 drm: Improve drm_mm search (and fix topdown allocation) with rbtrees 25596d7 drm: Use drm_mm_insert_node_in_range_generic() for everyone 419b0874 drm: Apply range restriction after color adjustment when allocation ddaf9d5 drm: Wrap drm_mm_node.hole_follows d680764 drm: Apply tight eviction scanning to color_adjust d8a5adf drm: Simplify drm_mm scan-list manipulation 3abacac drm: Optimise power-of-two alignments in drm_mm_scan_add_block() 48ff21d drm: Compute tight evictions for drm_mm_scan 71bad2c drm: Fix application of color vs range restriction when scanning drm_mm 2e4ed5a drm: Unconditionally do the range check in drm_mm_scan_add_block() 64b0df4 drm: Rename prev_node to hole in drm_mm_scan_add_block() 78fe28e drm: Extract struct drm_mm_scan from struct drm_mm 9d2a85a drm: Add asserts to catch overflow in drm_mm_init() and drm_mm_init_scan() 1031757 drm: Simplify drm_mm_clean() 87dd432 drm: Detect overflow in drm_mm_reserve_node() e797ab0 drm: Fix kerneldoc for drm_mm_scan_remove_block() 0d8045e drm: Promote drm_mm alignment to u64 c93021e drm/i915: Build DRM range manager selftests for CI 20763e9 drm: kselftest for drm_mm and restricted color eviction 184bc9575 drm: kselftest for drm_mm and color eviction cad08ee drm: kselftest for drm_mm and color adjustment 4c17168 drm: kselftest for drm_mm and top-down allocation c669929 drm: kselftest for drm_mm and range restricted eviction fed4139 drm: kselftest for drm_mm and eviction dda6cf3 drm: kselftest for drm_mm and alignment d735a21 drm: kselftest for drm_mm_insert_node_in_range() 10bbc02 drm: kselftest for drm_mm_replace_node() 04e4898 drm: kselftest for drm_mm_insert_node() 450f1db drm: kselftest for drm_mm_reserve_node() 5ccc47c drm: kselftest for drm_mm_debug() 1d041c9 drm: kselftest for drm_mm_init() 8de9fad drm: Add a simple prime number generator 02bbb5b drm: Add a simple generator of random permutations 75629c2 drm: Add some kselftests for the DRM range manager (struct drm_mm) ef10a68 drm: Compile time enabling for asserts in drm_mm 0f89700 drm: Constify the drm_mm API 36f3c26 drm: Add drm_mm_for_each_node_safe() 9f0be0f drm/i915: Simplify i915_gtt_color_adjust() c1d54ce drm/i915: Use the MRU stack search after evicting == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3306/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2] drm/i915: relax uncritical udelay_range()
On Fri, 16 Dec 2016, Nicholas Mc Guire wrote: > udelay_range(1, 2) is inefficient and as discussions with Jani Nikula > unnecessary here. This replaces this > tight setting with a relaxed delay of min=20 and max=50 which helps > the hrtimer subsystem optimize timer handling. > > Fixes: commit be4fc046bed3 ("drm/i915: add VLV DSI PLL Calculations") > Link: http://lkml.org/lkml/2016/12/15/147 > Signed-off-by: Nicholas Mc Guire Pushed to drm-intel-next-queued, thanks for the patch. BR, Jani. > --- > > V2: use relaxed uslee_range() rather than udelay > fix documentation of changed timings > > Problem found by coccinelle: > > Patch was compile tested with: x86_64_defconfig (implies CONFIG_DRM_I915) > > Patch is against 4.9.0 (localversion-next is next-20161215) > > drivers/gpu/drm/i915/intel_dsi_pll.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c > b/drivers/gpu/drm/i915/intel_dsi_pll.c > index 56eff60..d210bc4 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c > @@ -156,8 +156,10 @@ static void vlv_enable_dsi_pll(struct intel_encoder > *encoder, > vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, > config->dsi_pll.ctrl & ~DSI_PLL_VCO_EN); > > - /* wait at least 0.5 us after ungating before enabling VCO */ > - usleep_range(1, 10); > + /* wait at least 0.5 us after ungating before enabling VCO, > + * allow hrtimer subsystem optimization by relaxing timing > + */ > + usleep_range(10, 50); > > vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, config->dsi_pll.ctrl); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ??? Fi.CI.BAT: failure for drm/i915: relax uncritical udelay_range() settings
On Fri, 16 Dec 2016, Nicholas Mc Guire wrote: > On Fri, Dec 16, 2016 at 07:29:59AM +, Saarinen, Jani wrote: >> > == Series Details == >> > >> > Series: drm/i915: relax uncritical udelay_range() settings >> > URL : https://patchwork.freedesktop.org/series/16900/ >> > State : failure >> > >> > == Summary == >> > >> > Series 16900v1 drm/i915: relax uncritical udelay_range() settings >> > https://patchwork.freedesktop.org/api/1.0/series/16900/revisions/1/mbox/ >> > >> > Test gem_ringfill: >> > Subgroup basic-default-hang: >> > pass -> INCOMPLETE (fi-hsw-4770) >> running: igt/gem_ringfill/basic-default-hang >> [117/247] skip: 8, pass: 109 / >> Build timed out (after 17 minutes). Marking the build as aborted. >> > This might be a conflict caused by the initial patch which was discssed > and then after agreeing that rather than moving to udelay() usleep:range > with som adjustments would be the right way to go. > are both patches queued now ? > > I think I caused the problem by changing the subject line of the patch > while adding a V2 - because the original subject line was no longer corrct > (it noted udely) - the V2 patch is marked as "rev 1" in patchwork though. > > not quite clear - anyway sorry if I made some sort of a mess here > the patch applies cleanly on next-20161216. Nah, some other hickup in CI on a machine that's not affected by your change. Pushed to drm-intel-next-queued, thanks for the patch. BR, Jani. > > thx! > hofrat > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/40] drm: Add a simple prime number generator
On Fri, Dec 16, 2016 at 07:46:46AM +, Chris Wilson wrote: > Prime numbers are interesting for testing components that use multiplies > and divides, such as testing struct drm_mm alignment computations. > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/Kconfig | 4 + > drivers/gpu/drm/Makefile| 1 + > drivers/gpu/drm/lib/drm_prime_numbers.c | 175 > > drivers/gpu/drm/lib/drm_prime_numbers.h | 10 ++ > 4 files changed, 190 insertions(+) > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.c > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.h Hm, why not put this in lib/ ? Don't see anything DRM-specific here at first glance and this might be useful to others. Or others might come up with improvements and they'll be more likely to discover it outside of DRM. Same for the random permutations patch. Thanks, Lukas > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 2e6ae95459e4..93895898d596 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -53,6 +53,7 @@ config DRM_DEBUG_MM_SELFTEST > depends on DRM > depends on DEBUG_KERNEL > select DRM_LIB_RANDOM > + select DRM_LIB_PRIMES > default n > help > This option provides a kernel module that can be used to test > @@ -340,3 +341,6 @@ config DRM_LIB_RANDOM > bool > default n > > +config DRM_LIB_PRIMES > + bool > + default n > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 0fa16275fdae..bbd390fa8914 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -19,6 +19,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \ > drm_dumb_buffers.o drm_mode_config.o > > drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o > +obj-$(CONFIG_DRM_LIB_PRIMES) += lib/drm_prime_numbers.o > obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/test-drm_mm.o > > drm-$(CONFIG_COMPAT) += drm_ioc32.o > diff --git a/drivers/gpu/drm/lib/drm_prime_numbers.c > b/drivers/gpu/drm/lib/drm_prime_numbers.c > new file mode 100644 > index ..839563d9b787 > --- /dev/null > +++ b/drivers/gpu/drm/lib/drm_prime_numbers.c > @@ -0,0 +1,175 @@ > +#include > +#include > +#include > + > +#include "drm_prime_numbers.h" > + > +static DEFINE_MUTEX(lock); > + > +static struct primes { > + struct rcu_head rcu; > + unsigned long last, sz; > + unsigned long primes[]; > +} __rcu *primes; > + > +static bool slow_is_prime_number(unsigned long x) > +{ > + unsigned long y = int_sqrt(x) + 1; > + > + while (y > 1) { > + if ((x % y) == 0) > + break; > + y--; > + } > + > + return y == 1; > +} > + > +static unsigned long slow_next_prime_number(unsigned long x) > +{ > + for (;;) { > + if (slow_is_prime_number(++x)) > + return x; > + } > +} > + > +static unsigned long mark_multiples(unsigned long x, > + unsigned long *p, > + unsigned long start, > + unsigned long end) > +{ > + unsigned long m; > + > + m = 2 * x; > + if (m < start) > + m = (start / x + 1) * x; > + > + while (m < end) { > + __clear_bit(m, p); > + m += x; > + } > + > + return x; > +} > + > +static struct primes *expand(unsigned long x) > +{ > + unsigned long sz, y, prev; > + struct primes *p, *new; > + > + sz = x * x; > + if (sz < x) > + return NULL; > + > + mutex_lock(&lock); > + p = rcu_dereference_protected(primes, lockdep_is_held(&lock)); > + if (p && x < p->last) > + goto unlock; > + > + sz = round_up(sz, BITS_PER_LONG); > + new = kmalloc(sizeof(*new) + sz / sizeof(long), GFP_KERNEL); > + if (!new) { > + p = NULL; > + goto unlock; > + } > + > + /* Where memory permits, track the primes using the > + * Sieve of Eratosthenes. > + */ > + if (p) { > + prev = p->sz; > + memcpy(new->primes, p->primes, prev / BITS_PER_LONG); > + } else { > + prev = 0; > + } > + memset(new->primes + prev / BITS_PER_LONG, > +0xff, (sz - prev) / sizeof(long)); > + for (y = 2UL; y < sz; y = find_next_bit(new->primes, sz, y + 1)) > + new->last = mark_multiples(y, new->primes, prev, sz); > + new->sz = sz; > + > + rcu_assign_pointer(primes, new); > + if (p) > + kfree_rcu(p, rcu); > + p = new; > + > +unlock: > + mutex_unlock(&lock); > + return p; > +} > + > +unsigned long drm_next_prime_number(unsigned long x) > +{ > + struct primes *p; > + > + if (x < 2) > + return 2; > + > + rcu_read_lock(); > + p = rcu_dereference(primes); > + if (!p || x >= p->last) { > + rcu_read_unlo
Re: [Intel-gfx] [PATCH v2 07/40] drm: Add a simple generator of random permutations
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > When testing, we want a random but yet reproducible order in which to > process elements. Here we create an array which is a random (using the > Tausworthe PRNG) permutation of the order in which to execute. > > v2: Tidier code by David Herrmann > > Signed-off-by: Chris Wilson > Cc: Joonas Lahtinen > Cc: David Herrmann > @@ -0,0 +1,41 @@ > +#include > +#include > +#include > +#include > +#include > + > +#include "drm_random.h" > + > +static inline u32 prandom_u32_max_state(u32 ep_ro, struct rnd_state *state) > +{ > + return upper_32_bits((u64)prandom_u32_state(state) * ep_ro); > +} > + To be submitted upstream. If you prefix the function here, there won't be a conflict when the upstream part gets merged. > +++ b/drivers/gpu/drm/lib/drm_random.h > @@ -0,0 +1,21 @@ > +#ifndef __DRM_RANDOM_H__ > +#define __DRM_RANDOM_H > + > +#include > + > +#define RND_STATE_INITIALIZER(seed__) ({ \ > + struct rnd_state state__; \ > + prandom_seed_state(&state__, (seed__)); \ > + state__;\ > +}) > + > +#define RND_STATE(name__, seed__) \ > + struct rnd_state name__ = RND_STATE_INITIALIZER(seed__) For upstream submission too. Same comment as above. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 09/40] drm: kselftest for drm_mm_init()
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > Simple first test to just exercise initialisation of struct drm_mm. > > Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/40] drm: Add a simple prime number generator
On Fri, Dec 16, 2016 at 10:31:17AM +0100, Lukas Wunner wrote: > On Fri, Dec 16, 2016 at 07:46:46AM +, Chris Wilson wrote: > > Prime numbers are interesting for testing components that use multiplies > > and divides, such as testing struct drm_mm alignment computations. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/Kconfig | 4 + > > drivers/gpu/drm/Makefile| 1 + > > drivers/gpu/drm/lib/drm_prime_numbers.c | 175 > > > > drivers/gpu/drm/lib/drm_prime_numbers.h | 10 ++ > > 4 files changed, 190 insertions(+) > > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.c > > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.h > > Hm, why not put this in lib/ ? Don't see anything DRM-specific here > at first glance and this might be useful to others. Or others might > come up with improvements and they'll be more likely to discover it > outside of DRM. Because that is a 3+ month cycle before I can then apply the testcases, and without the testscases do you want the bugfixes? If I put in in drm/lib then lift it, I can use it immediately and drop the local copy once merged. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 10/40] drm: kselftest for drm_mm_debug()
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > Simple test to just exercise calling the debug dumper on the drm_mm. > > Signed-off-by: Chris Wilson This is rather meta already. Not entirely sure how good of a selftest this is when we do not validate the generated output, or do you at the runner side? Code itself is (but I'm unsure of the usefulness as is); Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 10/40] drm: kselftest for drm_mm_debug()
On Fri, Dec 16, 2016 at 11:44:39AM +0200, Joonas Lahtinen wrote: > On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > > Simple test to just exercise calling the debug dumper on the drm_mm. > > > > Signed-off-by: Chris Wilson > > This is rather meta already. Not entirely sure how good of a selftest > this is when we do not validate the generated output, or do you at the > runner side? No, it is just to ensure we have coverage of that function. All it does is make sure it doesn't explode, under very tame circumstances. I thought of doing a few mocks to capture the output and decided that was asking a little too much for a debug function. Still don't have coverage for the debugfs dumper... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/40] drm: Add a simple prime number generator
On Fri, Dec 16, 2016 at 09:43:54AM +, Chris Wilson wrote: > On Fri, Dec 16, 2016 at 10:31:17AM +0100, Lukas Wunner wrote: > > On Fri, Dec 16, 2016 at 07:46:46AM +, Chris Wilson wrote: > > > Prime numbers are interesting for testing components that use multiplies > > > and divides, such as testing struct drm_mm alignment computations. > > > > > > Signed-off-by: Chris Wilson > > > --- > > > drivers/gpu/drm/Kconfig | 4 + > > > drivers/gpu/drm/Makefile| 1 + > > > drivers/gpu/drm/lib/drm_prime_numbers.c | 175 > > > > > > drivers/gpu/drm/lib/drm_prime_numbers.h | 10 ++ > > > 4 files changed, 190 insertions(+) > > > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.c > > > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.h > > > > Hm, why not put this in lib/ ? Don't see anything DRM-specific here > > at first glance and this might be useful to others. Or others might > > come up with improvements and they'll be more likely to discover it > > outside of DRM. > > Because that is a 3+ month cycle before I can then apply the testcases, > and without the testscases do you want the bugfixes? Do patches for lib/ have to go through a different tree? Don't think so, I've seen e.g. changes to lib/ucs2_string.c go through the EFI tree. It seems to me lib/ is sort of free for all. > If I put in in drm/lib then lift it, I can use it immediately and drop > the local copy once merged. That is also workable of course. Anyway, it was just a suggestion. Thanks, Lukas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 24/40] drm: Fix kerneldoc for drm_mm_scan_remove_block()
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > The nodes must be removed in the *reverse* order. This is correct in the > overview, but backwards in the function description. Whilst here add > Intel's copyright statement and tweak some formatting. > > Signed-off-by: Chris Wilson It's like a Christmas gift for Daniel. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 25/40] drm: Detect overflow in drm_mm_reserve_node()
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > Protect ourselves from a caller passing in node.start + node.size that > will overflow and trick us into reserving that node. > > Signed-off-by: Chris Wilson I was about to suggest an additional check (but didn't). A combined check is much more clever. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/40] drm: Add a simple prime number generator
On Fri, Dec 16, 2016 at 11:08:10AM +0100, Lukas Wunner wrote: > On Fri, Dec 16, 2016 at 09:43:54AM +, Chris Wilson wrote: > > On Fri, Dec 16, 2016 at 10:31:17AM +0100, Lukas Wunner wrote: > > > On Fri, Dec 16, 2016 at 07:46:46AM +, Chris Wilson wrote: > > > > Prime numbers are interesting for testing components that use multiplies > > > > and divides, such as testing struct drm_mm alignment computations. > > > > > > > > Signed-off-by: Chris Wilson > > > > --- > > > > drivers/gpu/drm/Kconfig | 4 + > > > > drivers/gpu/drm/Makefile| 1 + > > > > drivers/gpu/drm/lib/drm_prime_numbers.c | 175 > > > > > > > > drivers/gpu/drm/lib/drm_prime_numbers.h | 10 ++ > > > > 4 files changed, 190 insertions(+) > > > > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.c > > > > create mode 100644 drivers/gpu/drm/lib/drm_prime_numbers.h > > > > > > Hm, why not put this in lib/ ? Don't see anything DRM-specific here > > > at first glance and this might be useful to others. Or others might > > > come up with improvements and they'll be more likely to discover it > > > outside of DRM. > > > > Because that is a 3+ month cycle before I can then apply the testcases, > > and without the testscases do you want the bugfixes? > > Do patches for lib/ have to go through a different tree? > Don't think so, I've seen e.g. changes to lib/ucs2_string.c > go through the EFI tree. It seems to me lib/ is sort of > free for all. Hmm, I was expecting to shepherd them through say Andrew Morton. lib/random32.c is maintained by David Miller, so definitely would like to present a simple set of pre-reviewed patches. But it looks like we could create lib/prime_numbers.c without too much consternation. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm: Add a new connector atomic property for link status
From: Manasi Navare At the time userspace does setcrtc, we've already promised the mode would work. The promise is based on the theoretical capabilities of the link, but it's possible we can't reach this in practice. The DP spec describes how the link should be reduced, but we can't reduce the link below the requirements of the mode. Black screen follows. One idea would be to have setcrtc return a failure. However, it already should not fail as the atomic checks have passed. It would also conflict with the idea of making setcrtc asynchronous in the future, returning before the actual mode setting and link training. Another idea is to train the link "upfront" at hotplug time, before pruning the mode list, so that we can do the pruning based on practical not theoretical capabilities. However, the changes for link training are pretty drastic, all for the sake of error handling and DP compliance, when the most common happy day scenario is the current approach of link training at mode setting time, using the optimal parameters for the mode. It is also not certain all hardware could do this without the pipe on; not even all our hardware can do this. Some of this can be solved, but not trivially. Both of the above ideas also fail to address link degradation *during* operation. The solution is to add a new "link-status" connector property in order to address link training failure in a way that: a) changes the current happy day scenario as little as possible, to avoid regressions, b) can be implemented the same way by all drm drivers, c) is still opt-in for the drivers and userspace, and opting out doesn't regress the user experience, d) doesn't prevent drivers from implementing better or alternate approaches, possibly without userspace involvement. And, of course, handles all the issues presented. In the usual happy day scenario, this is always "good". If something fails during or after a mode set, the kernel driver can set the link status to "bad" and issue a hotplug uevent for userspace to have it re-check the valid modes through GET_CONNECTOR IOCTL, and try modeset again. If the theoretical capabilities of the link can't be reached, the mode list is trimmed based on that. v7 by Jani: * Rebase, simplify set property while at it, checkpatch fix v6: * Fix a typo in kernel doc (Sean Paul) v5: * Clarify doc for silent rejection of atomic properties by driver (Daniel Vetter) v4: * Add comments in kernel-doc format (Daniel Vetter) * Update the kernel-doc for link-status (Sean Paul) v3: * Fixed a build error (Jani Saarinen) v2: * Removed connector->link_status (Daniel Vetter) * Set connector->state->link_status in drm_mode_connector_set_link_status_property (Daniel Vetter) * Set the connector_changed flag to true if connector->state->link_status changed. * Reset link_status to GOOD in update_output_state (Daniel Vetter) * Never allow userspace to set link status from Good To Bad (Daniel Vetter) Reviewed-by: Sean Paul Reviewed-by: Daniel Vetter Reviewed-by: Jani Nikula Acked-by: Tony Cheng Acked-by: Harry Wentland Cc: Jani Nikula Cc: Daniel Vetter Cc: Ville Syrjala Cc: Chris Wilson Cc: Sean Paul Signed-off-by: Manasi Navare Signed-off-by: Jani Nikula --- drivers/gpu/drm/drm_atomic.c| 16 drivers/gpu/drm/drm_atomic_helper.c | 15 +++ drivers/gpu/drm/drm_connector.c | 52 + include/drm/drm_connector.h | 19 ++ include/drm/drm_mode_config.h | 5 include/uapi/drm/drm_mode.h | 4 +++ 6 files changed, 111 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index ff38592134f5..91fd8a9a7526 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -,6 +,20 @@ int drm_atomic_connector_set_property(struct drm_connector *connector, state->tv.saturation = val; } else if (property == config->tv_hue_property) { state->tv.hue = val; + } else if (property == config->link_status_property) { + /* Never downgrade from GOOD to BAD on userspace's request here, +* only hw issues can do that. +* +* For an atomic property the userspace doesn't need to be able +* to understand all the properties, but needs to be able to +* restore the state it wants on VT switch. So if the userspace +* tries to change the link_status from GOOD to BAD, driver +* silently rejects it and returns a 0. This prevents userspace +* from accidently breaking the display when it restores the +* state. +*/ + if (state->link_status != DRM_LINK_STATUS_GOOD) + state->link_status = val; } else if (connector->funcs->atomic_set_property) { return connector->funcs->atomic_set_property(connector,
[Intel-gfx] [PATCH 0/2] drm: link status property and DP link training failure handling
The two remaining patches from [1], rebased. BR, Jani. [1] 1480984058-552-1-git-send-email-manasi.d.navare@intel.com">http://mid.mail-archive.com/1480984058-552-1-git-send-email-manasi.d.navare@intel.com Manasi Navare (2): drm: Add a new connector atomic property for link status drm/i915: Implement Link Rate fallback on Link training failure drivers/gpu/drm/drm_atomic.c | 16 + drivers/gpu/drm/drm_atomic_helper.c | 15 drivers/gpu/drm/drm_connector.c | 52 +++ drivers/gpu/drm/i915/intel_dp.c | 27 ++ drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++-- drivers/gpu/drm/i915/intel_drv.h | 3 ++ include/drm/drm_connector.h | 19 ++ include/drm/drm_mode_config.h | 5 +++ include/uapi/drm/drm_mode.h | 4 +++ 9 files changed, 161 insertions(+), 2 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Implement Link Rate fallback on Link training failure
From: Manasi Navare If link training at a link rate optimal for a particular mode fails during modeset's atomic commit phase, then we let the modeset complete and then retry. We save the link rate value at which link training failed, update the link status property to "BAD" and use a lower link rate to prune the modes. It will redo the modeset on the current mode at lower link rate or if the current mode gets pruned due to lower link constraints then, it will send a hotplug uevent for userspace to handle it. This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4, 4.3.1.6. v9: * Use the trimmed max values of link rate/lane count based on link train fallback (Daniel Vetter) v8: * Set link_status to BAD first and then call mode_valid (Jani Nikula) v7: Remove the redundant variable in previous patch itself v6: * Obtain link rate index from fallback_link_rate using the helper intel_dp_link_rate_index (Jani Nikula) * Include fallback within intel_dp_start_link_train (Jani Nikula) v5: * Move set link status to drm core (Daniel Vetter, Jani Nikula) v4: * Add fallback support for non DDI platforms too * Set connector->link status inside set_link_status function (Jani Nikula) v3: * Set link status property to BAd unconditionally (Jani Nikula) * Dont use two separate variables link_train_failed and link_status to indicate same thing (Jani Nikula) v2: * Squashed a few patches (Jani Nikula) Acked-by: Tony Cheng Acked-by: Harry Wentland Cc: Jani Nikula Cc: Daniel Vetter Cc: Ville Syrjala Signed-off-by: Manasi Navare Reviewed-by: Jani Nikula Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_dp.c | 27 +++ drivers/gpu/drm/i915/intel_dp_link_training.c | 22 -- drivers/gpu/drm/i915/intel_drv.h | 3 +++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 45ebc9632633..97d1e03d22b8 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5671,6 +5671,29 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, return false; } +static void intel_dp_modeset_retry_work_fn(struct work_struct *work) +{ + struct intel_connector *intel_connector; + struct drm_connector *connector; + + intel_connector = container_of(work, typeof(*intel_connector), + modeset_retry_work); + connector = &intel_connector->base; + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, + connector->name); + + /* Grab the locks before changing connector property*/ + mutex_lock(&connector->dev->mode_config.mutex); + /* Set connector link status to BAD and send a Uevent to notify +* userspace to do a modeset. +*/ + drm_mode_connector_set_link_status_property(connector, + DRM_MODE_LINK_STATUS_BAD); + mutex_unlock(&connector->dev->mode_config.mutex); + /* Send Hotplug uevent so userspace can reprobe */ + drm_kms_helper_hotplug_event(connector->dev); +} + bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, struct intel_connector *intel_connector) @@ -5683,6 +5706,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, enum port port = intel_dig_port->port; int type; + /* Initialize the work for modeset in case of link train failure */ + INIT_WORK(&intel_connector->modeset_retry_work, + intel_dp_modeset_retry_work_fn); + if (WARN(intel_dig_port->max_lanes < 1, "Not enough lanes (%d) for DP on port %c\n", intel_dig_port->max_lanes, port_name(port))) diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index 0048b520baf7..955b239e7c2d 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -313,6 +313,24 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) void intel_dp_start_link_train(struct intel_dp *intel_dp) { - intel_dp_link_training_clock_recovery(intel_dp); - intel_dp_link_training_channel_equalization(intel_dp); + struct intel_connector *intel_connector = intel_dp->attached_connector; + + if (!intel_dp_link_training_clock_recovery(intel_dp)) + goto failure_handling; + if (!intel_dp_link_training_channel_equalization(intel_dp)) + goto failure_handling; + + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", + intel_dp->link_rate, intel_dp->lane_count); + return; + + failure_handling: + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", + intel_dp->link_rate, intel_dp->lane_count); + if (!intel_dp_get_l
Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: Simplify intel_guc_load()
On Thu, Dec 15, 2016 at 05:38:16PM +0100, Michal Wajdeczko wrote: > On Thu, Dec 15, 2016 at 04:47:06PM +0100, Arkadiusz Hiler wrote: > > Current version of intel_guc_load() does a lot: > > - cares about submission > > - loads huc > > - implement WA > > > > This change offloads some of the logic to intel_uc_load(), which now > > cares about the above. > > > > Cc: Anusha Srivatsa > > Cc: Jeff McGee > > Cc: Michal Winiarski > > Signed-off-by: Arkadiusz Hiler > > --- > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > drivers/gpu/drm/i915/intel_guc_loader.c | 126 > > +--- > > drivers/gpu/drm/i915/intel_uc.c | 83 + > > drivers/gpu/drm/i915/intel_uc.h | 8 ++ > > 4 files changed, 110 insertions(+), 109 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index 6af4e85..76b52c6 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -4412,7 +4412,7 @@ i915_gem_init_hw(struct drm_i915_private *dev_priv) > > intel_mocs_init_l3cc_table(dev_priv); > > > > /* We can't enable contexts until all firmware is loaded */ > > - ret = intel_guc_load(dev_priv); > > + ret = intel_uc_load(dev_priv); > > if (ret) > > goto out; > > > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > > b/drivers/gpu/drm/i915/intel_guc_loader.c > > index f8b28b1..b76b556 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > > @@ -97,7 +97,7 @@ const char *intel_guc_fw_status_repr(enum > > intel_guc_fw_status status) > > } > > }; > > > > -static void guc_interrupts_release(struct drm_i915_private *dev_priv) > > +void guc_interrupts_release(struct drm_i915_private *dev_priv) > > { > > struct intel_engine_cs *engine; > > enum intel_engine_id id; > > @@ -115,7 +115,7 @@ static void guc_interrupts_release(struct > > drm_i915_private *dev_priv) > > I915_WRITE(GUC_WD_VECS_IER, 0); > > } > > > > -static void guc_interrupts_capture(struct drm_i915_private *dev_priv) > > +void guc_interrupts_capture(struct drm_i915_private *dev_priv) > > { > > struct intel_engine_cs *engine; > > enum intel_engine_id id; > > @@ -334,7 +334,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private > > *dev_priv, > > return ret; > > } > > > > -static u32 guc_wopcm_size(struct drm_i915_private *dev_priv) > > +u32 guc_wopcm_size(struct drm_i915_private *dev_priv) > > { > > u32 wopcm_size = GUC_WOPCM_TOP; > > > > @@ -417,7 +417,7 @@ static int guc_ucode_xfer(struct drm_i915_private > > *dev_priv) > > return ret; > > } > > > > -static int guc_hw_reset(struct drm_i915_private *dev_priv) > > +int guc_hw_reset(struct drm_i915_private *dev_priv) > > { > > int ret; > > u32 guc_status; > > @@ -452,75 +452,37 @@ int intel_guc_load(struct drm_i915_private *dev_priv) > > { > > struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > > const char *fw_path = guc_fw->guc_fw_path; > > - int retries, ret, err; > > + int ret; > > > > DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n", > > fw_path, > > intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status), > > intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); > > > > - /* Loading forbidden, or no firmware to load? */ > > - if (!i915.enable_guc_loading) { > > - err = 0; > > - goto fail; > > - } else if (fw_path == NULL) { > > + if (fw_path == NULL) { > > /* Device is known to have no uCode (e.g. no GuC) */ > > - err = -ENXIO; > > - goto fail; > > + return -ENXIO; > > } else if (*fw_path == '\0') { > > /* Device has a GuC but we don't know what f/w to load? */ > > WARN(1, "No GuC firmware known for this platform!\n"); > > - err = -ENODEV; > > - goto fail; > > + return -ENODEV; > > } > > > > /* Fetch failed, or already fetched but failed to load? */ > > if (guc_fw->guc_fw_fetch_status != GUC_FIRMWARE_SUCCESS) { > > - err = -EIO; > > - goto fail; > > + return -EIO; > > } else if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL) { > > - err = -ENOEXEC; > > - goto fail; > > + return -ENOEXEC; > > } > > > > - guc_interrupts_release(dev_priv); > > - gen9_reset_guc_interrupts(dev_priv); > > - > > guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING; > > > > - DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n", > > - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status), > > - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); > > - > > - err = i915_guc_submission_init(dev_priv); > > - if (err) > > - goto fail; > > - > > - /* > > -* WaEnableuKernelHeaderValidFix:skl,bxt > > -* For
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm: link status property and DP link training failure handling
== Series Details == Series: drm: link status property and DP link training failure handling URL : https://patchwork.freedesktop.org/series/16912/ State : failure == Summary == Series 16912v1 drm: link status property and DP link training failure handling https://patchwork.freedesktop.org/api/1.0/series/16912/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-reload-final: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-reload-inject: dmesg-warn -> INCOMPLETE (fi-skl-6700k) pass -> DMESG-WARN (fi-kbl-7500u) pass -> DMESG-WARN (fi-ilk-650) Test gem_busy: Subgroup basic-hang-default: fail -> PASS (fi-hsw-4770r) Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-ilk-650) Test gem_flink_basic: Subgroup bad-open: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic: pass -> DMESG-WARN (fi-ilk-650) Subgroup double-flink: pass -> DMESG-WARN (fi-ilk-650) Test kms_addfb_basic: Subgroup addfb25-bad-modifier: pass -> DMESG-WARN (fi-ilk-650) Subgroup addfb25-framebuffer-vs-set-tiling: pass -> DMESG-WARN (fi-ilk-650) Subgroup addfb25-modifier-no-flag: pass -> DMESG-WARN (fi-ilk-650) Subgroup addfb25-x-tiled: pass -> DMESG-WARN (fi-ilk-650) Test kms_busy: Subgroup basic-flip-default-a: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-flip-default-b: pass -> DMESG-WARN (fi-ilk-650) Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-busy-flip-before-cursor-varying-size: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-flip-after-cursor-legacy: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-flip-after-cursor-varying-size: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-flip-before-cursor-legacy: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-flip-before-cursor-varying-size: pass -> DMESG-WARN (fi-ilk-650) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-flip-vs-wf_vblank: pass -> DMESG-WARN (fi-ilk-650) Subgroup basic-plain-flip: pass -> DMESG-WARN (fi-ilk-650) Test kms_pipe_crc_basic: Subgroup bad-nb-words-1: pass -> DMESG-WARN (fi-ilk-650) Subgroup bad-nb-words-3: pass -> DMESG-WARN (fi-ilk-650) Subgroup bad-pipe: pass -> DMESG-WARN (fi-ilk-650) Subgroup bad-source: pass -> DMESG-WARN (fi-ilk-650) Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (fi-ilk-650) Subgroup hang-read-crc-pipe-b: pass -> DMESG-WARN (fi-ilk-650) Subgroup nonblocking-crc-pipe-a: pass -> DMESG-WARN (fi-ilk-650) Subgroup nonblocking-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (fi-ilk-650) Subgroup nonblocking-crc-pipe-b: pass -> DMESG-WARN (fi-ilk-650) Subgroup nonblocking-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (fi-ilk-650) Subgroup read-crc-pipe-a: pass -> DMESG-WARN (fi-ilk-650) Subgroup read-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (fi-ilk-650) Subgroup read-crc-pipe-b: pass -> DMESG-WARN (fi-ilk-650) Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (fi-ilk-650) Subgroup suspend-read-crc-pipe-a: pass -> DMESG-WARN (fi-ilk-650) Subgroup suspend-read-crc-pipe-b: pass -> DMESG-WARN (fi-ilk-650) Test kms_setmode: Subgroup basic-clone-single-crtc: pass -> DMESG-WARN (fi-ilk-650) WARNING: Long output truncated e5dd5ad8937cd82ea07bde765bde5b4c09d81dcb drm-tip: 2016y-12m-16d-09h-25m-11s UTC integration manifest db2460d drm/i915: Implement Link Rate fallback on Link training failure d835410 drm: Add a new connector atomic property for link status == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3307/ __
Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: Simplify intel_guc_load()
On Thu, Dec 15, 2016 at 02:26:29PM -0800, Daniele Ceraolo Spurio wrote: > > > On 15/12/16 07:47, Arkadiusz Hiler wrote: > > Current version of intel_guc_load() does a lot: > > - cares about submission > > - loads huc > > - implement WA > > > > This change offloads some of the logic to intel_uc_load(), which now > > cares about the above. > > > > Cc: Anusha Srivatsa > > Cc: Jeff McGee > > Cc: Michal Winiarski > > Signed-off-by: Arkadiusz Hiler > > --- > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > drivers/gpu/drm/i915/intel_guc_loader.c | 126 > > +--- > > drivers/gpu/drm/i915/intel_uc.c | 83 + > > drivers/gpu/drm/i915/intel_uc.h | 8 ++ > > 4 files changed, 110 insertions(+), 109 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index 6af4e85..76b52c6 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -417,7 +417,7 @@ static int guc_ucode_xfer(struct drm_i915_private > > *dev_priv) > > return ret; > > } > > > > -static int guc_hw_reset(struct drm_i915_private *dev_priv) > > +int guc_hw_reset(struct drm_i915_private *dev_priv) > If I haven't missed anything, guc_hw_reset is only called in 1 place, so we > could keep the function static and move it to intel_uc.c. Okay. > > { > > int ret; > > u32 guc_status; > > @@ -452,75 +452,37 @@ int intel_guc_load(struct drm_i915_private *dev_priv) > > { > > struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > > const char *fw_path = guc_fw->guc_fw_path; > > - int retries, ret, err; > > + int ret; > > > > DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n", > > fw_path, > > intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status), > > intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); > > > > - /* Loading forbidden, or no firmware to load? */ > > - if (!i915.enable_guc_loading) { > > - err = 0; > > - goto fail; > > - } else if (fw_path == NULL) { > > + if (fw_path == NULL) { > > /* Device is known to have no uCode (e.g. no GuC) */ > > - err = -ENXIO; > > - goto fail; > > + return -ENXIO; > > } else if (*fw_path == '\0') { > > /* Device has a GuC but we don't know what f/w to load? */ > > WARN(1, "No GuC firmware known for this platform!\n"); > > - err = -ENODEV; > > - goto fail; > > + return -ENODEV; > > } > > > > /* Fetch failed, or already fetched but failed to load? */ > > if (guc_fw->guc_fw_fetch_status != GUC_FIRMWARE_SUCCESS) { > > - err = -EIO; > > - goto fail; > > + return -EIO; > > } else if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL) { > > - err = -ENOEXEC; > > - goto fail; > > + return -ENOEXEC; > > } > > > > - guc_interrupts_release(dev_priv); > > - gen9_reset_guc_interrupts(dev_priv); > > - > > guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING; > > > > - DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n", > > - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status), > > - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); > > - > > - err = i915_guc_submission_init(dev_priv); > > - if (err) > > - goto fail; > > - > > - /* > > -* WaEnableuKernelHeaderValidFix:skl,bxt > > -* For BXT, this is only upto B0 but below WA is required for later > > -* steppings also so this is extended as well. > > -*/ > > This comment is removed, but the WA is applicable to all SKL steppings and > is also applicable to HuC according to the specs so I suggest to retain the > comment and move it to intel_uc_load(). I missread the commend. I'll leave this as a WaEnableuKernelHeaderValidFix:skl since it is fixed for BXT. > > /* WaEnableGuCBootHashCheckNotSet:skl,bxt */ > > The implementation of this WA is now outside this function and it is marked > as such there. I'd personally prefer to remove this comment from here as it > might cause confusion, but no strong feelings either way. The implementation is twofold now - the the function which returns -EAGAIN if we failed at the step we know may fail and we may want to retry on it. Then you have functions that handles the actuall three attempts. So I prefer to keep the note on the WA in both places. > > - for (retries = 3; ; ) { > > - /* > > -* Always reset the GuC just before (re)loading, so > > -* that the state and timing are fairly predictable > > -*/ > > - err = guc_hw_reset(dev_priv); > > - if (err) > > - goto fail; > > + /* we may want to retry guc ucode transfer */ > > + ret = guc_ucode_xfer(dev_priv); > > > > - err = guc_ucode_xfer(dev_priv); > > -
Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Rename _setup() to _load()
On Thu, Dec 15, 2016 at 05:22:53PM +0100, Michal Wajdeczko wrote: > On Thu, Dec 15, 2016 at 04:47:04PM +0100, Arkadiusz Hiler wrote: > > GuC historically has two "startup" functions called _init() and _setup() > > > > Then HuC came with it's _init() and _load(). > > > > To make naming more consistent this commit renames intel_guc_setup() to > > intel_guc_load() as it it seams more fitting (it's in intel_guc_loader.c > > after all). > > > > Cc: Anusha Srivatsa > > Cc: Jeff McGee > > Cc: Michal Winiarski > > Signed-off-by: Arkadiusz Hiler > > --- > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++-- > > drivers/gpu/drm/i915/intel_uc.h | 2 +- > > 3 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index f86a71d9..6af4e85 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -4412,7 +4412,7 @@ i915_gem_init_hw(struct drm_i915_private *dev_priv) > > intel_mocs_init_l3cc_table(dev_priv); > > > > /* We can't enable contexts until all firmware is loaded */ > > - ret = intel_guc_setup(dev_priv); > > + ret = intel_guc_load(dev_priv); > > if (ret) > > goto out; > > > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > > b/drivers/gpu/drm/i915/intel_guc_loader.c > > index 21db697..f8b28b1 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > > @@ -436,19 +436,19 @@ static int guc_hw_reset(struct drm_i915_private > > *dev_priv) > > } > > > > /** > > - * intel_guc_setup() - finish preparing the GuC for activity > > + * intel_guc_load() - finish preparing the GuC for activity > > * @dev_priv: i915 device private > > * > > - * Called from gem_init_hw() during driver loading and also after a GPU > > reset. > > + * Called during driver loading and also after a GPU reset. > > * > > * The main action required here it to load the GuC uCode into the device. > > * The firmware image should have already been fetched into memory by the > > - * earlier call to intel_guc_init(), so here we need only check that > > worked, > > - * and then transfer the image to the h/w. > > + * earlier call to intel_guc_init(), so here we need only check that > > + * worked, and then transfer the image to the h/w. > > * > > * Return: non-zero code on error > > */ > > -int intel_guc_setup(struct drm_i915_private *dev_priv) > > +int intel_guc_load(struct drm_i915_private *dev_priv) > > Can we use this refactor effort to fix also inconsistency in params > passed to functions that start with "intel_guc_" prefix ? > Some require dev_priv, while other expect pointer to intel_guc struct. > My preferrence would be latter syntax with *guc. > > Note that if required we can easily get dev_priv using guc_to_i915(). > > Other option, use "i915_guc" prefix and then pass dev_priv to distinguish > between these two forms. > Good idea. I'll include it in vol. 2. > Michal > > > { > > struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > > const char *fw_path = guc_fw->guc_fw_path; > > @@ -717,7 +717,7 @@ static void guc_fw_fetch(struct drm_i915_private > > *dev_priv, > > * Called early during driver load, but after GEM is initialised. > > * > > * The firmware will be transferred to the GuC's memory later, > > - * when intel_guc_setup() is called. > > + * when intel_guc_load() is called. > > */ > > void intel_guc_init(struct drm_i915_private *dev_priv) > > { > > diff --git a/drivers/gpu/drm/i915/intel_uc.h > > b/drivers/gpu/drm/i915/intel_uc.h > > index 11f5608..7222e6c 100644 > > --- a/drivers/gpu/drm/i915/intel_uc.h > > +++ b/drivers/gpu/drm/i915/intel_uc.h > > @@ -179,7 +179,7 @@ int intel_guc_log_control(struct intel_guc *guc, u32 > > control_val); > > > > /* intel_guc_loader.c */ > > extern void intel_guc_init(struct drm_i915_private *dev_priv); > > -extern int intel_guc_setup(struct drm_i915_private *dev_priv); > > +extern int intel_guc_load(struct drm_i915_private *dev_priv); > > extern void intel_guc_fini(struct drm_i915_private *dev_priv); > > extern const char *intel_guc_fw_status_repr(enum intel_guc_fw_status > > status); > > extern int intel_guc_suspend(struct drm_i915_private *dev_priv); > > -- > > 2.9.3 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Rename _setup() to _load()
On Thu, Dec 15, 2016 at 03:57:01PM +, Chris Wilson wrote: > On Thu, Dec 15, 2016 at 04:47:04PM +0100, Arkadiusz Hiler wrote: > > GuC historically has two "startup" functions called _init() and _setup() > > > > Then HuC came with it's _init() and _load(). > > > > To make naming more consistent this commit renames intel_guc_setup() to > > intel_guc_load() as it it seams more fitting (it's in intel_guc_loader.c > > after all). > > Or init_hw as that is the initialisation phase it is called from. > -Chris Since it's intel_guc_loader.c I somehow prefer _load() here. But intel_uc_load() which, is introduced with the series and call intel_guc_load() can be renamed to intel_uc_init_hw() What do you think? -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 i-g-t] tools: Add intel_dp_compliance for DisplayPort 1.2 compliance automation
A couple of comments below, and some bonus nitpicks. On Wed, Dec 07, 2016 at 02:04:52PM -0800, Manasi Navare wrote: > This is the userspace component of the Displayport Compliance > testing software required for compliance testing of the I915 > Display Port driver. This must be running in order to successfully > complete Display Port compliance testing. This app and the kernel > code that accompanies it has been written to satify the requirements > of the Displayport Link CTS 1.2 rev1.1 specification from VESA. > Note that this application does not support eDP compliance testing. > This utility has an automation support for the Link training tests > (4.3.1.1. - 4.3.2.3), EDID tests (4.2.2.3 > - 4.2.2.6) and Video Pattern generation tests (4.3.3.1) from CTS > specification 1.2 Rev 1.1. > > This tool has the support for responding to the hotplug uevents > sent by compliance testting unit after each test. > > The Linux DUT running this utility must be in text (console) mode > and cannot have any other display manager running. Since this uses > sysfs nodes for kernel interaction, this utility should be run as > Root. Once this user application is up and running, waiting for > test requests, the test appliance software on the windows host > can now be used to execute the compliance tests. > > This app is based on some prior work done in April 2015 (by > Todd Previte ) > > v2: > * Add mode unset on hotplug uevent on disconnect (Manasi Navare) > > Cc: Petri Latvala > Cc: Marius Vlad > Cc: Daniel Vetter > Signed-off-by: Manasi Navare > --- > tools/Makefile.am |3 +- > tools/Makefile.sources |7 + > tools/intel_dp_compliance.c | 1060 > +++ > tools/intel_dp_compliance.h | 35 ++ > tools/intel_dp_compliance_hotplug.c | 123 > 5 files changed, 1227 insertions(+), 1 deletion(-) > create mode 100644 tools/intel_dp_compliance.c > create mode 100644 tools/intel_dp_compliance.h > create mode 100644 tools/intel_dp_compliance_hotplug.c > > diff --git a/tools/Makefile.am b/tools/Makefile.am > index 18f86f6..eac6d64 100644 > --- a/tools/Makefile.am > +++ b/tools/Makefile.am > @@ -13,7 +13,7 @@ AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib > AM_CFLAGS = $(DEBUG_CFLAGS) $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \ > $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) -DPKGDATADIR=\"$(pkgdatadir)\" \ > $(WERROR_CFLAGS) > -LDADD = $(top_builddir)/lib/libintel_tools.la > +LDADD = $(top_builddir)/lib/libintel_tools.la $(GLIB_LIBS) > AM_LDFLAGS = -Wl,--as-needed > > > @@ -24,6 +24,7 @@ moduledir = $(libdir) > intel_aubdump_la_LDFLAGS = -module -avoid-version -no-undefined > intel_aubdump_la_SOURCES = aubdump.c > intel_aubdump_la_LIBADD = $(top_builddir)/lib/libintel_tools.la -ldl > +intel_dp_compliance_la_LIBADD = $(LDADD) > > bin_SCRIPTS = intel_aubdump > CLEANFILES = $(bin_SCRIPTS) This makes all tools link to glib, and then sets intel_dp_compliance.la (which doesn't exist) to be built with default linking flags. The Makefile.am changes should be just diff --git a/tools/Makefile.am b/tools/Makefile.am index 18f86f6..bd8f512 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -16,6 +16,7 @@ AM_CFLAGS = $(DEBUG_CFLAGS) $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \ LDADD = $(top_builddir)/lib/libintel_tools.la AM_LDFLAGS = -Wl,--as-needed +intel_dp_compliance_LDADD = $(top_builddir)/lib/libintel_tools.la $(GLIB_LIBS) # aubdumper > diff --git a/tools/Makefile.sources b/tools/Makefile.sources > index be58871..09c0667 100644 > --- a/tools/Makefile.sources > +++ b/tools/Makefile.sources > @@ -13,6 +13,7 @@ tools_prog_lists = \ > intel_bios_reader \ > intel_display_crc \ > intel_display_poller\ > + intel_dp_compliance \ > intel_forcewaked\ > intel_gpu_frequency \ > intel_firmware_decode \ > @@ -55,3 +56,9 @@ intel_l3_parity_SOURCES = \ > intel_l3_parity.h \ > intel_l3_udev_listener.c > > +intel_dp_compliance_SOURCES = \ > +intel_dp_compliance.c \ > +intel_dp_compliance.h \ > +intel_dp_compliance_hotplug.c \ > +$(NULL) > + > diff --git a/tools/intel_dp_compliance.c b/tools/intel_dp_compliance.c > new file mode 100644 > index 000..807d3f4 > --- /dev/null > +++ b/tools/intel_dp_compliance.c > @@ -0,0 +1,1060 @@ > +/* > + * Copyright ? 2014-2015 Intel Corporation 2014-2016 if there are changes done in 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 s
Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Rename _setup() to _load()
On Fri, Dec 16, 2016 at 12:47:26PM +0100, Arkadiusz Hiler wrote: > On Thu, Dec 15, 2016 at 03:57:01PM +, Chris Wilson wrote: > > On Thu, Dec 15, 2016 at 04:47:04PM +0100, Arkadiusz Hiler wrote: > > > GuC historically has two "startup" functions called _init() and _setup() > > > > > > Then HuC came with it's _init() and _load(). > > > > > > To make naming more consistent this commit renames intel_guc_setup() to > > > intel_guc_load() as it it seams more fitting (it's in intel_guc_loader.c > > > after all). > > > > Or init_hw as that is the initialisation phase it is called from. > > -Chris > > Since it's intel_guc_loader.c I somehow prefer _load() here. > > But intel_uc_load() which, is introduced with the series and call > intel_guc_load() can be renamed to intel_uc_init_hw() > > What do you think? We want to push the "phase verb" as far as it makes sense, especially along the chain i.e. driver -> subsystem -> subsubsystem -> ... Once we are in the handler, it should use the right functions named appropriate. I still think carrying the phase verb as far as possible is important (more important say for init_early) as that carries the information about the rest of the driver state and the limitations we must keep in mind. Good taste should prevail ofc; the actual work must be done by sensibly named functions. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 37/40] drm: Apply range restriction after color adjustment when allocation
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > mm->color_adjust() compares the hole with its neighbouring nodes. They > only abutt before we restrict the hole, so we have to apply color_adjust > before we apply the range restriction. > > Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 38/40] drm: Use drm_mm_insert_node_in_range_generic() for everyone
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > Remove a superfluous helper as drm_mm_insert_node is equivalent to > insert_node_in_range with a range of (0, U64_MAX). > > Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 36/40] drm: Wrap drm_mm_node.hole_follows
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > Insulate users from changed to the internal hole tracking within changes ^ > struct drm_mm_node by using an accessor for hole_follows. > > Signed-off-by: Chris Wilson The function name could be soemthing beginning with drm_mm_node... Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 34/40] drm: Simplify drm_mm scan-list manipulation
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > Since we mandate a strict reverse-order of drm_mm_scan_remove_block() > after drm_mm_scan_add_block() we can further simplify the list > manipulations when generating the temporary scan-hole. > > v2: Highlight the games being played with the lists to track the scan > holes without allocation. > > Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 27/40] drm: Add asserts to catch overflow in drm_mm_init() and drm_mm_init_scan()
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > A simple assert to ensure that we don't overflow start + size when > initialising the drm_mm, or its scanner. > > In future, we may want to switch to tracking the value of ranges (rather > than size) so that we can cover the full u64, for example like resource > tracking. > > Signed-off-by: Chris Wilson > @@ -729,6 +729,8 @@ void drm_mm_init_scan(struct drm_mm *mm, > u64 alignment, > unsigned long color) > { > + DRM_MM_BUG_ON(size == 0); Nitpicking, DIM will complain that this should be !size. > + > > mm->scan_color = color; > > mm->scan_alignment = alignment; > > mm->scan_size = size; > @@ -764,6 +766,9 @@ void drm_mm_init_scan_with_range(struct drm_mm *mm, > u64 start, > u64 end) > { Ditto, could have simply DRM_MM_BUG_ON(!size); Reviewed-by: Joonas Lahtinen Regards, Joonas > + DRM_MM_BUG_ON(start >= end); > + DRM_MM_BUG_ON(size == 0 || size > end - start); > + -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix use after free in logical_render_ring_init
From: Tvrtko Ursulin Commit 3b3f1650b1ca ("drm/i915: Allocate intel_engine_cs structure only for the enabled engines") introduced the dynanically allocated engine instances and created an potential use after free scenario in logical_render_ring_init where lrc_destroy_wa_ctx_obj could be called after the engine instance has been freed. This can only happen during engine setup/init error handling which luckily does not happen ever in practice. Fix is to not call lrc_destroy_wa_ctx_obj since it would have already been executed from the preceding engine cleanup. Signed-off-by: Tvrtko Ursulin Reported-by: Dan Carpenter Fixes: 3b3f1650b1ca ("drm/i915: Allocate intel_engine_cs structure only for the enabled engines") Cc: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Cc: Daniel Vetter Cc: Jani Nikula --- drivers/gpu/drm/i915/intel_lrc.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 8b412880e88c..877adedbf833 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1965,12 +1965,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine) ret); } - ret = logical_ring_init(engine); - if (ret) { - lrc_destroy_wa_ctx_obj(engine); - } - - return ret; + return logical_ring_init(engine); } int logical_xcs_ring_init(struct intel_engine_cs *engine) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] lib: Add a simple prime number generator
Prime numbers are interesting for testing components that use multiplies and divides, such as testing DRM's struct drm_mm alignment computations. v2: Move to lib/, add selftest Signed-off-by: Chris Wilson Cc: Lukas Wunner --- include/linux/prime_numbers.h | 13 +++ lib/Kconfig | 7 ++ lib/Makefile | 2 + lib/prime_numbers.c | 230 ++ 4 files changed, 252 insertions(+) create mode 100644 include/linux/prime_numbers.h create mode 100644 lib/prime_numbers.c diff --git a/include/linux/prime_numbers.h b/include/linux/prime_numbers.h new file mode 100644 index ..877f6acbd0b6 --- /dev/null +++ b/include/linux/prime_numbers.h @@ -0,0 +1,13 @@ +#ifndef __LINUX_PRIME_NUMBERS_H +#define __LINUX_PRIME_NUMBERS_H + +#include + +bool is_prime_number(unsigned long x); +unsigned long next_prime_number(unsigned long x); + +/* A useful white-lie here is that 1 is prime. */ +#define for_each_prime_number(prime, max) \ + for (prime = 1; prime < (max); prime = next_prime_number(prime)) + +#endif /* !__LINUX_PRIME_NUMBERS_H */ diff --git a/lib/Kconfig b/lib/Kconfig index 260a80e313b9..1788a1f50d28 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -550,4 +550,11 @@ config STACKDEPOT config SBITMAP bool +config PRIME_NUMBERS + tristate "Prime number generator" + default n + help + Provides a helper module to generate prime numbers. Useful for writing + test code, especially when checking multiplication and divison. + endmenu diff --git a/lib/Makefile b/lib/Makefile index 50144a3aeebd..c664143fd917 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -197,6 +197,8 @@ obj-$(CONFIG_ASN1) += asn1_decoder.o obj-$(CONFIG_FONT_SUPPORT) += fonts/ +obj-$(CONFIG_PRIME_NUMBERS) += prime_numbers.o + hostprogs-y:= gen_crc32table clean-files:= crc32table.h diff --git a/lib/prime_numbers.c b/lib/prime_numbers.c new file mode 100644 index ..ba08be0ff1ed --- /dev/null +++ b/lib/prime_numbers.c @@ -0,0 +1,230 @@ +#define pr_fmt(fmt) "prime numbers: " #fmt + +#include +#include +#include +#include +#include + +struct primes { + struct rcu_head rcu; + unsigned long last, sz; + unsigned long primes[]; +}; + +#if BITS_PER_LONG == 64 +static const struct primes small_primes = { + .last = 61, + .sz = 64, + .primes = { 0x28208a20a08a28ae } +}; +#elif BITS_PER_LONG == 32 +static const struct primes small_primes = { + .last = 31, + .sz = 32, + .primes = { 0xa08a28ae } +}; +#else +#error "unhandled BITS_PER_LONG" +#endif + +static DEFINE_MUTEX(lock); +static const struct primes __rcu *primes = &small_primes; + +static unsigned long selftest_max; + +static bool slow_is_prime_number(unsigned long x) +{ + unsigned long y = int_sqrt(x); + + while (y > 1) { + if ((x % y) == 0) + break; + y--; + } + + return y == 1; +} + +static unsigned long slow_next_prime_number(unsigned long x) +{ + for (;;) { + if (slow_is_prime_number(++x)) + return x; + } +} + +static unsigned long mark_multiples(unsigned long x, + unsigned long *p, + unsigned long start, + unsigned long end) +{ + unsigned long m; + + m = 2 * x; + if (m < start) + m = roundup(start, x); + + while (m < end) { + __clear_bit(m, p); + m += x; + } + + return x; +} + +static const struct primes *expand_to_next(unsigned long x) +{ + const struct primes *p; + struct primes *new; + unsigned long sz, y; + + rcu_read_unlock(); + + /* Betrand's Theorem states: +* For all n > 1, there exists a prime p: n < p <= 2*n. +*/ + sz = 2 * x + 1; + if (sz < x) + return NULL; + + sz = round_up(sz, BITS_PER_LONG); + new = kmalloc(sizeof(*new) + sz / sizeof(long), + GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN); + if (!new) + new = vmalloc(sizeof(*new) + sz / sizeof(long)); + if (!new) + return NULL; + + mutex_lock(&lock); + p = rcu_dereference_protected(primes, lockdep_is_held(&lock)); + if (x < p->last) { + kfree(new); + goto relock; + } + + /* Where memory permits, track the primes using the +* Sieve of Eratosthenes. +*/ + memcpy(new->primes, p->primes, p->sz / BITS_PER_LONG * sizeof(long)); + memset(new->primes + p->sz / BITS_PER_LONG, + 0xff, (sz - p->sz) / BITS_PER_LONG * sizeof(long)); + for (y = 2UL; y < sz; y = find_next_bit(new->primes, sz, y + 1)) + new->last = mark_multiples(y, new->primes, p->sz, sz); + new->sz = sz; +
Re: [Intel-gfx] [PATCH] drm/i915: Fix use after free in logical_render_ring_init
On Fri, Dec 16, 2016 at 01:18:42PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Commit 3b3f1650b1ca ("drm/i915: Allocate intel_engine_cs > structure only for the enabled engines") introduced the > dynanically allocated engine instances and created an > potential use after free scenario in logical_render_ring_init > where lrc_destroy_wa_ctx_obj could be called after the engine > instance has been freed. > > This can only happen during engine setup/init error handling > which luckily does not happen ever in practice. > > Fix is to not call lrc_destroy_wa_ctx_obj since it would have > already been executed from the preceding engine cleanup. > > Signed-off-by: Tvrtko Ursulin > Reported-by: Dan Carpenter > Fixes: 3b3f1650b1ca ("drm/i915: Allocate intel_engine_cs structure only for > the enabled engines") > Cc: Chris Wilson > Cc: Joonas Lahtinen > Cc: Tvrtko Ursulin > Cc: Daniel Vetter > Cc: Jani Nikula Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RESEND][GIT PULL] i915/gvt KVMGT tree for 4.10
Hi, Linus This is KVMGT pull for 4.10 as explained by Daniel. The last minute rebase is to appease git pull-request since the diffstat was obvious bonghits and somehow included vfio/mdev again despite that was merged already. Thanks. The following changes since commit 9439b3710df688d853eb6cb4851256f2c92b1797: Merge tag 'drm-for-v4.10' of git://people.freedesktop.org/~airlied/linux (2016-12-13 09:35:09 -0800) are available in the git repository at: https://github.com/01org/gvt-linux.git tags/kvmgt-vfio-mdev-for-v4.10-rc1 for you to fetch changes up to 659643f7d81432189c2c87230e2feee4c75c14c1: drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT (2016-12-16 16:55:26 +0800) kvmgt-vfio-mdev-for-v4.10-rc1 This is KVMGT support depending on VFIO/mdev framework. Jike Song (3): drm/i915/gvt/kvmgt: replace kmalloc() by kzalloc() drm/i915/gvt/kvmgt: read/write GPA via KVM API drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/gvt/Makefile | 2 - drivers/gpu/drm/i915/gvt/gvt.h| 6 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 975 +++--- 4 files changed, 923 insertions(+), 61 deletions(-) -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 36/40] drm: Wrap drm_mm_node.hole_follows
On Fri, Dec 16, 2016 at 03:04:31PM +0200, Joonas Lahtinen wrote: > On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > > Insulate users from changed to the internal hole tracking within > > changes ^ > > > struct drm_mm_node by using an accessor for hole_follows. > > > > Signed-off-by: Chris Wilson > > The function name could be soemthing beginning with drm_mm_node... Didn't like it as drm_mm_node_hole_follows(), and rationalised it away as saying this is really drm_mm state that happens to be hidden away inside the user's drm_mm_node. I guess we should look at some /* private */ markup. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 03/40] drm: Add drm_mm_for_each_node_safe()
On Fri, Dec 16, 2016 at 11:06:25AM +0200, Joonas Lahtinen wrote: > On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > > A complement to drm_mm_for_each_node(), wraps list_for_each_entry_safe() > > for walking the list of nodes safe against removal. > > > > Most of the diff is about __drm_mm_nodes(mm), which could be split into > own patch and keep the R-b's. Added a note and applied to drm-misc, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 36/40] drm: Wrap drm_mm_node.hole_follows
On pe, 2016-12-16 at 13:31 +, Chris Wilson wrote: > On Fri, Dec 16, 2016 at 03:04:31PM +0200, Joonas Lahtinen wrote: > > > > On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > > > > > > Insulate users from changed to the internal hole tracking within > > > > changes ^ > > > > > > > > struct drm_mm_node by using an accessor for hole_follows. > > > > > > Signed-off-by: Chris Wilson > > > > The function name could be soemthing beginning with drm_mm_node... > > Didn't like it as drm_mm_node_hole_follows(), and rationalised it away as > saying this is really drm_mm state that happens to be hidden away inside > the user's drm_mm_node. I guess we should look at some /* private */ markup. That makes sense now that you mention it. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 04/40] drm: Constify the drm_mm API
On Fri, Dec 16, 2016 at 07:46:42AM +, Chris Wilson wrote: > Mark up the pointers as constant through the API where appropriate. > > Signed-off-by: Chris Wilson Ok merged this and the patch right before, then Chris told me on irc that I need to stop because he's mixing a v3 of this series. -Daniel > --- > drivers/gpu/drm/drm_mm.c| 24 > drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- > include/drm/drm_mm.h| 27 +-- > 3 files changed, 26 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c > index 6e0735539545..7573661302a4 100644 > --- a/drivers/gpu/drm/drm_mm.c > +++ b/drivers/gpu/drm/drm_mm.c > @@ -174,9 +174,9 @@ INTERVAL_TREE_DEFINE(struct drm_mm_node, rb, >START, LAST, static inline, drm_mm_interval_tree) > > struct drm_mm_node * > -__drm_mm_interval_first(struct drm_mm *mm, u64 start, u64 last) > +__drm_mm_interval_first(const struct drm_mm *mm, u64 start, u64 last) > { > - return drm_mm_interval_tree_iter_first(&mm->interval_tree, > + return drm_mm_interval_tree_iter_first((struct rb_root > *)&mm->interval_tree, > start, last); > } > EXPORT_SYMBOL(__drm_mm_interval_first); > @@ -881,9 +881,9 @@ EXPORT_SYMBOL(drm_mm_scan_remove_block); > * True if the allocator is completely free, false if there's still a node > * allocated in it. > */ > -bool drm_mm_clean(struct drm_mm * mm) > +bool drm_mm_clean(const struct drm_mm *mm) > { > - struct list_head *head = __drm_mm_nodes(mm); > + const struct list_head *head = __drm_mm_nodes(mm); > > return (head->next->next == head); > } > @@ -897,7 +897,7 @@ EXPORT_SYMBOL(drm_mm_clean); > * > * Note that @mm must be cleared to 0 before calling this function. > */ > -void drm_mm_init(struct drm_mm * mm, u64 start, u64 size) > +void drm_mm_init(struct drm_mm *mm, u64 start, u64 size) > { > INIT_LIST_HEAD(&mm->hole_stack); > mm->scanned_blocks = 0; > @@ -936,8 +936,8 @@ void drm_mm_takedown(struct drm_mm *mm) > } > EXPORT_SYMBOL(drm_mm_takedown); > > -static u64 drm_mm_debug_hole(struct drm_mm_node *entry, > - const char *prefix) > +static u64 drm_mm_debug_hole(const struct drm_mm_node *entry, > + const char *prefix) > { > u64 hole_start, hole_end, hole_size; > > @@ -958,9 +958,9 @@ static u64 drm_mm_debug_hole(struct drm_mm_node *entry, > * @mm: drm_mm allocator to dump > * @prefix: prefix to use for dumping to dmesg > */ > -void drm_mm_debug_table(struct drm_mm *mm, const char *prefix) > +void drm_mm_debug_table(const struct drm_mm *mm, const char *prefix) > { > - struct drm_mm_node *entry; > + const struct drm_mm_node *entry; > u64 total_used = 0, total_free = 0, total = 0; > > total_free += drm_mm_debug_hole(&mm->head_node, prefix); > @@ -979,7 +979,7 @@ void drm_mm_debug_table(struct drm_mm *mm, const char > *prefix) > EXPORT_SYMBOL(drm_mm_debug_table); > > #if defined(CONFIG_DEBUG_FS) > -static u64 drm_mm_dump_hole(struct seq_file *m, struct drm_mm_node *entry) > +static u64 drm_mm_dump_hole(struct seq_file *m, const struct drm_mm_node > *entry) > { > u64 hole_start, hole_end, hole_size; > > @@ -1000,9 +1000,9 @@ static u64 drm_mm_dump_hole(struct seq_file *m, struct > drm_mm_node *entry) > * @m: seq_file to dump to > * @mm: drm_mm allocator to dump > */ > -int drm_mm_dump_table(struct seq_file *m, struct drm_mm *mm) > +int drm_mm_dump_table(struct seq_file *m, const struct drm_mm *mm) > { > - struct drm_mm_node *entry; > + const struct drm_mm_node *entry; > u64 total_used = 0, total_free = 0, total = 0; > > total_free += drm_mm_dump_hole(m, &mm->head_node); > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 4d82f38a000b..97fd66e4e3d0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2699,7 +2699,7 @@ void i915_gem_gtt_finish_pages(struct > drm_i915_gem_object *obj, > dma_unmap_sg(kdev, pages->sgl, pages->nents, PCI_DMA_BIDIRECTIONAL); > } > > -static void i915_gtt_color_adjust(struct drm_mm_node *node, > +static void i915_gtt_color_adjust(const struct drm_mm_node *node, > unsigned long color, > u64 *start, > u64 *end) > diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h > index 0cc1b78c9ec2..5c7f15875b6a 100644 > --- a/include/drm/drm_mm.h > +++ b/include/drm/drm_mm.h > @@ -102,7 +102,8 @@ struct drm_mm { > u64 scan_end; > struct drm_mm_node *prev_scanned_node; > > - void (*color_adjust)(struct drm_mm_node *node, unsigned long color, > + void (*color_adjust)(const struct drm_mm_node *node, > + unsigned long color, >
Re: [Intel-gfx] [PATCH v2 39/40] drm: Improve drm_mm search (and fix topdown allocation) with rbtrees
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > The drm_mm range manager claimed to support top-down insertion, but it > was neither searching for the top-most hole that could fit the > allocation request nor fitting the request to the hole correctly. > > In order to search the range efficiently, we create a secondary index > for the holes using either their size or their address. This index > allows us to find the smallest hole or the hole at the bottom or top of > the range efficiently, whilst keeping the hole stack to rapidly service > evictions. > > v2: Search for holes both high and low. Rename flags to mode. > > Signed-off-by: Chris Wilson > +static struct drm_mm_node *best_hole(struct drm_mm *mm, u64 size) > +{ > + struct rb_node *best = NULL; > + struct rb_node **link = &mm->holes_size.rb_node; > + while (*link) { > + struct rb_node *rb = *link; > + if (size <= rb_hole_size(rb)) > + link = &rb->rb_left, best = rb; Missed this split. With that addressed, this is; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/2] drm: link status property and DP link training failure handling
On Fri, Dec 16, 2016 at 12:29:05PM +0200, Jani Nikula wrote: > The two remaining patches from [1], rebased. > > BR, > Jani. > > > [1] > 1480984058-552-1-git-send-email-manasi.d.navare@intel.com">http://mid.mail-archive.com/1480984058-552-1-git-send-email-manasi.d.navare@intel.com Just for the record, I think the only thing missing here is the Xorg review on the -modesetting patch. As soon as we have that I can vacuum this up (probably best through drm-misc, but not sure). -Daniel > > > Manasi Navare (2): > drm: Add a new connector atomic property for link status > drm/i915: Implement Link Rate fallback on Link training failure > > drivers/gpu/drm/drm_atomic.c | 16 + > drivers/gpu/drm/drm_atomic_helper.c | 15 > drivers/gpu/drm/drm_connector.c | 52 > +++ > drivers/gpu/drm/i915/intel_dp.c | 27 ++ > drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++-- > drivers/gpu/drm/i915/intel_drv.h | 3 ++ > include/drm/drm_connector.h | 19 ++ > include/drm/drm_mode_config.h | 5 +++ > include/uapi/drm/drm_mode.h | 4 +++ > 9 files changed, 161 insertions(+), 2 deletions(-) > > -- > 2.1.4 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] lib: Add a simple prime number generator
On Fri, Dec 16, 2016 at 01:23:23PM +, Chris Wilson wrote: > Prime numbers are interesting for testing components that use multiplies > and divides, such as testing DRM's struct drm_mm alignment computations. > > v2: Move to lib/, add selftest > > Signed-off-by: Chris Wilson > Cc: Lukas Wunner > --- > include/linux/prime_numbers.h | 13 +++ > lib/Kconfig | 7 ++ > lib/Makefile | 2 + > lib/prime_numbers.c | 230 > ++ > 4 files changed, 252 insertions(+) > create mode 100644 include/linux/prime_numbers.h > create mode 100644 lib/prime_numbers.c > > diff --git a/include/linux/prime_numbers.h b/include/linux/prime_numbers.h > new file mode 100644 > index ..877f6acbd0b6 > --- /dev/null > +++ b/include/linux/prime_numbers.h > @@ -0,0 +1,13 @@ > +#ifndef __LINUX_PRIME_NUMBERS_H > +#define __LINUX_PRIME_NUMBERS_H > + > +#include > + > +bool is_prime_number(unsigned long x); > +unsigned long next_prime_number(unsigned long x); > + > +/* A useful white-lie here is that 1 is prime. */ > +#define for_each_prime_number(prime, max) \ > + for (prime = 1; prime < (max); prime = next_prime_number(prime)) > + > +#endif /* !__LINUX_PRIME_NUMBERS_H */ > diff --git a/lib/Kconfig b/lib/Kconfig > index 260a80e313b9..1788a1f50d28 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -550,4 +550,11 @@ config STACKDEPOT > config SBITMAP > bool > > +config PRIME_NUMBERS > + tristate "Prime number generator" > + default n > + help > + Provides a helper module to generate prime numbers. Useful for writing > + test code, especially when checking multiplication and divison. > + > endmenu > diff --git a/lib/Makefile b/lib/Makefile > index 50144a3aeebd..c664143fd917 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -197,6 +197,8 @@ obj-$(CONFIG_ASN1) += asn1_decoder.o > > obj-$(CONFIG_FONT_SUPPORT) += fonts/ > > +obj-$(CONFIG_PRIME_NUMBERS) += prime_numbers.o > + > hostprogs-y := gen_crc32table > clean-files := crc32table.h > > diff --git a/lib/prime_numbers.c b/lib/prime_numbers.c > new file mode 100644 > index ..ba08be0ff1ed > --- /dev/null > +++ b/lib/prime_numbers.c > @@ -0,0 +1,230 @@ > +#define pr_fmt(fmt) "prime numbers: " #fmt > + > +#include > +#include > +#include > +#include > +#include > + > +struct primes { > + struct rcu_head rcu; > + unsigned long last, sz; > + unsigned long primes[]; > +}; > + > +#if BITS_PER_LONG == 64 > +static const struct primes small_primes = { > + .last = 61, > + .sz = 64, > + .primes = { 0x28208a20a08a28ae } Ugh, extracted this from primes[0] and forgot the mark up skips 0,1 > + for (last = 0, x = 2; x < max; x++) { > + bool slow = slow_is_prime_number(x); > + bool fast = slow_is_prime_number(x); Which didn't help! -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in logical_render_ring_init
== Series Details == Series: drm/i915: Fix use after free in logical_render_ring_init URL : https://patchwork.freedesktop.org/series/16918/ State : success == Summary == Series 16918v1 drm/i915: Fix use after free in logical_render_ring_init https://patchwork.freedesktop.org/api/1.0/series/16918/revisions/1/mbox/ Test gem_busy: Subgroup basic-hang-default: fail -> PASS (fi-hsw-4770r) fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:247 pass:222 dwarn:0 dfail:0 fail:0 skip:25 fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20 fi-skl-6770hqtotal:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 e5dd5ad8937cd82ea07bde765bde5b4c09d81dcb drm-tip: 2016y-12m-16d-09h-25m-11s UTC integration manifest 899a8cc4 drm/i915: Fix use after free in logical_render_ring_init == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3308/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 39/40] drm: Improve drm_mm search (and fix topdown allocation) with rbtrees
On Fri, Dec 16, 2016 at 03:46:18PM +0200, Joonas Lahtinen wrote: > On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > > The drm_mm range manager claimed to support top-down insertion, but it > > was neither searching for the top-most hole that could fit the > > allocation request nor fitting the request to the hole correctly. > > > > In order to search the range efficiently, we create a secondary index > > for the holes using either their size or their address. This index > > allows us to find the smallest hole or the hole at the bottom or top of > > the range efficiently, whilst keeping the hole stack to rapidly service > > evictions. > > > > v2: Search for holes both high and low. Rename flags to mode. > > > > Signed-off-by: Chris Wilson > > > > > +static struct drm_mm_node *best_hole(struct drm_mm *mm, u64 size) > > +{ > > + struct rb_node *best = NULL; > > + struct rb_node **link = &mm->holes_size.rb_node; > > + while (*link) { > > + struct rb_node *rb = *link; > > + if (size <= rb_hole_size(rb)) > > + link = &rb->rb_left, best = rb; > > Missed this split. You must own shares in a company selling braces! :-p -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] lib: Add a simple prime number generator
Prime numbers are interesting for testing components that use multiplies and divides, such as testing DRM's struct drm_mm alignment computations. v2: Move to lib/, add selftest v3: Fix initial constants (exclude 0/1 from being primes) Signed-off-by: Chris Wilson Cc: Lukas Wunner --- include/linux/prime_numbers.h | 13 +++ lib/Kconfig | 7 ++ lib/Makefile | 2 + lib/prime_numbers.c | 230 ++ 4 files changed, 252 insertions(+) create mode 100644 include/linux/prime_numbers.h create mode 100644 lib/prime_numbers.c diff --git a/include/linux/prime_numbers.h b/include/linux/prime_numbers.h new file mode 100644 index ..877f6acbd0b6 --- /dev/null +++ b/include/linux/prime_numbers.h @@ -0,0 +1,13 @@ +#ifndef __LINUX_PRIME_NUMBERS_H +#define __LINUX_PRIME_NUMBERS_H + +#include + +bool is_prime_number(unsigned long x); +unsigned long next_prime_number(unsigned long x); + +/* A useful white-lie here is that 1 is prime. */ +#define for_each_prime_number(prime, max) \ + for (prime = 1; prime < (max); prime = next_prime_number(prime)) + +#endif /* !__LINUX_PRIME_NUMBERS_H */ diff --git a/lib/Kconfig b/lib/Kconfig index 260a80e313b9..1788a1f50d28 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -550,4 +550,11 @@ config STACKDEPOT config SBITMAP bool +config PRIME_NUMBERS + tristate "Prime number generator" + default n + help + Provides a helper module to generate prime numbers. Useful for writing + test code, especially when checking multiplication and divison. + endmenu diff --git a/lib/Makefile b/lib/Makefile index 50144a3aeebd..c664143fd917 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -197,6 +197,8 @@ obj-$(CONFIG_ASN1) += asn1_decoder.o obj-$(CONFIG_FONT_SUPPORT) += fonts/ +obj-$(CONFIG_PRIME_NUMBERS) += prime_numbers.o + hostprogs-y:= gen_crc32table clean-files:= crc32table.h diff --git a/lib/prime_numbers.c b/lib/prime_numbers.c new file mode 100644 index ..8c3cdd05d4ae --- /dev/null +++ b/lib/prime_numbers.c @@ -0,0 +1,230 @@ +#define pr_fmt(fmt) "prime numbers: " fmt "\n" + +#include +#include +#include +#include +#include + +struct primes { + struct rcu_head rcu; + unsigned long last, sz; + unsigned long primes[]; +}; + +#if BITS_PER_LONG == 64 +static const struct primes small_primes = { + .last = 61, + .sz = 64, + .primes = { 0x28208a20a08a28ac } +}; +#elif BITS_PER_LONG == 32 +static const struct primes small_primes = { + .last = 31, + .sz = 32, + .primes = { 0xa08a28ac } +}; +#else +#error "unhandled BITS_PER_LONG" +#endif + +static DEFINE_MUTEX(lock); +static const struct primes __rcu *primes = &small_primes; + +static unsigned long selftest_max; + +static bool slow_is_prime_number(unsigned long x) +{ + unsigned long y = int_sqrt(x); + + while (y > 1) { + if ((x % y) == 0) + break; + y--; + } + + return y == 1; +} + +static unsigned long slow_next_prime_number(unsigned long x) +{ + for (;;) { + if (slow_is_prime_number(++x)) + return x; + } +} + +static unsigned long mark_multiples(unsigned long x, + unsigned long *p, + unsigned long start, + unsigned long end) +{ + unsigned long m; + + m = 2 * x; + if (m < start) + m = roundup(start, x); + + while (m < end) { + __clear_bit(m, p); + m += x; + } + + return x; +} + +static const struct primes *expand_to_next(unsigned long x) +{ + const struct primes *p; + struct primes *new; + unsigned long sz, y; + + rcu_read_unlock(); + + /* Betrand's Theorem states: +* For all n > 1, there exists a prime p: n < p <= 2*n. +*/ + sz = 2 * x + 1; + if (sz < x) + return NULL; + + sz = round_up(sz, BITS_PER_LONG); + new = kmalloc(sizeof(*new) + sz / sizeof(long), + GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN); + if (!new) + new = vmalloc(sizeof(*new) + sz / sizeof(long)); + if (!new) + return NULL; + + mutex_lock(&lock); + p = rcu_dereference_protected(primes, lockdep_is_held(&lock)); + if (x < p->last) { + kfree(new); + goto relock; + } + + /* Where memory permits, track the primes using the +* Sieve of Eratosthenes. +*/ + memcpy(new->primes, p->primes, p->sz / BITS_PER_LONG * sizeof(long)); + memset(new->primes + p->sz / BITS_PER_LONG, + 0xff, (sz - p->sz) / BITS_PER_LONG * sizeof(long)); + for (y = 2UL; y < sz; y = find_next_bit(new->primes, sz, y + 1)) + new->last = mark_
Re: [Intel-gfx] [PATCH v2 12/40] drm: kselftest for drm_mm_insert_node()
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > Exercise drm_mm_insert_node(), check that we can't overfill a range and > that the lists are correct after reserving/removing. > > v2: Extract helpers for the repeated tests > v3: Iterate over all allocation flags > > Signed-off-by: Chris Wilson > +static u64 misaligned(struct drm_mm_node *node, u64 alignment) I'm not sure if 'misalignment' would be better name or not. This makes me think of bool returning one. > +static bool expect_insert_fail(struct drm_mm *mm, u64 size) > +{ > + struct drm_mm_node tmp = {}; > + int err; > + > + err = drm_mm_insert_node(mm, &tmp, size, 0, DRM_MM_SEARCH_DEFAULT); > + if (err != -ENOSPC) { For speed (this function gets called a lot); if (likely(err == -ENOSPC)) return true; > +static int __igt_insert(unsigned int count, u64 size) > +{ > > + for (mode = insert_modes; mode->name; mode++) { > + for (n = 0; n < count; n++) { > + node = &nodes[n]; > + err = drm_mm_insert_node_generic(&mm, node, size, 0, n, > + mode->search_flags, > + mode->create_flags); > + if (err || !assert_node(node, &mm, size, 0, n)) { > + pr_err("%s insert failed, size %llu step %d\n", > + mode->name, size, n); > + ret = err ?: -EINVAL; > + goto out; > + } This construct is three times in this patch; Could be expect_insert_generic? Apart from nitpicks, no complaints; was much easier to review now! Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/9] drm/i915: Don't populate plane->plane for cursors and sprites
Em Qui, 2016-12-15 às 21:59 +0200, Ville Syrjälä escreveu: > On Thu, Dec 15, 2016 at 05:50:02PM -0200, Paulo Zanoni wrote: > > > > Em Qui, 2016-12-15 às 21:11 +0200, Ville Syrjälä escreveu: > > > > > > On Thu, Dec 15, 2016 at 05:05:05PM -0200, Paulo Zanoni wrote: > > > > > > > > > > > > Em Ter, 2016-11-22 às 18:02 +0200, ville.syrj...@linux.intel.co > > > > m > > > > escreveu: > > > > > > > > > > > > > > > From: Ville Syrjälä > > > > > > > > > > With plane->plane now purely reserved for the primary planes, > > > > > let's > > > > > not even populate it for cursors and sprites. Let's switch > > > > > the > > > > > type > > > > > to enum plane as well since it's no longer being abused for > > > > > anything > > > > > else. > > > > > > > > Since it's a primary plane thing, I think it makes more sense > > > > to > > > > just > > > > leave it at struct intel_crtc instead of having a field that's > > > > unused > > > > and doesn't make sense in some cases. > > > > > > Primary plane aren't really primary planes on old platforms > > > though. > > > That's just a software concept that has no bearing on the > > > hardware. > > > > Please explain more since that's not my understanding. I just > > opened > > the gen 2 and 3 spec docs and they do contain tons of "primary > > plane" > > references. I know that the hardware is a little more flexible > > there, > > Yep. Planes can be moved to any pipe in general. > > > > > but as far as I understand we don't even implement that. > > Not at the moment. But I have plans... > > > > > Besides, both > > our driver and our API do have the concept of a primary plane, so > > it > > makes sense for the code to be organized that way, IMHO. > > A plane is a plane, a pipe is a pipe. IMO you're trying to make it > more confusing by mixing up the two. I think that goes against your original argument to remove the plane- >plane assignment for everything except for primary planes. Anyway, your argumentation is very short and dismissive, it's hard to understand what exactly you are thinking and try to move the discussion forward. I suppose we'll just have to agree to disagree here. In one solution we create an inconsistency where the plane struct has a field that's only applicable and can only be used to some specific planes (even though "every plane is a plane"). In the other solution we create the inconsistency where the crtc<->plane connection is assigned in the way not chosen by the hardware and this may be a problem if we ever get to implement the plane assignment flexibility that's allowed by the hardware. Maybe having a 3rd opinion here would help. > > > > > Besides, I > > think our code organizations should always better fit the new > > hardware, > > since otherwise we'll make i915.ko development even harder than it > > is > > for new contributors/employees. > > If you don't mix planes and pipes there can't be any confusion. > > > > > > > (And when I see those specs and realize how different the HW was, > > then > > I remember that in order to explain to the new people why some > > things > > are the way they are in our code I have to explain how the HW was > > 10 > > years ago, I start thinking again that maybe we should just have > > i915- > > old.ko and i915-new.ko, since either we make our > > abstractions/design > > fit one thing or we make it fit another...) > > From the register POV hw was almost identical up to BDW at least. > It's > just a few bits (the pipe selector) that vanished from the registers. > SKL+ is a little more different but still the registers even live in > the > same address. > > Also given what recent history has shown us, I wouldn't be at all > surprised if we would go back to a flexible plane<->pipe assignment > in the hardware at some point in the future. A lot of the other > features that were present in old hardware has been making a comeback > in recent years. > > > > > > > > > > > > > > > > > > > > > > > The crtc struct already includes > > > > some fields that are specific to primary planes, so I think > > > > it's a > > > > good > > > > place. Or we could create a new class: struct > > > > intel_primary_plane { > > > > struct intel_plane base; enum plane legacy_plane }; > > > > > > > > Following is a patch suggestion (probably impossible to apply > > > > due > > > > to my > > > > editor breaking long lines). Any arguments against it? > > > > > > > > --8<-- > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > > b/drivers/gpu/drm/i915/intel_display.c > > > > index bc1af87..c54b1a7 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -15065,14 +15065,6 @@ intel_primary_plane_create(struct > > > > drm_i915_private *dev_priv, enum pipe pipe) > > > > state->scaler_id = -1; > > > > } > > > > primary->pipe = pipe; > > > > - /* > > > > -
Re: [Intel-gfx] [PATCH v2 14/40] drm: kselftest for drm_mm_insert_node_in_range()
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > Exercise drm_mm_insert_node_in_range(), check that we only allocate from > the specified range. > > v2: Use all allocation flags > > Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 35/40] drm: Apply tight eviction scanning to color_adjust
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > Using mm->color_adjust makes the eviction scanner much tricker since we > don't know the actual neighbours of the target hole until after it is > created (after scanning is complete). To work out whether we need to > evict the neighbours because they impact upon the hole, we have to then > check the hole afterwards - requiring an extra step in the user of the > eviction scanner when they apply color_adjust. > > v2: Massage kerneldoc. > > Signed-off-by: Chris Wilson It's not optimal as a separate function, but good for now. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 35/40] drm: Apply tight eviction scanning to color_adjust
On Fri, Dec 16, 2016 at 04:14:30PM +0200, Joonas Lahtinen wrote: > On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > > Using mm->color_adjust makes the eviction scanner much tricker since we > > don't know the actual neighbours of the target hole until after it is > > created (after scanning is complete). To work out whether we need to > > evict the neighbours because they impact upon the hole, we have to then > > check the hole afterwards - requiring an extra step in the user of the > > eviction scanner when they apply color_adjust. > > > > v2: Massage kerneldoc. > > > > Signed-off-by: Chris Wilson > > It's not optimal as a separate function, but good for now. Yes, it's nasty that it requires a separate step by the caller to catch for residual overlap. The problem is that we do require the mm.color_adjust to be performed to tell if the user thinks there will be overlap - and that requires the final state of the hole and its neighbours. Alternatives I considered were just a color_expand parameter and always evicting the neighbouring nodes (but that is not great for us as most of our nodes have one colour). Or I considered searching the node list to find the overlaps and flag them (presuming eviction happened as we intended). They all seemed to be worse than just asking the user to check after eviction if they are using color_adjust. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Use drm_mm_nodes() as shorthand for the list of nodes under struct drm_mm
Fairly commonly we want to inspect the node list on the struct drm_mm, which is buried within an embedded node. Bring it to the surface with a bit of syntatic sugar. Note this was intended to be split from commit ad579002c8ec ("drm: Add drm_mm_for_each_node_safe()") before being applied, but my timing sucks. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Cc: Daniel Vetter --- drivers/gpu/drm/drm_mm.c | 8 include/drm/drm_mm.h | 18 +++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 7573661302a4..4257c86cc305 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -138,7 +138,7 @@ static void show_leaks(struct drm_mm *mm) if (!buf) return; - list_for_each_entry(node, __drm_mm_nodes(mm), node_list) { + list_for_each_entry(node, drm_mm_nodes(mm), node_list) { struct stack_trace trace = { .entries = entries, .max_entries = STACKDEPTH @@ -320,7 +320,7 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node) if (hole->start < end) return -ENOSPC; } else { - hole = list_entry(__drm_mm_nodes(mm), typeof(*hole), node_list); + hole = list_entry(drm_mm_nodes(mm), typeof(*hole), node_list); } hole = list_last_entry(&hole->node_list, typeof(*hole), node_list); @@ -883,7 +883,7 @@ EXPORT_SYMBOL(drm_mm_scan_remove_block); */ bool drm_mm_clean(const struct drm_mm *mm) { - const struct list_head *head = __drm_mm_nodes(mm); + const struct list_head *head = drm_mm_nodes(mm); return (head->next->next == head); } @@ -929,7 +929,7 @@ EXPORT_SYMBOL(drm_mm_init); */ void drm_mm_takedown(struct drm_mm *mm) { - if (WARN(!list_empty(__drm_mm_nodes(mm)), + if (WARN(!list_empty(drm_mm_nodes(mm)), "Memory manager not clean during takedown.\n")) show_leaks(mm); diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 5c7f15875b6a..f6a68ed5ecaf 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -180,7 +180,19 @@ static inline u64 drm_mm_hole_node_end(const struct drm_mm_node *hole_node) return __drm_mm_hole_node_end(hole_node); } -#define __drm_mm_nodes(mm) (&(mm)->head_node.node_list) +/** + * drm_mm_nodes - list of nodes under the drm_mm range manager + * @mm: the struct drm_mm range manger + * + * As the drm_mm range manager hides its node_list deep with its + * structure, extracting it looks painful and repetitive. This is + * not expected to be used outside of the drm_mm_for_each_node() + * macros and similar internal functions. + * + * Returns: + * The node list, may be empty. + */ +#define drm_mm_nodes(mm) (&(mm)->head_node.node_list) /** * drm_mm_for_each_node - iterator to walk over all allocated nodes @@ -191,7 +203,7 @@ static inline u64 drm_mm_hole_node_end(const struct drm_mm_node *hole_node) * with list_for_each, so not save against removal of elements. */ #define drm_mm_for_each_node(entry, mm) \ - list_for_each_entry(entry, __drm_mm_nodes(mm), node_list) + list_for_each_entry(entry, drm_mm_nodes(mm), node_list) /** * drm_mm_for_each_node_safe - iterator to walk over all allocated nodes @@ -203,7 +215,7 @@ static inline u64 drm_mm_hole_node_end(const struct drm_mm_node *hole_node) * with list_for_each_safe, so save against removal of elements. */ #define drm_mm_for_each_node_safe(entry, next, mm) \ - list_for_each_entry_safe(entry, next, __drm_mm_nodes(mm), node_list) + list_for_each_entry_safe(entry, next, drm_mm_nodes(mm), node_list) #define __drm_mm_for_each_hole(entry, mm, hole_start, hole_end, backwards) \ for (entry = list_entry((backwards) ? (mm)->hole_stack.prev : (mm)->hole_stack.next, struct drm_mm_node, hole_stack); \ -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix use after free in logical_render_ring_init
On 16/12/2016 13:53, Patchwork wrote: == Series Details == Series: drm/i915: Fix use after free in logical_render_ring_init URL : https://patchwork.freedesktop.org/series/16918/ State : success == Summary == Series 16918v1 drm/i915: Fix use after free in logical_render_ring_init https://patchwork.freedesktop.org/api/1.0/series/16918/revisions/1/mbox/ Test gem_busy: Subgroup basic-hang-default: fail -> PASS (fi-hsw-4770r) fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:247 pass:222 dwarn:0 dfail:0 fail:0 skip:25 fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20 fi-skl-6770hqtotal:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 e5dd5ad8937cd82ea07bde765bde5b4c09d81dcb drm-tip: 2016y-12m-16d-09h-25m-11s UTC integration manifest 899a8cc4 drm/i915: Fix use after free in logical_render_ring_init Pushed, thanks for the review. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 20/40] drm: kselftest for drm_mm and color eviction
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > Check that after applying the driver's color adjustment, eviction > scanning find a suitable hole. > > Signed-off-by: Chris Wilson > +static int evict_color(struct drm_mm *mm, > + struct evict_node *nodes, > + unsigned int *order, > + unsigned int count, > + unsigned int size, > + unsigned int alignment, > + unsigned long color, > + const struct insert_mode *mode) expect_evict_color? evict_color gave me an impression of single-way action (when reading the next patch). Just a nitpick, so; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 21/40] drm: kselftest for drm_mm and restricted color eviction
On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > Check that after applying the driver's color adjustment, restricted > eviction scanning find a suitable hole. > > Signed-off-by: Chris Wilson > +static int igt_color_evict_range(void *ignored) > +{ > + for (mode = evict_modes; mode->name; mode++) { > + for (n = 1; n <= range_size; n <<= 1) { > + drm_random_reorder(order, range_size, &prng); > + err = evict_color(&mm, range_start, range_end, > + nodes, order, total_size, > + n, 1, color++, > + mode); > + if (err) { > + pr_err("%s evict_color(size=%u) failed for > range [%x, %x]\n", > + mode->name, n, range_start, range_end); > + goto out; > + } This evict_color + error check again repeats, could maybe be helper. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 29/40] drm: Rename prev_node to hole in drm_mm_scan_add_block()
On pe, 2016-12-16 at 07:47 +, Chris Wilson wrote: > Acknowledging that we were building up the hole was more useful to me > when reading the code, than knowing the relationship between this node > and the previous node. > > Signed-off-by: Chris Wilson I'm not purely against, so if you think it's more intuitive; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
On Thu, Dec 15, 2016 at 10:42:53PM +, Chris Wilson wrote: > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote: > > From: Peter Antoine > > > > This patch will allow for getparams to return the status of the HuC. > > As the HuC has to be validated by the GuC this patch uses the validated > > status to show when the HuC is loaded and ready for use. You cannot use > > the loaded status as with the GuC as the HuC is verified after it is > > loaded and is not usable until it is verified. > > > > v2: removed the forewakes as the registers are already force-woken. > > (T.Ursulin) > > v4: rebased. > > v5: rebased on top of drm-tip. > > v6: rebased. Removed any reference to intel_huc.h > > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS. > > Remove intel_is_huc_valid() since it is used only in one place. > > Put the case of I915_PARAM_HAS_HUC() in the right place. > > > > Signed-off-by: Peter Antoine > > Reviewed-by: Arkadiusz Hiler > > --- > > drivers/gpu/drm/i915/i915_drv.c | 4 > > drivers/gpu/drm/i915/intel_huc_loader.c | 1 - > > include/uapi/drm/i915_drm.h | 1 + > > 3 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 85a47c2..0bc016d 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -49,6 +49,7 @@ > > #include "i915_trace.h" > > #include "i915_vgpu.h" > > #include "intel_drv.h" > > +#include "intel_uc.h" > > > > static struct drm_driver driver; > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void > > *data, > > case I915_PARAM_MIN_EU_IN_POOL: > > value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool; > > break; > > + case I915_PARAM_HUC_STATUS: > > + value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED; > > Same question as last time: does the device need to be awake? We know is > one of the GT power wells, so presumably we need an rpm_get/rpm_put as > well to access the register. > -Chris I get: [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE 24704 [ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device [ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended [ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704 [ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device consistently from: value = I915_READ(HUC_STATUS2); DRM_DEBUG_DRIVER("HUC_STATUS2 PRE %d\n", value); i915_pm_ops.runtime_suspend(dev_priv->drm.dev); value = I915_READ(HUC_STATUS2); DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value); i915_pm_ops.runtime_resume(dev_priv->drm.dev); > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/2] drm: link status property and DP link training failure handling
On Fri, 16 Dec 2016, Daniel Vetter wrote: > On Fri, Dec 16, 2016 at 12:29:05PM +0200, Jani Nikula wrote: >> The two remaining patches from [1], rebased. >> >> BR, >> Jani. >> >> >> [1] >> 1480984058-552-1-git-send-email-manasi.d.navare@intel.com">http://mid.mail-archive.com/1480984058-552-1-git-send-email-manasi.d.navare@intel.com > > Just for the record, I think the only thing missing here is the Xorg > review on the -modesetting patch. As soon as we have that I can vacuum > this up (probably best through drm-misc, but not sure). Yeah I rebased this (and provided a debug hack privately) so Martin can test the modesetting changes. BR, Jani. > -Daniel > >> >> >> Manasi Navare (2): >> drm: Add a new connector atomic property for link status >> drm/i915: Implement Link Rate fallback on Link training failure >> >> drivers/gpu/drm/drm_atomic.c | 16 + >> drivers/gpu/drm/drm_atomic_helper.c | 15 >> drivers/gpu/drm/drm_connector.c | 52 >> +++ >> drivers/gpu/drm/i915/intel_dp.c | 27 ++ >> drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++-- >> drivers/gpu/drm/i915/intel_drv.h | 3 ++ >> include/drm/drm_connector.h | 19 ++ >> include/drm/drm_mode_config.h | 5 +++ >> include/uapi/drm/drm_mode.h | 4 +++ >> 9 files changed, 161 insertions(+), 2 deletions(-) >> >> -- >> 2.1.4 >> >> ___ >> dri-devel mailing list >> dri-de...@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 20/40] drm: kselftest for drm_mm and color eviction
On Fri, Dec 16, 2016 at 04:38:12PM +0200, Joonas Lahtinen wrote: > On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > > Check that after applying the driver's color adjustment, eviction > > scanning find a suitable hole. > > > > Signed-off-by: Chris Wilson > > > > > +static int evict_color(struct drm_mm *mm, > > + struct evict_node *nodes, > > + unsigned int *order, > > + unsigned int count, > > + unsigned int size, > > + unsigned int alignment, > > + unsigned long color, > > + const struct insert_mode *mode) > > expect_evict_color? evict_color gave me an impression of single-way > action (when reading the next patch). I thought of this and evict_something() as being the final level test runner (with the callers iterating over various paramters). expect_insert_fail/expect_insert_in_range_fail definitely are the single action check result matches expectations (return bool). Not convinced these two fall into the same category, so I'll leave as is. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/9] drm/i915: Don't populate plane->plane for cursors and sprites
On Fri, 2016-12-16 at 12:07 -0200, Paulo Zanoni wrote: > Em Qui, 2016-12-15 às 21:59 +0200, Ville Syrjälä escreveu: > > > > On Thu, Dec 15, 2016 at 05:50:02PM -0200, Paulo Zanoni wrote: > > > > > > > > > Em Qui, 2016-12-15 às 21:11 +0200, Ville Syrjälä escreveu: > > > > > > > > > > > > On Thu, Dec 15, 2016 at 05:05:05PM -0200, Paulo Zanoni wrote: > > > > > > > > > > > > > > > > > > > > Em Ter, 2016-11-22 às 18:02 +0200, ville.syrj...@linux.intel.co > > > > > m > > > > > escreveu: > > > > > > > > > > > > > > > > > > > > > > > > From: Ville Syrjälä > > > > > > > > > > > > With plane->plane now purely reserved for the primary planes, > > > > > > let's > > > > > > not even populate it for cursors and sprites. Let's switch > > > > > > the > > > > > > type > > > > > > to enum plane as well since it's no longer being abused for > > > > > > anything > > > > > > else. > > > > > Since it's a primary plane thing, I think it makes more sense > > > > > to > > > > > just > > > > > leave it at struct intel_crtc instead of having a field that's > > > > > unused > > > > > and doesn't make sense in some cases. > > > > Primary plane aren't really primary planes on old platforms > > > > though. > > > > That's just a software concept that has no bearing on the > > > > hardware. > > > Please explain more since that's not my understanding. I just > > > opened > > > the gen 2 and 3 spec docs and they do contain tons of "primary > > > plane" > > > references. I know that the hardware is a little more flexible > > > there, > > Yep. Planes can be moved to any pipe in general. > > > > > > > > > > > but as far as I understand we don't even implement that. > > Not at the moment. But I have plans... > > > > > > > > > > > Besides, both > > > our driver and our API do have the concept of a primary plane, so > > > it > > > makes sense for the code to be organized that way, IMHO. > > A plane is a plane, a pipe is a pipe. IMO you're trying to make it > > more confusing by mixing up the two. > I think that goes against your original argument to remove the plane- > > > > plane assignment for everything except for primary planes. > Anyway, your argumentation is very short and dismissive, it's hard to > understand what exactly you are thinking and try to move the discussion > forward. > > I suppose we'll just have to agree to disagree here. In one solution we > create an inconsistency where the plane struct has a field that's only > applicable and can only be used to some specific planes (even though > "every plane is a plane"). In the other solution we create the > inconsistency where the crtc<->plane connection is assigned in the way > not chosen by the hardware and this may be a problem if we ever get to > implement the plane assignment flexibility that's allowed by the > hardware. Maybe having a 3rd opinion here would help. I find the two plane fields in struct intel_plane a bit confusing, but I don't think the right solution is to leave that in intel_crtc. If I understand correctly, we have a problem of uniquely identifying a plane, since that takes different forms in different platforms. Neither of the generically named enums, enum plane and enum plane_id, is sufficiently generic. Would it make sense to turn things up-side down? Now we are trying to make a crtc be able to find the proper registers for a plane. Instead we could just not do it and define everything in terms of functions that take planes. Then we could hide the details of mapping some bit of information in intel_plane to the appropriate registers in platform-dependent vfuncs. And perhaps give those enums a platform-dependent name, i.e., enum i8xx_plane_id, enum skl_plane_id, etc. Anyway, I haven't looked much into it, so just ignore me if I'm way off. Ander > > > > > > > > > > > > > Besides, I > > > think our code organizations should always better fit the new > > > hardware, > > > since otherwise we'll make i915.ko development even harder than it > > > is > > > for new contributors/employees. > > If you don't mix planes and pipes there can't be any confusion. > > > > > > > > > > > > > > (And when I see those specs and realize how different the HW was, > > > then > > > I remember that in order to explain to the new people why some > > > things > > > are the way they are in our code I have to explain how the HW was > > > 10 > > > years ago, I start thinking again that maybe we should just have > > > i915- > > > old.ko and i915-new.ko, since either we make our > > > abstractions/design > > > fit one thing or we make it fit another...) > > From the register POV hw was almost identical up to BDW at least. > > It's > > just a few bits (the pipe selector) that vanished from the registers. > > SKL+ is a little more different but still the registers even live in > > the > > same address. > > > > Also given what recent history has shown us, I wouldn't be at all > > surprised if we would go back to a flexible plane<->pipe assignment > > in the har
Re: [Intel-gfx] [PATCH v2 12/40] drm: kselftest for drm_mm_insert_node()
On Fri, Dec 16, 2016 at 04:02:12PM +0200, Joonas Lahtinen wrote: > On pe, 2016-12-16 at 07:46 +, Chris Wilson wrote: > > Exercise drm_mm_insert_node(), check that we can't overfill a range and > > that the lists are correct after reserving/removing. > > > > v2: Extract helpers for the repeated tests > > v3: Iterate over all allocation flags > > > > Signed-off-by: Chris Wilson > > > > > +static u64 misaligned(struct drm_mm_node *node, u64 alignment) > > I'm not sure if 'misalignment' would be better name or not. This makes > me think of bool returning one. > > > +static bool expect_insert_fail(struct drm_mm *mm, u64 size) > > +{ > > + struct drm_mm_node tmp = {}; > > + int err; > > + > > + err = drm_mm_insert_node(mm, &tmp, size, 0, DRM_MM_SEARCH_DEFAULT); > > + if (err != -ENOSPC) { > > For speed (this function gets called a lot); > > if (likely(err == -ENOSPC)) > return true; > > > +static int __igt_insert(unsigned int count, u64 size) > > +{ > > > > > > > + for (mode = insert_modes; mode->name; mode++) { > > + for (n = 0; n < count; n++) { > > + node = &nodes[n]; > > + err = drm_mm_insert_node_generic(&mm, node, size, 0, n, > > + mode->search_flags, > > + mode->create_flags); > > + if (err || !assert_node(node, &mm, size, 0, n)) { > > + pr_err("%s insert failed, size %llu step %d\n", > > + mode->name, size, n); > > + ret = err ?: -EINVAL; > > + goto out; > > + } > > This construct is three times in this patch; Could be > expect_insert_generic? In this patch alone, indeed it gets used again and again. I resisted making it an insert and check function simply because I didn't want to fall into the trap of using it everywhere and so missing out exercising drm_mm_insert_node and friends. However, 3 occurences of the same in one patch... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/13] drm/irq: drm_legacy_ prefix for legacy ioctls
On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter wrote: > Spotted while auditing our ioctl table. Also nuke the > not-really-kerneldoc comments, we don't document internals and > definitely don't want to mislead people with the old dragons. > > I think with this all the legacy ioctls now have proper drm_legacy_ > prefixes. > Reviewed-by: Sean Paul > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_internal.h | 8 > drivers/gpu/drm/drm_ioctl.c| 4 ++-- > drivers/gpu/drm/drm_irq.c | 30 -- > 3 files changed, 10 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h > index db80ec860e33..a6213f814345 100644 > --- a/drivers/gpu/drm/drm_internal.h > +++ b/drivers/gpu/drm/drm_internal.h > @@ -58,10 +58,10 @@ extern unsigned int drm_timestamp_monotonic; > /* IOCTLS */ > int drm_wait_vblank(struct drm_device *dev, void *data, > struct drm_file *filp); > -int drm_control(struct drm_device *dev, void *data, > - struct drm_file *file_priv); > -int drm_modeset_ctl(struct drm_device *dev, void *data, > - struct drm_file *file_priv); > +int drm_legacy_irq_control(struct drm_device *dev, void *data, > + struct drm_file *file_priv); > +int drm_legacy_modeset_ctl(struct drm_device *dev, void *data, > + struct drm_file *file_priv); > > /* drm_auth.c */ > int drm_getmagic(struct drm_device *dev, void *data, > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index e21a18ac968e..a16b6fbd1004 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -581,7 +581,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > DRM_IOCTL_DEF(DRM_IOCTL_FREE_BUFS, drm_legacy_freebufs, DRM_AUTH), > DRM_IOCTL_DEF(DRM_IOCTL_DMA, drm_legacy_dma_ioctl, DRM_AUTH), > > - DRM_IOCTL_DEF(DRM_IOCTL_CONTROL, drm_control, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > + DRM_IOCTL_DEF(DRM_IOCTL_CONTROL, drm_legacy_irq_control, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > > #if IS_ENABLED(CONFIG_AGP) > DRM_IOCTL_DEF(DRM_IOCTL_AGP_ACQUIRE, drm_agp_acquire_ioctl, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > @@ -599,7 +599,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > > DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, > DRM_MASTER|DRM_UNLOCKED), > > - DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0), > + DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_legacy_modeset_ctl, 0), > > DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 273625a85036..feb091310ffe 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -579,19 +579,8 @@ int drm_irq_uninstall(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_irq_uninstall); > > -/* > - * IRQ control ioctl. > - * > - * \param inode device inode. > - * \param file_priv DRM file private. > - * \param cmd command. > - * \param arg user argument, pointing to a drm_control structure. > - * \return zero on success or a negative number on failure. > - * > - * Calls irq_install() or irq_uninstall() according to \p arg. > - */ > -int drm_control(struct drm_device *dev, void *data, > - struct drm_file *file_priv) > +int drm_legacy_irq_control(struct drm_device *dev, void *data, > + struct drm_file *file_priv) > { > struct drm_control *ctl = data; > int ret = 0, irq; > @@ -1442,19 +1431,8 @@ static void drm_legacy_vblank_post_modeset(struct > drm_device *dev, > } > } > > -/* > - * drm_modeset_ctl - handle vblank event counter changes across mode switch > - * @DRM_IOCTL_ARGS: standard ioctl arguments > - * > - * Applications should call the %_DRM_PRE_MODESET and %_DRM_POST_MODESET > - * ioctls around modesetting so that any lost vblank events are accounted > for. > - * > - * Generally the counter will reset across mode sets. If interrupts are > - * enabled around this call, we don't have to do anything since the counter > - * will have already been incremented. > - */ > -int drm_modeset_ctl(struct drm_device *dev, void *data, > - struct drm_file *file_priv) > +int drm_legacy_modeset_ctl(struct drm_device *dev, void *data, > + struct drm_file *file_priv) > { > struct drm_modeset_ctl *modeset = data; > unsigned int pipe; > -- > 2.11.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/13] drm: Move atomic debugfs functions into drm_crtc_internal.h
On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter wrote: > This is not driver interface stuff. > Reviewed-by: Sean Paul > Fixes: 6559c901cb48 ("drm/atomic: add debugfs file to dump out atomic state") > Cc: Rob Clark > Cc: Sean Paul > Cc: Daniel Vetter > Cc: Jani Nikula > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_crtc_internal.h | 6 ++ > drivers/gpu/drm/drm_debugfs.c | 1 + > include/drm/drm_atomic.h| 6 -- > 3 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h > b/drivers/gpu/drm/drm_crtc_internal.h > index cdf6860c9d22..0b8454c7dc00 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -174,6 +174,12 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev, >void *data, struct drm_file *file_priv); > > /* drm_atomic.c */ > +#ifdef CONFIG_DEBUG_FS > +struct drm_minor; > +int drm_atomic_debugfs_init(struct drm_minor *minor); > +int drm_atomic_debugfs_cleanup(struct drm_minor *minor); > +#endif > + > int drm_atomic_get_property(struct drm_mode_object *obj, > struct drm_property *property, uint64_t *val); > int drm_mode_atomic_ioctl(struct drm_device *dev, > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 2e3e46a53805..37fd612d57a6 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -38,6 +38,7 @@ > #include > #include > #include "drm_internal.h" > +#include "drm_crtc_internal.h" > > #if defined(CONFIG_DEBUG_FS) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 01d02e1935b1..c35b4aba36b6 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -379,12 +379,6 @@ int __must_check drm_atomic_nonblocking_commit(struct > drm_atomic_state *state); > > void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > -#ifdef CONFIG_DEBUG_FS > -struct drm_minor; > -int drm_atomic_debugfs_init(struct drm_minor *minor); > -int drm_atomic_debugfs_cleanup(struct drm_minor *minor); > -#endif > - > #define for_each_connector_in_state(__state, connector, connector_state, > __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && > \ > -- > 2.11.0 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/13] drm: locking&new iterators for connector_list
On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter wrote: > The requirements for connector_list locking are a bit tricky: > - We need to be able to jump over zombie conectors (i.e. with refcount > == 0, but not yet removed from the list). If instead we require that > there's no zombies on the list then the final kref_put must happen > under the list protection lock, which means that locking context > leaks all over the place. Not pretty - better to deal with zombies > and wrap the locking just around the list_del in the destructor. > > - When we walk the list we must _not_ hold the connector list lock. We > walk the connector list at an absolutely massive amounts of places, > if all those places can't ever call drm_connector_unreference the > code would get unecessarily complicated. > > - connector_list needs it own lock, again too many places that walk it > that we could reuse e.g. mode_config.mutex without resulting in > inversions. > > - Lots of code uses these loops to look-up a connector, i.e. they want > to be able to call drm_connector_reference. But on the other hand we > want connectors to stay on that list until they're dead (i.e. > connector_list can't hold a full reference), which means despite the > "can't hold lock for the loop body" rule we need to make sure a > connector doesn't suddenly become a zombie. > > At first Dave&I discussed various horror-show approaches using srcu, > but turns out it's fairly easy: > > - For the loop body we always hold an additional reference to the > current connector. That means it can't zombify, and it also means > it'll stay on the list, which means we can use it as our iterator to > find the next connector. > > - When we try to find the next connector we only have to jump over > zombies. To make sure we don't chase bad pointers that entire loop > is protected with the new connect_list_lock spinlock. And because we > know that we're starting out with a non-zombie (need to drop our > reference for the old connector only after we have our new one), > we're guranteed to still be on the connector_list and either find > the next non-zombie or complete the iteration. > > - Only downside is that we need to make sure that the temporary > reference for the loop body doesn't leak. iter_get/put() functions + > lockdep make sure that's the case. > > - To avoid a flag day the new iterator macro has an _iter postfix. We > can rename it back once all the users of the unsafe version are gone > (there's about 100 list walkers for the connector_list). > > For now this patch only converts all the list walking in the core, > leaving helpers and drivers for later patches. The nice thing is that > we can now finally remove 2 FIXME comments from the > register/unregister functions. > > v2: > - use irqsafe spinlocks, so that we can use this in drm_state_dump > too. > - nuke drm_modeset_lock_all from drm_connector_init, now entirely > cargo-culted nonsense. > > v3: > - do {} while (!kref_get_unless_zero), makes for a tidier loop (Dave). > - pretty kerneldoc > - add EXPORT_SYMBOL, helpers&drivers are supposed to use this. > > v4: Change lockdep annotations to only check whether we release the > iter fake lock again (i.e. make sure that iter_put is called), but > not check any locking dependecies itself. That seams to require a > recursive read lock in trylock mode. > > Cc: Dave Airlie > Cc: Chris Wilson Reviewed-by: Sean Paul > Signed-off-by: Daniel Vetter ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/13] drm: Drop locking cargo-cult from drm_mode_config_init
On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter wrote: > This is single-threaded setup code, no need for locks. And anyway, > all properties need to be set up before the driver is registered > anyway, they can't be hot-added. > Reviewed-by: Sean Paul > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_mode_config.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mode_config.c > b/drivers/gpu/drm/drm_mode_config.c > index b1e8bbceaf39..85a25fd9eff8 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -374,9 +374,7 @@ void drm_mode_config_init(struct drm_device *dev) > idr_init(&dev->mode_config.tile_idr); > ida_init(&dev->mode_config.connector_ida); > > - drm_modeset_lock_all(dev); > drm_mode_create_standard_properties(dev); > - drm_modeset_unlock_all(dev); > > /* Just to be sure */ > dev->mode_config.num_fb = 0; > -- > 2.11.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/13] drm: prevent double-(un)registration for connectors
On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter wrote: > If we're unlucky then the registration from a hotplugged connector > might race with the final registration step on driver load. And since > MST topology discover is asynchronous that's even somewhat likely. > > v2: Also update the kerneldoc for @registered! > > Cc: Chris Wilson > Reported-by: Chris Wilson With Chris' suggested improvements Reviewed-by: Sean Paul > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_connector.c | 18 +- > include/drm/drm_connector.h | 12 +++- > 2 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index b4e09b00..0d4728704099 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -223,6 +223,7 @@ int drm_connector_init(struct drm_device *dev, > > INIT_LIST_HEAD(&connector->probed_modes); > INIT_LIST_HEAD(&connector->modes); > + mutex_init(&connector->mutex); > connector->edid_blob_ptr = NULL; > connector->status = connector_status_unknown; > > @@ -373,14 +374,15 @@ EXPORT_SYMBOL(drm_connector_cleanup); > */ > int drm_connector_register(struct drm_connector *connector) > { > - int ret; > + int ret = 0; > > + mutex_lock(&connector->mutex); > if (connector->registered) > - return 0; > + goto unlock; > > ret = drm_sysfs_connector_add(connector); > if (ret) > - return ret; > + goto unlock; > > ret = drm_debugfs_connector_add(connector); > if (ret) { > @@ -396,12 +398,14 @@ int drm_connector_register(struct drm_connector > *connector) > drm_mode_object_register(connector->dev, &connector->base); > > connector->registered = true; > - return 0; > + goto unlock; > > err_debugfs: > drm_debugfs_connector_remove(connector); > err_sysfs: > drm_sysfs_connector_remove(connector); > +unlock: > + mutex_unlock(&connector->mutex); > return ret; > } > EXPORT_SYMBOL(drm_connector_register); > @@ -414,8 +418,11 @@ EXPORT_SYMBOL(drm_connector_register); > */ > void drm_connector_unregister(struct drm_connector *connector) > { > - if (!connector->registered) > + mutex_lock(&connector->mutex); > + if (!connector->registered) { > + mutex_unlock(&connector->mutex); > return; > + } > > if (connector->funcs->early_unregister) > connector->funcs->early_unregister(connector); > @@ -424,6 +431,7 @@ void drm_connector_unregister(struct drm_connector > *connector) > drm_debugfs_connector_remove(connector); > > connector->registered = false; > + mutex_unlock(&connector->mutex); > } > EXPORT_SYMBOL(drm_connector_unregister); > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 0e41a2e184a9..a24559ef8bb7 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -559,7 +559,6 @@ struct drm_cmdline_mode { > * @interlace_allowed: can this connector handle interlaced modes? > * @doublescan_allowed: can this connector handle doublescan? > * @stereo_allowed: can this connector handle stereo modes? > - * @registered: is this connector exposed (registered) with userspace? > * @modes: modes available on this connector (from fill_modes() + user) > * @status: one of the drm_connector_status enums (connected, not, or > unknown) > * @probed_modes: list of modes derived directly from the display > @@ -608,6 +607,13 @@ struct drm_connector { > char *name; > > /** > +* @mutex: Lock for general connector state, but currently only > protects > +* @registered. Most of the connector state is still protected by the > +* mutex in &drm_mode_config. > +*/ > + struct mutex mutex; > + > + /** > * @index: Compacted connector index, which matches the position > inside > * the mode_config.list for drivers not supporting hot-add/removing. > Can > * be used as an array index. It is invariant over the lifetime of the > @@ -620,6 +626,10 @@ struct drm_connector { > bool interlace_allowed; > bool doublescan_allowed; > bool stereo_allowed; > + /** > +* @registered: Is this connector exposed (registered) with userspace? > +* Protected by @mutex. > +*/ > bool registered; > struct list_head modes; /* list of modes on this connector */ > > -- > 2.11.0 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://l
Re: [Intel-gfx] [PATCH] drm: Convert all helpers to drm_connector_list_iter
On Thu, Dec 15, 2016 at 10:58 AM, Daniel Vetter wrote: > Mostly nothing special (except making sure that really all error paths > and friends call iter_put). > > v2: Don't forget the raw connector_list walking in > drm_helper_move_panel_connectors_to_head. That one unfortunately can't > be converted to the iterator helpers, but since it's just some list > splicing best to just wrap the entire thing up in one critical > section. > > v3: Bail out after iter_put (Harry). > > Cc: Harry Wentland Reviewed-by: Sean Paul > Reviewed-by: Harry Wentland > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_atomic_helper.c | 39 > drivers/gpu/drm/drm_crtc_helper.c| 49 > > drivers/gpu/drm/drm_fb_helper.c | 12 ++--- > drivers/gpu/drm/drm_modeset_helper.c | 2 ++ > drivers/gpu/drm/drm_plane_helper.c | 5 +++- > drivers/gpu/drm/drm_probe_helper.c | 18 - > 6 files changed, 92 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 23767df72615..e2e15a9903a9 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -94,9 +94,10 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, > { > struct drm_connector_state *conn_state; > struct drm_connector *connector; > + struct drm_connector_list_iter conn_iter; > struct drm_encoder *encoder; > unsigned encoder_mask = 0; > - int i, ret; > + int i, ret = 0; > > /* > * First loop, find all newly assigned encoders from the connectors > @@ -144,7 +145,8 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, > * and the crtc is disabled if no encoder is left. This preserves > * compatibility with the legacy set_config behavior. > */ > - drm_for_each_connector(connector, state->dev) { > + drm_connector_list_iter_get(state->dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > struct drm_crtc_state *crtc_state; > > if (drm_atomic_get_existing_connector_state(state, connector)) > @@ -160,12 +162,15 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, > connector->state->crtc->base.id, > connector->state->crtc->name, > connector->base.id, connector->name); > - return -EINVAL; > + ret = -EINVAL; > + goto out; > } > > conn_state = drm_atomic_get_connector_state(state, connector); > - if (IS_ERR(conn_state)) > - return PTR_ERR(conn_state); > + if (IS_ERR(conn_state)) { > + ret = PTR_ERR(conn_state); > + goto out; > + } > > DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], > disabling [CONNECTOR:%d:%s]\n", > encoder->base.id, encoder->name, > @@ -176,19 +181,21 @@ static int handle_conflicting_encoders(struct > drm_atomic_state *state, > > ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); > if (ret) > - return ret; > + goto out; > > if (!crtc_state->connector_mask) { > ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, > NULL); > if (ret < 0) > - return ret; > + goto out; > > crtc_state->active = false; > } > } > +out: > + drm_connector_list_iter_put(&conn_iter); > > - return 0; > + return ret; > } > > static void > @@ -2442,6 +2449,7 @@ int drm_atomic_helper_disable_all(struct drm_device > *dev, > { > struct drm_atomic_state *state; > struct drm_connector *conn; > + struct drm_connector_list_iter conn_iter; > int err; > > state = drm_atomic_state_alloc(dev); > @@ -2450,7 +2458,8 @@ int drm_atomic_helper_disable_all(struct drm_device > *dev, > > state->acquire_ctx = ctx; > > - drm_for_each_connector(conn, dev) { > + drm_connector_list_iter_get(dev, &conn_iter); > + drm_for_each_connector_iter(conn, &conn_iter) { > struct drm_crtc *crtc = conn->state->crtc; > struct drm_crtc_state *crtc_state; > > @@ -2468,6 +2477,7 @@ int drm_atomic_helper_disable_all(struct drm_device > *dev, > > err = drm_atomic_commit(state); > free: > + drm_connector_list_iter_put(&conn_iter); > drm_atomic_state_put(state);
Re: [Intel-gfx] [PATCH 07/13] drm: Clean up connectors by unreferencing them
On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter wrote: > Only static connectors should be left at this point, and we should be > able to clean them out by simply dropping that last reference still > around from drm_connector_init. > > If that leaves anything behind then we have a driver bug. > > Doing the final cleanup this way also allows us to use > drm_connector_iter, removing the very last place where we walk > connector_list explicitly in drm core&helpers. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_mode_config.c | 15 +++ > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mode_config.c > b/drivers/gpu/drm/drm_mode_config.c > index 747a26df0e90..a942536abd60 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -397,7 +397,8 @@ EXPORT_SYMBOL(drm_mode_config_init); > */ > void drm_mode_config_cleanup(struct drm_device *dev) > { > - struct drm_connector *connector, *ot; > + struct drm_connector *connector; > + struct drm_connector_list_iter conn_iter; > struct drm_crtc *crtc, *ct; > struct drm_encoder *encoder, *enct; > struct drm_framebuffer *fb, *fbt; > @@ -410,10 +411,16 @@ void drm_mode_config_cleanup(struct drm_device *dev) > encoder->funcs->destroy(encoder); > } > > - list_for_each_entry_safe(connector, ot, > -&dev->mode_config.connector_list, head) { > - connector->funcs->destroy(connector); > + drm_connector_list_iter_get(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > + /* drm_connector_list_iter holds an full reference to the > +* current connector itself, which means it is inherently safe > +* against unreferencing the current connector - but not > against > +* deleting it right away. */ pedantic nit: doesn't conform to CodingStyle Reviewed-by: Sean Paul > + drm_connector_unreference(connector); > } > + drm_connector_list_iter_put(&conn_iter); > + WARN_ON(!list_empty(&dev->mode_config.connector_list)); > > list_for_each_entry_safe(property, pt, > &dev->mode_config.property_list, > head) { > -- > 2.11.0 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/13] drm: Tighten locking in drm_mode_getconnector
On Tue, Dec 13, 2016 at 6:08 PM, Daniel Vetter wrote: > - Modeset state needs mode_config->connection mutex, that covers > figuring out the encoder, and reading properties (since in the > atomic case those need to look at connector->state). > > - Don't hold any locks for stuff that's invariant (i.e. possible > connectors). > > - Same for connector lookup and unref, those don't need any locks. > > - And finally the probe stuff is only protected by mode_config->mutex. > > While at it updated the kerneldoc for these fields in drm_connector > and add docs explaining what's protected by which locks. > Reviewed-by: Sean Paul > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_connector.c | 92 > - > include/drm/drm_connector.h | 23 +-- > 2 files changed, 63 insertions(+), 52 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 0d4728704099..44b556d5d40c 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1160,43 +1160,65 @@ int drm_mode_getconnector(struct drm_device *dev, > void *data, > > memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo)); > > - mutex_lock(&dev->mode_config.mutex); > - > connector = drm_connector_lookup(dev, out_resp->connector_id); > - if (!connector) { > - ret = -ENOENT; > - goto out_unlock; > - } > + if (!connector) > + return -ENOENT; > + > + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > + encoder = drm_connector_get_encoder(connector); > + if (encoder) > + out_resp->encoder_id = encoder->base.id; > + else > + out_resp->encoder_id = 0; > + > + ret = drm_mode_object_get_properties(&connector->base, > file_priv->atomic, > + (uint32_t __user *)(unsigned > long)(out_resp->props_ptr), > + (uint64_t __user *)(unsigned > long)(out_resp->prop_values_ptr), > + &out_resp->count_props); > + drm_modeset_unlock(&dev->mode_config.connection_mutex); > + if (ret) > + goto out_unref; > > for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) > if (connector->encoder_ids[i] != 0) > encoders_count++; > > + if ((out_resp->count_encoders >= encoders_count) && encoders_count) { > + copied = 0; > + encoder_ptr = (uint32_t __user *)(unsigned > long)(out_resp->encoders_ptr); > + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { > + if (connector->encoder_ids[i] != 0) { > + if (put_user(connector->encoder_ids[i], > +encoder_ptr + copied)) { > + ret = -EFAULT; > + goto out_unref; > + } > + copied++; > + } > + } > + } > + out_resp->count_encoders = encoders_count; > + > + out_resp->connector_id = connector->base.id; > + out_resp->connector_type = connector->connector_type; > + out_resp->connector_type_id = connector->connector_type_id; > + > + mutex_lock(&dev->mode_config.mutex); > if (out_resp->count_modes == 0) { > connector->funcs->fill_modes(connector, > dev->mode_config.max_width, > dev->mode_config.max_height); > } > > - /* delayed so we get modes regardless of pre-fill_modes state */ > - list_for_each_entry(mode, &connector->modes, head) > - if (drm_mode_expose_to_userspace(mode, file_priv)) > - mode_count++; > - > - out_resp->connector_id = connector->base.id; > - out_resp->connector_type = connector->connector_type; > - out_resp->connector_type_id = connector->connector_type_id; > out_resp->mm_width = connector->display_info.width_mm; > out_resp->mm_height = connector->display_info.height_mm; > out_resp->subpixel = connector->display_info.subpixel_order; > out_resp->connection = connector->status; > > - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > - encoder = drm_connector_get_encoder(connector); > - if (encoder) > - out_resp->encoder_id = encoder->base.id; > - else > - out_resp->encoder_id = 0; > + /* delayed so we get modes regardless of pre-fill_modes state */ > + list_for_each_entry(mode, &connector->modes, head) > + if (drm_mode_expose_to_userspace(mode, file_priv)) > + mode_count++; > > /* > * This ioctl is called twice, once to determine how much space is
[Intel-gfx] [drm-intel:drm-intel-nightly 922/930] drivers/gpu/drm/i915/i915_gem_gtt.c:2732:9: note: in expansion of macro 'list_first_entry_or_null'
tree: git://anongit.freedesktop.org/drm-intel drm-intel-nightly head: ca1c03136b168816ac65c5945776908e464fca6b commit: 45b186f111f1623b257d183920cd4aab16a1acd5 [922/930] drm: Constify the drm_mm API config: i386-randconfig-x005-201650 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout 45b186f111f1623b257d183920cd4aab16a1acd5 # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): In file included from include/linux/mutex.h:14:0, from include/linux/seq_file.h:7, from drivers/gpu/drm/i915/i915_gem_gtt.c:26: drivers/gpu/drm/i915/i915_gem_gtt.c: In function 'i915_gtt_color_adjust': >> include/linux/list.h:385:29: warning: initialization discards 'const' >> qualifier from pointer target type [-Wdiscarded-qualifiers] struct list_head *head__ = (ptr); \ ^ >> drivers/gpu/drm/i915/i915_gem_gtt.c:2732:9: note: in expansion of macro >> 'list_first_entry_or_null' node = list_first_entry_or_null(&node->node_list, ^~~~ vim +/list_first_entry_or_null +2732 drivers/gpu/drm/i915/i915_gem_gtt.c 307dc25b Chris Wilson 2016-08-05 2716/* Wait a bit, in hopes it avoids the hang */ 307dc25b Chris Wilson 2016-08-05 2717udelay(10); 307dc25b Chris Wilson 2016-08-05 2718} 307dc25b Chris Wilson 2016-08-05 2719} 5c042287 Ben Widawsky 2011-10-17 2720 03ac84f1 Chris Wilson 2016-10-28 2721dma_unmap_sg(kdev, pages->sgl, pages->nents, PCI_DMA_BIDIRECTIONAL); 7c2e6fdf Daniel Vetter 2010-11-06 2722 } 644ec02b Daniel Vetter 2012-03-26 2723 45b186f1 Chris Wilson 2016-12-16 2724 static void i915_gtt_color_adjust(const struct drm_mm_node *node, 42d6ab48 Chris Wilson 2012-07-26 2725 unsigned long color, 440fd528 Thierry Reding 2015-01-23 2726 u64 *start, 440fd528 Thierry Reding 2015-01-23 2727 u64 *end) 42d6ab48 Chris Wilson 2012-07-26 2728 { 42d6ab48 Chris Wilson 2012-07-26 2729if (node->color != color) 42d6ab48 Chris Wilson 2012-07-26 2730*start += 4096; 42d6ab48 Chris Wilson 2012-07-26 2731 2a1d7752 Chris Wilson 2016-07-26 @2732node = list_first_entry_or_null(&node->node_list, 42d6ab48 Chris Wilson 2012-07-26 2733 struct drm_mm_node, 42d6ab48 Chris Wilson 2012-07-26 2734 node_list); 2a1d7752 Chris Wilson 2016-07-26 2735if (node && node->allocated && node->color != color) 42d6ab48 Chris Wilson 2012-07-26 2736*end -= 4096; 42d6ab48 Chris Wilson 2012-07-26 2737 } fbe5d36e Ben Widawsky 2013-11-04 2738 f6b9d5ca Chris Wilson 2016-08-04 2739 int i915_gem_init_ggtt(struct drm_i915_private *dev_priv) 644ec02b Daniel Vetter 2012-03-26 2740 { :: The code at line 2732 was first introduced by commit :: 2a1d775201081c400d7e60ceb8e5ac887d11b1f7 drm/i915: Prefer list_first_entry_or_null :: TO: Chris Wilson :: CC: Chris Wilson --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915/DMC/GLK: Load DMC on GLK
From: Anusha Srivatsa This patch loads the DMC on GLK. There is a single firmware image for all steppings on a GLK. Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa Reviewed-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_csr.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 9cbb8d8..0085bc7 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -34,6 +34,10 @@ * low-power state and comes back to normal. */ +#define I915_CSR_GLK "i915/glk_dmc_ver1_01.bin" +MODULE_FIRMWARE(I915_CSR_GLK); +#define GLK_CSR_VERSION_REQUIRED CSR_VERSION(1, 1) + #define I915_CSR_KBL "i915/kbl_dmc_ver1_01.bin" MODULE_FIRMWARE(I915_CSR_KBL); #define KBL_CSR_VERSION_REQUIRED CSR_VERSION(1, 1) @@ -286,7 +290,9 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv, csr->version = css_header->version; - if (IS_KABYLAKE(dev_priv)) { + if (IS_GEMINILAKE(dev_priv)) { + required_version = GLK_CSR_VERSION_REQUIRED; + } else if (IS_KABYLAKE(dev_priv)) { required_version = KBL_CSR_VERSION_REQUIRED; } else if (IS_SKYLAKE(dev_priv)) { required_version = SKL_CSR_VERSION_REQUIRED; @@ -435,7 +441,9 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv) if (!HAS_CSR(dev_priv)) return; - if (IS_KABYLAKE(dev_priv)) + if (IS_GEMINILAKE(dev_priv)) + csr->fw_path = I915_CSR_GLK; + else if (IS_KABYLAKE(dev_priv)) csr->fw_path = I915_CSR_KBL; else if (IS_SKYLAKE(dev_priv)) csr->fw_path = I915_CSR_SKL; -- 2.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915/glk: Convert a few more IS_BROXTON() to IS_GEN9_LP()
From: Michel Thierry Commit 89b3c3c7ee9d ("drm/i915/glk: Reuse broxton's cdclk code for GLK") missed a few of occurences of IS_BROXTON() that should have been coverted to IS_GEN9_LP(). Fixes: 89b3c3c7ee9d ("drm/i915/glk: Reuse broxton's cdclk code for GLK") Cc: Ander Conselvan de Oliveira Cc: Rodrigo Vivi Cc: Daniel Vetter Cc: Jani Nikula Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Michel Thierry Signed-off-by: Tomasz Lis Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/i915_sysfs.c| 2 +- drivers/gpu/drm/i915/intel_device_info.c | 2 +- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_guc_loader.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 40c0ac7..376ac95 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -58,7 +58,7 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) units <<= 8; - } else if (IS_BROXTON(dev_priv)) { + } else if (IS_GEN9_LP(dev_priv)) { units = 1; div = 1200; /* 833.33ns */ } diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index c46415b..6aeb1ed 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -192,7 +192,7 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv) (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) && hweight8(sseu->slice_mask) > 1; sseu->has_subslice_pg = - IS_BROXTON(dev_priv) && sseu_subslice_total(sseu) > 1; + IS_GEN9_LP(dev_priv) && sseu_subslice_total(sseu) > 1; sseu->has_eu_pg = sseu->eu_per_subslice > 2; if (IS_BROXTON(dev_priv)) { diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 45ebc96..ae08c19 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3375,7 +3375,7 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp) if (HAS_DDI(dev_priv)) { signal_levels = ddi_signal_levels(intel_dp); - if (IS_BROXTON(dev_priv)) + if (IS_GEN9_LP(dev_priv)) signal_levels = 0; else mask = DDI_BUF_EMP_MASK; diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 21db697..8b74525 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -339,7 +339,7 @@ static u32 guc_wopcm_size(struct drm_i915_private *dev_priv) u32 wopcm_size = GUC_WOPCM_TOP; /* On BXT, the top of WOPCM is reserved for RC6 context */ - if (IS_BROXTON(dev_priv)) + if (IS_GEN9_LP(dev_priv)) wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED; return wopcm_size; @@ -388,7 +388,7 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv) if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0)) I915_WRITE(GEN6_GFXPAUSE, 0x30FFF); - if (IS_BROXTON(dev_priv)) + if (IS_GEN9_LP(dev_priv)) I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE); else I915_WRITE(GEN9_GT_PM_CONFIG, GT_DOORBELL_ENABLE); -- 2.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/glk: Add missing bits to allow runtime pm suspend on GLK.
From: Rodrigo Vivi Besides having the DMC firmware in place and loaded let's handle runtime suspend and dc9 as we do for Broxton. Cc: Ander Conselvan de Oliveira Signed-off-by: Rodrigo Vivi Reviewed-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/i915_drv.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6428588..034de9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1472,7 +1472,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) intel_display_set_init_power(dev_priv, false); - fw_csr = !IS_BROXTON(dev_priv) && + fw_csr = !IS_GEN9_LP(dev_priv) && suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload; /* * In case of firmware assisted context save/restore don't manually @@ -1485,7 +1485,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) intel_power_domains_suspend(dev_priv); ret = 0; - if (IS_BROXTON(dev_priv)) + if (IS_GEN9_LP(dev_priv)) bxt_enable_dc9(dev_priv); else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_enable_pc8(dev_priv); @@ -1693,7 +1693,7 @@ static int i915_drm_resume_early(struct drm_device *dev) intel_uncore_early_sanitize(dev_priv, true); - if (IS_BROXTON(dev_priv)) { + if (IS_GEN9_LP(dev_priv)) { if (!dev_priv->suspended_to_idle) gen9_sanitize_dc_state(dev_priv); bxt_disable_dc9(dev_priv); @@ -1703,7 +1703,7 @@ static int i915_drm_resume_early(struct drm_device *dev) intel_uncore_sanitize(dev_priv); - if (IS_BROXTON(dev_priv) || + if (IS_GEN9_LP(dev_priv) || !(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload)) intel_power_domains_init_hw(dev_priv, true); @@ -2326,7 +2326,7 @@ static int intel_runtime_suspend(struct device *kdev) intel_runtime_pm_disable_interrupts(dev_priv); ret = 0; - if (IS_BROXTON(dev_priv)) { + if (IS_GEN9_LP(dev_priv)) { bxt_display_core_uninit(dev_priv); bxt_enable_dc9(dev_priv); } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { @@ -2411,7 +2411,7 @@ static int intel_runtime_resume(struct device *kdev) if (IS_GEN6(dev_priv)) intel_init_pch_refclk(dev_priv); - if (IS_BROXTON(dev_priv)) { + if (IS_GEN9_LP(dev_priv)) { bxt_disable_dc9(dev_priv); bxt_display_core_init(dev_priv, true); if (dev_priv->csr.dmc_payload && -- 2.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [drm-intel:drm-intel-nightly 922/930] include/linux/list.h:385:29: error: initialization discards 'const' qualifier from pointer target type
tree: git://anongit.freedesktop.org/drm-intel drm-intel-nightly head: ca1c03136b168816ac65c5945776908e464fca6b commit: 45b186f111f1623b257d183920cd4aab16a1acd5 [922/930] drm: Constify the drm_mm API config: x86_64-randconfig-x008-201650 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout 45b186f111f1623b257d183920cd4aab16a1acd5 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): In file included from include/linux/mutex.h:14:0, from include/linux/seq_file.h:7, from drivers/gpu/drm/i915/i915_gem_gtt.c:26: drivers/gpu/drm/i915/i915_gem_gtt.c: In function 'i915_gtt_color_adjust': >> include/linux/list.h:385:29: error: initialization discards 'const' >> qualifier from pointer target type [-Werror=discarded-qualifiers] struct list_head *head__ = (ptr); \ ^ drivers/gpu/drm/i915/i915_gem_gtt.c:2732:9: note: in expansion of macro 'list_first_entry_or_null' node = list_first_entry_or_null(&node->node_list, ^~~~ cc1: all warnings being treated as errors vim +/const +385 include/linux/list.h 3943f42c Andrey Utkin 2014-11-14 369 * @member: the name of the list_head within the struct. 93be3c2e Oleg Nesterov 2013-11-12 370 * 93be3c2e Oleg Nesterov 2013-11-12 371 * Note, that list is expected to be not empty. 93be3c2e Oleg Nesterov 2013-11-12 372 */ 93be3c2e Oleg Nesterov 2013-11-12 373 #define list_last_entry(ptr, type, member) \ 93be3c2e Oleg Nesterov 2013-11-12 374 list_entry((ptr)->prev, type, member) 93be3c2e Oleg Nesterov 2013-11-12 375 93be3c2e Oleg Nesterov 2013-11-12 376 /** 6d7581e6 Jiri Pirko2013-05-29 377 * list_first_entry_or_null - get the first element from a list 6d7581e6 Jiri Pirko2013-05-29 378 * @ptr:the list head to take the element from. 6d7581e6 Jiri Pirko2013-05-29 379 * @type: the type of the struct this is embedded in. 3943f42c Andrey Utkin 2014-11-14 380 * @member: the name of the list_head within the struct. 6d7581e6 Jiri Pirko2013-05-29 381 * 6d7581e6 Jiri Pirko2013-05-29 382 * Note that if the list is empty, it returns NULL. 6d7581e6 Jiri Pirko2013-05-29 383 */ 12adfd88 Chris Wilson 2016-07-23 384 #define list_first_entry_or_null(ptr, type, member) ({ \ 12adfd88 Chris Wilson 2016-07-23 @385 struct list_head *head__ = (ptr); \ 12adfd88 Chris Wilson 2016-07-23 386 struct list_head *pos__ = READ_ONCE(head__->next); \ 12adfd88 Chris Wilson 2016-07-23 387 pos__ != head__ ? list_entry(pos__, type, member) : NULL; \ 12adfd88 Chris Wilson 2016-07-23 388 }) 6d7581e6 Jiri Pirko2013-05-29 389 6d7581e6 Jiri Pirko2013-05-29 390 /** 008208c6 Oleg Nesterov 2013-11-12 391 * list_next_entry - get the next element in list 008208c6 Oleg Nesterov 2013-11-12 392 * @pos:the type * to cursor 3943f42c Andrey Utkin 2014-11-14 393 * @member: the name of the list_head within the struct. :: The code at line 385 was first introduced by commit :: 12adfd882c5f37548acaba4f043a158b3c54468b list: Expand list_first_entry_or_null() :: TO: Chris Wilson :: CC: Paul E. McKenney --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: Simplify intel_guc_load()
On 15/12/2016 15:47, Arkadiusz Hiler wrote: Current version of intel_guc_load() does a lot: - cares about submission - loads huc Not yet, no? So instead you could say that you are preparing the groundworks to make adding in the HuC fit better. - implement WA This change offloads some of the logic to intel_uc_load(), which now cares about the above. Cc: Anusha Srivatsa Cc: Jeff McGee Cc: Michal Winiarski Signed-off-by: Arkadiusz Hiler --- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/intel_guc_loader.c | 126 +--- drivers/gpu/drm/i915/intel_uc.c | 83 + drivers/gpu/drm/i915/intel_uc.h | 8 ++ 4 files changed, 110 insertions(+), 109 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 6af4e85..76b52c6 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4412,7 +4412,7 @@ i915_gem_init_hw(struct drm_i915_private *dev_priv) intel_mocs_init_l3cc_table(dev_priv); /* We can't enable contexts until all firmware is loaded */ - ret = intel_guc_load(dev_priv); + ret = intel_uc_load(dev_priv); if (ret) goto out; diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index f8b28b1..b76b556 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -97,7 +97,7 @@ const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status) } }; -static void guc_interrupts_release(struct drm_i915_private *dev_priv) +void guc_interrupts_release(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; enum intel_engine_id id; @@ -115,7 +115,7 @@ static void guc_interrupts_release(struct drm_i915_private *dev_priv) I915_WRITE(GUC_WD_VECS_IER, 0); } -static void guc_interrupts_capture(struct drm_i915_private *dev_priv) +void guc_interrupts_capture(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; enum intel_engine_id id; @@ -334,7 +334,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, return ret; } -static u32 guc_wopcm_size(struct drm_i915_private *dev_priv) +u32 guc_wopcm_size(struct drm_i915_private *dev_priv) { u32 wopcm_size = GUC_WOPCM_TOP; @@ -417,7 +417,7 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv) return ret; } -static int guc_hw_reset(struct drm_i915_private *dev_priv) +int guc_hw_reset(struct drm_i915_private *dev_priv) { int ret; u32 guc_status; @@ -452,75 +452,37 @@ int intel_guc_load(struct drm_i915_private *dev_priv) { struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; const char *fw_path = guc_fw->guc_fw_path; - int retries, ret, err; + int ret; DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n", fw_path, intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status), intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); - /* Loading forbidden, or no firmware to load? */ - if (!i915.enable_guc_loading) { - err = 0; - goto fail; - } else if (fw_path == NULL) { + if (fw_path == NULL) { /* Device is known to have no uCode (e.g. no GuC) */ - err = -ENXIO; - goto fail; + return -ENXIO; } else if (*fw_path == '\0') { /* Device has a GuC but we don't know what f/w to load? */ WARN(1, "No GuC firmware known for this platform!\n"); - err = -ENODEV; - goto fail; + return -ENODEV; } /* Fetch failed, or already fetched but failed to load? */ if (guc_fw->guc_fw_fetch_status != GUC_FIRMWARE_SUCCESS) { - err = -EIO; - goto fail; + return -EIO; } else if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL) { - err = -ENOEXEC; - goto fail; + return -ENOEXEC; } - guc_interrupts_release(dev_priv); - gen9_reset_guc_interrupts(dev_priv); - guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING; - DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n", - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status), - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); - - err = i915_guc_submission_init(dev_priv); - if (err) - goto fail; - - /* -* WaEnableuKernelHeaderValidFix:skl,bxt -* For BXT, this is only upto B0 but below WA is required for later -* steppings also so this is extended as well. -*/ /* WaEnableGuCBootHashCheckNotSet:skl,bxt */ - for (retries = 3; ; ) { - /* -* Always reset the
Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote: > From: Peter Antoine > > This patch will allow for getparams to return the status of the HuC. > As the HuC has to be validated by the GuC this patch uses the validated > status to show when the HuC is loaded and ready for use. You cannot use > the loaded status as with the GuC as the HuC is verified after it is > loaded and is not usable until it is verified. > > v2: removed the forewakes as the registers are already force-woken. > (T.Ursulin) > v4: rebased. > v5: rebased on top of drm-tip. > v6: rebased. Removed any reference to intel_huc.h > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS. > Remove intel_is_huc_valid() since it is used only in one place. > Put the case of I915_PARAM_HAS_HUC() in the right place. > > Signed-off-by: Peter Antoine > Reviewed-by: Arkadiusz Hiler You've retained my rb without asking me. With the changes you've made and confirmation that MEDIA FW that I915_READ() assumes: Reviewed-by: Arkadiusz Hiler > --- > drivers/gpu/drm/i915/i915_drv.c | 4 > drivers/gpu/drm/i915/intel_huc_loader.c | 1 - > include/uapi/drm/i915_drm.h | 1 + > 3 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 85a47c2..0bc016d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -49,6 +49,7 @@ > #include "i915_trace.h" > #include "i915_vgpu.h" > #include "intel_drv.h" > +#include "intel_uc.h" > > static struct drm_driver driver; > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void > *data, > case I915_PARAM_MIN_EU_IN_POOL: > value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool; > break; > + case I915_PARAM_HUC_STATUS: > + value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED; > + break; > case I915_PARAM_MMAP_GTT_VERSION: > /* Though we've started our numbering from 1, and so class all >* earlier versions as 0, in effect their value is undefined as > diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c > b/drivers/gpu/drm/i915/intel_huc_loader.c > index d8c5266..b06a613 100644 > --- a/drivers/gpu/drm/i915/intel_huc_loader.c > +++ b/drivers/gpu/drm/i915/intel_huc_loader.c > @@ -288,4 +288,3 @@ void intel_huc_fini(struct drm_device *dev) > > huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; > } > - > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index da32c2f..57093b4 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -395,6 +395,7 @@ typedef struct drm_i915_irq_wait { > * priorities and the driver will attempt to execute batches in priority > order. > */ > #define I915_PARAM_HAS_SCHEDULER 41 > +#define I915_PARAM_HUC_STATUS 42 > > typedef struct drm_i915_getparam { > __s32 param; > -- > 2.7.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: Simplify guc_fw_path
On 15/12/2016 15:47, Arkadiusz Hiler wrote: Currently guc_fw_path values can represent one of three possible states: 1) NULL - device without GuC 2) '\0' - device with GuC but no known firmware 3) else - device with GuC and known firmware Second case is used only to WARN at the later stage. We can WARN right away and merge cases 1 and 2 for later handling. Cc: Anusha Srivatsa Cc: Jeff McGee Cc: Michal Winiarski Signed-off-by: Arkadiusz Hiler --- drivers/gpu/drm/i915/intel_guc_loader.c | 22 +++--- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 0bb5fd1..075a103 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -460,12 +460,8 @@ int intel_guc_load(struct drm_i915_private *dev_priv) intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); if (fw_path == NULL) { - /* Device is known to have no uCode (e.g. no GuC) */ + /* We do not have uCode for the device */ return -ENXIO; - } else if (*fw_path == '\0') { - /* Device has a GuC but we don't know what f/w to load? */ - WARN(1, "No GuC firmware known for this platform!\n"); - return -ENODEV; } /* Fetch failed, or already fetched but failed to load? */ @@ -628,11 +624,9 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv, void intel_guc_init(struct drm_i915_private *dev_priv) { struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; - const char *fw_path; + const char *fw_path = NULL; - if (!HAS_GUC_UCODE(dev_priv)) { - fw_path = NULL; - } else if (IS_SKYLAKE(dev_priv)) { + if (IS_SKYLAKE(dev_priv)) { fw_path = I915_SKL_GUC_UCODE; guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR; guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR; @@ -644,24 +638,22 @@ void intel_guc_init(struct drm_i915_private *dev_priv) fw_path = I915_KBL_GUC_UCODE; guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR; guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR; - } else { - fw_path = ""; /* unknown device */ + } else if (HAS_GUC_UCODE(dev_priv)) { + WARN(1, "No GuC firmware known for platform with GuC!\n"); I think you should also set i915.enable_guc_loading to zero here to avoid the later stages bothering with partial setup and cleanup. intel_uc_load specifically I think would with this patch run some setup then immediately fail and have to unwind. Regards, Tvrtko } guc_fw->guc_fw_path = fw_path; guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE; guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE; - /* Early (and silent) return if GuC loading is disabled */ + /* Early return if we do not have firmware to fetch */ if (fw_path == NULL) return; - if (*fw_path == '\0') - return; guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_PENDING; DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path); + guc_fw_fetch(dev_priv, guc_fw); - /* status must now be FAIL or SUCCESS */ } /** ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm/i915/guc: Make the GuC fw loading helper functions general
On 15/12/2016 22:29, anushasr wrote: From: Peter Antoine Rename some of the GuC fw loading code to make them more general. We will utilise them for HuC loading as well. s/intel_guc_fw/intel_uc_fw/g s/GUC_FIRMWARE/UC_FIRMWARE/g Struct intel_guc_fw is renamed to intel_uc_fw. Prefix of tts members, such as 'guc' or 'guc_fw' either is renamed to 'uc' or removed for same purpose. v2: rebased on top of nightly. reapplied the search/replace as upstream code as changed. v3: rebased again on drm-nightly. v4: removed G from messages in shared fw fetch function. v5: rebased. v7: rebased. v8: rebased. v9: rebased. v10: rebased. v11: rebased. v12: rebased on top of drm-tip v13: rebased.Updated dev to dev_priv in intel_guc_setup(), guc_fw_getch() and intel_guc_init(). v14: rebased. Remove uint32_t fw_type to patch 2. Add INTEL_ prefix for fields in enum intel_uc_fw_status. Remove uc_dev field since its never used.Rename uc_fw to just fw and guc_fw to fw to avoid redundency. Signed-off-by: Anusha Srivatsa Signed-off-by: Alex Dai Signed-off-by: Peter Antoine --- drivers/gpu/drm/i915/i915_debugfs.c| 12 +-- drivers/gpu/drm/i915/i915_guc_submission.c | 4 +- drivers/gpu/drm/i915/intel_guc_loader.c| 157 +++-- drivers/gpu/drm/i915/intel_uc.h| 36 +++ 4 files changed, 105 insertions(+), 104 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 15deb2b..008afe6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2328,7 +2328,7 @@ static int i915_llc(struct seq_file *m, void *data) static int i915_guc_load_status_info(struct seq_file *m, void *data) { struct drm_i915_private *dev_priv = node_to_i915(m->private); - struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; + struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; u32 tmp, i; if (!HAS_GUC_UCODE(dev_priv)) @@ -2336,15 +2336,15 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data) seq_printf(m, "GuC firmware status:\n"); seq_printf(m, "\tpath: %s\n", - guc_fw->guc_fw_path); + guc_fw->uc_fw_path); seq_printf(m, "\tfetch: %s\n", - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status)); + intel_uc_fw_status_repr(guc_fw->fetch_status)); seq_printf(m, "\tload: %s\n", - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); + intel_uc_fw_status_repr(guc_fw->load_status)); seq_printf(m, "\tversion wanted: %d.%d\n", - guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted); + guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted); seq_printf(m, "\tversion found: %d.%d\n", - guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found); + guc_fw->major_ver_found, guc_fw->minor_ver_found); seq_printf(m, "\theader: offset is %d; size = %d\n", guc_fw->header_offset, guc_fw->header_size); seq_printf(m, "\tuCode: offset is %d; size = %d\n", diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 7fa4e74..b8ad8ff 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -1493,7 +1493,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv) struct i915_gem_context *ctx; u32 data[3]; - if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) + if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) return 0; gen9_disable_guc_interrupts(dev_priv); @@ -1520,7 +1520,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv) struct i915_gem_context *ctx; u32 data[3]; - if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS) + if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) return 0; if (i915.guc_log_level >= 0) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 21db697..9b3dbea 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -81,16 +81,16 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE); MODULE_FIRMWARE(I915_KBL_GUC_UCODE); /* User-friendly representation of an enum */ -const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status) +const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status) { switch (status) { - case GUC_FIRMWARE_FAIL: + case INTEL_UC_FIRMWARE_FAIL: return "FAIL"; - case GUC_FIRMWARE_NONE: + case INTEL_UC_FIRMWARE_NONE: return "NONE"; - case GUC_FIRMWARE_PENDING: + case INTEL_UC_FIRMWARE_PENDING: return "PENDING"; - case GUC_FIRMWARE_SUCCESS: + case INTEL_UC_FIRMWARE_SUCCESS: return "SU
Re: [Intel-gfx] Guc parameter Handling
On Thu, Dec 15, 2016 at 10:36:40PM +, Srivatsa, Anusha wrote: > Hi All, > > I was wondering if we intend to keep -1 and 2 for the > enable_guc_submission parameter. Since now we are gating guc loads if > either guc_submission or enable_huc parameter is set, why have a > -1(platform default) and 2(forcefully load) option? We anyway do not > have any special default set per platform. For now the default is 0 on > all platforms. Moving forward if GuC gets more stable and we want to > set a default to a certain platform, we can add -1 then. > > Also, why have a 2? We can use enable_guc_submission=1 in order to > make sure the guc is loaded and guc_submission is enabled and set > enable_guc_submission=0 to make sure guc submission is not used. I've asked around on IRC yesterday for the exact same thing, and it seems that no one realy does remembery why the "2" was introduced in the first place. We not simplifying it, if we do not have real use case for having 1 and 2 separate? > Any thought on this? > Cheers, > Anusha -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote: > On Thu, Dec 15, 2016 at 10:42:53PM +, Chris Wilson wrote: > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote: > > > From: Peter Antoine > > > > > > This patch will allow for getparams to return the status of the HuC. > > > As the HuC has to be validated by the GuC this patch uses the validated > > > status to show when the HuC is loaded and ready for use. You cannot use > > > the loaded status as with the GuC as the HuC is verified after it is > > > loaded and is not usable until it is verified. > > > > > > v2: removed the forewakes as the registers are already force-woken. > > > (T.Ursulin) > > > v4: rebased. > > > v5: rebased on top of drm-tip. > > > v6: rebased. Removed any reference to intel_huc.h > > > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS. > > > Remove intel_is_huc_valid() since it is used only in one place. > > > Put the case of I915_PARAM_HAS_HUC() in the right place. > > > > > > Signed-off-by: Peter Antoine > > > Reviewed-by: Arkadiusz Hiler > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 4 > > > drivers/gpu/drm/i915/intel_huc_loader.c | 1 - > > > include/uapi/drm/i915_drm.h | 1 + > > > 3 files changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index 85a47c2..0bc016d 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -49,6 +49,7 @@ > > > #include "i915_trace.h" > > > #include "i915_vgpu.h" > > > #include "intel_drv.h" > > > +#include "intel_uc.h" > > > > > > static struct drm_driver driver; > > > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, void > > > *data, > > > case I915_PARAM_MIN_EU_IN_POOL: > > > value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool; > > > break; > > > + case I915_PARAM_HUC_STATUS: > > > + value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED; > > > > Same question as last time: does the device need to be awake? We know is > > one of the GT power wells, so presumably we need an rpm_get/rpm_put as > > well to access the register. > > -Chris > > I get: > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE 24704 > [ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device > [ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended > [ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704 > [ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device > > consistently from: > > value = I915_READ(HUC_STATUS2); > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE %d\n", value); > i915_pm_ops.runtime_suspend(dev_priv->drm.dev); > > value = I915_READ(HUC_STATUS2); > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value); > i915_pm_ops.runtime_resume(dev_priv->drm.dev); Also do the test with i915.mmio_debug= -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/8] drm/i915/huc: Add HuC fw loading support
On 15/12/2016 22:29, anushasr wrote: From: Anusha Srivatsa The HuC loading process is similar to GuC. The intel_uc_fw_fetch() is used for both cases. HuC loading needs to be before GuC loading. The WOPCM setting must be done early before loading any of them. v2: rebased on-top of drm-intel-nightly. removed if(HAS_GUC()) before the guc call. (D.Gordon) update huc_version number of format. v3: rebased to drm-intel-nightly, changed the file name format to match the one in the huc package. Changed dev->dev_private to to_i915() v4: moved function back to where it was. change wait_for_atomic to wait_for. v5: rebased + comment changes. v7: rebased. v8: rebased. v9: rebased. Changed the year in the copyright message to reflect the right year.Correct the comments,remove the unwanted WARN message, replace drm_gem_object_unreference() with i915_gem_object_put().Make the prototypes in intel_huc.h non-extern. v10: rebased. Update the file construction done by HuC. It is similar to GuC.Adopted the approach used in- https://patchwork.freedesktop.org/patch/104355/ v11: Fix warnings remove old declaration v12: Change dev to dev_priv in macro definition. Corrected comments. v13: rebased. v14: rebased on top of drm-tip v15: rebased. Updated functions intel_huc_load(),intel_huc_init() and intel_uc_fw_fetch() to accept dev_priv instead of dev. Moved contents of intel_huc.h to intel_uc.h v16: change SKL_FW_ to SKL_HUC_FW_. Add intel_ prefix to guc_wopcm_size(). Remove unwanted checks in intel_uc.h. Rename huc_fw in struct intel_huc to simply fw to avoid redundency. Cc: Tvrtko Ursulin Tested-by: Xiang Haihao Signed-off-by: Anusha Srivatsa Signed-off-by: Alex Dai Signed-off-by: Peter Antoine --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_guc_reg.h | 3 + drivers/gpu/drm/i915/intel_guc_loader.c | 11 +- drivers/gpu/drm/i915/intel_huc_loader.c | 264 drivers/gpu/drm/i915/intel_uc.h | 18 +++ 7 files changed, 297 insertions(+), 7 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_huc_loader.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 5196509..45ae124 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -57,6 +57,7 @@ i915-y += i915_cmd_parser.o \ # general-purpose microcontroller (GuC) support i915-y += intel_uc.o \ intel_guc_loader.o \ + intel_huc_loader.o \ i915_guc_submission.o # autogenerated null render state diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6428588..85a47c2 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -600,6 +600,7 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_irq; + intel_huc_init(dev_priv); intel_guc_init(dev_priv); ret = i915_gem_init(dev_priv); @@ -627,6 +628,7 @@ static int i915_load_modeset_init(struct drm_device *dev) DRM_ERROR("failed to idle hardware; continuing to unload!\n"); i915_gem_fini(dev_priv); cleanup_irq: + intel_huc_fini(dev); intel_guc_fini(dev_priv); drm_irq_uninstall(dev); intel_teardown_gmbus(dev_priv); @@ -1313,7 +1315,7 @@ void i915_driver_unload(struct drm_device *dev) /* Flush any outstanding unpin_work. */ drain_workqueue(dev_priv->wq); - + intel_huc_fini(dev); intel_guc_fini(dev_priv); i915_gem_fini(dev_priv); intel_fbc_cleanup_cfb(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4199d26..bd5f235 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2134,6 +2134,7 @@ struct drm_i915_private { struct intel_gvt *gvt; + struct intel_huc huc; struct intel_guc guc; struct intel_csr csr; @@ -2908,7 +2909,7 @@ intel_info(const struct drm_i915_private *dev_priv) #define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc) #define HAS_GUC_UCODE(dev_priv)(HAS_GUC(dev_priv)) #define HAS_GUC_SCHED(dev_priv)(HAS_GUC(dev_priv)) - +#define HAS_HUC_UCODE(dev_priv)(HAS_GUC(dev_priv)) #define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer) #define HAS_POOLED_EU(dev_priv)((dev_priv)->info.has_pooled_eu) diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h index 5e638fc..f9829f6 100644 --- a/drivers/gpu/drm/i915/i915_guc_reg.h +++ b/drivers/gpu/drm/i915/i915_guc_reg.h @@ -61,9 +61,12 @@ #define DMA_ADDRESS_SPACE_GTT (8 << 16) #define DMA_COPY_SIZE _MMIO(0xc310) #define DMA_CTRL _MMIO(0xc314) +#define HUC_UKERNEL(1<<9) #define UOS_MOV
Re: [Intel-gfx] [PATCH 4/8] drm/i915/huc: Add BXT HuC Loading Support
On 15/12/2016 22:29, anushasr wrote: From: Anusha Srivatsa This patch adds the HuC Loading for the BXT by using the updated file construction. Version 1.7 of the HuC firmware. v2: rebased. v3: rebased on top of drm-tip v4: rebased. v5: rebased. Rename BXT_FW_MAJOR to BXT_HUC_FW_ Cc: Jeff Mcgee Signed-off-by: Anusha Srivatsa Reviewed-by: Jeff McGee --- drivers/gpu/drm/i915/intel_huc_loader.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c index 0f929cc..f36efd4 100644 --- a/drivers/gpu/drm/i915/intel_huc_loader.c +++ b/drivers/gpu/drm/i915/intel_huc_loader.c @@ -40,6 +40,10 @@ * Note that HuC firmware loading must be done before GuC loading. */ +#define BXT_HUC_FW_MAJOR 01 +#define BXT_HUC_FW_MINOR 07 +#define BXT_BLD_NUM 1398 + #define SKL_HUC_FW_MAJOR 01 #define SKL_HUC_FW_MINOR 07 #define SKL_BLD_NUM 1398 @@ -52,6 +56,9 @@ SKL_HUC_FW_MINOR, SKL_BLD_NUM) MODULE_FIRMWARE(I915_SKL_HUC_UCODE); +#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \ + BXT_HUC_FW_MINOR, BXT_BLD_NUM) +MODULE_FIRMWARE(I915_BXT_HUC_UCODE); More curiosity - given the versions between SKL and BXT are identical - is it actually the same binary file in both cases? Regards, Tvrtko /** * huc_ucode_xfer() - DMA's the firmware * @dev_priv: the drm device @@ -157,6 +164,10 @@ void intel_huc_init(struct drm_i915_private *dev_priv) fw_path = I915_SKL_HUC_UCODE; huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR; huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR; + } else if (IS_BROXTON(dev_priv)) { + fw_path = I915_BXT_HUC_UCODE; + huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR; + huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR; } huc_fw->uc_fw_path = fw_path; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Thu, Dec 15, 2016 at 02:29:49PM -0800, anushasr wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > v14: rebased. > > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c | 2 ++ > drivers/gpu/drm/i915/intel_uc.c | 62 > + > drivers/gpu/drm/i915/intel_uc.h | 1 + > 4 files changed, 66 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index ed1ab40..ce4e05e 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -506,6 +506,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > + INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, > INTEL_GUC_ACTION_LIMIT > }; > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 2257495..7605f36 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev_priv); > + > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index 8ae6795..b90ac57 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -138,3 +138,65 @@ int intel_guc_log_control(struct intel_guc *guc, u32 > control_val) > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev_priv: the drm_i915_device > + * > + * Triggers a HuC fw authentication request to the GuC via intel_guc_action_ > + * authenticate_huc interface. > + * interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_NONE || > + huc->fw.load_status == INTEL_UC_FIRMWARE_NONE) > + return; > + > + if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate"); Why this DRM_ERROR does not have tailing "\n"? Same goes for couple more in here. > + return; > + } > + > + if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: fw wasn't loaded. Nothing to authenticate"); > + return; > + } > + > + vma = i915_gem_object_ggtt_pin(huc->fw.uc_fw_obj, NULL, 0, 0, 0); > + if (IS_ERR(vma)) { > + DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma)); > + return; > + } > + > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + > + /* Specify auth action and where public signature is. */ > + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > + data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; > + > + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request\n"); > + goto out; > + } > + > + /* Check authentication status, it should be done by now */ > + ret = wait_for((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, 50); > + if (ret) { > + DRM_ERROR("HuC: Authentication failed\n"); > + goto out; > + } > + > +out: > + i915_vma_unpin(vma); > +} > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > index 57aef56..e69d47c 10064
Re: [Intel-gfx] [PATCH 5/8] drm/i915/HuC: Add KBL huC loading Support
On 15/12/2016 22:29, anushasr wrote: From: Anusha Srivatsa This patch adds the support to load HuC on KBL Version 2.0 v2: rebased. v3: rebased on top of drm-tip v4: rebased. v5: rebased. Rename KBL_FW_ to KBL_HUC_FW_ Cc: Jeff Mcgee Signed-off-by: Anusha Srivatsa Reviewed-by: Jeff McGee --- drivers/gpu/drm/i915/intel_huc_loader.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c index f36efd4..d8c5266 100644 --- a/drivers/gpu/drm/i915/intel_huc_loader.c +++ b/drivers/gpu/drm/i915/intel_huc_loader.c @@ -48,6 +48,10 @@ #define SKL_HUC_FW_MINOR 07 #define SKL_BLD_NUM 1398 +#define KBL_HUC_FW_MAJOR 02 +#define KBL_HUC_FW_MINOR 00 +#define KBL_BLD_NUM 1810 + #define HUC_FW_PATH(platform, major, minor, bld_num) \ "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \ __stringify(minor) "_" __stringify(bld_num) ".bin" @@ -59,6 +63,11 @@ MODULE_FIRMWARE(I915_SKL_HUC_UCODE); #define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \ BXT_HUC_FW_MINOR, BXT_BLD_NUM) MODULE_FIRMWARE(I915_BXT_HUC_UCODE); + +#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \ + KBL_HUC_FW_MINOR, KBL_BLD_NUM) +MODULE_FIRMWARE(I915_KBL_HUC_UCODE); + /** * huc_ucode_xfer() - DMA's the firmware * @dev_priv: the drm device @@ -168,8 +177,15 @@ void intel_huc_init(struct drm_i915_private *dev_priv) fw_path = I915_BXT_HUC_UCODE; huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR; huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR; + } else if (IS_KABYLAKE(dev_priv)) { + fw_path = I915_KBL_HUC_UCODE; + huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; + huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; } + if (fw_path == NULL) + return; + This looks suspiciously out of place in this patch. What's the reason for it? Regards, Tvrtko huc_fw->uc_fw_path = fw_path; huc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
On Fri, Dec 16, 2016 at 04:12:36PM +, Chris Wilson wrote: > On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote: > > On Thu, Dec 15, 2016 at 10:42:53PM +, Chris Wilson wrote: > > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote: > > > > From: Peter Antoine > > > > > > > > This patch will allow for getparams to return the status of the HuC. > > > > As the HuC has to be validated by the GuC this patch uses the validated > > > > status to show when the HuC is loaded and ready for use. You cannot use > > > > the loaded status as with the GuC as the HuC is verified after it is > > > > loaded and is not usable until it is verified. > > > > > > > > v2: removed the forewakes as the registers are already force-woken. > > > > (T.Ursulin) > > > > v4: rebased. > > > > v5: rebased on top of drm-tip. > > > > v6: rebased. Removed any reference to intel_huc.h > > > > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS. > > > > Remove intel_is_huc_valid() since it is used only in one place. > > > > Put the case of I915_PARAM_HAS_HUC() in the right place. > > > > > > > > Signed-off-by: Peter Antoine > > > > Reviewed-by: Arkadiusz Hiler > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 4 > > > > drivers/gpu/drm/i915/intel_huc_loader.c | 1 - > > > > include/uapi/drm/i915_drm.h | 1 + > > > > 3 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index 85a47c2..0bc016d 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -49,6 +49,7 @@ > > > > #include "i915_trace.h" > > > > #include "i915_vgpu.h" > > > > #include "intel_drv.h" > > > > +#include "intel_uc.h" > > > > > > > > static struct drm_driver driver; > > > > > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, > > > > void *data, > > > > case I915_PARAM_MIN_EU_IN_POOL: > > > > value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool; > > > > break; > > > > + case I915_PARAM_HUC_STATUS: > > > > + value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED; > > > > > > Same question as last time: does the device need to be awake? We know is > > > one of the GT power wells, so presumably we need an rpm_get/rpm_put as > > > well to access the register. > > > -Chris > > > > I get: > > > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE 24704 > > [ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device > > [ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended > > [ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704 > > [ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device > > > > consistently from: > > > > value = I915_READ(HUC_STATUS2); > > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE %d\n", value); > > i915_pm_ops.runtime_suspend(dev_priv->drm.dev); > > > > value = I915_READ(HUC_STATUS2); > > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value); > > i915_pm_ops.runtime_resume(dev_priv->drm.dev); > > Also do the test with i915.mmio_debug= > -Chris Same effect. Works. -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/8] drm/i915/huc: Add HuC fw loading support
On Fri, Dec 16, 2016 at 04:13:14PM +, Tvrtko Ursulin wrote: > > On 15/12/2016 22:29, anushasr wrote: > > From: Anusha Srivatsa > > > > The HuC loading process is similar to GuC. The intel_uc_fw_fetch() > > is used for both cases. > > > > HuC loading needs to be before GuC loading. The WOPCM setting must > > be done early before loading any of them. > > > > v2: rebased on-top of drm-intel-nightly. > > removed if(HAS_GUC()) before the guc call. (D.Gordon) > > update huc_version number of format. > > v3: rebased to drm-intel-nightly, changed the file name format to > > match the one in the huc package. > > Changed dev->dev_private to to_i915() > > v4: moved function back to where it was. > > change wait_for_atomic to wait_for. > > v5: rebased + comment changes. > > v7: rebased. > > v8: rebased. > > v9: rebased. Changed the year in the copyright message to reflect > > the right year.Correct the comments,remove the unwanted WARN message, > > replace drm_gem_object_unreference() with i915_gem_object_put().Make the > > prototypes in intel_huc.h non-extern. > > v10: rebased. Update the file construction done by HuC. It is similar to > > GuC.Adopted the approach used in- > > https://patchwork.freedesktop.org/patch/104355/ > > v11: Fix warnings remove old declaration > > v12: Change dev to dev_priv in macro definition. > > Corrected comments. > > v13: rebased. > > v14: rebased on top of drm-tip > > v15: rebased. Updated functions intel_huc_load(),intel_huc_init() and > > intel_uc_fw_fetch() to accept dev_priv instead of dev. Moved contents > > of intel_huc.h to intel_uc.h > > v16: change SKL_FW_ to SKL_HUC_FW_. Add intel_ prefix to guc_wopcm_size(). > > Remove unwanted checks in intel_uc.h. Rename huc_fw in struct intel_huc to > > simply fw to avoid redundency. > > > > Cc: Tvrtko Ursulin > > Tested-by: Xiang Haihao > > Signed-off-by: Anusha Srivatsa > > Signed-off-by: Alex Dai > > Signed-off-by: Peter Antoine > > --- > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/i915_drv.c | 4 +- > > drivers/gpu/drm/i915/i915_drv.h | 3 +- > > drivers/gpu/drm/i915/i915_guc_reg.h | 3 + > > drivers/gpu/drm/i915/intel_guc_loader.c | 11 +- > > drivers/gpu/drm/i915/intel_huc_loader.c | 264 > > > > drivers/gpu/drm/i915/intel_uc.h | 18 +++ > > 7 files changed, 297 insertions(+), 7 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/intel_huc_loader.c > > > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > > index 5196509..45ae124 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -57,6 +57,7 @@ i915-y += i915_cmd_parser.o \ > > # general-purpose microcontroller (GuC) support > > i915-y += intel_uc.o \ > > intel_guc_loader.o \ > > + intel_huc_loader.o \ > > i915_guc_submission.o > > > > # autogenerated null render state > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 6428588..85a47c2 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -600,6 +600,7 @@ static int i915_load_modeset_init(struct drm_device > > *dev) > > if (ret) > > goto cleanup_irq; > > > > + intel_huc_init(dev_priv); > > intel_guc_init(dev_priv); > > > > ret = i915_gem_init(dev_priv); > > @@ -627,6 +628,7 @@ static int i915_load_modeset_init(struct drm_device > > *dev) > > DRM_ERROR("failed to idle hardware; continuing to unload!\n"); > > i915_gem_fini(dev_priv); > > cleanup_irq: > > + intel_huc_fini(dev); > > intel_guc_fini(dev_priv); > > drm_irq_uninstall(dev); > > intel_teardown_gmbus(dev_priv); > > @@ -1313,7 +1315,7 @@ void i915_driver_unload(struct drm_device *dev) > > > > /* Flush any outstanding unpin_work. */ > > drain_workqueue(dev_priv->wq); > > - > > + intel_huc_fini(dev); > > intel_guc_fini(dev_priv); > > i915_gem_fini(dev_priv); > > intel_fbc_cleanup_cfb(dev_priv); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 4199d26..bd5f235 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2134,6 +2134,7 @@ struct drm_i915_private { > > > > struct intel_gvt *gvt; > > > > + struct intel_huc huc; > > struct intel_guc guc; > > > > struct intel_csr csr; > > @@ -2908,7 +2909,7 @@ intel_info(const struct drm_i915_private *dev_priv) > > #define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc) > > #define HAS_GUC_UCODE(dev_priv)(HAS_GUC(dev_priv)) > > #define HAS_GUC_SCHED(dev_priv)(HAS_GUC(dev_priv)) > > - > > +#define HAS_HUC_UCODE(dev_priv)(HAS_GUC(dev_priv)) > > #define HAS_RESOURCE_STREAMER(dev_priv) > > ((dev_priv)->info.has_resource_streamer) > > > > #define HAS_POOLED_EU(dev_priv)((dev_priv)->info.has_pooled_eu) > > diff --gi
Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
On Fri, Dec 16, 2016 at 05:21:38PM +0100, Arkadiusz Hiler wrote: > On Fri, Dec 16, 2016 at 04:12:36PM +, Chris Wilson wrote: > > On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote: > > > On Thu, Dec 15, 2016 at 10:42:53PM +, Chris Wilson wrote: > > > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote: > > > > > From: Peter Antoine > > > > > > > > > > This patch will allow for getparams to return the status of the HuC. > > > > > As the HuC has to be validated by the GuC this patch uses the > > > > > validated > > > > > status to show when the HuC is loaded and ready for use. You cannot > > > > > use > > > > > the loaded status as with the GuC as the HuC is verified after it is > > > > > loaded and is not usable until it is verified. > > > > > > > > > > v2: removed the forewakes as the registers are already force-woken. > > > > > (T.Ursulin) > > > > > v4: rebased. > > > > > v5: rebased on top of drm-tip. > > > > > v6: rebased. Removed any reference to intel_huc.h > > > > > v7: rebased. Rename I915_PARAM_HAS_HUC to I915_PARAM_HUC_STATUS. > > > > > Remove intel_is_huc_valid() since it is used only in one place. > > > > > Put the case of I915_PARAM_HAS_HUC() in the right place. > > > > > > > > > > Signed-off-by: Peter Antoine > > > > > Reviewed-by: Arkadiusz Hiler > > > > > --- > > > > > drivers/gpu/drm/i915/i915_drv.c | 4 > > > > > drivers/gpu/drm/i915/intel_huc_loader.c | 1 - > > > > > include/uapi/drm/i915_drm.h | 1 + > > > > > 3 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > > index 85a47c2..0bc016d 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > > @@ -49,6 +49,7 @@ > > > > > #include "i915_trace.h" > > > > > #include "i915_vgpu.h" > > > > > #include "intel_drv.h" > > > > > +#include "intel_uc.h" > > > > > > > > > > static struct drm_driver driver; > > > > > > > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device *dev, > > > > > void *data, > > > > > case I915_PARAM_MIN_EU_IN_POOL: > > > > > value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool; > > > > > break; > > > > > + case I915_PARAM_HUC_STATUS: > > > > > + value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED; > > > > > > > > Same question as last time: does the device need to be awake? We know is > > > > one of the GT power wells, so presumably we need an rpm_get/rpm_put as > > > > well to access the register. > > > > -Chris > > > > > > I get: > > > > > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 PRE > > > 24704 > > > [ 1588.571285] [drm:intel_runtime_suspend [i915]] Suspending device > > > [ 1588.575768] [drm:intel_runtime_suspend [i915]] Device suspended > > > [ 1588.577156] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST > > > 24704 > > > [ 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device > > > > > > consistently from: > > > > > > value = I915_READ(HUC_STATUS2); > > > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE %d\n", value); > > > i915_pm_ops.runtime_suspend(dev_priv->drm.dev); > > > > > > value = I915_READ(HUC_STATUS2); > > > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value); > > > i915_pm_ops.runtime_resume(dev_priv->drm.dev); > > > > Also do the test with i915.mmio_debug= > > -Chris > > Same effect. Works. Ok, then just mark up that we don't need rpm here so that we don't freak out in future scans for mmio access outside of rpm. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,01/40] drm/i915: Use the MRU stack search after evicting (rev3)
== Series Details == Series: series starting with [v2,01/40] drm/i915: Use the MRU stack search after evicting (rev3) URL : https://patchwork.freedesktop.org/series/16906/ State : failure == Summary == CC net/ipv4/tcp_cubic.o CC net/ipv4/xfrm4_policy.o LD drivers/net/phy/built-in.o CC net/ipv4/xfrm4_state.o CC net/ipv4/xfrm4_input.o CC net/ipv4/xfrm4_output.o CC net/ipv4/xfrm4_protocol.o CC net/ipv6/exthdrs_offload.o CC net/ipv6/inet6_hashtables.o CC net/ipv6/mcast_snoop.o LD drivers/rtc/built-in.o LD drivers/pci/pcie/pcieportdrv.o LD [M] drivers/net/ethernet/intel/igbvf/igbvf.o LD drivers/acpi/acpica/acpi.o LD kernel/events/built-in.o LD net/netlink/built-in.o LD net/unix/unix_diag.o LD kernel/sched/built-in.o LD drivers/acpi/acpica/built-in.o LD drivers/pci/pcie/aer/aerdriver.o LD kernel/built-in.o LD drivers/pci/pcie/aer/built-in.o LD drivers/usb/gadget/libcomposite.o LD drivers/usb/storage/usb-storage.o LD drivers/pci/pcie/built-in.o LD drivers/acpi/built-in.o LD drivers/usb/storage/built-in.o LD net/key/built-in.o LD drivers/tty/serial/8250/8250.o LD [M] drivers/usb/serial/usbserial.o LD drivers/thermal/thermal_sys.o LD mm/built-in.o LD drivers/thermal/built-in.o LD [M] drivers/misc/mei/mei-me.o LD lib/raid6/raid6_pq.o LD drivers/misc/built-in.o LD lib/raid6/built-in.o LD drivers/spi/built-in.o LD drivers/iommu/built-in.o LD drivers/usb/gadget/udc/udc-core.o LD drivers/usb/gadget/udc/built-in.o LD drivers/usb/gadget/built-in.o scripts/Makefile.build:544: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:544: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 make[1]: *** Waiting for unfinished jobs LD net/packet/built-in.o LD drivers/video/fbdev/core/fb.o LD drivers/video/fbdev/core/built-in.o LD [M] sound/pci/hda/snd-hda-codec-generic.o LD drivers/video/fbdev/built-in.o LD [M] drivers/net/ethernet/intel/e1000/e1000.o LD sound/pci/built-in.o LD net/unix/unix.o LD net/unix/built-in.o LD drivers/scsi/scsi_mod.o LD drivers/video/console/built-in.o LD drivers/video/built-in.o LD sound/built-in.o LD [M] drivers/net/ethernet/broadcom/genet/genet.o LD drivers/tty/serial/8250/8250_base.o LD drivers/tty/serial/8250/built-in.o LD drivers/tty/serial/built-in.o LD drivers/pci/built-in.o LD drivers/scsi/sd_mod.o LD drivers/scsi/built-in.o LD [M] drivers/net/ethernet/intel/igb/igb.o LD drivers/md/md-mod.o AR lib/lib.a LD drivers/md/built-in.o EXPORTS lib/lib-ksyms.o CC arch/x86/kernel/cpu/capflags.o LD arch/x86/kernel/cpu/built-in.o LD drivers/usb/core/usbcore.o LD arch/x86/kernel/built-in.o LD drivers/usb/core/built-in.o LD drivers/usb/host/xhci-hcd.o LD arch/x86/built-in.o LD drivers/tty/vt/built-in.o LD net/xfrm/built-in.o LD drivers/tty/built-in.o LD net/ipv6/ipv6.o LD fs/btrfs/btrfs.o LD net/ipv6/built-in.o LD lib/built-in.o LD fs/btrfs/built-in.o LD fs/ext4/ext4.o LD fs/ext4/built-in.o LD drivers/usb/host/built-in.o LD drivers/usb/built-in.o LD fs/built-in.o LD net/ipv4/built-in.o LD [M] drivers/net/ethernet/intel/e1000e/e1000e.o LD net/core/built-in.o LD net/built-in.o LD drivers/net/ethernet/built-in.o LD drivers/net/built-in.o Makefile:988: recipe for target 'drivers' failed make: *** [drivers] Error 2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/8] drm/i915/huc: Add HuC fw loading support
On 16/12/2016 16:29, Arkadiusz Hiler wrote: On Fri, Dec 16, 2016 at 04:13:14PM +, Tvrtko Ursulin wrote: On 15/12/2016 22:29, anushasr wrote: From: Anusha Srivatsa The HuC loading process is similar to GuC. The intel_uc_fw_fetch() is used for both cases. HuC loading needs to be before GuC loading. The WOPCM setting must be done early before loading any of them. v2: rebased on-top of drm-intel-nightly. removed if(HAS_GUC()) before the guc call. (D.Gordon) update huc_version number of format. v3: rebased to drm-intel-nightly, changed the file name format to match the one in the huc package. Changed dev->dev_private to to_i915() v4: moved function back to where it was. change wait_for_atomic to wait_for. v5: rebased + comment changes. v7: rebased. v8: rebased. v9: rebased. Changed the year in the copyright message to reflect the right year.Correct the comments,remove the unwanted WARN message, replace drm_gem_object_unreference() with i915_gem_object_put().Make the prototypes in intel_huc.h non-extern. v10: rebased. Update the file construction done by HuC. It is similar to GuC.Adopted the approach used in- https://patchwork.freedesktop.org/patch/104355/ v11: Fix warnings remove old declaration v12: Change dev to dev_priv in macro definition. Corrected comments. v13: rebased. v14: rebased on top of drm-tip v15: rebased. Updated functions intel_huc_load(),intel_huc_init() and intel_uc_fw_fetch() to accept dev_priv instead of dev. Moved contents of intel_huc.h to intel_uc.h v16: change SKL_FW_ to SKL_HUC_FW_. Add intel_ prefix to guc_wopcm_size(). Remove unwanted checks in intel_uc.h. Rename huc_fw in struct intel_huc to simply fw to avoid redundency. Cc: Tvrtko Ursulin Tested-by: Xiang Haihao Signed-off-by: Anusha Srivatsa Signed-off-by: Alex Dai Signed-off-by: Peter Antoine --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_guc_reg.h | 3 + drivers/gpu/drm/i915/intel_guc_loader.c | 11 +- drivers/gpu/drm/i915/intel_huc_loader.c | 264 drivers/gpu/drm/i915/intel_uc.h | 18 +++ 7 files changed, 297 insertions(+), 7 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_huc_loader.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 5196509..45ae124 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -57,6 +57,7 @@ i915-y += i915_cmd_parser.o \ # general-purpose microcontroller (GuC) support i915-y += intel_uc.o \ intel_guc_loader.o \ + intel_huc_loader.o \ i915_guc_submission.o # autogenerated null render state diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6428588..85a47c2 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -600,6 +600,7 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_irq; + intel_huc_init(dev_priv); intel_guc_init(dev_priv); ret = i915_gem_init(dev_priv); @@ -627,6 +628,7 @@ static int i915_load_modeset_init(struct drm_device *dev) DRM_ERROR("failed to idle hardware; continuing to unload!\n"); i915_gem_fini(dev_priv); cleanup_irq: + intel_huc_fini(dev); intel_guc_fini(dev_priv); drm_irq_uninstall(dev); intel_teardown_gmbus(dev_priv); @@ -1313,7 +1315,7 @@ void i915_driver_unload(struct drm_device *dev) /* Flush any outstanding unpin_work. */ drain_workqueue(dev_priv->wq); - + intel_huc_fini(dev); intel_guc_fini(dev_priv); i915_gem_fini(dev_priv); intel_fbc_cleanup_cfb(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4199d26..bd5f235 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2134,6 +2134,7 @@ struct drm_i915_private { struct intel_gvt *gvt; + struct intel_huc huc; struct intel_guc guc; struct intel_csr csr; @@ -2908,7 +2909,7 @@ intel_info(const struct drm_i915_private *dev_priv) #define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc) #define HAS_GUC_UCODE(dev_priv)(HAS_GUC(dev_priv)) #define HAS_GUC_SCHED(dev_priv)(HAS_GUC(dev_priv)) - +#define HAS_HUC_UCODE(dev_priv)(HAS_GUC(dev_priv)) #define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer) #define HAS_POOLED_EU(dev_priv)((dev_priv)->info.has_pooled_eu) diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h index 5e638fc..f9829f6 100644 --- a/drivers/gpu/drm/i915/i915_guc_reg.h +++ b/drivers/gpu/drm/i915/i915_guc_reg.h @@ -61,9 +61,12 @@ #define DMA_ADDRESS_SPACE_GTT (8 << 16) #define DMA_COPY_SIZE _MMIO(0xc310) #define D
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm: Use drm_mm_nodes() as shorthand for the list of nodes under struct drm_mm
== Series Details == Series: drm: Use drm_mm_nodes() as shorthand for the list of nodes under struct drm_mm URL : https://patchwork.freedesktop.org/series/16920/ State : failure == Summary == Series 16920v1 drm: Use drm_mm_nodes() as shorthand for the list of nodes under struct drm_mm https://patchwork.freedesktop.org/api/1.0/series/16920/revisions/1/mbox/ Test gem_ctx_switch: Subgroup basic-default: pass -> INCOMPLETE (fi-byt-j1900) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: pass -> SKIP (fi-bxt-j4205) fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:247 pass:222 dwarn:0 dfail:0 fail:0 skip:25 fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-j1900 total:24 pass:23 dwarn:0 dfail:0 fail:0 skip:0 fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20 fi-skl-6770hqtotal:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 705f1e8fef81d504f0032df8e21bdc2e74850b3a drm-tip: 2016y-12m-16d-15h-40m-02s UTC integration manifest 4a8cc5a drm: Use drm_mm_nodes() as shorthand for the list of nodes under struct drm_mm == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3310/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Dump more configuration information for DSI
On Wed, Dec 14, 2016 at 07:14:05PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Dump out more of the DSI configuration details during init. > This includes pclk, burst_mode_ratio, lane_count, pixel_overlap, > video_mode_format and reset_timer_val. > > v2: Dump more info (Chris) dphy_reg? No idea what it is, just another one I couldn't find printed. > Cc: Chris Wilson > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > index 3fd3bac5fccc..d2e3aa9c5022 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c > @@ -808,6 +808,15 @@ struct drm_panel *vbt_panel_init(struct intel_dsi > *intel_dsi, u16 panel_id) > 8); > intel_dsi->clk_hs_to_lp_count += extra_byte_count; > > + DRM_DEBUG_KMS("Pclk %d\n", intel_dsi->pclk); > + DRM_DEBUG_KMS("Pixel overlap %d\n", intel_dsi->pixel_overlap); > + DRM_DEBUG_KMS("Lane count %d\n", intel_dsi->lane_count); > + DRM_DEBUG_KMS("Video mode format %s\n", > + intel_dsi->video_mode_format == NON_BURST_SYNC_PULSE ? > "non-burst with sync pulse" : > + intel_dsi->video_mode_format == NON_BURST_SYNC_EVENTS ? > "non-burst with sync events" : > + intel_dsi->video_mode_format == BURST_MODE ? "burst" : > ""); Argh. The mipi_config.video_transfer_mode:2 has #define NON_BURST_SYNC_PULSE0x1 #define NON_BURST_SYNC_EVENTS 0x2 #define BURST_MODE 0x3 but intel_dist.video_mode_format is #define VIDEO_MODE_NON_BURST_WITH_SYNC_PULSE (1 << 0) #define VIDEO_MODE_NON_BURST_WITH_SYNC_EVENTS (2 << 0) #define VIDEO_MODE_BURST (3 << 0) They match just usage is consistent. However you want to resolve that, Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/DMC/GLK: Load DMC on GLK
== Series Details == Series: series starting with [1/3] drm/i915/DMC/GLK: Load DMC on GLK URL : https://patchwork.freedesktop.org/series/16926/ State : failure == Summary == Series 16926v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/16926/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload-inject: pass -> INCOMPLETE (fi-kbl-7500u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: pass -> SKIP (fi-bxt-j4205) fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:247 pass:222 dwarn:0 dfail:0 fail:0 skip:25 fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:7pass:6dwarn:0 dfail:0 fail:0 skip:0 fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hqtotal:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20 fi-skl-6770hqtotal:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 705f1e8fef81d504f0032df8e21bdc2e74850b3a drm-tip: 2016y-12m-16d-15h-40m-02s UTC integration manifest 376d7cf drm/i915/glk: Convert a few more IS_BROXTON() to IS_GEN9_LP() 8c9049c drm/i915/glk: Add missing bits to allow runtime pm suspend on GLK. 2a48676 drm/i915/DMC/GLK: Load DMC on GLK == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3311/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: Simplify intel_guc_load()
+ +fail: + /* +* We've failed to load the firmware :( +* +* Decide whether to disable GuC submission and fall back to +* execlist mode, and whether to hide the error by returning +* zero or to return -EIO, which the caller will treat as a +* nonfatal error (i.e. it doesn't prevent driver load, but +* marks the GPU as wedged until reset). +*/ + if (i915.enable_guc_loading > 1 || i915.enable_guc_submission > 1) + ret = -EIO; + else + ret = 0; + + if (i915.enable_guc_submission) { + i915.enable_guc_submission = 0; + DRM_INFO("GuC submission without firmware not supported\n"); + DRM_NOTE("Falling back from GuC submission to execlist mode\n"); If i915.enable_guc_submission > 1 we will mark the GPU as wedged so it might be worth retaining an error level message here in that scenario. If we are wedging the GPU you do not really care about the fallback, so theres no real use in having that promoted + those are the original levels that were already here. Anyway, it seems like the `enable_guc_* > 1` are likely to be gone. I've discussed that on IRC yesterday and no one seems to really remember why we've got it in the first place. Anusha posted similar concern here with her HuC series as well. Just to clarify (because as you said the case will probably go away), what I meant was an extra log for the > 1 case like we had in the original code, i.e: DRM_ERROR("GuC init failed: %d\n", ret); as otherwise we would have declared the GPU wedged without printing any error-level message to explain why. Daniele ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to getparams
>-Original Message- >From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] >Sent: Friday, December 16, 2016 8:31 AM >To: Hiler, Arkadiusz >Cc: Srivatsa, Anusha ; intel- >g...@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH 8/8] drm/i915/get_params: Add HuC status to >getparams > >On Fri, Dec 16, 2016 at 05:21:38PM +0100, Arkadiusz Hiler wrote: >> On Fri, Dec 16, 2016 at 04:12:36PM +, Chris Wilson wrote: >> > On Fri, Dec 16, 2016 at 03:43:46PM +0100, Arkadiusz Hiler wrote: >> > > On Thu, Dec 15, 2016 at 10:42:53PM +, Chris Wilson wrote: >> > > > On Thu, Dec 15, 2016 at 02:29:50PM -0800, anushasr wrote: >> > > > > From: Peter Antoine >> > > > > >> > > > > This patch will allow for getparams to return the status of the HuC. >> > > > > As the HuC has to be validated by the GuC this patch uses the >> > > > > validated status to show when the HuC is loaded and ready for >> > > > > use. You cannot use the loaded status as with the GuC as the >> > > > > HuC is verified after it is loaded and is not usable until it is >> > > > > verified. >> > > > > >> > > > > v2: removed the forewakes as the registers are already force-woken. >> > > > > (T.Ursulin) >> > > > > v4: rebased. >> > > > > v5: rebased on top of drm-tip. >> > > > > v6: rebased. Removed any reference to intel_huc.h >> > > > > v7: rebased. Rename I915_PARAM_HAS_HUC to >I915_PARAM_HUC_STATUS. >> > > > > Remove intel_is_huc_valid() since it is used only in one place. >> > > > > Put the case of I915_PARAM_HAS_HUC() in the right place. >> > > > > >> > > > > Signed-off-by: Peter Antoine >> > > > > Reviewed-by: Arkadiusz Hiler >> > > > > --- >> > > > > drivers/gpu/drm/i915/i915_drv.c | 4 >> > > > > drivers/gpu/drm/i915/intel_huc_loader.c | 1 - >> > > > > include/uapi/drm/i915_drm.h | 1 + >> > > > > 3 files changed, 5 insertions(+), 1 deletion(-) >> > > > > >> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c >> > > > > b/drivers/gpu/drm/i915/i915_drv.c index 85a47c2..0bc016d >> > > > > 100644 >> > > > > --- a/drivers/gpu/drm/i915/i915_drv.c >> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c >> > > > > @@ -49,6 +49,7 @@ >> > > > > #include "i915_trace.h" >> > > > > #include "i915_vgpu.h" >> > > > > #include "intel_drv.h" >> > > > > +#include "intel_uc.h" >> > > > > >> > > > > static struct drm_driver driver; >> > > > > >> > > > > @@ -315,6 +316,9 @@ static int i915_getparam(struct drm_device >*dev, void *data, >> > > > > case I915_PARAM_MIN_EU_IN_POOL: >> > > > > value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool; >> > > > > break; >> > > > > +case I915_PARAM_HUC_STATUS: >> > > > > +value = I915_READ(HUC_STATUS2) & >HUC_FW_VERIFIED; >> > > > >> > > > Same question as last time: does the device need to be awake? We >> > > > know is one of the GT power wells, so presumably we need an >> > > > rpm_get/rpm_put as well to access the register. >> > > > -Chris >> > > >> > > I get: >> > > >> > > [ 1588.570174] [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 >> > > PRE 24704 [ 1588.571285] [drm:intel_runtime_suspend [i915]] >> > > Suspending device [ 1588.575768] [drm:intel_runtime_suspend >> > > [i915]] Device suspended [ 1588.577156] >> > > [drm:i915_huc_load_status_info [i915]] HUC_STATUS2 POST 24704 [ >> > > 1588.578259] [drm:intel_runtime_resume [i915]] Resuming device >> > > >> > > consistently from: >> > > >> > > value = I915_READ(HUC_STATUS2); >> > > DRM_DEBUG_DRIVER("HUC_STATUS2 PRE %d\n", value); >> > > i915_pm_ops.runtime_suspend(dev_priv->drm.dev); >> > > >> > > value = I915_READ(HUC_STATUS2); >> > > DRM_DEBUG_DRIVER("HUC_STATUS2 POST %d\n", value); >> > > i915_pm_ops.runtime_resume(dev_priv->drm.dev); >> > >> > Also do the test with i915.mmio_debug= -Chris >> >> Same effect. Works. Thanks Arek for confirming. >Ok, then just mark up that we don't need rpm here so that we don't freak out in >future scans for mmio access outside of rpm. >-Chris Chris, v2 as changed by Tvrtko suggests that forcewakes are removed since the register is force waken. Are you suggesting that adding that rpm not being required in the commit message will make things much more clearer? Anusha > >-- >Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/8] drm/i915/huc: Add BXT HuC Loading Support
>-Original Message- >From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] >Sent: Friday, December 16, 2016 8:16 AM >To: Srivatsa, Anusha ; intel- >g...@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915/huc: Add BXT HuC Loading Support > > >On 15/12/2016 22:29, anushasr wrote: >> From: Anusha Srivatsa >> >> This patch adds the HuC Loading for the BXT by using the updated file >> construction. >> >> Version 1.7 of the HuC firmware. >> >> v2: rebased. >> v3: rebased on top of drm-tip >> v4: rebased. >> v5: rebased. Rename BXT_FW_MAJOR to BXT_HUC_FW_ >> >> Cc: Jeff Mcgee >> Signed-off-by: Anusha Srivatsa >> Reviewed-by: Jeff McGee >> --- >> drivers/gpu/drm/i915/intel_huc_loader.c | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c >> b/drivers/gpu/drm/i915/intel_huc_loader.c >> index 0f929cc..f36efd4 100644 >> --- a/drivers/gpu/drm/i915/intel_huc_loader.c >> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c >> @@ -40,6 +40,10 @@ >> * Note that HuC firmware loading must be done before GuC loading. >> */ >> >> +#define BXT_HUC_FW_MAJOR 01 >> +#define BXT_HUC_FW_MINOR 07 >> +#define BXT_BLD_NUM 1398 >> + >> #define SKL_HUC_FW_MAJOR 01 >> #define SKL_HUC_FW_MINOR 07 >> #define SKL_BLD_NUM 1398 >> @@ -52,6 +56,9 @@ >> SKL_HUC_FW_MINOR, SKL_BLD_NUM) >> MODULE_FIRMWARE(I915_SKL_HUC_UCODE); >> >> +#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \ >> +BXT_HUC_FW_MINOR, BXT_BLD_NUM) >> +MODULE_FIRMWARE(I915_BXT_HUC_UCODE); > >More curiosity - given the versions between SKL and BXT are identical - is it >actually the same binary file in both cases? Hi Tvrtko its different binary file. Cheers, Anusha >Regards, >Tvrtko > >> /** >> * huc_ucode_xfer() - DMA's the firmware >> * @dev_priv: the drm device >> @@ -157,6 +164,10 @@ void intel_huc_init(struct drm_i915_private *dev_priv) >> fw_path = I915_SKL_HUC_UCODE; >> huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR; >> huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR; >> +} else if (IS_BROXTON(dev_priv)) { >> +fw_path = I915_BXT_HUC_UCODE; >> +huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR; >> +huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR; >> } >> >> huc_fw->uc_fw_path = fw_path; >> ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx