Re: [Intel-gfx] [PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d
On pe, 2016-06-24 at 14:07 +0100, Chris Wilson wrote: > One of the numerous VT-d workarounds we require is that the display > hardware reads past the end of the buffer triggering VT-d faults. This > is acknowledged in the code as being safe "since we fill the unused > portions of the GGTT with the scratch page". Alas, that is no longer > always true and so we trigger DMAR read faults. > > Skylake also requires another workaround to avoid mixing VT-d and > unpopulated PTE, and so there we also need to ensure we fill unused > entries with the scratch page. Rather agressive W/A for just scanout, so maybe drop the scanout word? > > Reported-by: Mike Lothian > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96584 > Fixes: f7770bfd9fd2 ("drm/i915: Skip clearing the GGTT on full-ppgtt systems") > Signed-off-by: Chris Wilson > Cc: David Weinehall > Cc: Joonas Lahtinen Reviewed-by: Joonas Lahtinen > --- > drivers/gpu/drm/i915/i915_drv.h | 9 + > drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 11 +-- > 3 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 442386abd516..217ca3c9b1c6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2857,6 +2857,15 @@ struct drm_i915_cmd_table { > > #include "i915_trace.h" > > +static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private > *dev_priv) > +{ > +#ifdef CONFIG_INTEL_IOMMU > + if (INTEL_GEN(dev_priv) >= 6 && intel_iommu_gfx_mapped) > + return true; > +#endif > + return false; > +} > + > extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t > state); > extern int i915_resume_switcheroo(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index a8be1c2a8b9e..033fe10768e0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2608,7 +2608,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) > ggtt->base.unbind_vma = ggtt_unbind_vma; > ggtt->base.insert_page = gen8_ggtt_insert_page; > ggtt->base.clear_range = nop_clear_range; > - if (!USES_FULL_PPGTT(dev_priv)) > + if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv)) > ggtt->base.clear_range = gen8_ggtt_clear_range; > > ggtt->base.insert_entries = gen8_ggtt_insert_entries; > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index a8b0ec13dfe9..8145b65c4262 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2071,15 +2071,6 @@ static void intel_disable_pipe(struct intel_crtc *crtc) > intel_wait_for_pipe_off(crtc); > } > > -static bool need_vtd_wa(struct drm_device *dev) > -{ > -#ifdef CONFIG_INTEL_IOMMU > - if (INTEL_INFO(dev)->gen >= 6 && intel_iommu_gfx_mapped) > - return true; > -#endif > - return false; > -} > - > static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv) > { > return IS_GEN2(dev_priv) ? 2048 : 4096; > @@ -2262,7 +2253,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, > unsigned int rotation) > * we should always have valid PTE following the scanout preventing > * the VT-d warning. > */ > - if (need_vtd_wa(dev) && alignment < 256 * 1024) > + if (intel_scanout_needs_vtd_wa(dev_priv) && alignment < 256 * 1024) > alignment = 256 * 1024; > > /* -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d
On Tue, Jul 12, 2016 at 11:50:26AM +0300, Joonas Lahtinen wrote: > On pe, 2016-06-24 at 14:07 +0100, Chris Wilson wrote: > > One of the numerous VT-d workarounds we require is that the display > > hardware reads past the end of the buffer triggering VT-d faults. This > > is acknowledged in the code as being safe "since we fill the unused > > portions of the GGTT with the scratch page". Alas, that is no longer > > always true and so we trigger DMAR read faults. > > > > Skylake also requires another workaround to avoid mixing VT-d and > > unpopulated PTE, and so there we also need to ensure we fill unused > > entries with the scratch page. > > Rather agressive W/A for just scanout, so maybe drop the scanout word? The original w/a was just for VT'd + scanout, I was thinking of a comment, but it never materialised. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH resend xf86-video-intel] Fix fd (and mem) leak when intel_scrn_create fails
The probe functions in intel_module.c call intel_open_device() before calling intel_scrn_create(), but if the later fails because of e.g. an unsupported (new) pci-id they were not cleaning up the resources claimed by intel_open_device(), esp. leaking the fd is a problem because this breaks the fallback to the modesetting driver. This commit fixes this by adding a intel_close_device() cleanup function and calling that when intel_scrn_create() fails. Signed-off-by: Hans de Goede --- src/intel_device.c | 17 + src/intel_driver.h | 1 + src/intel_module.c | 19 --- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/intel_device.c b/src/intel_device.c index 54c1443..71fd851 100644 --- a/src/intel_device.c +++ b/src/intel_device.c @@ -643,6 +643,23 @@ err_path: return -1; } +void intel_close_device(int entity_num) +{ + struct intel_device *dev; + + dev = xf86GetEntityPrivate(entity_num, intel_device_key)->ptr; + + xf86GetEntityPrivate(entity_num, intel_device_key)->ptr = NULL; + + if (dev->master_count == 0) /* Don't close server-fds */ + close(dev->fd); + + if (dev->render_node != dev->master_node) + free(dev->render_node); + free(dev->master_node); + free(dev); +} + int __intel_peek_fd(ScrnInfoPtr scrn) { struct intel_device *dev; diff --git a/src/intel_driver.h b/src/intel_driver.h index fc9beaf..bece88a 100644 --- a/src/intel_driver.h +++ b/src/intel_driver.h @@ -124,6 +124,7 @@ int intel_entity_get_devid(int index); int intel_open_device(int entity_num, const struct pci_device *pci, struct xf86_platform_device *dev); +void intel_close_device(int entity_num); int __intel_peek_fd(ScrnInfoPtr scrn); struct intel_device *intel_get_device(ScrnInfoPtr scrn, int *fd); int intel_has_render_node(struct intel_device *dev); diff --git a/src/intel_module.c b/src/intel_module.c index 60835b9..bfc220d 100644 --- a/src/intel_module.c +++ b/src/intel_module.c @@ -631,6 +631,8 @@ static Bool intel_pci_probe(DriverPtr driver, struct pci_device *pci, intptr_tmatch_data) { + Bool result; + if (intel_open_device(entity_num, pci, NULL) == -1) { #if UMS switch (pci->device_id) { @@ -648,7 +650,11 @@ static Bool intel_pci_probe(DriverPtr driver, #endif } - return intel_scrn_create(driver, entity_num, match_data, 0); + result = intel_scrn_create(driver, entity_num, match_data, 0); + if (!result) + intel_close_device(entity_num); + + return result; } #ifdef XSERVER_PLATFORM_BUS @@ -659,6 +665,7 @@ intel_platform_probe(DriverPtr driver, intptr_t match_data) { unsigned scrn_flags = 0; + Bool result; if (intel_open_device(entity_num, dev->pdev, dev) == -1) return FALSE; @@ -670,10 +677,16 @@ intel_platform_probe(DriverPtr driver, } /* if we get any flags we don't understand fail to probe for now */ - if (flags) + if (flags) { + intel_close_device(entity_num); return FALSE; + } + + result = intel_scrn_create(driver, entity_num, match_data, scrn_flags); + if (!result) + intel_close_device(entity_num); - return intel_scrn_create(driver, entity_num, match_data, scrn_flags); + return result; } #endif -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros
On 11/07/16 19:01, Dave Gordon wrote: We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() provides several other useful intermediate levels such as NOTICE and WARNING. So this patch fills out the set by providing both regular and once-only macros for each of the levels INFO, NOTICE, and WARNING, using a common underlying macro that does all the token-pasting. DRM_ERROR is unchanged, as it's not just a printk wrapper. Signed-off-by: Dave Gordon --- include/drm/drmP.h | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/include/drm/drmP.h b/include/drm/drmP.h index cf918e3e..82648b1 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); /** \name Macros to make printk easier */ /*@{*/ +#define_DRM_PRINTK(once, level, fmt, ...) \ + do {\ + printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ +##__VA_ARGS__);\ + } while (0) + +#define DRM_INFO(fmt, ...) \ + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE(fmt, ...) \ + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) To me DRM_NOTICE would be better to keep consistent with kernel naming for the equivalent log level. +#define DRM_WARN(fmt, ...) \ + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) + +#define DRM_INFO_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__) +#define DRM_NOTE_ONCE(fmt, ...)\ + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) + /** * Error output. * @@ -187,12 +207,6 @@ void drm_err(const char *format, ...); drm_err(fmt, ##__VA_ARGS__);\ }) -#define DRM_INFO(fmt, ...) \ - printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - -#define DRM_INFO_ONCE(fmt, ...)\ - printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - /** * Debug output. * Otherwise acked by me. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/guc: downgrade some DRM_ERROR() messages to DRM_WARN()
On 11/07/16 19:01, Dave Gordon wrote: Where we're going to continue regardless of the problem, rather than fail, then the message should be a WARNing rather than an ERROR. Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/i915_guc_submission.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 2112e02..e299b64 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -114,10 +114,8 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) if (ret != -ETIMEDOUT) ret = -EIO; - DRM_ERROR("GUC: host2guc action 0x%X failed. ret=%d " - "status=0x%08X response=0x%08X\n", - data[0], ret, status, - I915_READ(SOFT_SCRATCH(15))); + DRM_WARN("Action 0x%X failed; ret=%d status=0x%08X response=0x%08X\n", +data[0], ret, status, I915_READ(SOFT_SCRATCH(15))); Hm, this does propagate the error code to the callers some which will act and log the failure. Majority won't though - like suspend/resume etc. In those cases it feels more like an error than a warning. dev_priv->guc.action_fail += 1; dev_priv->guc.action_err = ret; @@ -553,8 +551,8 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) if (db_ret.db_status == GUC_DOORBELL_DISABLED) break; - DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n", - db_cmp.cookie, db_ret.cookie); + DRM_WARN("Cookie mismatch. Expected %d, found %d\n", +db_cmp.cookie, db_ret.cookie); This one is interesting, error is propagated out a bit but then ignored in actual command submission. If the above message means command will not be submitted error is probably more appropriate. Or perhaps we cannot tell if the command was submitted or not in this case? /* update the cookie to newly read cookie from GuC */ db_cmp.cookie = db_ret.cookie; @@ -726,8 +724,8 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) /* Restore to original value */ err = guc_update_doorbell_id(guc, client, db_id); if (err) - DRM_ERROR("Failed to restore doorbell to %d, err %d\n", - db_id, err); + DRM_WARN("Failed to restore doorbell to %d, err %d\n", +db_id, err); for (i = 0; i < GUC_MAX_DOORBELLS; ++i) { i915_reg_t drbreg = GEN8_DRBREGL(i); @@ -819,8 +817,6 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) return client; err: - DRM_ERROR("FAILED to create priority %u GuC client!\n", priority); - guc_client_free(dev_priv, client); return NULL; } @@ -998,7 +994,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv) GUC_CTX_PRIORITY_KMD_NORMAL, dev_priv->kernel_context); if (!client) { - DRM_ERROR("Failed to create execbuf guc_client\n"); + DRM_ERROR("Failed to create normal GuC client!\n"); return -ENOMEM; } Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/guc: revisit GuC loader message levels
On 11/07/16 19:01, Dave Gordon wrote: Some downgraded from DRM_ERROR() to DRM_WARN(), some eliminated, and a few upgraded from DRM_INFO() to DRM_NOTE() or DRM_WARN(). Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/intel_guc_loader.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 605c696..fd032eb 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -140,12 +140,14 @@ static u32 get_gttype(struct drm_i915_private *dev_priv) static u32 get_core_family(struct drm_i915_private *dev_priv) { - switch (INTEL_INFO(dev_priv)->gen) { + u32 gen = INTEL_GEN(dev_priv); + + switch (gen) { case 9: return GFXCORE_FAMILY_GEN9; default: - DRM_ERROR("GUC: unsupported core family\n"); + DRM_WARN("GEN%d does not support GuC operation\n", gen); return GFXCORE_FAMILY_UNKNOWN; } } @@ -433,7 +435,7 @@ int intel_guc_setup(struct drm_device *dev) goto fail; } else if (*fw_path == '\0') { /* Device has a GuC but we don't know what f/w to load? */ - DRM_INFO("No GuC firmware known for this platform\n"); + DRM_WARN("No GuC firmware known for this platform\n"); err = -ENODEV; goto fail; } @@ -471,10 +473,8 @@ int intel_guc_setup(struct drm_device *dev) * that the state and timing are fairly predictable */ err = i915_reset_guc(dev_priv); - if (err) { - DRM_ERROR("GuC reset failed: %d\n", err); + if (err) goto fail; - } err = guc_ucode_xfer(dev_priv); if (!err) @@ -532,15 +532,15 @@ int intel_guc_setup(struct drm_device *dev) else if (err == 0) DRM_INFO("GuC firmware load skipped\n"); else if (ret != -EIO) - DRM_INFO("GuC firmware load failed: %d\n", err); + DRM_NOTE("GuC firmware load failed: %d\n", err); else - DRM_ERROR("GuC firmware load failed: %d\n", err); + DRM_WARN("GuC firmware load failed: %d\n", err); if (i915.enable_guc_submission) { if (fw_path == NULL) DRM_INFO("GuC submission without firmware not supported\n"); if (ret == 0) - DRM_INFO("Falling back from GuC submission to execlist mode\n"); + DRM_NOTE("Falling back from GuC submission to execlist mode\n"); else DRM_ERROR("GuC init failed: %d\n", ret); } @@ -656,7 +656,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw) fail: DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n", err, fw, guc_fw->guc_fw_obj); - DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n", + DRM_WARN("Failed to fetch GuC firmware from %s (error %d)\n", guc_fw->guc_fw_path, err); mutex_lock(&dev->struct_mutex); R-b if you also change all the other DRM_ERRORs in guc_fw_fetch to DRM_DEBUG_DRIVER and merge this last two log lines (DRM_DEBUG_DRIVER + DRM_WARN) to one. :) Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/guc: downgrade some DRM_ERROR() messages to DRM_WARN()
On Tue, Jul 12, 2016 at 10:20:43AM +0100, Tvrtko Ursulin wrote: > On 11/07/16 19:01, Dave Gordon wrote: > >@@ -553,8 +551,8 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) > > if (db_ret.db_status == GUC_DOORBELL_DISABLED) > > break; > > > >-DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n", > >- db_cmp.cookie, db_ret.cookie); > >+DRM_WARN("Cookie mismatch. Expected %d, found %d\n", > >+ db_cmp.cookie, db_ret.cookie); > > This one is interesting, error is propagated out a bit but then > ignored in actual command submission. > > If the above message means command will not be submitted error is > probably more appropriate. Or perhaps we cannot tell if the command > was submitted or not in this case? It's insignificant. An actual error would result in a GPU hang, and without being recorded in the error state any message here is useless. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915/guc: downgrade some DRM_ERROR() messages to DRM_WARN()
On 12/07/16 10:27, Chris Wilson wrote: On Tue, Jul 12, 2016 at 10:20:43AM +0100, Tvrtko Ursulin wrote: On 11/07/16 19:01, Dave Gordon wrote: @@ -553,8 +551,8 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) if (db_ret.db_status == GUC_DOORBELL_DISABLED) break; - DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n", - db_cmp.cookie, db_ret.cookie); + DRM_WARN("Cookie mismatch. Expected %d, found %d\n", +db_cmp.cookie, db_ret.cookie); This one is interesting, error is propagated out a bit but then ignored in actual command submission. If the above message means command will not be submitted error is probably more appropriate. Or perhaps we cannot tell if the command was submitted or not in this case? It's insignificant. An actual error would result in a GPU hang, and without being recorded in the error state any message here is useless. I don't agree that it is useless, if it is a very unexpected situation it deserves to be logged. People do store and look at logs when things go bad. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Bad flicker on skylake HQD due to code in the 4.7 merge window
On Fri, Jun 24, 2016 at 01:42:06PM +0100, Chris Wilson wrote: > On Fri, Jun 24, 2016 at 12:48:17PM +0100, Steven Newbury wrote: > > On Fri, 2016-06-24 at 11:59 +0100, Chris Wilson wrote: > > > On Thu, Jun 23, 2016 at 02:14:12PM +0100, Steven Newbury wrote: > > > > On Thu, 2016-06-23 at 15:59 +0300, Jani Nikula wrote: > > > > > On Thu, 23 Jun 2016, Steven Newbury > > > > > wrote: > > > > > > I'm seeing this on my IvyBridge. I'll try reverting the commit > > > > > > here > > > > > > too, to see if it's the same issue. > > > > > > > > > > IvyBridge doesn't have low vswing for eDP. If reverting helps, > > > > > it's a > > > > > different failure mode. > > > > > > > > > It must be something else then. Actually, in my case linus/master > > > > is > > > > okay. I saw the subject and though it must be the same issue. I'm > > > > seeing it with drm-intel nightly/next branches. Shall I try to > > > > bisect > > > > it? Symptoms are similar, although I would describe it more like > > > > flashes of a different buffer across parts of the screen. > > > > > > Try reverting ee042aa40b66d18d465206845b0752c6a617ba3f instead. > > > -Chris > > > > > > > Yes, thanks, that "fixed" it. So atomic commits not working properly > > on IVB? > > Not yet it seems. Something seems to be off in the timing, but has so > far eluded capture. Fyi we reverted this for now to fix things up: commit 527b6abe5fd2d24fba69e9564a2d608e1796ca8d Author: Chris Wilson Date: Fri Jun 24 13:44:03 2016 +0100 Revert "drm/i915: Use atomic commits for legacy page_flips" Kudos to Chris for handling this in my absence. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 0/2] drm/i915/opregion: proper handling of DIDL and CADL
Op 01-07-16 om 15:51 schreef Jani Nikula: > On Fri, 01 Jul 2016, Rainer Koenig wrote: >> Found a problem: After screensaver kicked in and display was turned off >> the brightness keys stop working. >> >> Problem can be reproduced like that: >> >> 1. Boot laptop >> 2. Test brightness keys, they are working >> 3. open Terminal and issue "xset -display :0 dpms force off" >> 4. the screen goes blank (like after the screensaver timeout) >> 5. push a key to bring the screen back >> 6. test brightness keys again, now they don't work >> >> If the system is sent to suspend and woken up everything is fine again. >> >> Behaviour happens on the 4.7.0-rc5 kernel from the opregion-didl-v4 branch. >> Before I compiled the 4.7.0-r4 from the same git repository. On this >> (v3) everything still works after the screen was blanked. > Maarten, I think the difference is between where and when the calls to > cadl update are made. Did you see my comment on patch 2? ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix legacy gamma lut updates in Linux 4.7-rc6
Updating legacy gamma tables, e.g., via RandR doesn't work at all as of Linux 4.7-rc6. Reason seems to be that the required call to drm_atomic_helper_commit_planes_on_crtc is skipped in intel_atomic_commit after userspace set new gamma tables, because neither crtc->state->planes_changed nor update_pipe (= pipe_config->update_pipe) are true. Removing the check for planes_changed || update_pipe fixes gamma table updates. The code for Linux 4.8 drm-next has changed a lot in that area wrt. 4.7, but the new code for 4.8 also removed those checks and calls drm_atomic_helper_commit_planes_on_crtc unconditionally, and legacy gamma lut updates work on drm-next, so this seems to be the right solution. Tested also shutdown/reboot, suspend/resume, (un-)plugging displays, mode switches for resolution/refresh rate, display rotation, and page-flipping/pageflip timing on Intel HD Ironlake to confirm the fix apparently doesn't break anything under X11. Signed-off-by: Mario Kleiner Cc: Maarten Lankhorst Cc: Lionel Landwerlin Cc: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 04452cf..eb8fb36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13685,7 +13685,6 @@ static int intel_atomic_commit(struct drm_device *dev, bool modeset = needs_modeset(crtc->state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state); - bool update_pipe = !modeset && pipe_config->update_pipe; if (modeset && crtc->state->active) { update_scanline_offset(to_intel_crtc(crtc)); @@ -13699,8 +13698,7 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_get_existing_plane_state(state, crtc->primary)) intel_fbc_enable(intel_crtc); - if (crtc->state->active && - (crtc->state->planes_changed || update_pipe)) + if (crtc->state->active) drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); if (pipe_config->base.active && needs_vblank_wait(pipe_config)) -- 2.7.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)
On Mon, Jul 11, 2016 at 04:24:45PM +0100, Chris Wilson wrote: > On Mon, Jul 11, 2016 at 12:10:40PM -0300, Gustavo Padovan wrote: > > 2016-07-11 Chris Wilson : > > > > > vGEM buffers are useful for passing data between software clients and > > > hardware renders. By allowing the user to create and attach fences to > > > the exported vGEM buffers (on the dma-buf), the user can implement a > > > deferred renderer and queue hardware operations like flipping and then > > > signal the buffer readiness (i.e. this allows the user to schedule > > > operations out-of-order, but have them complete in-order). > > > > > > This also makes it much easier to write tightly controlled testcases for > > > dma-buf fencing and signaling between hardware drivers. > > > > > > Testcase: igt/vgem_basic/dmabuf-fence > > > Signed-off-by: Chris Wilson > > > Cc: Sean Paul > > > Cc: Zach Reizner > > > Cc: Gustavo Padovan > > > Acked-by: Zach Reizner > > > --- > > > drivers/gpu/drm/vgem/Makefile | 2 +- > > > drivers/gpu/drm/vgem/vgem_drv.c | 34 ++ > > > drivers/gpu/drm/vgem/vgem_drv.h | 18 > > > drivers/gpu/drm/vgem/vgem_fence.c | 220 > > > ++ > > > include/uapi/drm/vgem_drm.h | 62 +++ > > > 5 files changed, 335 insertions(+), 1 deletion(-) > > > create mode 100644 drivers/gpu/drm/vgem/vgem_fence.c > > > create mode 100644 include/uapi/drm/vgem_drm.h > > > > > > diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile > > > index 3f4c7b842028..bfcdea1330e6 100644 > > > --- a/drivers/gpu/drm/vgem/Makefile > > > +++ b/drivers/gpu/drm/vgem/Makefile > > > @@ -1,4 +1,4 @@ > > > ccflags-y := -Iinclude/drm > > > -vgem-y := vgem_drv.o > > > +vgem-y := vgem_drv.o vgem_fence.o > > > > > > obj-$(CONFIG_DRM_VGEM) += vgem.o > > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c > > > b/drivers/gpu/drm/vgem/vgem_drv.c > > > index b5fb968d2d5c..2659e5cda857 100644 > > > --- a/drivers/gpu/drm/vgem/vgem_drv.c > > > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > > > @@ -83,6 +83,34 @@ static const struct vm_operations_struct > > > vgem_gem_vm_ops = { > > > .close = drm_gem_vm_close, > > > }; > > > > > > +static int vgem_open(struct drm_device *dev, struct drm_file *file) > > > +{ > > > + struct vgem_file *vfile; > > > + int ret; > > > + > > > + vfile = kzalloc(sizeof(*vfile), GFP_KERNEL); > > > + if (!vfile) > > > + return -ENOMEM; > > > + > > > + file->driver_priv = vfile; > > > + > > > + ret = vgem_fence_open(vfile); > > > + if (ret) { > > > + kfree(vfile); > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static void vgem_preclose(struct drm_device *dev, struct drm_file *file) > > > +{ > > > + struct vgem_file *vfile = file->driver_priv; > > > + > > > + vgem_fence_close(vfile); > > > + kfree(vfile); > > > +} > > > + > > > /* ioctls */ > > > > > > static struct drm_gem_object *vgem_gem_create(struct drm_device *dev, > > > @@ -164,6 +192,8 @@ unref: > > > } > > > > > > static struct drm_ioctl_desc vgem_ioctls[] = { > > > + DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl, > > > DRM_AUTH|DRM_RENDER_ALLOW), > > > + DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, > > > DRM_AUTH|DRM_RENDER_ALLOW), > > > }; > > > > > > static int vgem_mmap(struct file *filp, struct vm_area_struct *vma) > > > @@ -271,9 +301,12 @@ static int vgem_prime_mmap(struct drm_gem_object > > > *obj, > > > > > > static struct drm_driver vgem_driver = { > > > .driver_features= DRIVER_GEM | DRIVER_PRIME, > > > + .open = vgem_open, > > > + .preclose = vgem_preclose, > > > .gem_free_object_unlocked = vgem_gem_free_object, > > > .gem_vm_ops = &vgem_gem_vm_ops, > > > .ioctls = vgem_ioctls, > > > + .num_ioctls = ARRAY_SIZE(vgem_ioctls), > > > .fops = &vgem_driver_fops, > > > > > > .dumb_create= vgem_gem_dumb_create, > > > @@ -328,5 +361,6 @@ module_init(vgem_init); > > > module_exit(vgem_exit); > > > > > > MODULE_AUTHOR("Red Hat, Inc."); > > > +MODULE_AUTHOR("Intel Corporation"); > > > MODULE_DESCRIPTION(DRIVER_DESC); > > > MODULE_LICENSE("GPL and additional rights"); > > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.h > > > b/drivers/gpu/drm/vgem/vgem_drv.h > > > index 988cbaae7588..88ce21010e28 100644 > > > --- a/drivers/gpu/drm/vgem/vgem_drv.h > > > +++ b/drivers/gpu/drm/vgem/vgem_drv.h > > > @@ -32,9 +32,27 @@ > > > #include > > > #include > > > > > > +#include > > > + > > > +struct vgem_file { > > > + struct idr fence_idr; > > > + struct mutex fence_mutex; > > > + u64 fence_context; > > > + atomic_t fence_seqno; > > > +}; > > > + > > > #define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base) > > > struct drm_vgem_gem_object { > > > struct drm_gem_object base; > >
Re: [Intel-gfx] [PATCH 32/44] drm/i915: Move module init/exit to i915_pci.c
On ke, 2016-06-15 at 13:18 +0100, Chris Wilson wrote: > The module init/exit routines are a wrapper around the PCI device > init/exit, so move them across. > > Note that in order to avoid exporting the driver struct, instead of > manipulating driver.features inside i915_init we instead opt to simply > exit if i915.modeset is disabled. > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Joonas Lahtinen Reviewed-by: Joonas Lahtinen > --- > drivers/gpu/drm/i915/i915_drv.c | 51 > +++-- > drivers/gpu/drm/i915/i915_pci.c | 45 +++- > 2 files changed, 47 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index f65556b94978..8836a3d24460 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -28,7 +28,6 @@ > */ > > #include > -#include > #include > #include > #include > @@ -1587,6 +1586,9 @@ int i915_driver_load(struct pci_dev *pdev, const struct > pci_device_id *ent) > struct drm_i915_private *dev_priv; > int ret; > > + if (i915.nuclear_pageflip) > + driver.driver_features |= DRIVER_ATOMIC; > + > ret = 0; > dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); > if (dev_priv) > @@ -3013,50 +3015,3 @@ static struct drm_driver driver = { > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > }; > - > -static int __init i915_init(void) > -{ > - extern struct pci_driver i915_pci_driver; > - > - /* > - * Enable KMS by default, unless explicitly overriden by > - * either the i915.modeset prarameter or by the > - * vga_text_mode_force boot option. > - */ > - > - if (i915.modeset == 0) > - driver.driver_features &= ~DRIVER_MODESET; > - > - if (vgacon_text_force() && i915.modeset == -1) > - driver.driver_features &= ~DRIVER_MODESET; > - > - if (!(driver.driver_features & DRIVER_MODESET)) { > - /* Silently fail loading to not upset userspace. */ > - DRM_DEBUG_DRIVER("KMS and UMS disabled.\n"); > - return 0; > - } > - > - if (i915.nuclear_pageflip) > - driver.driver_features |= DRIVER_ATOMIC; > - > - return drm_pci_init(&driver, &i915_pci_driver); > -} > - > -static void __exit i915_exit(void) > -{ > - extern struct pci_driver i915_pci_driver; > - > - if (!(driver.driver_features & DRIVER_MODESET)) > - return; /* Never loaded a driver. */ > - > - drm_pci_exit(&driver, &i915_pci_driver); > -} > - > -module_init(i915_init); > -module_exit(i915_exit); > - > -MODULE_AUTHOR("Tungsten Graphics, Inc."); > -MODULE_AUTHOR("Intel Corporation"); > - > -MODULE_DESCRIPTION(DRIVER_DESC); > -MODULE_LICENSE("GPL and additional rights"); > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 9aa07f38caea..14fc456bc637 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -22,6 +22,7 @@ > * > */ > > +#include > #include > #include > > @@ -452,10 +453,52 @@ static void i915_pci_remove(struct pci_dev *pdev) > > extern const struct dev_pm_ops i915_pm_ops; > > -struct pci_driver i915_pci_driver = { > +static struct pci_driver i915_pci_driver = { > .name = DRIVER_NAME, > .id_table = pciidlist, > .probe = i915_pci_probe, > .remove = i915_pci_remove, > .driver.pm = &i915_pm_ops, > }; > + > +static int __init i915_init(void) > +{ > + bool use_kms = true; > + > + /* > + * Enable KMS by default, unless explicitly overriden by > + * either the i915.modeset prarameter or by the > + * vga_text_mode_force boot option. > + */ > + > + if (i915.modeset == 0) > + use_kms = false; > + > + if (vgacon_text_force() && i915.modeset == -1) > + use_kms = false; > + > + if (!use_kms) { > + /* Silently fail loading to not upset userspace. */ > + DRM_DEBUG_DRIVER("KMS and UMS disabled.\n"); > + return 0; > + } > + > + return pci_register_driver(&i915_pci_driver); > +} > + > +static void __exit i915_exit(void) > +{ > + if (!i915_pci_driver.driver.owner) > + return; > + > + pci_unregister_driver(&i915_pci_driver); > +} > + > +module_init(i915_init); > +module_exit(i915_exit); > + > +MODULE_AUTHOR("Tungsten Graphics, Inc."); > +MODULE_AUTHOR("Intel Corporation"); > + > +MODULE_DESCRIPTION(DRIVER_DESC); > +MODULE_LICENSE("GPL and additional rights"); -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/vgem: Enable dmabuf interface for export
On Mon, Jul 11, 2016 at 02:08:07PM +0100, Chris Wilson wrote: > Enable the standard GEM dma-buf interface provided by the DRM core, but > only for exporting the VGEM object. This allows passing around the VGEM > objects created from the dumb interface and using them as sources > elsewhere. Creating a VGEM object for a foriegn handle is not supported. > > v2: With additional completeness. > v3: Need to clear the CPU cache upon exporting the dma-addresses. > v4: Use drm_gem_put_pages() as well. > v5: Use drm_prime_pages_to_sg() > > Testcase: igt/vgem_basic/dmabuf-* > Testcase: igt/prime_vgem > Signed-off-by: Chris Wilson > Cc: Sean Paul > Cc: Zach Reizner > Acked-by: Zach Reizner > Reviewed-by: Matthew Auld Merged the first 2 patches from this series. -Daniel > --- > drivers/gpu/drm/vgem/vgem_drv.c | 89 > - > 1 file changed, 88 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c > index c161b6d7e427..b5fb968d2d5c 100644 > --- a/drivers/gpu/drm/vgem/vgem_drv.c > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > @@ -192,14 +192,101 @@ static const struct file_operations vgem_driver_fops = > { > .release= drm_release, > }; > > +static int vgem_prime_pin(struct drm_gem_object *obj) > +{ > + long n_pages = obj->size >> PAGE_SHIFT; > + struct page **pages; > + > + /* Flush the object from the CPU cache so that importers can rely > + * on coherent indirect access via the exported dma-address. > + */ > + pages = drm_gem_get_pages(obj); > + if (IS_ERR(pages)) > + return PTR_ERR(pages); > + > + drm_clflush_pages(pages, n_pages); > + drm_gem_put_pages(obj, pages, true, false); > + > + return 0; > +} > + > +static struct sg_table *vgem_prime_get_sg_table(struct drm_gem_object *obj) > +{ > + struct sg_table *st; > + struct page **pages; > + > + pages = drm_gem_get_pages(obj); > + if (IS_ERR(pages)) > + return ERR_CAST(pages); > + > + st = drm_prime_pages_to_sg(pages, obj->size >> PAGE_SHIFT); > + drm_gem_put_pages(obj, pages, false, false); > + > + return st; > +} > + > +static void *vgem_prime_vmap(struct drm_gem_object *obj) > +{ > + long n_pages = obj->size >> PAGE_SHIFT; > + struct page **pages; > + void *addr; > + > + pages = drm_gem_get_pages(obj); > + if (IS_ERR(pages)) > + return NULL; > + > + addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); > + drm_gem_put_pages(obj, pages, false, false); > + > + return addr; > +} > + > +static void vgem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) > +{ > + vunmap(vaddr); > +} > + > +static int vgem_prime_mmap(struct drm_gem_object *obj, > +struct vm_area_struct *vma) > +{ > + int ret; > + > + if (obj->size < vma->vm_end - vma->vm_start) > + return -EINVAL; > + > + if (!obj->filp) > + return -ENODEV; > + > + ret = obj->filp->f_op->mmap(obj->filp, vma); > + if (ret) > + return ret; > + > + fput(vma->vm_file); > + vma->vm_file = get_file(obj->filp); > + vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; > + vma->vm_page_prot = > pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); > + > + return 0; > +} > + > static struct drm_driver vgem_driver = { > - .driver_features= DRIVER_GEM, > + .driver_features= DRIVER_GEM | DRIVER_PRIME, > .gem_free_object_unlocked = vgem_gem_free_object, > .gem_vm_ops = &vgem_gem_vm_ops, > .ioctls = vgem_ioctls, > .fops = &vgem_driver_fops, > + > .dumb_create= vgem_gem_dumb_create, > .dumb_map_offset= vgem_gem_dumb_map, > + > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > + .gem_prime_pin = vgem_prime_pin, > + .gem_prime_export = drm_gem_prime_export, > + .gem_prime_get_sg_table = vgem_prime_get_sg_table, > + .gem_prime_vmap = vgem_prime_vmap, > + .gem_prime_vunmap = vgem_prime_vunmap, > + .gem_prime_mmap = vgem_prime_mmap, > + > .name = DRIVER_NAME, > .desc = DRIVER_DESC, > .date = DRIVER_DATE, > -- > 2.8.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Update ifdeffery for mutex->owner
Reviewed-by: Matthew Auld ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix legacy gamma lut updates in Linux 4.7-rc6
Hi Mario, There was a couple of patch to fix this issue : https://patchwork.freedesktop.org/series/5467/ https://patchwork.freedesktop.org/series/5466/ I tested this late last week on drm-intel-nightly, it seems a series of revert fixed most of the issues. Cheers, - Lionel On 12/07/16 11:33, Mario Kleiner wrote: Updating legacy gamma tables, e.g., via RandR doesn't work at all as of Linux 4.7-rc6. Reason seems to be that the required call to drm_atomic_helper_commit_planes_on_crtc is skipped in intel_atomic_commit after userspace set new gamma tables, because neither crtc->state->planes_changed nor update_pipe (= pipe_config->update_pipe) are true. Removing the check for planes_changed || update_pipe fixes gamma table updates. The code for Linux 4.8 drm-next has changed a lot in that area wrt. 4.7, but the new code for 4.8 also removed those checks and calls drm_atomic_helper_commit_planes_on_crtc unconditionally, and legacy gamma lut updates work on drm-next, so this seems to be the right solution. Tested also shutdown/reboot, suspend/resume, (un-)plugging displays, mode switches for resolution/refresh rate, display rotation, and page-flipping/pageflip timing on Intel HD Ironlake to confirm the fix apparently doesn't break anything under X11. Signed-off-by: Mario Kleiner Cc: Maarten Lankhorst Cc: Lionel Landwerlin Cc: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 04452cf..eb8fb36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13685,7 +13685,6 @@ static int intel_atomic_commit(struct drm_device *dev, bool modeset = needs_modeset(crtc->state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state); - bool update_pipe = !modeset && pipe_config->update_pipe; if (modeset && crtc->state->active) { update_scanline_offset(to_intel_crtc(crtc)); @@ -13699,8 +13698,7 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_get_existing_plane_state(state, crtc->primary)) intel_fbc_enable(intel_crtc); - if (crtc->state->active && - (crtc->state->planes_changed || update_pipe)) + if (crtc->state->active) drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); if (pipe_config->base.active && needs_vblank_wait(pipe_config)) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set the access right of kernel param "i915.enable_gvt" to read-only.
On ma, 2016-06-20 at 08:17 -0400, Zhi Wang wrote: > The access right of kernel param "i915.enable_gvt" should be read-only as > it only applies during module load (and is not *runtime* writable). > > Cc: Chris Wilson Reviewed-by: Joonas Lahtinen Maybe also add, Suggested-by: Chris Wilson > Signed-off-by: Zhi Wang > --- > drivers/gpu/drm/i915/i915_params.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c > b/drivers/gpu/drm/i915/i915_params.c > index 7effe68..8b13bfa 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -224,6 +224,6 @@ module_param_named(enable_dpcd_backlight, > i915.enable_dpcd_backlight, bool, 0600 > MODULE_PARM_DESC(enable_dpcd_backlight, > "Enable support for DPCD backlight control (default:false)"); > > -module_param_named(enable_gvt, i915.enable_gvt, bool, 0600); > +module_param_named(enable_gvt, i915.enable_gvt, bool, 0400); > MODULE_PARM_DESC(enable_gvt, > "Enable support for Intel GVT-g graphics virtualization host > support(default:false)"); -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Update ifdeffery for mutex->owner
On Tue, Jul 12, 2016 at 11:48:09AM +0100, Matthew Auld wrote: > Reviewed-by: Matthew Auld Thanks, pushed. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)
On Tue, Jul 12, 2016 at 12:44:17PM +0200, Daniel Vetter wrote: > On Mon, Jul 11, 2016 at 04:24:45PM +0100, Chris Wilson wrote: > > That doesn't fit the out-of-order unbound nature of the interface. The > > interface is just a collection of fences that userspace associates with > > the buffer that it may signal at any time. (Having no strict timeline is > > an advantage!) > > Fences on the same timeline are supposed to be signalled in-order. If you > want full out-of-order fences then you need to grab a new timeline number > for each one. Drivers can and do merge fences on the same timeline and > just carry the one with the largest seqno around. Ugh. Timelines simply don't mean anything everywhere - a very leaky abstration. Nevertheless, a fence_context per vgem_fence would do the trick. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros
On Mon, Jul 11, 2016 at 07:01:27PM +0100, Dave Gordon wrote: > We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() > provides several other useful intermediate levels such as NOTICE and > WARNING. So this patch fills out the set by providing both regular and > once-only macros for each of the levels INFO, NOTICE, and WARNING, using > a common underlying macro that does all the token-pasting. > > DRM_ERROR is unchanged, as it's not just a printk wrapper. > > Signed-off-by: Dave Gordon > --- > include/drm/drmP.h | 26 -- > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index cf918e3e..82648b1 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); > /** \name Macros to make printk easier */ > /*@{*/ > > +#define _DRM_PRINTK(once, level, fmt, ...) > \ Tab after `#define`? > + do {\ > + printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ > + ##__VA_ARGS__);\ > + } while (0) > + > +#define DRM_INFO(fmt, ...) \ > + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) > +#define DRM_NOTE(fmt, ...) \ > + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) > +#define DRM_WARN(fmt, ...) \ > + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) > + > +#define DRM_INFO_ONCE(fmt, ...) > \ > + _DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__) Missing ## here; should be: ##__VA_ARGS__ The rest looks good. Reviewed-by: Eric Engestrom > +#define DRM_NOTE_ONCE(fmt, ...) \ > + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) > +#define DRM_WARN_ONCE(fmt, ...) > \ > + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) > + > /** > * Error output. > * > @@ -187,12 +207,6 @@ void drm_err(const char *format, ...); > drm_err(fmt, ##__VA_ARGS__);\ > }) > > -#define DRM_INFO(fmt, ...) \ > - printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > - > -#define DRM_INFO_ONCE(fmt, ...) \ > - printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > - > /** > * Debug output. > * > -- > 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions
On pe, 2016-07-01 at 14:47 -0700, Francisco Jerez wrote: > Hi Imre, > > Imre Deak writes: > > > Use named struct initializers for clarity. Also fix the target > > cache > > definition to reflect its role in GEN9 onwards. On GEN8 a TC value > > of 0 > > meant ELLC but on GEN9+ it means the TC and LRU controls are taken > > from > > the PTE. > > > I vaguely recall bringing up this apparent inconsistency with the > hardware spec during review of the original series of patches > programming the MOCS tables. IIRC TC value 0 behaved as ELLC-only on > the simulator, but the hardware docs claimed it would take the target > cache from the PTE controls, not sure what the real hardware actually > does. At least the documentation seems to be quite consistent on this. It's true that you have to set the MOCS TC to 0 for an eLLC-only mapping, but that depends on the (default) 0 of the PTE [3:2] field (override to eLLC-only). So maybe during simulation PTE was at the default eLLC- only, hence your observed behavior. In any case we don't use TC PTE passthrough for any MOCS index atm, so I would suggest keeping the code in line with the documentation for now. --Imre > Maybe Peter can confirm whether this was intentional? > > > No functional change, igt/gem_mocs_settings still passing after > > this > > change. > > > > v2: (Chris) > > - Add back the hexa literals for the entries. > > Add note that igt/gem_mocs_settings still passes. > > > > CC: Rong R Yang > > CC: Yakui Zhao > > CC: Chris Wilson > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/intel_mocs.c | 88 > > +++ > > 1 file changed, 61 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c > > b/drivers/gpu/drm/i915/intel_mocs.c > > index 3c1482b..d36e609 100644 > > --- a/drivers/gpu/drm/i915/intel_mocs.c > > +++ b/drivers/gpu/drm/i915/intel_mocs.c > > @@ -66,9 +66,10 @@ struct drm_i915_mocs_table { > > #define L3_WB 3 > > > > /* Target cache */ > > -#define ELLC 0 > > -#define LLC1 > > -#define LLC_ELLC 2 > > +#define LE_TC_PAGETABLE0 > > +#define LE_TC_LLC 1 > > +#define LE_TC_LLC_ELLC 2 > > +#define LE_TC_LLC_ELLC_ALT 3 > > > > /* > > * MOCS tables > > @@ -96,34 +97,67 @@ struct drm_i915_mocs_table { > > * end. > > */ > > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > - /* { 0x0009, 0x0010 } */ > > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | > > LE_LRUM(0) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > > - /* { 0x0038, 0x0030 } */ > > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) > > | LE_LRUM(3) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > > - /* { 0x003b, 0x0030 } */ > > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | > > LE_LRUM(3) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > > + { /* 0x0009 */ > > + .control_value = LE_CACHEABILITY(LE_UC) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + > > + /* 0x0010 */ > > + .l3cc_value =L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_UC), > > + }, > > + { > > + /* 0x0038 */ > > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + /* 0x0030 */ > > + .l3cc_value =L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_WB), > > + }, > > + { > > + /* 0x003b */ > > + .control_value = LE_CACHEABILITY(LE_WB) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + /* 0x0030 */ > > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_WB), > > + }, > > }; > > > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > - /* { 0x0009, 0x0010 } */ > > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | > > LE_LRUM(0) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > > - /* { 0x0038, 0x0030 } */ > > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) > > | LE_LRUM(3) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM
[Intel-gfx] [drm-intel:topic/drm-misc 2/2] drivers/gpu/drm/vgem/vgem_drv.c:238:53: error: 'PAGE_KERNEL_IO' undeclared
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 commit: e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 [2/2] drm/vgem: Enable dmabuf interface for export config: sparc64-allmodconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 # save the attached .config to linux build tree make.cross ARCH=sparc64 All errors (new ones prefixed by >>): drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_prime_vmap': >> drivers/gpu/drm/vgem/vgem_drv.c:238:53: error: 'PAGE_KERNEL_IO' undeclared >> (first use in this function) addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ drivers/gpu/drm/vgem/vgem_drv.c:238:53: note: each undeclared identifier is reported only once for each function it appears in vim +/PAGE_KERNEL_IO +238 drivers/gpu/drm/vgem/vgem_drv.c 232 void *addr; 233 234 pages = drm_gem_get_pages(obj); 235 if (IS_ERR(pages)) 236 return NULL; 237 > 238 addr = vmap(pages, n_pages, 0, > pgprot_writecombine(PAGE_KERNEL_IO)); 239 drm_gem_put_pages(obj, pages, false, false); 240 241 return addr; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Check live status before reading edid
On Thu, Jul 09, 2015 at 07:27:57PM +0200, Daniel Vetter wrote: > On Thu, Jul 09, 2015 at 05:34:29PM +0530, Sonika Jindal wrote: > > Adding this for SKL onwards. > > > > Signed-off-by: Sonika Jindal > > I think this is the critical piece really, and I'd like to roll it out for > all platforms. git has the code for all the old ones, so no big deal to > digg that out. Also we need your fix for the interrupt handling first ofc, > otherwise this won't work. > > Then we can apply this and give it some good testing before we start > relying on it with caching hdmi probe status. I know this means that > things will be slower, but I've been burned a bit too much the last few > times we've tried this. And I really think we need the most amount of > testing we can get (hence rolling this out for all platforms). If your > hpd irq handling bugfix is indeed the bug that will be confirmed quickly, > otherwise it means back to debugging. "things will be slower" is a bit of an understatement, sadly. On machines with no external display connected (the typical usecase for any device with an eDP, such as a laptop, tablet, etc.), the approach to repeat live status reads until buggy displays can be bothered to reply makes us spend twice as long as needed on resume. While it's nice that we can support buggy hardware, I think that we also, at the very least, should allow for skipping said those workarounds when not needed. Either by allow for disabling the lvie status reads (proven to work on older platforms since that was the default behaviour for a long, long time, and tested to work on at least Broadwell & Skylake ThinkPads) or by making the number of LSR reads configurable. The former would be the simplest solution, the latter would meet the letter of the spec, and allow for more precise tuning of behaviour; people who have a display that needs -- say -- for LSR reads to work reliably shouldn't have to wait for 9. While allowing for unusual use-cases / buggy hardware is great, we shouldn't miss out on the benefits that working hardware can give to the common use-cases. Just my 2¢. I'm feeling sorely tempted to treat this as a proper regression, and simply submit a revert patch, seeing as it slows down resume by something like 200ms, but seeing as there was mention of a case where incorrect EDID-information might end up being read after resume on some Chromebooks, I'll just try to open a discussion instead. Kind regards, David Weinehall ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-intel:topic/drm-misc 2/2] drivers/gpu/drm/vgem/vgem_drv.c:238:53: error: 'PAGE_KERNEL_IO' undeclared
On Tue, Jul 12, 2016 at 07:23:55PM +0800, kbuild test robot wrote: > tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc > head: e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 > commit: e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 [2/2] drm/vgem: Enable > dmabuf interface for export > config: sparc64-allmodconfig (attached as .config) > compiler: sparc64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205 > reproduce: > wget > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > git checkout e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 > # save the attached .config to linux build tree > make.cross ARCH=sparc64 > > All errors (new ones prefixed by >>): > >drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_prime_vmap': > >> drivers/gpu/drm/vgem/vgem_drv.c:238:53: error: 'PAGE_KERNEL_IO' undeclared > >> (first use in this function) > addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); > ^ >drivers/gpu/drm/vgem/vgem_drv.c:238:53: note: each undeclared identifier > is reported only once for each function it appears in > > vim +/PAGE_KERNEL_IO +238 drivers/gpu/drm/vgem/vgem_drv.c > >232void *addr; >233 >234pages = drm_gem_get_pages(obj); >235if (IS_ERR(pages)) >236return NULL; >237 > > 238addr = vmap(pages, n_pages, 0, > pgprot_writecombine(PAGE_KERNEL_IO)); sparc64 I guess we need an ifdef PAGE_KERNEL_IO ? or just rely on pgprot_writecombine(PAGE_KERNEL) being good enough -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: symbolic names for user load/submission preferences
On 11/07/16 20:58, Chris Wilson wrote: On Mon, Jul 11, 2016 at 06:12:40PM +0100, Dave Gordon wrote: The existing code that accesses the "enable_guc_loading" and "enable_guc_submission" parameters uses explicit numerical values for the various possibilities, including in some cases relying on boolean 0/1 mapping to specific values (which could be confusing for maintainers). So this patch just provides and uses names for the values representing the DEFAULT, DISABLED, PREFERRED, and MANDATORY options that the user can select (-1, 0, 1, 2 respectively). When is MANDATORY a good idea? If the hw doesn't support any other mechanism, then it will shut itself down gracefully if setup fails. If the user wants to force guc for testing, they only need to set the module parameter then check the guc is enabled afterwards and fail the test. At what point do we need such a warty user interface to the kernel? -Chris Validation like it, so it's REALLY REALLY OBVIOUS if the system is misconfigured (e.g. wrong firmware version) as driver initialisation will fail rather than quietly continue by falling back to execlists. Remember Daniel originally insisted on NO FALLBACK -- again, so that developers and testers didn't get confused by the system continuing to work despite the presence of a (hardware,firmware,driver) bug -- so that's the option that provides it. Of course it's not what end-users want, and so it's not what end-users get. You only get NO-FALLBACK mode if you specifically ask for it. Note also, all this is already implemented, this patch just provides symbolic names for the code to use instead of literal numbers. .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI] drm/i915: Provide argument names for static stubs
Make sure we keep kbuilder happy in all of its random configs by providing argument names for compile-time stubs. In file included from drivers/gpu/drm/i915/intel_dp_mst.c:27:0: drivers/gpu/drm/i915/i915_drv.h: In function 'i915_debugfs_register': >> drivers/gpu/drm/i915/i915_drv.h:3612:48: error: parameter name omitted static inline int i915_debugfs_register(struct drm_i915_private *) {return 0;} ^~~~ drivers/gpu/drm/i915/i915_drv.h: In function 'i915_debugfs_unregister': drivers/gpu/drm/i915/i915_drv.h:3613:51: error: parameter name omitted static inline void i915_debugfs_unregister(struct drm_i915_private *) {} Reported-by: 0day Signed-off-by: Chris Wilson Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 03e1bfaa5a41..e76cfe2e2471 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3644,8 +3644,8 @@ void i915_debugfs_unregister(struct drm_i915_private *dev_priv); int i915_debugfs_connector_add(struct drm_connector *connector); void intel_display_crc_init(struct drm_device *dev); #else -static inline int i915_debugfs_register(struct drm_i915_private *) {return 0;} -static inline void i915_debugfs_unregister(struct drm_i915_private *) {} +static inline int i915_debugfs_register(struct drm_i915_private *dev_priv) {return 0;} +static inline void i915_debugfs_unregister(struct drm_i915_private *dev_priv) {} static inline int i915_debugfs_connector_add(struct drm_connector *connector) { return 0; } static inline void intel_display_crc_init(struct drm_device *dev) {} -- 2.8.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Ignore panel type from OpRegion on SKL
From: Ville Syrjälä Dell XPS 13 9350 apparently doesn't like it when we use the panel type from OpRegion. The OpRegion panel type (0) tells us to use use low vswing for eDP, whereas the VBT panel type (2) tells us to use normal vswing. The problem is that low vswing results in some display flickers. Since no one seems to know how this stuff is supposed to be handled, let's just ignore the OpRegion panel type on SKL for now. Reported-by: James Bottomley Cc: James Bottomley Cc: drm-intel-fi...@lists.freedesktop.org References: https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_opregion.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index c27d5eb063d0..259760f12d16 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -1072,5 +1072,16 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv) return -ENODEV; } + /* +* FIXME On Dell XPS 13 9350 the OpRegion panel type (0) gives us +* low vswing for eDP, whereas the VBT panel type (2) gives us normal +* vswing instead. Low vswing results in some display flickers, so +* let's simply ignore the OpRegion panel type on SKL for now. +*/ + if (IS_SKYLAKE(dev_priv)) { + DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret); + return -ENODEV; + } + return ret - 1; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix legacy gamma lut updates in Linux 4.7-rc6
On 07/12/2016 12:50 PM, Lionel Landwerlin wrote: Hi Mario, Hi Lionel, There was a couple of patch to fix this issue : https://patchwork.freedesktop.org/series/5467/ https://patchwork.freedesktop.org/series/5466/ Looking at them they should fix the issue, but they seem to be stuck in review? I tested this late last week on drm-intel-nightly, it seems a series of revert fixed most of the issues. You mean something else has fixed legacy gamma updates, as i can't find above patches applied on drm-intel-nightly? Are those fixes supposed to be already part of 4.7-rc7, the final rc afaik? thanks, -mario Cheers, - Lionel On 12/07/16 11:33, Mario Kleiner wrote: Updating legacy gamma tables, e.g., via RandR doesn't work at all as of Linux 4.7-rc6. Reason seems to be that the required call to drm_atomic_helper_commit_planes_on_crtc is skipped in intel_atomic_commit after userspace set new gamma tables, because neither crtc->state->planes_changed nor update_pipe (= pipe_config->update_pipe) are true. Removing the check for planes_changed || update_pipe fixes gamma table updates. The code for Linux 4.8 drm-next has changed a lot in that area wrt. 4.7, but the new code for 4.8 also removed those checks and calls drm_atomic_helper_commit_planes_on_crtc unconditionally, and legacy gamma lut updates work on drm-next, so this seems to be the right solution. Tested also shutdown/reboot, suspend/resume, (un-)plugging displays, mode switches for resolution/refresh rate, display rotation, and page-flipping/pageflip timing on Intel HD Ironlake to confirm the fix apparently doesn't break anything under X11. Signed-off-by: Mario Kleiner Cc: Maarten Lankhorst Cc: Lionel Landwerlin Cc: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 04452cf..eb8fb36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13685,7 +13685,6 @@ static int intel_atomic_commit(struct drm_device *dev, bool modeset = needs_modeset(crtc->state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state); -bool update_pipe = !modeset && pipe_config->update_pipe; if (modeset && crtc->state->active) { update_scanline_offset(to_intel_crtc(crtc)); @@ -13699,8 +13698,7 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_get_existing_plane_state(state, crtc->primary)) intel_fbc_enable(intel_crtc); -if (crtc->state->active && -(crtc->state->planes_changed || update_pipe)) +if (crtc->state->active) drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); if (pipe_config->base.active && needs_vblank_wait(pipe_config)) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] backlight: Avoid double fbcon backlight handling
On Thu, Jun 30, 2016 at 12:30:56PM +0100, Chris Wilson wrote: > Backlights controlled by i915.ko and only associated with its connectors > and also only associated with the intel_drmfb fbcon, controlled by > i915.ko. In this situation, we already handle adjusting the backlight > when the fbcon is blanked/unblanked and do not require backlight trying > to do the same. > > Attempting to register with the fbdev as a client causes lockdep to warn > about a dependency cycle: The fbdev notifier strikes again! Last time I looked into this I think the proper solution would be to split the backlight part from the other fbdev notifier (which is used by fbcon for reacting to fbdev device reg/unreg events). I think that would fix this too, with the added bonus of slightly untangling the fbcon locking mess. And it's also the one part of untangling this mess which should be possible without any trouble - I've simply never done it since entirely getting rid of the fbdev notifier for fbcon is a lot more work. -Daniel > > [ 18.983763] == > [ 18.983763] [ INFO: possible circular locking dependency detected ] > [ 18.983766] 4.7.0-rc5+ #524 Tainted: G O > [ 18.983767] --- > [ 18.983767] kworker/u8:0/6 is trying to acquire lock: > [ 18.983777] (&dev->mode_config.mutex){+.+.+.}, at: [] > drm_modeset_lock_all+0x40/0x120 > [ 18.983777] >but task is already holding lock: > [ 18.983782] ((fb_notifier_list).rwsem){.+}, at: [] > __blocking_notifier_call_chain+0x35/0x70 > [ 18.983783] >which lock already depends on the new lock. > > [ 18.983783] >the existing dependency chain (in reverse order) is: > [ 18.983785] >-> #1 ((fb_notifier_list).rwsem){.+}: > [ 18.983789][] lock_acquire+0xb1/0x200 > [ 18.983792][] down_write+0x44/0x80 > [ 18.983795][] > blocking_notifier_chain_register+0x21/0xb0 > [ 18.983798][] fb_register_client+0x18/0x20 > [ 18.983800][] > backlight_device_register+0x136/0x260 > [ 18.983852][] > intel_backlight_device_register+0xa2/0x160 [i915] > [ 18.983892][] intel_connector_register+0xe/0x10 > [i915] > [ 18.983932][] > intel_dp_connector_register+0x1b/0x80 [i915] > [ 18.983936][] drm_connector_register+0x4a/0x80 > [ 18.983938][] > drm_connector_register_all+0x64/0xf0 > [ 18.983940][] > drm_modeset_register_all+0x174/0x1c0 > [ 18.983942][] drm_dev_register+0xc2/0xd0 > [ 18.983976][] i915_driver_load+0x1547/0x2200 > [i915] > [ 18.984010][] i915_pci_probe+0x4f/0x70 [i915] > [ 18.984014][] local_pci_probe+0x45/0xa0 > [ 18.984015][] pci_device_probe+0xdb/0x130 > [ 18.984018][] driver_probe_device+0x223/0x440 > [ 18.984020][] __driver_attach+0xd5/0x100 > [ 18.984022][] bus_for_each_dev+0x66/0xa0 > [ 18.984023][] driver_attach+0x1e/0x20 > [ 18.984025][] bus_add_driver+0x1ee/0x280 > [ 18.984028][] driver_register+0x60/0xe0 > [ 18.984030][] __pci_register_driver+0x60/0x70 > [ 18.984063][] i915_init+0x5b/0x62 [i915] > [ 18.984067][] do_one_initcall+0x3d/0x150 > [ 18.984070][] do_init_module+0x5f/0x1d9 > [ 18.984073][] load_module+0x20e6/0x27e0 > [ 18.984075][] SYSC_finit_module+0xc3/0xf0 > [ 18.984076][] SyS_finit_module+0xe/0x10 > [ 18.984079][] entry_SYSCALL_64_fastpath+0x1c/0xac > [ 18.984081] >-> #0 (&dev->mode_config.mutex){+.+.+.}: > [ 18.984083][] __lock_acquire+0x10fc/0x1260 > [ 18.984085][] lock_acquire+0xb1/0x200 > [ 18.984088][] mutex_lock_nested+0x67/0x3c0 > [ 18.984090][] drm_modeset_lock_all+0x40/0x120 > [ 18.984093][] > drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80 > [ 18.984095][] drm_fb_helper_set_par+0x2d/0x50 > [ 18.984134][] intel_fbdev_set_par+0x1a/0x60 > [i915] > [ 18.984136][] fbcon_init+0x586/0x610 > [ 18.984139][] visual_init+0xca/0x130 > [ 18.984141][] do_bind_con_driver+0x1c1/0x3a0 > [ 18.984143][] do_take_over_console+0x116/0x180 > [ 18.984145][] do_fbcon_takeover+0x57/0xb0 > [ 18.984147][] fbcon_event_notify+0x658/0x750 > [ 18.984150][] notifier_call_chain+0x3e/0xb0 > [ 18.984152][] > __blocking_notifier_call_chain+0x4d/0x70 > [ 18.984154][] > blocking_notifier_call_chain+0x16/0x20 > [ 18.984156][] fb_notifier_call_chain+0x1b/0x20 > [ 18.984158][] register_framebuffer+0x251/0x330 > [ 18.984160][] > drm_fb_helper_initial_config+0x25f/0x3f0 > [ 18.984199][] > intel_fbdev_initial_config+0x18/0x30 [i915] > [ 18.984201][] async_run_entry_fn+0x48/0x150
[Intel-gfx] [drm-intel:topic/drm-misc 2/11] arch/ia64/include/asm/pgtable.h:353:45: note: in expansion of macro 'pgprot_val'
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: 37035e7411fc90d57e4aca83ef654f50c3a0f626 commit: e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 [2/11] drm/vgem: Enable dmabuf interface for export config: ia64-allyesconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 4.9.0 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 # save the attached .config to linux build tree make.cross ARCH=ia64 All warnings (new ones prefixed by >>): In file included from arch/ia64/include/asm/ptrace.h:45:0, from arch/ia64/include/asm/processor.h:19, from arch/ia64/include/asm/thread_info.h:11, from include/linux/thread_info.h:54, from include/asm-generic/preempt.h:4, from arch/ia64/include/generated/asm/preempt.h:1, from include/linux/preempt.h:59, from include/linux/spinlock.h:50, from include/linux/seqlock.h:35, from include/linux/time.h:5, from include/linux/stat.h:18, from include/linux/module.h:10, from drivers/gpu/drm/vgem/vgem_drv.c:33: drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_prime_vmap': drivers/gpu/drm/vgem/vgem_drv.c:238:53: error: 'PAGE_KERNEL_IO' undeclared (first use in this function) addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ arch/ia64/include/asm/page.h:194:37: note: in definition of macro '__pgprot' # define __pgprot(x) ((pgprot_t) { (x) } ) ^ >> arch/ia64/include/asm/pgtable.h:353:45: note: in expansion of macro >> 'pgprot_val' #define pgprot_writecombine(prot) __pgprot((pgprot_val(prot) & ~_PAGE_MA_MASK) | _PAGE_MA_WC) ^ drivers/gpu/drm/vgem/vgem_drv.c:238:33: note: in expansion of macro 'pgprot_writecombine' addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ drivers/gpu/drm/vgem/vgem_drv.c:238:53: note: each undeclared identifier is reported only once for each function it appears in addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ arch/ia64/include/asm/page.h:194:37: note: in definition of macro '__pgprot' # define __pgprot(x) ((pgprot_t) { (x) } ) ^ >> arch/ia64/include/asm/pgtable.h:353:45: note: in expansion of macro >> 'pgprot_val' #define pgprot_writecombine(prot) __pgprot((pgprot_val(prot) & ~_PAGE_MA_MASK) | _PAGE_MA_WC) ^ drivers/gpu/drm/vgem/vgem_drv.c:238:33: note: in expansion of macro 'pgprot_writecombine' addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ vim +/pgprot_val +353 arch/ia64/include/asm/pgtable.h 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 337 pte_pfn(*ptep) != pte_pfn(pteval))) 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 338 /* load_module() calles flush_icache_range() explicitly*/ 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 339 __ia64_sync_icache_dcache(pteval); 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 340 *ptep = pteval; 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 341 } 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 342 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 343 #define set_pte_at(mm,addr,ptep,pteval) set_pte(ptep,pteval) 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 344 954ffcb3 include/asm-ia64/pgtable.h KAMEZAWA Hiroyuki 2007-10-16 345 /* 32e62c63 include/asm-ia64/pgtable.h Bjorn Helgaas 2006-05-05 346 * Make page protection values cacheable, uncacheable, or write- 32e62c63 include/asm-ia64/pgtable.h Bjorn Helgaas 2006-05-05 347 * combining. Note that "protection" is really a misnomer here as the 32e62c63 include/asm-ia64/pgtable.h Bjorn Helgaas 2006-05-05 348 * protection value contains the memory attribute bits, dirty bits, and 32e62c63 include/asm-ia64/pgtable.h Bjorn Helgaas 2006-05-05 349 * various other bits as well. ^1da177e include/asm-ia64/pgtable.h Linus Torvalds2005-04-16 350 */ 32e62c63 include/asm-ia64/pgtable.h Bjorn Helgaas 2006-05-05 351 #define pgprot_cacheable(prot) __pgprot((pgprot_val(prot) & ~_PAGE_MA_MASK) | _PAGE_MA_WB) ^1da177e include/asm-ia64/pgtable.h Linus Torvalds2005-04-16 352 #define pgprot_
Re: [Intel-gfx] [PATCH 2/3] drm/i915/guc: downgrade some DRM_ERROR() messages to DRM_WARN()
On 12/07/16 10:20, Tvrtko Ursulin wrote: On 11/07/16 19:01, Dave Gordon wrote: Where we're going to continue regardless of the problem, rather than fail, then the message should be a WARNing rather than an ERROR. Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/i915_guc_submission.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 2112e02..e299b64 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -114,10 +114,8 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) if (ret != -ETIMEDOUT) ret = -EIO; -DRM_ERROR("GUC: host2guc action 0x%X failed. ret=%d " -"status=0x%08X response=0x%08X\n", -data[0], ret, status, -I915_READ(SOFT_SCRATCH(15))); +DRM_WARN("Action 0x%X failed; ret=%d status=0x%08X response=0x%08X\n", + data[0], ret, status, I915_READ(SOFT_SCRATCH(15))); Hm, this does propagate the error code to the callers some which will act and log the failure. Majority won't though - like suspend/resume etc. In those cases it feels more like an error than a warning. It's definitely something that shouldn't happen, so we need to log it; and it has to be done at this level because we don't pass enough information back to leave it to the caller. But OTOH this layer doesn't have enough information to determine just how serious a failure is. So as a compromise the idea is to log a WARNING here, and then the caller can choose to: 1. pass the failure up (until we reach a layer with more context) 2. quietly disregard the failure and continue anyway 3. report an ERROR and fail/abort the process. That way we should get all the useful information about the root cause of something that ends up as an ERROR, while neither ignoring nor being too verbose about failures from which we may ultimately recover. For example, one of the callers (the doorbell h/w initialisation code) considers some failures as interesting but not critical (DEBUG level) but other instances of the exact same operation are fatal (ERROR). dev_priv->guc.action_fail += 1; dev_priv->guc.action_err = ret; @@ -553,8 +551,8 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) if (db_ret.db_status == GUC_DOORBELL_DISABLED) break; -DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n", - db_cmp.cookie, db_ret.cookie); +DRM_WARN("Cookie mismatch. Expected %d, found %d\n", + db_cmp.cookie, db_ret.cookie); This one is interesting, error is propagated out a bit but then ignored in actual command submission. If the above message means command will not be submitted error is probably more appropriate. Or perhaps we cannot tell if the command was submitted or not in this case? Note that this is inside a retry loop. It shouldn't ever happen, but if it does we'll report it and try one more time. If it keeps happening (which would require active interference by some other party) we won't be able to ring the doorbell and the failure will be propagated back out of the GuC submission code. OTOH the caller then ignores it because "submission is not allowed to fail" (!) And yes, it is then undefined as to whether the command has been submitted or not. If it hasn't we'll expect a GPU hang later. .Dave. /* update the cookie to newly read cookie from GuC */ db_cmp.cookie = db_ret.cookie; @@ -726,8 +724,8 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) /* Restore to original value */ err = guc_update_doorbell_id(guc, client, db_id); if (err) -DRM_ERROR("Failed to restore doorbell to %d, err %d\n", -db_id, err); +DRM_WARN("Failed to restore doorbell to %d, err %d\n", + db_id, err); for (i = 0; i < GUC_MAX_DOORBELLS; ++i) { i915_reg_t drbreg = GEN8_DRBREGL(i); @@ -819,8 +817,6 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) return client; err: -DRM_ERROR("FAILED to create priority %u GuC client!\n", priority); - guc_client_free(dev_priv, client); return NULL; } @@ -998,7 +994,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv) GUC_CTX_PRIORITY_KMD_NORMAL, dev_priv->kernel_context); if (!client) { -DRM_ERROR("Failed to create execbuf guc_client\n"); +DRM_ERROR("Failed to create normal GuC client!\n"); return -ENOMEM; } Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [drm-intel:topic/drm-misc 2/11] drivers/gpu/drm/vgem/vgem_drv.c:238:33: note: in expansion of macro 'pgprot_writecombine'
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: 37035e7411fc90d57e4aca83ef654f50c3a0f626 commit: e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 [2/11] drm/vgem: Enable dmabuf interface for export config: m68k-allyesconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 4.9.0 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout e6f15b763ab2bc47000ec302123e2fb3bf2ad7d4 # save the attached .config to linux build tree make.cross ARCH=m68k All warnings (new ones prefixed by >>): In file included from arch/m68k/include/asm/thread_info.h:5:0, from include/linux/thread_info.h:54, from include/asm-generic/preempt.h:4, from arch/m68k/include/generated/asm/preempt.h:1, from include/linux/preempt.h:59, from include/linux/spinlock.h:50, from include/linux/seqlock.h:35, from include/linux/time.h:5, from include/linux/stat.h:18, from include/linux/module.h:10, from drivers/gpu/drm/vgem/vgem_drv.c:33: drivers/gpu/drm/vgem/vgem_drv.c: In function 'vgem_prime_vmap': drivers/gpu/drm/vgem/vgem_drv.c:238:53: error: 'PAGE_KERNEL_IO' undeclared (first use in this function) addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ arch/m68k/include/asm/page.h:37:36: note: in definition of macro '__pgprot' #define __pgprot(x) ((pgprot_t) { (x) } ) ^ >> arch/m68k/include/asm/pgtable_mm.h:164:15: note: in expansion of macro >> 'pgprot_val' ? (__pgprot(pgprot_val(prot) | __SUN3_PAGE_NOCACHE)) \ ^ >> include/asm-generic/pgtable.h:306:29: note: in expansion of macro >> 'pgprot_noncached' #define pgprot_writecombine pgprot_noncached ^ >> drivers/gpu/drm/vgem/vgem_drv.c:238:33: note: in expansion of macro >> 'pgprot_writecombine' addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ drivers/gpu/drm/vgem/vgem_drv.c:238:53: note: each undeclared identifier is reported only once for each function it appears in addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ arch/m68k/include/asm/page.h:37:36: note: in definition of macro '__pgprot' #define __pgprot(x) ((pgprot_t) { (x) } ) ^ >> arch/m68k/include/asm/pgtable_mm.h:164:15: note: in expansion of macro >> 'pgprot_val' ? (__pgprot(pgprot_val(prot) | __SUN3_PAGE_NOCACHE)) \ ^ >> include/asm-generic/pgtable.h:306:29: note: in expansion of macro >> 'pgprot_noncached' #define pgprot_writecombine pgprot_noncached ^ >> drivers/gpu/drm/vgem/vgem_drv.c:238:33: note: in expansion of macro >> 'pgprot_writecombine' addr = vmap(pages, n_pages, 0, pgprot_writecombine(PAGE_KERNEL_IO)); ^ vim +/pgprot_writecombine +238 drivers/gpu/drm/vgem/vgem_drv.c 222 st = drm_prime_pages_to_sg(pages, obj->size >> PAGE_SHIFT); 223 drm_gem_put_pages(obj, pages, false, false); 224 225 return st; 226 } 227 228 static void *vgem_prime_vmap(struct drm_gem_object *obj) 229 { 230 long n_pages = obj->size >> PAGE_SHIFT; 231 struct page **pages; 232 void *addr; 233 234 pages = drm_gem_get_pages(obj); 235 if (IS_ERR(pages)) 236 return NULL; 237 > 238 addr = vmap(pages, n_pages, 0, > pgprot_writecombine(PAGE_KERNEL_IO)); 239 drm_gem_put_pages(obj, pages, false, false); 240 241 return addr; 242 } 243 244 static void vgem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) 245 { 246 vunmap(vaddr); --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915: Provide argument names for static stubs
== Series Details == Series: drm/i915: Provide argument names for static stubs URL : https://patchwork.freedesktop.org/series/9751/ State : failure == Summary == Series 9751v1 drm/i915: Provide argument names for static stubs http://patchwork.freedesktop.org/api/1.0/series/9751/revisions/1/mbox Test gem_sync: Subgroup basic-store-each: pass -> DMESG-FAIL (ro-bdw-i7-5600u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: skip -> DMESG-WARN (ro-bdw-i7-5557U) fi-kbl-qkkr total:237 pass:174 dwarn:27 dfail:0 fail:9 skip:27 fi-skl-i5-6260u total:237 pass:218 dwarn:0 dfail:0 fail:7 skip:12 fi-skl-i7-6700k total:237 pass:204 dwarn:0 dfail:0 fail:7 skip:26 fi-snb-i7-2600 total:237 pass:190 dwarn:0 dfail:0 fail:7 skip:40 ro-bdw-i5-5250u total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5557U total:237 pass:213 dwarn:2 dfail:0 fail:7 skip:15 ro-bdw-i7-5600u total:237 pass:198 dwarn:0 dfail:1 fail:7 skip:31 ro-bsw-n3050 total:218 pass:170 dwarn:0 dfail:0 fail:5 skip:42 ro-byt-n2820 total:237 pass:188 dwarn:0 dfail:0 fail:11 skip:38 ro-hsw-i3-4010u total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-hsw-i7-4770r total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-ilk-i7-620lm total:237 pass:166 dwarn:0 dfail:0 fail:8 skip:63 ro-ilk1-i5-650 total:232 pass:166 dwarn:0 dfail:0 fail:8 skip:58 ro-ivb-i7-3770 total:237 pass:197 dwarn:0 dfail:0 fail:7 skip:33 ro-skl3-i5-6260u total:237 pass:217 dwarn:1 dfail:0 fail:7 skip:12 ro-snb-i7-2620M total:237 pass:188 dwarn:0 dfail:0 fail:8 skip:41 Results at /archive/results/CI_IGT_test/RO_Patchwork_1472/ 7d4e6e0 drm-intel-nightly: 2016y-07m-12d-11h-23m-08s UTC integration manifest 80dc765 drm/i915: Provide argument names for static stubs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)
vGEM buffers are useful for passing data between software clients and hardware renders. By allowing the user to create and attach fences to the exported vGEM buffers (on the dma-buf), the user can implement a deferred renderer and queue hardware operations like flipping and then signal the buffer readiness (i.e. this allows the user to schedule operations out-of-order, but have them complete in-order). This also makes it much easier to write tightly controlled testcases for dma-buf fencing and signaling between hardware drivers. v2: Don't pretend the fences exist in an ordered timeline, but allocate a separate fence-context for each fence so that the fences are unordered. Testcase: igt/vgem_basic/dmabuf-fence Signed-off-by: Chris Wilson Cc: Sean Paul Cc: Zach Reizner Cc: Gustavo Padovan Cc: Daniel Vetter Acked-by: Zach Reizner --- drivers/gpu/drm/vgem/Makefile | 2 +- drivers/gpu/drm/vgem/vgem_drv.c | 34 +++ drivers/gpu/drm/vgem/vgem_drv.h | 16 +++ drivers/gpu/drm/vgem/vgem_fence.c | 207 ++ include/uapi/drm/vgem_drm.h | 62 5 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/vgem/vgem_fence.c create mode 100644 include/uapi/drm/vgem_drm.h diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile index 3f4c7b842028..bfcdea1330e6 100644 --- a/drivers/gpu/drm/vgem/Makefile +++ b/drivers/gpu/drm/vgem/Makefile @@ -1,4 +1,4 @@ ccflags-y := -Iinclude/drm -vgem-y := vgem_drv.o +vgem-y := vgem_drv.o vgem_fence.o obj-$(CONFIG_DRM_VGEM) += vgem.o diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c index 29c2aab3c1a7..c15bafb06665 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.c +++ b/drivers/gpu/drm/vgem/vgem_drv.c @@ -83,6 +83,34 @@ static const struct vm_operations_struct vgem_gem_vm_ops = { .close = drm_gem_vm_close, }; +static int vgem_open(struct drm_device *dev, struct drm_file *file) +{ + struct vgem_file *vfile; + int ret; + + vfile = kzalloc(sizeof(*vfile), GFP_KERNEL); + if (!vfile) + return -ENOMEM; + + file->driver_priv = vfile; + + ret = vgem_fence_open(vfile); + if (ret) { + kfree(vfile); + return ret; + } + + return 0; +} + +static void vgem_preclose(struct drm_device *dev, struct drm_file *file) +{ + struct vgem_file *vfile = file->driver_priv; + + vgem_fence_close(vfile); + kfree(vfile); +} + /* ioctls */ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev, @@ -164,6 +192,8 @@ unref: } static struct drm_ioctl_desc vgem_ioctls[] = { + DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), }; static int vgem_mmap(struct file *filp, struct vm_area_struct *vma) @@ -271,9 +301,12 @@ static int vgem_prime_mmap(struct drm_gem_object *obj, static struct drm_driver vgem_driver = { .driver_features= DRIVER_GEM | DRIVER_PRIME, + .open = vgem_open, + .preclose = vgem_preclose, .gem_free_object_unlocked = vgem_gem_free_object, .gem_vm_ops = &vgem_gem_vm_ops, .ioctls = vgem_ioctls, + .num_ioctls = ARRAY_SIZE(vgem_ioctls), .fops = &vgem_driver_fops, .dumb_create= vgem_gem_dumb_create, @@ -328,5 +361,6 @@ module_init(vgem_init); module_exit(vgem_exit); MODULE_AUTHOR("Red Hat, Inc."); +MODULE_AUTHOR("Intel Corporation"); MODULE_DESCRIPTION(DRIVER_DESC); MODULE_LICENSE("GPL and additional rights"); diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h index 988cbaae7588..1f8798ad329c 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.h +++ b/drivers/gpu/drm/vgem/vgem_drv.h @@ -32,9 +32,25 @@ #include #include +#include + +struct vgem_file { + struct idr fence_idr; + struct mutex fence_mutex; +}; + #define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base) struct drm_vgem_gem_object { struct drm_gem_object base; }; +int vgem_fence_open(struct vgem_file *file); +int vgem_fence_attach_ioctl(struct drm_device *dev, + void *data, + struct drm_file *file); +int vgem_fence_signal_ioctl(struct drm_device *dev, + void *data, + struct drm_file *file); +void vgem_fence_close(struct vgem_file *file); + #endif diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c new file mode 100644 index ..63095331c446 --- /dev/null +++ b/drivers/gpu/drm/vgem/vgem_fence.c @@ -0,0 +1,207 @@ +/* + * Copyright 2016 Intel Corporation + * + *
[Intel-gfx] [PATCH 1/9] drm/i915: Fix iboost setting for DDI with 4 lanes on SKL
From: Ville Syrjälä Bspec says: "For DDIA with x4 capability (DDI_BUF_CTL DDIA Lane Capability Control = DDIA x4), the I_boost value has to be programmed in both tx_blnclegsctl_0 and tx_blnclegsctl_4." Currently we only program tx_blnclegsctl_0. Let's do the other one as well. Cc: David Weinehall Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ddi.c | 36 +++- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8bfde75789f6..e6f9f05b3a4a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1536,6 +1536,7 @@ enum skl_disp_power_wells { #define BALANCE_LEG_MASK(port) (7<<(8+3*(port))) /* Balance leg disable bits */ #define BALANCE_LEG_DISABLE_SHIFT 23 +#define BALANCE_LEG_DISABLE(port) (1 << (23 + (port))) /* * Fence registers diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index dd1d6fe12297..75354cd9bbab 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1379,14 +1379,30 @@ void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc) TRANS_CLK_SEL_DISABLED); } -static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv, - u32 level, enum port port, int type) +static void _skl_ddi_set_iboost(struct drm_i915_private *dev_priv, + enum port port, uint8_t iboost) { + u32 tmp; + + tmp = I915_READ(DISPIO_CR_TX_BMU_CR0); + tmp &= ~(BALANCE_LEG_MASK(port) | BALANCE_LEG_DISABLE(port)); + if (iboost) + tmp |= iboost << BALANCE_LEG_SHIFT(port); + else + tmp |= BALANCE_LEG_DISABLE(port); + I915_WRITE(DISPIO_CR_TX_BMU_CR0, tmp); +} + +static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level) +{ + struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base); + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev); + enum port port = intel_dig_port->port; + int type = encoder->type; const struct ddi_buf_trans *ddi_translations; uint8_t iboost; uint8_t dp_iboost, hdmi_iboost; int n_entries; - u32 reg; /* VBT may override standard boost values */ dp_iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level; @@ -1428,16 +1444,10 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv, return; } - reg = I915_READ(DISPIO_CR_TX_BMU_CR0); - reg &= ~BALANCE_LEG_MASK(port); - reg &= ~(1 << (BALANCE_LEG_DISABLE_SHIFT + port)); - - if (iboost) - reg |= iboost << BALANCE_LEG_SHIFT(port); - else - reg |= 1 << (BALANCE_LEG_DISABLE_SHIFT + port); + _skl_ddi_set_iboost(dev_priv, port, iboost); - I915_WRITE(DISPIO_CR_TX_BMU_CR0, reg); + if (port == PORT_A && intel_dig_port->max_lanes == 4) + _skl_ddi_set_iboost(dev_priv, PORT_E, iboost); } static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv, @@ -1568,7 +1578,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp) level = translate_signal_level(signal_levels); if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) - skl_ddi_set_iboost(dev_priv, level, port, encoder->type); + skl_ddi_set_iboost(encoder, level); else if (IS_BROXTON(dev_priv)) bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: Split DP/eDP/FDI and HDMI/DVI DDI buffer programming apart
From: Ville Syrjälä DDI buffer prorgramming works quite differently depending on the mode of the DDI port (DP/eDP/FDI vs. HDMI/DVI). Let's split the function that does the programming into two matching variants as well. Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ddi.c| 75 +++-- drivers/gpu/drm/i915/intel_dp_mst.c | 4 +- drivers/gpu/drm/i915/intel_drv.h| 2 +- 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 35b979956d6d..5cae63aa3905 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -421,28 +421,20 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por /* * Starting with Haswell, DDI port buffers must be programmed with correct - * values in advance. The buffer values are different for FDI and DP modes, - * but the HDMI/DVI fields are shared among those. So we program the DDI - * in either FDI or DP modes only, as HDMI connections will work with both - * of those + * values in advance. This function programs the correct values for + * DP/eDP/FDI use cases. */ -void intel_prepare_ddi_buffer(struct intel_encoder *encoder) +void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); u32 iboost_bit = 0; - int i, n_hdmi_entries, n_dp_entries, n_edp_entries, - size; - int hdmi_level; - enum port port; + int i, n_dp_entries, n_edp_entries, size; + enum port port = intel_ddi_get_encoder_port(encoder); const struct ddi_buf_trans *ddi_translations_fdi; const struct ddi_buf_trans *ddi_translations_dp; const struct ddi_buf_trans *ddi_translations_edp; - const struct ddi_buf_trans *ddi_translations_hdmi; const struct ddi_buf_trans *ddi_translations; - port = intel_ddi_get_encoder_port(encoder); - hdmi_level = intel_ddi_hdmi_level(dev_priv, port); - if (IS_BROXTON(dev_priv)) return; @@ -452,8 +444,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) skl_get_buf_trans_dp(dev_priv, &n_dp_entries); ddi_translations_edp = skl_get_buf_trans_edp(dev_priv, &n_edp_entries); - ddi_translations_hdmi = - skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries); + /* If we're boosting the current, set bit 31 of trans1 */ if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level || dev_priv->vbt.ddi_port_info[port].dp_boost_level) @@ -466,7 +457,6 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) } else if (IS_BROADWELL(dev_priv)) { ddi_translations_fdi = bdw_ddi_translations_fdi; ddi_translations_dp = bdw_ddi_translations_dp; - if (dev_priv->vbt.edp.low_vswing) { ddi_translations_edp = bdw_ddi_translations_edp; n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp); @@ -474,27 +464,19 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) ddi_translations_edp = bdw_ddi_translations_dp; n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_dp); } - - ddi_translations_hdmi = bdw_ddi_translations_hdmi; - n_dp_entries = ARRAY_SIZE(bdw_ddi_translations_dp); - n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi); } else if (IS_HASWELL(dev_priv)) { ddi_translations_fdi = hsw_ddi_translations_fdi; ddi_translations_dp = hsw_ddi_translations_dp; ddi_translations_edp = hsw_ddi_translations_dp; - ddi_translations_hdmi = hsw_ddi_translations_hdmi; n_dp_entries = n_edp_entries = ARRAY_SIZE(hsw_ddi_translations_dp); - n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi); } else { WARN(1, "ddi translation table missing\n"); ddi_translations_edp = bdw_ddi_translations_dp; ddi_translations_fdi = bdw_ddi_translations_fdi; ddi_translations_dp = bdw_ddi_translations_dp; - ddi_translations_hdmi = bdw_ddi_translations_hdmi; n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp); n_dp_entries = ARRAY_SIZE(bdw_ddi_translations_dp); - n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi); } switch (encoder->type) { @@ -503,7 +485,6 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) size = n_edp_entries; break; case INTEL_OUTPUT_DP: - case INTEL_OUTPUT_HDMI: ddi_translations = dd
[Intel-gfx] [PATCH 0/9] drm/i915: SKL iboost fixes
From: Ville Syrjälä While looking into the recent low vs. normal vswing SKL regression, I ended up going through the iboost handling as well. I spotted several problems which I've tried to fix. I also picked up a bunch of related patches from my earlier split DDI encoder series [1]. Entire series available here: git://github.com/vsyrjala/linux.git ddi_iboost_fixes_2 [1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/082384.html Ville Syrjälä (9): drm/i915: Fix iboost setting for DDI with 4 lanes on SKL drm/i915: Name the "iboost bit" drm/i915: Program iboost settings for HDMI/DVI on SKL drm/i915: Move bxt_ddi_vswing_sequence() call into intel_ddi_pre_enable() for HDMI drm/i915: Explicitly use ddi buf trans entry 9 for hdmi drm/i915: Split DP/eDP/FDI and HDMI/DVI DDI buffer programming apart drm/i915: Get the iboost setting based on the port type drm/i915: Simplify intel_ddi_get_encoder_port() drm/i915: Extract bdw_get_buf_trans_edp() drivers/gpu/drm/i915/i915_reg.h | 2 + drivers/gpu/drm/i915/intel_ddi.c| 230 +--- drivers/gpu/drm/i915/intel_dp_mst.c | 4 +- drivers/gpu/drm/i915/intel_drv.h| 2 +- 4 files changed, 137 insertions(+), 101 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm/i915: Get the iboost setting based on the port type
From: Ville Syrjälä Program the 'iboost_bit' based on what the VBT says it should be for the specific port type, rather than assume it's always the same for DP and HDMI. Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ddi.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 5cae63aa3905..5720202db8f0 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -446,8 +446,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder) skl_get_buf_trans_edp(dev_priv, &n_edp_entries); /* If we're boosting the current, set bit 31 of trans1 */ - if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level || - dev_priv->vbt.ddi_port_info[port].dp_boost_level) + if (dev_priv->vbt.ddi_port_info[port].dp_boost_level) iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE; if (WARN_ON(encoder->type == INTEL_OUTPUT_EDP && @@ -524,9 +523,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder) if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) { ddi_translations_hdmi = skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries); + /* If we're boosting the current, set bit 31 of trans1 */ - if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level || - dev_priv->vbt.ddi_port_info[port].dp_boost_level) + if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level) iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE; } else if (IS_BROADWELL(dev_priv)) { ddi_translations_hdmi = bdw_ddi_translations_hdmi; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 9/9] drm/i915: Extract bdw_get_buf_trans_edp()
From: Ville Syrjälä Make the BDW and SKL code a bit more similar by extracting the low vswing handling for BDW into a helper, as we already have it like that for SKL+. Cc: Mika Kahola Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ddi.c | 20 +--- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index c581751a645a..fc2ef2d76091 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -320,6 +320,18 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder) } static const struct ddi_buf_trans * +bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries) +{ + if (dev_priv->vbt.edp.low_vswing) { + *n_entries = ARRAY_SIZE(bdw_ddi_translations_edp); + return bdw_ddi_translations_edp; + } else { + *n_entries = ARRAY_SIZE(bdw_ddi_translations_dp); + return bdw_ddi_translations_dp; + } +} + +static const struct ddi_buf_trans * skl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries) { if (IS_SKL_ULX(dev_priv) || IS_KBL_ULX(dev_priv)) { @@ -436,13 +448,7 @@ void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder) } else if (IS_BROADWELL(dev_priv)) { ddi_translations_fdi = bdw_ddi_translations_fdi; ddi_translations_dp = bdw_ddi_translations_dp; - if (dev_priv->vbt.edp.low_vswing) { - ddi_translations_edp = bdw_ddi_translations_edp; - n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp); - } else { - ddi_translations_edp = bdw_ddi_translations_dp; - n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_dp); - } + ddi_translations_edp = bdw_get_buf_trans_edp(dev_priv, &n_edp_entries); n_dp_entries = ARRAY_SIZE(bdw_ddi_translations_dp); } else if (IS_HASWELL(dev_priv)) { ddi_translations_fdi = hsw_ddi_translations_fdi; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/9] drm/i915: Name the "iboost bit"
From: Ville Syrjälä Give a proper name for the SKL DDI_BUF_TRANS iboost bit. Cc: David Weinehall Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ddi.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index e6f9f05b3a4a..ede4cd3531f8 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7483,6 +7483,7 @@ enum { #define _DDI_BUF_TRANS_A 0x64E00 #define _DDI_BUF_TRANS_B 0x64E60 #define DDI_BUF_TRANS_LO(port, i) _MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8) +#define DDI_BUF_BALANCE_LEG_ENABLE(1 << 31) #define DDI_BUF_TRANS_HI(port, i) _MMIO(_PORT(port, _DDI_BUF_TRANS_A, _DDI_BUF_TRANS_B) + (i) * 8 + 4) /* Sideband Interface (SBI) is programmed indirectly, via diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 75354cd9bbab..78e1fdaec86d 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -434,7 +434,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) /* If we're boosting the current, set bit 31 of trans1 */ if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level || dev_priv->vbt.ddi_port_info[port].dp_boost_level) - iboost_bit = 1<<31; + iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE; if (WARN_ON(encoder->type == INTEL_OUTPUT_EDP && port != PORT_A && port != PORT_E && -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/9] drm/i915: Explicitly use ddi buf trans entry 9 for hdmi
From: Ville Syrjälä When the DDI port is in HDMI/DVI mode, it automagically uses the buffer translations values from entry 9. Let's make that explicit in the code. Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ddi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 52589bbea8da..35b979956d6d 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -526,9 +526,9 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) return; /* Entry 9 is for HDMI: */ - I915_WRITE(DDI_BUF_TRANS_LO(port, i), + I915_WRITE(DDI_BUF_TRANS_LO(port, 9), ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit); - I915_WRITE(DDI_BUF_TRANS_HI(port, i), + I915_WRITE(DDI_BUF_TRANS_HI(port, 9), ddi_translations_hdmi[hdmi_level].trans2); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/9] drm/i915: Move bxt_ddi_vswing_sequence() call into intel_ddi_pre_enable() for HDMI
From: Ville Syrjälä Now that the SKL iboost programming is done from intel_ddi_pre_enable() for HDMI, let's move the BXT bxt_ddi_vswing_sequence() call there as well. This makes things look more similar to the DP/eDP case which is handled in ddi_signal_levels(). Cc: Imre Deak Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ddi.c | 15 --- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 09fe580dfd49..52589bbea8da 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -301,9 +301,6 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = { { 154, 0x9A, 1, 128, true },/* 9: 12000 */ }; -static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv, - u32 level, enum port port, int type); - static void ddi_get_encoder_port(struct intel_encoder *intel_encoder, struct intel_digital_port **dig_port, enum port *port) @@ -446,15 +443,8 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) port = intel_ddi_get_encoder_port(encoder); hdmi_level = intel_ddi_hdmi_level(dev_priv, port); - if (IS_BROXTON(dev_priv)) { - if (encoder->type != INTEL_OUTPUT_HDMI) - return; - - /* Vswing programming for HDMI */ - bxt_ddi_vswing_sequence(dev_priv, hdmi_level, port, - INTEL_OUTPUT_HDMI); + if (IS_BROXTON(dev_priv)) return; - } if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) { ddi_translations_fdi = NULL; @@ -1676,6 +1666,9 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) skl_ddi_set_iboost(intel_encoder, level); + else if (IS_BROXTON(dev_priv)) + bxt_ddi_vswing_sequence(dev_priv, level, port, + INTEL_OUTPUT_HDMI); intel_hdmi->set_infoframes(encoder, crtc->config->has_hdmi_sink, -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/9] drm/i915: Simplify intel_ddi_get_encoder_port()
From: Ville Syrjälä We no longer have any need to look up the intel_digital_port based on the passed in intel_encoder, but we still want to look up the port. Let's just move that logic into intel_ddi_get_encoder_port() and drop the dig_port stuff. Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ddi.c | 36 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 5720202db8f0..c581751a645a 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -301,44 +301,24 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = { { 154, 0x9A, 1, 128, true },/* 9: 12000 */ }; -static void ddi_get_encoder_port(struct intel_encoder *intel_encoder, -struct intel_digital_port **dig_port, -enum port *port) +enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder) { - struct drm_encoder *encoder = &intel_encoder->base; - - switch (intel_encoder->type) { + switch (encoder->type) { case INTEL_OUTPUT_DP_MST: - *dig_port = enc_to_mst(encoder)->primary; - *port = (*dig_port)->port; - break; - default: - WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type); - /* fallthrough and treat as unknown */ + return enc_to_mst(&encoder->base)->primary->port; case INTEL_OUTPUT_DP: case INTEL_OUTPUT_EDP: case INTEL_OUTPUT_HDMI: case INTEL_OUTPUT_UNKNOWN: - *dig_port = enc_to_dig_port(encoder); - *port = (*dig_port)->port; - break; + return enc_to_dig_port(&encoder->base)->port; case INTEL_OUTPUT_ANALOG: - *dig_port = NULL; - *port = PORT_E; - break; + return PORT_E; + default: + MISSING_CASE(encoder->type); + return PORT_A; } } -enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder) -{ - struct intel_digital_port *dig_port; - enum port port; - - ddi_get_encoder_port(intel_encoder, &dig_port, &port); - - return port; -} - static const struct ddi_buf_trans * skl_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries) { -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/9] drm/i915: Program iboost settings for HDMI/DVI on SKL
From: Ville Syrjälä Currently we fail to program the iboost stuff for HDMI/DVI. Let's remedy that. Cc: David Weinehall Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ddi.c | 51 +++- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 78e1fdaec86d..09fe580dfd49 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -388,6 +388,40 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries) } } +static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port) +{ + int n_hdmi_entries; + int hdmi_level; + int hdmi_default_entry; + + hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift; + + if (IS_BROXTON(dev_priv)) + return hdmi_level; + + if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) { + skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries); + hdmi_default_entry = 8; + } else if (IS_BROADWELL(dev_priv)) { + n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi); + hdmi_default_entry = 7; + } else if (IS_HASWELL(dev_priv)) { + n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi); + hdmi_default_entry = 6; + } else { + WARN(1, "ddi translation table missing\n"); + n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi); + hdmi_default_entry = 7; + } + + /* Choose a good default if VBT is badly populated */ + if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN || + hdmi_level >= n_hdmi_entries) + hdmi_level = hdmi_default_entry; + + return hdmi_level; +} + /* * Starting with Haswell, DDI port buffers must be programmed with correct * values in advance. The buffer values are different for FDI and DP modes, @@ -399,7 +433,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); u32 iboost_bit = 0; - int i, n_hdmi_entries, n_dp_entries, n_edp_entries, hdmi_default_entry, + int i, n_hdmi_entries, n_dp_entries, n_edp_entries, size; int hdmi_level; enum port port; @@ -410,7 +444,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) const struct ddi_buf_trans *ddi_translations; port = intel_ddi_get_encoder_port(encoder); - hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift; + hdmi_level = intel_ddi_hdmi_level(dev_priv, port); if (IS_BROXTON(dev_priv)) { if (encoder->type != INTEL_OUTPUT_HDMI) @@ -430,7 +464,6 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) skl_get_buf_trans_edp(dev_priv, &n_edp_entries); ddi_translations_hdmi = skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries); - hdmi_default_entry = 8; /* If we're boosting the current, set bit 31 of trans1 */ if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level || dev_priv->vbt.ddi_port_info[port].dp_boost_level) @@ -456,7 +489,6 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) n_dp_entries = ARRAY_SIZE(bdw_ddi_translations_dp); n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi); - hdmi_default_entry = 7; } else if (IS_HASWELL(dev_priv)) { ddi_translations_fdi = hsw_ddi_translations_fdi; ddi_translations_dp = hsw_ddi_translations_dp; @@ -464,7 +496,6 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) ddi_translations_hdmi = hsw_ddi_translations_hdmi; n_dp_entries = n_edp_entries = ARRAY_SIZE(hsw_ddi_translations_dp); n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi); - hdmi_default_entry = 6; } else { WARN(1, "ddi translation table missing\n"); ddi_translations_edp = bdw_ddi_translations_dp; @@ -474,7 +505,6 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp); n_dp_entries = ARRAY_SIZE(bdw_ddi_translations_dp); n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi); - hdmi_default_entry = 7; } switch (encoder->type) { @@ -505,11 +535,6 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) if (encoder->type != INTEL_OUTPUT_HDMI) return; - /* Choose a good default if VBT is badly populated */ - if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN || - hdmi_level >= n_hdmi_entries) - hdmi_level = hdmi_default
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
Chris Wilson writes: > On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display > power well and so the sna-hda audio codec acquires the display power > well while it is operational. However, Skylake separates the powerwells > again, and so we must remember to acquire the rpm wakeref for ourselves > whilst tweaking the registers. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 > Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for > SKL") > Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala > Cc: Libin Yang > Cc: Takashi Iwai > Cc: Marius Vlad > --- > drivers/gpu/drm/i915/intel_audio.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > b/drivers/gpu/drm/i915/intel_audio.c > index 5d5f6bc10e85..eedcce6478ef 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -600,6 +600,8 @@ static void > i915_audio_component_codec_wake_override(struct device *dev, > if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv)) > return; > > + intel_runtime_pm_get(dev_priv); > + > /* >* Enable/disable generating the codec wake signal, overriding the >* internal logic to generate the codec wake to controller. > @@ -615,6 +617,8 @@ static void > i915_audio_component_codec_wake_override(struct device *dev, > I915_WRITE(HSW_AUD_CHICKENBIT, tmp); > usleep_range(1000, 1500); > } > + > + intel_runtime_pm_put(dev_priv); > } > > /* Get CDCLK in kHz */ > @@ -648,6 +652,8 @@ static int i915_audio_component_sync_audio_rate(struct > device *dev, > !IS_HASWELL(dev_priv)) > return 0; > > + intel_runtime_pm_get(dev_priv); > + > mutex_lock(&dev_priv->av_mutex); > /* 1. get the pipe */ > intel_encoder = dev_priv->dig_port_map[port]; > @@ -698,6 +704,7 @@ static int i915_audio_component_sync_audio_rate(struct > device *dev, > > unlock: > mutex_unlock(&dev_priv->av_mutex); > + intel_runtime_pm_put(dev_priv); > return err; > } > > -- > 2.8.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Ro.CI.BAT: warning for drm/i915: Ignore panel type from OpRegion on SKL
== Series Details == Series: drm/i915: Ignore panel type from OpRegion on SKL URL : https://patchwork.freedesktop.org/series/9752/ State : warning == Summary == Series 9752v1 drm/i915: Ignore panel type from OpRegion on SKL http://patchwork.freedesktop.org/api/1.0/series/9752/revisions/1/mbox Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (ro-byt-n2820) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: skip -> DMESG-WARN (ro-bdw-i7-5557U) fi-kbl-qkkr total:237 pass:174 dwarn:28 dfail:1 fail:7 skip:27 fi-skl-i5-6260u total:237 pass:218 dwarn:0 dfail:0 fail:7 skip:12 fi-skl-i7-6700k total:237 pass:204 dwarn:0 dfail:0 fail:7 skip:26 fi-snb-i7-2600 total:237 pass:190 dwarn:0 dfail:0 fail:7 skip:40 ro-bdw-i5-5250u total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5557U total:237 pass:213 dwarn:2 dfail:0 fail:7 skip:15 ro-bdw-i7-5600u total:237 pass:199 dwarn:0 dfail:0 fail:7 skip:31 ro-bsw-n3050 total:218 pass:170 dwarn:0 dfail:0 fail:5 skip:42 ro-byt-n2820 total:237 pass:189 dwarn:0 dfail:0 fail:10 skip:38 ro-hsw-i3-4010u total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-hsw-i7-4770r total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-ilk-i7-620lm total:237 pass:166 dwarn:0 dfail:0 fail:8 skip:63 ro-ilk1-i5-650 total:232 pass:166 dwarn:0 dfail:0 fail:8 skip:58 ro-ivb-i7-3770 total:237 pass:197 dwarn:0 dfail:0 fail:7 skip:33 ro-skl3-i5-6260u total:237 pass:217 dwarn:1 dfail:0 fail:7 skip:12 ro-snb-i7-2620M total:237 pass:188 dwarn:0 dfail:0 fail:8 skip:41 Results at /archive/results/CI_IGT_test/RO_Patchwork_1473/ 7d4e6e0 drm-intel-nightly: 2016y-07m-12d-11h-23m-08s UTC integration manifest 5bd5ff5 drm/i915: Ignore panel type from OpRegion on SKL ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
On Tue, Jul 12, 2016 at 04:10:22PM +0300, Mika Kuoppala wrote: > Chris Wilson writes: > > > On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display > > power well and so the sna-hda audio codec acquires the display power > > well while it is operational. However, Skylake separates the powerwells > > again, and so we must remember to acquire the rpm wakeref for ourselves > > whilst tweaking the registers. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 > > Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for > > SKL") > > Signed-off-by: Chris Wilson > > Reviewed-by: Mika Kuoppala > > > Cc: Libin Yang > > Cc: Takashi Iwai > > Cc: Marius Vlad Marius, could you provide a tested by? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros
On 12/07/16 10:06, Tvrtko Ursulin wrote: On 11/07/16 19:01, Dave Gordon wrote: We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() provides several other useful intermediate levels such as NOTICE and WARNING. So this patch fills out the set by providing both regular and once-only macros for each of the levels INFO, NOTICE, and WARNING, using a common underlying macro that does all the token-pasting. DRM_ERROR is unchanged, as it's not just a printk wrapper. Signed-off-by: Dave Gordon --- include/drm/drmP.h | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/include/drm/drmP.h b/include/drm/drmP.h index cf918e3e..82648b1 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); /** \name Macros to make printk easier */ /*@{*/ +#define_DRM_PRINTK(once, level, fmt, ...)\ +do {\ +printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ + ##__VA_ARGS__);\ +} while (0) + +#define DRM_INFO(fmt, ...)\ +_DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE(fmt, ...)\ +_DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) To me DRM_NOTICE would be better to keep consistent with kernel naming for the equivalent log level. Maybe, but then we'd probably want DRM_WARNING() as well, and the names get cumbersome, especially when you want to tag "_ONCE" on the end as well. I liked the consistency of {INFO,NOTE,WARN} all being four letters ;) Any comments from dri-devel on INFO/NOTE/WARN vs INFO/NOTICE/WARNING? Or any other suggestions? .Dave. +#define DRM_WARN(fmt, ...)\ +_DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) + +#define DRM_INFO_ONCE(fmt, ...)\ +_DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__) +#define DRM_NOTE_ONCE(fmt, ...)\ +_DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN_ONCE(fmt, ...)\ +_DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) + /** * Error output. * @@ -187,12 +207,6 @@ void drm_err(const char *format, ...); drm_err(fmt, ##__VA_ARGS__);\ }) -#define DRM_INFO(fmt, ...)\ -printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - -#define DRM_INFO_ONCE(fmt, ...)\ -printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - /** * Debug output. * Otherwise acked by me. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Legacy gamma table updates broken in 4.7-rc4
On Wed, Jul 06, 2016 at 12:26:44PM +0200, Mario Kleiner wrote: > A strange one. In Linux 4.7-rc4, at least as build by the Ubuntu mainline > ppa, gamma table updates via RandR don't work. No errors are reported and > the X-Server thinks everything went well, but on Intel Ironlake and > Ivybridge the updates don't have any visual effect. > > The same problem doesn't happen with current drm-next, so something was > fixed. Looking at the new code in intel_color.c i can't see anything obvious > that would break it on 4.7-rc but make it work on drm-next? > > Are there some gamma fixes in drm-next that didn't make it into 4.7-rc yet? Not sure what's going on, but might need a reverse bisect to cherry-pick the fixes over. It's indeed supposed to keep working. Lionel has been working on this, adding him. -Daniel > > Thanks, > -mario > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros
On 12/07/16 14:28, Dave Gordon wrote: On 12/07/16 10:06, Tvrtko Ursulin wrote: On 11/07/16 19:01, Dave Gordon wrote: We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() provides several other useful intermediate levels such as NOTICE and WARNING. So this patch fills out the set by providing both regular and once-only macros for each of the levels INFO, NOTICE, and WARNING, using a common underlying macro that does all the token-pasting. DRM_ERROR is unchanged, as it's not just a printk wrapper. Signed-off-by: Dave Gordon --- include/drm/drmP.h | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/include/drm/drmP.h b/include/drm/drmP.h index cf918e3e..82648b1 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); /** \name Macros to make printk easier */ /*@{*/ +#define_DRM_PRINTK(once, level, fmt, ...)\ +do {\ +printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ + ##__VA_ARGS__);\ +} while (0) + +#define DRM_INFO(fmt, ...)\ +_DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE(fmt, ...)\ +_DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) To me DRM_NOTICE would be better to keep consistent with kernel naming for the equivalent log level. Maybe, but then we'd probably want DRM_WARNING() as well, and the names get cumbersome, especially when you want to tag "_ONCE" on the end as well. I liked the consistency of {INFO,NOTE,WARN} all being four letters ;) Any comments from dri-devel on INFO/NOTE/WARN vs INFO/NOTICE/WARNING? Or any other suggestions? Luckily kernel offers us precedent to avoid the DRM_WARNING verbosity and establish the only exception where log level symbolic name does not match the printk helper name. :) #define pr_emerg(fmt, ...) \ printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__) #define pr_alert(fmt, ...) \ printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__) #define pr_crit(fmt, ...) \ printk(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__) #define pr_err(fmt, ...) \ printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__) #define pr_warning(fmt, ...) \ printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__) #define pr_warn pr_warning #define pr_notice(fmt, ...) \ printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__) #define pr_info(fmt, ...) \ printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) And short form is indeed more popular: $ grep pr_warn\( drivers/ -r | wc -l 1935 $ grep pr_warning\( drivers/ -r | wc -l 141 Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 08/10] drm/i915: Check pixel rate for DP to VGA dongle
On Wed, Jul 06, 2016 at 02:04:52PM +0300, Mika Kahola wrote: > Filter out a mode that exceeds the max pixel rate setting > for DP to VGA dongle. This is defined in DPCD register 0x81 > if detailed cap info i.e. info field is 4 bytes long and > it is available for DP downstream port. > > The register defines the pixel rate divided by 8 in MP/s. > > v2: DPCD read outs and computation moved to drm (Ville, Daniel) > v3: Sink pixel rate computation moved to drm_dp_max_sink_dotclock() > function (Daniel) > v4: Use of drm_dp_helper.c routines to compute max pixel clock (Ville) > v5: Use of intel_dp->downstream_ports to read out port capabilities. > Code restructuring (Ville) > > Signed-off-by: Mika Kahola > --- > drivers/gpu/drm/i915/intel_dp.c | 26 ++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index ffa43ec..76a654e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -190,6 +190,20 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes) > return (max_link_clock * max_lanes * 8) / 10; > } > > +static int > +intel_dp_downstream_max_clock(struct intel_dp *intel_dp, int clock) > +{ > + int dp_ds_clk; > + > + dp_ds_clk = drm_dp_downstream_max_clock(intel_dp->dpcd, > + intel_dp->downstream_ports); > + > + if (dp_ds_clk == 0) > + return clock; > + > + return min(clock, dp_ds_clk); > +} > + > static enum drm_mode_status > intel_dp_mode_valid(struct drm_connector *connector, > struct drm_display_mode *mode) > @@ -201,6 +215,18 @@ intel_dp_mode_valid(struct drm_connector *connector, > int max_rate, mode_rate, max_lanes, max_link_clock; > int max_dotclk = to_i915(connector->dev)->max_dotclk_freq; > > + bool is_branch_device = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & > + DP_DWN_STRM_PORT_PRESENT; > + int type; > + > + if (is_branch_device) { Shouldn't we move this check into the drm dp helper? It can always return 0 for "no downstream port restrictions". > + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; > + > + if (type == DP_DS_PORT_TYPE_VGA) Same here. -Daniel > + max_dotclk = intel_dp_downstream_max_clock(intel_dp, > +max_dotclk); > + } > + > if (is_edp(intel_dp) && fixed_mode) { > if (mode->hdisplay > fixed_mode->hdisplay) > return MODE_PANEL; > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 10/10] drm/i915: Add DP branch device info on debugfs
On Wed, Jul 06, 2016 at 02:04:54PM +0300, Mika Kahola wrote: > Read DisplayPort branch device info from through debugfs > interface. > > v2: use drm_dp_helper routines to collect data > v3: cleanup to match the drm_dp_helper.c patches introduced > earlier in this series > v4: move DP branch device info to function 'intel_dp_branch_device_info()' > > Signed-off-by: Mika Kahola Shouldn't we do any additional debugfs support at the dp helper level too? That way the parsing and debug code are all in the same place. Driver's shouldn't even need to register the debugfs files themselves imo. -Daniel > --- > drivers/gpu/drm/i915/i915_debugfs.c | 75 > + > 1 file changed, 75 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 9989b6a..7f57b3c 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2942,16 +2942,91 @@ static void intel_panel_info(struct seq_file *m, > struct intel_panel *panel) > intel_seq_print_mode(m, 2, mode); > } > > +static void intel_dp_branch_device_info(struct seq_file *m, struct intel_dp > *intel_dp) > +{ > + struct drm_dp_revision rev; > + bool detailed_cap_info = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & > + DP_DETAILED_CAP_INFO_AVAILABLE; > + int type; > + int clk; > + int bpc; > + char id[6]; > + > + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; > + > + switch (type) { > + case DP_DS_PORT_TYPE_DP: > + seq_printf(m, "\ttype: DisplayPort\n"); > + break; > + case DP_DS_PORT_TYPE_VGA: > + seq_printf(m, "\ttype: VGA\n"); > + break; > + case DP_DS_PORT_TYPE_DVI: > + seq_printf(m, "\ttype: DVI\n"); > + break; > + case DP_DS_PORT_TYPE_HDMI: > + seq_printf(m, "\ttype: HDMI\n"); > + break; > + case DP_DS_PORT_TYPE_NON_EDID: > + seq_printf(m, "\ttype: others without EDID support\n"); > + break; > + case DP_DS_PORT_TYPE_DP_DUALMODE: > + seq_printf(m, "\ttype: DP++\n"); > + break; > + case DP_DS_PORT_TYPE_WIRELESS: > + seq_printf(m, "\ttype: Wireless\n"); > + break; > + default: > + seq_printf(m, "\ttype: N/A\n"); > + } > + > + drm_dp_downstream_id(&intel_dp->aux, id); > + seq_printf(m, "\tDevice id: %s\n", id); > + > + rev = drm_dp_downstream_hw_rev(&intel_dp->aux); > + seq_printf(m, "\tHW revision: %.2d.%.2d\n", rev.major, rev.minor); > + > + rev = drm_dp_downstream_sw_rev(&intel_dp->aux); > + seq_printf(m, "\tSW revision: %.2d.%.2d\n", rev.major, rev.minor); > + > + if (detailed_cap_info) { > + clk = drm_dp_downstream_max_clock(intel_dp->dpcd, > + intel_dp->downstream_ports); > + > + if (clk > 0) { > + if (type == DP_DS_PORT_TYPE_VGA) > + seq_printf(m, "\tMax dot clock: %d kHz\n", clk); > + else > + seq_printf(m, "\tMax TMDS clock: %d kHz\n", > clk); > + } > + > + bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd, > + intel_dp->downstream_ports); > + > + if (bpc > 0) > + seq_printf(m, "\tMax bpc: %d\n", bpc); > + } > +} > + > static void intel_dp_info(struct seq_file *m, > struct intel_connector *intel_connector) > { > struct intel_encoder *intel_encoder = intel_connector->encoder; > struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base); > + bool is_branch_device; > > seq_printf(m, "\tDPCD rev: %x\n", intel_dp->dpcd[DP_DPCD_REV]); > seq_printf(m, "\taudio support: %s\n", yesno(intel_dp->has_audio)); > + > if (intel_connector->base.connector_type == DRM_MODE_CONNECTOR_eDP) > intel_panel_info(m, &intel_connector->panel); > + > + is_branch_device = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & > + DP_DWN_STRM_PORT_PRESENT; > + seq_printf(m, "\tbranch device: %s\n", yesno(is_branch_device)); > + > + if (is_branch_device) > + intel_dp_branch_device_info(m, intel_dp); > } > > static void intel_hdmi_info(struct seq_file *m, > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 06/10] drm: Read DP branch device HW revision
On Wed, Jul 06, 2016 at 02:04:50PM +0300, Mika Kahola wrote: > HW revision is mandatory field for DisplayPort branch > devices. This is defined in DPCD register field 0x509. > > Signed-off-by: Mika Kahola > --- > drivers/gpu/drm/drm_dp_helper.c | 21 + > include/drm/drm_dp_helper.h | 7 +++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 4003464..cfd75df 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -514,6 +514,27 @@ int drm_dp_downstream_max_bpc(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > EXPORT_SYMBOL(drm_dp_downstream_max_bpc); > > /** > + * drm_dp_downstream_hw_rev() - read DP branch device HW revision > + * @aux: DisplayPort AUX channel > + * > + * Returns HW revision on succes or negative error code on failure > + */ > +struct drm_dp_revision drm_dp_downstream_hw_rev(struct drm_dp_aux *aux) > +{ > + uint8_t tmp; > + struct drm_dp_revision rev = { .major = -EINVAL, .minor = -EINVAL }; > + > + if (drm_dp_dpcd_read(aux, DP_BRANCH_HW_REV, &tmp, 1) != 1) > + return rev; > + > + rev.major = (tmp & 0xf0) >> 4; > + rev.minor = tmp & 0xf; > + > + return rev; > +} > +EXPORT_SYMBOL(drm_dp_downstream_hw_rev); > + > +/** > * drm_dp_downstream_id() - identify branch device > * @aux: DisplayPort AUX channel > * > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 8264d54..5f577e4 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -446,6 +446,7 @@ > #define DP_SINK_OUI 0x400 > #define DP_BRANCH_OUI0x500 > #define DP_BRANCH_ID0x503 > +#define DP_BRANCH_HW_REV0x509 > > #define DP_SET_POWER0x600 > # define DP_SET_POWER_D00x1 > @@ -803,6 +804,11 @@ struct drm_dp_link { > unsigned long capabilities; > }; > > +struct drm_dp_revision { > + int major; > + int minor; > +}; Atm we have two styles of helpers: - The ones that put decoded values into struct drm_dp_link - A pile of functions that return individual values. This seems to add a third one. I guess it'd be better to just move everything over to putting parsed values into drm_dp_link and fill that out for everything ... -Daniel > + > int drm_dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link); > int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link); > int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link); > @@ -812,6 +818,7 @@ int drm_dp_downstream_max_clock(const u8 > dpcd[DP_RECEIVER_CAP_SIZE], > int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE], > const u8 port_cap[4]); > int drm_dp_downstream_id(struct drm_dp_aux *aux, char id[6]); > +struct drm_dp_revision drm_dp_downstream_hw_rev(struct drm_dp_aux *aux); > > void drm_dp_aux_init(struct drm_dp_aux *aux); > int drm_dp_aux_register(struct drm_dp_aux *aux); > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/64] drm: Restore double clflush on the last partial cacheline
On Thu, Jul 07, 2016 at 09:41:12AM +0100, Chris Wilson wrote: > This effectively reverts > > commit afcd950cafea6e27b739fe7772cbbeed37d05b8b > Author: Chris Wilson > Date: Wed Jun 10 15:58:01 2015 +0100 > > drm: Avoid the double clflush on the last cache line in > drm_clflush_virt_range() > > as we have observed issues with serialisation of the clflush operations > on Baytrail+ Atoms with partial updates. Applying the double flush on the > last cacheline forces that clflush to be ordered with respect to the > previous clflush, and the mfence then protects against prefetches crossing > the clflush boundary. > > The same issue can be demonstrated in userspace with igt/gem_exec_flush. > > Fixes: afcd950cafea6 (drm: Avoid the double clflush on the last cache...) > Testcase: igt/gem_concurrent_blit > Testcase: igt/gem_partial_pread_pwrite > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92845 > Signed-off-by: Chris Wilson > Cc: dri-de...@lists.freedesktop.org > Cc: Akash Goel > Cc: Imre Deak > Cc: Daniel Vetter > Cc: Jason Ekstrand > Cc: sta...@vger.kernel.org > Reviewed-by: Mika Kuoppala It's duct-tape, but oh well. Applied to drm-misc. -Daniel > --- > drivers/gpu/drm/drm_cache.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index 059f7c39c582..a7916e5f8864 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -136,6 +136,7 @@ drm_clflush_virt_range(void *addr, unsigned long length) > mb(); > for (; addr < end; addr += size) > clflushopt(addr); > + clflushopt(end - 1); /* force serialisation */ > mb(); > return; > } > -- > 2.8.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 09/10] drm/i915: Update bits per component for display info
On Wed, Jul 06, 2016 at 02:04:53PM +0300, Mika Kahola wrote: > DisplayPort branch device may define max supported bits per > component. Update display info based on this value if bpc > is defined. > > v2: cleanup to match the drm_dp_helper.c patches introduced > earlier in this series > > Signed-off-by: Mika Kahola > --- > drivers/gpu/drm/i915/intel_dp.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 76a654e..53ec844 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3932,6 +3932,14 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > uint8_t *dpcd = intel_dp->dpcd; > uint8_t type; > > + if (dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT) { > + int bpc = drm_dp_downstream_max_bpc(dpcd, > + intel_dp->downstream_ports); > + > + if (bpc > 0) > + intel_dp->attached_connector->base.display_info.bpc = > bpc; > + } I think a function in the dp helpers to correctly fill out the connector's display info would be neater. -Daniel > + > if (!intel_dp_get_dpcd(intel_dp)) > return connector_status_disconnected; > > @@ -3968,6 +3976,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) > return connector_status_unknown; > } > > + > /* Anything else is out of spec, warn and ignore */ > DRM_DEBUG_KMS("Broken DP branch device, ignoring\n"); > return connector_status_disconnected; > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)
On Tue, Jul 12, 2016 at 12:04:03PM +0100, Chris Wilson wrote: > On Tue, Jul 12, 2016 at 12:44:17PM +0200, Daniel Vetter wrote: > > On Mon, Jul 11, 2016 at 04:24:45PM +0100, Chris Wilson wrote: > > > That doesn't fit the out-of-order unbound nature of the interface. The > > > interface is just a collection of fences that userspace associates with > > > the buffer that it may signal at any time. (Having no strict timeline is > > > an advantage!) > > > > Fences on the same timeline are supposed to be signalled in-order. If you > > want full out-of-order fences then you need to grab a new timeline number > > for each one. Drivers can and do merge fences on the same timeline and > > just carry the one with the largest seqno around. > > Ugh. Timelines simply don't mean anything everywhere - a very leaky > abstration. > > Nevertheless, a fence_context per vgem_fence would do the trick. Yeah it's a bit meh, but allocating plenty of them is how we currently cope with it. I suggested that we have a special FENCE_TIMELINE_UNORDERED flag (we need it for fence_array too), but that wasn't popular. I still expect it to happen eventually ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros
On Mon, Jul 11, 2016 at 07:01:27PM +0100, Dave Gordon wrote: > We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() > provides several other useful intermediate levels such as NOTICE and > WARNING. So this patch fills out the set by providing both regular and > once-only macros for each of the levels INFO, NOTICE, and WARNING, using > a common underlying macro that does all the token-pasting. > > DRM_ERROR is unchanged, as it's not just a printk wrapper. > > Signed-off-by: Dave Gordon I'm not sure what exactly the brave new drm debug model should look like (probably some form of pimped dynamic debug printk, to be able to be backwards compatible with the gazillion of blog posts recommending to capture dmesg with drm.debug=0xe). But extending these is probably not what we want ... -Daniel > --- > include/drm/drmP.h | 26 -- > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index cf918e3e..82648b1 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); > /** \name Macros to make printk easier */ > /*@{*/ > > +#define _DRM_PRINTK(once, level, fmt, ...) > \ > + do {\ > + printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ > + ##__VA_ARGS__);\ > + } while (0) > + > +#define DRM_INFO(fmt, ...) \ > + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) > +#define DRM_NOTE(fmt, ...) \ > + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) > +#define DRM_WARN(fmt, ...) \ > + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) > + > +#define DRM_INFO_ONCE(fmt, ...) > \ > + _DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__) > +#define DRM_NOTE_ONCE(fmt, ...) \ > + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) > +#define DRM_WARN_ONCE(fmt, ...) > \ > + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) > + > /** > * Error output. > * > @@ -187,12 +207,6 @@ void drm_err(const char *format, ...); > drm_err(fmt, ##__VA_ARGS__);\ > }) > > -#define DRM_INFO(fmt, ...) \ > - printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > - > -#define DRM_INFO_ONCE(fmt, ...) \ > - printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > - > /** > * Debug output. > * > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 46/64] drm/i915: Count how many VMA are bound for an object
On 07/07/16 09:41, Chris Wilson wrote: Since we may have VMA allocated for an object, but we interrupted their binding, there is a disparity between have elements on the obj->vma_list and being bound. i915_gem_obj_bound_any() does this check, but this is not rigorously observed - add an explicit count to make it easier. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_debugfs.c | 12 +-- drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/i915_gem.c | 34 +--- drivers/gpu/drm/i915/i915_gem_shrinker.c | 17 +--- drivers/gpu/drm/i915/i915_gem_stolen.c | 1 + 5 files changed, 23 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 327064823bb0..d3852828878f 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -174,6 +174,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) if (obj->fence_reg != I915_FENCE_REG_NONE) seq_printf(m, " (fence: %d)", obj->fence_reg); list_for_each_entry(vma, &obj->vma_list, obj_link) { + if (!drm_mm_node_allocated(&vma->node)) + continue; + seq_printf(m, " (%sgtt offset: %08llx, size: %08llx", vma->is_ggtt ? "g" : "pp", vma->node.start, vma->node.size); @@ -335,11 +338,11 @@ static int per_file_stats(int id, void *ptr, void *data) struct drm_i915_gem_object *obj = ptr; struct file_stats *stats = data; struct i915_vma *vma; - int bound = 0; stats->count++; stats->total += obj->base.size; - + if (!obj->bind_count) + stats->unbound += obj->base.size; if (obj->base.name || obj->base.dma_buf) stats->shared += obj->base.size; @@ -347,8 +350,6 @@ static int per_file_stats(int id, void *ptr, void *data) if (!drm_mm_node_allocated(&vma->node)) continue; - bound++; - if (vma->is_ggtt) { stats->global += vma->node.size; } else { @@ -366,9 +367,6 @@ static int per_file_stats(int id, void *ptr, void *data) stats->inactive += vma->node.size; } - if (!bound) - stats->unbound += obj->base.size; - return 0; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a19d18ea074e..633585054669 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2217,6 +2217,8 @@ struct drm_i915_gem_object { unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS; unsigned int has_wc_mmap; + /** Count of VMA actually bound by this object */ + unsigned int bind_count; unsigned int pin_display; struct sg_table *pages; @@ -3246,7 +3248,6 @@ i915_gem_obj_ggtt_offset(struct drm_i915_gem_object *o) return i915_gem_obj_ggtt_offset_view(o, &i915_ggtt_view_normal); } -bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o); bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o, const struct i915_ggtt_view *view); bool i915_gem_obj_bound(struct drm_i915_gem_object *o, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 98f3945fc50f..c6816f9969d5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2107,7 +2107,7 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj) if (obj->pages_pin_count) return -EBUSY; - BUG_ON(i915_gem_obj_bound_any(obj)); + GEM_BUG_ON(obj->bind_count); /* ->put_pages might need to allocate memory for the bit17 swizzle * array, hence protect them from being reaped by removing them from gtt @@ -2957,7 +2957,6 @@ static void __i915_vma_iounmap(struct i915_vma *vma) static int __i915_vma_unbind(struct i915_vma *vma, bool wait) { struct drm_i915_gem_object *obj = vma->obj; - struct drm_i915_private *dev_priv = to_i915(obj->base.dev); int ret; if (list_empty(&vma->obj_link)) @@ -2971,7 +2970,8 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait) if (vma->pin_count) return -EBUSY; - BUG_ON(obj->pages == NULL); + GEM_BUG_ON(obj->bind_count == 0); + GEM_BUG_ON(!obj->pages); if (wait) { ret = i915_gem_object_wait_rendering(obj, false); @@ -3011,8 +3011,9 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait) /* Since the unbound list is global, only move to that list if * no more VMAs exist. */ - if (list_empty(&obj->vma_list)) - list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list); + if (--obj->bind_count == 0) +
Re: [Intel-gfx] [PATCH 46/64] drm/i915: Count how many VMA are bound for an object
On Tue, Jul 12, 2016 at 03:30:25PM +0100, Tvrtko Ursulin wrote: > On 07/07/16 09:41, Chris Wilson wrote: > >@@ -3684,6 +3684,9 @@ i915_gem_object_pin_to_display_plane(struct > >drm_i915_gem_object *obj, > > old_read_domains, > > old_write_domain); > > > >+/* Increment the pages_pin_count to guard against the shrinker */ > >+obj->pages_pin_count++; > > Would it be clearer to look at obj->pin_display in the shrinker? > Although it looks like special casing out of the cleanliness of the > design in both case so I am not sure. Yeah. I liked the mechanism of telling the shrinker to avoid certain pages by only having to control the pages_pin_count. It feels easier to explain to others "the shrinker may reap any object that hasn't pinned its pages" (explicitly called i915_gem_object_pin_pages for its own use) rather than that + plus a list of exceptions known by the shrinker. > >@@ -82,7 +67,7 @@ static bool can_release_pages(struct drm_i915_gem_object > >*obj) > > * to the GPU, simply unbinding from the GPU is not going to succeed > > * in releasing our pin count on the pages themselves. > > */ > >-if (obj->pages_pin_count != num_vma_bound(obj)) > >+if (obj->pages_pin_count != obj->bind_count) > > Would this be clearer as obj->pages_pin_count > obj->bind_count ? Ok. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros
On Tue, Jul 12, 2016 at 03:53:55PM +0100, Dave Gordon wrote: > On 12/07/16 15:25, Daniel Vetter wrote: > > On Mon, Jul 11, 2016 at 07:01:27PM +0100, Dave Gordon wrote: > > > We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() > > > provides several other useful intermediate levels such as NOTICE and > > > WARNING. So this patch fills out the set by providing both regular and > > > once-only macros for each of the levels INFO, NOTICE, and WARNING, using > > > a common underlying macro that does all the token-pasting. > > > > > > DRM_ERROR is unchanged, as it's not just a printk wrapper. > > > > > > Signed-off-by: Dave Gordon > > > > I'm not sure what exactly the brave new drm debug model should look like > > (probably some form of pimped dynamic debug printk, to be able to be > > backwards compatible with the gazillion of blog posts recommending to > > capture dmesg with drm.debug=0xe). But extending these is probably not > > what we want ... > > -Daniel > > These are not debug of any sort, these message are intended to be seen by > the user (or administrator), and these macros allow us to emit the messages > at the most appropriate kernel message level. Hm ok, I guess we can extend them for that. -Daniel > > .Dave. > > > > --- > > > include/drm/drmP.h | 26 -- > > > 1 file changed, 20 insertions(+), 6 deletions(-) > > > > > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > > > index cf918e3e..82648b1 100644 > > > --- a/include/drm/drmP.h > > > +++ b/include/drm/drmP.h > > > @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); > > > /** \name Macros to make printk easier */ > > > /*@{*/ > > > > > > +#define _DRM_PRINTK(once, level, fmt, ...) > > > \ > > > + do {\ > > > + printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ > > > + ##__VA_ARGS__);\ > > > + } while (0) > > > + > > > +#define DRM_INFO(fmt, ...) > > > \ > > > + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) > > > +#define DRM_NOTE(fmt, ...) > > > \ > > > + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) > > > +#define DRM_WARN(fmt, ...) > > > \ > > > + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) > > > + > > > +#define DRM_INFO_ONCE(fmt, ...) > > > \ > > > + _DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__) > > > +#define DRM_NOTE_ONCE(fmt, ...) \ > > > + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) > > > +#define DRM_WARN_ONCE(fmt, ...) > > > \ > > > + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) > > > + > > > /** > > >* Error output. > > >* > > > @@ -187,12 +207,6 @@ void drm_err(const char *format, ...); > > > drm_err(fmt, ##__VA_ARGS__); > > > \ > > > }) > > > > > > -#define DRM_INFO(fmt, ...) \ > > > - printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > > > - > > > -#define DRM_INFO_ONCE(fmt, ...) \ > > > - printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) > > > - > > > /** > > >* Debug output. > > >* > > > -- > > > 1.9.1 > > > > > > ___ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros
On 12/07/16 15:25, Daniel Vetter wrote: On Mon, Jul 11, 2016 at 07:01:27PM +0100, Dave Gordon wrote: We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() provides several other useful intermediate levels such as NOTICE and WARNING. So this patch fills out the set by providing both regular and once-only macros for each of the levels INFO, NOTICE, and WARNING, using a common underlying macro that does all the token-pasting. DRM_ERROR is unchanged, as it's not just a printk wrapper. Signed-off-by: Dave Gordon I'm not sure what exactly the brave new drm debug model should look like (probably some form of pimped dynamic debug printk, to be able to be backwards compatible with the gazillion of blog posts recommending to capture dmesg with drm.debug=0xe). But extending these is probably not what we want ... -Daniel These are not debug of any sort, these message are intended to be seen by the user (or administrator), and these macros allow us to emit the messages at the most appropriate kernel message level. .Dave. --- include/drm/drmP.h | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/include/drm/drmP.h b/include/drm/drmP.h index cf918e3e..82648b1 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); /** \name Macros to make printk easier */ /*@{*/ +#define_DRM_PRINTK(once, level, fmt, ...) \ + do {\ + printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ +##__VA_ARGS__);\ + } while (0) + +#define DRM_INFO(fmt, ...) \ + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE(fmt, ...) \ + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN(fmt, ...) \ + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) + +#define DRM_INFO_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__) +#define DRM_NOTE_ONCE(fmt, ...)\ + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) + /** * Error output. * @@ -187,12 +207,6 @@ void drm_err(const char *format, ...); drm_err(fmt, ##__VA_ARGS__);\ }) -#define DRM_INFO(fmt, ...) \ - printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - -#define DRM_INFO_ONCE(fmt, ...)\ - printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - /** * Debug output. * -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix legacy gamma lut updates in Linux 4.7-rc6
On 12/07/16 13:11, Mario Kleiner wrote: On 07/12/2016 12:50 PM, Lionel Landwerlin wrote: Hi Mario, Hi Lionel, There was a couple of patch to fix this issue : https://patchwork.freedesktop.org/series/5467/ https://patchwork.freedesktop.org/series/5466/ Looking at them they should fix the issue, but they seem to be stuck in review? I tested this late last week on drm-intel-nightly, it seems a series of revert fixed most of the issues. You mean something else has fixed legacy gamma updates, as i can't find above patches applied on drm-intel-nightly? This revert on drm-intel-nightly seems to have fixed the problem : https://cgit.freedesktop.org/drm-intel/commit/drivers/gpu/drm/i915?id=e42aeef1237b7c969a77b7f726c50f6cb832185f Are those fixes supposed to be already part of 4.7-rc7, the final rc afaik? I haven't seen it on 4.7-rc7. thanks, -mario Cheers, - Lionel On 12/07/16 11:33, Mario Kleiner wrote: Updating legacy gamma tables, e.g., via RandR doesn't work at all as of Linux 4.7-rc6. Reason seems to be that the required call to drm_atomic_helper_commit_planes_on_crtc is skipped in intel_atomic_commit after userspace set new gamma tables, because neither crtc->state->planes_changed nor update_pipe (= pipe_config->update_pipe) are true. Removing the check for planes_changed || update_pipe fixes gamma table updates. The code for Linux 4.8 drm-next has changed a lot in that area wrt. 4.7, but the new code for 4.8 also removed those checks and calls drm_atomic_helper_commit_planes_on_crtc unconditionally, and legacy gamma lut updates work on drm-next, so this seems to be the right solution. Tested also shutdown/reboot, suspend/resume, (un-)plugging displays, mode switches for resolution/refresh rate, display rotation, and page-flipping/pageflip timing on Intel HD Ironlake to confirm the fix apparently doesn't break anything under X11. Signed-off-by: Mario Kleiner Cc: Maarten Lankhorst Cc: Lionel Landwerlin Cc: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 04452cf..eb8fb36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13685,7 +13685,6 @@ static int intel_atomic_commit(struct drm_device *dev, bool modeset = needs_modeset(crtc->state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state); -bool update_pipe = !modeset && pipe_config->update_pipe; if (modeset && crtc->state->active) { update_scanline_offset(to_intel_crtc(crtc)); @@ -13699,8 +13698,7 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_get_existing_plane_state(state, crtc->primary)) intel_fbc_enable(intel_crtc); -if (crtc->state->active && -(crtc->state->planes_changed || update_pipe)) +if (crtc->state->active) drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); if (pipe_config->base.active && needs_vblank_wait(pipe_config)) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 47/64] drm/i915: Be more careful when unbinding vma
On 07/07/16 09:41, Chris Wilson wrote: When we call i915_vma_unbind(), we will wait upon outstanding rendering. This will also trigger a retirement phase, which may update the object lists. If, we extend request tracking to the VMA itself (rather than keep it at the encompassing object), then there is a potential that the obj->vma_list be modified for other elements upon i915_vma_unbind(). As a result, if we walk over the object list and call i915_vma_unbind(), we need to be prepared for that list to change. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_gem.c | 57 +++- drivers/gpu/drm/i915/i915_gem_shrinker.c | 7 +--- drivers/gpu/drm/i915/i915_gem_userptr.c | 4 +-- 4 files changed, 46 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 633585054669..27e1182544a2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3032,6 +3032,8 @@ int __must_check i915_vma_unbind(struct i915_vma *vma); * _guarantee_ VMA in question is _not in use_ anywhere. */ int __must_check __i915_vma_unbind_no_wait(struct i915_vma *vma); + +int i915_gem_object_unbind(struct drm_i915_gem_object *obj); int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv); void i915_gem_release_mmap(struct drm_i915_gem_object *obj); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c6816f9969d5..28a3079a7892 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -283,18 +283,38 @@ static const struct drm_i915_gem_object_ops i915_gem_phys_ops = { .release = i915_gem_object_release_phys, }; +int +i915_gem_object_unbind(struct drm_i915_gem_object *obj) +{ + struct i915_vma *vma; + LIST_HEAD(still_in_list); + int ret; + + /* The vma will only be freed if it is marked as closed, and if we wait +* upon rendering to the vma, we may unbind anything in the list. +*/ + while ((vma = list_first_entry_or_null(&obj->vma_list, + struct i915_vma, + obj_link))) { + list_move_tail(&vma->obj_link, &still_in_list); + ret = i915_vma_unbind(vma); + if (ret) + break; + } + list_splice(&still_in_list, &obj->vma_list); + + return ret; +} + static int drop_pages(struct drm_i915_gem_object *obj) { - struct i915_vma *vma, *next; int ret; i915_gem_object_get(obj); - list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link) - if (i915_vma_unbind(vma)) - break; - - ret = i915_gem_object_put_pages(obj); + ret = i915_gem_object_unbind(obj); + if (ret == 0) + ret = i915_gem_object_put_pages(obj); i915_gem_object_put(obj); return ret; @@ -3442,8 +3462,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write) int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, enum i915_cache_level cache_level) { - struct drm_device *dev = obj->base.dev; - struct i915_vma *vma, *next; + struct i915_vma *vma; int ret = 0; if (obj->cache_level == cache_level) @@ -3454,7 +3473,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, * catch the issue of the CS prefetch crossing page boundaries and * reading an invalid PTE on older architectures. */ - list_for_each_entry_safe(vma, next, &obj->vma_list, obj_link) { +restart: + list_for_each_entry(vma, &obj->vma_list, obj_link) { if (!drm_mm_node_allocated(&vma->node)) continue; @@ -3463,11 +3483,18 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, return -EBUSY; } - if (!i915_gem_valid_gtt_space(vma, cache_level)) { - ret = i915_vma_unbind(vma); - if (ret) - return ret; - } + if (i915_gem_valid_gtt_space(vma, cache_level)) + continue; + + ret = i915_vma_unbind(vma); + if (ret) + return ret; + + /* As unbinding may affect other elements in the +* obj->vma_list (due to side-effects from retiring +* an active vma), play safe and restart the iterator. +*/ + goto restart; } Does not look efficient for long lists but I don't see a solution right now. Any chance of this O(N^2) iteration hurting us in the real
Re: [Intel-gfx] [PATCH 3/3] drm/i915/guc: revisit GuC loader message levels
On 12/07/16 10:26, Tvrtko Ursulin wrote: On 11/07/16 19:01, Dave Gordon wrote: Some downgraded from DRM_ERROR() to DRM_WARN(), some eliminated, and a few upgraded from DRM_INFO() to DRM_NOTE() or DRM_WARN(). Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/intel_guc_loader.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 605c696..fd032eb 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -140,12 +140,14 @@ static u32 get_gttype(struct drm_i915_private *dev_priv) static u32 get_core_family(struct drm_i915_private *dev_priv) { -switch (INTEL_INFO(dev_priv)->gen) { +u32 gen = INTEL_GEN(dev_priv); + +switch (gen) { case 9: return GFXCORE_FAMILY_GEN9; default: -DRM_ERROR("GUC: unsupported core family\n"); +DRM_WARN("GEN%d does not support GuC operation\n", gen); return GFXCORE_FAMILY_UNKNOWN; } } @@ -433,7 +435,7 @@ int intel_guc_setup(struct drm_device *dev) goto fail; } else if (*fw_path == '\0') { /* Device has a GuC but we don't know what f/w to load? */ -DRM_INFO("No GuC firmware known for this platform\n"); +DRM_WARN("No GuC firmware known for this platform\n"); err = -ENODEV; goto fail; } @@ -471,10 +473,8 @@ int intel_guc_setup(struct drm_device *dev) * that the state and timing are fairly predictable */ err = i915_reset_guc(dev_priv); -if (err) { -DRM_ERROR("GuC reset failed: %d\n", err); +if (err) goto fail; -} err = guc_ucode_xfer(dev_priv); if (!err) @@ -532,15 +532,15 @@ int intel_guc_setup(struct drm_device *dev) else if (err == 0) DRM_INFO("GuC firmware load skipped\n"); else if (ret != -EIO) -DRM_INFO("GuC firmware load failed: %d\n", err); +DRM_NOTE("GuC firmware load failed: %d\n", err); else -DRM_ERROR("GuC firmware load failed: %d\n", err); +DRM_WARN("GuC firmware load failed: %d\n", err); if (i915.enable_guc_submission) { if (fw_path == NULL) DRM_INFO("GuC submission without firmware not supported\n"); if (ret == 0) -DRM_INFO("Falling back from GuC submission to execlist mode\n"); +DRM_NOTE("Falling back from GuC submission to execlist mode\n"); else DRM_ERROR("GuC init failed: %d\n", ret); } @@ -656,7 +656,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw) fail: DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n", err, fw, guc_fw->guc_fw_obj); -DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n", +DRM_WARN("Failed to fetch GuC firmware from %s (error %d)\n", guc_fw->guc_fw_path, err); mutex_lock(&dev->struct_mutex); R-b if you also change all the other DRM_ERRORs in guc_fw_fetch to DRM_DEBUG_DRIVER and merge this last two log lines (DRM_DEBUG_DRIVER + DRM_WARN) to one. :) Regards, Tvrtko No, that wouldn't be appropriate. We want the user to be informed if any of these failures occurs, because it means their system is in some way misconfigured e.g. corrupted firmware file. That's definitely not a DEBUG-only event, and it must be logged even if we're going to try to continue in fallback mode. I could change all the earlier ERRORs to NOTEs and leave just the last one as an ERROR i.e. explanation first, consequence after. As for the DEBUG, that's for a different purpose. Whereas the various ERROR/NOTE/INFO messages relate to the existence, format, or content of the required firmware file in the filesystem or ramdisk, the DEBUG is about internal failures such as not being able to allocate memory, over which the user/administrator has no direct control. I might swap them round though (i.e. DEBUG after the ERROR, to explain further than I want to in a user-facing message). .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 46/64] drm/i915: Count how many VMA are bound for an object
On 12/07/16 15:38, Chris Wilson wrote: On Tue, Jul 12, 2016 at 03:30:25PM +0100, Tvrtko Ursulin wrote: On 07/07/16 09:41, Chris Wilson wrote: @@ -3684,6 +3684,9 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, old_read_domains, old_write_domain); + /* Increment the pages_pin_count to guard against the shrinker */ + obj->pages_pin_count++; Would it be clearer to look at obj->pin_display in the shrinker? Although it looks like special casing out of the cleanliness of the design in both case so I am not sure. Yeah. I liked the mechanism of telling the shrinker to avoid certain pages by only having to control the pages_pin_count. It feels easier to explain to others "the shrinker may reap any object that hasn't pinned its pages" (explicitly called i915_gem_object_pin_pages for its own use) rather than that + plus a list of exceptions known by the shrinker. Hm but wait a minute, framebuffer already has the VMA pinned. So how will the shrinker reap it? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 48/64] drm/i915: Kill drop_pages()
On 07/07/16 09:41, Chris Wilson wrote: The drop_pages() function is a dangerous trap in that it can release the passed in object pointer and so unless the caller is aware, it can easily trick us into using the stale object afterwards. Move it into its solitary callsite where we know it is safe. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 20 +--- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 28a3079a7892..b6497e9961ab 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -306,20 +306,6 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj) return ret; } -static int -drop_pages(struct drm_i915_gem_object *obj) -{ - int ret; - - i915_gem_object_get(obj); - ret = i915_gem_object_unbind(obj); - if (ret == 0) - ret = i915_gem_object_put_pages(obj); - i915_gem_object_put(obj); - - return ret; -} - int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align) @@ -340,7 +326,11 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, if (obj->base.filp == NULL) return -EINVAL; - ret = drop_pages(obj); + ret = i915_gem_object_unbind(obj); + if (ret) + return ret; + + ret = i915_gem_object_put_pages(obj); if (ret) return ret; Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Add support for the SAGV, fix underrun hangs
On Mon, Jul 11, 2016 at 06:00:46PM -0400, Lyude wrote: > Since the watermark calculations for Skylake are still broken, we're apt > to hitting underruns very easily under multi-monitor configurations. > While it would be lovely if this was fixed, it's not. Another problem > that's been coming from this however, is the mysterious issue of > underruns causing full system hangs. An easy way to reproduce this with > a skylake system: > > - Get a laptop with a skylake GPU, and hook up two external monitors to > it > - Move the cursor from the built-in LCD to one of the external displays > as quickly as you can > - You'll get a few pipe underruns, and eventually the entire system will > just freeze. > > After doing a lot of investigation and reading through the bspec, I > found the existence of the SAGV, which is responsible for adjusting the > system agent voltage and clock frequencies depending on how much power > we need. According to the bspec: > > "The display engine access to system memory is blocked during the > adjustment time. SAGV defaults to enabled. Software must use the > GT-driver pcode mailbox to disable SAGV when the display engine is not > able to tolerate the blocking time." > > The rest of the bspec goes on to explain that software can simply leave > the SAGV enabled, and disable it when we use interlaced pipes/have more > then one pipe active. > > Sure enough, with this patchset the system hangs resulting from pipe > underruns on Skylake have completely vanished on my T460s. > > Cc: Daniel Vetter > Cc: Ville Syrjälä > Signed-off-by: Lyude > --- > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/i915_reg.h | 5 ++ > drivers/gpu/drm/i915/intel_pm.c | 103 > > 3 files changed, 110 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 03e1bfa..660d0a6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1959,6 +1959,8 @@ struct drm_i915_private { > struct i915_suspend_saved_registers regfile; > struct vlv_s0ix_state vlv_s0ix_state; > > + bool skl_sagv_enabled; > + > struct { > /* >* Raw watermark latency values: > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8bfde75..9b2eb0b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7162,6 +7162,11 @@ enum { > #define HSW_PCODE_DE_WRITE_FREQ_REQ0x17 > #define DISPLAY_IPS_CONTROL0x19 > #defineHSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A > +#define GEN9_PCODE_SAGV_CONTROL0x21 > +#define GEN9_SAGV_DISABLE0x0 > +#define GEN9_SAGV_LOW_FREQ 0x1 > +#define GEN9_SAGV_HIGH_FREQ 0x2 > +#define GEN9_SAGV_DYNAMIC_FREQ 0x3 > #define GEN6_PCODE_DATA _MMIO(0x138128) > #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 > #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5a8ee0c..07807aa 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2876,6 +2876,102 @@ skl_wm_plane_id(const struct intel_plane *plane) > } > > static void > +skl_sagv_get_hw_state(struct drm_i915_private *dev_priv) > +{ > + u32 temp; > + int ret; > + > + mutex_lock(&dev_priv->rps.hw_lock); > + ret = sandybridge_pcode_read(dev_priv, GEN9_PCODE_SAGV_CONTROL, &temp); > + mutex_unlock(&dev_priv->rps.hw_lock); > + > + if (!ret) { > + dev_priv->skl_sagv_enabled = !!(temp & GEN9_SAGV_DYNAMIC_FREQ); > + } else { > + /* > + * If for some reason we can't access the SAGV state, follow > + * the bspec and assume it's enabled > + */ > + DRM_ERROR("Failed to get SAGV state, assuming enabled\n"); > + dev_priv->skl_sagv_enabled = true; > + } > +} > + > +/* > + * SAGV dynamically adjusts the system agent voltage and clock frequencies > + * depending on power and performance requirements. The display engine access > + * to system memory is blocked during the adjustment time. Having this > enabled > + * in multi-pipe configurations can cause issues (such as underruns causing > + * full system hangs), and the bspec also suggests that software disable it > + * when more then one pipe is enabled. > + */ > +static int > +skl_enable_sagv(struct drm_i915_private *dev_priv) > +{ > + int ret; > + > + if (dev_priv->skl_sagv_enabled) > + return 0; > + > + mutex_lock(&dev_priv->rps.hw_lock); > + DRM_DEBUG_KMS("Enabling the SAGV\n"); > + > + ret = sandybridge_pcode_write(dev_priv, GEN9_PCODE_SAGV_CONTROL, > + GEN9_SAGV_DYNAMIC_FRE
Re: [Intel-gfx] [PATCH] drm/i915: Fix legacy gamma lut updates in Linux 4.7-rc6
On 07/12/2016 05:02 PM, Lionel Landwerlin wrote: On 12/07/16 13:11, Mario Kleiner wrote: On 07/12/2016 12:50 PM, Lionel Landwerlin wrote: Hi Mario, Hi Lionel, There was a couple of patch to fix this issue : https://patchwork.freedesktop.org/series/5467/ https://patchwork.freedesktop.org/series/5466/ Looking at them they should fix the issue, but they seem to be stuck in review? I tested this late last week on drm-intel-nightly, it seems a series of revert fixed most of the issues. You mean something else has fixed legacy gamma updates, as i can't find above patches applied on drm-intel-nightly? This revert on drm-intel-nightly seems to have fixed the problem : https://cgit.freedesktop.org/drm-intel/commit/drivers/gpu/drm/i915?id=e42aeef1237b7c969a77b7f726c50f6cb832185f Ok, with that intel-nightly looks like drm-next for 4.8 and that indeed has working lut updates in my testing. My own patch was motivated by the way the implementation is done in intel_atomic_commit_tail() from drm-next. Are those fixes supposed to be already part of 4.7-rc7, the final rc afaik? I haven't seen it on 4.7-rc7. I just checked Linus tree for 4.7-rc7 and there the code in intel_display.c didn't receive any updates since 13 days and looks like the broken code from rc6 which according to my testing doesn't work. So i'd assume legacy gamma table updates are broken in Linux 4.7 final rc atm. Couldn't test, because for some weird reason 4.7-rc7 doesn't even boot on my laptop :( - However i got that via a quick install from Ubuntu's mainline ppa so it could be some unrelated problem with their ppa builds. I think either my patch would fix it, but is untested wrt. nuclear pageflip, or those two patches you referenced, which apparently didn't move forward. What now? -mario thanks, -mario Cheers, - Lionel On 12/07/16 11:33, Mario Kleiner wrote: Updating legacy gamma tables, e.g., via RandR doesn't work at all as of Linux 4.7-rc6. Reason seems to be that the required call to drm_atomic_helper_commit_planes_on_crtc is skipped in intel_atomic_commit after userspace set new gamma tables, because neither crtc->state->planes_changed nor update_pipe (= pipe_config->update_pipe) are true. Removing the check for planes_changed || update_pipe fixes gamma table updates. The code for Linux 4.8 drm-next has changed a lot in that area wrt. 4.7, but the new code for 4.8 also removed those checks and calls drm_atomic_helper_commit_planes_on_crtc unconditionally, and legacy gamma lut updates work on drm-next, so this seems to be the right solution. Tested also shutdown/reboot, suspend/resume, (un-)plugging displays, mode switches for resolution/refresh rate, display rotation, and page-flipping/pageflip timing on Intel HD Ironlake to confirm the fix apparently doesn't break anything under X11. Signed-off-by: Mario Kleiner Cc: Maarten Lankhorst Cc: Lionel Landwerlin Cc: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 04452cf..eb8fb36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13685,7 +13685,6 @@ static int intel_atomic_commit(struct drm_device *dev, bool modeset = needs_modeset(crtc->state); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->state); -bool update_pipe = !modeset && pipe_config->update_pipe; if (modeset && crtc->state->active) { update_scanline_offset(to_intel_crtc(crtc)); @@ -13699,8 +13698,7 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_get_existing_plane_state(state, crtc->primary)) intel_fbc_enable(intel_crtc); -if (crtc->state->active && -(crtc->state->planes_changed || update_pipe)) +if (crtc->state->active) drm_atomic_helper_commit_planes_on_crtc(old_crtc_state); if (pipe_config->base.active && needs_vblank_wait(pipe_config)) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 49/64] drm/i915: Introduce i915_gem_active for request tracking
On 07/07/16 09:41, Chris Wilson wrote: In the next patch, request tracking is made more generic and for that we need a new expanded struct and to separate out the logic changes from the mechanical churn, we split out the structure renaming into this patch. v2: Writer's block. Add some spiel about why we track requests. v3: Now i915_gem_active. v4: Now with i915_gem_active_set() for attaching to the active request. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_debugfs.c| 11 +++--- drivers/gpu/drm/i915/i915_drv.h| 9 +++-- drivers/gpu/drm/i915/i915_gem.c| 58 +++--- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 8 +++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +-- drivers/gpu/drm/i915/i915_gem_fence.c | 6 ++-- drivers/gpu/drm/i915/i915_gem_request.h| 41 + drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +- drivers/gpu/drm/i915/i915_gem_userptr.c| 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 7 ++-- drivers/gpu/drm/i915/intel_display.c | 8 ++--- 11 files changed, 98 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d3852828878f..dd832eace487 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -155,10 +155,10 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) obj->base.write_domain); for_each_engine_id(engine, dev_priv, id) seq_printf(m, "%x ", - i915_gem_request_get_seqno(obj->last_read_req[id])); + i915_gem_request_get_seqno(obj->last_read[id].request)); seq_printf(m, "] %x %x%s%s%s", - i915_gem_request_get_seqno(obj->last_write_req), - i915_gem_request_get_seqno(obj->last_fenced_req), + i915_gem_request_get_seqno(obj->last_write.request), + i915_gem_request_get_seqno(obj->last_fence.request), i915_cache_level_str(to_i915(obj->base.dev), obj->cache_level), obj->dirty ? " dirty" : "", obj->madv == I915_MADV_DONTNEED ? " purgeable" : ""); @@ -195,9 +195,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) *t = '\0'; seq_printf(m, " (%s mappable)", s); } - if (obj->last_write_req != NULL) - seq_printf(m, " (%s)", - i915_gem_request_get_engine(obj->last_write_req)->name); + if (obj->last_write.request) + seq_printf(m, " (%s)", obj->last_write.request->engine->name); if (obj->frontbuffer_bits) seq_printf(m, " (frontbuffer: 0x%03x)", obj->frontbuffer_bits); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 27e1182544a2..5b096c65646c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2238,11 +2238,10 @@ struct drm_i915_gem_object { * requests on one ring where the write request is older than the * read request. This allows for the CPU to read from an active * buffer by only waiting for the write to complete. -* */ - struct drm_i915_gem_request *last_read_req[I915_NUM_ENGINES]; - struct drm_i915_gem_request *last_write_req; - /** Breadcrumb of last fenced GPU access to the buffer. */ - struct drm_i915_gem_request *last_fenced_req; +*/ + struct i915_gem_active last_read[I915_NUM_ENGINES]; + struct i915_gem_active last_write; + struct i915_gem_active last_fence; /** Current tiling stride for the object, if it's tiled. */ uint32_t stride; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b6497e9961ab..5f302faf86e7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1353,23 +1353,23 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj, int ret, i; if (readonly) { - if (obj->last_write_req != NULL) { - ret = i915_wait_request(obj->last_write_req); + if (obj->last_write.request) { + ret = i915_wait_request(obj->last_write.request); if (ret) return ret; - i = obj->last_write_req->engine->id; - if (obj->last_read_req[i] == obj->last_write_req) + i = obj->last_write.request->engine->id; + if (obj->last_read[i].request == obj->last_write.request) i915_gem_object_retire__read(obj, i); else i915_gem_object_retire__write(obj); } } else { for (i = 0; i < I915_NU
Re: [Intel-gfx] [PATCH 46/64] drm/i915: Count how many VMA are bound for an object
On Tue, Jul 12, 2016 at 04:12:43PM +0100, Tvrtko Ursulin wrote: > > On 12/07/16 15:38, Chris Wilson wrote: > >On Tue, Jul 12, 2016 at 03:30:25PM +0100, Tvrtko Ursulin wrote: > >>On 07/07/16 09:41, Chris Wilson wrote: > >>>@@ -3684,6 +3684,9 @@ i915_gem_object_pin_to_display_plane(struct > >>>drm_i915_gem_object *obj, > >>> old_read_domains, > >>> old_write_domain); > >>> > >>>+ /* Increment the pages_pin_count to guard against the shrinker */ > >>>+ obj->pages_pin_count++; > >> > >>Would it be clearer to look at obj->pin_display in the shrinker? > >>Although it looks like special casing out of the cleanliness of the > >>design in both case so I am not sure. > > > >Yeah. I liked the mechanism of telling the shrinker to avoid certain > >pages by only having to control the pages_pin_count. It feels easier to > >explain to others "the shrinker may reap any object that hasn't pinned > >its pages" (explicitly called i915_gem_object_pin_pages for its own use) > >rather than that + plus a list of exceptions known by the shrinker. > > Hm but wait a minute, framebuffer already has the VMA pinned. So how > will the shrinker reap it? It won't, but it would try. Worse it would include the object in its estimates for shrinkable pages. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Ro.CI.BAT: failure for drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)
== Series Details == Series: drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl) URL : https://patchwork.freedesktop.org/series/9757/ State : failure == Summary == Series 9757v1 drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl) http://patchwork.freedesktop.org/api/1.0/series/9757/revisions/1/mbox Test gem_sync: Subgroup basic-store-each: pass -> DMESG-FAIL (ro-bdw-i7-5600u) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: dmesg-warn -> SKIP (ro-bdw-i7-5557U) fi-kbl-qkkr total:237 pass:174 dwarn:27 dfail:1 fail:7 skip:28 fi-skl-i5-6260u total:237 pass:218 dwarn:0 dfail:0 fail:7 skip:12 fi-skl-i7-6700k total:237 pass:204 dwarn:0 dfail:0 fail:7 skip:26 fi-snb-i7-2600 total:237 pass:190 dwarn:0 dfail:0 fail:7 skip:40 ro-bdw-i5-5250u total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5557U total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5600u total:237 pass:198 dwarn:0 dfail:1 fail:7 skip:31 ro-bsw-n3050 total:217 pass:172 dwarn:0 dfail:0 fail:2 skip:42 ro-byt-n2820 total:237 pass:191 dwarn:0 dfail:0 fail:8 skip:38 ro-hsw-i3-4010u total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-hsw-i7-4770r total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-ilk-i7-620lm total:237 pass:166 dwarn:0 dfail:0 fail:8 skip:63 ro-ilk1-i5-650 total:232 pass:166 dwarn:0 dfail:0 fail:8 skip:58 ro-ivb-i7-3770 total:237 pass:197 dwarn:0 dfail:0 fail:7 skip:33 ro-skl3-i5-6260u total:237 pass:217 dwarn:1 dfail:0 fail:7 skip:12 ro-snb-i7-2620M total:237 pass:188 dwarn:0 dfail:0 fail:8 skip:41 Results at /archive/results/CI_IGT_test/RO_Patchwork_1474/ 9561f5c drm-intel-nightly: 2016y-07m-12d-15h-14m-43s UTC integration manifest befd5bc drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Unbreak interrupts on pre-gen6
From: Ville Syrjälä Prior to gen6 we didn't have per-ring IMR registers, which means that since commit 61ff75ac20ff ("drm/i915: Simplify enabling user-interrupts with L3-remapping") we're now masking off all interrupts when init_render_ring() gets called. That's rather rude. Let's limit the ring IMR frobbing to machines that actually have the per-ring IMR registers. Cc: Chris Wilson Cc: Tvrtko Ursulin Fixes: 61ff75ac20ff ("drm/i915: Simplify enabling user-interrupts with L3-remapping") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 61e00bf9e87f..c8e77c082b21 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1305,7 +1305,8 @@ static int init_render_ring(struct intel_engine_cs *engine) if (IS_GEN(dev_priv, 6, 7)) I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING)); - I915_WRITE_IMR(engine, ~engine->irq_keep_mask); + if (INTEL_INFO(dev_priv)->gen >= 6) + I915_WRITE_IMR(engine, ~engine->irq_keep_mask); return init_workarounds_ring(engine); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 49/64] drm/i915: Introduce i915_gem_active for request tracking
On Tue, Jul 12, 2016 at 05:05:44PM +0100, Tvrtko Ursulin wrote: > > On 07/07/16 09:41, Chris Wilson wrote: > >@@ -2383,10 +2383,10 @@ void i915_vma_move_to_active(struct i915_vma *vma, > > static void > > i915_gem_object_retire__write(struct drm_i915_gem_object *obj) > > { > >-GEM_BUG_ON(obj->last_write_req == NULL); > >-GEM_BUG_ON(!(obj->active & > >intel_engine_flag(obj->last_write_req->engine))); > >+GEM_BUG_ON(!obj->last_write.request); > >+GEM_BUG_ON(!(obj->active & > >intel_engine_flag(obj->last_write.request->engine))); > > > >-i915_gem_request_assign(&obj->last_write_req, NULL); > >+i915_gem_request_assign(&obj->last_write.request, NULL); > > Why not use i915_gem_active_set here? It will be strange to have a mix. That would be strange imo. This is only a staging patch, but setting the active from inside the retirement handler isn't clean and would look odd later. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Ro.CI.BAT: success for drm/i915: SKL iboost fixes
== Series Details == Series: drm/i915: SKL iboost fixes URL : https://patchwork.freedesktop.org/series/9759/ State : success == Summary == Series 9759v1 drm/i915: SKL iboost fixes http://patchwork.freedesktop.org/api/1.0/series/9759/revisions/1/mbox fi-kbl-qkkr total:237 pass:174 dwarn:29 dfail:0 fail:8 skip:26 fi-skl-i5-6260u total:237 pass:218 dwarn:0 dfail:0 fail:7 skip:12 fi-skl-i7-6700k total:237 pass:204 dwarn:0 dfail:0 fail:7 skip:26 fi-snb-i7-2600 total:237 pass:190 dwarn:0 dfail:0 fail:7 skip:40 ro-bdw-i5-5250u total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5600u total:237 pass:199 dwarn:0 dfail:0 fail:7 skip:31 ro-bsw-n3050 total:217 pass:172 dwarn:0 dfail:0 fail:2 skip:42 ro-byt-n2820 total:237 pass:191 dwarn:0 dfail:0 fail:8 skip:38 ro-hsw-i3-4010u total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-hsw-i7-4770r total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-ilk-i7-620lm total:237 pass:166 dwarn:0 dfail:0 fail:8 skip:63 ro-ilk1-i5-650 total:232 pass:166 dwarn:0 dfail:0 fail:8 skip:58 ro-ivb-i7-3770 total:237 pass:197 dwarn:0 dfail:0 fail:7 skip:33 ro-skl3-i5-6260u total:237 pass:217 dwarn:1 dfail:0 fail:7 skip:12 ro-snb-i7-2620M total:237 pass:188 dwarn:0 dfail:0 fail:8 skip:41 ro-bdw-i7-5557U failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_1475/ 9561f5c drm-intel-nightly: 2016y-07m-12d-15h-14m-43s UTC integration manifest 6702900 drm/i915: Extract bdw_get_buf_trans_edp() cdbeeab drm/i915: Simplify intel_ddi_get_encoder_port() e2e6f7f drm/i915: Get the iboost setting based on the port type 66c5620 drm/i915: Split DP/eDP/FDI and HDMI/DVI DDI buffer programming apart ced0723 drm/i915: Explicitly use ddi buf trans entry 9 for hdmi a1b5ad7 drm/i915: Move bxt_ddi_vswing_sequence() call into intel_ddi_pre_enable() for HDMI 647ef91 drm/i915: Program iboost settings for HDMI/DVI on SKL 3fd961b drm/i915: Name the "iboost bit" fe4c1a3 drm/i915: Fix iboost setting for DDI with 4 lanes on SKL ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 47/64] drm/i915: Be more careful when unbinding vma
On Tue, Jul 12, 2016 at 04:04:57PM +0100, Tvrtko Ursulin wrote: > >@@ -3463,11 +3483,18 @@ int i915_gem_object_set_cache_level(struct > >drm_i915_gem_object *obj, > > return -EBUSY; > > } > > > >-if (!i915_gem_valid_gtt_space(vma, cache_level)) { > >-ret = i915_vma_unbind(vma); > >-if (ret) > >-return ret; > >-} > >+if (i915_gem_valid_gtt_space(vma, cache_level)) > >+continue; > >+ > >+ret = i915_vma_unbind(vma); > >+if (ret) > >+return ret; > >+ > >+/* As unbinding may affect other elements in the > >+ * obj->vma_list (due to side-effects from retiring > >+ * an active vma), play safe and restart the iterator. > >+ */ > >+goto restart; > > } > > Does not look efficient for long lists but I don't see a solution > right now. Any chance of this O(N^2) iteration hurting us in the > real world? Defintely not pretty, but couldn't think of a clean & safe alternative, so went with simple for a change. Fortunately, it is more or less a one-time conversion, operating on a shared buffer between normally 2 clients. However, they will full-ppgtt or we would not have multple vma, and so not affected. Single client multiple vma case would be partial vma. That could be a much larger list... But on the afffected machines, such vma would already be in the correct cache regime and so not need rebinding. I don't forsee (or have seen) anybody spinning here, so I am quite happy to punt the problem until someone complains. At least I think I can write a test case to have lots of partial vma, but not necessarily able to trigger the restart. That would require careful contrl of fragmentation with the ggtt... But mixing gtt faults of a bunch of small objects and different partials of a large should be enough... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Unbreak interrupts on pre-gen6
On Tue, Jul 12, 2016 at 07:24:47PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Prior to gen6 we didn't have per-ring IMR registers, which means that > since commit 61ff75ac20ff ("drm/i915: Simplify enabling > user-interrupts with L3-remapping") we're now masking off all interrupts > when init_render_ring() gets called. That confused me, we're just writing to a non-existent register, so it shouldn't have any effect. > That's rather rude. Let's limit > the ring IMR frobbing to machines that actually have the per-ring IMR > registers. > > Cc: Chris Wilson > Cc: Tvrtko Ursulin > Fixes: 61ff75ac20ff ("drm/i915: Simplify enabling user-interrupts with > L3-remapping") > Signed-off-by: Ville Syrjälä Reviewd-by: Chris Wilson Did you see anything to cause concern? I've run this patch on gen2-9, so I wonder what I missed and how. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Ro.CI.BAT: success for drm/i915: Unbreak interrupts on pre-gen6
== Series Details == Series: drm/i915: Unbreak interrupts on pre-gen6 URL : https://patchwork.freedesktop.org/series/9762/ State : success == Summary == Series 9762v1 drm/i915: Unbreak interrupts on pre-gen6 http://patchwork.freedesktop.org/api/1.0/series/9762/revisions/1/mbox Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: dmesg-warn -> SKIP (ro-bdw-i7-5557U) fi-kbl-qkkr total:237 pass:175 dwarn:29 dfail:0 fail:7 skip:26 fi-skl-i5-6260u total:237 pass:218 dwarn:0 dfail:0 fail:7 skip:12 fi-skl-i7-6700k total:237 pass:204 dwarn:0 dfail:0 fail:7 skip:26 fi-snb-i7-2600 total:237 pass:190 dwarn:0 dfail:0 fail:7 skip:40 ro-bdw-i5-5250u total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5557U total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5600u total:237 pass:199 dwarn:0 dfail:0 fail:7 skip:31 ro-bsw-n3050 total:217 pass:171 dwarn:1 dfail:0 fail:2 skip:42 ro-byt-n2820 total:237 pass:191 dwarn:0 dfail:0 fail:8 skip:38 ro-hsw-i3-4010u total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-hsw-i7-4770r total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-ilk-i7-620lm total:237 pass:166 dwarn:0 dfail:0 fail:8 skip:63 ro-ilk1-i5-650 total:232 pass:166 dwarn:0 dfail:0 fail:8 skip:58 ro-ivb-i7-3770 total:237 pass:197 dwarn:0 dfail:0 fail:7 skip:33 ro-skl3-i5-6260u total:237 pass:217 dwarn:1 dfail:0 fail:7 skip:12 ro-snb-i7-2620M total:237 pass:188 dwarn:0 dfail:0 fail:8 skip:41 Results at /archive/results/CI_IGT_test/RO_Patchwork_1476/ 9561f5c drm-intel-nightly: 2016y-07m-12d-15h-14m-43s UTC integration manifest bdf47d5 drm/i915: Unbreak interrupts on pre-gen6 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/skl: Add support for the SAGV, fix underrun hangs
Since the watermark calculations for Skylake are still broken, we're apt to hitting underruns very easily under multi-monitor configurations. While it would be lovely if this was fixed, it's not. Another problem that's been coming from this however, is the mysterious issue of underruns causing full system hangs. An easy way to reproduce this with a skylake system: - Get a laptop with a skylake GPU, and hook up two external monitors to it - Move the cursor from the built-in LCD to one of the external displays as quickly as you can - You'll get a few pipe underruns, and eventually the entire system will just freeze. After doing a lot of investigation and reading through the bspec, I found the existence of the SAGV, which is responsible for adjusting the system agent voltage and clock frequencies depending on how much power we need. According to the bspec: "The display engine access to system memory is blocked during the adjustment time. SAGV defaults to enabled. Software must use the GT-driver pcode mailbox to disable SAGV when the display engine is not able to tolerate the blocking time." The rest of the bspec goes on to explain that software can simply leave the SAGV enabled, and disable it when we use interlaced pipes/have more then one pipe active. Sure enough, with this patchset the system hangs resulting from pipe underruns on Skylake have completely vanished on my T460s. Additionally, the bspec mentions turning off the SAGV with more then one pipe enabled as a workaround for display underruns. While this patch doesn't entirely fix that, it looks like it does improve the situation a little bit so it's likely this is going to be required to make watermarks on Skylake fully functional. Changes since v1: - Added comments about this probably being one of the requirements to fixing Skylake's watermark issues - Minor style nitpicks from Matt Roper - Disable these functions on Broxton, since it doesn't have an SAGV Cc: Matt Roper Cc: Daniel Vetter Cc: Ville Syrjälä Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_reg.h | 5 ++ drivers/gpu/drm/i915/intel_pm.c | 110 3 files changed, 117 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 03e1bfa..660d0a6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1959,6 +1959,8 @@ struct drm_i915_private { struct i915_suspend_saved_registers regfile; struct vlv_s0ix_state vlv_s0ix_state; + bool skl_sagv_enabled; + struct { /* * Raw watermark latency values: diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8bfde75..9b2eb0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7162,6 +7162,11 @@ enum { #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17 #define DISPLAY_IPS_CONTROL 0x19 #define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A +#define GEN9_PCODE_SAGV_CONTROL 0x21 +#define GEN9_SAGV_DISABLE 0x0 +#define GEN9_SAGV_LOW_FREQ 0x1 +#define GEN9_SAGV_HIGH_FREQ0x2 +#define GEN9_SAGV_DYNAMIC_FREQ 0x3 #define GEN6_PCODE_DATA_MMIO(0x138128) #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5a8ee0c..539a82a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2876,6 +2876,109 @@ skl_wm_plane_id(const struct intel_plane *plane) } static void +skl_sagv_get_hw_state(struct drm_i915_private *dev_priv) +{ + u32 temp; + int ret; + + if (IS_BROXTON(dev_priv)) + return; + + mutex_lock(&dev_priv->rps.hw_lock); + ret = sandybridge_pcode_read(dev_priv, GEN9_PCODE_SAGV_CONTROL, &temp); + mutex_unlock(&dev_priv->rps.hw_lock); + + if (!ret) { + dev_priv->skl_sagv_enabled = !!(temp & GEN9_SAGV_DYNAMIC_FREQ); + } else { + /* +* If for some reason we can't access the SAGV state, follow +* the bspec and assume it's enabled +*/ + DRM_ERROR("Failed to get SAGV state, assuming enabled\n"); + dev_priv->skl_sagv_enabled = true; + } +} + +/* + * SAGV dynamically adjusts the system agent voltage and clock frequencies + * depending on power and performance requirements. The display engine access + * to system memory is blocked during the adjustment time. Having this enabled + * in multi-pipe configurations can cause issues (such as underruns causing + * full system hangs), and the bspec also suggests that software disable it + * when more then one pipe is enabled. + */ +st
[Intel-gfx] [PATCH v3] drm/i915/skl: Add support for the SAGV, fix underrun hangs
Since the watermark calculations for Skylake are still broken, we're apt to hitting underruns very easily under multi-monitor configurations. While it would be lovely if this was fixed, it's not. Another problem that's been coming from this however, is the mysterious issue of underruns causing full system hangs. An easy way to reproduce this with a skylake system: - Get a laptop with a skylake GPU, and hook up two external monitors to it - Move the cursor from the built-in LCD to one of the external displays as quickly as you can - You'll get a few pipe underruns, and eventually the entire system will just freeze. After doing a lot of investigation and reading through the bspec, I found the existence of the SAGV, which is responsible for adjusting the system agent voltage and clock frequencies depending on how much power we need. According to the bspec: "The display engine access to system memory is blocked during the adjustment time. SAGV defaults to enabled. Software must use the GT-driver pcode mailbox to disable SAGV when the display engine is not able to tolerate the blocking time." The rest of the bspec goes on to explain that software can simply leave the SAGV enabled, and disable it when we use interlaced pipes/have more then one pipe active. Sure enough, with this patchset the system hangs resulting from pipe underruns on Skylake have completely vanished on my T460s. Additionally, the bspec mentions turning off the SAGV with more then one pipe enabled as a workaround for display underruns. While this patch doesn't entirely fix that, it looks like it does improve the situation a little bit so it's likely this is going to be required to make watermarks on Skylake fully functional. Changes since v2: - Really apply minor style nitpicks to patch this time Changes since v1: - Added comments about this probably being one of the requirements to fixing Skylake's watermark issues - Minor style nitpicks from Matt Roper - Disable these functions on Broxton, since it doesn't have an SAGV Cc: Matt Roper Cc: Daniel Vetter Cc: Ville Syrjälä Signed-off-by: Lyude --- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_reg.h | 5 ++ drivers/gpu/drm/i915/intel_pm.c | 110 3 files changed, 117 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 03e1bfa..660d0a6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1959,6 +1959,8 @@ struct drm_i915_private { struct i915_suspend_saved_registers regfile; struct vlv_s0ix_state vlv_s0ix_state; + bool skl_sagv_enabled; + struct { /* * Raw watermark latency values: diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 8bfde75..9b2eb0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7162,6 +7162,11 @@ enum { #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17 #define DISPLAY_IPS_CONTROL 0x19 #define HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A +#define GEN9_PCODE_SAGV_CONTROL 0x21 +#define GEN9_SAGV_DISABLE 0x0 +#define GEN9_SAGV_LOW_FREQ 0x1 +#define GEN9_SAGV_HIGH_FREQ0x2 +#define GEN9_SAGV_DYNAMIC_FREQ 0x3 #define GEN6_PCODE_DATA_MMIO(0x138128) #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5a8ee0c..948564a 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2876,6 +2876,109 @@ skl_wm_plane_id(const struct intel_plane *plane) } static void +skl_sagv_get_hw_state(struct drm_i915_private *dev_priv) +{ + u32 temp; + int ret; + + if (IS_BROXTON(dev_priv)) + return; + + mutex_lock(&dev_priv->rps.hw_lock); + ret = sandybridge_pcode_read(dev_priv, GEN9_PCODE_SAGV_CONTROL, &temp); + mutex_unlock(&dev_priv->rps.hw_lock); + + if (!ret) { + dev_priv->skl_sagv_enabled = !!(temp & GEN9_SAGV_DYNAMIC_FREQ); + } else { + /* +* If for some reason we can't access the SAGV state, follow +* the bspec and assume it's enabled +*/ + DRM_ERROR("Failed to get SAGV state, assuming enabled\n"); + dev_priv->skl_sagv_enabled = true; + } +} + +/* + * SAGV dynamically adjusts the system agent voltage and clock frequencies + * depending on power and performance requirements. The display engine access + * to system memory is blocked during the adjustment time. Having this enabled + * in multi-pipe configurations can cause issues (such as underruns causing + * full system hangs), and the bspec also suggests
Re: [Intel-gfx] [PATCH v3] drm/i915/skl: Add support for the SAGV, fix underrun hangs
On Tue, Jul 12, 2016 at 01:36:03PM -0400, Lyude wrote: > Since the watermark calculations for Skylake are still broken, we're apt > to hitting underruns very easily under multi-monitor configurations. > While it would be lovely if this was fixed, it's not. Another problem > that's been coming from this however, is the mysterious issue of > underruns causing full system hangs. An easy way to reproduce this with > a skylake system: > > - Get a laptop with a skylake GPU, and hook up two external monitors to > it > - Move the cursor from the built-in LCD to one of the external displays > as quickly as you can > - You'll get a few pipe underruns, and eventually the entire system will > just freeze. > > After doing a lot of investigation and reading through the bspec, I > found the existence of the SAGV, which is responsible for adjusting the > system agent voltage and clock frequencies depending on how much power > we need. According to the bspec: > > "The display engine access to system memory is blocked during the > adjustment time. SAGV defaults to enabled. Software must use the > GT-driver pcode mailbox to disable SAGV when the display engine is not > able to tolerate the blocking time." > > The rest of the bspec goes on to explain that software can simply leave > the SAGV enabled, and disable it when we use interlaced pipes/have more > then one pipe active. > > Sure enough, with this patchset the system hangs resulting from pipe > underruns on Skylake have completely vanished on my T460s. Additionally, > the bspec mentions turning off the SAGV with more then one pipe enabled > as a workaround for display underruns. While this patch doesn't entirely > fix that, it looks like it does improve the situation a little bit so > it's likely this is going to be required to make watermarks on Skylake > fully functional. > > Changes since v2: > - Really apply minor style nitpicks to patch this time > Changes since v1: > - Added comments about this probably being one of the requirements to >fixing Skylake's watermark issues > - Minor style nitpicks from Matt Roper > - Disable these functions on Broxton, since it doesn't have an SAGV > > Cc: Matt Roper > Cc: Daniel Vetter > Cc: Ville Syrjälä > Signed-off-by: Lyude I don't have a SKL to try this out on (only BXT here), but this matches my interpretation of the current bspec text, so Reviewed-by: Matt Roper I think this also applies to Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94625 And maybe a +cc stable? Matt > --- > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/i915_reg.h | 5 ++ > drivers/gpu/drm/i915/intel_pm.c | 110 > > 3 files changed, 117 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 03e1bfa..660d0a6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1959,6 +1959,8 @@ struct drm_i915_private { > struct i915_suspend_saved_registers regfile; > struct vlv_s0ix_state vlv_s0ix_state; > > + bool skl_sagv_enabled; > + > struct { > /* >* Raw watermark latency values: > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8bfde75..9b2eb0b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7162,6 +7162,11 @@ enum { > #define HSW_PCODE_DE_WRITE_FREQ_REQ0x17 > #define DISPLAY_IPS_CONTROL0x19 > #defineHSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL 0x1A > +#define GEN9_PCODE_SAGV_CONTROL0x21 > +#define GEN9_SAGV_DISABLE0x0 > +#define GEN9_SAGV_LOW_FREQ 0x1 > +#define GEN9_SAGV_HIGH_FREQ 0x2 > +#define GEN9_SAGV_DYNAMIC_FREQ 0x3 > #define GEN6_PCODE_DATA _MMIO(0x138128) > #define GEN6_PCODE_FREQ_IA_RATIO_SHIFT 8 > #define GEN6_PCODE_FREQ_RING_RATIO_SHIFT 16 > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5a8ee0c..948564a 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2876,6 +2876,109 @@ skl_wm_plane_id(const struct intel_plane *plane) > } > > static void > +skl_sagv_get_hw_state(struct drm_i915_private *dev_priv) > +{ > + u32 temp; > + int ret; > + > + if (IS_BROXTON(dev_priv)) > + return; > + > + mutex_lock(&dev_priv->rps.hw_lock); > + ret = sandybridge_pcode_read(dev_priv, GEN9_PCODE_SAGV_CONTROL, &temp); > + mutex_unlock(&dev_priv->rps.hw_lock); > + > + if (!ret) { > + dev_priv->skl_sagv_enabled = !!(temp & GEN9_SAGV_DYNAMIC_FREQ); > + } else { > + /* > + * If for some reason we can't access the SAGV state, follow > + * the bspec and assume it's enabled
[Intel-gfx] [PATCH v2 1/3] drm: extra printk() wrapper macros
We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() provides several other useful intermediate levels such as NOTICE and WARNING. So this patch fills out the set by providing both regular and once-only macros for each of the levels INFO, NOTICE, and WARNING, using a common underlying macro that does all the token-pasting. DRM_ERROR is unchanged, as it's not just a printk wrapper. v2: Fix whitespace, missing ## (Eric Engestrom) Signed-off-by: Dave Gordon Reviewed-by: Eric Engestrom --- include/drm/drmP.h | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/include/drm/drmP.h b/include/drm/drmP.h index c2fe2cf..1f53cc2 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); /** \name Macros to make printk easier */ /*@{*/ +#define _DRM_PRINTK(once, level, fmt, ...) \ + do {\ + printk##once(KERN_##level "[" DRM_NAME "] " fmt,\ +##__VA_ARGS__);\ + } while (0) + +#define DRM_INFO(fmt, ...) \ + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE(fmt, ...) \ + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN(fmt, ...) \ + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) + +#define DRM_INFO_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) + /** * Error output. * @@ -187,12 +207,6 @@ void drm_err(const char *format, ...); drm_err(fmt, ##__VA_ARGS__);\ }) -#define DRM_INFO(fmt, ...) \ - printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - -#define DRM_INFO_ONCE(fmt, ...)\ - printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - /** * Debug output. * -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/3] drm/i915/guc: downgrade some DRM_ERROR() messages to DRM_WARN()
Where we're going to continue regardless of the problem, rather than fail, then the message should be a WARNing rather than an ERROR. Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/i915_guc_submission.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 2112e02..e299b64 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -114,10 +114,8 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) if (ret != -ETIMEDOUT) ret = -EIO; - DRM_ERROR("GUC: host2guc action 0x%X failed. ret=%d " - "status=0x%08X response=0x%08X\n", - data[0], ret, status, - I915_READ(SOFT_SCRATCH(15))); + DRM_WARN("Action 0x%X failed; ret=%d status=0x%08X response=0x%08X\n", +data[0], ret, status, I915_READ(SOFT_SCRATCH(15))); dev_priv->guc.action_fail += 1; dev_priv->guc.action_err = ret; @@ -553,8 +551,8 @@ static int guc_ring_doorbell(struct i915_guc_client *gc) if (db_ret.db_status == GUC_DOORBELL_DISABLED) break; - DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n", - db_cmp.cookie, db_ret.cookie); + DRM_WARN("Cookie mismatch. Expected %d, found %d\n", +db_cmp.cookie, db_ret.cookie); /* update the cookie to newly read cookie from GuC */ db_cmp.cookie = db_ret.cookie; @@ -726,8 +724,8 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) /* Restore to original value */ err = guc_update_doorbell_id(guc, client, db_id); if (err) - DRM_ERROR("Failed to restore doorbell to %d, err %d\n", - db_id, err); + DRM_WARN("Failed to restore doorbell to %d, err %d\n", +db_id, err); for (i = 0; i < GUC_MAX_DOORBELLS; ++i) { i915_reg_t drbreg = GEN8_DRBREGL(i); @@ -819,8 +817,6 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) return client; err: - DRM_ERROR("FAILED to create priority %u GuC client!\n", priority); - guc_client_free(dev_priv, client); return NULL; } @@ -998,7 +994,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv) GUC_CTX_PRIORITY_KMD_NORMAL, dev_priv->kernel_context); if (!client) { - DRM_ERROR("Failed to create execbuf guc_client\n"); + DRM_ERROR("Failed to create normal GuC client!\n"); return -ENOMEM; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] drm/i915/guc: revisit GuC loader message levels
Some downgraded from DRM_ERROR() to DRM_WARN() or DRM_NOTE(), a few upgraded from DRM_INFO() to DRM_NOTE() or DRM_WARN(), and one eliminated completely. A typical failure mode might now look like this in the dmesg log: [drm] Failed to fetch valid GuC firmware from i915/skl_guc_ver6_1.bin (error -2) [drm] GuC firmware load failed: -5 [drm] Falling back from GuC submission to execlist mode which provides sufficient notice that * there is a problem with the firmware binary * and consequently loading the GuC has failed * and so we have selected the fallback (execlist) mode while not cluttering the log with developer-only details. v2: different permutation of levels :) Signed-off-by: Dave Gordon --- drivers/gpu/drm/i915/intel_guc_loader.c | 34 - 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 605c696..a2f4fa4 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -140,12 +140,14 @@ static u32 get_gttype(struct drm_i915_private *dev_priv) static u32 get_core_family(struct drm_i915_private *dev_priv) { - switch (INTEL_INFO(dev_priv)->gen) { + u32 gen = INTEL_GEN(dev_priv); + + switch (gen) { case 9: return GFXCORE_FAMILY_GEN9; default: - DRM_ERROR("GUC: unsupported core family\n"); + DRM_WARN("GEN%d does not support GuC operation\n", gen); return GFXCORE_FAMILY_UNKNOWN; } } @@ -433,7 +435,7 @@ int intel_guc_setup(struct drm_device *dev) goto fail; } else if (*fw_path == '\0') { /* Device has a GuC but we don't know what f/w to load? */ - DRM_INFO("No GuC firmware known for this platform\n"); + DRM_WARN("No GuC firmware known for this platform\n"); err = -ENODEV; goto fail; } @@ -471,10 +473,8 @@ int intel_guc_setup(struct drm_device *dev) * that the state and timing are fairly predictable */ err = i915_reset_guc(dev_priv); - if (err) { - DRM_ERROR("GuC reset failed: %d\n", err); + if (err) goto fail; - } err = guc_ucode_xfer(dev_priv); if (!err) @@ -532,15 +532,15 @@ int intel_guc_setup(struct drm_device *dev) else if (err == 0) DRM_INFO("GuC firmware load skipped\n"); else if (ret != -EIO) - DRM_INFO("GuC firmware load failed: %d\n", err); + DRM_NOTE("GuC firmware load failed: %d\n", err); else - DRM_ERROR("GuC firmware load failed: %d\n", err); + DRM_WARN("GuC firmware load failed: %d\n", err); if (i915.enable_guc_submission) { if (fw_path == NULL) DRM_INFO("GuC submission without firmware not supported\n"); if (ret == 0) - DRM_INFO("Falling back from GuC submission to execlist mode\n"); + DRM_NOTE("Falling back from GuC submission to execlist mode\n"); else DRM_ERROR("GuC init failed: %d\n", ret); } @@ -571,7 +571,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw) /* Check the size of the blob before examining buffer contents */ if (fw->size < sizeof(struct guc_css_header)) { - DRM_ERROR("Firmware header is missing\n"); + DRM_NOTE("Firmware header is missing\n"); goto fail; } @@ -583,7 +583,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw) css->key_size_dw - css->exponent_size_dw) * sizeof(u32); if (guc_fw->header_size != sizeof(struct guc_css_header)) { - DRM_ERROR("CSS header definition mismatch\n"); + DRM_NOTE("CSS header definition mismatch\n"); goto fail; } @@ -593,7 +593,7 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw) /* now RSA */ if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) { - DRM_ERROR("RSA key size is bad\n"); + DRM_NOTE("RSA key size is bad\n"); goto fail; } guc_fw->rsa_offset = guc_fw->ucode_offset + guc_fw->ucode_size; @@ -602,14 +602,14 @@ static void guc_fw_fetch(struct drm_device *dev, struct intel_guc_fw *guc_fw) /* At least, it should have header, uCode and RSA. Size of all three. */ size = guc_fw->header_size + guc_fw->ucode_size + guc_fw->rsa_size; if (fw->size < size) { - DRM_ERROR("Missing firmware components\n"); + DRM_NOTE("Missing firmware components\n"); goto fai
Re: [Intel-gfx] [PATCH] drm/i915: Unbreak interrupts on pre-gen6
On Tue, Jul 12, 2016 at 05:47:02PM +0100, Chris Wilson wrote: > On Tue, Jul 12, 2016 at 07:24:47PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Prior to gen6 we didn't have per-ring IMR registers, which means that > > since commit 61ff75ac20ff ("drm/i915: Simplify enabling > > user-interrupts with L3-remapping") we're now masking off all interrupts > > when init_render_ring() gets called. > > That confused me, we're just writing to a non-existent register, so it > shouldn't have any effect. RING_IMR(RCS) == 0x20a8 == IMR > > > That's rather rude. Let's limit > > the ring IMR frobbing to machines that actually have the per-ring IMR > > registers. > > > > Cc: Chris Wilson > > Cc: Tvrtko Ursulin > > Fixes: 61ff75ac20ff ("drm/i915: Simplify enabling user-interrupts with > > L3-remapping") > > Signed-off-by: Ville Syrjälä > Reviewd-by: Chris Wilson > > Did you see anything to cause concern? I've run this patch on gen2-9, so > I wonder what I missed and how. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Kernel stability on baytrail machines
Hi Alan, (Adding interested people to this thread) On 09 Apr 08:14 PM, One Thousand Gnomes wrote: > > > I do feel that the importance of the mentioned bug is currently > > > underestimated. Can anyone here give a note, how much current linux > > > kernel is supposed to be stable on general baytrail machines? > > > > If you did not get any replies... you might want to check MAINTAINERS file, > > and > > put Intel x86 maintainers on Cc list. > > > > I'm sure someone cares :-). > > Yes we care, and there are people looking at the various reports. > Are there any updates on the status of this issue? The current bugzilla report [1] marks this as a power management issue. However, many reports indicate that it would only freeze when running X, so it's not completely clear if it's related to the gfx driver too. Also, do we know which CPUs are affect by this issue? and which are NOT affected :) - would be quite relevant in picking a CPU for a product. [1] https://bugzilla.kernel.org/show_bug.cgi?id=109051 -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Unbreak interrupts on pre-gen6
On Tue, Jul 12, 2016 at 10:13:48PM +0300, Ville Syrjälä wrote: > On Tue, Jul 12, 2016 at 05:47:02PM +0100, Chris Wilson wrote: > > On Tue, Jul 12, 2016 at 07:24:47PM +0300, ville.syrj...@linux.intel.com > > wrote: > > > From: Ville Syrjälä > > > > > > Prior to gen6 we didn't have per-ring IMR registers, which means that > > > since commit 61ff75ac20ff ("drm/i915: Simplify enabling > > > user-interrupts with L3-remapping") we're now masking off all interrupts > > > when init_render_ring() gets called. > > > > That confused me, we're just writing to a non-existent register, so it > > shouldn't have any effect. > > RING_IMR(RCS) == 0x20a8 == IMR Ah (I expected the global IIR et al not to be in the ring block). And since we unmask everything, nothing is broken at first glance. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Kernel stability on baytrail machines
On Tue 2016-07-12 16:41:58, Ezequiel Garcia wrote: > Hi Alan, > > (Adding interested people to this thread) > > On 09 Apr 08:14 PM, One Thousand Gnomes wrote: > > > > I do feel that the importance of the mentioned bug is currently > > > > underestimated. Can anyone here give a note, how much current linux > > > > kernel is supposed to be stable on general baytrail machines? > > > > > > If you did not get any replies... you might want to check MAINTAINERS > > > file, and > > > put Intel x86 maintainers on Cc list. > > > > > > I'm sure someone cares :-). > > > > Yes we care, and there are people looking at the various reports. > > > > Are there any updates on the status of this issue? > > The current bugzilla report [1] marks this as a power management > issue. However, many reports indicate that it would only freeze > when running X, so it's not completely clear if it's related to > the gfx driver too. Does "intel_idle.max_cstate=1" fix it for you? If you feel it is X-only problem, you may want to provide details about your graphics subsystem (DRM enabled? framebuffer only?) and probably cc. ...actually... you may want to verify if it happens in unaccelerated X. INTEL DRM DRIVERS (excluding Poulsbo, Moorestown and derivative chipsets) M: Daniel Vetter M: Jani Nikula L: intel-gfx@lists.freedesktop.org L: dri-de...@lists.freedesktop.org W: https://01.org/linuxgraphics/ Q: http://patchwork.freedesktop.org/project/intel-gfx/ T: git git://anongit.freedesktop.org/drm-intel S: Supported F: drivers/gpu/drm/i915/ F: include/drm/i915* F: include/uapi/drm/i915_drm.h Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions
On 07/01/2016 09:40 PM, Deak, Imre wrote: Use named struct initializers for clarity. Also fix the target cache definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0 meant ELLC but on GEN9+ it means the TC and LRU controls are taken from the PTE. No functional change, igt/gem_mocs_settings still passing after this change. v2: (Chris) - Add back the hexa literals for the entries. Add note that igt/gem_mocs_settings still passes. CC: Rong R Yang CC: Yakui Zhao CC: Chris Wilson Signed-off-by: Imre Deak It is helpful to understand the MOCS table definition after cleaning up. Add: Acked-by: Zhao Yakui Thanks Yakui --- drivers/gpu/drm/i915/intel_mocs.c | 88 +++ 1 file changed, 61 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index 3c1482b..d36e609 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -66,9 +66,10 @@ struct drm_i915_mocs_table { #define L3_WB 3 /* Target cache */ -#define ELLC 0 -#define LLC1 -#define LLC_ELLC 2 +#define LE_TC_PAGETABLE0 +#define LE_TC_LLC 1 +#define LE_TC_LLC_ELLC 2 +#define LE_TC_LLC_ELLC_ALT 3 /* * MOCS tables @@ -96,34 +97,67 @@ struct drm_i915_mocs_table { * end. */ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { - /* { 0x0009, 0x0010 } */ - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, - /* { 0x0038, 0x0030 } */ - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, - /* { 0x003b, 0x0030 } */ - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } + { /* 0x0009 */ + .control_value = LE_CACHEABILITY(LE_UC) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + + /* 0x0010 */ + .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), + }, + { + /* 0x0038 */ + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + /* 0x0030 */ + .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), + }, + { + /* 0x003b */ + .control_value = LE_CACHEABILITY(LE_WB) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + /* 0x0030 */ + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), + }, }; /* NOTE: the LE_TGT_CACHE is not used on Broxton */ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { - /* { 0x0009, 0x0010 } */ - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, - /* { 0x0038, 0x0030 } */ - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, - /* { 0x003b, 0x0030 } */ - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } + { + /* 0x0009 */ + .control_value = LE_CACHEABILITY(LE_UC) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + + /* 0x0010 */ + .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), + }, + { + /* 0x0038 */ + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + + /* 0x0030 */ + .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries
On 07/01/2016 09:40 PM, Deak, Imre wrote: The purpose for each MOCS entry isn't well defined atm. Defining these is important to remove any uncertainty about the use of these entries for example in terms of performance and GPU/CPU coherency. Suggested by Ville. CC: Rong R Yang CC: Yakui Zhao CC: Ville Syrjälä CC: Chris Wilson Signed-off-by: Imre Deak This looks readable and meaningful after giving proper names to MOCS entry index. But not sure whether the comment of I915_MOCS_CACHE has one typo? --- drivers/gpu/drm/i915/intel_mocs.c | 13 +++-- include/uapi/drm/i915_drm.h | 24 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index 927825f..86adc11 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { * end. */ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { - { /* 0x0009 */ + [I915_MOCS_UNCACHED] = { + /* 0x0009 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* 0x0010 */ .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), }, - { + [I915_MOCS_AUTO] = { /* 0x0038 */ .control_value = LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* 0x0030 */ .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, - { + [I915_MOCS_CACHED] = { /* 0x003b */ .control_value = LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* NOTE: the LE_TGT_CACHE is not used on Broxton */ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { - { + [I915_MOCS_UNCACHED] = { /* 0x0009 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { /* 0x0010 */ .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), }, - { + [I915_MOCS_AUTO] = { /* 0x0038 */ .control_value = LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { /* 0x0030 */ .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, - { + [I915_MOCS_CACHED] = { /* 0x0039 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index c17d63d..a5d116f 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -62,6 +62,30 @@ extern "C" { #define I915_ERROR_UEVENT "ERROR" #define I915_RESET_UEVENT "RESET" +/* + * MOCS indexes used for GPU surfaces, defining the cacheability of the + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. + */ +enum i915_mocs_table_index { + /* +* Not cached anywhere, coherency between CPU and GPU accesses is +* guaranteed. +*/ + I915_MOCS_UNCACHED, + /* +* Cacheability and coherency controlled by the kernel automatically +* based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current +* usage of the surface (used for display scanout or not). +*/ + I915_MOCS_AUTO, + /* +* Cached in all GPU caches available on the platform. +* Coherency between CPU and GPU accesses to the surface is not +* guaranteed without extra synchronization. +*/ IMO the coherency is guaranteed without extra synchronization for the MOCS_CACHED. + I915_MOCS_CACHED, +}; + /* Each region is a minimum of 16k, and there are at most 255 of them. */ #define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config
On 07/01/2016 09:40 PM, Deak, Imre wrote: Setting a write-back cache policy in the MOCS entry definition also implies snooping, which has a considerable overhead. This is unexpected for a few reasons: - From user-space's point of view since it didn't want a coherent surface (it didn't set the buffer as such via the set caching IOCTL). - There is a separate MOCS entry field for snooping (which we never set). - This MOCS table is about caching in (e)LLC and there is no (e)LLC on BXT. There is a separate table for L3 cache control. Considering the above the current behavior of snooping looks like an unintentional side-effect of the WB setting. Changing it to be LLC-UC gets rid of the snooping without any ill-effects. For a coherent surface the application would use a separate MOCS entry at index 1 and call the set caching IOCTL to setup the PTE entries for the corresponding buffer to be snooped. In the future we could also add a new MOCS entry for coherent surfaces. This resulted in 70% improvement in synthetic texturing benchmarks. Kudos to Valtteri Rantala, Eero Tamminen and Michael T Frederick and Ville who helped to narrow the source of problem to the kernel and to the snooping behaviour in particular. With a follow-up change to adjust the 3rd entry value igt/gem_mocs_settings is passing after this change. v2: - Rebase on v2 of patch 1/2. v3: - Set the entry as LLC uncached instead of PTE-passthrough. This way we also keep snooping disabled, but we also make the cacheability/ coherency setting indepent of the PTE which is managed by the kernel. (Chris) CC: Rong R Yang CC: Yakui Zhao CC: Valtteri Rantala CC: Eero Tamminen CC: Michael T Frederick CC: Ville Syrjälä CC: Chris Wilson Signed-off-by: Imre Deak As the BXT has no LLC, setting the WB-policy will add the extra overhead. In such case the patch looks more reasonable for BXT. Add: Acked-by: Zhao Yakui --- drivers/gpu/drm/i915/intel_mocs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index d36e609..927825f 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -149,8 +149,8 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { .l3cc_value =L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, { - /* 0x003b */ - .control_value = LE_CACHEABILITY(LE_WB) | + /* 0x0039 */ + .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0), ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-misc tree with the arm tree
Hi all, Today's linux-next merge of the drm-misc tree got a conflict in: drivers/gpu/drm/rockchip/rockchip_drm_drv.c between commit: 062993b15e8e ("drm: convert DT component matching to component_match_add_release()") from the arm tree and commit: 6d5fa28c13b9 ("gpu: drm: rockchip_drm_drv: add missing of_node_put after calling of_parse_phandle") from the drm-misc tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/gpu/drm/rockchip/rockchip_drm_drv.c index 7fd20c0e1fc8,f0bd1ee8b128.. --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@@ -438,9 -433,8 +438,10 @@@ static int rockchip_drm_platform_probe( is_support_iommu = false; } + of_node_put(iommu); - component_match_add(dev, &match, compare_of, port->parent); + of_node_get(port->parent); + component_match_add_release(dev, &match, release_of, + compare_of, port->parent); of_node_put(port); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Ro.CI.BAT: failure for drm/i915/skl: Add support for the SAGV, fix underrun hangs (rev3)
== Series Details == Series: drm/i915/skl: Add support for the SAGV, fix underrun hangs (rev3) URL : https://patchwork.freedesktop.org/series/9736/ State : failure == Summary == Series 9736v3 drm/i915/skl: Add support for the SAGV, fix underrun hangs http://patchwork.freedesktop.org/api/1.0/series/9736/revisions/3/mbox Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (ro-hsw-i7-4770r) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: dmesg-warn -> SKIP (ro-bdw-i7-5557U) ro-bdw-i5-5250u total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5557U total:237 pass:213 dwarn:1 dfail:0 fail:7 skip:16 ro-bdw-i7-5600u total:237 pass:199 dwarn:0 dfail:0 fail:7 skip:31 ro-bsw-n3050 total:218 pass:172 dwarn:0 dfail:0 fail:3 skip:42 ro-byt-n2820 total:237 pass:191 dwarn:0 dfail:0 fail:8 skip:38 ro-hsw-i3-4010u total:237 pass:206 dwarn:0 dfail:0 fail:7 skip:24 ro-hsw-i7-4770r total:237 pass:205 dwarn:0 dfail:0 fail:8 skip:24 ro-ilk-i7-620lm total:237 pass:166 dwarn:0 dfail:0 fail:8 skip:63 ro-ilk1-i5-650 total:232 pass:166 dwarn:0 dfail:0 fail:8 skip:58 ro-ivb-i7-3770 total:237 pass:197 dwarn:0 dfail:0 fail:7 skip:33 ro-skl3-i5-6260u total:237 pass:217 dwarn:1 dfail:0 fail:7 skip:12 ro-snb-i7-2620M total:237 pass:188 dwarn:0 dfail:0 fail:8 skip:41 Results at /archive/results/CI_IGT_test/RO_Patchwork_1478/ 9561f5c drm-intel-nightly: 2016y-07m-12d-15h-14m-43s UTC integration manifest 964a49a drm/i915/skl: Add support for the SAGV, fix underrun hangs ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set the access right of kernel param "i915.enable_gvt" to read-only.
Thanks! :P. Welcome back. On 07/12/16 18:50, Joonas Lahtinen wrote: On ma, 2016-06-20 at 08:17 -0400, Zhi Wang wrote: The access right of kernel param "i915.enable_gvt" should be read-only as it only applies during module load (and is not *runtime* writable). Cc: Chris Wilson Reviewed-by: Joonas Lahtinen Maybe also add, Suggested-by: Chris Wilson Signed-off-by: Zhi Wang --- drivers/gpu/drm/i915/i915_params.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 7effe68..8b13bfa 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -224,6 +224,6 @@ module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600 MODULE_PARM_DESC(enable_dpcd_backlight, "Enable support for DPCD backlight control (default:false)"); -module_param_named(enable_gvt, i915.enable_gvt, bool, 0600); +module_param_named(enable_gvt, i915.enable_gvt, bool, 0400); MODULE_PARM_DESC(enable_gvt, "Enable support for Intel GVT-g graphics virtualization host support(default:false)"); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx