Re: [Intel-gfx] [PATCH 13/13] drm/i915: Announce support for framebuffer modifiers
On Tue, Feb 10, 2015 at 05:16:16PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Let the DRM core know we can handle it. > > v2: Change to boolean true. (Daniel Vetter) > > Signed-off-by: Tvrtko Ursulin > Signed-off-by: Daniel Vetter All merged except for the drm core patch - I'll do some testcases for that one first. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index e5e9221..0f2a6c7 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13172,6 +13172,8 @@ void intel_modeset_init(struct drm_device *dev) > dev->mode_config.preferred_depth = 24; > dev->mode_config.prefer_shadow = 1; > > + dev->mode_config.allow_fb_modifiers = true; > + > dev->mode_config.funcs = &intel_mode_funcs; > > intel_init_quirks(dev); > -- > 2.2.2 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/13] drm/i915: Announce support for framebuffer modifiers
On Tue, Feb 10, 2015 at 06:46:39PM -0800, shuang...@intel.com wrote: > Tested-By: PRC QA PRTS (Patch Regression Test System Contact: > shuang...@intel.com) > Task id: 5750 > -Summary- > Platform Delta drm-intel-nightly Series Applied > PNV +5-4 275/283 276/283 > ILK 310/315 310/315 > SNB +3-1 320/346 322/346 > IVB -1 380/384 379/384 > BYT 296/296 296/296 > HSW +3-1 422/428 424/428 > BDW 318/333 318/333 > -Detailed- > Platform Testdrm-intel-nightly > Series Applied > *PNV igt_gem_fence_thrash_bo-write-verify-none PASS(3, M7) FAIL(1, > M7) > *PNV igt_gem_fence_thrash_bo-write-verify-x PASS(3, M7) FAIL(1, M7) > *PNV igt_gem_fence_thrash_bo-write-verify-y PASS(4, M7) FAIL(1, M7) > PNV igt_gem_userptr_blits_create-destroy-sync NRUN(1, M7)PASS(6, M7) >PASS(1, M7) > PNV igt_gen3_render_linear_blits FAIL(2, M7)PASS(2, M7) PASS(1, > M7) > PNV igt_gen3_render_mixed_blits FAIL(2, M7)PASS(2, M7) PASS(1, M7) > PNV igt_gen3_render_tiledx_blits FAIL(2, M7)TIMEOUT(1, M7)PASS(4, M7) > PASS(1, M7) > PNV igt_gen3_render_tiledy_blits FAIL(3, M7)PASS(2, M7) PASS(1, > M7) > *PNV igt_gem_tiled_pread_pwrite PASS(4, M7) FAIL(1, M7) > *SNB igt_kms_flip_bo-too-big BLACKLIST(1, M35) PASS(1, M35) > *SNB igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M35) > PASS(1, M35) > *SNB igt_kms_flip_event_leak NSPT(5, M35) PASS(1, M35) > *SNB igt_kms_flip_tiling_flip-changes-tiling PASS(2, M35) FAIL(1, > M35) > IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(1, M4)PASS(4, M4) >DMESG_WARN(1, M4) > *HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance > PASS(3, M40) DMESG_WARN(1, M40) > *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M40) PASS(1, M40) > *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M40) > PASS(1, M40) Why does PRTS report BLACKLIST -> PASS changes in the public test results? Can you please fix that of if it's more involved create a JIRA for it? Thanks, Daniel > HSW igt_kms_flip_plain-flip-fb-recreate-interruptible TIMEOUT(5, > M40)PASS(3, M40) PASS(1, M40) > Note: You need to pay more attention to line start with '*' > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/13] drm/i915: Use fb modifiers in intel_check_cursor_plane
On Tue, Feb 10, 2015 at 05:16:14PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Also drop the mutex since with universal planes object tiling mode is > locked down while assigned to a framebuffer. I've clarified this to say that universal implies fb exists, and that implies the tiling is locked down. > > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/intel_display.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 38c2909..edd6cfe 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12175,12 +12175,11 @@ intel_check_cursor_plane(struct drm_plane *plane, > return 0; > > /* we only need to pin inside GTT if cursor is non-phy */ > - mutex_lock(&dev->struct_mutex); > - if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) { > + if (!INTEL_INFO(dev)->cursor_needs_physical && I've dropped the cursor_needs_physical check here too because it doesn't make a lot of sense really. Tiled cursors really don't make any sense at all. -Daniel > + fb->modifier[0] != DRM_FORMAT_MOD_NONE) { > DRM_DEBUG_KMS("cursor cannot be tiled\n"); > ret = -EINVAL; > } > - mutex_unlock(&dev->struct_mutex); > > finish: > if (intel_crtc->active) { > -- > 2.2.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 0/8] Add enlightenments for vGPU
On 2/10/2015 8:11 PM, Tvrtko Ursulin wrote: On 02/10/2015 11:05 AM, Yu Zhang wrote: This patch set includes necessary code changes when i915 driver runs inside a VM. Though ideally we can run an unmodified i915 driver in VM, adding such enlightenments can greatly reduce the virtualization complexity in orders of magnitude. Code changes for the host side, which includes the actual Intel GVT-g implementation, will be sent out in other patches. The primary change introduced here is to implement so-called "address space ballooning" technique. XenGT partitions global graphics memory among multiple VMs, so each VM can directly access a portion of the memory without hypervisor's intervention, e.g. filling textures or queuing commands. However with the partitioning an unmodified i915 driver would assume a smaller graphics memory starting from address ZERO, so requires XenGT core module (vgt) to translate the graphics address between 'guest view' and 'host view', for all registers and command opcodes which contain a graphics memory address. To reduce the complexity, XenGT introduces "address space ballooning", by telling the exact partitioning knowledge to each guest i915 driver, which then reserves and prevents non-allocated portions from allocation. Then vgt module only needs to scan and validate graphics addresses without complexity of translation. Note: The partitioning of global graphics memory may break some applications, with large objects in the aperture, because current userspace assumes half of the aperture usable. That would need separate fix either in user space (e.g. remove assumption in mesa) or in kernel (with some faulting mechanism). The partitioning knowledge is conveyed through a reserved MMIO range, called PVINFO, which will be architecturally reserved in future hardware generations. Another information carried through PVINFO is about the number of fence registers. As a global resource, XenGT also partitions them among VMs. Other changes are trivial as optimizations, to either reduce the trap overhead or disable power management features which don't make sense in a virtualized environment. Yu Zhang (8): drm/i915: Introduce a PV INFO page structure for Intel GVT-g. drm/i915: Adds graphic address space ballooning logic drm/i915: Partition the fence registers for vGPU in i915 driver drm/i915: Disable framebuffer compression for i915 driver in VM drm/i915: Add the display switch logic for vGPU in i915 driver drm/i915: Disable power management for i915 driver in VM drm/i915: Create vGPU specific MMIO operations to reduce traps drm/i915: Support alias ppgtt in VM if ppgtt is enabled All my comments have been addressed (and I especially like the ASCII diagram of the memory space!) so you can put my r-b on all the patches from this series: Reviewed-by: Tvrtko Ursulin Thank you very much, Tvrtko. This is great. :-) Btw, should I resend another version patch series, which add the "Reviewed-by" line, or will Daniel directly merge these patches and add the "Reviewed-by" at that time? I'm willing to take whatever actions necessary, but I'm not familiar with this process. Thanks again. :) Yu Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/13] drm/i915/skl: Use fb modifiers for sprites
On Tue, Feb 10, 2015 at 05:16:13PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/intel_sprite.c | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index 0a52c44..9e6f0e5 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -245,11 +245,11 @@ skl_update_plane(struct drm_plane *drm_plane, struct > drm_crtc *crtc, > BUG(); > } > > - switch (obj->tiling_mode) { > - case I915_TILING_NONE: > + switch (fb->modifier[0]) { > + case DRM_FORMAT_MOD_NONE: > stride = fb->pitches[0] >> 6; > break; > - case I915_TILING_X: > + case I915_FORMAT_MOD_X_TILED: > plane_ctl |= PLANE_CTL_TILED_X; > stride = fb->pitches[0] >> 9; > break; > @@ -1076,7 +1076,6 @@ intel_check_sprite_plane(struct drm_plane *plane, > struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_framebuffer *fb = state->base.fb; > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > int crtc_x, crtc_y; > unsigned int crtc_w, crtc_h; > uint32_t src_x, src_y, src_w, src_h; > @@ -1107,9 +1106,9 @@ intel_check_sprite_plane(struct drm_plane *plane, > } > > /* Sprite planes can be linear or x-tiled surfaces */ > - switch (obj->tiling_mode) { > - case I915_TILING_NONE: > - case I915_TILING_X: > + switch (fb->modifier[0]) { > + case DRM_FORMAT_MOD_NONE: > + case I915_FORMAT_MOD_X_TILED: This check here is redundant (framebuffer_init checks this too), so I've removed it. -Daniel > break; > default: > DRM_DEBUG_KMS("Unsupported tiling mode\n"); > -- > 2.2.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/13] drm/i915/skl: CS flips are not supported with execlists
On Tue, Feb 10, 2015 at 05:16:12PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Therefore remove dead code. Commit message should state that skl requires execlist, otherwise it's not really clear why this is dead code. I've added that. -Daniel > > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/intel_display.c | 72 > ++-- > 1 file changed, 4 insertions(+), 68 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index df47031..38c2909 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9632,69 +9632,6 @@ static int intel_queue_mmio_flip(struct drm_device > *dev, > return 0; > } > > -static int intel_gen9_queue_flip(struct drm_device *dev, > - struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - struct drm_i915_gem_object *obj, > - struct intel_engine_cs *ring, > - uint32_t flags) > -{ > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - uint32_t plane = 0, stride; > - int ret; > - > - switch(intel_crtc->pipe) { > - case PIPE_A: > - plane = MI_DISPLAY_FLIP_SKL_PLANE_1_A; > - break; > - case PIPE_B: > - plane = MI_DISPLAY_FLIP_SKL_PLANE_1_B; > - break; > - case PIPE_C: > - plane = MI_DISPLAY_FLIP_SKL_PLANE_1_C; > - break; > - default: > - WARN_ONCE(1, "unknown plane in flip command\n"); > - return -ENODEV; > - } > - > - switch (obj->tiling_mode) { > - case I915_TILING_NONE: > - stride = fb->pitches[0] >> 6; > - break; > - case I915_TILING_X: > - stride = fb->pitches[0] >> 9; > - break; > - default: > - WARN_ONCE(1, "unknown tiling in flip command\n"); > - return -ENODEV; > - } > - > - ret = intel_ring_begin(ring, 10); > - if (ret) > - return ret; > - > - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > - intel_ring_emit(ring, DERRMR); > - intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE | > - DERRMR_PIPEB_PRI_FLIP_DONE | > - DERRMR_PIPEC_PRI_FLIP_DONE)); > - intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) | > - MI_SRM_LRM_GLOBAL_GTT); > - intel_ring_emit(ring, DERRMR); > - intel_ring_emit(ring, ring->scratch.gtt_offset + 256); > - intel_ring_emit(ring, 0); > - > - intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane); > - intel_ring_emit(ring, stride << 6 | obj->tiling_mode); > - intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset); > - > - intel_mark_page_flip_active(intel_crtc); > - __intel_ring_advance(ring); > - > - return 0; > -} > - > static int intel_default_queue_flip(struct drm_device *dev, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > @@ -12994,9 +12931,6 @@ static void intel_init_display(struct drm_device *dev) > valleyview_modeset_global_resources; > } > > - /* Default just returns -ENODEV to indicate unsupported */ > - dev_priv->display.queue_flip = intel_default_queue_flip; > - > switch (INTEL_INFO(dev)->gen) { > case 2: > dev_priv->display.queue_flip = intel_gen2_queue_flip; > @@ -13019,8 +12953,10 @@ static void intel_init_display(struct drm_device > *dev) > dev_priv->display.queue_flip = intel_gen7_queue_flip; > break; > case 9: > - dev_priv->display.queue_flip = intel_gen9_queue_flip; > - break; > + /* Drop through - unsupported since execlist only. */ > + default: > + /* Default just returns -ENODEV to indicate unsupported */ > + dev_priv->display.queue_flip = intel_default_queue_flip; > } > > intel_panel_init_backlight_funcs(dev); > -- > 2.2.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/13] drm/i915: Show frame buffer modifier in debug info
On Tue, Feb 10, 2015 at 05:16:07PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/i915_debugfs.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 2e1f723..164fa82 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1778,11 +1778,12 @@ static int i915_gem_framebuffer_info(struct seq_file > *m, void *data) > ifbdev = dev_priv->fbdev; > fb = to_intel_framebuffer(ifbdev->helper.fb); > > - seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, refcount %d, obj > ", > + seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, > refcount %d, obj ", > fb->base.width, > fb->base.height, > fb->base.depth, > fb->base.bits_per_pixel, > +fb->base.modifier[0], > atomic_read(&fb->base.refcount.refcount)); > describe_obj(m, fb->obj); > seq_putc(m, '\n'); > @@ -1793,11 +1794,12 @@ static int i915_gem_framebuffer_info(struct seq_file > *m, void *data) > if (ifbdev && &fb->base == ifbdev->helper.fb) > continue; > > - seq_printf(m, "user size: %d x %d, depth %d, %d bpp, refcount > %d, obj ", > + seq_printf(m, "user size: %d x %d, depth %d, %d bpp, modifier > 0x%llx, refcount %d, obj ", > fb->base.width, > fb->base.height, > fb->base.depth, > fb->base.bits_per_pixel, > +fb->base.modifier[0], Just an aside, in case you get around to decoding modifiers: We should probably switch to printing the fourcc code and decode that while at it. And perhaps print the stride too, since with the new tiling there will be no underlying fence any more to guess the stride. Perhaps we need a describe_fb helper to keep this manageble, too. -Daniel > atomic_read(&fb->base.refcount.refcount)); > describe_obj(m, fb->obj); > seq_putc(m, '\n'); > -- > 2.2.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Tue, Feb 10, 2015 at 10:26:02PM -0800, O'Rourke, Tom wrote: > On Thu, Jan 29, 2015 at 08:56:06PM -0500, Michael Auchter wrote: > > On Thu, Jan 29, 2015 at 06:12:31PM +0100, Daniel Vetter wrote: > > > On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote: > > > > On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote: > > > > > On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote: > > > > > > On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote: > > > > > > > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) > > > > > > > resulted in > > > > > > > this WARN at boot (and pretty frequently afterwards): > > > > > > > > > > > > > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 > > > > > > > gen6_set_rps+0x371/0x3c0() > > > > > > > WARN_ON(val > dev_priv->rps.max_freq_softlimit) > > > > > > > > > > > > [snip] > > > > > > > > > > > > > I'm not at all familiar with this hardware, but I took a quick > > > > > > > look into > > > > > > > what changed with this commit for my laptop. Before the commit, > > > > > > > rps.min_freq_softlimit is 4 (from rps.min_freq) and > > > > > > > rps.max_freq_softlimit is 22. > > > > > > > > > > > > > > After the commit, rps.min_freq_softlimit is set to the > > > > > > > rps.efficient_freq value read from pcode, which is 34 on my > > > > > > > laptop. > > > > > > > So later when gen6_set_rps() is called with > > > > > > > rps.min_freq_softlimit that > > > > > > > warning is hit. > > > > > > > > > > > > > > Any thoughts? It certainly seems fishy that this commit causes > > > > > > > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit. > > > > > > > > > > > > Very fishy indeed. Moral of this story, never trust hw. > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > > > index 3e630feb18e4..bbedd2901c54 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > > @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct > > > > > > drm_device *dev) > > > > > > &ddcc_status); > > > > > > if (0 == ret) > > > > > > dev_priv->rps.efficient_freq = > > > > > > - (ddcc_status >> 8) & 0xff; > > > > > > + clamp_t(u8, > > > > > > + (ddcc_status >> 8) & 0xff, > > > > > > + dev_priv->rps.min_freq, > > > > > > + dev_priv->rps.max_freq); > > > > > > > > > > Maybe better to fall back to rp1_freq if this is bogus? > > > > > > > > > [TOR:] Michael, Thank you for bringing this problem to our attention. > > > > > > > > Yes, this function needs some range checking to maintain > > > > RPn <= RPe <= RP0. > > > > > > > > A value of 34 seems too high for RPe. > > > > What values does the Carbon X1 (Haswell) have for RPn and RP0? > > > > > > 4 & 22, already in Micheal's original bug report. > > > > > > Tom, can you pls polish the clamping into a proper patch with m-l > > > references? > > > > > > Micheal, can you please test the first hunk from Chris (the one that adds > > > the clamp) to make sure it does indeed address the WARN_ON you're seeing? > > > > The clamp suggested by Chris does indeed fix the WARN_ON. > > > > In the case where RPe is greater than RP0, RPe will now be clamped to > > RP0. Is this likely to result in increased power consumption? > > > > At a quick glance on my laptop it does not (idling around 5W before and > > after) but Ville had suggested earlier to fall back to RP1, which would > > be more consistent with previous kernels. > > > > Thanks again for the quick responses, > > Michael > > [TOR:] Michael, I discussed this report with a pcode architect here. > > The RPe value is clamped to the [RPn, RP0] range by pcode before > returning the value to the driver on Broadwell but not on Haswell. > > On Haswell, an efficient frequency value above RP0 is not a garbage > value and could result from a relatively flat efficiency curve. In > this situation, leakage power would dominate the efficiency curve > such that running at lower frequencies may not save power overall. > Higher leakage power may result from a higher package temperature. > > Running at RP0 may actually save power compared to RP1 by allowing > more time in RC6. Hm, I'd just go with clamping since that's what bdw does too. So Chris diff essentially. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Clamp efficient frequency to valid range
From: Tom O'Rourke The efficient frequency (RPe) should stay in the range RPn <= RPe <= RP0. The pcode clamps the returned value internally on Broadwell but not on Haswell. Fix for missing range check in commit 93ee29203f506582cca2bcec5f05041526d9ab0a Author: Tom O'Rourke Date: Wed Nov 19 14:21:52 2014 -0800 drm/i915: Use efficient frequency for HSW/BDW Reference: http://lists.freedesktop.org/archives/intel-gfx/2015-February/059802.html Reported-by: Michael Auchter Suggested-by: Chris Wilson Signed-off-by: Tom O'Rourke --- drivers/gpu/drm/i915/intel_pm.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a3b979d..602c443 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3998,7 +3998,10 @@ static void gen6_init_rps_frequencies(struct drm_device *dev) &ddcc_status); if (0 == ret) dev_priv->rps.efficient_freq = - (ddcc_status >> 8) & 0xff; + clamp_t(u8, + ((ddcc_status >> 8) & 0xff), + dev_priv->rps.min_freq, + dev_priv->rps.max_freq); } /* Preserve min/max settings in case of re-init */ -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Thu, Jan 29, 2015 at 08:56:06PM -0500, Michael Auchter wrote: > On Thu, Jan 29, 2015 at 06:12:31PM +0100, Daniel Vetter wrote: > > On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote: > > > On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote: > > > > On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote: > > > > > On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote: > > > > > > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) > > > > > > resulted in > > > > > > this WARN at boot (and pretty frequently afterwards): > > > > > > > > > > > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 > > > > > > gen6_set_rps+0x371/0x3c0() > > > > > > WARN_ON(val > dev_priv->rps.max_freq_softlimit) > > > > > > > > > > [snip] > > > > > > > > > > > I'm not at all familiar with this hardware, but I took a quick look > > > > > > into > > > > > > what changed with this commit for my laptop. Before the commit, > > > > > > rps.min_freq_softlimit is 4 (from rps.min_freq) and > > > > > > rps.max_freq_softlimit is 22. > > > > > > > > > > > > After the commit, rps.min_freq_softlimit is set to the > > > > > > rps.efficient_freq value read from pcode, which is 34 on my laptop. > > > > > > So later when gen6_set_rps() is called with rps.min_freq_softlimit > > > > > > that > > > > > > warning is hit. > > > > > > > > > > > > Any thoughts? It certainly seems fishy that this commit causes > > > > > > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit. > > > > > > > > > > Very fishy indeed. Moral of this story, never trust hw. > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > > index 3e630feb18e4..bbedd2901c54 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct > > > > > drm_device *dev) > > > > > &ddcc_status); > > > > > if (0 == ret) > > > > > dev_priv->rps.efficient_freq = > > > > > - (ddcc_status >> 8) & 0xff; > > > > > + clamp_t(u8, > > > > > + (ddcc_status >> 8) & 0xff, > > > > > + dev_priv->rps.min_freq, > > > > > + dev_priv->rps.max_freq); > > > > > > > > Maybe better to fall back to rp1_freq if this is bogus? > > > > > > > [TOR:] Michael, Thank you for bringing this problem to our attention. > > > > > > Yes, this function needs some range checking to maintain > > > RPn <= RPe <= RP0. > > > > > > A value of 34 seems too high for RPe. > > > What values does the Carbon X1 (Haswell) have for RPn and RP0? > > > > 4 & 22, already in Micheal's original bug report. > > > > Tom, can you pls polish the clamping into a proper patch with m-l > > references? > > > > Micheal, can you please test the first hunk from Chris (the one that adds > > the clamp) to make sure it does indeed address the WARN_ON you're seeing? > > The clamp suggested by Chris does indeed fix the WARN_ON. > > In the case where RPe is greater than RP0, RPe will now be clamped to > RP0. Is this likely to result in increased power consumption? > > At a quick glance on my laptop it does not (idling around 5W before and > after) but Ville had suggested earlier to fall back to RP1, which would > be more consistent with previous kernels. > > Thanks again for the quick responses, > Michael [TOR:] Michael, I discussed this report with a pcode architect here. The RPe value is clamped to the [RPn, RP0] range by pcode before returning the value to the driver on Broadwell but not on Haswell. On Haswell, an efficient frequency value above RP0 is not a garbage value and could result from a relatively flat efficiency curve. In this situation, leakage power would dominate the efficiency curve such that running at lower frequencies may not save power overall. Higher leakage power may result from a higher package temperature. Running at RP0 may actually save power compared to RP1 by allowing more time in RC6. Thanks, Tom O'Rourke ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Stop requesting error_state reports.
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5751 -Summary- Platform Delta drm-intel-nightly Series Applied PNV +4-4 275/283 275/283 ILK 310/315 310/315 SNB +3 320/346 323/346 IVB -1 380/384 379/384 BYT 296/296 296/296 HSW +3-1 422/428 424/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_fence_thrash_bo-write-verify-none PASS(4, M7) FAIL(1, M7) *PNV igt_gem_fence_thrash_bo-write-verify-x PASS(4, M7) FAIL(1, M7) *PNV igt_gem_fence_thrash_bo-write-verify-y PASS(5, M7) FAIL(1, M7) PNV igt_gem_userptr_blits_coherency-sync CRASH(4, M7)PASS(2, M7) PASS(1, M7) PNV igt_gem_userptr_blits_coherency-unsync CRASH(4, M7)PASS(3, M7) PASS(1, M7) PNV igt_gem_userptr_blits_create-destroy-sync NRUN(1, M7)PASS(7, M7) PASS(1, M7) *PNV igt_gem_userptr_blits_forked-unsync-swapping-mempressure-interruptible PASS(2, M7) NO_RESULT(1, M7) PNV igt_gen3_render_tiledx_blits FAIL(3, M7)TIMEOUT(1, M7)PASS(4, M7) FAIL(1, M7) PNV igt_gen3_render_tiledy_blits FAIL(3, M7)PASS(3, M7) PASS(1, M7) *SNB igt_kms_flip_bo-too-big BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_event_leak NSPT(5, M35) PASS(1, M35) *IVB igt_gem_storedw_batches_loop_secure-dispatch PASS(2, M4) DMESG_WARN(1, M4) *HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance PASS(3, M40) DMESG_WARN(1, M40) *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M40) PASS(1, M40) *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M40) PASS(1, M40) HSW igt_kms_flip_plain-flip-fb-recreate-interruptible TIMEOUT(5, M40)PASS(4, M40) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/13] drm/i915: Announce support for framebuffer modifiers
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5750 -Summary- Platform Delta drm-intel-nightly Series Applied PNV +5-4 275/283 276/283 ILK 310/315 310/315 SNB +3-1 320/346 322/346 IVB -1 380/384 379/384 BYT 296/296 296/296 HSW +3-1 422/428 424/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_fence_thrash_bo-write-verify-none PASS(3, M7) FAIL(1, M7) *PNV igt_gem_fence_thrash_bo-write-verify-x PASS(3, M7) FAIL(1, M7) *PNV igt_gem_fence_thrash_bo-write-verify-y PASS(4, M7) FAIL(1, M7) PNV igt_gem_userptr_blits_create-destroy-sync NRUN(1, M7)PASS(6, M7) PASS(1, M7) PNV igt_gen3_render_linear_blits FAIL(2, M7)PASS(2, M7) PASS(1, M7) PNV igt_gen3_render_mixed_blits FAIL(2, M7)PASS(2, M7) PASS(1, M7) PNV igt_gen3_render_tiledx_blits FAIL(2, M7)TIMEOUT(1, M7)PASS(4, M7) PASS(1, M7) PNV igt_gen3_render_tiledy_blits FAIL(3, M7)PASS(2, M7) PASS(1, M7) *PNV igt_gem_tiled_pread_pwrite PASS(4, M7) FAIL(1, M7) *SNB igt_kms_flip_bo-too-big BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_event_leak NSPT(5, M35) PASS(1, M35) *SNB igt_kms_flip_tiling_flip-changes-tiling PASS(2, M35) FAIL(1, M35) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(1, M4)PASS(4, M4) DMESG_WARN(1, M4) *HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance PASS(3, M40) DMESG_WARN(1, M40) *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M40) PASS(1, M40) *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M40) PASS(1, M40) HSW igt_kms_flip_plain-flip-fb-recreate-interruptible TIMEOUT(5, M40)PASS(3, M40) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not invalidate obj->pages under mempressure
On 02/09/2015 08:46 AM, Chris Wilson wrote: > On Sun, Feb 08, 2015 at 03:27:13PM -0800, Sean V Kelley wrote: >> >> >> On 01/16/2015 08:05 PM, Daniel Vetter wrote: >>> On Thu, Jan 15, 2015 at 08:44:00PM +, Chris Wilson wrote: On Thu, Jan 15, 2015 at 08:36:15PM +0100, Daniel Vetter wrote: > On Wed, Jan 14, 2015 at 9:34 PM, Chris Wilson > wrote: >> This (partially) reverts >> >> commit 5537252b6b6d71fb1a8ed7395a8e5babf91953fd Author: Chris >> Wilson Date: Tue Mar 25 13:23:06 >> 2014 + >> >> drm/i915: Invalidate our pages under memory pressure > > Shouldn't we also revert the hunk in i915_gem_free_objects? > Without the truncate vs. invalidate disdinction it seems to > have lost it's reason for existence ... No, setting MADV_DONTNEED has other nice properties during put_pages() - I think it is useful in its own right, for example that is where my page stealing code goes... >>> >>> Well right now I can't make sense of this bit any more (tbh I >>> didn't with the other code either, but overlooked that while >>> reviewing). When it's just there for future work but atm dead code >>> I prefer for it to get removed. -Daniel >> >> >> So can we also revert the hunk in i915_gem_free_objects? I would like >> to get this patch merged, it looks like that is the primary concern. > > A problem I have is that the test written to hit the exact condition > considered in the changelog does not ellict the bug. > > Can you test whether > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 39e032615b31..6269204ba16f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1030,6 +1030,7 @@ i915_gem_execbuffer_move_to_active(struct list_head > *vmas, > /* update for the implicit flush after a batch */ > obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; > } > + obj->dirty = 1; > if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) { > i915_gem_request_assign(&obj->last_fenced_req, req); > if (entry->flags & __EXEC_OBJECT_HAS_FENCE) { > > makes the bug go away. If so, I think the bug is in the caller not > setting reloc domains correctly. I think you may be right. This implies a caller issue, because essentially you are forcing a write back here as if it were in the write domain. No corruption seen. I will add reloc domains to my growing audit list. via drm-intel-nightly: 2014y-12m-08d-22h-24m-34s UTC integration manifest diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0c25f62..4cb2755 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -970,6 +970,8 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, /* update for the implicit flush after a batch */ obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS; } + + obj->dirty = 1; if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) { i915_gem_request_assign(&obj->last_fenced_req, req); if (entry->flags & __EXEC_OBJECT_HAS_FENCE) { Sean > -Chris > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Introduce bit definitions of CTXT_SR_CTRL register.
This patch introduces 2 bit definitions of context save/restore control register. Thanks comments from David/Thomas/Daniel. Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/intel_lrc.c | 3 ++- drivers/gpu/drm/i915/intel_lrc.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d05f3bc..2196e9c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1668,7 +1668,8 @@ populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object *ctx_o reg_state[CTX_LRI_HEADER_0] |= MI_LRI_FORCE_POSTED; reg_state[CTX_CONTEXT_CONTROL] = RING_CONTEXT_CONTROL(ring); reg_state[CTX_CONTEXT_CONTROL+1] = - _MASKED_BIT_ENABLE((1<<3) | MI_RESTORE_INHIBIT); + _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH | + CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT); reg_state[CTX_RING_HEAD] = RING_HEAD(ring->mmio_base); reg_state[CTX_RING_HEAD+1] = 0; reg_state[CTX_RING_TAIL] = RING_TAIL(ring->mmio_base); diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 6f2d7da..ced191f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -30,6 +30,8 @@ #define RING_ELSP(ring)((ring)->mmio_base+0x230) #define RING_EXECLIST_STATUS(ring) ((ring)->mmio_base+0x234) #define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244) +#define CTX_CTRL_INHIBIT_SYN_CTX_SWITCH (1 << 3) +#define CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT (1 << 0) #define RING_CONTEXT_STATUS_BUF(ring) ((ring)->mmio_base+0x370) #define RING_CONTEXT_STATUS_PTR(ring) ((ring)->mmio_base+0x3a0) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
Sorry it's correct, has already been set with _MASK_ENABLE_BIT(). Let me cook a patch. :) -Original Message- From: Wang, Zhi A Sent: Wednesday, February 11, 2015 8:18 AM To: 'Daniel Vetter'; Gordon, David S Cc: Daniel, Thomas; Intel-gfx@lists.freedesktop.org Subject: RE: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c Hi Daniel & Gordon & Thomas: Just found that, my bad. Thanks for giving so much introduction for me, a beginner. :) Let me cook a patch for it. BTW: CTXT_SR_CTRL register looks like a register needs mask. Does the bit 0 restore inhibit also needs _MASKED_BIT_ENABLE here? It looks like bit 0 is not set with _MASKED_BIT_ENABLE(). -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, February 11, 2015 6:19 AM To: Gordon, David S Cc: Wang, Zhi A; Daniel, Thomas; Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c On Tue, Feb 10, 2015 at 04:25:20PM +, Dave Gordon wrote: > On 09/02/15 15:05, Wang, Zhi A wrote: > > Hi Gurus: > > Forgive my junior HW knowledge, I just found that in execlist > > context initialization function populate_lr_context(), this line: > > > > reg_state[CTX_CONTEXT_CONTROL+1] = > > _MASKED_BIT_ENABLE((1<<3) | > > MI_RESTORE_INHIBIT); > > > > It seems the "Inhibit Synchronous Context Switch" bit is not set > > here, so when HW is trying to wait some events, e.g. semaphore, > > according to Bspec, per my basic understanding, it will switch out > > current context with some reason bit set. Here comes my question, I > > think if this situation happen, should i915 remember this context > > and try to re-schedule it in a proper time, e.g. before submitting a > > new context until the context_completed bit in CSB is set? I don't > > find a register that will remember the context switched out by > > waiting event, so I think it should be i915 to handle this situation > > or just set "Inhibit Synchronous Context Switch" bit here?... > > But that's exactly what it does already ... it sets bit 3, which is > the "Inhibit Synchronous Context Switch" bit you refer to. > > What's wrong here is that it should use a #define for this bit, and > for the "Restore Inhibit" bit -- for which it's actually using a value > defined in a completely different context (no pun intended), namely > the bits in a MI_SET_CONTEXT *instruction*. It just happens to work > because they have the same numerical value. So, what we need is: > > > > #define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244) > #define CTX_CTRL_INHIBIT_SYN_CTX_SWITCH (1 << 3) > #define CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT (1 << 0) > > and then change the line you pointed out to use the correct symbolic names. Can I have this in patch form please? I.e. convert to diff, paste the above into commit message and add sob line. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
Hi Daniel & Gordon & Thomas: Just found that, my bad. Thanks for giving so much introduction for me, a beginner. :) Let me cook a patch for it. BTW: CTXT_SR_CTRL register looks like a register needs mask. Does the bit 0 restore inhibit also needs _MASKED_BIT_ENABLE here? It looks like bit 0 is not set with _MASKED_BIT_ENABLE(). -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, February 11, 2015 6:19 AM To: Gordon, David S Cc: Wang, Zhi A; Daniel, Thomas; Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c On Tue, Feb 10, 2015 at 04:25:20PM +, Dave Gordon wrote: > On 09/02/15 15:05, Wang, Zhi A wrote: > > Hi Gurus: > > Forgive my junior HW knowledge, I just found that in execlist > > context initialization function populate_lr_context(), this line: > > > > reg_state[CTX_CONTEXT_CONTROL+1] = > > _MASKED_BIT_ENABLE((1<<3) | > > MI_RESTORE_INHIBIT); > > > > It seems the "Inhibit Synchronous Context Switch" bit is not set > > here, so when HW is trying to wait some events, e.g. semaphore, > > according to Bspec, per my basic understanding, it will switch out > > current context with some reason bit set. Here comes my question, I > > think if this situation happen, should i915 remember this context > > and try to re-schedule it in a proper time, e.g. before submitting a > > new context until the context_completed bit in CSB is set? I don't > > find a register that will remember the context switched out by > > waiting event, so I think it should be i915 to handle this situation > > or just set "Inhibit Synchronous Context Switch" bit here?... > > But that's exactly what it does already ... it sets bit 3, which is > the "Inhibit Synchronous Context Switch" bit you refer to. > > What's wrong here is that it should use a #define for this bit, and > for the "Restore Inhibit" bit -- for which it's actually using a value > defined in a completely different context (no pun intended), namely > the bits in a MI_SET_CONTEXT *instruction*. It just happens to work > because they have the same numerical value. So, what we need is: > > > > #define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244) > #define CTX_CTRL_INHIBIT_SYN_CTX_SWITCH (1 << 3) > #define CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT (1 << 0) > > and then change the line you pointed out to use the correct symbolic names. Can I have this in patch form please? I.e. convert to diff, paste the above into commit message and add sob line. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/12] drm/i915: Enable the maxfifo PM5 mode when appropriate on CHV
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5748 -Summary- Platform Delta drm-intel-nightly Series Applied PNV +3 275/283 278/283 ILK +1-2 310/315 309/315 SNB +3 320/346 323/346 IVB -1 380/384 379/384 BYT 296/296 296/296 HSW +3 422/428 425/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync CRASH(3, M7)PASS(2, M7) PASS(1, M7) PNV igt_gem_userptr_blits_coherency-unsync CRASH(3, M7)PASS(3, M7) PASS(1, M7) PNV igt_gem_userptr_blits_create-destroy-sync NRUN(1, M7)PASS(5, M7) PASS(1, M7) PNV igt_gen3_render_tiledx_blits FAIL(2, M7)TIMEOUT(1, M7)PASS(3, M7) FAIL(1, M7) ILK igt_gem_fenced_exec_thrash_no-spare-fences-busy-interruptible DMESG_WARN(1, M26)PASS(1, M26) DMESG_WARN(1, M26) *ILK igt_gem_unfence_active_buffers PASS(3, M26) DMESG_WARN(1, M26) *ILK igt_drv_suspend_forcewake DMESG_WARN(3, M26) PASS(1, M26) *SNB igt_kms_flip_bo-too-big BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_event_leak NSPT(5, M35) PASS(1, M35) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(1, M4)PASS(4, M4) DMESG_WARN(1, M4) *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M40) PASS(1, M40) *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M40) PASS(1, M40) HSW igt_kms_flip_plain-flip-fb-recreate-interruptible TIMEOUT(5, M40)PASS(2, M40) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Stop requesting error_state reports.
On Tue, Feb 10, 2015 at 02:32:48PM -0800, Rodrigo Vivi wrote: > On Tue, 2015-02-10 at 23:02 +0100, Daniel Vetter wrote: > > On Tue, Feb 10, 2015 at 11:27:30AM -0800, Rodrigo Vivi wrote: > > > These error states are great to know gpu state when it hangs. > > > > > > But since we don't have automated tools to do analysis we are > > > facing much noise on bugzilla with end users reporting just > > > because "log asked to", while gpu reset worked and users probably > > > never notice any screen issue. Most of these reportes don't know > > > when it happened or how to retrigger the issue and somethimes > > > they are not even on the mood to retest again. > > > > Hm, maybe we should reword it to make sure we only get good testers? > > > > Instead of "Please file ..." do a "If you can build&test kernels and see > > other issues together with this gpu hang notice file ..."? > > This is just one thing. Some OSVs complains we have to much noise for > end users. So 2 noises: bugzilla and logs. OSV noise is a different issue. We have already have i915.verbose_state_checks for that, maybe we need to add another one for gpu hangs. But imo that should actually come from OSV, not development (like Rob Clark has done for the state checker). > > I agree with Chris that we can't just mute these, overall the information > > is imo valuable. We just need to get better at filtering them, and have > > better information in the error states. E.g. with dri3 the pid/commi is > > always the one for X, Mika has a small patch to fix that. > > I do agree this error state is valuable. I really like it. > > > > > Closing our eyes won't make the bugs go away. > > Indeed. But this patch doesn't intend to close the eyes, but just open > when it has to be opened, i.e. when drm.debug is set. Imo drm.debug is too silent, we've had that ages ago and it resulted in lots of unecessary roundtrips in bug reports because reporters almost never attach the error state if there's a gpu hang. So that imo doesn't help either with reducing bug team workload. The message is this verbose because a single line was not good enough ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/chipset: Cache devid
On Tue, Feb 10, 2015 at 11:45:12PM +0100, Daniel Vetter wrote: > On Tue, Feb 10, 2015 at 11:39:45PM +0100, Daniel Vetter wrote: > > On Tue, Feb 10, 2015 at 11:37:42PM +0100, Daniel Vetter wrote: > > > On Tue, Feb 10, 2015 at 10:28:22PM +, Chris Wilson wrote: > > > > On Tue, Feb 10, 2015 at 10:59:16PM +0100, Daniel Vetter wrote: > > > > > Chris Wilson complained that this adds a lot of noise to the test > > > > > startup when full debugging is enabled, so let's cache it. We can do > > > > > that since there's only ever one intel gpu in a given system. > > > > > > > > > > Cc: Chris Wilson > > > > > Signed-off-by: Daniel Vetter > > > > > > > > Couldn't we move the devid cache to lib/drmtest.c::is_intel() ? > > > > > > Sounds like just another place where we should use the helper from > > > intel_chipset.c. Next patch in-flight ... > > > > Ok I'm blind, is_intel can fail. So I guess I should extract a new > > __get_drm_devid which can fail, put the caching in there (plus override) > > and use that in in intel_chipset.c ... > > Doesn't really work since doing the ioctl is part of the dance we do to > figure out whether the fd is really an intel or not :( Something like: diff --git a/lib/drmtest.c b/lib/drmtest.c index 7cdef36..4090a4a 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -72,6 +72,8 @@ * and [batchbuffer](intel-gpu-tools-intel-batchbuffer.html) libraries as dependencies. */ +uint16_t __drm_device_id; + static int is_i915_device(int fd) { drm_version_t version; @@ -100,7 +102,11 @@ is_intel(int fd) if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp))) return 0; - return IS_INTEL(devid); + if (!IS_INTEL(devid)) + return 0; + + __drm_device_id = devid; + return 1; } static void check_stop_rings(void) diff --git a/lib/drmtest.h b/lib/drmtest.h index 508cc83..fabf43e 100644 --- a/lib/drmtest.h +++ b/lib/drmtest.h @@ -70,6 +70,8 @@ static inline void *igt_mmap64(void *addr, size_t length, int prot, int flags, */ #define ALIGN(v, a) (((v) + (a)-1) & ~((a)-1)) +extern uint16_t __drm_device_id; + int drm_get_card(void); int __drm_open_any(void); int drm_open_any(void); diff --git a/lib/intel_chipset.c b/lib/intel_chipset.c index fafd232..33177c6 100644 --- a/lib/intel_chipset.c +++ b/lib/intel_chipset.c @@ -125,26 +125,15 @@ intel_get_pci_device(void) uint32_t intel_get_drm_devid(int fd) { - uint32_t devid = 0; const char *override; - override = getenv("INTEL_DEVID_OVERRIDE"); - if (override) { - devid = strtod(override, NULL); - } else { - struct drm_i915_getparam gp; - int ret; - - memset(&gp, 0, sizeof(gp)); - gp.param = I915_PARAM_CHIPSET_ID; - gp.value = (int *)&devid; - - ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)); - igt_assert(ret == 0); - errno = 0; - } + igt_assert(__drm_device_id); - return devid; + override = getenv("INTEL_DEVID_OVERRIDE"); + if (override) + return strtod(override, NULL); + else + return __drm_device_id; } /** -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/chipset: Cache devid
On Tue, Feb 10, 2015 at 11:39:45PM +0100, Daniel Vetter wrote: > On Tue, Feb 10, 2015 at 11:37:42PM +0100, Daniel Vetter wrote: > > On Tue, Feb 10, 2015 at 10:28:22PM +, Chris Wilson wrote: > > > On Tue, Feb 10, 2015 at 10:59:16PM +0100, Daniel Vetter wrote: > > > > Chris Wilson complained that this adds a lot of noise to the test > > > > startup when full debugging is enabled, so let's cache it. We can do > > > > that since there's only ever one intel gpu in a given system. > > > > > > > > Cc: Chris Wilson > > > > Signed-off-by: Daniel Vetter > > > > > > Couldn't we move the devid cache to lib/drmtest.c::is_intel() ? > > > > Sounds like just another place where we should use the helper from > > intel_chipset.c. Next patch in-flight ... > > Ok I'm blind, is_intel can fail. So I guess I should extract a new > __get_drm_devid which can fail, put the caching in there (plus override) > and use that in in intel_chipset.c ... Doesn't really work since doing the ioctl is part of the dance we do to figure out whether the fd is really an intel or not :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/chipset: Cache devid
On Tue, Feb 10, 2015 at 11:37:42PM +0100, Daniel Vetter wrote: > On Tue, Feb 10, 2015 at 10:28:22PM +, Chris Wilson wrote: > > On Tue, Feb 10, 2015 at 10:59:16PM +0100, Daniel Vetter wrote: > > > Chris Wilson complained that this adds a lot of noise to the test > > > startup when full debugging is enabled, so let's cache it. We can do > > > that since there's only ever one intel gpu in a given system. > > > > > > Cc: Chris Wilson > > > Signed-off-by: Daniel Vetter > > > > Couldn't we move the devid cache to lib/drmtest.c::is_intel() ? > > Sounds like just another place where we should use the helper from > intel_chipset.c. Next patch in-flight ... Ok I'm blind, is_intel can fail. So I guess I should extract a new __get_drm_devid which can fail, put the caching in there (plus override) and use that in in intel_chipset.c ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/chipset: Cache devid
On Tue, Feb 10, 2015 at 10:28:22PM +, Chris Wilson wrote: > On Tue, Feb 10, 2015 at 10:59:16PM +0100, Daniel Vetter wrote: > > Chris Wilson complained that this adds a lot of noise to the test > > startup when full debugging is enabled, so let's cache it. We can do > > that since there's only ever one intel gpu in a given system. > > > > Cc: Chris Wilson > > Signed-off-by: Daniel Vetter > > Couldn't we move the devid cache to lib/drmtest.c::is_intel() ? Sounds like just another place where we should use the helper from intel_chipset.c. Next patch in-flight ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Stop requesting error_state reports.
On Tue, 2015-02-10 at 23:02 +0100, Daniel Vetter wrote: > On Tue, Feb 10, 2015 at 11:27:30AM -0800, Rodrigo Vivi wrote: > > These error states are great to know gpu state when it hangs. > > > > But since we don't have automated tools to do analysis we are > > facing much noise on bugzilla with end users reporting just > > because "log asked to", while gpu reset worked and users probably > > never notice any screen issue. Most of these reportes don't know > > when it happened or how to retrigger the issue and somethimes > > they are not even on the mood to retest again. > > Hm, maybe we should reword it to make sure we only get good testers? > > Instead of "Please file ..." do a "If you can build&test kernels and see > other issues together with this gpu hang notice file ..."? This is just one thing. Some OSVs complains we have to much noise for end users. So 2 noises: bugzilla and logs. > > I agree with Chris that we can't just mute these, overall the information > is imo valuable. We just need to get better at filtering them, and have > better information in the error states. E.g. with dri3 the pid/commi is > always the one for X, Mika has a small patch to fix that. I do agree this error state is valuable. I really like it. > > Closing our eyes won't make the bugs go away. Indeed. But this patch doesn't intend to close the eyes, but just open when it has to be opened, i.e. when drm.debug is set. When users face a issue that bother/matter he would enabled debug and than we would receive the report. And QA/OSVs/Devs should always let drm.debug enabled in certain level anyway. > -Daniel Thanks, Rodrigo. > > > > > So, let's minimize our and end user's noise and protect this smaller > > message with drm.debug. Developers, OSVs and users that face > > real screen issue (should) always enabled this debug and will see > > the message when error state got dumped. > > > > Signed-off-by: Rodrigo Vivi > > --- > > drivers/gpu/drm/i915/i915_gpu_error.c | 6 +- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > > b/drivers/gpu/drm/i915/i915_gpu_error.c > > index 48ddbf4..77d63be 100644 > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > > @@ -1297,11 +1297,7 @@ void i915_capture_error_state(struct drm_device > > *dev, bool wedged, > > } > > > > if (!warned) { > > - DRM_INFO("GPU hangs can indicate a bug anywhere in the entire > > gfx stack, including userspace.\n"); > > - DRM_INFO("Please file a _new_ bug report on > > bugs.freedesktop.org against DRI -> DRM/Intel\n"); > > - DRM_INFO("drm/i915 developers can then reassign to the right > > component if it's not a kernel issue.\n"); > > - DRM_INFO("The gpu crash dump is required to analyze gpu hangs, > > so please always attach it.\n"); > > - DRM_INFO("GPU crash dump saved to > > /sys/class/drm/card%d/error\n", dev->primary->index); > > + DRM_DEBUG_DRIVER("GPU crash dump saved to > > /sys/class/drm/card%d/error\n", dev->primary->index); > > warned = true; > > } > > } > > -- > > 1.9.3 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/chipset: Cache devid
On Tue, Feb 10, 2015 at 10:59:16PM +0100, Daniel Vetter wrote: > Chris Wilson complained that this adds a lot of noise to the test > startup when full debugging is enabled, so let's cache it. We can do > that since there's only ever one intel gpu in a given system. > > Cc: Chris Wilson > Signed-off-by: Daniel Vetter Couldn't we move the devid cache to lib/drmtest.c::is_intel() ? Then I wonder what to do about the getenv override. Whether that is also better inside is_intel(). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Align initial plane backing objects correctly
On Tue, Feb 10, 2015 at 11:10:51PM +0100, Daniel Vetter wrote: > Some bios really like to joke and start the planes at an offset ... > hooray! > > Align start and end to fix this. > > v2: Fixup calculation of size, spotted by Chris Wilson. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86883 > Cc: sta...@vger.kernel.org > Cc: Johannes W > Cc: Chris Wilson > Cc: Jani Nikula > Reported-by: Johannes W > Signed-off-by: Daniel Vetter > > -- > > Johannes, can you please test this patch instead of the one from > Chris? > > Thanks, Daniel > --- > drivers/gpu/drm/i915/i915_gem_stolen.c | 5 + > drivers/gpu/drm/i915/intel_display.c | 18 -- > 2 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c > b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 59401f3b902c..4382696087c9 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -485,10 +485,7 @@ i915_gem_object_create_stolen_for_preallocated(struct > drm_device *dev, > stolen_offset, gtt_offset, size); > > /* KISS and expect everything to be page-aligned */ > - BUG_ON(stolen_offset & 4095); > - BUG_ON(size & 4095); > - > - if (WARN_ON(size == 0)) > + if (WARN_ON(size == 0 || stolen_offset & 4095 || size & 4095)) > return NULL; I'd vote for keeping these as two separate WARNs, or using a format string to print the bogus values. > stolen = kzalloc(sizeof(*stolen), GFP_KERNEL); > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 2655b63d65e9..308a620f2594 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2372,13 +2372,19 @@ intel_alloc_plane_obj(struct intel_crtc *crtc, > struct drm_i915_gem_object *obj = NULL; > struct drm_mode_fb_cmd2 mode_cmd = { 0 }; > struct drm_framebuffer *fb = &plane_config->fb->base; > - u32 base = plane_config->base; > + u32 base_aligned = round_down(plane_config->base, PAGE_SIZE); > + u32 size_aligned = round_up(plane_config->base + plane_config->size, > + PAGE_SIZE); > + > + size_aligned -= base_aligned; > > if (plane_config->size == 0) > return false; > > - obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base, > - > plane_config->size); Didn't we used to have a comment here wondering if the 1:1 mapping was sane. That feels like it might be a factor here. Given that that our plane_obj does not match the one used by the BIOS, do we have some mechanism for forcing the modeset (presuming that fastboot is otherwise successful)? > + obj = i915_gem_object_create_stolen_for_preallocated(dev, > + base_aligned, > + base_aligned, > + size_aligned); > if (!obj) > return false; > @@ -7694,7 +7700,7 @@ skylake_get_initial_plane_config(struct intel_crtc > *crtc, > aligned_height = intel_fb_align_height(dev, fb->height, > plane_config->tiling); > > - plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE); > + plane_config->size = fb->pitches[0] * aligned_height, PAGE_SIZE; Stray comma operator. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
On Tue, Feb 10, 2015 at 04:25:20PM +, Dave Gordon wrote: > On 09/02/15 15:05, Wang, Zhi A wrote: > > Hi Gurus: > > Forgive my junior HW knowledge, I just found that in execlist > > context initialization function populate_lr_context(), this line: > > > > reg_state[CTX_CONTEXT_CONTROL+1] = > > _MASKED_BIT_ENABLE((1<<3) | MI_RESTORE_INHIBIT); > > > > It seems the "Inhibit Synchronous Context Switch" bit is not set > > here, so when HW is trying to wait some events, e.g. semaphore, according to > > Bspec, per my basic understanding, it will switch out current context > > with some reason bit set. Here comes my question, I think if this > > situation happen, should i915 remember this context and try to > > re-schedule it in a proper time, e.g. before submitting a new context > > until the context_completed bit in CSB is set? I don't find a register > > that will remember the context switched out by waiting event, so I think > > it should be i915 to handle this situation or just set "Inhibit > > Synchronous Context Switch" bit here?... > > But that's exactly what it does already ... it sets bit 3, which is the > "Inhibit Synchronous Context Switch" bit you refer to. > > What's wrong here is that it should use a #define for this bit, and for > the "Restore Inhibit" bit -- for which it's actually using a value > defined in a completely different context (no pun intended), namely the > bits in a MI_SET_CONTEXT *instruction*. It just happens to work because > they have the same numerical value. So, what we need is: > > > > #define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244) > #define CTX_CTRL_INHIBIT_SYN_CTX_SWITCH (1 << 3) > #define CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT (1 << 0) > > and then change the line you pointed out to use the correct symbolic names. Can I have this in patch form please? I.e. convert to diff, paste the above into commit message and add sob line. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Align initial plane backing objects correctly
Some bios really like to joke and start the planes at an offset ... hooray! Align start and end to fix this. v2: Fixup calculation of size, spotted by Chris Wilson. v3: Fix serious fumble I've just spotted. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86883 Cc: sta...@vger.kernel.org Cc: Johannes W Cc: Chris Wilson Cc: Jani Nikula Reported-by: Johannes W Signed-off-by: Daniel Vetter -- Johannes, can you please test this patch instead of the one from Chris? Thanks, Daniel --- drivers/gpu/drm/i915/i915_gem_stolen.c | 5 + drivers/gpu/drm/i915/intel_display.c | 18 -- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 59401f3b902c..4382696087c9 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -485,10 +485,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, stolen_offset, gtt_offset, size); /* KISS and expect everything to be page-aligned */ - BUG_ON(stolen_offset & 4095); - BUG_ON(size & 4095); - - if (WARN_ON(size == 0)) + if (WARN_ON(size == 0 || stolen_offset & 4095 || size & 4095)) return NULL; stolen = kzalloc(sizeof(*stolen), GFP_KERNEL); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2655b63d65e9..fc855b9548ec 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2372,13 +2372,19 @@ intel_alloc_plane_obj(struct intel_crtc *crtc, struct drm_i915_gem_object *obj = NULL; struct drm_mode_fb_cmd2 mode_cmd = { 0 }; struct drm_framebuffer *fb = &plane_config->fb->base; - u32 base = plane_config->base; + u32 base_aligned = round_down(plane_config->base, PAGE_SIZE); + u32 size_aligned = round_up(plane_config->base + plane_config->size, + PAGE_SIZE); + + size_aligned -= base_aligned; if (plane_config->size == 0) return false; - obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base, - plane_config->size); + obj = i915_gem_object_create_stolen_for_preallocated(dev, +base_aligned, +base_aligned, +size_aligned); if (!obj) return false; @@ -6654,7 +6660,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, aligned_height = intel_fb_align_height(dev, fb->height, plane_config->tiling); - plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height); + plane_config->size = fb->pitches[0] * aligned_height; DRM_DEBUG_KMS("pipe/plane %c/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", pipe_name(pipe), plane, fb->width, fb->height, @@ -7694,7 +7700,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, aligned_height = intel_fb_align_height(dev, fb->height, plane_config->tiling); - plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE); + plane_config->size = fb->pitches[0] * aligned_height; DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", pipe_name(pipe), fb->width, fb->height, @@ -7787,7 +7793,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, aligned_height = intel_fb_align_height(dev, fb->height, plane_config->tiling); - plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height); + plane_config->size = fb->pitches[0] * aligned_height; DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", pipe_name(pipe), fb->width, fb->height, -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Align initial plane backing objects correctly
Some bios really like to joke and start the planes at an offset ... hooray! Align start and end to fix this. v2: Fixup calculation of size, spotted by Chris Wilson. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86883 Cc: sta...@vger.kernel.org Cc: Johannes W Cc: Chris Wilson Cc: Jani Nikula Reported-by: Johannes W Signed-off-by: Daniel Vetter -- Johannes, can you please test this patch instead of the one from Chris? Thanks, Daniel --- drivers/gpu/drm/i915/i915_gem_stolen.c | 5 + drivers/gpu/drm/i915/intel_display.c | 18 -- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 59401f3b902c..4382696087c9 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -485,10 +485,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, stolen_offset, gtt_offset, size); /* KISS and expect everything to be page-aligned */ - BUG_ON(stolen_offset & 4095); - BUG_ON(size & 4095); - - if (WARN_ON(size == 0)) + if (WARN_ON(size == 0 || stolen_offset & 4095 || size & 4095)) return NULL; stolen = kzalloc(sizeof(*stolen), GFP_KERNEL); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2655b63d65e9..308a620f2594 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2372,13 +2372,19 @@ intel_alloc_plane_obj(struct intel_crtc *crtc, struct drm_i915_gem_object *obj = NULL; struct drm_mode_fb_cmd2 mode_cmd = { 0 }; struct drm_framebuffer *fb = &plane_config->fb->base; - u32 base = plane_config->base; + u32 base_aligned = round_down(plane_config->base, PAGE_SIZE); + u32 size_aligned = round_up(plane_config->base + plane_config->size, + PAGE_SIZE); + + size_aligned -= base_aligned; if (plane_config->size == 0) return false; - obj = i915_gem_object_create_stolen_for_preallocated(dev, base, base, - plane_config->size); + obj = i915_gem_object_create_stolen_for_preallocated(dev, +base_aligned, +base_aligned, +size_aligned); if (!obj) return false; @@ -6654,7 +6660,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, aligned_height = intel_fb_align_height(dev, fb->height, plane_config->tiling); - plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height); + plane_config->size = fb->pitches[0] * aligned_height; DRM_DEBUG_KMS("pipe/plane %c/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", pipe_name(pipe), plane, fb->width, fb->height, @@ -7694,7 +7700,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, aligned_height = intel_fb_align_height(dev, fb->height, plane_config->tiling); - plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE); + plane_config->size = fb->pitches[0] * aligned_height, PAGE_SIZE; DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", pipe_name(pipe), fb->width, fb->height, @@ -7787,7 +7793,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, aligned_height = intel_fb_align_height(dev, fb->height, plane_config->tiling); - plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height); + plane_config->size = fb->pitches[0] * aligned_height; DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n", pipe_name(pipe), fb->width, fb->height, -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Stop requesting error_state reports.
On Tue, Feb 10, 2015 at 11:27:30AM -0800, Rodrigo Vivi wrote: > These error states are great to know gpu state when it hangs. > > But since we don't have automated tools to do analysis we are > facing much noise on bugzilla with end users reporting just > because "log asked to", while gpu reset worked and users probably > never notice any screen issue. Most of these reportes don't know > when it happened or how to retrigger the issue and somethimes > they are not even on the mood to retest again. Hm, maybe we should reword it to make sure we only get good testers? Instead of "Please file ..." do a "If you can build&test kernels and see other issues together with this gpu hang notice file ..."? I agree with Chris that we can't just mute these, overall the information is imo valuable. We just need to get better at filtering them, and have better information in the error states. E.g. with dri3 the pid/commi is always the one for X, Mika has a small patch to fix that. Closing our eyes won't make the bugs go away. -Daniel > > So, let's minimize our and end user's noise and protect this smaller > message with drm.debug. Developers, OSVs and users that face > real screen issue (should) always enabled this debug and will see > the message when error state got dumped. > > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/i915_gpu_error.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > b/drivers/gpu/drm/i915/i915_gpu_error.c > index 48ddbf4..77d63be 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1297,11 +1297,7 @@ void i915_capture_error_state(struct drm_device *dev, > bool wedged, > } > > if (!warned) { > - DRM_INFO("GPU hangs can indicate a bug anywhere in the entire > gfx stack, including userspace.\n"); > - DRM_INFO("Please file a _new_ bug report on > bugs.freedesktop.org against DRI -> DRM/Intel\n"); > - DRM_INFO("drm/i915 developers can then reassign to the right > component if it's not a kernel issue.\n"); > - DRM_INFO("The gpu crash dump is required to analyze gpu hangs, > so please always attach it.\n"); > - DRM_INFO("GPU crash dump saved to > /sys/class/drm/card%d/error\n", dev->primary->index); > + DRM_DEBUG_DRIVER("GPU crash dump saved to > /sys/class/drm/card%d/error\n", dev->primary->index); > warned = true; > } > } > -- > 1.9.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 03/17] lib/ioctl: gem_ prefix for igt_require_mmap_wc
On Tue, Feb 10, 2015 at 07:05:46PM +0100, Daniel Vetter wrote: > We stick to the overall prefix even for magic require functions. That seems a bit perverse. Move the #define to a new header perhaps, but the require is a function of igt, not part of the kernel GEM interface. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] lib/chipset: Cache devid
Chris Wilson complained that this adds a lot of noise to the test startup when full debugging is enabled, so let's cache it. We can do that since there's only ever one intel gpu in a given system. Cc: Chris Wilson Signed-off-by: Daniel Vetter --- lib/intel_chipset.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/intel_chipset.c b/lib/intel_chipset.c index fafd232f89c0..64d66eb29bae 100644 --- a/lib/intel_chipset.c +++ b/lib/intel_chipset.c @@ -125,9 +125,12 @@ intel_get_pci_device(void) uint32_t intel_get_drm_devid(int fd) { - uint32_t devid = 0; + static uint32_t devid = 0; const char *override; + if (devid) + return devid; + override = getenv("INTEL_DEVID_OVERRIDE"); if (override) { devid = strtod(override, NULL); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 01/17] lib/gt: api polish for igt_can_hang_ring
On Tue, Feb 10, 2015 at 09:36:43PM +, Chris Wilson wrote: > On Tue, Feb 10, 2015 at 07:05:44PM +0100, Daniel Vetter wrote: > > Align with common igt library style: > > - Push the igt_require into the function. > > - Push the intel_gen into the function. > > Ugh. intel_gen(intel_get_drm_devid(fd)) is the utmost worst offender > when it comes to polluting tests and debug logs with extra ioctls, > making test bring up harder than it should be. Well I did look at all of them and made sure it's at most a constant factor per subtest. And if you want to fix it the solution imo is certainyl not to cache this everywhere in each test (and complicate them), but in the library. I'll throw a patch for that on top. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 05/17] lib/ioctls: make gem_context_set/get_param infallible
On Tue, Feb 10, 2015 at 07:05:48PM +0100, Daniel Vetter wrote: > We have separate require checks already, so these failing is a bug in > the test logic. That makes it impossible to use the wrappers to test the ioctl though. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 13/17] tests: Align subtest with naming convention
On Tue, Feb 10, 2015 at 07:05:56PM +0100, Daniel Vetter wrote: > Yeah, historically grown but we should try to be somewhat consistent. > It helps with filtering testcases. I think you are going the wrong way, since the current consensus prefers XCS naming. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Really ignore long HPD pulses on eDP
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5747 -Summary- Platform Delta drm-intel-nightly Series Applied PNV +5-4 275/283 276/283 ILK +1 310/315 311/315 SNB +2 320/346 322/346 IVB -1 380/384 379/384 BYT 296/296 296/296 HSW +3 422/428 425/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_fence_thrash_bo-write-verify-none PASS(2, M7) FAIL(1, M7) *PNV igt_gem_fence_thrash_bo-write-verify-x PASS(2, M7) FAIL(1, M7) *PNV igt_gem_fence_thrash_bo-write-verify-y PASS(3, M7) FAIL(1, M7) PNV igt_gem_userptr_blits_coherency-unsync CRASH(2, M7)PASS(3, M7) PASS(1, M7) PNV igt_gem_userptr_blits_create-destroy-sync NRUN(1, M7)PASS(4, M7) PASS(1, M7) PNV igt_gen3_render_linear_blits FAIL(1, M7)PASS(2, M7) PASS(1, M7) PNV igt_gen3_render_mixed_blits FAIL(2, M7)PASS(1, M7) PASS(1, M7) PNV igt_gen3_render_tiledx_blits FAIL(1, M7)TIMEOUT(1, M7)PASS(3, M7) PASS(1, M7) *PNV igt_gem_tiled_pread_pwrite PASS(3, M7) FAIL(1, M7) *ILK igt_drv_suspend_forcewake DMESG_WARN(3, M26) PASS(1, M26) *SNB igt_kms_flip_bo-too-big BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M35) PASS(1, M35) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(1, M4)PASS(3, M4) DMESG_WARN(1, M4) *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M40) PASS(1, M40) *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M40) PASS(1, M40) HSW igt_kms_flip_plain-flip-fb-recreate-interruptible TIMEOUT(4, M40)PASS(2, M40) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 01/17] lib/gt: api polish for igt_can_hang_ring
On Tue, Feb 10, 2015 at 07:05:44PM +0100, Daniel Vetter wrote: > Align with common igt library style: > - Push the igt_require into the function. > - Push the intel_gen into the function. Ugh. intel_gen(intel_get_drm_devid(fd)) is the utmost worst offender when it comes to polluting tests and debug logs with extra ioctls, making test bring up harder than it should be. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Stop requesting error_state reports.
On Tue, Feb 10, 2015 at 11:27:30AM -0800, Rodrigo Vivi wrote: > These error states are great to know gpu state when it hangs. > > But since we don't have automated tools to do analysis we are > facing much noise on bugzilla with end users reporting just > because "log asked to", while gpu reset worked and users probably > never notice any screen issue. Other than the corruption and the machine stuttering for a number of seconds, sometimes many times per hour. I think that justifies having some form of user visible message about what just happened. > Most of these reportes don't know > when it happened or how to retrigger the issue and somethimes > they are not even on the mood to retest again. It is the goal of the error state to capture exactly enough information for post-mortem analysis. As you feel it is inadequate, please augment it. > So, let's minimize our and end user's noise and protect this smaller > message with drm.debug. Developers, OSVs and users that face > real screen issue (should) always enabled this debug and will see > the message when error state got dumped. I don't think the tradeoff is worth it. If you have automatic bug reporting, then yes you can remove the plea to have the user do it, but not before. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: obey wbinvd threshold in more places
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5739 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 282/283 282/283 ILK 271/278 271/278 SNB +2-22 340/346 320/346 IVB +1-2 378/384 377/384 BYT 296/296 296/296 HSW +4 421/428 425/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt_drv_suspend_debugfs-reader DMESG_WARN(1, M37) NO_RESULT(1, M37) *ILK igt_drv_suspend_fence-restore-tiled2untiled DMESG_WARN(1, M37) NO_RESULT(1, M37) *ILK igt_drv_suspend_fence-restore-untiled DMESG_WARN(1, M37) NO_RESULT(1, M37) *ILK igt_drv_suspend_forcewake DMESG_WARN(1, M37) NO_RESULT(1, M37) *ILK igt_gem_workarounds_suspend-resume DMESG_WARN(1, M37) INIT(1, M37) SNB igt_kms_cursor_crc_cursor-size-change NSPT(1, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_flip_event_leak NSPT(1, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_flip_modeset-vs-vblank-race DMESG_WARN(1, M22)PASS(1, M22) PASS(1, M22) SNB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_pipe_crc_basic_read-crc-pipe-A DMESG_WARN(1, M22)PASS(6, M22) PASS(1, M22) SNB igt_kms_rotation_crc_primary-rotation NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_rotation_crc_sprite-rotation NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_cursor NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_cursor-dpms NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_dpms-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_drm-resources-equal NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_fences NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_fences-dpms NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-execbuf NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-mmap-cpu NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-mmap-gtt NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-pread NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_i2c NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_modeset-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_pci-d3-state NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_rte NSPT(2, M22)PASS(1, M22) NSPT(1, M22) IVB igt_gem_pwrite_pread_snooped-copy-performance DMESG_WARN(1, M34)PASS(5, M34) DMESG_WARN(1, M34) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(2, M34)PASS(2, M34) PASS(1, M34) IVB igt_gem_storedw_batches_loop_secure-dispatch DMESG_WARN(1, M34)PASS(3, M34) DMESG_WARN(1, M34) HSW igt_gem_storedw_loop_blt DMESG_WARN(3, M20)PASS(3, M20) PASS(1, M20) HSW igt_gem_storedw_loop_vebox DMESG_WARN(3, M20)PASS(2, M20) PASS(1, M20) *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M20) PASS(1, M20) *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M20) PASS(1, M20) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/10] drm/i915: Garbage collect orphaned prototypes
There have been quite a bit of development lately, leaving behing lonely protypes. Time to bid them farewell. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_drv.h | 4 drivers/gpu/drm/i915/intel_drv.h | 9 - 2 files changed, 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7dfdd3c..6a020a9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2511,8 +2511,6 @@ extern int i915_max_ioctl; extern int i915_suspend_legacy(struct drm_device *dev, pm_message_t state); extern int i915_resume_legacy(struct drm_device *dev); -extern int i915_master_create(struct drm_device *dev, struct drm_master *master); -extern void i915_master_destroy(struct drm_device *dev, struct drm_master *master); /* i915_params.c */ struct i915_params { @@ -3213,8 +3211,6 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data, int i915_get_reset_stats_ioctl(struct drm_device *dev, void *data, struct drm_file *file); -void intel_notify_mmio_flip(struct intel_engine_cs *ring); - /* overlay */ extern struct intel_overlay_error_state *intel_overlay_capture_error_state(struct drm_device *dev); extern void intel_overlay_print_error_state(struct drm_i915_error_state_buf *e, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 76b3c20..8052d9c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1038,12 +1038,6 @@ void intel_dp_hot_plug(struct intel_encoder *intel_encoder); void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv); uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes); void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes); -int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h); -int intel_disable_plane(struct drm_plane *plane); void intel_plane_destroy(struct drm_plane *plane); void intel_edp_drrs_enable(struct intel_dp *intel_dp); void intel_edp_drrs_disable(struct intel_dp *intel_dp); @@ -1232,9 +1226,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob); int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane); void intel_flush_primary_plane(struct drm_i915_private *dev_priv, enum plane plane); -int intel_plane_set_property(struct drm_plane *plane, -struct drm_property *prop, -uint64_t val); int intel_plane_restore(struct drm_plane *plane); int intel_sprite_set_colorkey(struct drm_device *dev, void *data, struct drm_file *file_priv); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/10] Random cleanups
Ran my cleanup script again and caught a few tiny oversights. -- Damien Damien Lespiau (10): drm/i915: Garbage collect orphaned prototypes drm/i915: Make intel_ring_setup_status_page() static drm/i915: Remove intel_dsi_cmd.h drm/i915: Make intel_lr_context_render_state_init() static drm/i915: Make intel_logical_ring_begin() static drm/i915: Make intel_logical_ring_advance_and_submit() static drm/i915: Make intel_dp_check_link_status() static drm/i915: Make intel_dp_unpack_aux() static drm/i915: MAke intel_unpin_fb_obj() static drm/i915: Remove the IS_SNB_G1 define drivers/gpu/drm/i915/i915_drv.h | 7 - drivers/gpu/drm/i915/intel_display.c| 2 +- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_drv.h| 12 -- drivers/gpu/drm/i915/intel_dsi_cmd.h| 39 - drivers/gpu/drm/i915/intel_lrc.c| 293 drivers/gpu/drm/i915/intel_lrc.h| 9 - drivers/gpu/drm/i915/intel_ringbuffer.c | 124 +++--- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 9 files changed, 212 insertions(+), 279 deletions(-) delete mode 100644 drivers/gpu/drm/i915/intel_dsi_cmd.h -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/10] drm/i915: MAke intel_unpin_fb_obj() static
This function is not used outside of intel_display.c since; commit cf4c7c12258ed9367f4fc45238f5f50d2db892c1 Author: Matt Roper Date: Thu Dec 4 10:27:42 2014 -0800 drm/i915: Make all plane disables use 'update_plane' (v5) Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2655b63..736d254 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2282,7 +2282,7 @@ err_interruptible: return ret; } -void intel_unpin_fb_obj(struct drm_i915_gem_object *obj) +static void intel_unpin_fb_obj(struct drm_i915_gem_object *obj) { WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex)); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8c43479..1df1a14 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -933,7 +933,6 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, int intel_pin_and_fence_fb_obj(struct drm_plane *plane, struct drm_framebuffer *fb, struct intel_engine_cs *pipelined); -void intel_unpin_fb_obj(struct drm_i915_gem_object *obj); struct drm_framebuffer * __intel_framebuffer_create(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/10] drm/i915: Make intel_dp_unpack_aux() static
This was introduced in: commit 0bc12bcb1b9686d7011f16410ba17ed0740167c3 Author: Rodrigo Vivi Date: Fri Nov 14 08:52:28 2014 -0800 drm/i915: Introduce intel_psr.c Put the unpack function is unused at this date. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b9966dd..ae40127 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -243,7 +243,7 @@ uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes) return v; } -void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes) +static void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes) { int i; if (dst_bytes > 4) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f056036..8c43479 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1036,7 +1036,6 @@ int intel_dp_max_link_bw(struct intel_dp *intel_dp); void intel_dp_hot_plug(struct intel_encoder *intel_encoder); void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv); uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes); -void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes); void intel_plane_destroy(struct drm_plane *plane); void intel_edp_drrs_enable(struct intel_dp *intel_dp); void intel_edp_drrs_disable(struct intel_dp *intel_dp); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/10] drm/i915: Make intel_logical_ring_begin() static
This function is only used in intel_lrc.c, so restrict it to that file. One user of intel_logical_ring_begin() that was defined before the function was moved after it (it's less code movement than moving intel_logical_ring_begin() and its dependencies. intel_logical_ring_begin() was also removed from the documentation generation as it's not exposed from its compilation unit anymore. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_lrc.c | 218 +++ drivers/gpu/drm/i915/intel_lrc.h | 3 - 2 files changed, 109 insertions(+), 112 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index a818497..45d92f4 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -610,112 +610,6 @@ static int execlists_move_to_gpu(struct intel_ringbuffer *ringbuf, return logical_ring_invalidate_all_caches(ringbuf, ctx); } -/** - * execlists_submission() - submit a batchbuffer for execution, Execlists style - * @dev: DRM device. - * @file: DRM file. - * @ring: Engine Command Streamer to submit to. - * @ctx: Context to employ for this submission. - * @args: execbuffer call arguments. - * @vmas: list of vmas. - * @batch_obj: the batchbuffer to submit. - * @exec_start: batchbuffer start virtual address pointer. - * @flags: translated execbuffer call flags. - * - * This is the evil twin version of i915_gem_ringbuffer_submission. It abstracts - * away the submission details of the execbuffer ioctl call. - * - * Return: non-zero if the submission fails. - */ -int intel_execlists_submission(struct drm_device *dev, struct drm_file *file, - struct intel_engine_cs *ring, - struct intel_context *ctx, - struct drm_i915_gem_execbuffer2 *args, - struct list_head *vmas, - struct drm_i915_gem_object *batch_obj, - u64 exec_start, u32 flags) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; - int instp_mode; - u32 instp_mask; - int ret; - - instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK; - instp_mask = I915_EXEC_CONSTANTS_MASK; - switch (instp_mode) { - case I915_EXEC_CONSTANTS_REL_GENERAL: - case I915_EXEC_CONSTANTS_ABSOLUTE: - case I915_EXEC_CONSTANTS_REL_SURFACE: - if (instp_mode != 0 && ring != &dev_priv->ring[RCS]) { - DRM_DEBUG("non-0 rel constants mode on non-RCS\n"); - return -EINVAL; - } - - if (instp_mode != dev_priv->relative_constants_mode) { - if (instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) { - DRM_DEBUG("rel surface constants mode invalid on gen5+\n"); - return -EINVAL; - } - - /* The HW changed the meaning on this bit on gen6 */ - instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE; - } - break; - default: - DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode); - return -EINVAL; - } - - if (args->num_cliprects != 0) { - DRM_DEBUG("clip rectangles are only valid on pre-gen5\n"); - return -EINVAL; - } else { - if (args->DR4 == 0x) { - DRM_DEBUG("UXA submitting garbage DR4, fixing up\n"); - args->DR4 = 0; - } - - if (args->DR1 || args->DR4 || args->cliprects_ptr) { - DRM_DEBUG("0 cliprects but dirt in cliprects fields\n"); - return -EINVAL; - } - } - - if (args->flags & I915_EXEC_GEN7_SOL_RESET) { - DRM_DEBUG("sol reset is gen7 only\n"); - return -EINVAL; - } - - ret = execlists_move_to_gpu(ringbuf, ctx, vmas); - if (ret) - return ret; - - if (ring == &dev_priv->ring[RCS] && - instp_mode != dev_priv->relative_constants_mode) { - ret = intel_logical_ring_begin(ringbuf, ctx, 4); - if (ret) - return ret; - - intel_logical_ring_emit(ringbuf, MI_NOOP); - intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1)); - intel_logical_ring_emit(ringbuf, INSTPM); - intel_logical_ring_emit(ringbuf, instp_mask << 16 | instp_mode); - intel_logical_ring_advance(ringbuf); - - dev_priv->relative_constants_mode = instp_mode; - } - - ret = ring->emit_bb_start(ringbuf, ctx, exec_start, flags); - if (ret) - return ret; - - i915_gem_execbuffer_move_to_active(
[Intel-gfx] [PATCH 06/10] drm/i915: Make intel_logical_ring_advance_and_submit() static
This function is only used in intel_lrc.c, so restrict it to that file. The function was moved around to avoid a forward declaration and group it with its user. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_lrc.c | 9 + drivers/gpu/drm/i915/intel_lrc.h | 4 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 45d92f4..5c04510 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -679,7 +679,7 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf, return 0; } -/** +/* * intel_logical_ring_advance_and_submit() - advance the tail and submit the workload * @ringbuf: Logical Ringbuffer to advance. * @@ -688,9 +688,10 @@ int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf, * on a queue waiting for the ELSP to be ready to accept a new context submission. At that * point, the tail *inside* the context is updated and the ELSP written to. */ -void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf, - struct intel_context *ctx, - struct drm_i915_gem_request *request) +static void +intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf, + struct intel_context *ctx, + struct drm_i915_gem_request *request) { struct intel_engine_cs *ring = ringbuf->ring; diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 454ecb0..98910b5 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -40,10 +40,6 @@ int intel_logical_rings_init(struct drm_device *dev); int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf, struct intel_context *ctx); -void intel_logical_ring_advance_and_submit( - struct intel_ringbuffer *ringbuf, - struct intel_context *ctx, - struct drm_i915_gem_request *request); /** * intel_logical_ring_advance() - advance the ringbuffer tail * @ringbuf: Ringbuffer to advance. -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/10] drm/i915: Remove the IS_SNB_G1 define
The last (only?) user of this was removed in: commit 2208d655a91f9879bd9a39ff9df05dd668b3512c Author: Daniel Vetter Date: Fri Nov 14 09:25:29 2014 +0100 drm/i915: drop WaSetupGtModeTdRowDispatch:snb Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_drv.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6a020a9..88b56c4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2373,9 +2373,6 @@ struct drm_i915_cmd_table { #define IS_IVB_GT1(dev)(INTEL_DEVID(dev) == 0x0156 || \ INTEL_DEVID(dev) == 0x0152 || \ INTEL_DEVID(dev) == 0x015a) -#define IS_SNB_GT1(dev)(INTEL_DEVID(dev) == 0x0102 || \ -INTEL_DEVID(dev) == 0x0106 || \ -INTEL_DEVID(dev) == 0x010A) #define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview) #define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_valleyview && IS_GEN8(dev)) #define IS_HASWELL(dev)(INTEL_INFO(dev)->is_haswell) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/10] drm/i915: Remove intel_dsi_cmd.h
This header has been unusued since: commit 063c86f60ad4064b2cf62041bee8c6389e180b76 Author: Jani Nikula Date: Fri Jan 16 14:27:27 2015 +0200 drm/i915/dsi: remove intel_dsi_cmd.c and the unused functions therein Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_dsi_cmd.h | 39 1 file changed, 39 deletions(-) delete mode 100644 drivers/gpu/drm/i915/intel_dsi_cmd.h diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.h b/drivers/gpu/drm/i915/intel_dsi_cmd.h deleted file mode 100644 index 8867790..000 --- a/drivers/gpu/drm/i915/intel_dsi_cmd.h +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright © 2013 Intel Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - * - * Author: Jani Nikula - */ - -#ifndef _INTEL_DSI_DSI_H -#define _INTEL_DSI_DSI_H - -#include -#include -#include -#include "i915_drv.h" -#include "intel_drv.h" -#include "intel_dsi.h" - -void dsi_hs_mode_enable(struct intel_dsi *intel_dsi, bool enable, - enum port port); - -#endif /* _INTEL_DSI_DSI_H */ -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/10] drm/i915: Make intel_lr_context_render_state_init() static
This function is only used in intel_lrc.c, so restrict it to that file. The function was moved around to avoid a forward declaration and group it with its user. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_lrc.c | 66 drivers/gpu/drm/i915/intel_lrc.h | 2 -- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 5d88c7a..a818497 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1336,6 +1336,39 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf, return 0; } +static int intel_lr_context_render_state_init(struct intel_engine_cs *ring, + struct intel_context *ctx) +{ + struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; + struct render_state so; + struct drm_i915_file_private *file_priv = ctx->file_priv; + struct drm_file *file = file_priv ? file_priv->file : NULL; + int ret; + + ret = i915_gem_render_state_prepare(ring, &so); + if (ret) + return ret; + + if (so.rodata == NULL) + return 0; + + ret = ring->emit_bb_start(ringbuf, + ctx, + so.ggtt_offset, + I915_DISPATCH_SECURE); + if (ret) + goto out; + + i915_vma_move_to_active(i915_gem_obj_to_ggtt(so.obj), ring); + + ret = __i915_add_request(ring, file, so.obj); + /* intel_logical_ring_add_request moves object to inactive if it +* fails */ +out: + i915_gem_render_state_fini(&so); + return ret; +} + static int gen8_init_rcs_context(struct intel_engine_cs *ring, struct intel_context *ctx) { @@ -1604,39 +1637,6 @@ cleanup_render_ring: return ret; } -int intel_lr_context_render_state_init(struct intel_engine_cs *ring, - struct intel_context *ctx) -{ - struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; - struct render_state so; - struct drm_i915_file_private *file_priv = ctx->file_priv; - struct drm_file *file = file_priv ? file_priv->file : NULL; - int ret; - - ret = i915_gem_render_state_prepare(ring, &so); - if (ret) - return ret; - - if (so.rodata == NULL) - return 0; - - ret = ring->emit_bb_start(ringbuf, - ctx, - so.ggtt_offset, - I915_DISPATCH_SECURE); - if (ret) - goto out; - - i915_vma_move_to_active(i915_gem_obj_to_ggtt(so.obj), ring); - - ret = __i915_add_request(ring, file, so.obj); - /* intel_logical_ring_add_request moves object to inactive if it -* fails */ -out: - i915_gem_render_state_fini(&so); - return ret; -} - static u32 make_rpcs(struct drm_device *dev) { diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 6f2d7da..94057b9 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -70,8 +70,6 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords); /* Logical Ring Contexts */ -int intel_lr_context_render_state_init(struct intel_engine_cs *ring, - struct intel_context *ctx); void intel_lr_context_free(struct intel_context *ctx); int intel_lr_context_deferred_create(struct intel_context *ctx, struct intel_engine_cs *ring); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/10] drm/i915: Make intel_dp_check_link_status() static
This function is only used in intel_dp.c since: commit 0e32b39ceed665bfa4a77a4bc307b6652b991632 Author: Dave Airlie Date: Fri May 2 14:02:48 2014 +1000 drm/i915: add DP 1.2 MST support (v0.7) Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4a60c6a..b9966dd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3806,7 +3806,7 @@ go_again: * 3. Use Link Training from 2.5.3.3 and 3.5.1.3 * 4. Check link status on receipt of hot-plug interrupt */ -void +static void intel_dp_check_link_status(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8052d9c..f056036 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1018,7 +1018,6 @@ void intel_dp_complete_link_train(struct intel_dp *intel_dp); void intel_dp_stop_link_train(struct intel_dp *intel_dp); void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); void intel_dp_encoder_destroy(struct drm_encoder *encoder); -void intel_dp_check_link_status(struct intel_dp *intel_dp); int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc); bool intel_dp_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/10] drm/i915: Make intel_ring_setup_status_page() static
This function is only used in intel_ringbuffer.c, so restrict it to that file. The function was moved around to avoid a forward declaration and group it with its user. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_ringbuffer.c | 124 drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 2 files changed, 62 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0631780..0a7b1af 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -502,6 +502,68 @@ static void ring_setup_phys_status_page(struct intel_engine_cs *ring) I915_WRITE(HWS_PGA, addr); } +void intel_ring_setup_status_page(struct intel_engine_cs *ring) +{ + struct drm_device *dev = ring->dev; + struct drm_i915_private *dev_priv = ring->dev->dev_private; + u32 mmio = 0; + + /* The ring status page addresses are no longer next to the rest of +* the ring registers as of gen7. +*/ + if (IS_GEN7(dev)) { + switch (ring->id) { + case RCS: + mmio = RENDER_HWS_PGA_GEN7; + break; + case BCS: + mmio = BLT_HWS_PGA_GEN7; + break; + /* +* VCS2 actually doesn't exist on Gen7. Only shut up +* gcc switch check warning +*/ + case VCS2: + case VCS: + mmio = BSD_HWS_PGA_GEN7; + break; + case VECS: + mmio = VEBOX_HWS_PGA_GEN7; + break; + } + } else if (IS_GEN6(ring->dev)) { + mmio = RING_HWS_PGA_GEN6(ring->mmio_base); + } else { + /* XXX: gen8 returns to sanity */ + mmio = RING_HWS_PGA(ring->mmio_base); + } + + I915_WRITE(mmio, (u32)ring->status_page.gfx_addr); + POSTING_READ(mmio); + + /* +* Flush the TLB for this page +* +* FIXME: These two bits have disappeared on gen8, so a question +* arises: do we still need this and if so how should we go about +* invalidating the TLB? +*/ + if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8) { + u32 reg = RING_INSTPM(ring->mmio_base); + + /* ring should be idle before issuing a sync flush*/ + WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0); + + I915_WRITE(reg, + _MASKED_BIT_ENABLE(INSTPM_TLB_INVALIDATE | + INSTPM_SYNC_FLUSH)); + if (wait_for((I915_READ(reg) & INSTPM_SYNC_FLUSH) == 0, +1000)) + DRM_ERROR("%s: wait for SyncFlush to complete for TLB invalidation timed out\n", + ring->name); + } +} + static bool stop_ring(struct intel_engine_cs *ring) { struct drm_i915_private *dev_priv = to_i915(ring->dev); @@ -1486,68 +1548,6 @@ i8xx_ring_put_irq(struct intel_engine_cs *ring) spin_unlock_irqrestore(&dev_priv->irq_lock, flags); } -void intel_ring_setup_status_page(struct intel_engine_cs *ring) -{ - struct drm_device *dev = ring->dev; - struct drm_i915_private *dev_priv = ring->dev->dev_private; - u32 mmio = 0; - - /* The ring status page addresses are no longer next to the rest of -* the ring registers as of gen7. -*/ - if (IS_GEN7(dev)) { - switch (ring->id) { - case RCS: - mmio = RENDER_HWS_PGA_GEN7; - break; - case BCS: - mmio = BLT_HWS_PGA_GEN7; - break; - /* -* VCS2 actually doesn't exist on Gen7. Only shut up -* gcc switch check warning -*/ - case VCS2: - case VCS: - mmio = BSD_HWS_PGA_GEN7; - break; - case VECS: - mmio = VEBOX_HWS_PGA_GEN7; - break; - } - } else if (IS_GEN6(ring->dev)) { - mmio = RING_HWS_PGA_GEN6(ring->mmio_base); - } else { - /* XXX: gen8 returns to sanity */ - mmio = RING_HWS_PGA(ring->mmio_base); - } - - I915_WRITE(mmio, (u32)ring->status_page.gfx_addr); - POSTING_READ(mmio); - - /* -* Flush the TLB for this page -* -* FIXME: These two bits have disappeared on gen8, so a question -* arises: do we still need this and if so how should we go about -* invalidating the TLB? -*/ - if (INTEL_INFO(dev)->gen >= 6 && INTEL_INFO(dev)->gen < 8) { - u32 reg = RIN
[Intel-gfx] [PATCH] drm/i915: Stop requesting error_state reports.
These error states are great to know gpu state when it hangs. But since we don't have automated tools to do analysis we are facing much noise on bugzilla with end users reporting just because "log asked to", while gpu reset worked and users probably never notice any screen issue. Most of these reportes don't know when it happened or how to retrigger the issue and somethimes they are not even on the mood to retest again. So, let's minimize our and end user's noise and protect this smaller message with drm.debug. Developers, OSVs and users that face real screen issue (should) always enabled this debug and will see the message when error state got dumped. Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_gpu_error.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 48ddbf4..77d63be 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1297,11 +1297,7 @@ void i915_capture_error_state(struct drm_device *dev, bool wedged, } if (!warned) { - DRM_INFO("GPU hangs can indicate a bug anywhere in the entire gfx stack, including userspace.\n"); - DRM_INFO("Please file a _new_ bug report on bugs.freedesktop.org against DRI -> DRM/Intel\n"); - DRM_INFO("drm/i915 developers can then reassign to the right component if it's not a kernel issue.\n"); - DRM_INFO("The gpu crash dump is required to analyze gpu hangs, so please always attach it.\n"); - DRM_INFO("GPU crash dump saved to /sys/class/drm/card%d/error\n", dev->primary->index); + DRM_DEBUG_DRIVER("GPU crash dump saved to /sys/class/drm/card%d/error\n", dev->primary->index); warned = true; } } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/15] drm/i915: Parse VBT PSR block.
On Fri, Nov 14, 2014 at 08:52:30AM -0800, Rodrigo Vivi wrote: > +struct psr_table { > + /* Feature bits */ > + u8 full_link:1; > + u8 require_aux_to_wakeup:1; > + u8 feature_bits_rsvd:6; > + > + /* Wait times */ > + u8 idle_frames:4; > + u8 lines_to_wait:3; > + u8 wait_times_rsvd:1; > + > + /* TP wake up time in multiple of 100 */ > + u16 tp1_wakeup_time; > + u16 tp2_tp3_wakeup_time; > +} __packed; ... > + /* Allowed VBT values goes from 0 to 15 */ > + dev_priv->vbt.psr.idle_frames = psr_table->idle_frames < 0 ? 0 : > + psr_table->idle_frames > 15 ? 15 : psr_table->idle_frames; smatch gives a warning here saying those conditions are always true. psr_table->idle_frames being 4 bits in a bitfield, we can see that smatch may well be right. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 11/17] tests: Add invalid pad tests for ctx create/destroy
We've missed them, and the kernel isn't nasty enough and forgot to check them. To add these tests convert the existing create/destroy tests over to subtests. v2: Do the basic create/destroy in ctx_bad_destroy in a fixture so that all the tests skip properly. Signed-off-by: Daniel Vetter --- tests/gem_ctx_bad_destroy.c | 48 +++-- tests/gem_ctx_create.c | 42 +++ 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/tests/gem_ctx_bad_destroy.c b/tests/gem_ctx_bad_destroy.c index 368bf95f2fcb..ee89763870ef 100644 --- a/tests/gem_ctx_bad_destroy.c +++ b/tests/gem_ctx_bad_destroy.c @@ -38,28 +38,46 @@ IGT_TEST_DESCRIPTION("Negative test cases for destroy contexts."); -igt_simple_main +uint32_t ctx_id; +int fd; + +igt_main { - uint32_t ctx_id; - int fd; + igt_fixture { + fd = drm_open_any_render(); + + ctx_id = gem_context_create(fd); + /* Make sure a proper destroy works first */ + gem_context_destroy(fd, ctx_id); + } - igt_skip_on_simulation(); + /* try double destroy */ + igt_subtest("double-destroy") { + ctx_id = gem_context_create(fd); + gem_context_destroy(fd, ctx_id); + igt_assert(__gem_context_destroy(fd, ctx_id) == -ENOENT); + } - fd = drm_open_any_render(); + igt_subtest("invalid-ctx") + igt_assert(__gem_context_destroy(fd, 2) == -ENOENT); - ctx_id = gem_context_create(fd); + igt_subtest("invalid-default-ctx") + igt_assert(__gem_context_destroy(fd, 0) == -ENOENT); - /* Make sure a proper destroy works first */ - gem_context_destroy(fd, ctx_id); + igt_subtest("invalid-pad") { + struct drm_i915_gem_context_destroy destroy; - /* try double destroy */ - igt_assert(__gem_context_destroy(fd, ctx_id) == -ENOENT); + ctx_id = gem_context_create(fd); - /* destroy something random */ - igt_assert(__gem_context_destroy(fd, 2) == -ENOENT); + memset(&destroy, 0, sizeof(destroy)); + destroy.ctx_id = ctx_id; + destroy.pad = 1; - /* Try to destroy the default context */ - igt_assert(__gem_context_destroy(fd, 0) == -ENOENT); + igt_assert(drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy) < 0 && + errno == EINVAL); + gem_context_destroy(fd, ctx_id); + } - close(fd); + igt_fixture + close(fd); } diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c index 1c710fdebbf7..046c974dfb6a 100644 --- a/tests/gem_ctx_create.c +++ b/tests/gem_ctx_create.c @@ -32,22 +32,40 @@ #include "ioctl_wrappers.h" #include "drmtest.h" -igt_simple_main +int ret, fd; +struct drm_i915_gem_context_create create; + +igt_main { - int ret, fd; - struct drm_i915_gem_context_create create; + igt_fixture + fd = drm_open_any_render(); + + igt_subtest("basic") { + create.ctx_id = rand(); + create.pad = 0; + + + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create); + igt_skip_on(ret != 0 && (errno == ENODEV || errno == EINVAL)); + igt_assert(ret == 0); + igt_assert(create.ctx_id != 0); + } - igt_skip_on_simulation(); + igt_subtest("invalid-pad") { + create.ctx_id = rand(); + create.pad = 0; - create.ctx_id = rand(); - create.pad = rand(); + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create); + igt_skip_on(ret != 0 && (errno == ENODEV || errno == EINVAL)); + igt_assert(ret == 0); + igt_assert(create.ctx_id != 0); - fd = drm_open_any_render(); + create.pad = 1; - ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create); - igt_skip_on(ret != 0 && (errno == ENODEV || errno == EINVAL)); - igt_assert(ret == 0); - igt_assert(create.ctx_id != 0); + igt_assert(drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create) < 0 && + errno == EINVAL); + } - close(fd); + igt_fixture + close(fd); } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 14/17] lib/igt_aux: s/swap/igt_swap/
It collides with the subtest naming convention glossary entry for swap. Which makes the docbook xml stuff unhappy. Signed-off-by: Daniel Vetter --- lib/igt.cocci | 4 ++-- lib/igt_aux.h | 2 +- tests/eviction_common.c | 2 +- tests/gem_ctx_thrash.c | 4 ++-- tests/gem_seqno_wrap.c | 2 +- tests/gem_stress.c | 2 +- tests/kms_flip.c| 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/igt.cocci b/lib/igt.cocci index fd4ad2564bea..41a8beb36029 100644 --- a/lib/igt.cocci +++ b/lib/igt.cocci @@ -92,7 +92,7 @@ expression E; - assert(E); + igt_assert(E); -// Replace open-coded swap() +// Replace open-coded igt_swap() @@ type T; T a, b, tmp; @@ -100,7 +100,7 @@ T a, b, tmp; - tmp = a; - a = b; - b = tmp; -+ swap(a, b); ++ igt_swap(a, b); // Replace open-coded min() @@ diff --git a/lib/igt_aux.h b/lib/igt_aux.h index 798a5b45fcb9..edc36a221922 100644 --- a/lib/igt_aux.h +++ b/lib/igt_aux.h @@ -94,7 +94,7 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode); #define min(a, b) ((a) < (b) ? (a) : (b)) #define max(a, b) ((a) > (b) ? (a) : (b)) -#define swap(a, b) do {\ +#define igt_swap(a, b) do {\ typeof(a) _tmp = (a); \ (a) = (b); \ (b) = _tmp; \ diff --git a/tests/eviction_common.c b/tests/eviction_common.c index 4a12dcbc345d..b18c2a73f9c7 100644 --- a/tests/eviction_common.c +++ b/tests/eviction_common.c @@ -55,7 +55,7 @@ static void exchange_uint32_t(void *array, unsigned i, unsigned j) { uint32_t *i_arr = array; - swap(i_arr[i], i_arr[j]); + igt_swap(i_arr[i], i_arr[j]); } static int minor_evictions(int fd, struct igt_eviction_test_ops *ops, diff --git a/tests/gem_ctx_thrash.c b/tests/gem_ctx_thrash.c index 5c272338e795..b4818f4e6337 100644 --- a/tests/gem_ctx_thrash.c +++ b/tests/gem_ctx_thrash.c @@ -55,13 +55,13 @@ static int ctx_per_thread; static void xchg_ptr(void *array, unsigned i, unsigned j) { void **A = array; - swap(A[i], A[j]); + igt_swap(A[i], A[j]); } static void xchg_int(void *array, unsigned i, unsigned j) { int *A = array; - swap(A[i], A[j]); + igt_swap(A[i], A[j]); } static int reopen(int _fd) diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c index 42493efdb20e..d07ec96064eb 100644 --- a/tests/gem_seqno_wrap.c +++ b/tests/gem_seqno_wrap.c @@ -172,7 +172,7 @@ static void exchange_uint(void *array, unsigned i, unsigned j) { unsigned *i_arr = array; - swap(i_arr[i], i_arr[j]); + igt_swap(i_arr[i], i_arr[j]); } static void run_sync_test(int num_buffers, bool verify) diff --git a/tests/gem_stress.c b/tests/gem_stress.c index 9f20bde2f45a..f687b2d1c795 100644 --- a/tests/gem_stress.c +++ b/tests/gem_stress.c @@ -571,7 +571,7 @@ static void exchange_uint(void *array, unsigned i, unsigned j) { unsigned *i_arr = array; - swap(i_arr[i], i_arr[j]); + igt_swap(i_arr[i], i_arr[j]); } static void copy_tiles(unsigned *permutation) diff --git a/tests/kms_flip.c b/tests/kms_flip.c index 292b7bc60f09..09dc3c794fdf 100644 --- a/tests/kms_flip.c +++ b/tests/kms_flip.c @@ -242,7 +242,7 @@ static int _emit_dummy_load__bcs(struct test_output *o, int limit, int timeout) 2048, 2048, 2048*4, 2048*4); - swap(src_bo, dst_bo); + igt_swap(src_bo, dst_bo); } blit_copy(fb_bo, src_bo, min(o->fb_width, 2048), min(o->fb_height, 2048), @@ -357,7 +357,7 @@ static int _emit_dummy_load__rcs(struct test_output *o, int limit, int timeout) 2048, 2048, dst, 0, 0); - swap(src, dst); + igt_swap(src, dst); } copyfunc(batch, NULL, src, 0, 0, -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 12/17] tests/gem_ppgtt: Start rcs before bcs for context tests
This way the igt_require for the ctx support is hit before we've launched a bazillion threads and need to wait until they're all done. Signed-off-by: Daniel Vetter --- tests/gem_ppgtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c index 0ee4f5b2a31e..850c99ea4b8a 100644 --- a/tests/gem_ppgtt.c +++ b/tests/gem_ppgtt.c @@ -220,8 +220,8 @@ int main(int argc, char **argv) igt_subtest("bcs-vs-rcs-ctxN") { dri_bo *bcs[1], *rcs[N_CHILD]; - fork_bcs_copy(0x4000, bcs, 1); fork_rcs_copy(0x8000 / N_CHILD, rcs, N_CHILD, CREATE_CONTEXT); + fork_bcs_copy(0x4000, bcs, 1); igt_waitchildren(); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 15/17] tests/gem_wait: Adjust makefile
I've forgotten to do this in commit e4753d2d96fbb88077e70820793137f45f02c9ba Author: Daniel Vetter Date: Mon Sep 29 14:42:33 2014 +0200 tests/gem_wait_render_timeout: Convert to subtests Signed-off-by: Daniel Vetter --- tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index ec2bd301fac1..5efc8d8c0e0d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -73,7 +73,7 @@ gen7_forcewake_mt_LDADD = $(LDADD) -lpthread gem_userptr_blits_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) gem_userptr_blits_LDADD = $(LDADD) -lpthread -gem_wait_render_timeout_LDADD = $(LDADD) -lrt +gem_wait_LDADD = $(LDADD) -lrt kms_flip_LDADD = $(LDADD) -lrt -lpthread prime_nv_test_CFLAGS = $(AM_CFLAGS) $(DRM_NOUVEAU_CFLAGS) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 08/17] tests/gem_reset_stat: Use new ctx helpers
A bit more invasive since getting rid off all the places meant to flatten some of the control flow with implicit igt_require. Signed-off-by: Daniel Vetter --- tests/gem_reset_stats.c | 117 +++- 1 file changed, 27 insertions(+), 90 deletions(-) diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c index 646d6dac052e..faa209c1d28a 100644 --- a/tests/gem_reset_stats.c +++ b/tests/gem_reset_stats.c @@ -53,7 +53,6 @@ #define RS_UNKNOWN (1 << 2) static uint32_t devid; -static bool hw_contexts; struct local_drm_i915_reset_stats { __u32 ctx_id; @@ -64,20 +63,8 @@ struct local_drm_i915_reset_stats { __u32 pad; }; -struct local_drm_i915_gem_context_create { - __u32 ctx_id; - __u32 pad; -}; - -struct local_drm_i915_gem_context_destroy { - __u32 ctx_id; - __u32 pad; -}; - #define MAX_FD 32 -#define CONTEXT_CREATE_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2d, struct local_drm_i915_gem_context_create) -#define CONTEXT_DESTROY_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2e, struct local_drm_i915_gem_context_destroy) #define GET_RESET_STATS_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x32, struct local_drm_i915_reset_stats) #define LOCAL_I915_EXEC_VEBOX (4 << 0) @@ -89,64 +76,32 @@ static bool gem_has_render(int fd) return true; } -static bool has_context(const struct target_ring *ring); - static const struct target_ring { uint32_t exec; bool (*present)(int fd); - bool (*contexts)(const struct target_ring *ring); const char *name; } rings[] = { - { I915_EXEC_RENDER, gem_has_render, has_context, "render" }, - { I915_EXEC_BLT, gem_has_blt, has_context, "blt" }, - { I915_EXEC_BSD, gem_has_bsd, has_context, "bsd" }, - { LOCAL_I915_EXEC_VEBOX, gem_has_vebox, has_context, "vebox" }, + { I915_EXEC_RENDER, gem_has_render, "render" }, + { I915_EXEC_BLT, gem_has_blt, "blt" }, + { I915_EXEC_BSD, gem_has_bsd, "bsd" }, + { LOCAL_I915_EXEC_VEBOX, gem_has_vebox, "vebox" }, }; -static bool has_context(const struct target_ring *ring) +static void check_context(const struct target_ring *ring) { - if (!hw_contexts) - return false; + int fd = drm_open_any(); - if(ring->exec == I915_EXEC_RENDER) - return true; + gem_context_destroy(fd, + gem_context_create(fd)); + close(fd); - return false; + igt_require(ring->exec == I915_EXEC_RENDER); } #define NUM_RINGS (sizeof(rings)/sizeof(struct target_ring)) static const struct target_ring *current_ring; -static uint32_t context_create(int fd) -{ - struct local_drm_i915_gem_context_create create; - int ret; - - create.ctx_id = rand(); - create.pad = rand(); - - ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create); - igt_assert(ret == 0); - - return create.ctx_id; -} - -static int context_destroy(int fd, uint32_t ctx_id) -{ - int ret; - struct local_drm_i915_gem_context_destroy destroy; - - destroy.ctx_id = ctx_id; - destroy.pad = rand(); - - ret = drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &destroy); - if (ret != 0) - return -errno; - - return 0; -} - static int gem_reset_stats(int fd, int ctx_id, struct local_drm_i915_reset_stats *rs) { @@ -450,7 +405,7 @@ static void test_rs_ctx(int num_fds, int num_ctx, int hang_index, assert_reset_status(fd[i], 0, RS_NO_ERROR); for (j = 0; j < num_ctx; j++) { - ctx[i][j] = context_create(fd[i]); + ctx[i][j] = gem_context_create(fd[i]); } @@ -502,7 +457,7 @@ static void test_rs_ctx(int num_fds, int num_ctx, int hang_index, for (i = 0; i < num_fds; i++) { for (j = 0; j < num_ctx; j++) { gem_close(fd[i], h[i][j]); - igt_assert(context_destroy(fd[i], ctx[i][j]) == 0); + gem_context_destroy(fd[i], ctx[i][j]); } assert_reset_status(fd[i], 0, RS_NO_ERROR); @@ -610,8 +565,8 @@ static void test_ban_ctx(void) assert_reset_status(fd, 0, RS_NO_ERROR); - ctx_good = context_create(fd); - ctx_bad = context_create(fd); + ctx_good = gem_context_create(fd); + ctx_bad = gem_context_create(fd); assert_reset_status(fd, 0, RS_NO_ERROR); assert_reset_status(fd, ctx_good, RS_NO_ERROR); @@ -681,8 +636,8 @@ static void test_ban_ctx(void) igt_assert(h1 >= 0); gem_close(fd, h1); - igt_assert(context_destroy(fd, ctx_good) == 0); - igt_assert(context_destroy(fd, ctx_bad) == 0); + gem_context_destroy(fd, ctx_good); + gem_context_destroy(fd, ctx_bad); igt_assert(gem_reset_status(fd, ctx_good) < 0); igt_assert(gem_reset_status(fd, ctx_bad) < 0);
[Intel-gfx] [PATCH i-g-t 16/17] doc: Consolidate naming conventions into docbook
Duplication just means it gets out of sync. Also update they keyword list in the Makefile, not everything was listed. And add a new "invalid" keyword. While at it update NEWS. Signed-off-by: Daniel Vetter --- CONTRIBUTING | 2 +- NEWS | 12 docs/reference/intel-gpu-tools/Makefile.am | 2 +- .../intel-gpu-tools/igt_test_programs.xml | 7 +++ tests/NAMING-CONVENTION| 71 -- 5 files changed, 21 insertions(+), 73 deletions(-) delete mode 100644 tests/NAMING-CONVENTION diff --git a/CONTRIBUTING b/CONTRIBUTING index 7c20fdb734ce..e2c352e78fa0 100644 --- a/CONTRIBUTING +++ b/CONTRIBUTING @@ -22,7 +22,7 @@ A short list of contribution guidelines: developer's certificate of origin: http://developercertificate.org/ - When submitting new testcases please follow the naming conventions documented - in tests/NAMING-CONVENTION. Also please make full use of all the helpers and + in the generated documentation. Also please make full use of all the helpers and convenience macros provided by the igt library. The semantic patch lib/igt.cocci can help with the more automatic conversions. diff --git a/NEWS b/NEWS index 67146a3c50af..becf1b246852 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,18 @@ Release 1.10 (-XX-XX) - New frequency manipulation tool (intel_gpu_frequency) +- Adjustments for the Solaris port (Alan Coopersmith). + +- Remove tests/NAMING-CONVENTION since it's all in the docbook now, to avoid + divergent conventions. + +- New CRITICAL log level for really serious stuff (Thomas Wood). + +- Interactive test mode can now be enabled by the shared cmdline option + --interactive-debug=$var (Rodrigo Vivi). + +- Piles of new testcases and improvements to existing ones as usual. + Release 1.9 (2014-12-12) diff --git a/docs/reference/intel-gpu-tools/Makefile.am b/docs/reference/intel-gpu-tools/Makefile.am index 0a47764d8696..0f10eaab4d6a 100644 --- a/docs/reference/intel-gpu-tools/Makefile.am +++ b/docs/reference/intel-gpu-tools/Makefile.am @@ -1,7 +1,7 @@ ## Process this file with automake to produce Makefile.in TESTLISTS = $(top_builddir)/tests/single-tests.txt $(top_builddir)/tests/multi-tests.txt -KEYWORDS = (hang|swap|thrash|crc|tiled|tiling|rte|ctx|exec|rpm) +KEYWORDS = (invalid|hang|swap|thrash|crc|tiled|tiling|rte|ctx|render|blt|bsd|vebox|exec|rpm) xml/igt_test_programs_%_programs.xml: $(TESTLISTS) mkdir -p `dirname $@` diff --git a/docs/reference/intel-gpu-tools/igt_test_programs.xml b/docs/reference/intel-gpu-tools/igt_test_programs.xml index 36ad1f358e04..bf8a939d6665 100644 --- a/docs/reference/intel-gpu-tools/igt_test_programs.xml +++ b/docs/reference/intel-gpu-tools/igt_test_programs.xml @@ -197,6 +197,13 @@ various features of the test and can be used to filter and select particular tests. + + invalid + +Negative tests to validate kernel interface input validation. + + + hang diff --git a/tests/NAMING-CONVENTION b/tests/NAMING-CONVENTION deleted file mode 100644 index 7c27fdbb16c2.. --- a/tests/NAMING-CONVENTION +++ /dev/null @@ -1,71 +0,0 @@ -Naming Convention of i-g-t Tests and Subtests -= - -To facilitate easy test selection with piglit we need a somewhat consistent -naming scheme for tests and subtests. - -Test Prefixes -- - -core_: Test for core drm ioctls and behaviour. - -kms_: Used for modesetting tests. - -drm_: Tests for libdrm behaviour, currently just testing the buffer cache -reaping. - -gem_: Used for all kinds of GEM tests. - -prime_: Used for buffer sharing tests, both for self-importing (used by -dri3/wayland) and actual multi-gpu tests. - -drv_: Tests for overall driver behaviour like module reload, s/r, debugfs files. - -pm_: Tests for power management features like runtime PM, tuning knobs in sysfs -and also performance tuning features. - -gen3_: Used by Chris' gen3 specific tiling/fencing tests. Generally tests that -only run on some platforms don't have a specific prefix but just skip on -platforms where the test doesn't apply. - -debugfs_/sysfs_: Mostly for tests that use sysfs/debugfs but tend to tests all -sorts of things. Please consider using a more appropriate prefix from above if -the main point isn't to test sysfs/debugfs, but a driver subsystem/feature. - -igt_: Testcase which test the i-g-t infrastructure itself and which are all run -through "make check" while building i-g-t. - -(Sub-)Test patterns - -Much more powerful for filtering sets of tests are patterns anywhere in either -the test or subtest name. - -hang: Tests that provoke gpu hangs - -swap: Tests that force their full working sets through swap. Dreadfully slow on -machines with spinning rust and tons of memory. - -thrash: Tests
[Intel-gfx] [PATCH i-g-t 13/17] tests: Align subtest with naming convention
Yeah, historically grown but we should try to be somewhat consistent. It helps with filtering testcases. Signed-off-by: Daniel Vetter --- tests/gem_concurrent_blit.c | 4 ++-- tests/gem_ppgtt.c | 4 ++-- tests/kms_flip.c| 16 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/gem_concurrent_blit.c b/tests/gem_concurrent_blit.c index cec6ea49f95c..0b0adce7892c 100644 --- a/tests/gem_concurrent_blit.c +++ b/tests/gem_concurrent_blit.c @@ -725,8 +725,8 @@ run_basic_modes(const struct access_mode *mode, { "cpu", cpu_copy_bo, cpu_require }, { "gtt", gtt_copy_bo, gtt_require }, { "wc", wc_copy_bo, wc_require }, - { "bcs", blt_copy_bo, bcs_require }, - { "rcs", render_copy_bo, rcs_require }, + { "blt", blt_copy_bo, bcs_require }, + { "render", render_copy_bo, rcs_require }, { NULL, NULL } }, *p; const struct { diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c index 850c99ea4b8a..5bf773c25270 100644 --- a/tests/gem_ppgtt.c +++ b/tests/gem_ppgtt.c @@ -205,7 +205,7 @@ int main(int argc, char **argv) { igt_subtest_init(argc, argv); - igt_subtest("bcs-vs-rcs-ctx0") { + igt_subtest("blt-vs-render-ctx0") { dri_bo *bcs[1], *rcs[N_CHILD]; fork_bcs_copy(0x4000, bcs, 1); @@ -217,7 +217,7 @@ int main(int argc, char **argv) surfaces_check(rcs, N_CHILD, 0x8000 / N_CHILD); } - igt_subtest("bcs-vs-rcs-ctxN") { + igt_subtest("blt-vs-render-ctxN") { dri_bo *bcs[1], *rcs[N_CHILD]; fork_rcs_copy(0x8000 / N_CHILD, rcs, N_CHILD, CREATE_CONTEXT); diff --git a/tests/kms_flip.c b/tests/kms_flip.c index 557bcd4ad3f6..292b7bc60f09 100644 --- a/tests/kms_flip.c +++ b/tests/kms_flip.c @@ -1640,12 +1640,12 @@ int main(int argc, char **argv) "blocking-absolute-wf_vblank" }, { 60, TEST_VBLANK | TEST_DPMS | TEST_EINVAL, "wf_vblank-vs-dpms" }, { 60, TEST_VBLANK | TEST_DPMS | TEST_WITH_DUMMY_BCS, - "bcs-wf_vblank-vs-dpms" }, + "blt-wf_vblank-vs-dpms" }, { 60, TEST_VBLANK | TEST_DPMS | TEST_WITH_DUMMY_RCS, "rcs-wf_vblank-vs-dpms" }, { 60, TEST_VBLANK | TEST_MODESET | TEST_EINVAL, "wf_vblank-vs-modeset" }, { 60, TEST_VBLANK | TEST_MODESET | TEST_WITH_DUMMY_BCS, - "bcs-wf_vblank-vs-modeset" }, + "blt-wf_vblank-vs-modeset" }, { 60, TEST_VBLANK | TEST_MODESET | TEST_WITH_DUMMY_RCS, "rcs-wf_vblank-vs-modeset" }, @@ -1657,14 +1657,14 @@ int main(int argc, char **argv) "plain-flip-fb-recreate" }, { 30, TEST_FLIP | TEST_RMFB | TEST_MODESET , "flip-vs-rmfb" }, { 60, TEST_FLIP | TEST_DPMS | TEST_EINVAL, "flip-vs-dpms" }, - { 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_BCS, "bcs-flip-vs-dpms" }, - { 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_RCS, "rcs-flip-vs-dpms" }, + { 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_BCS, "blt-flip-vs-dpms" }, + { 60, TEST_FLIP | TEST_DPMS | TEST_WITH_DUMMY_RCS, "render-flip-vs-dpms" }, { 30, TEST_FLIP | TEST_PAN, "flip-vs-panning" }, - { 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_BCS, "bcs-flip-vs-panning" }, - { 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_RCS, "rcs-flip-vs-panning" }, + { 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_BCS, "blt-flip-vs-panning" }, + { 60, TEST_FLIP | TEST_PAN | TEST_WITH_DUMMY_RCS, "render-flip-vs-panning" }, { 60, TEST_FLIP | TEST_MODESET | TEST_EINVAL, "flip-vs-modeset" }, - { 60, TEST_FLIP | TEST_MODESET | TEST_WITH_DUMMY_BCS, "bcs-flip-vs-modeset" }, - { 60, TEST_FLIP | TEST_MODESET | TEST_WITH_DUMMY_RCS, "rcs-flip-vs-modeset" }, + { 60, TEST_FLIP | TEST_MODESET | TEST_WITH_DUMMY_BCS, "blt-flip-vs-modeset" }, + { 60, TEST_FLIP | TEST_MODESET | TEST_WITH_DUMMY_RCS, "render-flip-vs-modeset" }, { 30, TEST_FLIP | TEST_VBLANK_EXPIRED_SEQ, "flip-vs-expired-vblank" }, -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 17/17] lib/igt_gt: Document and consolidate
Also move forcewake and stop_rings code from igt_debugfs to igt_gt since it fits better. And move the hang injection fork helpers from igt_aux to igt_gt, too. Also push the intel_gen call into igt_hang_ring while at it. Signed-off-by: Daniel Vetter --- .../intel-gpu-tools/intel-gpu-tools-docs.xml | 1 + lib/drmtest.c | 1 + lib/igt_aux.c | 68 -- lib/igt_aux.h | 3 - lib/igt_debugfs.c | 122 --- lib/igt_debugfs.h | 35 --- lib/igt_gt.c | 240 - lib/igt_gt.h | 46 +++- lib/intel_mmio.c | 2 +- tests/drv_hangman.c| 2 +- tests/drv_suspend.c| 2 +- tests/gem_concurrent_blit.c| 4 +- tests/gem_ctx_exec.c | 2 +- tests/gem_evict_alignment.c| 1 + tests/gem_evict_everything.c | 1 + tests/gem_pread_after_blit.c | 2 +- tests/gem_reloc_vs_gpu.c | 2 +- tests/gem_reset_stats.c| 2 +- tests/gem_workarounds.c| 2 +- tests/kms_flip.c | 2 +- tests/kms_pipe_crc_basic.c | 2 +- tests/pm_rpm.c | 2 +- tests/pm_rps.c | 2 +- 23 files changed, 300 insertions(+), 246 deletions(-) diff --git a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml index 9cfb836bb8bd..6c953fd6841f 100644 --- a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml +++ b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml @@ -21,6 +21,7 @@ + diff --git a/lib/drmtest.c b/lib/drmtest.c index 7cdef36655d0..1d6e882c0fea 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -51,6 +51,7 @@ #include "i915_drm.h" #include "intel_chipset.h" #include "intel_io.h" +#include "igt_gt.h" #include "igt_debugfs.h" #include "version.h" #include "config.h" diff --git a/lib/igt_aux.c b/lib/igt_aux.c index aefa0863e9e9..b31f0cdb71fc 100644 --- a/lib/igt_aux.c +++ b/lib/igt_aux.c @@ -132,74 +132,6 @@ void igt_stop_signal_helper(void) sig_stat = 0; } -/* GPU abusers */ -static struct igt_helper_process hang_helper; -static void __attribute__((noreturn)) -hang_helper_process(pid_t pid, int fd, int gen) -{ - while (1) { - if (kill(pid, 0)) /* Parent has died, so must we. */ - exit(0); - - igt_post_hang_ring(fd, - igt_hang_ring(fd, gen, I915_EXEC_DEFAULT)); - - sleep(1); - } -} - -/** - * igt_fork_hang_helper: - * - * Fork a child process using #igt_fork_helper to hang the default engine - * of the GPU at regular intervals. - * - * This is useful to exercise slow running code (such as aperture placement) - * which needs to be robust against a GPU reset. - * - * In tests with subtests this function can be called outside of failure - * catching code blocks like #igt_fixture or #igt_subtest. - */ -int igt_fork_hang_helper(void) -{ - int fd, gen; - - if (igt_only_list_subtests()) - return 1; - - fd = drm_open_any(); - if (fd == -1) - return 0; - - gen = intel_gen(intel_get_drm_devid(fd)); - if (gen < 5) { - close(fd); - return 0; - } - - igt_fork_helper(&hang_helper) - hang_helper_process(getppid(), fd, gen); - - close(fd); - return 1; -} - -/** - * igt_stop_hang_helper: - * - * Stops the child process spawned with igt_fork_hang_helper(). - * - * In tests with subtests this function can be called outside of failure - * catching code blocks like #igt_fixture or #igt_subtest. - */ -void igt_stop_hang_helper(void) -{ - if (igt_only_list_subtests()) - return; - - igt_stop_helper(&hang_helper); -} - /** * igt_check_boolean_env_var: * @env_var: environment variable name diff --git a/lib/igt_aux.h b/lib/igt_aux.h index edc36a221922..7f42b337f2ec 100644 --- a/lib/igt_aux.h +++ b/lib/igt_aux.h @@ -39,9 +39,6 @@ extern int num_trash_bos; void igt_fork_signal_helper(void); void igt_stop_signal_helper(void); -int igt_fork_hang_helper(void); -void igt_stop_hang_helper(void); - void igt_exchange_int(void *array, unsigned i, unsigned j); void igt_permute_array(void *array, unsigned size, void (*exchange_func)(void *array, diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index b44333e840da..a2cec45a1460 100644 --- a/lib
[Intel-gfx] [PATCH i-g-t 09/17] lib/ioctl: Document ctx param functions
And move them so that they're grouped with the other context wrappers. Signed-off-by: Daniel Vetter --- lib/ioctl_wrappers.c | 79 +++- lib/ioctl_wrappers.h | 22 +++ 2 files changed, 63 insertions(+), 38 deletions(-) diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c index e86b3c2bfa46..288ab8d7e0e2 100644 --- a/lib/ioctl_wrappers.c +++ b/lib/ioctl_wrappers.c @@ -663,6 +663,59 @@ void gem_context_destroy(int fd, uint32_t ctx_id) } /** + * gem_context_get_param: + * @fd: open i915 drm file descriptor + * @p: i915 hw context parameter + * + * This is a wraps the CONTEXT_GET_PARAM ioctl, which is used to free a hardware + * context. Not that similarly to gem_set_caching() this wrapper calls + * igt_require() internally to correctly skip on kernels and platforms where hw + * context parameter support is not available. + */ +void gem_context_get_param(int fd, struct local_i915_gem_context_param *p) +{ +#define LOCAL_I915_GEM_CONTEXT_GETPARAM 0x34 +#define LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_CONTEXT_GETPARAM, struct local_i915_gem_context_param) + do_ioctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, p); +} + +/** + * gem_context_set_param: + * @fd: open i915 drm file descriptor + * @p: i915 hw context parameter + * + * This is a wraps the CONTEXT_SET_PARAM ioctl, which is used to free a hardware + * context. Not that similarly to gem_set_caching() this wrapper calls + * igt_require() internally to correctly skip on kernels and platforms where hw + * context parameter support is not available. + */ +void gem_context_set_param(int fd, struct local_i915_gem_context_param *p) +{ +#define LOCAL_I915_GEM_CONTEXT_SETPARAM 0x35 +#define LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_CONTEXT_SETPARAM, struct local_i915_gem_context_param) + do_ioctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, p); +} + +/** + * gem_require_caching: + * @fd: open i915 drm file descriptor + * + * Feature test macro to query whether hw context parameter support for @param + * is available. Automatically skips through igt_require() if not. + */ +void gem_context_require_param(int fd, uint64_t param) +{ + struct local_i915_gem_context_param p; + + p.context = 0; + p.param = param; + p.value = 0; + p.size = 0; + + igt_require(drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, &p) == 0); +} + +/** * gem_sw_finish: * @fd: open i915 drm file descriptor * @handle: gem buffer object handle @@ -1089,29 +1142,3 @@ off_t prime_get_size(int dma_buf_fd) return ret; } - -void gem_context_get_param(int fd, struct local_i915_gem_context_param *p) -{ -#define LOCAL_I915_GEM_CONTEXT_GETPARAM 0x34 -#define LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_CONTEXT_GETPARAM, struct local_i915_gem_context_param) - do_ioctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, p); -} - -void gem_context_set_param(int fd, struct local_i915_gem_context_param *p) -{ -#define LOCAL_I915_GEM_CONTEXT_SETPARAM 0x35 -#define LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_CONTEXT_SETPARAM, struct local_i915_gem_context_param) - do_ioctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, p); -} - -void gem_context_require_param(int fd, uint64_t param) -{ - struct local_i915_gem_context_param p; - - p.context = 0; - p.param = param; - p.value = 0; - p.size = 0; - - igt_require(drmIoctl(fd, LOCAL_I915_GEM_CONTEXT_GETPARAM, &p) == 0); -} diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index ad10bd03d370..ea9f5598e7e3 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -97,6 +97,16 @@ int gem_madvise(int fd, uint32_t handle, int state); uint32_t gem_context_create(int fd); void gem_context_destroy(int fd, uint32_t ctx_id); int __gem_context_destroy(int fd, uint32_t ctx_id); +struct local_i915_gem_context_param { + uint32_t context; + uint32_t size; + uint64_t param; +#define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1 + uint64_t value; +}; +void gem_context_require_param(int fd, uint64_t param); +void gem_context_get_param(int fd, struct local_i915_gem_context_param *p); +void gem_context_set_param(int fd, struct local_i915_gem_context_param *p); void gem_sw_finish(int fd, uint32_t handle); @@ -125,16 +135,4 @@ int prime_handle_to_fd(int fd, uint32_t handle); uint32_t prime_fd_to_handle(int fd, int dma_buf_fd); off_t prime_get_size(int dma_buf_fd); -struct local_i915_gem_context_param { - uint32_t context; - uint32_t size; - uint64_t param; -#define LOCAL_CONTEXT_PARAM_BAN_PERIOD 0x1 - uint64_t value; -}; - -void gem_context_require_param(int fd, uint64_t param); -void gem_context_get_param(int fd, struct local_i915_gem_context_param *p); -void gem_context_set_param(i
[Intel-gfx] [PATCH i-g-t 07/17] tests/gem_ctx_*: Use helpers
Signed-off-by: Daniel Vetter --- tests/gem_ctx_bad_destroy.c | 25 + tests/gem_ctx_create.c | 11 ++- tests/gem_ctx_exec.c| 23 --- 3 files changed, 11 insertions(+), 48 deletions(-) diff --git a/tests/gem_ctx_bad_destroy.c b/tests/gem_ctx_bad_destroy.c index d848265c7802..368bf95f2fcb 100644 --- a/tests/gem_ctx_bad_destroy.c +++ b/tests/gem_ctx_bad_destroy.c @@ -38,18 +38,10 @@ IGT_TEST_DESCRIPTION("Negative test cases for destroy contexts."); -struct local_drm_i915_context_destroy { - __u32 ctx_id; - __u32 pad; -}; - -#define CONTEXT_DESTROY_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2e, struct local_drm_i915_context_destroy) - igt_simple_main { - struct local_drm_i915_context_destroy destroy; uint32_t ctx_id; - int ret, fd; + int fd; igt_skip_on_simulation(); @@ -57,24 +49,17 @@ igt_simple_main ctx_id = gem_context_create(fd); - destroy.ctx_id = ctx_id; /* Make sure a proper destroy works first */ - ret = drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &destroy); - igt_assert(ret == 0); + gem_context_destroy(fd, ctx_id); /* try double destroy */ - ret = drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &destroy); - igt_assert(ret != 0 && errno == ENOENT); + igt_assert(__gem_context_destroy(fd, ctx_id) == -ENOENT); /* destroy something random */ - destroy.ctx_id = 2; - ret = drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &destroy); - igt_assert(ret != 0 && errno == ENOENT); + igt_assert(__gem_context_destroy(fd, 2) == -ENOENT); /* Try to destroy the default context */ - destroy.ctx_id = 0; - ret = drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &destroy); - igt_assert(ret != 0 && errno == ENOENT); + igt_assert(__gem_context_destroy(fd, 0) == -ENOENT); close(fd); } diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c index 522e7b1ca022..1c710fdebbf7 100644 --- a/tests/gem_ctx_create.c +++ b/tests/gem_ctx_create.c @@ -32,17 +32,10 @@ #include "ioctl_wrappers.h" #include "drmtest.h" -struct local_drm_i915_gem_context_create { - __u32 ctx_id; - __u32 pad; -}; - -#define CONTEXT_CREATE_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2d, struct local_drm_i915_gem_context_create) - igt_simple_main { int ret, fd; - struct local_drm_i915_gem_context_create create; + struct drm_i915_gem_context_create create; igt_skip_on_simulation(); @@ -51,7 +44,7 @@ igt_simple_main fd = drm_open_any_render(); - ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create); + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create); igt_skip_on(ret != 0 && (errno == ENODEV || errno == EINVAL)); igt_assert(ret == 0); igt_assert(create.ctx_id != 0); diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c index 7a6ed9b1f1d2..ca5bf640adaf 100644 --- a/tests/gem_ctx_exec.c +++ b/tests/gem_ctx_exec.c @@ -50,21 +50,6 @@ IGT_TEST_DESCRIPTION("Test basic context switch functionality."); -struct local_drm_i915_gem_context_destroy { - __u32 ctx_id; - __u32 pad; -}; - -#define CONTEXT_DESTROY_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2e, struct local_drm_i915_gem_context_destroy) - -static void context_destroy(int fd, uint32_t ctx_id) -{ - struct local_drm_i915_gem_context_destroy destroy; - destroy.ctx_id = ctx_id; - do_ioctl(fd, CONTEXT_DESTROY_IOCTL, &destroy); -#include "igt_aux.h" -} - /* Copied from gem_exec_nop.c */ static int exec(int fd, uint32_t handle, int ring, int ctx_id) { @@ -183,7 +168,7 @@ igt_main /* check that we can create contexts. */ ctx_id = gem_context_create(fd); - context_destroy(fd, ctx_id); + gem_context_destroy(fd, ctx_id); gem_write(fd, handle, 0, batch, sizeof(batch)); } @@ -191,12 +176,12 @@ igt_main ctx_id = gem_context_create(fd); igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0); gem_sync(fd, handle); - context_destroy(fd, ctx_id); + gem_context_destroy(fd, ctx_id); ctx_id = gem_context_create(fd); igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) == 0); gem_sync(fd, handle); - context_destroy(fd, ctx_id); + gem_context_destroy(fd, ctx_id); igt_assert(exec(fd, handle, I915_EXEC_RENDER, ctx_id) < 0); gem_sync(fd, handle); @@ -227,6 +212,6 @@ igt_main gem_sync(fd, handle); } - context_destroy(fd, ctx_id); + gem_context_destroy(fd, ctx_id); } } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gf
[Intel-gfx] [PATCH i-g-t 05/17] lib/ioctls: make gem_context_set/get_param infallible
We have separate require checks already, so these failing is a bug in the test logic. Signed-off-by: Daniel Vetter --- lib/igt_gt.c | 2 +- lib/ioctl_wrappers.c | 18 +- lib/ioctl_wrappers.h | 4 ++-- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/igt_gt.c b/lib/igt_gt.c index 615f9893b876..e02219acc6fd 100644 --- a/lib/igt_gt.c +++ b/lib/igt_gt.c @@ -55,7 +55,7 @@ struct igt_hang_ring igt_hang_ring(int fd, int gen, int ring) ban = param.value; param.value = 0; - igt_require(gem_context_set_param(fd, ¶m) == 0); + gem_context_set_param(fd, ¶m); memset(&reloc, 0, sizeof(reloc)); memset(&exec, 0, sizeof(exec)); diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c index dd89e2c57e7f..c8c0e1c16f8b 100644 --- a/lib/ioctl_wrappers.c +++ b/lib/ioctl_wrappers.c @@ -1058,26 +1058,18 @@ off_t prime_get_size(int dma_buf_fd) return ret; } -int gem_context_get_param(int fd, struct local_i915_gem_context_param *p) +void gem_context_get_param(int fd, struct local_i915_gem_context_param *p) { #define LOCAL_I915_GEM_CONTEXT_GETPARAM 0x34 #define LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_CONTEXT_GETPARAM, struct local_i915_gem_context_param) - if (drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, p)) - return -1; - - errno = 0; - return 0; + do_ioctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM, p); } -int gem_context_set_param(int fd, struct local_i915_gem_context_param *p) +void gem_context_set_param(int fd, struct local_i915_gem_context_param *p) { #define LOCAL_I915_GEM_CONTEXT_SETPARAM 0x35 #define LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + LOCAL_I915_GEM_CONTEXT_SETPARAM, struct local_i915_gem_context_param) - if (drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, p)) - return -1; - - errno = 0; - return 0; + do_ioctl(fd, LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, p); } void gem_context_require_param(int fd, uint64_t param) @@ -1089,5 +1081,5 @@ void gem_context_require_param(int fd, uint64_t param) p.value = 0; p.size = 0; - igt_require(gem_context_get_param(fd, &p) == 0); + igt_require(drmIoctl(fd, LOCAL_I915_GEM_CONTEXT_GETPARAM, &p) == 0); } diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index 663b3da2ecd4..23b8c9d56e44 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -132,7 +132,7 @@ struct local_i915_gem_context_param { }; void gem_context_require_param(int fd, uint64_t param); -int gem_context_get_param(int fd, struct local_i915_gem_context_param *p); -int gem_context_set_param(int fd, struct local_i915_gem_context_param *p); +void gem_context_get_param(int fd, struct local_i915_gem_context_param *p); +void gem_context_set_param(int fd, struct local_i915_gem_context_param *p); #endif /* IOCTL_WRAPPERS_H */ -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 03/17] lib/ioctl: gem_ prefix for igt_require_mmap_wc
We stick to the overall prefix even for magic require functions. Signed-off-by: Daniel Vetter --- lib/ioctl_wrappers.h| 11 ++- tests/gem_concurrent_blit.c | 4 ++-- tests/gem_fence_upload.c| 2 +- tests/gem_mmap_wc.c | 16 tests/gem_tiled_wc.c| 2 +- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index a1017ecd3767..8d8fa46d1942 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -65,7 +65,16 @@ void *gem_mmap__cpu(int fd, uint32_t handle, int offset, int size, int prot); bool gem_mmap__has_wc(int fd); void *gem_mmap__wc(int fd, uint32_t handle, int offset, int size, int prot); -#define igt_require_mmap_wc(x) igt_require(gem_mmap__has_wc(x)) + +/** + * gem_require_mmap_wc: + * @fd: open i915 drm file descriptor + * + * Feature test macro to query whether direct (i.e. cpu access path, bypassing + * the gtt) write-combine memory mappings are available. Automatically skips + * through igt_require() if not. + */ +#define gem_require_mmap_wc(x) igt_require(gem_mmap__has_wc(x)) /** * gem_mmap: diff --git a/tests/gem_concurrent_blit.c b/tests/gem_concurrent_blit.c index 245ad4581783..cec6ea49f95c 100644 --- a/tests/gem_concurrent_blit.c +++ b/tests/gem_concurrent_blit.c @@ -174,7 +174,7 @@ wc_create_bo(drm_intel_bufmgr *bufmgr, int width, int height) { drm_intel_bo *bo; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); bo = unmapped_create_bo(bufmgr, width, height); bo->virtual = gem_mmap__wc(fd, bo->handle, 0, bo->size, PROT_READ | PROT_WRITE); @@ -696,7 +696,7 @@ static void gtt_require(void) static void wc_require(void) { bit17_require(); - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); } static void bcs_require(void) diff --git a/tests/gem_fence_upload.c b/tests/gem_fence_upload.c index 81f797b4ba86..9595bc822e59 100644 --- a/tests/gem_fence_upload.c +++ b/tests/gem_fence_upload.c @@ -345,7 +345,7 @@ static void wc_contention(void) double linear[2], tiled[2]; fd = drm_open_any(); - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); num_fences = gem_available_fences(fd); igt_require(num_fences > 0); diff --git a/tests/gem_mmap_wc.c b/tests/gem_mmap_wc.c index 87916b6e81c0..73a97d556927 100644 --- a/tests/gem_mmap_wc.c +++ b/tests/gem_mmap_wc.c @@ -130,7 +130,7 @@ test_copy(int fd) { void *src, *dst; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); /* copy from a fresh src to fresh dst to force pagefault on both */ src = create_pointer(fd); @@ -180,7 +180,7 @@ test_read_write2(int fd, enum test_read_write order) void *r, *w; volatile uint32_t val = 0; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); handle = gem_create(fd, OBJECT_SIZE); set_domain(fd, handle); @@ -210,7 +210,7 @@ test_write(int fd) void *src; uint32_t dst; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); /* copy from a fresh src to fresh dst to force pagefault on both */ src = create_pointer(fd); @@ -229,7 +229,7 @@ test_write_gtt(int fd) char *dst_gtt; void *src; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); dst = gem_create(fd, OBJECT_SIZE); set_domain(fd, dst); @@ -253,7 +253,7 @@ test_read(int fd) void *dst; uint32_t src; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); /* copy from a fresh src to fresh dst to force pagefault on both */ dst = create_pointer(fd); @@ -271,7 +271,7 @@ test_write_cpu_read_wc(int fd) uint32_t handle; uint32_t *src, *dst; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); handle = gem_create(fd, OBJECT_SIZE); @@ -296,7 +296,7 @@ test_write_gtt_read_wc(int fd) uint32_t handle; uint32_t *src, *dst; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); handle = gem_create(fd, OBJECT_SIZE); set_domain(fd, handle); @@ -390,7 +390,7 @@ test_fault_concurrent(int fd) struct thread_fault_concurrent thread[64]; int n; - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); for (n = 0; n < 32; n++) { ptr[n] = create_pointer(fd); diff --git a/tests/gem_tiled_wc.c b/tests/gem_tiled_wc.c index f705378173d7..b0f7a655a17a 100644 --- a/tests/gem_tiled_wc.c +++ b/tests/gem_tiled_wc.c @@ -136,7 +136,7 @@ igt_simple_main uint32_t handle; fd = drm_open_any(); - igt_require_mmap_wc(fd); + gem_require_mmap_wc(fd); handle = create_bo(fd); get_tiling(fd, handle, &tiling, &swizzle); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/ma
[Intel-gfx] [PATCH i-g-t 04/17] igt/ioctls: doc for gem_mmap
Just spotted while driving around. gtkdoc needs the full parameter list otherwise it doesn't recognize it as a function. So add them. Signed-off-by: Daniel Vetter --- lib/ioctl_wrappers.h | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index 8d8fa46d1942..663b3da2ecd4 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -78,10 +78,19 @@ void *gem_mmap__wc(int fd, uint32_t handle, int offset, int size, int prot); /** * gem_mmap: + * @fd: open i915 drm file descriptor + * @handle: gem buffer object handle + * @size: size of the gem buffer + * @prot: memory protection bits as used by mmap() + * + * This functions wraps up procedure to establish a memory mapping through the + * GTT. * * This is a simple convenience alias to gem_mmap__gtt() + * + * Returns: A pointer to the created memory mapping. */ -#define gem_mmap gem_mmap__gtt +#define gem_mmap(fd, handle, size, prot) gem_mmap__gtt(fd, handle, size, prot) int gem_madvise(int fd, uint32_t handle, int state); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 10/17] tests: Add gem_ctx_param_basic
Boring ioctl validation. Luckily no gaps found while doing it. v2: git add ftw! v3: Fixes: - args->size is an outparam for get, adjust test. - Pick an invalid param, not an invalid ioctl number ... tsk. Signed-off-by: Daniel Vetter --- tests/.gitignore| 1 + tests/Makefile.sources | 1 + tests/gem_ctx_exec.c| 3 +- tests/gem_ctx_param_basic. | 172 tests/gem_ctx_param_basic.c | 137 +++ 5 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 tests/gem_ctx_param_basic. create mode 100644 tests/gem_ctx_param_basic.c diff --git a/tests/.gitignore b/tests/.gitignore index 88a6405394b2..7b4dd94722a2 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -30,6 +30,7 @@ gem_ctx_basic gem_ctx_create gem_ctx_exec gem_ctx_thrash +gem_ctx_param_basic gem_double_irq_loop gem_dummy_reloc_loop gem_evict_alignment diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 74deec3127fb..51e8376b24e6 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -23,6 +23,7 @@ TESTS_progs_M = \ gem_close_race \ gem_concurrent_blit \ gem_cs_tlb \ + gem_ctx_param_basic \ gem_ctx_bad_exec \ gem_ctx_exec \ gem_dummy_reloc_loop \ diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c index ca5bf640adaf..ead3d463003a 100644 --- a/tests/gem_ctx_exec.c +++ b/tests/gem_ctx_exec.c @@ -161,7 +161,8 @@ int fd; igt_main { igt_skip_on_simulation(); - igt_fixture { + + igt_fixture { fd = drm_open_any_render(); handle = gem_create(fd, 4096); diff --git a/tests/gem_ctx_param_basic. b/tests/gem_ctx_param_basic. new file mode 100644 index ..2d866b3eee26 --- /dev/null +++ b/tests/gem_ctx_param_basic. @@ -0,0 +1,172 @@ +/* + * Copyright © 2011 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + *Ben Widawsky + * + */ + +/* + * This test is useful for finding memory and refcount leaks. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "drm.h" +#include "ioctl_wrappers.h" +#include "drmtest.h" +#include "intel_bufmgr.h" +#include "intel_batchbuffer.h" +#include "intel_io.h" +#include "intel_chipset.h" + +IGT_TEST_DESCRIPTION("Basic test for memory and refcount leaks."); + +/* options */ +int num_contexts = 10; +int uncontexted = 0; /* test only context create/destroy */ +int multiple_fds = 1; +int iter = 1; + +/* globals */ +pthread_t *threads; +int devid; +int fd; + +static void init_buffer(drm_intel_bufmgr *bufmgr, + struct igt_buf *buf, + uint32_t size) +{ + buf->bo = drm_intel_bo_alloc(bufmgr, "", size, 4096); + buf->size = size; + igt_assert(buf->bo); + buf->tiling = I915_TILING_NONE; + buf->stride = 4096; +} + +static void *work(void *arg) +{ + struct intel_batchbuffer *batch; + igt_render_copyfunc_t rendercopy = igt_get_render_copyfunc(devid); + drm_intel_context *context; + drm_intel_bufmgr *bufmgr; + int td_fd; + int i; + + if (multiple_fds) + td_fd = fd = drm_open_any_render(); + else + td_fd = fd; + + igt_assert(td_fd >= 0); + + bufmgr = drm_intel_bufmgr_gem_init(td_fd, 4096); + batch = intel_batchbuffer_alloc(bufmgr, devid); + context = drm_intel_gem_context_create(bufmgr); + igt_require(context); + + for (i = 0; i < iter; i++) { + struct igt_buf src, dst; + + init_buffer(bufmgr, &src, 4096); + init_buffer(bufmgr, &dst, 4096); + + + if (uncontexted) { + igt_assert(rendercopy); + rendercopy(batch, NU
[Intel-gfx] [PATCH i-g-t 01/17] lib/gt: api polish for igt_can_hang_ring
Align with common igt library style: - Push the igt_require into the function. - Push the intel_gen into the function. Signed-off-by: Daniel Vetter --- lib/igt_gt.c | 12 lib/igt_gt.h | 2 +- tests/gem_concurrent_blit.c | 2 +- tests/gem_pread_after_blit.c | 2 +- tests/gem_reloc_vs_gpu.c | 8 +++- 5 files changed, 10 insertions(+), 16 deletions(-) diff --git a/lib/igt_gt.c b/lib/igt_gt.c index 526cbee03308..c003a7ca30f0 100644 --- a/lib/igt_gt.c +++ b/lib/igt_gt.c @@ -29,16 +29,12 @@ #include "igt_debugfs.h" #include "ioctl_wrappers.h" #include "intel_reg.h" +#include "intel_chipset.h" -int igt_can_hang_ring(int fd, int gen, int ring) +void igt_require_hang_ring(int fd, int ring) { - if (!gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD)) - return 0; - - if (gen < 5) /* safe resets */ - return 0; - - return 1; + igt_require(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD)); + igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5); } struct igt_hang_ring igt_hang_ring(int fd, int gen, int ring) diff --git a/lib/igt_gt.h b/lib/igt_gt.h index 19bbcef2a91e..1ed78837cf5b 100644 --- a/lib/igt_gt.h +++ b/lib/igt_gt.h @@ -24,7 +24,7 @@ #ifndef IGT_GT_H #define IGT_GT_H -int igt_can_hang_ring(int fd, int gen, int ring); +void igt_require_hang_ring(int fd, int ring); struct igt_hang_ring { unsigned handle; diff --git a/tests/gem_concurrent_blit.c b/tests/gem_concurrent_blit.c index 726198012c14..245ad4581783 100644 --- a/tests/gem_concurrent_blit.c +++ b/tests/gem_concurrent_blit.c @@ -474,7 +474,7 @@ static struct igt_hang_ring rcs_hang(void) static void hang_require(void) { - igt_require(igt_can_hang_ring(fd, gen, -1)); + igt_require_hang_ring(fd, -1); } static void do_overwrite_source(const struct access_mode *mode, diff --git a/tests/gem_pread_after_blit.c b/tests/gem_pread_after_blit.c index c09c8dc49ee0..6e4bd6257feb 100644 --- a/tests/gem_pread_after_blit.c +++ b/tests/gem_pread_after_blit.c @@ -239,7 +239,7 @@ igt_main igt_stop_signal_helper(); igt_subtest_f("%s-hang", t->name) { - igt_require(igt_can_hang_ring(fd, batch->gen, -1)); + igt_require_hang_ring(fd, -1); do_test(fd, t->cache, src, start, dst, 1, bcs_hang); } } diff --git a/tests/gem_reloc_vs_gpu.c b/tests/gem_reloc_vs_gpu.c index bb8615b6a593..68bd17d3ae5b 100644 --- a/tests/gem_reloc_vs_gpu.c +++ b/tests/gem_reloc_vs_gpu.c @@ -263,9 +263,7 @@ static void do_forked_test(int fd, unsigned flags) struct igt_helper_process thrasher = {}; if (flags & HANG) - igt_require(igt_can_hang_ring(fd, - intel_gen(devid), - I915_EXEC_BLT)); + igt_require_hang_ring(fd, I915_EXEC_BLT); if (flags & (THRASH | THRASH_INACTIVE)) { uint64_t val = (flags & THRASH_INACTIVE) ? @@ -328,7 +326,7 @@ igt_main do_test(fd, false, no_hang); igt_subtest("interruptible-hang") { - igt_require(igt_can_hang_ring(fd, intel_gen(devid), I915_EXEC_BLT)); + igt_require_hang_ring(fd, I915_EXEC_BLT); do_test(fd, false, bcs_hang); } @@ -336,7 +334,7 @@ igt_main do_test(fd, true, no_hang); igt_subtest("faulting-reloc-interruptible-hang") { - igt_require(igt_can_hang_ring(fd, intel_gen(devid), I915_EXEC_BLT)); + igt_require_hang_ring(fd, I915_EXEC_BLT); do_test(fd, true, bcs_hang); } igt_stop_signal_helper(); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 06/17] lib/ioctl: Add gem_context_destroy helpers
We also need a raw version for some tests. Signed-off-by: Daniel Vetter --- lib/ioctl_wrappers.c | 32 lib/ioctl_wrappers.h | 2 ++ 2 files changed, 34 insertions(+) diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c index c8c0e1c16f8b..e86b3c2bfa46 100644 --- a/lib/ioctl_wrappers.c +++ b/lib/ioctl_wrappers.c @@ -630,6 +630,38 @@ uint32_t gem_context_create(int fd) return create.ctx_id; } +int __gem_context_destroy(int fd, uint32_t ctx_id) +{ + struct drm_i915_gem_context_destroy destroy; + int ret; + + memset(&destroy, 0, sizeof(destroy)); + destroy.ctx_id = ctx_id; + + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy); + if (ret) + return -errno; + return 0; +} + +/** + * gem_context_create: + * @fd: open i915 drm file descriptor + * @ctx_id: i915 hw context id + * + * This is a wraps the CONTEXT_DESTROY ioctl, which is used to free a hardware + * context. + */ +void gem_context_destroy(int fd, uint32_t ctx_id) +{ + struct drm_i915_gem_context_destroy destroy; + + memset(&destroy, 0, sizeof(destroy)); + destroy.ctx_id = ctx_id; + + do_ioctl(fd, DRM_IOCTL_I915_GEM_CONTEXT_DESTROY, &destroy); +} + /** * gem_sw_finish: * @fd: open i915 drm file descriptor diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index 23b8c9d56e44..ad10bd03d370 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -95,6 +95,8 @@ void *gem_mmap__wc(int fd, uint32_t handle, int offset, int size, int prot); int gem_madvise(int fd, uint32_t handle, int state); uint32_t gem_context_create(int fd); +void gem_context_destroy(int fd, uint32_t ctx_id); +int __gem_context_destroy(int fd, uint32_t ctx_id); void gem_sw_finish(int fd, uint32_t handle); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 02/17] lib/ioctl: api polish for gem_context_has_param
Just push the igt_require down to align with the usual style. Signed-off-by: Daniel Vetter --- lib/igt_gt.c | 2 +- lib/ioctl_wrappers.c | 4 ++-- lib/ioctl_wrappers.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/igt_gt.c b/lib/igt_gt.c index c003a7ca30f0..615f9893b876 100644 --- a/lib/igt_gt.c +++ b/lib/igt_gt.c @@ -33,7 +33,7 @@ void igt_require_hang_ring(int fd, int ring) { - igt_require(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD)); + gem_context_require_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD); igt_require(intel_gen(intel_get_drm_devid(fd)) >= 5); } diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c index 19a457ac2b34..dd89e2c57e7f 100644 --- a/lib/ioctl_wrappers.c +++ b/lib/ioctl_wrappers.c @@ -1080,7 +1080,7 @@ int gem_context_set_param(int fd, struct local_i915_gem_context_param *p) return 0; } -int gem_context_has_param(int fd, uint64_t param) +void gem_context_require_param(int fd, uint64_t param) { struct local_i915_gem_context_param p; @@ -1089,5 +1089,5 @@ int gem_context_has_param(int fd, uint64_t param) p.value = 0; p.size = 0; - return gem_context_get_param(fd, &p) == 0; + igt_require(gem_context_get_param(fd, &p) == 0); } diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index 30ab83628a04..a1017ecd3767 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -113,7 +113,7 @@ struct local_i915_gem_context_param { uint64_t value; }; -int gem_context_has_param(int fd, uint64_t param); +void gem_context_require_param(int fd, uint64_t param); int gem_context_get_param(int fd, struct local_i915_gem_context_param *p); int gem_context_set_param(int fd, struct local_i915_gem_context_param *p); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/13] drm: Also check unused fields for addfb2
From: Daniel Vetter Just the usual paranoia ... v2: Fixed format strings. (Tvrtko Ursulin) Signed-off-by: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/drm_crtc.c | 17 + 1 file changed, 17 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e6e2de3..61e7163 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3269,6 +3269,23 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } } + for (; i < 4; i++) { + if (r->handles[i]) { + DRM_DEBUG_KMS("buffer object handle for unused plane %d\n", i); + return -EINVAL; + } + + if (r->pitches[i] || r->offsets[i]) { + DRM_DEBUG_KMS("buffer pitch/offset for unused plane %d", i); + return -EINVAL; + } + + if (r->modifier[i]) { + DRM_DEBUG_KMS("fb modifer for unused plane %d", i); + return -EINVAL; + } + } + return 0; } -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests/kms_addfb: Add support for fb modifiers
From: Tvrtko Ursulin Just a few basic tests to make sure fb modifiers can be used and behave sanely when mixed with the old set_tiling API. v2: * Review feedback from Daniel Vetter: 1. Move cap detection into the subtest so skipping works. 2. Added some gtkdoc comments. 3. Two more test cases. 4. Removed unused parts for now. v3: * Removed two tests which do not make sense any more after the fb modifier rewrite. Signed-off-by: Tvrtko Ursulin --- lib/ioctl_wrappers.c | 17 + lib/ioctl_wrappers.h | 37 tests/kms_addfb.c| 68 3 files changed, 122 insertions(+) diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c index 19a457a..0a7841a 100644 --- a/lib/ioctl_wrappers.c +++ b/lib/ioctl_wrappers.c @@ -1091,3 +1091,20 @@ int gem_context_has_param(int fd, uint64_t param) return gem_context_get_param(fd, &p) == 0; } + +void igt_require_fb_modifiers(int fd) +{ + static unsigned int has_modifiers, cap_modifiers_tested; + + if (!cap_modifiers_tested) { + uint64_t cap_modifiers; + int ret; + + ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers); + igt_assert(ret == 0 || errno == EINVAL); + has_modifiers = ret == 0 && cap_modifiers == 1; + cap_modifiers_tested = 1; + } + + igt_require(has_modifiers); +} diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index 30ab836..9459130 100644 --- a/lib/ioctl_wrappers.h +++ b/lib/ioctl_wrappers.h @@ -117,4 +117,41 @@ int gem_context_has_param(int fd, uint64_t param); int gem_context_get_param(int fd, struct local_i915_gem_context_param *p); int gem_context_set_param(int fd, struct local_i915_gem_context_param *p); +struct local_drm_mode_fb_cmd2 { + uint32_t fb_id; + uint32_t width, height; + uint32_t pixel_format; + uint32_t flags; + uint32_t handles[4]; + uint32_t pitches[4]; + uint32_t offsets[4]; + uint64_t modifier[4]; +}; + +#define LOCAL_DRM_MODE_FB_MODIFIERS(1<<1) + +#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL 0x01 + +#define local_fourcc_mod_code(vendor, val) \ + uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \ + (val & 0x00ffL)) + +#define LOCAL_I915_FORMAT_MOD_NONE local_fourcc_mod_code(INTEL, \ + 0x00L) +#define LOCAL_I915_FORMAT_MOD_X_TILED local_fourcc_mod_code(INTEL, \ + 0x01L) + +#define LOCAL_DRM_IOCTL_MODE_ADDFB2DRM_IOWR(0xB8, \ +struct local_drm_mode_fb_cmd2) + +#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS 0x10 + +/** + * igt_require_fb_modifiers: + * @fd: Open DRM file descriptor. + * + * Requires presence of DRM_CAP_ADDFB2_MODIFIERS. + */ +void igt_require_fb_modifiers(int fd); + #endif /* IOCTL_WRAPPERS_H */ diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c index 756589e..625aa27 100644 --- a/tests/kms_addfb.c +++ b/tests/kms_addfb.c @@ -213,6 +213,72 @@ static void size_tests(int fd) } } +static void addfb25_tests(int fd) +{ + struct local_drm_mode_fb_cmd2 f = {}; + + + memset(&f, 0, sizeof(f)); + + f.width = 1024; + f.height = 1024; + f.pixel_format = DRM_FORMAT_XRGB; + f.pitches[0] = 1024*4; + f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE; + + igt_fixture { + gem_bo = gem_create(fd, 1024*1024*4); + igt_assert(gem_bo); + } + + f.handles[0] = gem_bo; + + igt_subtest("addfb25-modifier-no-flag") { + igt_require_fb_modifiers(fd); + + f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED; + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL); + f.fb_id = 0; + } + + f.flags = LOCAL_DRM_MODE_FB_MODIFIERS; + + igt_fixture + gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4); + f.pitches[0] = 1024*4; + + igt_subtest("addfb25-X-tiled-mismatch") { + igt_require_fb_modifiers(fd); + + f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE; + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL); + f.fb_id = 0; + } + + igt_subtest("addfb25-X-tiled") { + igt_require_fb_modifiers(fd); + + f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED; + igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0); + igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0); + f.fb_id = 0; + } + + igt_subtest("addfb25-framebuffer-vs-set-tiling") { + igt_require_fb_modifiers(fd); + + igt_assert(drmIoctl(fd
[Intel-gfx] [PATCH 07/13] drm/i915: Switch +intel_fb_align_height to fb format modifiers
From: Daniel Vetter With this we can treat the fb format modifier completely independently from the fencing mode in obj->tiling_mode in the initial plane code. Which means new tiling modes without any gtt fence are now fully support in the core i915 driver code. v2: Also add pixel_format while at it, we need this to compute the height for the new tiling formats. Signed-off-by: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_display.c | 20 ++-- drivers/gpu/drm/i915/intel_drv.h | 3 ++- drivers/gpu/drm/i915/intel_fbdev.c | 3 ++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f2622e3..ec172f9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2190,11 +2190,15 @@ static bool need_vtd_wa(struct drm_device *dev) } int -intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) +intel_fb_align_height(struct drm_device *dev, int height, + uint32_t pixel_format, + uint64_t fb_format_modifier) { int tile_height; - tile_height = tiling ? (IS_GEN2(dev) ? 16 : 8) : 1; + tile_height = fb_format_modifier == I915_FORMAT_MOD_X_TILED ? + (IS_GEN2(dev) ? 16 : 8) : 1; + return ALIGN(height, tile_height); } @@ -6658,7 +6662,8 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, fb->pitches[0] = val & 0xffc0; aligned_height = intel_fb_align_height(dev, fb->height, - plane_config->tiling); + fb->pixel_format, + fb->modifier[0]); plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height); @@ -7700,7 +7705,8 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, fb->pitches[0] = (val & 0x3ff) * stride_mult; aligned_height = intel_fb_align_height(dev, fb->height, - plane_config->tiling); + fb->pixel_format, + fb->modifier[0]); plane_config->size = ALIGN(fb->pitches[0] * aligned_height, PAGE_SIZE); @@ -7796,7 +7802,8 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, fb->pitches[0] = val & 0xffc0; aligned_height = intel_fb_align_height(dev, fb->height, - plane_config->tiling); + fb->pixel_format, + fb->modifier[0]); plane_config->size = PAGE_ALIGN(fb->pitches[0] * aligned_height); @@ -12824,7 +12831,8 @@ static int intel_framebuffer_init(struct drm_device *dev, return -EINVAL; aligned_height = intel_fb_align_height(dev, mode_cmd->height, - obj->tiling_mode); + mode_cmd->pixel_format, + mode_cmd->modifier[0]); /* FIXME drm helper for size checks (especially planar formats)? */ if (obj->base.size < aligned_height * mode_cmd->pitches[0]) return -EINVAL; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 76b3c20..b9598ba 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -879,7 +879,8 @@ void intel_frontbuffer_flip(struct drm_device *dev, } int intel_fb_align_height(struct drm_device *dev, int height, - unsigned int tiling); + uint32_t pixel_format, + uint64_t fb_format_modifier); void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire); diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 3001a867..234a699 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -594,7 +594,8 @@ static bool intel_fbdev_init_bios(struct drm_device *dev, cur_size = intel_crtc->config->base.adjusted_mode.crtc_vdisplay; cur_size = intel_fb_align_height(dev, cur_size, -plane_config->tiling); +fb->base.pixel_format, +fb->base.modifier[0]); cur_size *= fb->base.pitches[0]; DRM_DEBUG_KMS("pipe %c area: %dx%d, bpp: %d, size: %d\n", pipe_name(intel_crtc->pipe), -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/13] drm/i915: Announce support for framebuffer modifiers
From: Tvrtko Ursulin Let the DRM core know we can handle it. v2: Change to boolean true. (Daniel Vetter) Signed-off-by: Tvrtko Ursulin Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e5e9221..0f2a6c7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13172,6 +13172,8 @@ void intel_modeset_init(struct drm_device *dev) dev->mode_config.preferred_depth = 24; dev->mode_config.prefer_shadow = 1; + dev->mode_config.allow_fb_modifiers = true; + dev->mode_config.funcs = &intel_mode_funcs; intel_init_quirks(dev); -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/13] drm/i915/skl: CS flips are not supported with execlists
From: Tvrtko Ursulin Therefore remove dead code. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_display.c | 72 ++-- 1 file changed, 4 insertions(+), 68 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index df47031..38c2909 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9632,69 +9632,6 @@ static int intel_queue_mmio_flip(struct drm_device *dev, return 0; } -static int intel_gen9_queue_flip(struct drm_device *dev, -struct drm_crtc *crtc, -struct drm_framebuffer *fb, -struct drm_i915_gem_object *obj, -struct intel_engine_cs *ring, -uint32_t flags) -{ - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - uint32_t plane = 0, stride; - int ret; - - switch(intel_crtc->pipe) { - case PIPE_A: - plane = MI_DISPLAY_FLIP_SKL_PLANE_1_A; - break; - case PIPE_B: - plane = MI_DISPLAY_FLIP_SKL_PLANE_1_B; - break; - case PIPE_C: - plane = MI_DISPLAY_FLIP_SKL_PLANE_1_C; - break; - default: - WARN_ONCE(1, "unknown plane in flip command\n"); - return -ENODEV; - } - - switch (obj->tiling_mode) { - case I915_TILING_NONE: - stride = fb->pitches[0] >> 6; - break; - case I915_TILING_X: - stride = fb->pitches[0] >> 9; - break; - default: - WARN_ONCE(1, "unknown tiling in flip command\n"); - return -ENODEV; - } - - ret = intel_ring_begin(ring, 10); - if (ret) - return ret; - - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); - intel_ring_emit(ring, DERRMR); - intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE | - DERRMR_PIPEB_PRI_FLIP_DONE | - DERRMR_PIPEC_PRI_FLIP_DONE)); - intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) | - MI_SRM_LRM_GLOBAL_GTT); - intel_ring_emit(ring, DERRMR); - intel_ring_emit(ring, ring->scratch.gtt_offset + 256); - intel_ring_emit(ring, 0); - - intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane); - intel_ring_emit(ring, stride << 6 | obj->tiling_mode); - intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset); - - intel_mark_page_flip_active(intel_crtc); - __intel_ring_advance(ring); - - return 0; -} - static int intel_default_queue_flip(struct drm_device *dev, struct drm_crtc *crtc, struct drm_framebuffer *fb, @@ -12994,9 +12931,6 @@ static void intel_init_display(struct drm_device *dev) valleyview_modeset_global_resources; } - /* Default just returns -ENODEV to indicate unsupported */ - dev_priv->display.queue_flip = intel_default_queue_flip; - switch (INTEL_INFO(dev)->gen) { case 2: dev_priv->display.queue_flip = intel_gen2_queue_flip; @@ -13019,8 +12953,10 @@ static void intel_init_display(struct drm_device *dev) dev_priv->display.queue_flip = intel_gen7_queue_flip; break; case 9: - dev_priv->display.queue_flip = intel_gen9_queue_flip; - break; + /* Drop through - unsupported since execlist only. */ + default: + /* Default just returns -ENODEV to indicate unsupported */ + dev_priv->display.queue_flip = intel_default_queue_flip; } intel_panel_init_backlight_funcs(dev); -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/13] drm/i915: Add fb format modifier support
From: Daniel Vetter Currently we don't support anything but X tiled. And for an easier transition it makes a lot of sense to just keep requiring that X tiled is properly fenced. Which means we need to do absolutely nothing in old code to support fb modifiers, yay! v2: Fix the Y tiling check, noticed by Tvrtko. v3: Catch Y-tiled fb for legacy addfb again (Tvrtko) and explain why we want X tiling to match in the comment. Cc: Tvrtko Ursulin Signed-off-by: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_display.c | 28 +++- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3fe9598..4886ff8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12707,7 +12707,24 @@ static int intel_framebuffer_init(struct drm_device *dev, WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - if (obj->tiling_mode == I915_TILING_Y) { + if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) { + /* Enforce that fb modifier and tiling mode match, but only for +* X-tiled. This is needed for FBC. */ + if (!!(obj->tiling_mode == I915_TILING_X) != + !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) { + DRM_DEBUG("tiling_mode doesn't match fb modifier\n"); + return -EINVAL; + } + } else { + if (obj->tiling_mode == I915_TILING_X) + mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED; + else if (obj->tiling_mode == I915_TILING_Y) { + DRM_DEBUG("No Y tiling for legacy addfb\n"); + return -EINVAL; + } + } + + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED) { DRM_DEBUG("hardware does not support tiling Y\n"); return -EINVAL; } @@ -12721,12 +12738,12 @@ static int intel_framebuffer_init(struct drm_device *dev, if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) { pitch_limit = 32*1024; } else if (INTEL_INFO(dev)->gen >= 4) { - if (obj->tiling_mode) + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED) pitch_limit = 16*1024; else pitch_limit = 32*1024; } else if (INTEL_INFO(dev)->gen >= 3) { - if (obj->tiling_mode) + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED) pitch_limit = 8*1024; else pitch_limit = 16*1024; @@ -12736,12 +12753,13 @@ static int intel_framebuffer_init(struct drm_device *dev, if (mode_cmd->pitches[0] > pitch_limit) { DRM_DEBUG("%s pitch (%d) must be at less than %d\n", - obj->tiling_mode ? "tiled" : "linear", + mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED ? + "tiled" : "linear", mode_cmd->pitches[0], pitch_limit); return -EINVAL; } - if (obj->tiling_mode != I915_TILING_NONE && + if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED && mode_cmd->pitches[0] != obj->stride) { DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n", mode_cmd->pitches[0], obj->stride); -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/13] drm/i915: Use fb modifiers in intel_check_cursor_plane
From: Tvrtko Ursulin Also drop the mutex since with universal planes object tiling mode is locked down while assigned to a framebuffer. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_display.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 38c2909..edd6cfe 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12175,12 +12175,11 @@ intel_check_cursor_plane(struct drm_plane *plane, return 0; /* we only need to pin inside GTT if cursor is non-phy */ - mutex_lock(&dev->struct_mutex); - if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) { + if (!INTEL_INFO(dev)->cursor_needs_physical && + fb->modifier[0] != DRM_FORMAT_MOD_NONE) { DRM_DEBUG_KMS("cursor cannot be tiled\n"); ret = -EINVAL; } - mutex_unlock(&dev->struct_mutex); finish: if (intel_crtc->active) { -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 00/13] i915 fb modifier support, respun
From: Tvrtko Ursulin Aggregated patch series containing Daniel's take on fb modifiers plus some missing bits, fixes and cleanups. Also including Rob Clark's RFC so PRTS can attempt to test this. Needs updated kms_addfb i-g-t which will follow. Daniel Vetter (5): drm/i915: Add fb format modifier support drm: Also check unused fields for addfb2 drm/i915: Set up fb format modifier for initial plane config drm/i915: Switch +intel_fb_align_height to fb format modifiers drm/i915: Use fb format modifiers in skylake_update_primary_plane Rob Clark (1): RFC: drm: add support for tiled/compressed/etc modifier in addfb2 Tvrtko Ursulin (7): drm/i915: Add tiled framebuffer modifiers drm/i915: Show frame buffer modifier in debug info drm/i915/skl: CS flips are not supported with execlists drm/i915/skl: Use fb modifiers for sprites drm/i915: Use fb modifiers in intel_check_cursor_plane drm/i915: Use fb modifiers in intel_pin_and_fence_fb_obj drm/i915: Announce support for framebuffer modifiers drivers/gpu/drm/drm_crtc.c | 31 ++- drivers/gpu/drm/drm_crtc_helper.c| 1 + drivers/gpu/drm/drm_ioctl.c | 3 + drivers/gpu/drm/i915/i915_debugfs.c | 6 +- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 164 +++ drivers/gpu/drm/i915/intel_drv.h | 3 +- drivers/gpu/drm/i915/intel_fbdev.c | 3 +- drivers/gpu/drm/i915/intel_sprite.c | 13 ++- include/drm/drm_crtc.h | 4 + include/uapi/drm/drm.h | 1 + include/uapi/drm/drm_fourcc.h| 63 ++ include/uapi/drm/drm_mode.h | 9 ++ 13 files changed, 195 insertions(+), 107 deletions(-) -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/13] RFC: drm: add support for tiled/compressed/etc modifier in addfb2
From: Rob Clark In DRM/KMS we are lacking a good way to deal with tiled/compressed formats. Especially in the case of dmabuf/prime buffer sharing, where we cannot always rely on under-the-hood flags passed to driver specific gem-create ioctl to pass around these extra flags. The proposal is to add a per-plane format modifier. This allows to, if necessary, use different tiling patters for sub-sampled planes, etc. The format modifiers are added at the end of the ioctl struct, so for legacy userspace it will be zero padded. v1: original v1.5: increase modifier to 64b v2: Incorporate review comments from the big thread, plus a few more. - Add a getcap so that userspace doesn't have to jump through hoops. - Allow modifiers only when a flag is set. That way drivers know when they're dealing with old userspace and need to fish out e.g. tiling from other information. - After rolling out checks for ->modifier to all drivers I've decided that this is way too fragile and needs an explicit opt-in flag. So do that instead. - Add a define (just for documentation really) for the "NONE" modifier. Imo we don't need to add mask #defines since drivers really should only do exact matches against values defined with fourcc_mod_code. - Drop the Samsung tiling modifier on Rob's request since he's not yet sure whether that one is accurate. v3: - Also add a new ->modifier[] array to struct drm_framebuffer and fill it in drm_helper_mode_fill_fb_struct. Requested by Tvrkto Uruslin. - Remove TODO in comment and add code comment that modifiers should be properly documented, requested by Rob. Cc: Rob Clark Cc: Tvrtko Ursulin Cc: Laurent Pinchart Cc: Daniel Stone Cc: Ville Syrjälä Cc: Michel Dänzer Signed-off-by: Rob Clark (v1.5) Reviewed-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c| 14 +- drivers/gpu/drm/drm_crtc_helper.c | 1 + drivers/gpu/drm/drm_ioctl.c | 3 +++ include/drm/drm_crtc.h| 4 include/uapi/drm/drm.h| 1 + include/uapi/drm/drm_fourcc.h | 32 include/uapi/drm/drm_mode.h | 9 + 7 files changed, 63 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 6b00173..e6e2de3 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3261,6 +3261,12 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i); return -EINVAL; } + + if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) { + DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n", + r->modifier[i], i); + return -EINVAL; + } } return 0; @@ -3274,7 +3280,7 @@ static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev, struct drm_framebuffer *fb; int ret; - if (r->flags & ~DRM_MODE_FB_INTERLACED) { + if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); return ERR_PTR(-EINVAL); } @@ -3290,6 +3296,12 @@ static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev, return ERR_PTR(-EINVAL); } + if (r->flags & DRM_MODE_FB_MODIFIERS && + !dev->mode_config.allow_fb_modifiers) { + DRM_DEBUG_KMS("driver does not support fb modifiers\n"); + return ERR_PTR(-EINVAL); + } + ret = framebuffer_check(r); if (ret) return ERR_PTR(ret); diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index b1979e7..3053aab 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -837,6 +837,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb, for (i = 0; i < 4; i++) { fb->pitches[i] = mode_cmd->pitches[i]; fb->offsets[i] = mode_cmd->offsets[i]; + fb->modifier[i] = mode_cmd->modifier[i]; } drm_fb_get_bpp_depth(mode_cmd->pixel_format, &fb->depth, &fb->bits_per_pixel); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 3785d66..a6d773a 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -321,6 +321,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ else req->value = 64; break; + case DRM_CAP_ADDFB2_MODIFIERS: + req->value = dev->mode_config.allow_fb_modifiers; + break; default: return -EINVAL; } diff --git a/include/drm/drm_crtc
[Intel-gfx] [PATCH 08/13] drm/i915: Use fb format modifiers in skylake_update_primary_plane
From: Daniel Vetter Just a little demo really. We probably need to introduce skl specific functions for a lot of the format validation stuff, or at least helpers. Specifically I think intel_framebuffer_init and intel_fb_align_height must be adjusted to have an i915_ and a skl_ variant. And only shared code should be converted to fb modifiers, platform code (like the plane config readout can keep on using old tiling_mode defines to avoid some churn). Cc: Tvrtko Ursulin Signed-off-by: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ec172f9..df47031 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2779,11 +2779,11 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc, * The stride is either expressed as a multiple of 64 bytes chunks for * linear buffers or in number of tiles for tiled buffers. */ - switch (obj->tiling_mode) { - case I915_TILING_NONE: + switch (fb->modifier[0]) { + case DRM_FORMAT_MOD_NONE: stride = fb->pitches[0] >> 6; break; - case I915_TILING_X: + case I915_FORMAT_MOD_X_TILED: plane_ctl |= PLANE_CTL_TILED_X; stride = fb->pitches[0] >> 9; break; -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/13] drm/i915: Use fb modifiers in intel_pin_and_fence_fb_obj
From: Tvrtko Ursulin And at the same time replace BUG() with a warning and handle it gracefuly. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_display.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index edd6cfe..e5e9221 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2215,8 +2215,8 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - switch (obj->tiling_mode) { - case I915_TILING_NONE: + switch (fb->modifier[0]) { + case DRM_FORMAT_MOD_NONE: if (INTEL_INFO(dev)->gen >= 9) alignment = 256 * 1024; else if (IS_BROADWATER(dev) || IS_CRESTLINE(dev)) @@ -2226,7 +2226,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, else alignment = 64 * 1024; break; - case I915_TILING_X: + case I915_FORMAT_MOD_X_TILED: if (INTEL_INFO(dev)->gen >= 9) alignment = 256 * 1024; else { @@ -2234,11 +2234,12 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, alignment = 0; } break; - case I915_TILING_Y: + case I915_FORMAT_MOD_Y_TILED: WARN(1, "Y tiled bo slipped through, driver bug!\n"); return -EINVAL; default: - BUG(); + MISSING_CASE(fb->modifier[0]); + return -EINVAL; } /* Note that the w/a also requires 64 PTE of padding following the -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/13] drm/i915: Show frame buffer modifier in debug info
From: Tvrtko Ursulin Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_debugfs.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 2e1f723..164fa82 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1778,11 +1778,12 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) ifbdev = dev_priv->fbdev; fb = to_intel_framebuffer(ifbdev->helper.fb); - seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, refcount %d, obj ", + seq_printf(m, "fbcon size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ", fb->base.width, fb->base.height, fb->base.depth, fb->base.bits_per_pixel, + fb->base.modifier[0], atomic_read(&fb->base.refcount.refcount)); describe_obj(m, fb->obj); seq_putc(m, '\n'); @@ -1793,11 +1794,12 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) if (ifbdev && &fb->base == ifbdev->helper.fb) continue; - seq_printf(m, "user size: %d x %d, depth %d, %d bpp, refcount %d, obj ", + seq_printf(m, "user size: %d x %d, depth %d, %d bpp, modifier 0x%llx, refcount %d, obj ", fb->base.width, fb->base.height, fb->base.depth, fb->base.bits_per_pixel, + fb->base.modifier[0], atomic_read(&fb->base.refcount.refcount)); describe_obj(m, fb->obj); seq_putc(m, '\n'); -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/13] drm/i915: Set up fb format modifier for initial plane config
From: Daniel Vetter No functional changes yet since intel_framebuffer_init would have fixed this up for us. But this is prep work to be able to handle new tiling layouts in the initial plane config code. Follow-up patches will start to make use of this and switch over to fb modifiers where needed. Signed-off-by: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_display.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4886ff8..f2622e3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2390,6 +2390,8 @@ intel_alloc_plane_obj(struct intel_crtc *crtc, mode_cmd.width = fb->width; mode_cmd.height = fb->height; mode_cmd.pitches[0] = fb->pitches[0]; + mode_cmd.modifier[0] = fb->modifier[0]; + mode_cmd.flags = DRM_MODE_FB_MODIFIERS; mutex_lock(&dev->struct_mutex); @@ -6625,9 +6627,12 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, fb = &intel_fb->base; - if (INTEL_INFO(dev)->gen >= 4) - if (val & DISPPLANE_TILED) + if (INTEL_INFO(dev)->gen >= 4) { + if (val & DISPPLANE_TILED) { plane_config->tiling = I915_TILING_X; + fb->modifier[0] = I915_FORMAT_MOD_X_TILED; + } + } pixel_format = val & DISPPLANE_PIXFORMAT_MASK; fourcc = i9xx_format_to_fourcc(pixel_format); @@ -7659,8 +7664,10 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, if (!(val & PLANE_CTL_ENABLE)) goto error; - if (val & PLANE_CTL_TILED_MASK) + if (val & PLANE_CTL_TILED_MASK) { plane_config->tiling = I915_TILING_X; + fb->modifier[0] = I915_FORMAT_MOD_X_TILED; + } pixel_format = val & PLANE_CTL_FORMAT_MASK; fourcc = skl_format_to_fourcc(pixel_format, @@ -7758,9 +7765,12 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, fb = &intel_fb->base; - if (INTEL_INFO(dev)->gen >= 4) - if (val & DISPPLANE_TILED) + if (INTEL_INFO(dev)->gen >= 4) { + if (val & DISPPLANE_TILED) { plane_config->tiling = I915_TILING_X; + fb->modifier[0] = I915_FORMAT_MOD_X_TILED; + } + } pixel_format = val & DISPPLANE_PIXFORMAT_MASK; fourcc = i9xx_format_to_fourcc(pixel_format); -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/13] drm/i915/skl: Use fb modifiers for sprites
From: Tvrtko Ursulin Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_sprite.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0a52c44..9e6f0e5 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -245,11 +245,11 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc, BUG(); } - switch (obj->tiling_mode) { - case I915_TILING_NONE: + switch (fb->modifier[0]) { + case DRM_FORMAT_MOD_NONE: stride = fb->pitches[0] >> 6; break; - case I915_TILING_X: + case I915_FORMAT_MOD_X_TILED: plane_ctl |= PLANE_CTL_TILED_X; stride = fb->pitches[0] >> 9; break; @@ -1076,7 +1076,6 @@ intel_check_sprite_plane(struct drm_plane *plane, struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_framebuffer *fb = state->base.fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); int crtc_x, crtc_y; unsigned int crtc_w, crtc_h; uint32_t src_x, src_y, src_w, src_h; @@ -1107,9 +1106,9 @@ intel_check_sprite_plane(struct drm_plane *plane, } /* Sprite planes can be linear or x-tiled surfaces */ - switch (obj->tiling_mode) { - case I915_TILING_NONE: - case I915_TILING_X: + switch (fb->modifier[0]) { + case DRM_FORMAT_MOD_NONE: + case I915_FORMAT_MOD_X_TILED: break; default: DRM_DEBUG_KMS("Unsupported tiling mode\n"); -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/13] drm/i915: Add tiled framebuffer modifiers
From: Tvrtko Ursulin To be used from the new addfb2 extension. v2: - Drop Intel-specific untiled modfier. - Move to drm_fourcc.h. - Document layouts a bit and denote them as platform-specific and not useable for cross-driver sharing. - Add Y-tiling for completeness. - Drop special docstring markers to avoid confusing kerneldoc. v3: Give Y-tiling a unique idea, noticed by Tvrtko. Signed-off-by: Tvrtko Ursulin (v1) Cc: Tvrtko Ursulin Signed-off-by: Daniel Vetter Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_drv.h | 1 + include/uapi/drm/drm_fourcc.h | 31 +++ 2 files changed, 32 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 26ffe8b..ea4b250 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -31,6 +31,7 @@ #define _I915_DRV_H_ #include +#include #include "i915_reg.h" #include "intel_bios.h" diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 188e61f..1a5a357 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -161,4 +161,35 @@ * authoritative source for all of these. */ +/* Intel framebuffer modifiers */ + +/* + * Intel X-tiling layout + * + * This is a tiled layout using 4Kb tiles (except on gen2 where the tiles 2Kb) + * in row-major layout. Within the tile bytes are laid out row-major, with + * a platform-dependent stride. On top of that the memory can apply + * platform-depending swizzling of some higher address bits into bit6. + * + * This format is highly platforms specific and not useful for cross-driver + * sharing. It exists since on a given platform it does uniquely identify the + * layout in a simple way for i915-specific userspace. + */ +#define I915_FORMAT_MOD_X_TILEDfourcc_mod_code(INTEL, 1) + +/* + * Intel Y-tiling layout + * + * This is a tiled layout using 4Kb tiles (except on gen2 where the tiles 2Kb) + * in row-major layout. Within the tile bytes are laid out in OWORD (16 bytes) + * chunks column-major, with a platform-dependent height. On top of that the + * memory can apply platform-depending swizzling of some higher address bits + * into bit6. + * + * This format is highly platforms specific and not useful for cross-driver + * sharing. It exists since on a given platform it does uniquely identify the + * layout in a simple way for i915-specific userspace. + */ +#define I915_FORMAT_MOD_Y_TILEDfourcc_mod_code(INTEL, 2) + #endif /* DRM_FOURCC_H */ -- 2.2.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add tiled framebuffer modifiers
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5743 -Summary- Platform Delta drm-intel-nightly Series Applied PNV +4 275/283 279/283 ILK -1 310/315 309/315 SNB +3 320/346 323/346 IVB -1 380/384 379/384 BYT 296/296 296/296 HSW +3 422/428 425/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync CRASH(2, M7)PASS(1, M7) PASS(1, M7) PNV igt_gem_userptr_blits_coherency-unsync CRASH(2, M7)PASS(1, M7) PASS(1, M7) PNV igt_gem_userptr_blits_create-destroy-sync NRUN(1, M7)PASS(2, M7) PASS(1, M7) *PNV igt_gen3_render_tiledx_blits TIMEOUT(1, M7)PASS(2, M7) FAIL(1, M7) PNV igt_gen3_render_tiledy_blits FAIL(2, M7)PASS(1, M7) PASS(1, M7) *ILK igt_gem_unfence_active_buffers PASS(2, M26) DMESG_WARN(1, M26) *SNB igt_kms_flip_bo-too-big BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_event_leak NSPT(2, M35) PASS(1, M35) *IVB igt_gem_storedw_batches_loop_normal PASS(2, M4) DMESG_WARN(1, M4) *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M40) PASS(1, M40) *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M40) PASS(1, M40) HSW igt_kms_flip_plain-flip-fb-recreate-interruptible TIMEOUT(2, M40)PASS(1, M40) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915: Use fb format modifiers in skylake_update_primary_plane
On Mon, Feb 09, 2015 at 07:03:26PM +0100, Daniel Vetter wrote: > Just a little demo really. We probably need to introduce skl specific > functions for a lot of the format validation stuff, or at least > helpers. Specifically I think intel_framebuffer_init and > intel_fb_align_height must be adjusted to have an i915_ and a skl_ > variant. And only shared code should be converted to fb modifiers, > platform code (like the plane config readout can keep on using old > tiling_mode defines to avoid some churn). > > Cc: Tvrtko Ursulin > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 2d69cce03ab5..41b3ddc4068d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2773,11 +2773,11 @@ static void skylake_update_primary_plane(struct > drm_crtc *crtc, >* The stride is either expressed as a multiple of 64 bytes chunks for >* linear buffers or in number of tiles for tiled buffers. >*/ > - switch (obj->tiling_mode) { > - case I915_TILING_NONE: > + switch (fb->modifier[0]) { > + case DRM_FORMAT_MOD_NONE: > stride = fb->pitches[0] >> 6; > break; > - case I915_TILING_X: > + case I915_FORMAT_MOD_X_TILED: > plane_ctl |= PLANE_CTL_TILED_X; > stride = fb->pitches[0] >> 9; > break; Just a remark across the board is that this code won't work as we add new types of modifiers besides tiling. Might as wels reserve tiling bits today and select on them. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
On 09/02/15 15:05, Wang, Zhi A wrote: > Hi Gurus: > Forgive my junior HW knowledge, I just found that in execlist > context initialization function populate_lr_context(), this line: > > reg_state[CTX_CONTEXT_CONTROL+1] = > _MASKED_BIT_ENABLE((1<<3) | MI_RESTORE_INHIBIT); > > It seems the "Inhibit Synchronous Context Switch" bit is not set > here, so when HW is trying to wait some events, e.g. semaphore, according to > Bspec, per my basic understanding, it will switch out current context > with some reason bit set. Here comes my question, I think if this > situation happen, should i915 remember this context and try to > re-schedule it in a proper time, e.g. before submitting a new context > until the context_completed bit in CSB is set? I don't find a register > that will remember the context switched out by waiting event, so I think > it should be i915 to handle this situation or just set "Inhibit > Synchronous Context Switch" bit here?... But that's exactly what it does already ... it sets bit 3, which is the "Inhibit Synchronous Context Switch" bit you refer to. What's wrong here is that it should use a #define for this bit, and for the "Restore Inhibit" bit -- for which it's actually using a value defined in a completely different context (no pun intended), namely the bits in a MI_SET_CONTEXT *instruction*. It just happens to work because they have the same numerical value. So, what we need is: #define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244) #define CTX_CTRL_INHIBIT_SYN_CTX_SWITCH (1 << 3) #define CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT (1 << 0) and then change the line you pointed out to use the correct symbolic names. .Dave. > Thanks, > Zhi. > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Compile error - xserver-xorg-video-intel
Hello, xserver-xorg-video-intel from git does not compile on ubuntu precise box anymore... Here is error log: https://launchpadlibrarian.net/197217861/buildlog_ubuntu-precise-amd64.xserver-xorg-video-intel_2:2.99.917-git201502091709.928ff25~ubuntu12.04.1_FAILEDTOBUILD.txt.gz ../../../src/uxa/intel_dri.c: At top level: ../../../src/uxa/intel_dri.c:1491:1: error: unknown type name 'bool' ../../../src/uxa/intel_dri.c: In function 'is_level': ../../../src/uxa/intel_dri.c:1498:10: error: 'true' undeclared (first use in this function) ../../../src/uxa/intel_dri.c:1498:10: note: each undeclared identifier is reported only once for each function it appears in ../../../src/uxa/intel_dri.c:1521:9: error: 'false' undeclared (first use in this function) ../../../src/uxa/intel_dri.c: At top level: ../../../src/uxa/intel_dri.c:804:1: warning: 'allocate_back_buffer' defined but not used [-Wunused-function] ../../../src/uxa/intel_dri.c: In function 'is_level': ../../../src/uxa/intel_dri.c:1522:1: error: control reaches end of non-void function [-Werror=return-type] Can you fix it? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
Very appreciate. :) Thanks Thomas! -Original Message- From: Daniel, Thomas Sent: Tuesday, February 10, 2015 10:56 PM To: Wang, Zhi A; Intel-gfx@lists.freedesktop.org Subject: RE: About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c > -Original Message- > From: Wang, Zhi A > Sent: Tuesday, February 10, 2015 2:44 PM > To: Daniel, Thomas; Intel-gfx@lists.freedesktop.org > Subject: RE: About CTX_CONTEXT_CONTROL initialization in > populate_lr_context() intel_lrc.c > > Hi Thomas: > Thanks for the information! Learned a lot. :) > > I remember that semaphore is disabled on Gen8 for a long time ago. > Recently I found it has been enabled. just thought that if a workload > which isn't being protected by MI_ARB_ON_OFF may be switched out by > failing to wait semaphore here without that bit? Please check i915_drv.c i915_semaphore_is_enabled - should return false if execlists are enabled, or if we are on Gen8. > Would you mind to give a help to confirm the correct combos? That > would be nice and great helpful to me :) > > a. Set "Inhibit Synchronous Context Switch" bit, HW will wait until > the condition is true or specific event is generated if SW issue some > instructions like MI_SEMAPHORE_WAIT, I thought that's why SW don't > need to process CSB with wait_semaphore bit set. But if MI_ARB_ON_OFF > is ON, Can SW do a preemption here by submitting a new ELSP write combo? Correct. However the driver will never cause a pre-emption in this way at the moment. > b. Clear "Inhibit Synchronous Context Switch", HW will switch out > context directly after the condition is false or specific event is not > generated at that time, and write a CSB with wait_semaphore bit is > set. And SW has to be aware this kinds of CSB in this case I think... Correct. But as I said, I don't think any of these events will be generated. Thomas. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
> -Original Message- > From: Wang, Zhi A > Sent: Tuesday, February 10, 2015 2:44 PM > To: Daniel, Thomas; Intel-gfx@lists.freedesktop.org > Subject: RE: About CTX_CONTEXT_CONTROL initialization in > populate_lr_context() intel_lrc.c > > Hi Thomas: > Thanks for the information! Learned a lot. :) > > I remember that semaphore is disabled on Gen8 for a long time ago. > Recently I found it has been enabled. just thought that if a workload which > isn't being protected by MI_ARB_ON_OFF may be switched out by failing to > wait semaphore here without that bit? Please check i915_drv.c i915_semaphore_is_enabled - should return false if execlists are enabled, or if we are on Gen8. > Would you mind to give a help to confirm the correct combos? That would be > nice and great helpful to me :) > > a. Set "Inhibit Synchronous Context Switch" bit, HW will wait until the > condition is true or specific event is generated if SW issue some instructions > like MI_SEMAPHORE_WAIT, I thought that's why SW don't need to process > CSB with wait_semaphore bit set. But if MI_ARB_ON_OFF is ON, Can SW do a > preemption here by submitting a new ELSP write combo? Correct. However the driver will never cause a pre-emption in this way at the moment. > b. Clear "Inhibit Synchronous Context Switch", HW will switch out context > directly after the condition is false or specific event is not generated at > that > time, and write a CSB with wait_semaphore bit is set. And SW has to be > aware this kinds of CSB in this case I think... Correct. But as I said, I don't think any of these events will be generated. Thomas. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
Hi Thomas: Thanks for the information! Learned a lot. :) I remember that semaphore is disabled on Gen8 for a long time ago. Recently I found it has been enabled. just thought that if a workload which isn't being protected by MI_ARB_ON_OFF may be switched out by failing to wait semaphore here without that bit? Would you mind to give a help to confirm the correct combos? That would be nice and great helpful to me :) a. Set "Inhibit Synchronous Context Switch" bit, HW will wait until the condition is true or specific event is generated if SW issue some instructions like MI_SEMAPHORE_WAIT, I thought that's why SW don't need to process CSB with wait_semaphore bit set. But if MI_ARB_ON_OFF is ON, Can SW do a preemption here by submitting a new ELSP write combo? b. Clear "Inhibit Synchronous Context Switch", HW will switch out context directly after the condition is false or specific event is not generated at that time, and write a CSB with wait_semaphore bit is set. And SW has to be aware this kinds of CSB in this case I think... Don't know if there is something I missed...:) Thanks. -Original Message- From: Daniel, Thomas Sent: Tuesday, February 10, 2015 10:17 PM To: Wang, Zhi A; Wang, Zhi A; Intel-gfx@lists.freedesktop.org Subject: RE: About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On > Behalf Of Wang, Zhi A > Sent: Tuesday, February 10, 2015 1:27 PM > To: Wang, Zhi A; Intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in > populate_lr_context() intel_lrc.c > > Ping. If someone can confirm it. I can submit a patch, but I'm > concerned about introducing new register bit needs some internal process > > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On > Behalf Of Wang, Zhi A > Sent: Monday, February 09, 2015 11:05 PM > To: Intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in > populate_lr_context() intel_lrc.c > > Hi Gurus: > Forgive my junior HW knowledge, I just found that in execlist > context initialization function populate_lr_context(), this line: > > reg_state[CTX_CONTEXT_CONTROL+1] = > _MASKED_BIT_ENABLE((1<<3) | > MI_RESTORE_INHIBIT); > > It seems the "Inhibit Synchronous Context Switch" bit is not set here, > so when HW is trying to wait some events, e.g. semaphore, according to > Bspec, per my basic understanding, it will switch out current context > with some reason bit set. Here comes my question, I think if this > situation happen, should i915 remember this context and try to > re-schedule it in a proper time, e.g. before submitting a new context > until the context_completed bit in CSB is set? I don't find a register > that will remember the context switched out by waiting event, so I > think it should be i915 to handle this situation or just set "Inhibit > Synchronous Context Switch" bit here?... I don't think any of these events will be generated when execlists are enabled. I think there is a plan to use semaphores again, and a similar codepath will be needed to deal with context pre-emption when we support that, in order to re-submit pre-empted contexts. Thomas. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
> -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf > Of Wang, Zhi A > Sent: Tuesday, February 10, 2015 1:27 PM > To: Wang, Zhi A; Intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in > populate_lr_context() intel_lrc.c > > Ping. If someone can confirm it. I can submit a patch, but I'm concerned > about introducing new register bit needs some internal process > > -Original Message- > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf > Of Wang, Zhi A > Sent: Monday, February 09, 2015 11:05 PM > To: Intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] About CTX_CONTEXT_CONTROL initialization in > populate_lr_context() intel_lrc.c > > Hi Gurus: > Forgive my junior HW knowledge, I just found that in execlist context > initialization function populate_lr_context(), this line: > > reg_state[CTX_CONTEXT_CONTROL+1] = > _MASKED_BIT_ENABLE((1<<3) | MI_RESTORE_INHIBIT); > > It seems the "Inhibit Synchronous Context Switch" bit is not set here, so > when HW is trying to wait some events, e.g. semaphore, according to Bspec, > per my basic understanding, it will switch out current context with some > reason bit set. Here comes my question, I think if this situation happen, > should i915 remember this context and try to re-schedule it in a proper time, > e.g. before submitting a new context until the context_completed bit in CSB > is set? I don't find a register that will remember the context switched out by > waiting event, so I think it should be i915 to handle this situation or just > set > "Inhibit Synchronous Context Switch" bit here?... I don't think any of these events will be generated when execlists are enabled. I think there is a plan to use semaphores again, and a similar codepath will be needed to deal with context pre-emption when we support that, in order to re-submit pre-empted contexts. Thomas. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: handle all pixel formats in skylake_update_primary_plane()
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5742 -Summary- Platform Delta drm-intel-nightly Series Applied PNV +4-1 275/283 278/283 ILK 310/315 310/315 SNB +2 320/346 322/346 IVB 380/384 380/384 BYT 296/296 296/296 HSW +3 422/428 425/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync CRASH(1, M7)PASS(1, M7) PASS(1, M7) PNV igt_gem_userptr_blits_coherency-unsync CRASH(1, M7)PASS(1, M7) PASS(1, M7) PNV igt_gem_userptr_blits_create-destroy-sync NRUN(1, M7)PASS(1, M7) PASS(1, M7) *PNV igt_gen3_render_tiledx_blits TIMEOUT(1, M7)PASS(1, M7) FAIL(1, M7) PNV igt_gen3_render_tiledy_blits FAIL(1, M7)PASS(1, M7) PASS(1, M7) *PNV igt_gem_tiled_pread_pwrite PASS(2, M7) FAIL(1, M7) *SNB igt_kms_flip_bo-too-big BLACKLIST(1, M35) PASS(1, M35) *SNB igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M35) PASS(1, M35) *HSW igt_kms_flip_bo-too-big BLACKLIST(1, M40) PASS(1, M40) *HSW igt_kms_flip_bo-too-big-interruptible BLACKLIST(1, M40) PASS(1, M40) HSW igt_kms_flip_plain-flip-fb-recreate-interruptible TIMEOUT(1, M40)PASS(1, M40) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/18] drm/i915/skl: Implement WaBarrierPerformanceFixDisable
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5737 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 282/283 282/283 ILK 308/315 308/315 SNB +1-22 340/346 319/346 IVB +1-2 378/384 377/384 BYT 296/296 296/296 HSW +2 421/428 423/428 BDW 318/333 318/333 -Detailed- Platform Testdrm-intel-nightly Series Applied *SNB igt_kms_flip_dpms-vs-vblank-race PASS(4, M22) DMESG_WARN(1, M22) SNB igt_kms_flip_dpms-vs-vblank-race-interruptible DMESG_WARN(3, M22)PASS(2, M22) DMESG_WARN(1, M22) SNB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_pipe_crc_basic_read-crc-pipe-A DMESG_WARN(1, M22)PASS(6, M22) PASS(1, M22) SNB igt_kms_rotation_crc_primary-rotation NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_kms_rotation_crc_sprite-rotation NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_cursor NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_cursor-dpms NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_dpms-mode-unset-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_dpms-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_drm-resources-equal NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_fences NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_fences-dpms NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-execbuf NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-mmap-cpu NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-mmap-gtt NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_gem-pread NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_i2c NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_modeset-non-lpsp NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_modeset-non-lpsp-stress-no-wait NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_pci-d3-state NSPT(2, M22)PASS(1, M22) NSPT(1, M22) SNB igt_pm_rpm_rte NSPT(2, M22)PASS(1, M22) NSPT(1, M22) IVB igt_gem_pwrite_pread_snooped-copy-performance DMESG_WARN(1, M34)PASS(5, M34) DMESG_WARN(1, M34) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(2, M34)PASS(2, M34) PASS(1, M34) IVB igt_gem_storedw_batches_loop_secure-dispatch DMESG_WARN(1, M34)PASS(3, M34) DMESG_WARN(1, M34) HSW igt_gem_storedw_loop_blt DMESG_WARN(3, M20)PASS(3, M20) PASS(1, M20) HSW igt_gem_storedw_loop_vebox DMESG_WARN(3, M20)PASS(2, M20) PASS(1, M20) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/12] drm/i915: Reorganize VLV DDL setup
From: Ville Syrjälä Introduce struct vlv_wm_values to house VLV watermark/drain latency values. We start by using it when computing the drain latency values. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 9 drivers/gpu/drm/i915/intel_pm.c | 46 +++-- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 26ffe8b..5de69a0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1514,6 +1514,14 @@ struct ilk_wm_values { enum intel_ddb_partitioning partitioning; }; +struct vlv_wm_values { + struct { + uint8_t cursor; + uint8_t sprite[2]; + uint8_t primary; + } ddl[3]; +}; + struct skl_ddb_entry { uint16_t start, end;/* in number of blocks, 'end' is exclusive */ }; @@ -1870,6 +1878,7 @@ struct drm_i915_private { union { struct ilk_wm_values hw; struct skl_wm_values skl_hw; + struct vlv_wm_values vlv; }; } wm; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c4c2317..5515d10 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -711,6 +711,21 @@ static bool g4x_compute_srwm(struct drm_device *dev, display, cursor); } +static void vlv_write_wm_values(struct intel_crtc *crtc, + const struct vlv_wm_values *wm) +{ + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + enum pipe pipe = crtc->pipe; + + I915_WRITE(VLV_DDL(pipe), + (wm->ddl[pipe].cursor << DDL_CURSOR_SHIFT) | + (wm->ddl[pipe].sprite[1] << DDL_SPRITE_SHIFT(1)) | + (wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) | + (wm->ddl[pipe].primary << DDL_PLANE_SHIFT)); + + dev_priv->wm.vlv = *wm; +} + static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc, int pixel_size) { @@ -757,20 +772,19 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pixel_size; enum pipe pipe = intel_crtc->pipe; - int plane_dl; + struct vlv_wm_values wm = dev_priv->wm.vlv; - plane_dl = I915_READ(VLV_DDL(pipe)) & - ~(((DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK) << DDL_CURSOR_SHIFT) | - ((DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK) << DDL_PLANE_SHIFT)); + wm.ddl[pipe].primary = 0; + wm.ddl[pipe].cursor = 0; if (!intel_crtc_active(crtc)) { - I915_WRITE(VLV_DDL(pipe), plane_dl); + vlv_write_wm_values(intel_crtc, &wm); return; } /* Primary plane Drain Latency */ pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */ - plane_dl = vlv_compute_drain_latency(crtc, pixel_size) << DDL_PLANE_SHIFT; + wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, pixel_size); /* Cursor Drain Latency * BPP is always 4 for cursor @@ -779,9 +793,10 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc) /* Program cursor DL only if it is enabled */ if (intel_crtc->cursor_base) - plane_dl |= vlv_compute_drain_latency(crtc, pixel_size) << DDL_CURSOR_SHIFT; + wm.ddl[pipe].cursor = + vlv_compute_drain_latency(crtc, pixel_size); - I915_WRITE(VLV_DDL(pipe), plane_dl); + vlv_write_wm_values(intel_crtc, &wm); } #define single_plane_enabled(mask) is_power_of_2(mask) @@ -939,17 +954,18 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane, { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; - int pipe = to_intel_plane(plane)->pipe; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + enum pipe pipe = intel_crtc->pipe; int sprite = to_intel_plane(plane)->plane; - int sprite_dl; - - sprite_dl = I915_READ(VLV_DDL(pipe)) & - ~((DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK) << DDL_SPRITE_SHIFT(sprite)); + struct vlv_wm_values wm = dev_priv->wm.vlv; if (enabled) - sprite_dl |= vlv_compute_drain_latency(crtc, pixel_size) << DDL_SPRITE_SHIFT(sprite); + wm.ddl[pipe].sprite[sprite] = + vlv_compute_drain_latency(crtc, pixel_size); + else + wm.ddl[pipe].sprite[sprite] = 0; - I915_WRITE(VLV_DDL(pipe), sprite_dl); + vlv_write_wm_values(intel_crtc, &wm); } static void g4x_update_wm(struct drm_crtc *crtc) -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.
[Intel-gfx] [PATCH 11/12] drm/i915: Program PFI credits for VLV
From: Vidya Srinivas PFI credit programming is required when CD clock (related to data flow from display pipeline to end display) is greater than CZ clock (related to data flow from memory to display plane). This programming should be done when all planes are OFF to avoid intermittent hangs while accessing memory even from different Gfx units (not just display). If cdclk/czclk >=1, PFI credits could be set as any number. To get better performance, larger PFI credit can be assigned to PND. Otherwise if cdclk/czclk<1, the default PFI credit of 8 should be set. v2: - Change log to lower log level instead of DRM_ERROR - Change function name to valleyview_program_pfi_credits - Move program PFI credits to modeset_init instead of intel_set_mode - Change magic numbers to logical constants [vsyrjala v3: - only program in response to cdclk update - program the credits also when cdclk Signed-off-by: Gajanan Bhat Signed-off-by: Vandana Kannan Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 8 drivers/gpu/drm/i915/intel_display.c | 33 + 2 files changed, 41 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index aacf90b..a0a7688 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2061,6 +2061,14 @@ enum skl_disp_power_wells { #define CDCLK_FREQ_SHIFT 4 #define CDCLK_FREQ_MASK (0x1f << CDCLK_FREQ_SHIFT) #define CZCLK_FREQ_MASK 0xf + +#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C) +#define PFI_CREDIT_63(9 << 28) /* chv only */ +#define PFI_CREDIT_31(8 << 28) /* chv only */ +#define PFI_CREDIT(x)(((x) - 8) << 28) /* 8-15 */ +#define PFI_CREDIT_RESEND(1 << 27) +#define VGA_FAST_MODE_DISABLE(1 << 14) + #define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510) /* diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3fe9598..9dcab4b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4987,6 +4987,37 @@ static void valleyview_modeset_global_pipes(struct drm_device *dev, *prepare_pipes |= (1 << intel_crtc->pipe); } +static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv) +{ + unsigned int credits; + + if (DIV_ROUND_CLOSEST(dev_priv->vlv_cdclk_freq, 1000) >= dev_priv->rps.cz_freq) { + /* CHV suggested value is 31 or 63 */ + if (IS_CHERRYVIEW(dev_priv)) + credits = PFI_CREDIT_31; + else + credits = PFI_CREDIT(15); + } else { + credits = PFI_CREDIT(8); + } + + /* +* WA - write default credits before re-programming +* FIXME: should we also set the resend bit here? +*/ + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE | + PFI_CREDIT(8)); + + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE | + credits | PFI_CREDIT_RESEND); + + /* +* FIXME is this guaranteed to clear +* immediately or should we poll for it? +*/ + WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND); +} + static void valleyview_modeset_global_resources(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -5010,6 +5041,8 @@ static void valleyview_modeset_global_resources(struct drm_device *dev) else valleyview_set_cdclk(dev, req_cdclk); + vlv_program_pfi_credits(dev_priv); + intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A); } } -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/12] drm/i915: Pass plane to vlv_compute_drain_latency()
From: Ville Syrjälä Now that we have drm_planes for the cursor and primary we can move the pixel_size handling into vlv_compute_drain_latency() and just pass the appropriate plane to it. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 42 - 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5515d10..fffcf64 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -727,16 +727,26 @@ static void vlv_write_wm_values(struct intel_crtc *crtc, } static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc, -int pixel_size) +struct drm_plane *plane) { struct drm_device *dev = crtc->dev; - int entries, prec_mult, drain_latency; - int clock = to_intel_crtc(crtc)->config->base.adjusted_mode.crtc_clock; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + int entries, prec_mult, drain_latency, pixel_size; + int clock = intel_crtc->config->base.adjusted_mode.crtc_clock; const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64; + /* +* FIXME the plane might have an fb +* but be invisible (eg. due to clipping) +*/ + if (!intel_crtc->active || !plane->fb) + return 0; + if (WARN(clock == 0, "Pixel clock is zero!\n")) return 0; + pixel_size = drm_format_plane_cpp(plane->fb->pixel_format, 0); + if (WARN(pixel_size == 0, "Pixel size is zero!\n")) return 0; @@ -770,31 +780,11 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pixel_size; enum pipe pipe = intel_crtc->pipe; struct vlv_wm_values wm = dev_priv->wm.vlv; - wm.ddl[pipe].primary = 0; - wm.ddl[pipe].cursor = 0; - - if (!intel_crtc_active(crtc)) { - vlv_write_wm_values(intel_crtc, &wm); - return; - } - - /* Primary plane Drain Latency */ - pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */ - wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, pixel_size); - - /* Cursor Drain Latency -* BPP is always 4 for cursor -*/ - pixel_size = 4; - - /* Program cursor DL only if it is enabled */ - if (intel_crtc->cursor_base) - wm.ddl[pipe].cursor = - vlv_compute_drain_latency(crtc, pixel_size); + wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, crtc->primary); + wm.ddl[pipe].cursor = vlv_compute_drain_latency(crtc, crtc->cursor); vlv_write_wm_values(intel_crtc, &wm); } @@ -961,7 +951,7 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane, if (enabled) wm.ddl[pipe].sprite[sprite] = - vlv_compute_drain_latency(crtc, pixel_size); + vlv_compute_drain_latency(crtc, plane); else wm.ddl[pipe].sprite[sprite] = 0; -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/12] drm/i915: Enable the maxfifo PM5 mode when appropriate on CHV
From: Ville Syrjälä CHV has a new knob in Punit to select between some memory power savings modes PM2 and PM5. We can allow the deeper PM5 when maxfifo mode is enabled, so let's do so in the hopes for moar power savings. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 13 - 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a0a7688..2196e57 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -552,6 +552,9 @@ #define DSPFREQSTAT_MASK (0x3 << DSPFREQSTAT_SHIFT) #define DSPFREQGUAR_SHIFT14 #define DSPFREQGUAR_MASK (0x3 << DSPFREQGUAR_SHIFT) +#define DSP_MAXFIFO_PM5_STATUS (1 << 22) /* chv */ +#define DSP_AUTO_CDCLK_GATE_DISABLE (1 << 7) /* chv */ +#define DSP_MAXFIFO_PM5_ENABLE (1 << 6) /* chv */ #define _DP_SSC(val, pipe) ((val) << (2 * (pipe))) #define DP_SSC_MASK(pipe)_DP_SSC(0x3, (pipe)) #define DP_SSC_PWR_ON(pipe) _DP_SSC(0x0, (pipe)) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e6cbc24..4e11552 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -240,7 +240,18 @@ void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable) struct drm_device *dev = dev_priv->dev; u32 val; - if (IS_VALLEYVIEW(dev)) { + if (IS_CHERRYVIEW(dev)) { + I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0); + + mutex_lock(&dev_priv->rps.hw_lock); + val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ); + if (enable) + val |= DSP_MAXFIFO_PM5_ENABLE; + else + val &= ~DSP_MAXFIFO_PM5_ENABLE; + vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val); + mutex_unlock(&dev_priv->rps.hw_lock); + } else if (IS_VALLEYVIEW(dev)) { I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0); } else if (IS_G4X(dev) || IS_CRESTLINE(dev)) { I915_WRITE(FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0); -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/12] drm/i915: Kill DRAIN_LATENCY_PRECISION_* defines
From: Ville Syrjälä Kill the silly DRAIN_LATENCY_PRECISION_* defines and just use the raw number instead. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_pm.c | 12 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d8a0205..230320b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4166,10 +4166,6 @@ enum skl_disp_power_wells { #define DSPFW_PLANEA_WM1_HI_MASK (1<<0) /* drain latency register values*/ -#define DRAIN_LATENCY_PRECISION_8 8 -#define DRAIN_LATENCY_PRECISION_16 16 -#define DRAIN_LATENCY_PRECISION_32 32 -#define DRAIN_LATENCY_PRECISION_64 64 #define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe)) #define DDL_CURSOR_PRECISION_HIGH (1<<31) #define DDL_CURSOR_PRECISION_LOW (0<<31) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a70bce4..0f0281a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -728,11 +728,9 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc, entries = DIV_ROUND_UP(clock, 1000) * pixel_size; if (IS_CHERRYVIEW(dev)) - *prec_mult = (entries > 32) ? DRAIN_LATENCY_PRECISION_16 : - DRAIN_LATENCY_PRECISION_8; + *prec_mult = (entries > 32) ? 16 : 8; else - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 : - DRAIN_LATENCY_PRECISION_32; + *prec_mult = (entries > 128) ? 64 : 32; *drain_latency = (64 * (*prec_mult) * 4) / entries; if (*drain_latency > DRAIN_LATENCY_MASK) @@ -758,8 +756,7 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc) int drain_latency; enum pipe pipe = intel_crtc->pipe; int plane_prec, prec_mult, plane_dl; - const int high_precision = IS_CHERRYVIEW(dev) ? - DRAIN_LATENCY_PRECISION_16 : DRAIN_LATENCY_PRECISION_64; + const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64; plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH | DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH | @@ -957,8 +954,7 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane, int plane_prec; int sprite_dl; int prec_mult; - const int high_precision = IS_CHERRYVIEW(dev) ? - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_64; + const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64; sprite_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_SPRITE_PRECISION_HIGH(sprite) | (DRAIN_LATENCY_MASK << DDL_SPRITE_SHIFT(sprite))); -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/12] drm/i915: Rewrite VLV/CHV watermark code
From: Ville Syrjälä Assuming the PND deadline mechanism works reasonably we should do memory requests as early as possible so that PND has schedule the requests more intelligently. Currently we're still calculating the watermarks as if VLV/CHV are identical to g4x, which isn't the case. The current code also seems to calculate insufficient watermarks and hence we're seeing some underruns, especially on high resolution displays. To fix it just rip out the current code and replace is with something that tries to utilize PND as efficiently as possible. We now calculate the WM watermark to trigger when the FIFO still has 256us worth of data. 256us is the maximum deadline value supoorted by PND, so issuing memory requests earlier would mean we probably couldn't utilize the full FIFO as PND would attempt to return the data at least in at least 256us. We also clamp the watermark to at least 8 cachelines as that's the magic watermark that enabling trickle feed would also impose. I'm assuming it matches some burst size. In theory we could just enable trickle feed and ignore the WM values, except trickle feed doesn't work with max fifo mode anyway, so we'd still need to calculate the SR watermarks. It seems cleaner to just disable trickle feed and calculate all watermarks the same way. Also trickle feed wouldn't account for the 256us max deadline value, thoguh that may be a moot point in non-max fifo mode sicne the FIFOs are fairly small. On VLV max fifo mode can be used with either primary or sprite planes. So the code now also checks all the planes (apart from the cursor) when calculating the SR plane watermark. We don't have to worry about the WM1 watermarks since we're using the PND deadline scheme which means the hardware ignores WM1 values. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 11 ++ drivers/gpu/drm/i915/i915_reg.h | 4 +- drivers/gpu/drm/i915/intel_pm.c | 327 +--- 3 files changed, 185 insertions(+), 157 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5de69a0..b191b12 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1516,6 +1516,17 @@ struct ilk_wm_values { struct vlv_wm_values { struct { + uint16_t primary; + uint16_t sprite[2]; + uint8_t cursor; + } pipe[3]; + + struct { + uint16_t plane; + uint8_t cursor; + } sr; + + struct { uint8_t cursor; uint8_t sprite[2]; uint8_t primary; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5d377df..aacf90b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4127,7 +4127,7 @@ enum skl_disp_power_wells { /* vlv/chv high order bits */ #define DSPHOWM(VLV_DISPLAY_BASE + 0x70064) #define DSPFW_SR_HI_SHIFT24 -#define DSPFW_SR_HI_MASK (1<<24) +#define DSPFW_SR_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */ #define DSPFW_SPRITEF_HI_SHIFT 23 #define DSPFW_SPRITEF_HI_MASK(1<<23) #define DSPFW_SPRITEE_HI_SHIFT 22 @@ -4148,7 +4148,7 @@ enum skl_disp_power_wells { #define DSPFW_PLANEA_HI_MASK (1<<0) #define DSPHOWM1 (VLV_DISPLAY_BASE + 0x70068) #define DSPFW_SR_WM1_HI_SHIFT24 -#define DSPFW_SR_WM1_HI_MASK (1<<24) +#define DSPFW_SR_WM1_HI_MASK (3<<24) /* 2 bits for chv, 1 for vlv */ #define DSPFW_SPRITEF_WM1_HI_SHIFT 23 #define DSPFW_SPRITEF_WM1_HI_MASK(1<<23) #define DSPFW_SPRITEE_WM1_HI_SHIFT 22 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c021e92..d29c02c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -778,6 +778,55 @@ static void vlv_write_wm_values(struct intel_crtc *crtc, (wm->ddl[pipe].sprite[0] << DDL_SPRITE_SHIFT(0)) | (wm->ddl[pipe].primary << DDL_PLANE_SHIFT)); + I915_WRITE(DSPFW1, + ((wm->sr.plane << DSPFW_SR_SHIFT) & DSPFW_SR_MASK) | + ((wm->pipe[PIPE_B].cursor << DSPFW_CURSORB_SHIFT) & DSPFW_CURSORB_MASK) | + ((wm->pipe[PIPE_B].primary << DSPFW_PLANEB_SHIFT) & DSPFW_PLANEB_MASK_VLV) | + ((wm->pipe[PIPE_A].primary << DSPFW_PLANEA_SHIFT) & DSPFW_PLANEA_MASK_VLV)); + I915_WRITE(DSPFW2, + ((wm->pipe[PIPE_A].sprite[1] << DSPFW_SPRITEB_SHIFT) & DSPFW_SPRITEB_MASK_VLV) | + ((wm->pipe[PIPE_A].cursor << DSPFW_CURSORA_SHIFT) & DSPFW_CURSORA_MASK) | + ((wm->pipe[PIPE_A].sprite[0] << DSPFW_SPRITEA_SHIFT) & DSPFW_SPRITEA_MASK_VLV)); + I915_WRITE(DSPFW3, + ((wm->sr.cursor << DSPFW_CURSOR_SR_SHIFT) & DSPFW_CURSOR_SR_MASK)); + + if (IS
[Intel-gfx] [PATCH 08/12] drm/i915: Make sure PND deadline mode is enabled on VLV/CHV
From: Ville Syrjälä Poke at the CBR1_VLV register during init_clock_gating to make sure the PND deadline scheme is used. The hardware has two modes of operation wrt. watermarks: 1) PND deadline mode: - memory request deadline is calculated from actual FIFO level * DDL - WM1 watermark values are unused (AFAIK) - WM watermark level defines when to start fetching data from memory (assuming trickle feed is not used) 2) backup mode - deadline is based on FIFO status, DDL is unused - FIFO split into three regions with WM and WM1 watermarks, each part specifying a different FIFO status We want to use the PND deadline mode, so let's make sure the chicken bit is in the correct position on init. Also take the opportunity to refactor the shared code between VLV and CHV to a shared function. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 19 +-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 433d7e7..5d377df 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4177,6 +4177,9 @@ enum skl_disp_power_wells { #define DDL_PRECISION_LOW (0<<7) #define DRAIN_LATENCY_MASK 0x7f +#define CBR1_VLV (VLV_DISPLAY_BASE + 0x70400) +#define CBR_PND_DEADLINE_DISABLE (1<<31) + /* FIFO watermark sizes etc */ #define G4X_FIFO_LINE_SIZE 64 #define I915_FIFO_LINE_SIZE64 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e53038e..c021e92 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6197,11 +6197,22 @@ static void ivybridge_init_clock_gating(struct drm_device *dev) gen6_check_mch_setup(dev); } +static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv) +{ + I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE); + + /* +* Disable trickle feed and enable pnd deadline calculation +*/ + I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE); + I915_WRITE(CBR1_VLV, 0); +} + static void valleyview_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE); + vlv_init_display_clock_gating(dev_priv); /* WaDisableEarlyCull:vlv */ I915_WRITE(_3D_CHICKEN3, @@ -6249,8 +6260,6 @@ static void valleyview_init_clock_gating(struct drm_device *dev) I915_WRITE(GEN7_UCGCTL4, I915_READ(GEN7_UCGCTL4) | GEN7_L3BANK2X_CLOCK_GATE_DISABLE); - I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE); - /* * BSpec says this must be set, even though * WaDisable4x2SubspanOptimization isn't listed for VLV. @@ -6287,9 +6296,7 @@ static void cherryview_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE); - - I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE); + vlv_init_display_clock_gating(dev_priv); /* WaVSRefCountFullforceMissDisable:chv */ /* WaDSRefCountFullforceMissDisable:chv */ -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx