[Intel-gfx] [PATCH 0/3] Deprecate legacy crap
Hi all, Spurred by Ian's abuse of drmAgpSize I've figued it's time we accelerate the demisse of the fake agp stuff and our ums code a bit. The idea is to send out the same probe the radone guys have done and deprecate legacy stuff for now. Then (presuming we don't get any reports from hurt users) we can rip the code out after a few releases. In both cases (killing fake agp and killing ums) we can just keep doing what we've done thus far as contingency. But for ums I expect that we need to copy-paste the driver long term to stay sane. Just removing it is obviously easier ;-) Cheers, Daniel Daniel Vetter (3): drm/i915: Make AGP=n work even on gen3 drm/i915: Kill legeacy AGP for gen3 kms drm/i915: Deprecated UMS support drivers/gpu/drm/i915/Kconfig| 24 +++- drivers/gpu/drm/i915/i915_drv.c | 14 ++ 2 files changed, 21 insertions(+), 17 deletions(-) -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/13] drm/i915: backlight rewrite
On Fri, 08 Nov 2013, Jani Nikula jani.nik...@intel.com wrote: I've tested this so far on ILK and IVB, trying carefully keep it working commit by commit to keep things bisectable. More testing across platforms is very much needed. We have a history with backlight... I've now tested this on PNV and SNB too, seems to work fine. Cheers, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Make AGP=n work even on gen3
Most platforms din't hit this condition, but if we want to allow building without agp we should also make this allowed on gen3. Cc: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 38a344694e35..d7c922051c89 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -158,7 +158,7 @@ static struct drm_driver driver; #if IS_ENABLED(CONFIG_AGP_INTEL) extern int intel_agp_enabled; #else -static int intel_agp_enabled; +static int intel_agp_enabled = 1; #endif static const struct intel_device_info intel_i830_info = { -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Deprecated UMS support
It's been 5 years since kms support was merged and roughly 4 years since UMS support was ripped out from userspace drivers. Thus far it's not been a big burden to keep the ums paths alive, and we've made some good progress in better separating it from the kms code by sprinkling DRIVER_MODESET checks all over the place. But now that the drm demidlayering is within reach this changes. I want to make the driver loading code more robust using devres.c and other cool tricks. But that doesn't work with ums due to the shadow-attach trick. Which means we either a) need to split out a complete ums codebase like radeon has b) kill it for good. The 2nd option is obviously much less work than the first, so I think it's time to test the waters and see how many people out there still use ums. I've decided that silently failing to initialize the driver (and not e.g. failing to load the module) is the right thing. That way we should only get reports from users that actually care about some ums features (like accelerated gl or support for secondary outputs). Everyone else will just fall back to the vesa X driver. For developers there's a small info level dmesg output. The plan is to drop this Kconfig option after 3.16 (so gives us 2 full releases) and then start killing code for real 2-3 releases afterwards. That should be more than enough time for users to pipe up. Of course if anyone does we need to revisit this plan and maybe go with option a) above. Also enable the KMS support by default in Kconfig and polish the help texts a bit. Cc: Dave Airlie airl...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/Kconfig | 24 +++- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index b0f61679c598..b0fa4c4055ee 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -37,12 +37,11 @@ config DRM_I915 config DRM_I915_KMS bool Enable modesetting on intel by default depends on DRM_I915 + default y help - Choose this option if you want kernel modesetting enabled by default, - and you have a new enough userspace to support this. Running old - userspaces with this enabled will cause pain. Note that this causes - the driver to bind to PCI devices, which precludes loading things - like intelfb. + Choose this option if you want kernel modesetting enabled by default. + + If in doubt, say Y. config DRM_I915_FBDEV bool Enable legacy fbdev support for the modesettting intel driver @@ -57,9 +56,12 @@ config DRM_I915_FBDEV support. Note that this support also provide the linux console support on top of the intel modesetting driver. + If in doubt, say Y. + config DRM_I915_PRELIMINARY_HW_SUPPORT bool Enable preliminary support for prerelease Intel hardware by default depends on DRM_I915 + default n help Choose this option if you have prerelease Intel hardware and want the i915 driver to support it by default. You can enable such support at @@ -67,3 +69,15 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT option changes the default for that module option. If in doubt, say N. + +config DRM_I915_UMS + bool Enable userspace modesetting on Intel hardware (DEPRECATED) + depends on DRM_I915 + default n + help + Choose this option if you still need userspace modesetting. + + Userspace modesetting is deprecated for quite some time now, so + enable this only if you have ancient versions of the DDX drivers. + + If in doubt, say N. -- 1.8.4.rc3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/7] drm/i915: Baytrail MIPI DSI support Updated
On Sat, Nov 09, 2013 at 11:28:16AM +0100, Daniel Vetter wrote: On Sat, Nov 09, 2013 at 03:19:01PM +0530, Shobhit Kumar wrote: Hi All - These patches enhance the current support for MIPI DSI for Baytrail. They continue on the sub-encoder design and adds few more dev_ops to handle sequence correctly. Major changes are - 1. DSI Clock calculation based on pixel clock 2. Add new dev_ops for better sequencing the enable/disable path 3. Parameterized the hardcoded DSI parameters. These also forms building block for the generic MIPI driver to come in future based on enhancements in VBT. All these parameters are initialized or computed in the sub-encoder driver. Some of them might look unneccesary for now. I am also aware of the drm_bridge support now comming in and will in future migrate from sub-encoder design to drm_bridge. Just a quick aside: Thierry Reding from nvidia is also working on a DSI design for the tegra driver. Atm he seems to aim for a full-blown DSI bus based on his drm_panel patches for getting the panel metadata out of an ARM DT (we'd use VBT instead). Iirc there's no patches anywhere yet, but maybe Thierry could share a git branch somewhere with the wip stuff? Cc'ing Thierry and dri-devel in case a bigger discussion develops. I've been cleaning up the patches and was going to post them today. The implementation really isn't as full-blown as you make it sound =), primarily because the DSI panel that I have doesn't support things such as reading out the DDB, so I cannot test most of the functionality that I planned to. However I think introducing a DSI bus type is the right thing and it's been suggested recently that we have too few bus types. Furthermore it seems to be playing out rather nicely with the DRM panel work, so it would be really nice if Intel could test-drive this within their driver to see if it's good enough for their purposes as well. Is everyone working on that subscribed to dri-devel or should I Cc the intel-gfx mailing list (or someone in particular) when posting the patches? Thierry pgpq3X6u7CzHp.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/bdw: GEN8 backlight support
From: Ben Widawsky benjamin.widaw...@intel.com Prior to Haswell the CPU control register for backlight (BLC_PWM_CPU_CTL) toggled the PCH baclight pin for us. This made some sense as there was no pin on the CPU. With Haswell came the introduction of a CPU backlight pin, but the interface was still controlled by software with the same mechnism. Behind the scenes, hardware did all the dirty work for us. Broadwell no longer provides this for free. If we want to use the PCH backlight pin [1] then we have to set the override bit BLC_PWM_PCH_CTL1 and program BLC_PWM_PCH_CTL2 for the PWM values. This patch implements that. This patch is compile tested only, and given that I rarely if ever touch this code, careful review is welcome. [1] According to Art, we know of no devices that exist which use the CPU pin (and remember it has existed already on HSW). If such a device does exist, we'll have to handle it properly - this is left as TODO until then. v2: Drop the abstraction prep patch, as a bigger backlight overhaul is in the works, and do just the mimimal bdw enabling now. (by Jani) CC: Art Runyan arthur.j.run...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Jani Nikula jani.nik...@intel.com --- This supersedes: http://mid.gmane.org/1383889251-498-14-git-send-email-benjamin.widaw...@intel.com http://mid.gmane.org/1383889251-498-15-git-send-email-benjamin.widaw...@intel.com --- drivers/gpu/drm/i915/intel_panel.c | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index f161ac0..e6f782d 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -451,7 +451,9 @@ static u32 intel_panel_get_backlight(struct drm_device *dev, spin_lock_irqsave(dev_priv-backlight.lock, flags); - if (HAS_PCH_SPLIT(dev)) { + if (IS_BROADWELL(dev)) { + val = I915_READ(BLC_PWM_PCH_CTL2) BACKLIGHT_DUTY_CYCLE_MASK; + } else if (HAS_PCH_SPLIT(dev)) { val = I915_READ(BLC_PWM_CPU_CTL) BACKLIGHT_DUTY_CYCLE_MASK; } else { if (IS_VALLEYVIEW(dev)) @@ -479,6 +481,13 @@ static u32 intel_panel_get_backlight(struct drm_device *dev, return val; } +static void intel_bdw_panel_set_backlight(struct drm_device *dev, u32 level) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + u32 val = I915_READ(BLC_PWM_PCH_CTL2) ~BACKLIGHT_DUTY_CYCLE_MASK; + I915_WRITE(BLC_PWM_PCH_CTL2, val | level); +} + static void intel_pch_panel_set_backlight(struct drm_device *dev, u32 level) { struct drm_i915_private *dev_priv = dev-dev_private; @@ -496,7 +505,9 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, DRM_DEBUG_DRIVER(set backlight PWM = %d\n, level); level = intel_panel_compute_brightness(dev, pipe, level); - if (HAS_PCH_SPLIT(dev)) + if (IS_BROADWELL(dev)) + return intel_bdw_panel_set_backlight(dev, level); + else if (HAS_PCH_SPLIT(dev)) return intel_pch_panel_set_backlight(dev, level); if (is_backlight_combination_mode(dev)) { @@ -666,7 +677,16 @@ void intel_panel_enable_backlight(struct intel_connector *connector) POSTING_READ(reg); I915_WRITE(reg, tmp | BLM_PWM_ENABLE); - if (HAS_PCH_SPLIT(dev) + if (IS_BROADWELL(dev)) { + /* +* Broadwell requires PCH override to drive the PCH +* backlight pin. The above will configure the CPU +* backlight pin, which we don't plan to use. +*/ + tmp = I915_READ(BLC_PWM_PCH_CTL1); + tmp |= BLM_PCH_OVERRIDE_ENABLE | BLM_PCH_PWM_ENABLE; + I915_WRITE(BLC_PWM_PCH_CTL1, tmp); + } else if (HAS_PCH_SPLIT(dev) !(dev_priv-quirks QUIRK_NO_PCH_PWM_ENABLE)) { tmp = I915_READ(BLC_PWM_PCH_CTL1); tmp |= BLM_PCH_PWM_ENABLE; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/vlv: Make the vlv_dpio_read/vlv_dpio_write more PHY centric
On Wed, Nov 06, 2013 at 12:51:05PM +0200, Ville Syrjälä wrote: On Wed, Nov 06, 2013 at 02:36:35PM +0800, Chon Ming Lee wrote: vlv_dpio_read/write should be describe more in PHY centric instead of display controller centric. Create a enum dpio_channel for channel index and enum dpio_phy for PHY index. This should better to gather for upcoming platform. v2: Rebase the code based on drm/i915/vlv: Fix typo in the DPIO register define. v3: Rename vlv_phy to dpio_phy_iosf_port and define additional macro DPIO_PHY, and remove unrelated change. (Ville) Suggested-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Chon Ming Lee chon.ming@intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/21] drm/i915: Abstract backlight registers a bit
On Fri, 08 Nov 2013, Daniel Vetter dan...@ffwll.ch wrote: So I'd just go with adding a copy-pasted version of set/get/enable for broadwell. That should also fit in with Jani's plans for 3.14 with the complete backlight refactoring. Adding Jani so he can explain what he'd like. I'd like http://mid.gmane.org/1384161177-29196-1-git-send-email-jani.nik...@intel.com -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/7] drm/i915: Baytrail MIPI DSI support Updated
On 11/11/2013 02:20 PM, Thierry Reding wrote: On Sat, Nov 09, 2013 at 11:28:16AM +0100, Daniel Vetter wrote: On Sat, Nov 09, 2013 at 03:19:01PM +0530, Shobhit Kumar wrote: Hi All - These patches enhance the current support for MIPI DSI for Baytrail. They continue on the sub-encoder design and adds few more dev_ops to handle sequence correctly. Major changes are - 1. DSI Clock calculation based on pixel clock 2. Add new dev_ops for better sequencing the enable/disable path 3. Parameterized the hardcoded DSI parameters. These also forms building block for the generic MIPI driver to come in future based on enhancements in VBT. All these parameters are initialized or computed in the sub-encoder driver. Some of them might look unneccesary for now. I am also aware of the drm_bridge support now comming in and will in future migrate from sub-encoder design to drm_bridge. Just a quick aside: Thierry Reding from nvidia is also working on a DSI design for the tegra driver. Atm he seems to aim for a full-blown DSI bus based on his drm_panel patches for getting the panel metadata out of an ARM DT (we'd use VBT instead). Iirc there's no patches anywhere yet, but maybe Thierry could share a git branch somewhere with the wip stuff? Cc'ing Thierry and dri-devel in case a bigger discussion develops. I've been cleaning up the patches and was going to post them today. The implementation really isn't as full-blown as you make it sound =), primarily because the DSI panel that I have doesn't support things such as reading out the DDB, so I cannot test most of the functionality that I planned to. However I think introducing a DSI bus type is the right thing and it's been suggested recently that we have too few bus types. Furthermore it seems to be playing out rather nicely with the DRM panel work, so it would be really nice if Intel could test-drive this within their driver to see if it's good enough for their purposes as well. Interesting. Would be nice to have a look. Is everyone working on that subscribed to dri-devel or should I Cc the intel-gfx mailing list (or someone in particular) when posting the patches? Will keep an eye for your patches in dri-devel. Regards Shobhit ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [GMA500/CDV] HDMI AVI infoframe code
On Mon, Nov 11, 2013 at 6:57 AM, Gohad, Tushar tushar.go...@intel.com wrote: Folks, Hi From what I've seen there are no real HDMI connectors on either Poulsbo or Cedarview. It's just DVI with HDMI connectors. Though you might know of other hardware that I don't. SDVO_CMD_GET_SUPP_ENCODE always fails on my SDVO chips so maybe that can be pulled out of the gma500 driver entirely. Or it can stay and be fixed if hardware exists. Cheers Patrik When trying to get the GMA500 or the CDV DRM driver https://github.com/thomas001/cedarview-drm (w/ 3.8 Linux kernel) pass HDMI compliance, we are seeing failures of the nature: Verify that an AVI InfoFrame is transmitted on every two video fields. Error : No Legal AVI InfoFrame exists on two video fields. Apparently, avi_*_info_t structures are defined in https://github.com/thomas001/cedarview-drm/blob/master/staging/cdv/drv/psb_intel_hdmi.h the header but are never used. In https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/gma500/psb_intel_sdvo.c, I do see psb_intel_sdvo_set_avi_infoframe(), but appears to be really old based on the comment that says HDMI is not supported yet. Can any of the gma500/i915 code be reused for CDV infoframe code? Has anyone already done this is what I am trying to find out. Many thanks, Tushar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] Per Engine hang detection and recovery
From: Siluvery, Arun arun.siluv...@intel.com This patchset contains changes for Timeout detection and recovery (TDR) which provides per-engine hang detection and recovery. The current driver performs full gpu reset in case of a hang, TDR attempts to only reset the engine that is hung and it falls back to full reset if it fails. Full GPU reset can leave the system in a state where the display updates intermittently and possibly lock-up depending on the work load at the time of hang. TDR can help recover the system in those case thus increasing the stability. The changes are split in multiple patches. 1. Ring utility functions to save/restore context, reset ring etc 2. TDR hang detection logic and error recovery function 3. Debugfs changes to export TDR statistics. I have tested these changes on drm-intel-nightly with simple test which inserts a bad batch buffer on the specific to trigger a hang. TDR logic then detects this and recovers from it by skipping the bad batch. Please review and give your comments. regards Arun Siluvery, Arun (3): drm/1915: Add ring functions to save/restore context for per-ring reset drm/i915: Per-engine Timeout detection and recovery on HSW drm/i915: Export TDR hang count to debugfs drivers/gpu/drm/i915/i915_debugfs.c | 68 +++- drivers/gpu/drm/i915/i915_dma.c | 16 +- drivers/gpu/drm/i915/i915_drv.c | 195 +- drivers/gpu/drm/i915/i915_drv.h | 92 - drivers/gpu/drm/i915/i915_gem.c | 77 +++- drivers/gpu/drm/i915/i915_gpu_error.c | 25 +- drivers/gpu/drm/i915/i915_irq.c | 556 - drivers/gpu/drm/i915/i915_reg.h | 7 + drivers/gpu/drm/i915/intel_display.c| 25 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 607 +++- drivers/gpu/drm/i915/intel_ringbuffer.h | 51 +++ drivers/gpu/drm/i915/intel_uncore.c | 31 +- include/drm/drmP.h | 7 + 13 files changed, 1467 insertions(+), 290 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/1915: Add ring functions to save/restore context for per-ring reset
From: Siluvery, Arun arun.siluv...@intel.com Instead of full GPU reset, where possible a single ring can be reset individually. This patch adds functions to save ring's current state and it will be restored with the same state after reset. The state comprises of a set of ring specific registers. The actual hang detection and recovery changes are in subsequent patches. Signed-off-by: Siluvery, Arun arun.siluv...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 9 +- drivers/gpu/drm/i915/i915_reg.h | 7 + drivers/gpu/drm/i915/intel_ringbuffer.c | 595 drivers/gpu/drm/i915/intel_ringbuffer.h | 49 +++ drivers/gpu/drm/i915/intel_uncore.c | 25 ++ 5 files changed, 681 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b98a7c8..b0a244d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2423,6 +2423,7 @@ extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e, */ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv); void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv); +void gen6_gt_force_wake_restore(struct drm_device *dev); int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val); int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val); @@ -2456,13 +2457,13 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val); #define I915_READ16(reg) dev_priv-uncore.funcs.mmio_readw(dev_priv, (reg), true) #define I915_WRITE16(reg, val) dev_priv-uncore.funcs.mmio_writew(dev_priv, (reg), (val), true) -#define I915_READ16_NOTRACE(reg) dev_priv-uncore.funcs.mmio_readw(dev_priv, (reg), false) -#define I915_WRITE16_NOTRACE(reg, val) dev_priv-uncore.funcs.mmio_writew(dev_priv, (reg), (val), false) +#define I915_READ16_NOTRACE(reg) dev_priv-uncore.funcs.mmio_readw(dev_priv, (reg), false) +#define I915_WRITE16_NOTRACE(reg, val) dev_priv-uncore.funcs.mmio_writew(dev_priv, (reg), (val), false) #define I915_READ(reg) dev_priv-uncore.funcs.mmio_readl(dev_priv, (reg), true) #define I915_WRITE(reg, val) dev_priv-uncore.funcs.mmio_writel(dev_priv, (reg), (val), true) -#define I915_READ_NOTRACE(reg) dev_priv-uncore.funcs.mmio_readl(dev_priv, (reg), false) -#define I915_WRITE_NOTRACE(reg, val) dev_priv-uncore.funcs.mmio_writel(dev_priv, (reg), (val), false) +#define I915_READ_NOTRACE(reg) dev_priv-uncore.funcs.mmio_readl(dev_priv, (reg), false) +#define I915_WRITE_NOTRACE(reg, val) dev_priv-uncore.funcs.mmio_writel(dev_priv, (reg), (val), false) #define I915_WRITE64(reg, val) dev_priv-uncore.funcs.mmio_writeq(dev_priv, (reg), (val), true) #define I915_READ64(reg) dev_priv-uncore.funcs.mmio_readq(dev_priv, (reg), true) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 849e595..02e6ec6 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -33,6 +33,7 @@ #define _MASKED_BIT_ENABLE(a) (((a) 16) | (a)) #define _MASKED_BIT_DISABLE(a) ((a) 16) +#define _MASKED_BIT_ENABLE_ALL(a) (0x | (a)) /* PCI config space */ @@ -104,6 +105,7 @@ #define GEN6_GRDOM_RENDER (1 1) #define GEN6_GRDOM_MEDIA (1 2) #define GEN6_GRDOM_BLT(1 3) +#define GEN6_GRDOM_VEBOX (1 4) #define RING_PP_DIR_BASE(ring) ((ring)-mmio_base+0x228) #define RING_PP_DIR_BASE_READ(ring)((ring)-mmio_base+0x518) @@ -642,6 +644,8 @@ #define RING_SYNC_0(base) ((base)+0x40) #define RING_SYNC_1(base) ((base)+0x44) #define RING_SYNC_2(base) ((base)+0x48) +#define RING_MI_MODE(base) ((base)+0x9c) +#define RING_UHPTR(base) ((base)+0x134) #define GEN6_RVSYNC(RING_SYNC_0(RENDER_RING_BASE)) #define GEN6_RBSYNC(RING_SYNC_1(RENDER_RING_BASE)) #define GEN6_RVESYNC (RING_SYNC_2(RENDER_RING_BASE)) @@ -789,6 +793,8 @@ #define MI_MODE0x0209c # define VS_TIMER_DISPATCH (1 6) +# define MODE_STOP (1 8) +# define MODE_IDLE (1 9) # define MI_FLUSH_ENABLE (1 12) # define ASYNC_FLIP_PERF_DISABLE (1 14) @@ -799,6 +805,7 @@ #define GFX_MODE 0x02520 #define GFX_MODE_GEN7 0x0229c #define RING_MODE_GEN7(ring) ((ring)-mmio_base+0x29c) +#define RING_EXCC_GEN7(ring) ((ring)-mmio_base+0x028) #define GFX_RUN_LIST_ENABLE (115) #define GFX_TLB_INVALIDATE_ALWAYS(113) #define GFX_SURFACE_FAULT_ENABLE (112) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index b620337..cce29d0 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -46,6 +46,7 @@ void
[Intel-gfx] [PATCH 2/3] drm/i915: Per-engine Timeout detection and recovery on HSW
From: Siluvery, Arun arun.siluv...@intel.com TDR provides per-engine hang detection and recovery. If an engine hangs then the TDR will attempt to reset the engine and advance the command streamer to the next instruction in the ring. If it was in the middle of processing a batch buffer then control returns to the instruction following the batch buffer start command. This provides a less intrusive recovery mechanism because it only impacts the process which caused the hang. uevents are sent so that user mode can detect that something has gone wrong and take action if required. Issues: 1) Full GPU resets can leave the system in a state where the display updates intermittently or lock up the system. This problem already existed with the current driver which could only do full GPU resets in response to a hang on any of the rings. The problem has not been seen with per-engine TDR unless the driver falls back to full GPU reset, either because an individual ring reset fails or because the rings are hanging too quickly. Signed-off-by: Siluvery, Arun arun.siluv...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 6 +- drivers/gpu/drm/i915/i915_dma.c | 16 +- drivers/gpu/drm/i915/i915_drv.c | 195 ++- drivers/gpu/drm/i915/i915_drv.h | 80 - drivers/gpu/drm/i915/i915_gem.c | 77 - drivers/gpu/drm/i915/i915_gpu_error.c | 25 +- drivers/gpu/drm/i915/i915_irq.c | 555 ++-- drivers/gpu/drm/i915/intel_display.c| 25 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 20 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + drivers/gpu/drm/i915/intel_uncore.c | 6 +- include/drm/drmP.h | 7 + 12 files changed, 724 insertions(+), 290 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 6875b7a..0e5bcb4 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2604,9 +2604,13 @@ static int i915_wedged_set(void *data, u64 val) { struct drm_device *dev = data; + drm_i915_private_t *dev_priv = dev-dev_private; DRM_INFO(Manually setting wedged to %llu\n, val); - i915_handle_error(dev, val); + if (val) { + if (!i915_reset_in_progress(dev_priv-gpu_error)) + i915_handle_error(dev, NULL); + } return 0; } diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 0cab2d0..694da55 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1466,6 +1466,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) struct intel_device_info *info; int ret = 0, mmio_bar, mmio_size; uint32_t aperture_size; + uint32_t i; info = (struct intel_device_info *) flags; @@ -1661,6 +1662,17 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) acpi_video_register(); } + for (i = 0; i I915_NUM_RINGS; i++) { + dev_priv-hangcheck[i].count = 0; + dev_priv-hangcheck[i].last_acthd = 0; + dev_priv-hangcheck[i].ringid = i; + dev_priv-hangcheck[i].dev = dev; + + setup_timer(dev_priv-hangcheck[i].timer, + i915_hangcheck_sample, + (unsigned long) dev_priv-hangcheck[i]); + } + if (IS_GEN5(dev)) intel_gpu_ips_init(dev_priv); @@ -1703,6 +1715,7 @@ int i915_driver_unload(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; int ret; + uint32_t i; intel_gpu_ips_teardown(); @@ -1748,9 +1761,10 @@ int i915_driver_unload(struct drm_device *dev) } /* Free error state after interrupts are fully disabled. */ - del_timer_sync(dev_priv-gpu_error.hangcheck_timer); cancel_work_sync(dev_priv-gpu_error.work); i915_destroy_error_state(dev); + for (i = 0; i I915_NUM_RINGS; i++) + del_timer_sync(dev_priv-hangcheck[i].timer); cancel_delayed_work_sync(dev_priv-pc8.enable_work); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 24d58b0..1c8b96b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -113,6 +113,59 @@ MODULE_PARM_DESC(enable_hangcheck, WARNING: Disabling this can cause system wide hangs. (default: true)); +unsigned int i915_hangcheck_period __read_mostly = 1000; + +int hangcheck_period_set(const char *val, const struct kernel_param *kp) +{ + /* Custom set function so we can validate the range*/ + unsigned long num; + int ret; + + ret = kstrtoul(val, 0, num); + + if (ret) + return ret; + + /* Enforce minimum delay in ms */ + if ((num = MINIMUM_HANGCHECK_PERIOD) +
[Intel-gfx] [PATCH 3/3] drm/i915: Export TDR hang count to debugfs
From: Siluvery, Arun arun.siluv...@intel.com This patch adds changes to keep track of the number of the times TDR is triggered and the results for each ring are made available through debugfs. Signed-off-by: Siluvery, Arun arun.siluv...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 62 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/i915_irq.c | 1 + 3 files changed, 66 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 0e5bcb4..1556346 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2796,6 +2796,67 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops, i915_drop_caches_get, i915_drop_caches_set, 0x%08llx\n); +static ssize_t +i915_ring_hangcheck_read(struct file *filp, + char __user *ubuf, + size_t max, + loff_t *ppos) +{ + /* Returns the total number of times the rings +* have hung and been reset since boot */ + struct drm_device *dev = filp-private_data; + drm_i915_private_t *dev_priv = dev-dev_private; + char buf[200]; + int len; + + len = snprintf(buf, sizeof(buf), + GPU=0x%08X,RCS_T=0x%08X,VCS_T=0x%08X,BCS_T=0x%08X,VECS_T=0x%08X\n, + dev_priv-total_resets, + dev_priv-hangcheck[RCS].tdr_count, + dev_priv-hangcheck[VCS].tdr_count, + dev_priv-hangcheck[BCS].tdr_count, + dev_priv-hangcheck[VECS].tdr_count); + + if (len sizeof(buf)) + len = sizeof(buf); + + return simple_read_from_buffer(ubuf, max, ppos, buf, len); +} + +static ssize_t +i915_ring_hangcheck_write(struct file *filp, + const char __user *ubuf, + size_t cnt, + loff_t *ppos) +{ + struct drm_device *dev = filp-private_data; + struct drm_i915_private *dev_priv = dev-dev_private; + int ret; + uint32_t i; + + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + + for (i = 0; i I915_NUM_RINGS; i++) { + /* Reset the hangcheck counters */ + dev_priv-hangcheck[i].tdr_count = 0; + } + dev_priv-total_resets = 0; + + mutex_unlock(dev-struct_mutex); + + return cnt; +} + +static const struct file_operations i915_ring_hangcheck_fops = { + .owner = THIS_MODULE, + .open = simple_open, + .read = i915_ring_hangcheck_read, + .write = i915_ring_hangcheck_write, + .llseek = default_llseek, +}; + static int i915_max_freq_get(void *data, u64 *val) { @@ -3098,6 +3159,7 @@ static const struct i915_debugfs_files { {i915_error_state, i915_error_state_fops}, {i915_next_seqno, i915_next_seqno_fops}, {i915_display_crc_ctl, i915_display_crc_ctl_fops}, + {i915_ring_hangcheck, i915_ring_hangcheck_fops}, }; void intel_display_crc_init(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c539509..27225d6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1347,6 +1347,9 @@ struct intel_hangcheck { /* Keep a record of the last time the ring was reset */ unsigned long last_reset; + + /* Number of TDR hang detections for this ring */ + uint32_t tdr_count; }; typedef struct drm_i915_private { diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 78b2cbb..378383a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2517,6 +2517,7 @@ static bool i915_hangcheck_hung(struct intel_hangcheck *hc) DRM_DEBUG_TDR(hung=%d after kick ring\n, hung); } if (hung) { + hc-tdr_count++; i915_handle_error(dev, hc); } return hung; -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] intel_drv.so segfault
On Wed, Nov 06, 2013 at 11:26:07AM -0800, Ausmus, James wrote: On Wed, Nov 6, 2013 at 10:48 AM, Grant emailgr...@gmail.com wrote: Thank you, here's what I get: # addr2line -e /usr/lib64/xorg/modules/drivers/intel_drv.so -i 0x2fe79 0x3037f Grant - I'm assuming that this was done on the emerged xf86-video-intel, not the git-compiled one? /var/tmp/portage/x11-drivers/xf86-video-intel-2.99.905-r1/work/xf86-video-intel-2.99.905/src/sna/sna_accel.c:6079 git blame shows sna_accel.c:6079 (as of the 2.99.905 tag) last touched by 85fdc314 I think this is now https://bugs.freedesktop.org/show_bug.cgi?id=71415 and if so should already be fixed in git. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [GMA500/CDV] HDMI AVI infoframe code
On Mon, Nov 11, 2013 at 1:43 PM, Patrik Jakobsson patrik.r.jakobs...@gmail.com wrote: From what I've seen there are no real HDMI connectors on either Poulsbo or Cedarview. It's just DVI with HDMI connectors. Though you might know of other hardware that I don't. SDVO_CMD_GET_SUPP_ENCODE always fails on my SDVO chips so maybe that can be pulled out of the gma500 driver entirely. Or it can stay and be fixed if hardware exists. You need special hdmi sdvo encoders - all the hdmi features are optional and have been added in an update hdmi over sdvo spec. Note that the sdvo hdmi code in i915 isn't too massively spec compliant either since we fail to update the eld and a few related things. Otherwise I'd have suggested to copypaste that ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Per Engine hang detection and recovery
On Mon, Nov 11, 2013 at 02:58:31PM +, Siluvery, Arun wrote: From: Siluvery, Arun arun.siluv...@intel.com This patchset contains changes for Timeout detection and recovery (TDR) which provides per-engine hang detection and recovery. The current driver performs full gpu reset in case of a hang, TDR attempts to only reset the engine that is hung and it falls back to full reset if it fails. Full GPU reset can leave the system in a state where the display updates intermittently and possibly lock-up depending on the work load at the time of hang. TDR can help recover the system in those case thus increasing the stability. Are these hw lockups you've seen with full gpu reset or just kernel deadlocks? If it's the latter we've recently (re-)fixed a bunch of those, and if there are new ones we definitely want to fix them and add testcases to igt. So if you could share some of these hangs and their analysis/testcases that's be very interesting. That's of course on top of any other reset improvements. The changes are split in multiple patches. 1. Ring utility functions to save/restore context, reset ring etc 2. TDR hang detection logic and error recovery function 3. Debugfs changes to export TDR statistics. I have tested these changes on drm-intel-nightly with simple test which inserts a bad batch buffer on the specific to trigger a hang. TDR logic then detects this and recovers from it by skipping the bad batch. I want this testcase (as a patch to igt). Please review and give your comments. I'll try to have a look later this week, atm still busy with bdw upstreaming. One more meta-comment though: Something with your git setup seems to be broken, the patches don't have in-reply-to headers pointing at this cover letter and hence the threading is a bit broken. Cheers, Daniel regards Arun Siluvery, Arun (3): drm/1915: Add ring functions to save/restore context for per-ring reset drm/i915: Per-engine Timeout detection and recovery on HSW drm/i915: Export TDR hang count to debugfs drivers/gpu/drm/i915/i915_debugfs.c | 68 +++- drivers/gpu/drm/i915/i915_dma.c | 16 +- drivers/gpu/drm/i915/i915_drv.c | 195 +- drivers/gpu/drm/i915/i915_drv.h | 92 - drivers/gpu/drm/i915/i915_gem.c | 77 +++- drivers/gpu/drm/i915/i915_gpu_error.c | 25 +- drivers/gpu/drm/i915/i915_irq.c | 556 - drivers/gpu/drm/i915/i915_reg.h | 7 + drivers/gpu/drm/i915/intel_display.c| 25 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 607 +++- drivers/gpu/drm/i915/intel_ringbuffer.h | 51 +++ drivers/gpu/drm/i915/intel_uncore.c | 31 +- include/drm/drmP.h | 7 + 13 files changed, 1467 insertions(+), 290 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests/drv_suspend: Replace /dev/null with /dev/null 21
From: Oscar Mateo oscar.ma...@intel.com Some shells do not understand . For instance, my Ubuntu 12.04 machine has /bin/sh pointing to dash, which makes a mess out of (to the point that the helper processes cannot be killed). Signed-off-by: Oscar Mateo oscar.ma...@intel.com --- tests/drv_suspend.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/drv_suspend.c b/tests/drv_suspend.c index e526e2a..9b3df2b 100644 --- a/tests/drv_suspend.c +++ b/tests/drv_suspend.c @@ -106,7 +106,7 @@ test_debugfs_reader(void) static char tmp[1024]; snprintf(tmp, sizeof(tmp) - 1, -while true; do find %s/%i/ -type f | xargs cat /dev/null; done, +while true; do find %s/%i/ -type f | xargs cat /dev/null 21; done, dfs_base, drm_get_card()); assert(execl(/bin/sh, sh, -c, tmp, (char *) NULL) != -1); } @@ -131,7 +131,7 @@ test_sysfs_reader(void) static char tmp[1024]; snprintf(tmp, sizeof(tmp) - 1, -while true; do find %s%i*/ -type f | xargs cat /dev/null; done, +while true; do find %s%i*/ -type f | xargs cat /dev/null 21; done, dfs_base, drm_get_card()); assert(execl(/bin/sh, sh, -c, tmp, (char *) NULL) != -1); } -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Workaround for flicker with panning on the i830
On Mon, Nov 11, 2013 at 4:33 PM, Thomas Richter t...@math.tu-berlin.de wrote: Now, how much is known about the register DSPARB, found at offset 0x70030? Because, if I just feed this register with correct values (for whatever correct means), I do get a stable image on pipe A and pipe B. I haven't found out what correct is, precisely, but there is something I can do here. By default, on my setup, the value here is 0x17e5f, but depending on the scroll position, 0x17e51, 0x17e61, 0x17e6f or 0x17e63 create stable images. So there is at least some way how to get the image right, though I don't quite understand the mechanics yet. BTW, now that I looked at intel_regs_dump: When the screen is flickering, I do get pipe A underruns. I tried working with the watermark registers, but all I can get is a crashed GPU or a ripped display, so this is not quite the right approach. DSPARB seems to be the best trick so far, even though I don't quite get how it works. Oh, that's really interesting. gen2 has a unified display fifo on machines that support 2 outputs. DSPARB tells the hw how to exactly split this up between the two pipes. There are two bit ranges of interest here: bits0-8: AEND. This field selects one of the splits between two of the three portions of the display RAM allocated between display plane A, plane B, and display plane C. This field can only be programmed when one of the two pipes is disabled. When it is written. It takes effect on the next VBLANK for whichever pipe is currently active. The control granularity is 16-bytes. The total size of the Almador-M RAM is 288*16 bytes. The total size of the Montara-GM RAM is 256*16 bytes. bits 9-18: BEND. This field selects one of the split between the two of the three portions of the display RAM allocated between display planes A, B, and C. This field can only be programmed when one of the two display pipes is disabled. It takes effect on the next VBLANK for whichever pipe is currently active. It must be programmed to a number larger than the value in AEND. The control granularity is 16-bytes and the total size of the Almador-M RAM is 288*16 bytes and the Montara-GM RAM is 256*16 bytes. Almador-M is i830M, Montara-GM is i855GM. Other gen2 chips have a different meaning for this register. What we'd need to do here is to update this register when switching the number of active display pipes in the -modeset_global_resources hook. We also need to make sure we have updated watermark values set up already, before rewriting the value of DSPARB (since the watermarks depend upon the size of the fifo). For I start I'd go with splitting the fifo according to the display clock between plane A and B and giving nothing to plane C. We don't have any code to use plane C so giving everything to just A and B is better. I hope this helps you in your endeavor. Otherwise feel free to ping me on irc or mail. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Per Engine hang detection and recovery
On Mon, 2013-11-11 at 16:31 +0100, Daniel Vetter wrote: On Mon, Nov 11, 2013 at 02:58:31PM +, Siluvery, Arun wrote: From: Siluvery, Arun arun.siluv...@intel.com This patchset contains changes for Timeout detection and recovery (TDR) which provides per-engine hang detection and recovery. The current driver performs full gpu reset in case of a hang, TDR attempts to only reset the engine that is hung and it falls back to full reset if it fails. Full GPU reset can leave the system in a state where the display updates intermittently and possibly lock-up depending on the work load at the time of hang. TDR can help recover the system in those case thus increasing the stability. Are these hw lockups you've seen with full gpu reset or just kernel deadlocks? If it's the latter we've recently (re-)fixed a bunch of those, and if there are new ones we definitely want to fix them and add testcases to igt. So if you could share some of these hangs and their analysis/testcases that's be very interesting. That's of course on top of any other reset improvements. I think these are kernel lockups, unfortunately when this happens there is no response from the kernel, sending break is also not helping. I will try to get more details on this. The changes are split in multiple patches. 1. Ring utility functions to save/restore context, reset ring etc 2. TDR hang detection logic and error recovery function 3. Debugfs changes to export TDR statistics. I have tested these changes on drm-intel-nightly with simple test which inserts a bad batch buffer on the specific to trigger a hang. TDR logic then detects this and recovers from it by skipping the bad batch. I want this testcase (as a patch to igt). ok, I will send it to the mailing list. Please review and give your comments. I'll try to have a look later this week, atm still busy with bdw upstreaming. One more meta-comment though: Something with your git setup seems to be broken, the patches don't have in-reply-to headers pointing at this cover letter and hence the threading is a bit broken. ok thanks. yes my mistake I missed an option while generating the patches. Do you suggest resending all patches again? Cheers, Daniel regards Arun Siluvery, Arun (3): drm/1915: Add ring functions to save/restore context for per-ring reset drm/i915: Per-engine Timeout detection and recovery on HSW drm/i915: Export TDR hang count to debugfs drivers/gpu/drm/i915/i915_debugfs.c | 68 +++- drivers/gpu/drm/i915/i915_dma.c | 16 +- drivers/gpu/drm/i915/i915_drv.c | 195 +- drivers/gpu/drm/i915/i915_drv.h | 92 - drivers/gpu/drm/i915/i915_gem.c | 77 +++- drivers/gpu/drm/i915/i915_gpu_error.c | 25 +- drivers/gpu/drm/i915/i915_irq.c | 556 - drivers/gpu/drm/i915/i915_reg.h | 7 + drivers/gpu/drm/i915/intel_display.c| 25 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 607 +++- drivers/gpu/drm/i915/intel_ringbuffer.h | 51 +++ drivers/gpu/drm/i915/intel_uncore.c | 31 +- include/drm/drmP.h | 7 + 13 files changed, 1467 insertions(+), 290 deletions(-) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] lib/drmtest: Retire requests via drop caches after gem_quiescent_gpu
Hi Ben, I have drv_suspend running in a loop for a few hours now and I haven´t been able to hit that problem. Could you give some more info? were you using piglit? were you able to reproduce it systematically? I did have to fix the test for my system, thought (the output of the serial cat was going into std output instead of /dev/null and creating all kinds of craziness). I sent a patch to the mailing list. Cheers, Oscar -Original Message- From: Ben Widawsky [mailto:b...@bwidawsk.net] Sent: Monday, November 11, 2013 12:30 AM To: Daniel Vetter Cc: Mateo Lozano, Oscar; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] lib/drmtest: Retire requests via drop caches after gem_quiescent_gpu On Tue, Nov 05, 2013 at 07:35:16PM +0100, Daniel Vetter wrote: On Tue, Nov 05, 2013 at 02:15:19PM +, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com This helps make sure that the GPU is really quiescent by getting rid of any residual stuff. Signed-off-by: Oscar Mateo oscar.ma...@intel.com Merged, thanks for the patch. -Daniel I'm hitting what seems to be a race on gem_suspend. Please fix. Subtest debugfs-reader: SUCCESS Test assertion failure function igt_drop_caches_set, file igt_debugfs.c:336: --- lib/drmtest.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/drmtest.c b/lib/drmtest.c index e3fc166..d8fc60f 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -47,6 +47,7 @@ #include i915_drm.h #include intel_chipset.h #include intel_gpu_tools.h +#include igt_debugfs.h /* This file contains a bunch of wrapper functions to directly use gem ioctls. * Mostly useful to write kernel tests. */ @@ -163,6 +164,7 @@ void gem_quiescent_gpu(int fd) } gem_sync(fd, handle); + igt_drop_caches_set(DROP_RETIRE); } /** -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [GMA500/CDV] HDMI AVI infoframe code
Thanks Patrik and Daniel. Since one of the customers was able to get the Windows driver pass HDMI compliance, the hardware should be okay (or was worked around :-)). We'll try to port the i915 intel_hdmi.c code over to CDV. - Tushar From what I've seen there are no real HDMI connectors on either Poulsbo or Cedarview. It's just DVI with HDMI connectors. Though you might know of other hardware that I don't. SDVO_CMD_GET_SUPP_ENCODE always fails on my SDVO chips so maybe that can be pulled out of the gma500 driver entirely. Or it can stay and be fixed if hardware exists. You need special hdmi sdvo encoders - all the hdmi features are optional and have been added in an update hdmi over sdvo spec. Note that the sdvo hdmi code in i915 isn't too massively spec compliant either since we fail to update the eld and a few related things. Otherwise I'd have suggested to copypaste that ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drmtest: introduce kmsg_error functions
From: Paulo Zanoni paulo.r.zan...@intel.com These functions should help you checking for new Kernel error messages. One of the problems I had while writing the runtime PM test suite is that when you read the sysfs and debugfs files, the only way to detect errors is by checking dmesg, so I was always getting SUCCESS even if the test caught a bug. Also, we have so many debugfs/sysfs files that it was not easy to discover which file caused the error messages I was seeing. So this commit adds some infrastructure to allow us to automatically check for new errors on dmesg. Use it like this: int main(int argc, char *argv[]) { int fd, i; igt_fixture fd = kmsg_error_setup(); igt_subtest(t1) { kmsg_error_reset(fd); do_something(); kmsg_error_detect(); } igt_subtest(t2) { for (i = 0; i 10; i++) { char *file_name = get_file(i); kmsg_error_reset(fd); process_file(file_name); kmsg_error_detect(file_name): } } igt_fixture kmsg_error_teardown(fd); } Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- lib/drmtest.c | 79 +++ lib/drmtest.h | 13 ++ 2 files changed, 92 insertions(+) diff --git a/lib/drmtest.c b/lib/drmtest.c index d8fc60f..ebabfd8 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -41,6 +41,7 @@ #include linux/kd.h #include unistd.h #include sys/wait.h +#include poll.h #include drm_fourcc.h #include drmtest.h @@ -2307,3 +2308,81 @@ void igt_system_suspend_autoresume(void) ret = system(rtcwake -s 30 -m mem); igt_assert(ret == 0); } + +/* The kmsg error detecting functions allow you to catch new error messages from + * the Kernel. The idea is that you have to call kmsg_error_setup() only once at + * the beginning, and kmsg_error_teardown() at the end. And for every subtest, + * you run kmsg_error_reset() at the begin and kmsg_error_detect() at the end: + * this should make sure that kmsg_error_detect() will only catch the error + * messages that were introduced after the subtest started. */ +int kmsg_error_setup(void) +{ + int fd = open(/dev/kmsg, O_RDONLY); + + igt_assert_f(fd = 0, Can't open /dev/kmsg\n); + return fd; +} + +void kmsg_error_teardown(int fd) +{ + close(fd); +} + +/* You have to call this before running your subtest, so that the next time you + * call kmsg_error_detect you'll only look at the new kmsg lines. */ +void kmsg_error_reset(int fd) +{ + lseek(fd, 0, SEEK_END); +} + +static void kmsg_error_line_parse(const char *line, const char *error_msg) +{ + igt_assert_f(strstr(line, [ cut here ]) == 0, +%s\n, error_msg); + igt_assert_f(strstr(line, *ERROR*) == 0, %s\n, error_msg); + igt_assert_f(strstr(line, BUG: sleeping function called from invalid context at) == 0, +%s\n, error_msg); +} + +/* Keep reading the Kernel ring buffer and checking for errors. Stop reading if + * there's nothing new on the buffer after the timeout. Notice that every time + * you call this function, the time it will take to return will always be = the + * timeout. */ +void kmsg_error_detect(int fd, int timeout_ms, const char *error_msg) +{ + int i, rc; + int line_size = 128, line_used = 0, buf_size = 128; + char buf[buf_size]; + char *line; + struct pollfd pfd; + ssize_t readed; + + line = malloc(sizeof(char) * line_size); + igt_assert(line); + + pfd.fd = fd; + pfd.events = POLLIN | POLLPRI; + + while (1) { + pfd.revents = 0; + rc = poll(pfd, 1, timeout_ms); + if (!rc) + break; + + readed = read(fd, buf, buf_size - 1); + for (i = 0; i readed; i++) { + if (line_used + 1 = line_size) { + line = realloc(line, line_size * 2); + line_size *= 2; + igt_assert(line); + } + line[line_used++] = buf[i]; + if (buf[i] == '\n') { + line[line_used] = '\0'; + kmsg_error_line_parse(line, error_msg); + line_used = 0; + } + } + } + free(line); +} diff --git a/lib/drmtest.h b/lib/drmtest.h index a9fd0bc..05e3629 100644 --- a/lib/drmtest.h +++ b/lib/drmtest.h @@ -413,4 +413,17 @@ void igt_enable_prefault(void); /* suspend and auto-resume system */ void igt_system_suspend_autoresume(void); +/* Returns the fd for the other functions. */ +int kmsg_error_setup(void); +/* Closes the fd */ +void
[Intel-gfx] [PATCH 2/2] tests: add kms_edp_vdd_race
From: Paulo Zanoni paulo.r.zan...@intel.com We recently fixed a bug where it was impossible to do I2C transactions on eDP panels when they were disabled. Now it should be possible to do these transactions when the panel is disabled, but there's a race condition that triggers dmesg errors if we try do do the I2C transactions and set a mode on the panel at the same time. This program should reproduce this bug and check dmesg for errors. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- tests/.gitignore | 1 + tests/Makefile.am| 2 + tests/kms_edp_vdd_race.c | 226 +++ 3 files changed, 229 insertions(+) create mode 100644 tests/kms_edp_vdd_race.c diff --git a/tests/.gitignore b/tests/.gitignore index 09ea074..ddb61f9 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -102,6 +102,7 @@ igt_no_exit_list_only igt_no_subtest kms_addfb kms_cursor_crc +kms_edp_vdd_race kms_flip kms_pipe_crc_basic kms_render diff --git a/tests/Makefile.am b/tests/Makefile.am index 0426ec0..955d2a3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -52,6 +52,7 @@ TESTS_progs_M = \ gem_write_read_ring_switch \ kms_addfb \ kms_cursor_crc \ + kms_edp_vdd_race \ kms_flip \ kms_pipe_crc_basic \ kms_render \ @@ -236,6 +237,7 @@ gem_fence_thrash_LDADD = $(LDADD) -lpthread gem_flink_race_LDADD = $(LDADD) -lpthread gem_threaded_access_tiled_LDADD = $(LDADD) -lpthread prime_self_import_LDADD = $(LDADD) -lpthread +kms_edp_vdd_race_LDADD = $(LDADD) -lpthread gem_wait_render_timeout_LDADD = $(LDADD) -lrt kms_flip_LDADD = $(LDADD) -lrt diff --git a/tests/kms_edp_vdd_race.c b/tests/kms_edp_vdd_race.c new file mode 100644 index 000..a6bff65 --- /dev/null +++ b/tests/kms_edp_vdd_race.c @@ -0,0 +1,226 @@ +/* + * Copyright © 2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: Paulo Zanoni paulo.r.zan...@intel.com + * + */ + +#include dirent.h +#include fcntl.h +#include linux/i2c.h +#include linux/i2c-dev.h +#include pthread.h +#include string.h +#include sys/ioctl.h +#include sys/stat.h +#include sys/types.h + +#include drmtest.h + +int drm_fd, kmsg_fd; +drmModeResPtr res; +drmModeConnectorPtr edp_connector; +bool stop; + +static void disable_all_screens(void) +{ + int i, rc; + + for (i = 0; i res-count_crtcs; i++) { + rc = drmModeSetCrtc(drm_fd, res-crtcs[i], -1, 0, 0, + NULL, 0, NULL); + igt_assert(rc == 0); + } +} + +static void find_edp_connector(void) +{ + int i; + drmModeConnectorPtr c; + + edp_connector = NULL; + for (i = 0; i res-count_connectors; i++) { + c = drmModeGetConnector(drm_fd, res-connectors[i]); + + if (c-connector_type == DRM_MODE_CONNECTOR_eDP) { + igt_require(c-connection == DRM_MODE_CONNECTED); + edp_connector = c; + break; + } + + drmModeFreeConnector(c); + } + igt_require(edp_connector); +} + +static void read_edid_ioctl(int fd) +{ + unsigned char edid[128] = {}; + struct i2c_msg msgs[] = { + { /* Start at 0. */ + .addr = 0x50, + .flags = 0, + .len = 1, + .buf = edid, + }, { /* Now read the EDID. */ + .addr = 0x50, + .flags = I2C_M_RD, + .len = 128, + .buf = edid, + } + }; + struct i2c_rdwr_ioctl_data msgset = { + .msgs = msgs, + .nmsgs = 2, + }; + + ioctl(fd, I2C_RDWR, msgset); +} + +/* TODO: We're currently just trying to read all the I2C files. We should try to +
[Intel-gfx] [PATCH 2/2] intel: Silence warning in non-Valgrind build
From: Ian Romanick ian.d.roman...@intel.com Previously GCC was squaking about: intel_bufmgr_gem.c: In function 'drm_intel_gem_bo_map_unsynchronized': intel_bufmgr_gem.c:1325:20: warning: unused variable 'bo_gem' [-Wunused-variable] Wrapping with VG(); replaced that warning with: intel_bufmgr_gem.c: In function 'drm_intel_gem_bo_map_unsynchronized': intel_bufmgr_gem.c:1326:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] Which, looking at the definition of the VG macro, seems a bit weird. It's caused by the dangling ; left from the empty macro. Signed-off-by: Ian Romanick ian.d.roman...@intel.com Cc: Chia-I Wu olva...@gmail.com --- intel/intel_bufmgr_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 5b64a7f..dbadc52 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -1322,7 +1322,9 @@ int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo) int drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo) { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo-bufmgr; +#ifdef HAVE_VALGRIND drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; +#endif int ret; /* If the CPU cache isn't coherent with the GTT, then use a -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] intel: Add accessor to get HW context ID from a drm_intel_context
From: Ian Romanick ian.d.roman...@intel.com The drm_intel_context structure is, wisely, opaque. However, libdrm users may want to know the hardware context ID associated with the structure. Signed-off-by: Ian Romanick ian.d.roman...@intel.com Cc: Ben Widawsky b...@bwidawsk.net --- intel/intel_bufmgr.h | 1 + intel/intel_bufmgr_gem.c | 6 ++ 2 files changed, 7 insertions(+) diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h index 15f818e..7b28a70 100644 --- a/intel/intel_bufmgr.h +++ b/intel/intel_bufmgr.h @@ -194,6 +194,7 @@ drm_intel_context *drm_intel_gem_context_create(drm_intel_bufmgr *bufmgr); void drm_intel_gem_context_destroy(drm_intel_context *ctx); int drm_intel_gem_bo_context_exec(drm_intel_bo *bo, drm_intel_context *ctx, int used, unsigned int flags); +unsigned int drm_intel_gem_context_get_hw_context_id(const drm_intel_context *); int drm_intel_bo_gem_export_to_prime(drm_intel_bo *bo, int *prime_fd); drm_intel_bo *drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr *bufmgr, diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 029ca5d..5b64a7f 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -3020,6 +3020,12 @@ drm_intel_gem_context_destroy(drm_intel_context *ctx) free(ctx); } +unsigned int +drm_intel_gem_context_get_hw_context_id(const drm_intel_context *ctx) +{ + return ctx-ctx_id; +} + int drm_intel_reg_read(drm_intel_bufmgr *bufmgr, uint32_t offset, -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Per Engine hang detection and recovery
On Mon, Nov 11, 2013 at 03:49:01PM +, Siluvery, Arun wrote: On Mon, 2013-11-11 at 16:31 +0100, Daniel Vetter wrote: On Mon, Nov 11, 2013 at 02:58:31PM +, Siluvery, Arun wrote: From: Siluvery, Arun arun.siluv...@intel.com This patchset contains changes for Timeout detection and recovery (TDR) which provides per-engine hang detection and recovery. The current driver performs full gpu reset in case of a hang, TDR attempts to only reset the engine that is hung and it falls back to full reset if it fails. Full GPU reset can leave the system in a state where the display updates intermittently and possibly lock-up depending on the work load at the time of hang. TDR can help recover the system in those case thus increasing the stability. Are these hw lockups you've seen with full gpu reset or just kernel deadlocks? If it's the latter we've recently (re-)fixed a bunch of those, and if there are new ones we definitely want to fix them and add testcases to igt. So if you could share some of these hangs and their analysis/testcases that's be very interesting. That's of course on top of any other reset improvements. I think these are kernel lockups, unfortunately when this happens there is no response from the kernel, sending break is also not helping. I will try to get more details on this. Enabling lockdep and making sure you have the stuck task warning enabled occasionally helps to get something out of the system. Is the entire box dead or just everything related to gfx? The changes are split in multiple patches. 1. Ring utility functions to save/restore context, reset ring etc 2. TDR hang detection logic and error recovery function 3. Debugfs changes to export TDR statistics. I have tested these changes on drm-intel-nightly with simple test which inserts a bad batch buffer on the specific to trigger a hang. TDR logic then detects this and recovers from it by skipping the bad batch. I want this testcase (as a patch to igt). ok, I will send it to the mailing list. Thanks. Please review and give your comments. I'll try to have a look later this week, atm still busy with bdw upstreaming. One more meta-comment though: Something with your git setup seems to be broken, the patches don't have in-reply-to headers pointing at this cover letter and hence the threading is a bit broken. ok thanks. yes my mistake I missed an option while generating the patches. Do you suggest resending all patches again? No need, was just a quick reminder for next time around. Without threading patch groups are harder to find, especially when a bigger discussion ensued. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drmtest: introduce kmsg_error functions
On Mon, Nov 11, 2013 at 03:06:09PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com These functions should help you checking for new Kernel error messages. One of the problems I had while writing the runtime PM test suite is that when you read the sysfs and debugfs files, the only way to detect errors is by checking dmesg, so I was always getting SUCCESS even if the test caught a bug. Also, we have so many debugfs/sysfs files that it was not easy to discover which file caused the error messages I was seeing. So this commit adds some infrastructure to allow us to automatically check for new errors on dmesg. Use it like this: int main(int argc, char *argv[]) { int fd, i; igt_fixture fd = kmsg_error_setup(); igt_subtest(t1) { kmsg_error_reset(fd); do_something(); kmsg_error_detect(); } igt_subtest(t2) { for (i = 0; i 10; i++) { char *file_name = get_file(i); kmsg_error_reset(fd); process_file(file_name); kmsg_error_detect(file_name): } } igt_fixture kmsg_error_teardown(fd); } Imo that's the wrong approach. _Every_ test should fail if we end up with errors/backtraces in dmesg. And if you look for very specific stuff (like gpu hang or missed irq warnings) the approach thus far has been to expose that information somehow through debugfs files. That way we're independent from the exact string used in the kernel output. I think the right approach is to add this to the test runner, i.e. piglit. There's already very basic support to capture the (new) dmesg output for each test with the --dmesg option. Have you played around with that and tried to extend it to your liking? -Daniel Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- lib/drmtest.c | 79 +++ lib/drmtest.h | 13 ++ 2 files changed, 92 insertions(+) diff --git a/lib/drmtest.c b/lib/drmtest.c index d8fc60f..ebabfd8 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -41,6 +41,7 @@ #include linux/kd.h #include unistd.h #include sys/wait.h +#include poll.h #include drm_fourcc.h #include drmtest.h @@ -2307,3 +2308,81 @@ void igt_system_suspend_autoresume(void) ret = system(rtcwake -s 30 -m mem); igt_assert(ret == 0); } + +/* The kmsg error detecting functions allow you to catch new error messages from + * the Kernel. The idea is that you have to call kmsg_error_setup() only once at + * the beginning, and kmsg_error_teardown() at the end. And for every subtest, + * you run kmsg_error_reset() at the begin and kmsg_error_detect() at the end: + * this should make sure that kmsg_error_detect() will only catch the error + * messages that were introduced after the subtest started. */ +int kmsg_error_setup(void) +{ + int fd = open(/dev/kmsg, O_RDONLY); + + igt_assert_f(fd = 0, Can't open /dev/kmsg\n); + return fd; +} + +void kmsg_error_teardown(int fd) +{ + close(fd); +} + +/* You have to call this before running your subtest, so that the next time you + * call kmsg_error_detect you'll only look at the new kmsg lines. */ +void kmsg_error_reset(int fd) +{ + lseek(fd, 0, SEEK_END); +} + +static void kmsg_error_line_parse(const char *line, const char *error_msg) +{ + igt_assert_f(strstr(line, [ cut here ]) == 0, + %s\n, error_msg); + igt_assert_f(strstr(line, *ERROR*) == 0, %s\n, error_msg); + igt_assert_f(strstr(line, BUG: sleeping function called from invalid context at) == 0, + %s\n, error_msg); +} + +/* Keep reading the Kernel ring buffer and checking for errors. Stop reading if + * there's nothing new on the buffer after the timeout. Notice that every time + * you call this function, the time it will take to return will always be = the + * timeout. */ +void kmsg_error_detect(int fd, int timeout_ms, const char *error_msg) +{ + int i, rc; + int line_size = 128, line_used = 0, buf_size = 128; + char buf[buf_size]; + char *line; + struct pollfd pfd; + ssize_t readed; + + line = malloc(sizeof(char) * line_size); + igt_assert(line); + + pfd.fd = fd; + pfd.events = POLLIN | POLLPRI; + + while (1) { + pfd.revents = 0; + rc = poll(pfd, 1, timeout_ms); + if (!rc) + break; + + readed = read(fd, buf, buf_size - 1); + for (i = 0; i readed; i++) { + if (line_used + 1 = line_size) { + line = realloc(line, line_size * 2); + line_size *= 2; + igt_assert(line); +
Re: [Intel-gfx] [PATCH 2/2] tests: add kms_edp_vdd_race
On Mon, Nov 11, 2013 at 03:06:10PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We recently fixed a bug where it was impossible to do I2C transactions on eDP panels when they were disabled. Now it should be possible to do these transactions when the panel is disabled, but there's a race condition that triggers dmesg errors if we try do do the I2C transactions and set a mode on the panel at the same time. This program should reproduce this bug and check dmesg for errors. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Like I've said in the previous mail I think the generic dmesg error checking should be somewhere generic (and probably in piglit). Otherwise the test looks good. And the naming also matches the new convention ;-) -Daniel --- tests/.gitignore | 1 + tests/Makefile.am| 2 + tests/kms_edp_vdd_race.c | 226 +++ 3 files changed, 229 insertions(+) create mode 100644 tests/kms_edp_vdd_race.c diff --git a/tests/.gitignore b/tests/.gitignore index 09ea074..ddb61f9 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -102,6 +102,7 @@ igt_no_exit_list_only igt_no_subtest kms_addfb kms_cursor_crc +kms_edp_vdd_race kms_flip kms_pipe_crc_basic kms_render diff --git a/tests/Makefile.am b/tests/Makefile.am index 0426ec0..955d2a3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -52,6 +52,7 @@ TESTS_progs_M = \ gem_write_read_ring_switch \ kms_addfb \ kms_cursor_crc \ + kms_edp_vdd_race \ kms_flip \ kms_pipe_crc_basic \ kms_render \ @@ -236,6 +237,7 @@ gem_fence_thrash_LDADD = $(LDADD) -lpthread gem_flink_race_LDADD = $(LDADD) -lpthread gem_threaded_access_tiled_LDADD = $(LDADD) -lpthread prime_self_import_LDADD = $(LDADD) -lpthread +kms_edp_vdd_race_LDADD = $(LDADD) -lpthread gem_wait_render_timeout_LDADD = $(LDADD) -lrt kms_flip_LDADD = $(LDADD) -lrt diff --git a/tests/kms_edp_vdd_race.c b/tests/kms_edp_vdd_race.c new file mode 100644 index 000..a6bff65 --- /dev/null +++ b/tests/kms_edp_vdd_race.c @@ -0,0 +1,226 @@ +/* + * Copyright © 2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: Paulo Zanoni paulo.r.zan...@intel.com + * + */ + +#include dirent.h +#include fcntl.h +#include linux/i2c.h +#include linux/i2c-dev.h +#include pthread.h +#include string.h +#include sys/ioctl.h +#include sys/stat.h +#include sys/types.h + +#include drmtest.h + +int drm_fd, kmsg_fd; +drmModeResPtr res; +drmModeConnectorPtr edp_connector; +bool stop; + +static void disable_all_screens(void) +{ + int i, rc; + + for (i = 0; i res-count_crtcs; i++) { + rc = drmModeSetCrtc(drm_fd, res-crtcs[i], -1, 0, 0, + NULL, 0, NULL); + igt_assert(rc == 0); + } +} + +static void find_edp_connector(void) +{ + int i; + drmModeConnectorPtr c; + + edp_connector = NULL; + for (i = 0; i res-count_connectors; i++) { + c = drmModeGetConnector(drm_fd, res-connectors[i]); + + if (c-connector_type == DRM_MODE_CONNECTOR_eDP) { + igt_require(c-connection == DRM_MODE_CONNECTED); + edp_connector = c; + break; + } + + drmModeFreeConnector(c); + } + igt_require(edp_connector); +} + +static void read_edid_ioctl(int fd) +{ + unsigned char edid[128] = {}; + struct i2c_msg msgs[] = { + { /* Start at 0. */ + .addr = 0x50, + .flags = 0, + .len = 1, + .buf = edid, + }, { /* Now read the EDID. */ + .addr = 0x50, + .flags
Re: [Intel-gfx] [PATCH 1/2] drmtest: introduce kmsg_error functions
2013/11/11 Daniel Vetter dan...@ffwll.ch: On Mon, Nov 11, 2013 at 03:06:09PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com These functions should help you checking for new Kernel error messages. One of the problems I had while writing the runtime PM test suite is that when you read the sysfs and debugfs files, the only way to detect errors is by checking dmesg, so I was always getting SUCCESS even if the test caught a bug. Also, we have so many debugfs/sysfs files that it was not easy to discover which file caused the error messages I was seeing. So this commit adds some infrastructure to allow us to automatically check for new errors on dmesg. Use it like this: int main(int argc, char *argv[]) { int fd, i; igt_fixture fd = kmsg_error_setup(); igt_subtest(t1) { kmsg_error_reset(fd); do_something(); kmsg_error_detect(); } igt_subtest(t2) { for (i = 0; i 10; i++) { char *file_name = get_file(i); kmsg_error_reset(fd); process_file(file_name); kmsg_error_detect(file_name): } } igt_fixture kmsg_error_teardown(fd); } Imo that's the wrong approach. _Every_ test should fail if we end up with errors/backtraces in dmesg. That's exactly why I wrote code to check dmesg! We could, in the future, make the igt_subtest macros call this code automatically. And if you look for very specific stuff (like gpu hang or missed irq warnings) the approach thus far has been to expose that information somehow through debugfs files. So you're suggesting I should create some sort of debugfs interface to expose every single WARN our driver does? Doesn't really sound like a good idea, unless we invent our our I915_WARN, I915_ERROR, etc.. And we still won't catch the WARNs and ERRORs spit by drm.ko or anything outside i915.ko. That way we're independent from the exact string used in the kernel output. ZZ_check_dmesg already parses dmesg strings, I just copied it. Also, the whole IGT already relies way too much on being ran against very-recent libdrm/Kernels, we're just adding one more dependency. And we can also add the newer strings if somebody ever changes the WARN or DRM_ERROR output: it's not like our code will completely break, it just won't be as good. And we always require everybody to use IGT git master anyway. I don't see the problem. I think the right approach is to add this to the test runner, i.e. piglit. There's already very basic support to capture the (new) dmesg output for each test with the --dmesg option. Have you played around with that and tried to extend it to your liking? My goal is that I want to know, inside a test program, which line of code introduced the dmesg error, and if we use some sort of external approach like what you're suggesting this won't be possible. I have code that opens hundreds of sysfs and debugfs files, and I want to check dmesg after I open/close every single file, to be able to detect which one exactly causes the problem. I'm already using this locally and it *really* saved a lot of time for me. If we don 't accept this code inside drmtest.c, I'm gonna ask if I can push it directly to pm_pc8.c. -Daniel Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- lib/drmtest.c | 79 +++ lib/drmtest.h | 13 ++ 2 files changed, 92 insertions(+) diff --git a/lib/drmtest.c b/lib/drmtest.c index d8fc60f..ebabfd8 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -41,6 +41,7 @@ #include linux/kd.h #include unistd.h #include sys/wait.h +#include poll.h #include drm_fourcc.h #include drmtest.h @@ -2307,3 +2308,81 @@ void igt_system_suspend_autoresume(void) ret = system(rtcwake -s 30 -m mem); igt_assert(ret == 0); } + +/* The kmsg error detecting functions allow you to catch new error messages from + * the Kernel. The idea is that you have to call kmsg_error_setup() only once at + * the beginning, and kmsg_error_teardown() at the end. And for every subtest, + * you run kmsg_error_reset() at the begin and kmsg_error_detect() at the end: + * this should make sure that kmsg_error_detect() will only catch the error + * messages that were introduced after the subtest started. */ +int kmsg_error_setup(void) +{ + int fd = open(/dev/kmsg, O_RDONLY); + + igt_assert_f(fd = 0, Can't open /dev/kmsg\n); + return fd; +} + +void kmsg_error_teardown(int fd) +{ + close(fd); +} + +/* You have to call this before running your subtest, so that the next time you + * call kmsg_error_detect you'll only look at the new kmsg lines. */ +void kmsg_error_reset(int fd) +{ + lseek(fd, 0, SEEK_END); +} + +static void kmsg_error_line_parse(const char *line,
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Add BDW to ULT macro
On Fri, Nov 08, 2013 at 10:20:06AM -0800, Ben Widawsky wrote: For what we care about ULT and ULX are interchangeable. We know of 3 types of pciids for these cases. I am not sure if at some point we will need to distinguish ULT and ULX. Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net I think for -next we want a is_ult flag in our info struct. On modern platform this seems to be a much more useful disdinction than the is_mobile stuff from the gen2/3 days we still use ;-) Merged into bdw-fixes, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/i915_drv.h | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9944b26..8396eec 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1750,8 +1750,13 @@ struct drm_i915_file_private { #define IS_MOBILE(dev) (INTEL_INFO(dev)-is_mobile) #define IS_HSW_EARLY_SDV(dev)(IS_HASWELL(dev) \ ((dev)-pdev-device 0xFF00) == 0x0C00) -#define IS_ULT(dev) (IS_HASWELL(dev) \ +#define IS_BDW_ULT(dev) (IS_BROADWELL(dev) \ + (((dev)-pdev-device 0xf) == 0x2 || \ + ((dev)-pdev-device 0xf) == 0x6 || \ + ((dev)-pdev-device 0xf) == 0xe)) +#define IS_HSW_ULT(dev) (IS_HASWELL(dev) \ ((dev)-pdev-device 0xFF00) == 0x0A00) +#define IS_ULT(dev) (IS_HSW_ULT(dev) || IS_BDW_ULT(dev)) #define IS_HSW_GT3(dev) (IS_HASWELL(dev) \ ((dev)-pdev-device 0x00F0) == 0x0020) #define IS_PRELIMINARY_HW(intel_info) ((intel_info)-is_preliminary) -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: GEN8 backlight support
On Mon, Nov 11, 2013 at 11:12:57AM +0200, Jani Nikula wrote: From: Ben Widawsky benjamin.widaw...@intel.com Prior to Haswell the CPU control register for backlight (BLC_PWM_CPU_CTL) toggled the PCH baclight pin for us. This made some sense as there was no pin on the CPU. With Haswell came the introduction of a CPU backlight pin, but the interface was still controlled by software with the same mechnism. Behind the scenes, hardware did all the dirty work for us. Broadwell no longer provides this for free. If we want to use the PCH backlight pin [1] then we have to set the override bit BLC_PWM_PCH_CTL1 and program BLC_PWM_PCH_CTL2 for the PWM values. This patch implements that. This patch is compile tested only, and given that I rarely if ever touch this code, careful review is welcome. [1] According to Art, we know of no devices that exist which use the CPU pin (and remember it has existed already on HSW). If such a device does exist, we'll have to handle it properly - this is left as TODO until then. v2: Drop the abstraction prep patch, as a bigger backlight overhaul is in the works, and do just the mimimal bdw enabling now. (by Jani) CC: Art Runyan arthur.j.run...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Signed-off-by: Jani Nikula jani.nik...@intel.com Merged to bdw-fixes, thanks. -Daniel --- This supersedes: http://mid.gmane.org/1383889251-498-14-git-send-email-benjamin.widaw...@intel.com http://mid.gmane.org/1383889251-498-15-git-send-email-benjamin.widaw...@intel.com --- drivers/gpu/drm/i915/intel_panel.c | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index f161ac0..e6f782d 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -451,7 +451,9 @@ static u32 intel_panel_get_backlight(struct drm_device *dev, spin_lock_irqsave(dev_priv-backlight.lock, flags); - if (HAS_PCH_SPLIT(dev)) { + if (IS_BROADWELL(dev)) { + val = I915_READ(BLC_PWM_PCH_CTL2) BACKLIGHT_DUTY_CYCLE_MASK; + } else if (HAS_PCH_SPLIT(dev)) { val = I915_READ(BLC_PWM_CPU_CTL) BACKLIGHT_DUTY_CYCLE_MASK; } else { if (IS_VALLEYVIEW(dev)) @@ -479,6 +481,13 @@ static u32 intel_panel_get_backlight(struct drm_device *dev, return val; } +static void intel_bdw_panel_set_backlight(struct drm_device *dev, u32 level) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + u32 val = I915_READ(BLC_PWM_PCH_CTL2) ~BACKLIGHT_DUTY_CYCLE_MASK; + I915_WRITE(BLC_PWM_PCH_CTL2, val | level); +} + static void intel_pch_panel_set_backlight(struct drm_device *dev, u32 level) { struct drm_i915_private *dev_priv = dev-dev_private; @@ -496,7 +505,9 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev, DRM_DEBUG_DRIVER(set backlight PWM = %d\n, level); level = intel_panel_compute_brightness(dev, pipe, level); - if (HAS_PCH_SPLIT(dev)) + if (IS_BROADWELL(dev)) + return intel_bdw_panel_set_backlight(dev, level); + else if (HAS_PCH_SPLIT(dev)) return intel_pch_panel_set_backlight(dev, level); if (is_backlight_combination_mode(dev)) { @@ -666,7 +677,16 @@ void intel_panel_enable_backlight(struct intel_connector *connector) POSTING_READ(reg); I915_WRITE(reg, tmp | BLM_PWM_ENABLE); - if (HAS_PCH_SPLIT(dev) + if (IS_BROADWELL(dev)) { + /* + * Broadwell requires PCH override to drive the PCH + * backlight pin. The above will configure the CPU + * backlight pin, which we don't plan to use. + */ + tmp = I915_READ(BLC_PWM_PCH_CTL1); + tmp |= BLM_PCH_OVERRIDE_ENABLE | BLM_PCH_PWM_ENABLE; + I915_WRITE(BLC_PWM_PCH_CTL1, tmp); + } else if (HAS_PCH_SPLIT(dev) !(dev_priv-quirks QUIRK_NO_PCH_PWM_ENABLE)) { tmp = I915_READ(BLC_PWM_PCH_CTL1); tmp |= BLM_PCH_PWM_ENABLE; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] tests: add kms_edp_vdd_race
2013/11/11 Daniel Vetter dan...@ffwll.ch: On Mon, Nov 11, 2013 at 03:06:10PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We recently fixed a bug where it was impossible to do I2C transactions on eDP panels when they were disabled. Now it should be possible to do these transactions when the panel is disabled, but there's a race condition that triggers dmesg errors if we try do do the I2C transactions and set a mode on the panel at the same time. This program should reproduce this bug and check dmesg for errors. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Like I've said in the previous mail I think the generic dmesg error checking should be somewhere generic (and probably in piglit). Otherwise the test looks good. And the naming also matches the new convention ;-) Then this test will always give a SUCCESS. Not really what I wanted :( -Daniel --- tests/.gitignore | 1 + tests/Makefile.am| 2 + tests/kms_edp_vdd_race.c | 226 +++ 3 files changed, 229 insertions(+) create mode 100644 tests/kms_edp_vdd_race.c diff --git a/tests/.gitignore b/tests/.gitignore index 09ea074..ddb61f9 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -102,6 +102,7 @@ igt_no_exit_list_only igt_no_subtest kms_addfb kms_cursor_crc +kms_edp_vdd_race kms_flip kms_pipe_crc_basic kms_render diff --git a/tests/Makefile.am b/tests/Makefile.am index 0426ec0..955d2a3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -52,6 +52,7 @@ TESTS_progs_M = \ gem_write_read_ring_switch \ kms_addfb \ kms_cursor_crc \ + kms_edp_vdd_race \ kms_flip \ kms_pipe_crc_basic \ kms_render \ @@ -236,6 +237,7 @@ gem_fence_thrash_LDADD = $(LDADD) -lpthread gem_flink_race_LDADD = $(LDADD) -lpthread gem_threaded_access_tiled_LDADD = $(LDADD) -lpthread prime_self_import_LDADD = $(LDADD) -lpthread +kms_edp_vdd_race_LDADD = $(LDADD) -lpthread gem_wait_render_timeout_LDADD = $(LDADD) -lrt kms_flip_LDADD = $(LDADD) -lrt diff --git a/tests/kms_edp_vdd_race.c b/tests/kms_edp_vdd_race.c new file mode 100644 index 000..a6bff65 --- /dev/null +++ b/tests/kms_edp_vdd_race.c @@ -0,0 +1,226 @@ +/* + * Copyright © 2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: Paulo Zanoni paulo.r.zan...@intel.com + * + */ + +#include dirent.h +#include fcntl.h +#include linux/i2c.h +#include linux/i2c-dev.h +#include pthread.h +#include string.h +#include sys/ioctl.h +#include sys/stat.h +#include sys/types.h + +#include drmtest.h + +int drm_fd, kmsg_fd; +drmModeResPtr res; +drmModeConnectorPtr edp_connector; +bool stop; + +static void disable_all_screens(void) +{ + int i, rc; + + for (i = 0; i res-count_crtcs; i++) { + rc = drmModeSetCrtc(drm_fd, res-crtcs[i], -1, 0, 0, + NULL, 0, NULL); + igt_assert(rc == 0); + } +} + +static void find_edp_connector(void) +{ + int i; + drmModeConnectorPtr c; + + edp_connector = NULL; + for (i = 0; i res-count_connectors; i++) { + c = drmModeGetConnector(drm_fd, res-connectors[i]); + + if (c-connector_type == DRM_MODE_CONNECTOR_eDP) { + igt_require(c-connection == DRM_MODE_CONNECTED); + edp_connector = c; + break; + } + + drmModeFreeConnector(c); + } + igt_require(edp_connector); +} + +static void read_edid_ioctl(int fd) +{ + unsigned char edid[128] = {}; + struct i2c_msg msgs[] = { + { /* Start at 0. */ + .addr = 0x50, + .flags = 0, + .len = 1, +
Re: [Intel-gfx] [PATCH] tests/drv_suspend: Replace /dev/null with /dev/null 21
On Mon, Nov 11, 2013 at 03:36:55PM +, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com Some shells do not understand . For instance, my Ubuntu 12.04 machine has /bin/sh pointing to dash, which makes a mess out of (to the point that the helper processes cannot be killed). Signed-off-by: Oscar Mateo oscar.ma...@intel.com Looks good to me, pushed. Thanks for the patch. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] tests: add kms_edp_vdd_race
On Mon, Nov 11, 2013 at 04:25:36PM -0200, Paulo Zanoni wrote: 2013/11/11 Daniel Vetter dan...@ffwll.ch: On Mon, Nov 11, 2013 at 03:06:10PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We recently fixed a bug where it was impossible to do I2C transactions on eDP panels when they were disabled. Now it should be possible to do these transactions when the panel is disabled, but there's a race condition that triggers dmesg errors if we try do do the I2C transactions and set a mode on the panel at the same time. This program should reproduce this bug and check dmesg for errors. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Like I've said in the previous mail I think the generic dmesg error checking should be somewhere generic (and probably in piglit). Otherwise the test looks good. And the naming also matches the new convention ;-) Then this test will always give a SUCCESS. Not really what I wanted :( It's not the only one. We have tests that only annoy the in-kernel debug features like lockdep, object use-after-free and other stuff. Or all the WARN backtraces from testdisplay. And very often they all succeed. Checking dmesg in individual tests really doesn't make much sense imo and needs to be somewhere where it's done for _all_ testcases. QA already has that in their own testrunner infrastructure, unfortunately that's not shared with developers so we get to invent a new wheel. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drmtest: introduce kmsg_error functions
On Mon, Nov 11, 2013 at 04:18:04PM -0200, Paulo Zanoni wrote: 2013/11/11 Daniel Vetter dan...@ffwll.ch: On Mon, Nov 11, 2013 at 03:06:09PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com These functions should help you checking for new Kernel error messages. One of the problems I had while writing the runtime PM test suite is that when you read the sysfs and debugfs files, the only way to detect errors is by checking dmesg, so I was always getting SUCCESS even if the test caught a bug. Also, we have so many debugfs/sysfs files that it was not easy to discover which file caused the error messages I was seeing. So this commit adds some infrastructure to allow us to automatically check for new errors on dmesg. Use it like this: int main(int argc, char *argv[]) { int fd, i; igt_fixture fd = kmsg_error_setup(); igt_subtest(t1) { kmsg_error_reset(fd); do_something(); kmsg_error_detect(); } igt_subtest(t2) { for (i = 0; i 10; i++) { char *file_name = get_file(i); kmsg_error_reset(fd); process_file(file_name); kmsg_error_detect(file_name): } } igt_fixture kmsg_error_teardown(fd); } Imo that's the wrong approach. _Every_ test should fail if we end up with errors/backtraces in dmesg. That's exactly why I wrote code to check dmesg! We could, in the future, make the igt_subtest macros call this code automatically. That leaves out tests that aren't yet converted over to subtests ... And if you look for very specific stuff (like gpu hang or missed irq warnings) the approach thus far has been to expose that information somehow through debugfs files. So you're suggesting I should create some sort of debugfs interface to expose every single WARN our driver does? Doesn't really sound like a good idea, unless we invent our our I915_WARN, I915_ERROR, etc.. And we still won't catch the WARNs and ERRORs spit by drm.ko or anything outside i915.ko. Nope. I'm suggesting to do this in the piglit runner instead so that all tests profit automatically. Since as you say, we can't patch lockdep. That way we're independent from the exact string used in the kernel output. ZZ_check_dmesg already parses dmesg strings, I just copied it. Also, the whole IGT already relies way too much on being ran against very-recent libdrm/Kernels, we're just adding one more dependency. And we can also add the newer strings if somebody ever changes the WARN or DRM_ERROR output: it's not like our code will completely break, it just won't be as good. And we always require everybody to use IGT git master anyway. I don't see the problem. ZZ_check_dmesg was a quick hack done two years ago that should have been ported to something sane a long time ago. With piglit's non-deterministic test ordering it's pretty useless nowadays. I think the right approach is to add this to the test runner, i.e. piglit. There's already very basic support to capture the (new) dmesg output for each test with the --dmesg option. Have you played around with that and tried to extend it to your liking? My goal is that I want to know, inside a test program, which line of code introduced the dmesg error, and if we use some sort of external approach like what you're suggesting this won't be possible. I have code that opens hundreds of sysfs and debugfs files, and I want to check dmesg after I open/close every single file, to be able to detect which one exactly causes the problem. I'm already using this locally and it *really* saved a lot of time for me. If we don 't accept this code inside drmtest.c, I'm gonna ask if I can push it directly to pm_pc8.c. Hm, thus far I've just looked at the functions in the backtrace. Can you give an example of which kinds of bugs your hunting that need this? But in general I certainly don't want this in pc8.c. If it's indeed useful to check dmesg after each step in some tests then having some helpers in igt/lib makes sense. Just doing it for the overall test though just duplicates functionality which already exists in piglit. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] lib/drmtest: Retire requests via drop caches after gem_quiescent_gpu
On Mon, Nov 11, 2013 at 03:49:01PM +, Mateo Lozano, Oscar wrote: Hi Ben, I have drv_suspend running in a loop for a few hours now and I haven´t been able to hit that problem. Could you give some more info? were you using piglit? were you able to reproduce it systematically? I did have to fix the test for my system, thought (the output of the serial cat was going into std output instead of /dev/null and creating all kinds of craziness). I sent a patch to the mailing list. Cheers, Oscar I can get exactly what I was running when I get home. It was exactly something to the effect of: while [ 1 ] ; do drv_suspend --run-subtest debufs-reader ; done I may have put a few other tests before, be definitely none after. -Original Message- From: Ben Widawsky [mailto:b...@bwidawsk.net] Sent: Monday, November 11, 2013 12:30 AM To: Daniel Vetter Cc: Mateo Lozano, Oscar; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] lib/drmtest: Retire requests via drop caches after gem_quiescent_gpu On Tue, Nov 05, 2013 at 07:35:16PM +0100, Daniel Vetter wrote: On Tue, Nov 05, 2013 at 02:15:19PM +, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com This helps make sure that the GPU is really quiescent by getting rid of any residual stuff. Signed-off-by: Oscar Mateo oscar.ma...@intel.com Merged, thanks for the patch. -Daniel I'm hitting what seems to be a race on gem_suspend. Please fix. Subtest debugfs-reader: SUCCESS Test assertion failure function igt_drop_caches_set, file igt_debugfs.c:336: --- lib/drmtest.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/drmtest.c b/lib/drmtest.c index e3fc166..d8fc60f 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -47,6 +47,7 @@ #include i915_drm.h #include intel_chipset.h #include intel_gpu_tools.h +#include igt_debugfs.h /* This file contains a bunch of wrapper functions to directly use gem ioctls. * Mostly useful to write kernel tests. */ @@ -163,6 +164,7 @@ void gem_quiescent_gpu(int fd) } gem_sync(fd, handle); + igt_drop_caches_set(DROP_RETIRE); } /** -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] intel: Add accessor to get HW context ID from a drm_intel_context
Ian Romanick i...@freedesktop.org writes: From: Ian Romanick ian.d.roman...@intel.com The drm_intel_context structure is, wisely, opaque. However, libdrm users may want to know the hardware context ID associated with the structure. We've had a bunch of our other structures be partially transparent. The context id to be passed to the kernel could easily be public just like the gem handle in a BO is public. I would lean slightly toward that. But I don't feel strongly either way, so these two are: Reviewed-by: Eric Anholt e...@anholt.net pgpNMaTtYx36c.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] tests: add kms_edp_vdd_race
On Mon, Nov 11, 2013 at 04:54:32PM -0200, Paulo Zanoni wrote: 2013/11/11 Daniel Vetter dan...@ffwll.ch: On Mon, Nov 11, 2013 at 04:25:36PM -0200, Paulo Zanoni wrote: 2013/11/11 Daniel Vetter dan...@ffwll.ch: On Mon, Nov 11, 2013 at 03:06:10PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We recently fixed a bug where it was impossible to do I2C transactions on eDP panels when they were disabled. Now it should be possible to do these transactions when the panel is disabled, but there's a race condition that triggers dmesg errors if we try do do the I2C transactions and set a mode on the panel at the same time. This program should reproduce this bug and check dmesg for errors. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Like I've said in the previous mail I think the generic dmesg error checking should be somewhere generic (and probably in piglit). Otherwise the test looks good. And the naming also matches the new convention ;-) Then this test will always give a SUCCESS. Not really what I wanted :( It's not the only one. We have tests that only annoy the in-kernel debug features like lockdep, object use-after-free and other stuff. Or all the WARN backtraces from testdisplay. And very often they all succeed. And that's the problem I'm trying to solve. We have a solution, it's useful not just for me - you just gave examples of where it would be useful too -, yet, IMHO, you still didn't give a good technical reason on why you're rejecting it. Checking dmesg in individual tests really doesn't make much sense imo Well, IMHO it makes a lot of sense. It's even helping me write code, as I already explained. and needs to be somewhere where it's done for _all_ testcases. My code is not preventing that. In fact, I think it's helping us get to that point. QA already has that in their own testrunner infrastructure, unfortunately that's not shared with developers so we get to invent a new wheel. I just proposed these new wheels... Ok, lazy me finally got around to just doing it. I've sent 2 patches to the piglit mailing list which enable dmesg checking for igt runs by default in less than 5 lines of code. For all tests, at the subtest granularity. Imo this simplicity a technical reason to do it in piglit ;-) That leaves us with your use-case of very fine-grained checking of dmesg errors within a testcase. tbh I'm not really sold on this being that useful, but I'd be ok with merging the helper code if you convinced it's a great idea. One thing though which could be improved is the cleanup - imo it's much simpler to just have an atexit handler for such helpers. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] lib: adjust oom_score
This way the igt test will always be killed first (hopefully), preventing mayhem when one of the memory thrashing tests treatens to take down the entire system. To avoid any burden on test writers we adjust the oom score on drm_open, any of the fork helpers and subtest init. That should cover everything. Cc: Ben Widawsky b...@bwidawsk.net Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- lib/drmtest.c | 16 1 file changed, 16 insertions(+) diff --git a/lib/drmtest.c b/lib/drmtest.c index d8fc60f656e4..7f13e0561935 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -203,6 +203,15 @@ int drm_get_card(void) return -1; } +static void oom_adjust_for_doom(void) +{ + int fd; + const char always_kill[] = 1000; + + igt_assert(fd = open(/proc/self/oom_adj, O_WRONLY) != -1); + igt_assert(write(fd, always_kill, sizeof(always_kill)) == sizeof(always_kill)); +} + /** Open the first DRM device we can find, searching up to 16 device nodes */ static int __drm_open_any(void) { @@ -221,6 +230,8 @@ static int __drm_open_any(void) fd = -1; } + oom_adjust_for_doom(); + return fd; } @@ -250,6 +261,8 @@ static int __drm_open_any_render(void) return fd; } + oom_adjust_for_doom(); + return fd; } @@ -843,6 +856,7 @@ int igt_subtest_init_parse_opts(int argc, char **argv, } igt_install_exit_handler(check_igt_exit); + oom_adjust_for_doom(); out: return ret; @@ -1114,6 +1128,7 @@ bool __igt_fork_helper(struct igt_helper_process *proc) case 0: exit_handler_count = 0; reset_helper_process_list(); + oom_adjust_for_doom(); return true; default: @@ -1196,6 +1211,7 @@ bool __igt_fork(void) test_child = true; exit_handler_count = 0; reset_helper_process_list(); + oom_adjust_for_doom(); return true; default: -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/bdw: PIPE_[BC] I[ME]R moved to powerwell
The pipe B and pipe C interrupt mask and enable registers are now part of the pipe, so disabling the pipe power wells will lost the contests of the registers. Art totally debugged this one! v2: Use the irq_lock to clarify code, and prevent future bugs (Daniel) Cc: Art Runyan arthur.j.run...@intel.com Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_pm.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0a07d7c..38915dc 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5684,6 +5684,7 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable) { struct drm_i915_private *dev_priv = dev-dev_private; bool is_enabled, enable_requested; + unsigned long irqflags; uint32_t tmp; tmp = I915_READ(HSW_PWR_WELL_DRIVER); @@ -5701,9 +5702,22 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable) HSW_PWR_WELL_STATE_ENABLED), 20)) DRM_ERROR(Timeout enabling power well\n); } + + if (IS_BROADWELL(dev)) { + spin_lock_irqsave(dev_priv-irq_lock, irqflags); + I915_WRITE(GEN8_DE_PIPE_IMR(PIPE_B), + dev_priv-de_irq_mask[PIPE_B]); + I915_WRITE(GEN8_DE_PIPE_IER(PIPE_B), + ~dev_priv-de_irq_mask[PIPE_B] | GEN8_PIPE_VBLANK); + I915_WRITE(GEN8_DE_PIPE_IMR(PIPE_C), + dev_priv-de_irq_mask[PIPE_C]); + I915_WRITE(GEN8_DE_PIPE_IER(PIPE_C), + ~dev_priv-de_irq_mask[PIPE_C] | GEN8_PIPE_VBLANK); + POSTING_READ(GEN8_DE_PIPE_IER(PIPE_C)); + spin_unlock_irqrestore(dev_priv-irq_lock, irqflags); + } } else { if (enable_requested) { - unsigned long irqflags; enum pipe p; I915_WRITE(HSW_PWR_WELL_DRIVER, 0); -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: add i915_get_reset_stats_ioctl
On 11/08/2013 10:11 AM, Damien Lespiau wrote: On Wed, Oct 30, 2013 at 03:44:16PM +0200, Mika Kuoppala wrote: This ioctl returns reset stats for specified context. The struct returned contains context loss counters. reset_count:all resets across all contexts batch_active: active batches lost on resets batch_pending: pending batches lost on resets v2: get rid of state tracking completely and deliver only counts. Idea from Chris Wilson. v3: fix commit message v4: default context handled inside i915_gem_context_get_hang_stats v5: reset_count only for priviledged process v6: ctx=0 needs CAP_SYS_ADMIN for batch_* counters (Chris Wilson) v7: context hang stats never returns NULL v8: rebased on top of reworked context hang stats DRM_RENDER_ALLOW for ioctl v9: use DEFAULT_CONTEXT_ID. Improve comments for ioctl struct members Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com Cc: Ian Romanick i...@freedesktop.org Cc: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch The logic there looks good to me, I was just wondering if we wanted a bit more padding in case we want other counters or misc stuff. I think the current padding should be sufficient. The other things that could occur aren't things that we should return counts of. For those kinds of things, we'd only want to return flags. In fact, the current user space only uses the counts as flags anyway (is the count non-zero?). Reviewed-by: Damien Lespiau damien.lesp...@intel.com Also, Reviewed-by: Ian Romanick ian.d.roman...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: PIPE_[BC] I[ME]R moved to powerwell
On Mon, Nov 11, 2013 at 02:46:28PM -0800, Ben Widawsky wrote: The pipe B and pipe C interrupt mask and enable registers are now part of the pipe, so disabling the pipe power wells will lost the contests of the registers. Art totally debugged this one! v2: Use the irq_lock to clarify code, and prevent future bugs (Daniel) Cc: Art Runyan arthur.j.run...@intel.com Cc: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Merged to bdw-fixes, thanks. -Daniel --- drivers/gpu/drm/i915/intel_pm.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0a07d7c..38915dc 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5684,6 +5684,7 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable) { struct drm_i915_private *dev_priv = dev-dev_private; bool is_enabled, enable_requested; + unsigned long irqflags; uint32_t tmp; tmp = I915_READ(HSW_PWR_WELL_DRIVER); @@ -5701,9 +5702,22 @@ static void __intel_set_power_well(struct drm_device *dev, bool enable) HSW_PWR_WELL_STATE_ENABLED), 20)) DRM_ERROR(Timeout enabling power well\n); } + + if (IS_BROADWELL(dev)) { + spin_lock_irqsave(dev_priv-irq_lock, irqflags); + I915_WRITE(GEN8_DE_PIPE_IMR(PIPE_B), +dev_priv-de_irq_mask[PIPE_B]); + I915_WRITE(GEN8_DE_PIPE_IER(PIPE_B), +~dev_priv-de_irq_mask[PIPE_B] | GEN8_PIPE_VBLANK); + I915_WRITE(GEN8_DE_PIPE_IMR(PIPE_C), +dev_priv-de_irq_mask[PIPE_C]); + I915_WRITE(GEN8_DE_PIPE_IER(PIPE_C), +~dev_priv-de_irq_mask[PIPE_C] | GEN8_PIPE_VBLANK); + POSTING_READ(GEN8_DE_PIPE_IER(PIPE_C)); + spin_unlock_irqrestore(dev_priv-irq_lock, irqflags); + } } else { if (enable_requested) { - unsigned long irqflags; enum pipe p; I915_WRITE(HSW_PWR_WELL_DRIVER, 0); -- 1.8.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Nov 2013 MSR: Kevin Rogovin
MSR Nov 2013, Kevin Rogovin Benchmark Suite: Creation of a benchmark suite (found on Teamforge) where each benchmark stresses a particular point of an OpenGL implementation. A number of bugs have been found and reported from running this suite on Mesa: https://bugs.freedesktop.org/show_bug.cgi?id=71254 and https://bugs.freedesktop.org/show_bug.cgi?id=70613 . Additional benchmarks made to test vertex fetch, texture bind, draw call overhead and GLSL compiler tests. The benchmark suite development is ongoing. Requests are welcome for what should be tested isolated. Analysis: Contribute to analysis of API Trace logs of WebGL in Chrome under both Linux and ChromeOS. Mesa: Read and document the flow/control of Mesa and i965 backend. Chad Versary has setup an auto build of the produced doxygen output for this documentation athttp://people.freedesktop.org/~chadversary/mesa/doxygen/kevin-rogovin/ , feed back appreciated. Documentation is updated regularly. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx