Re: [Intel-gfx] [PATCH 1/2 V4] i915/drm: Add private api for power well usage
At Mon, 20 May 2013 19:26:57 +0800, Wang Xingchao wrote: Haswell Display audio depends on power well in graphic side, it should request power well before use it and release power well after use. I915 will not shutdown power well if it detects audio is using. This patch protects display audio crash for Intel Haswell C3 stepping board. Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com --- drivers/gpu/drm/i915/i915_dma.c |6 +++ drivers/gpu/drm/i915/i915_drv.h | 12 + drivers/gpu/drm/i915/intel_drv.h |4 ++ drivers/gpu/drm/i915/intel_pm.c | 98 +++--- include/drm/i915_powerwell.h | 36 ++ 5 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 include/drm/i915_powerwell.h diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a1648eb..2b9010a 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1652,6 +1652,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Start out suspended */ dev_priv-mm.suspended = 1; + if (IS_HASWELL(dev)) + i915_init_power_well(dev); + if (drm_core_check_feature(dev, DRIVER_MODESET)) { ret = i915_load_modeset_init(dev); if (ret 0) { @@ -1708,6 +1711,9 @@ int i915_driver_unload(struct drm_device *dev) intel_gpu_ips_teardown(); + if (IS_HASWELL(dev)) + i915_remove_power_well(dev); + i915_teardown_sysfs(dev); if (dev_priv-mm.inactive_shrinker.shrink) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3ac71db..a0f1a6d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -702,6 +702,15 @@ struct intel_ilk_power_mgmt { struct drm_i915_gem_object *renderctx; }; +/* Power well structure for haswell */ +struct i915_power_well { + struct drm_device *device; + spinlock_t lock; + /* power well enable/disable usage count */ + int count; + int i915_request; +}; + struct i915_dri1_state { unsigned allow_batchbuffer : 1; u32 __iomem *gfx_hws_cpu_addr; @@ -1048,6 +1057,9 @@ typedef struct drm_i915_private { * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips; + /* Haswell power well */ + struct i915_power_well *hsw_pwr; + enum no_fbc_reason no_fbc_reason; struct drm_mm_node *compressed_fb; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dfcf546..efcccab 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -759,6 +759,10 @@ extern void intel_update_fbc(struct drm_device *dev); extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv); extern void intel_gpu_ips_teardown(void); +/* Power well */ +extern int i915_init_power_well(struct drm_device *dev); +extern void i915_remove_power_well(struct drm_device *dev); + extern bool intel_using_power_well(struct drm_device *dev); extern void intel_init_power_well(struct drm_device *dev); extern void intel_set_power_well(struct drm_device *dev, bool enable); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0f4b46e..eec7aa8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4344,18 +4344,12 @@ bool intel_using_power_well(struct drm_device *dev) return true; } -void intel_set_power_well(struct drm_device *dev, bool enable) +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; uint32_t tmp; - if (!HAS_POWER_WELL(dev)) - return; - - if (!i915_disable_power_well !enable) - return; - tmp = I915_READ(HSW_PWR_WELL_DRIVER); is_enabled = tmp HSW_PWR_WELL_STATE; enable_requested = tmp HSW_PWR_WELL_ENABLE; @@ -4378,6 +4372,96 @@ void intel_set_power_well(struct drm_device *dev, bool enable) } } +static struct i915_power_well *hsw_pwr; + +/* Display audio driver power well request */ +void i915_request_power_well(void) +{ + if (hsw_pwr == NULL) + return; + + spin_lock_irq(hsw_pwr-lock); + if (!hsw_pwr-count++ + !hsw_pwr-i915_request) + __intel_set_power_well(hsw_pwr-device, true); + spin_unlock_irq(hsw_pwr-lock); +} +EXPORT_SYMBOL_GPL(i915_request_power_well); + +/* Display audio driver power well release */ +void i915_release_power_well(void) +{ + if (hsw_pwr == NULL) + return; + + spin_lock_irq(hsw_pwr-lock); + + /* i915 using power well */ + if (hsw_pwr-i915_request) { + spin_unlock_irq(hsw_pwr-lock); + return; +
Re: [Intel-gfx] [PATCH 2/2 V4] ALSA: hda - Add power-welll support for haswell HDA
At Mon, 20 May 2013 19:26:58 +0800, Wang Xingchao wrote: For Intel Haswell chip, HDA controller and codec have power well dependency from GPU side. This patch added support to request/release power well in audio driver. Power save feature should be enabled to get runtime power saving. Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com --- sound/pci/hda/Kconfig | 10 ++ sound/pci/hda/Makefile|3 ++ sound/pci/hda/hda_i915.c | 75 + sound/pci/hda/hda_i915.h | 35 + sound/pci/hda/hda_intel.c | 41 +++-- 5 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 sound/pci/hda/hda_i915.c create mode 100644 sound/pci/hda/hda_i915.h diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 80a7d44..c5a872c 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -152,6 +152,16 @@ config SND_HDA_CODEC_HDMI snd-hda-codec-hdmi. This module is automatically loaded at probing. +config SND_HDA_I915 + bool Build Display HD-audio controller/codec power well support for i915 cards + depends on DRM_I915 + help + Say Y here to include full HDMI and DisplayPort HD-audio controller/codec + power-well support for Intel Haswell graphics cards based on the i915 driver. + + Note that this option must be enabled for Intel Haswell C+ stepping machines, otherwise + the GPU audio controller/codecs will not be initialized or damaged when exit from S3 mode. + config SND_HDA_CODEC_CIRRUS bool Build Cirrus Logic codec support default y diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index 24a2514..4b0a4bc 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -6,6 +6,9 @@ snd-hda-codec-$(CONFIG_PROC_FS) += hda_proc.o snd-hda-codec-$(CONFIG_SND_HDA_HWDEP) += hda_hwdep.o snd-hda-codec-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o +# for haswell power well +snd-hda-intel-$(CONFIG_SND_HDA_I915) += hda_i915.o + # for trace-points CFLAGS_hda_codec.o := -I$(src) CFLAGS_hda_intel.o := -I$(src) diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c new file mode 100644 index 000..76c13d5 --- /dev/null +++ b/sound/pci/hda/hda_i915.c @@ -0,0 +1,75 @@ +/* + * hda_i915.c - routines for Haswell HDA controller power well support + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + */ + +#include linux/init.h +#include linux/module.h +#include sound/core.h +#include drm/i915_powerwell.h +#include hda_i915.h + +static void (*get_power)(void); +static void (*put_power)(void); + +void hda_display_power(bool enable) +{ + if (!get_power || !put_power) + return; + + snd_printdd(HDA display power %s \n, + enable ? Enable : Disable); + if (enable) + get_power(); + else + put_power(); +} + +int hda_i915_init(void) +{ + int err = 0; + + get_power = symbol_request(i915_request_power_well); + if (!get_power) { + snd_printk(KERN_WARNING hda-i915: get_power symbol get fail\n); + return -ENODEV; + } + + put_power = symbol_request(i915_release_power_well); + if (!put_power) { + symbol_put(i915_request_power_well); + get_power = NULL; + return -ENODEV; + } + + snd_printd(HDA driver get symbol successfully from i915 module\n); + + return err; +} + +int hda_i915_exit(void) +{ + if (get_power) { + symbol_put(i915_request_power_well); + get_power = NULL; + } + if (put_power) { + symbol_put(i915_release_power_well); + put_power = NULL; + } + + return 0; +} diff --git a/sound/pci/hda/hda_i915.h b/sound/pci/hda/hda_i915.h new file mode 100644 index 000..5a63da2 --- /dev/null +++ b/sound/pci/hda/hda_i915.h @@ -0,0 +1,35 @@ +/* + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or
Re: [Intel-gfx] [PATCH 1/6] drm/i915: add encoder get_config function v5
On Wed, May 15, 2013 at 06:45:35PM -0300, Paulo Zanoni wrote: 2013/5/14 Jesse Barnes jbar...@virtuousgeek.org: We can use this for fetching encoder specific pipe_config state, like mode flags, adjusted clock, etc. Just used for mode flags atm, so we can check the pipe config state at mode set time. v2: get_config when checking hw state too v3: fix DVO and LVDS mode flags (Ville) get SDVO DTD for flag fetch (Ville) v4: use input timings (Ville) correct command used (Ville) remove gen4 check (Ville) v5: get DDI flag config too Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch I applied this on my Haswell machine and booted it with both eDP-only and eDP+DP and now I don't see those error messages anymore. Queued for -next, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/intel_crt.c | 23 +++ drivers/gpu/drm/i915/intel_ddi.c | 23 +++ drivers/gpu/drm/i915/intel_display.c | 20 +--- drivers/gpu/drm/i915/intel_dp.c | 23 +++ drivers/gpu/drm/i915/intel_drv.h |4 drivers/gpu/drm/i915/intel_dvo.c | 21 + drivers/gpu/drm/i915/intel_hdmi.c| 23 +++ drivers/gpu/drm/i915/intel_lvds.c| 26 + drivers/gpu/drm/i915/intel_sdvo.c| 42 ++ 9 files changed, 202 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index cc414f1..789c4ef 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -81,6 +81,28 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder, return true; } +static void intel_crt_get_config(struct intel_encoder *encoder, +struct intel_crtc_config *pipe_config) +{ + struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; + struct intel_crt *crt = intel_encoder_to_crt(encoder); + u32 tmp, flags = 0; + + tmp = I915_READ(crt-adpa_reg); + + if (tmp ADPA_HSYNC_ACTIVE_HIGH) + flags |= DRM_MODE_FLAG_PHSYNC; + else + flags |= DRM_MODE_FLAG_NHSYNC; + + if (tmp ADPA_VSYNC_ACTIVE_HIGH) + flags |= DRM_MODE_FLAG_PVSYNC; + else + flags |= DRM_MODE_FLAG_NVSYNC; + + pipe_config-adjusted_mode.flags |= flags; +} + static void intel_disable_crt(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; @@ -784,6 +806,7 @@ void intel_crt_init(struct drm_device *dev) crt-base.compute_config = intel_crt_compute_config; crt-base.disable = intel_disable_crt; crt-base.enable = intel_enable_crt; + crt-base.get_config = intel_crt_get_config; if (I915_HAS_HOTPLUG(dev)) crt-base.hpd_pin = HPD_CRT; if (HAS_DDI(dev)) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index cddcf4a..27a74a9 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1254,6 +1254,28 @@ static void intel_ddi_hot_plug(struct intel_encoder *intel_encoder) intel_dp_check_link_status(intel_dp); } +static void intel_ddi_get_config(struct intel_encoder *encoder, +struct intel_crtc_config *pipe_config) +{ + struct drm_i915_private *dev_priv = encoder-base.dev-dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(encoder-base.crtc); + enum transcoder cpu_transcoder = intel_crtc-config.cpu_transcoder; + u32 temp, flags = 0; + + temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder)); + if (temp TRANS_DDI_PHSYNC) + flags |= DRM_MODE_FLAG_PHSYNC; + else + flags |= DRM_MODE_FLAG_NHSYNC; + if (temp TRANS_DDI_PVSYNC) + flags |= DRM_MODE_FLAG_PVSYNC; + else + flags |= DRM_MODE_FLAG_NVSYNC; + + pipe_config-adjusted_mode.flags |= flags; + pipe_config-pixel_multiplier = 1; +} + static void intel_ddi_destroy(struct drm_encoder *encoder) { /* HDMI has nothing special to destroy, so we can go with this. */ @@ -1313,6 +1335,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port) intel_encoder-disable = intel_disable_ddi; intel_encoder-post_disable = intel_ddi_post_disable; intel_encoder-get_hw_state = intel_ddi_get_hw_state; + intel_encoder-get_config = intel_ddi_get_config; intel_dig_port-port = port; intel_dig_port-port_reversal =
Re: [Intel-gfx] [RFC PATCH 1/3] drm/i915: group vlv iosf sideband register accessors to a new file
On Wed, May 15, 2013 at 09:40:30PM +0300, Jani Nikula wrote: No functional changes. Signed-off-by: Jani Nikula jani.nik...@intel.com I'd vote to move the sbi sideband stuff on the haswell pch to this place, too. Maybe in a follow-up patch though. -Daniel --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/intel_display.c | 37 -- drivers/gpu/drm/i915/intel_pm.c | 60 drivers/gpu/drm/i915/intel_sideband.c | 123 + 4 files changed, 124 insertions(+), 97 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_sideband.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 91f3ac6..40034ec 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -36,6 +36,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ intel_overlay.o \ intel_sprite.o \ intel_opregion.o \ + intel_sideband.o \ dvo_ch7xxx.o \ dvo_ch7017.o \ dvo_ivch.o \ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7358e4e..39af0e2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -381,43 +381,6 @@ static const intel_limit_t intel_limits_vlv_dp = { .find_pll = intel_vlv_find_best_pll, }; -u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg) -{ - WARN_ON(!mutex_is_locked(dev_priv-dpio_lock)); - - if (wait_for_atomic_us((I915_READ(DPIO_PKT) DPIO_BUSY) == 0, 100)) { - DRM_ERROR(DPIO idle wait timed out\n); - return 0; - } - - I915_WRITE(DPIO_REG, reg); - I915_WRITE(DPIO_PKT, DPIO_RID | DPIO_OP_READ | DPIO_PORTID | -DPIO_BYTE); - if (wait_for_atomic_us((I915_READ(DPIO_PKT) DPIO_BUSY) == 0, 100)) { - DRM_ERROR(DPIO read wait timed out\n); - return 0; - } - - return I915_READ(DPIO_DATA); -} - -void intel_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val) -{ - WARN_ON(!mutex_is_locked(dev_priv-dpio_lock)); - - if (wait_for_atomic_us((I915_READ(DPIO_PKT) DPIO_BUSY) == 0, 100)) { - DRM_ERROR(DPIO idle wait timed out\n); - return; - } - - I915_WRITE(DPIO_DATA, val); - I915_WRITE(DPIO_REG, reg); - I915_WRITE(DPIO_PKT, DPIO_RID | DPIO_OP_WRITE | DPIO_PORTID | -DPIO_BYTE); - if (wait_for_atomic_us((I915_READ(DPIO_PKT) DPIO_BUSY) == 0, 100)) - DRM_ERROR(DPIO write wait timed out\n); -} - static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc, int refclk) { diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1a76572..a06118d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4952,66 +4952,6 @@ int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u8 mbox, u32 val) return 0; } -static int vlv_punit_rw(struct drm_i915_private *dev_priv, u32 port, u8 opcode, - u8 addr, u32 *val) -{ - u32 cmd, devfn, be, bar; - - bar = 0; - be = 0xf; - devfn = PCI_DEVFN(2, 0); - - cmd = (devfn IOSF_DEVFN_SHIFT) | (opcode IOSF_OPCODE_SHIFT) | - (port IOSF_PORT_SHIFT) | (be IOSF_BYTE_ENABLES_SHIFT) | - (bar IOSF_BAR_SHIFT); - - WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); - - if (I915_READ(VLV_IOSF_DOORBELL_REQ) IOSF_SB_BUSY) { - DRM_DEBUG_DRIVER(warning: pcode (%s) mailbox access failed\n, - opcode == PUNIT_OPCODE_REG_READ ? - read : write); - return -EAGAIN; - } - - I915_WRITE(VLV_IOSF_ADDR, addr); - if (opcode == PUNIT_OPCODE_REG_WRITE) - I915_WRITE(VLV_IOSF_DATA, *val); - I915_WRITE(VLV_IOSF_DOORBELL_REQ, cmd); - - if (wait_for((I915_READ(VLV_IOSF_DOORBELL_REQ) IOSF_SB_BUSY) == 0, - 5)) { - DRM_ERROR(timeout waiting for pcode %s (%d) to finish\n, - opcode == PUNIT_OPCODE_REG_READ ? read : write, - addr); - return -ETIMEDOUT; - } - - if (opcode == PUNIT_OPCODE_REG_READ) - *val = I915_READ(VLV_IOSF_DATA); - I915_WRITE(VLV_IOSF_DATA, 0); - - return 0; -} - -int valleyview_punit_read(struct drm_i915_private *dev_priv, u8 addr, u32 *val) -{ - return vlv_punit_rw(dev_priv, IOSF_PORT_PUNIT, PUNIT_OPCODE_REG_READ, - addr, val); -} - -int valleyview_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val) -{ - return vlv_punit_rw(dev_priv, IOSF_PORT_PUNIT, PUNIT_OPCODE_REG_WRITE, - addr, val); -} - -int valleyview_nc_read(struct
Re: [Intel-gfx] [RFC PATCH 0/3] vlv sideband refactoring
On Wed, May 15, 2013 at 09:40:29PM +0300, Jani Nikula wrote: Okay, I'm stuck with this a bit, and whichever approach I choose I expect there to be a bunch of bikeshedding. So I won't polish this further before comments. Both the intel_dpio_{read,write} and valleyview_{punit,nc}_{read,write} use the IOSF sideband interface. They access the same registers and do mostly the same stuff, but no shared code. There are even duplicate register defines for the same registers. Both have locking, but the former use dpio_lock and the latter rps.hw_lock. It's racy. These patches refactor the sideband access to a single function that expects dpio_lock to be held. The dpio_lock is only used for sideband stuff, so it's a better match than rps.hw_lock for the purpose. The rps stuff still needs rps.hw_lock, since it's used to protect more than just the register access, so rps code will need to hold both locks. The bikeshedding department: 1) Do we need to propagate sideband errors from the functions (like the valleyview_punit_* functions do), or, since the return values are never checked anywhere anyway, can we just convert to the intel_dpio_* style (functions return the register value only) for all of them? I vote for void return and obnoxious WARNs (WARNs are optional if you want). There's usually not much we can do if the hw has gone south like that. 2) There will be quite a few more ports. Add new wrappers for each of them, or create generic read/write functions that need a port argument? Imo the wrappers aren't too bad right now still, i.e. we can re-bikeshed this once it happens. 3) Should the rps stuff take dpio_lock at a higher level than the wrappers? This is pretty much a requirement for the generic r/w function. I'm more unhappy about the inconsistency in locking, since the dpio lock is now held around large swaths of crtc disable/enable code, but only very small parts for the rps stuff. I expect this to eventually bite us if dpio sideband usage keeps growing. But again something we can fix once it blows up. 4) Does dpio really need a devfn different from the rest? I have no idea about this. Definitely looks ... interesting though ;-) I think Jesse should take a look here. 5) Your additional bikeshedding here. ;) Dropped one to also move the sbi stuff around. -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/3] drm/i915: implement IPS feature
On Thu, May 16, 2013 at 04:54:04PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Intermediate Pixel Storage is a feature that should reduce the number of times the display engine wakes up memory to read pixels, so it should allow deeper PC states. IPS can only be enabled on ULT pipe A with 8:8:8 pipe pixel formats. With eDP 1920x1080 and correct watermarks but without FBC this moves my PC7 residency from 2.5% to around 38%. v2: - It's tied to pipe A, not port A - Add pipe_config support (Chris) - Add some assertions (Chris) - Rebase against latest dinq Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Three small comments below. --- drivers/gpu/drm/i915/i915_reg.h | 11 + drivers/gpu/drm/i915/intel_display.c | 79 +++- drivers/gpu/drm/i915/intel_drv.h | 2 + 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 32eb97c..65339a7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -977,6 +977,8 @@ /* Framebuffer compression for Ivybridge */ #define IVB_FBC_RT_BASE 0x7020 +#define IPS_CTL 0x43408 +#define IPS_ENABLE (1 31) #define _HSW_PIPE_SLICE_CHICKEN_1_A 0x420B0 #define _HSW_PIPE_SLICE_CHICKEN_1_B 0x420B4 @@ -3621,6 +3623,15 @@ #define _LGC_PALETTE_B 0x4a800 #define LGC_PALETTE(pipe) _PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) +#define _GAMMA_MODE_A0x4a480 +#define _GAMMA_MODE_B0x4ac80 +#define GAMMA_MODE(pipe) _PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B) +#define GAMMA_MODE_MODE_MASK (3 0) +#define GAMMA_MODE_MODE_8bit (0 0) +#define GAMMA_MODE_MODE_10bit(1 0) +#define GAMMA_MODE_MODE_12bit(2 0) +#define GAMMA_MODE_MODE_SPLIT(3 0) + /* interrupts */ #define DE_MASTER_IRQ_CONTROL (1 31) #define DE_SPRITEB_FLIP_DONE(1 29) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d588ff6..5b41cf3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3340,6 +3340,53 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_wait_for_vblank(dev, intel_crtc-pipe); } +/* IPS only exists on ULT machines and is tied to pipe A. */ +static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) +{ + return (IS_ULT(crtc-base.dev) crtc-pipe == PIPE_A); +} + +static void hsw_enable_ips(struct intel_crtc *crtc) +{ + struct drm_i915_private *dev_priv = crtc-base.dev-dev_private; + + if (!hsw_crtc_supports_ips(crtc)) + return; + + if (crtc-config.pipe_bpp != 24) + return; + + DRM_DEBUG_KMS(Enabling IPS\n); + + crtc-config.ips_enabled = true; + + /* We can only enable IPS after we enable a plane and wait for a vblank. + * We guarantee that the plane is enabled by calling intel_enable_ips + * only after intel_enable_plane. And intel_enable_plane already waits + * for a vblank, so all we need to do here is to enable the IPS bit. */ + assert_plane_enabled(dev_priv, crtc-plane); + I915_WRITE(IPS_CTL, IPS_ENABLE); +} + +static void hsw_disable_ips(struct intel_crtc *crtc) +{ + struct drm_device *dev = crtc-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + + if (!hsw_crtc_supports_ips(crtc)) + return; Like Chris suggested I think it's better to use the pipe_config state here to decide whether to disable ips or not. Otherwise we could leak out an enabled IPS config if the BIOS sets it up, but the module param is not set. + + DRM_DEBUG_KMS(Disabling IPS\n); + + crtc-config.ips_enabled = false; + + assert_plane_enabled(dev_priv, crtc-plane); + I915_WRITE(IPS_CTL, 0); + + /* We need to wait for a vblank before we can disable the plane. */ + intel_wait_for_vblank(dev, crtc-pipe); +} + static void haswell_crtc_enable(struct drm_crtc *crtc) { struct drm_device *dev = crtc-dev; @@ -3387,6 +3434,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_crtc-config.has_pch_encoder); intel_enable_plane(dev_priv, plane, pipe); + hsw_enable_ips(intel_crtc); + if (intel_crtc-config.has_pch_encoder) lpt_pch_enable(crtc); @@ -3529,6 +3578,8 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) if (dev_priv-cfb_plane == plane) intel_disable_fbc(dev); + hsw_disable_ips(intel_crtc); + intel_disable_plane(dev_priv, plane, pipe); if (intel_crtc-config.has_pch_encoder) @@ -5021,6 +5072,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc, i9xx_get_pfit_config(crtc, pipe_config); + pipe_config-ips_enabled =
Re: [Intel-gfx] [PATCH] drm/i915: Propagate errors back from fb set-base
On Fri, May 03, 2013 at 06:55:35PM +0200, Daniel Vetter wrote: On Fri, May 3, 2013 at 6:36 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: Along the modesetting short cut where we skip trying to do a full modeset and instead simply update the framebuffer base registers, we failed to handle any errors reported. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk This regression has been introduced in commit 94352cf9a5328bb1a44288e6c2c1276695f8a356 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Thu Jul 5 22:51:56 2012 +0200 drm/i915: push crtc-fb update into pipe_set_base /me hangs head in shame Ping? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: merge VLV eDP and DP AUX clock divider calculation
On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we can calculate for both the clock divider for the 2MHz target rate at the same place. Afterwards we can also replace the is_cpu_edp() check with a check for port A. Signed-off-by: Imre Deak imre.d...@intel.com There's a now-dead IS_VLV case in intel_hrawclk which should be killed with this patch, too. -Daniel --- drivers/gpu/drm/i915/intel_dp.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 90ae58a..b99da13 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -317,11 +317,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, * Note that PCH attached eDP panels should use a 125MHz input * clock divider. */ - if (is_cpu_edp(intel_dp)) { + if (IS_VALLEYVIEW(dev)) { + aux_clock_divider = 100; + } else if (intel_dig_port-port == PORT_A) { if (HAS_DDI(dev)) aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) 1; - else if (IS_VALLEYVIEW(dev)) - aux_clock_divider = 100; else if (IS_GEN6(dev) || IS_GEN7(dev)) aux_clock_divider = 200; /* SNB IVB eDP input clock at 400Mhz */ else -- 1.7.10.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
Re: [Intel-gfx] [PATCH 1/4] drm/i915: stop using is_cpu_edp() in intel_disable/post_disable_dp
On Thu, May 16, 2013 at 02:40:34PM +0300, Imre Deak wrote: On port A and for Valleyview on port C we can have only eDP and in both cases it's a CPU port. So we can replace is_cpu_edp() with a port check for these two cases. This allows us to remove is_cpu_edp() completely in a later patch. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_dp.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4dae01a..90ae58a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1350,6 +1350,8 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder, static void intel_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); + enum port port = dp_to_dig_port(intel_dp)-port; + struct drm_device *dev = encoder-base.dev; /* Make sure the panel is off before trying to change the mode. But also * ensure that we have vdd while we switch off the panel. */ @@ -1359,16 +1361,17 @@ static void intel_disable_dp(struct intel_encoder *encoder) ironlake_edp_panel_off(intel_dp); /* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */ - if (!is_cpu_edp(intel_dp)) + if (port != PORT_A (port != PORT_C || !IS_VALLEYVIEW(dev))) I think this would read easier as port !(port == A || IS_VLV). I think that should also be more correct since there's no reason (besides hard-coding) that DP on port B (or external DP fwiw) should work different on vlv. -Daniel intel_dp_link_down(intel_dp); } static void intel_post_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); + enum port port = dp_to_dig_port(intel_dp)-port; struct drm_device *dev = encoder-base.dev; - if (is_cpu_edp(intel_dp)) { + if (port == PORT_A || (port == PORT_C IS_VALLEYVIEW(dev))) { intel_dp_link_down(intel_dp); if (!IS_VALLEYVIEW(dev)) ironlake_edp_pll_off(intel_dp); -- 1.7.10.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
Re: [Intel-gfx] [PATCH 3/9] drm/i915: use the mode-htotal to calculate linetime watermarks
On Fri, May 03, 2013 at 05:23:39PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com ... instead of mode-crtc_display. The spec says pipe horizontal total number of pixels and the Haswell Watermark Calculator tool uses the Pipe H Total instead of Pipe H Src as the value. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d056bc9..4cc5f99 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) * row at the given clock rate, multiplied by 8. * */ temp |= PIPE_WM_LINETIME_TIME( - ((mode-crtc_hdisplay * 1000) / mode-clock) * 8); + ((mode-htotal * 1000) / mode-clock) * 8); Big question: What's the right value for interlaced modes? On progressive mode crtc_ mode values match the non-crtc_ prefixed ones, but not so much for interlaced modes ... So if your tool expects something resembling what we program into the pipe registers, we need to change this again. Merged for now since the spec explicitly says pixels, but I'd like someone to double-check this. -Daniel /* IPS watermarks are only used by pipe A, and are ignored by * pipes B and C. They are calculated similarly to the common -- 1.7.10.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
Re: [Intel-gfx] [PATCH 5/9] drm/i915: use the correct clock when calculating linetime watermarks
On Fri, May 03, 2013 at 05:23:41PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com If we're using DP/eDP, adjusted_mode-clock may be just the port link clock, but we also can't use mode-clock because it's wrong when we're using the using panel fitter. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8468b40..3ca020c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2021,6 +2021,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc-pipe; struct drm_display_mode *mode = intel_crtc-config.adjusted_mode; + int target_clock; u32 temp; if (!intel_crtc_active(crtc)) { @@ -2028,6 +2029,11 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) return; } + if (intel_crtc-config.pixel_target_clock) + target_clock = intel_crtc-config.pixel_target_clock; + else + target_clock = intel_crtc-config.adjusted_mode.clock; I'll ignore this one here, since I already have the real fix at http://cgit.freedesktop.org/~danvet/drm/commit/?h=fdi-ditherid=0600051ee5eb6d13d385c8629c0ab0f7809346be I'll submit the series this patch is a part of asap to intel-gfx. Cheers, Daniel + temp = I915_READ(PIPE_WM_LINETIME(pipe)); temp = ~PIPE_WM_LINETIME_MASK; @@ -2035,7 +2041,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) * row at the given clock rate, multiplied by 8. * */ temp |= PIPE_WM_LINETIME_TIME( - DIV_ROUND_CLOSEST(mode-htotal * 1000 * 8, mode-clock)); + DIV_ROUND_CLOSEST(mode-htotal * 1000 * 8, target_clock)); /* IPS watermarks are only used by pipe A, and are ignored by * pipes B and C. They are calculated similarly to the common -- 1.7.10.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
Re: [Intel-gfx] [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround
On Fri, May 03, 2013 at 05:23:45PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround inside haswell_init_clock_gating and mentioned it is a workaround for early silicon revisions and should be removed later. This workaround is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the newer machines. So use the new one. Also notice that there's still another workaround for PRI_CTL that involves WM_DBG, but it's not the one we're reverting. And notice that we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables the LP watermarks when more than one pipe is used, and we really don't want this because we need the LP watermarks if we want to reach deeper PC states. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com I've applied all the patches in this series safe for the target_clock one. But this patch here is missing the w/a tag, can you please supply that one in a quick reply so that I can smash it into the patch? Thanks, Daniel --- drivers/gpu/drm/i915/i915_reg.h |3 +++ drivers/gpu/drm/i915/intel_pm.c | 10 ++ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index aec569f..5879f23 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3697,6 +3697,9 @@ # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE (1 5) # define CHICKEN3_DGMG_DONE_FIX_DISABLE (1 2) +#define CHICKEN_PAR1_1 0x42080 +#define FORCE_ARB_IDLE_PLANES (1 14) + #define DISP_ARB_CTL 0x45000 #define DISP_TILE_SURFACE_SWIZZLING (113) #define DISP_FBC_WM_DIS (115) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b56de92..2297476 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device *dev) /* WaSwitchSolVfFArbitrationPriority */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); - /* XXX: This is a workaround for early silicon revisions and should be - * removed later. - */ - I915_WRITE(WM_DBG, - I915_READ(WM_DBG) | - WM_DBG_DISALLOW_MULTIPLE_LP | - WM_DBG_DISALLOW_SPRITE | - WM_DBG_DISALLOW_MAXFIFO); + I915_WRITE(CHICKEN_PAR1_1, +I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES); lpt_init_clock_gating(dev); } -- 1.7.10.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] [ANNOUNCE] xf86-video-intel 2.21.7
Release 2.21.7 (2013-05-21) === A couple of weeks turned into a month and a couple of weeks... Amidst the usual bug fixes, we have added the complete set of Haswell PCI IDs - hopefully future proofing ourselves against being surprised by new products. We can also now use the correct term for the top of the range Haswell variants, GT3. * Fix several assertion failures hit by Jiri Slaby. * Allow XvMC to also target overlay/sprite planes. * Throw in a paranoid MI_FLUSH between BLT and RENDER operations on Ironlake. https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/1168066 * Prevent reuse of old framebuffers after a resize. https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/1157678 * Fix compilation with --enable-valgrind and no --enable-debug * Improve partial migration of render sources. * Fix origin of trapezoids. https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/1178020 * Introduce copy-on-write support for cloning pixmaps. The ultimate goal here is to efficiently support the TearFree mode of operation, but this provides immediate benefits with firefox - most importantly because of the inefficient way it now implements scrolling. Complete list of changes since 2.21.6 - Chris Wilson (47): sna: Remove assertions for mapped GPU bo if priv-cpu after GPU bo creation sna/video: Expand passthrough support for overlay planes sna/video: Textured video passthrough no longer relies upon XvMC sna: Document fence limits for gen4+ sna: Add a DBG option for testing userptr more thoroughly sna: Suppress hotplug events whilst VT switched away sna/xvmc: Wrap each output adaptor sna: Align uploads to start on page boundaries sna/gen7: Cache our kernels in L3 sna: Refine assertion about the existence of CPU damage when GPU damaged sna/gen5: Force a MI_FLUSH between using the BLT and RENDER engines sna: Flush the scanout cache after resizing the display sna: Add missing ';' sna/xvmc: silence a compiler warning sna: Only release the scanout cache whilst DRM_MASTER sna: Add VALGRIND_CFLAGS whilst compiling with --enable-valgrind xgvevent Prefer i830_dri.so for gen2 chipsets Revert xgvevent sna: Rephrase initialisation without a specific backend sna: Prevent accessing an uninitialised region in move_area_to_gpu() uxa/dri: Fix compile error for unknown 'bool' Add all reserved PCI-IDs for Haswell sna: Page align requests to userptr sna: Be careful not to preemptively upload portions of a SHM pixmap sna: Do not attempt to clean an active scanout sna: Handle cached upload buffers for partial migration to GPU sna: Add DBG statements for choice of spans vertex emitter sna/gen7: Add DBG for channel setup for render source sna: Add more debugging to unaligned trapezoids sna/trapezoids: Fix the determination of the trapezoid origin sna/gen4: Drop unused gen parameter to SF state setup sna/gen4: Tidy testing for an active vertex buffer id sna: Attempt to discard overwritten operations before CopyArea sna: Propagate clear color when replacing by a CopyArea sna: Propagate clears when using the BLT composite routines sna: Basic copy-on-write support for cloning pixmaps sna: Assert that the mapping is released before closing the GEM handle sna: Correct assertions to allow discarding of cpu hint for inplace ops sna: Clear mapped hints upon cloning a pair of pixmaps sna: Avoid replacing pinned bo when undoing a clone sna: Transfer ownership of the cloned bo to the pixmaps sna: Undo the clone when replacing the DRI pixmap sna: Add the missing ref(bo) when undoing the source clone sna: Clear the cow_list when discarding the clone upon pixmap destroy sna: Undo a few more overwritten operations upon a bo 2.21.7 release Rodrigo Vivi (2): Fix Haswell GT3 names. Adding more reserved PCI IDs for Haswell. git tag: 2.21.7 http://xorg.freedesktop.org/archive/individual/driver/xf86-video-intel-2.21.7.tar.bz2 MD5: c3a8b542fc4787ad17a5f0567a3429fd xf86-video-intel-2.21.7.tar.bz2 SHA1: 40dfeddc4828ad24d272cc69112202c71855dcdf xf86-video-intel-2.21.7.tar.bz2 SHA256: faeabba40079c49290f39992542d9b0cfd229bda4e389e1352926903038363d9 xf86-video-intel-2.21.7.tar.bz2 http://xorg.freedesktop.org/archive/individual/driver/xf86-video-intel-2.21.7.tar.gz MD5: 437101f023fe23fdd9e65881410ffaaf xf86-video-intel-2.21.7.tar.gz SHA1: 6e105fd5b66fbdd2ca9cd576a0f5a16d541a59c2 xf86-video-intel-2.21.7.tar.gz SHA256: 777d3e0a4c88b065f0b033b405fc67e7f5002fa374eddeedd96f0454d569ed56 xf86-video-intel-2.21.7.tar.gz -- Chris Wilson, Intel Open Source Technology Centre signature.asc Description: Digital signature
Re: [Intel-gfx] [PATCH] drm/i915: Propagate errors back from fb set-base
On Tue, May 21, 2013 at 09:54:09AM +0100, Chris Wilson wrote: On Fri, May 03, 2013 at 06:55:35PM +0200, Daniel Vetter wrote: On Fri, May 3, 2013 at 6:36 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: Along the modesetting short cut where we skip trying to do a full modeset and instead simply update the framebuffer base registers, we failed to handle any errors reported. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk This regression has been introduced in commit 94352cf9a5328bb1a44288e6c2c1276695f8a356 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Thu Jul 5 22:51:56 2012 +0200 drm/i915: push crtc-fb update into pipe_set_base /me hangs head in shame Ping? Picked up for -fixes, 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 1/4] drm/i915: stop using is_cpu_edp() in intel_disable/post_disable_dp
On Tue, 2013-05-21 at 11:15 +0200, Daniel Vetter wrote: On Thu, May 16, 2013 at 02:40:34PM +0300, Imre Deak wrote: On port A and for Valleyview on port C we can have only eDP and in both cases it's a CPU port. So we can replace is_cpu_edp() with a port check for these two cases. This allows us to remove is_cpu_edp() completely in a later patch. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_dp.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4dae01a..90ae58a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1350,6 +1350,8 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder, static void intel_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); + enum port port = dp_to_dig_port(intel_dp)-port; + struct drm_device *dev = encoder-base.dev; /* Make sure the panel is off before trying to change the mode. But also * ensure that we have vdd while we switch off the panel. */ @@ -1359,16 +1361,17 @@ static void intel_disable_dp(struct intel_encoder *encoder) ironlake_edp_panel_off(intel_dp); /* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */ - if (!is_cpu_edp(intel_dp)) + if (port != PORT_A (port != PORT_C || !IS_VALLEYVIEW(dev))) I think this would read easier as port !(port == A || IS_VLV). I think that should also be more correct since there's no reason (besides hard-coding) that DP on port B (or external DP fwiw) should work different on vlv. I don't have the VLV spec to verify this, so this is just keeping the current behavior. I'll try to get the spec and follow up on this. --Imre -Daniel intel_dp_link_down(intel_dp); } static void intel_post_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); + enum port port = dp_to_dig_port(intel_dp)-port; struct drm_device *dev = encoder-base.dev; - if (is_cpu_edp(intel_dp)) { + if (port == PORT_A || (port == PORT_C IS_VALLEYVIEW(dev))) { intel_dp_link_down(intel_dp); if (!IS_VALLEYVIEW(dev)) ironlake_edp_pll_off(intel_dp); -- 1.7.10.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 2/4] drm/i915: merge VLV eDP and DP AUX clock divider calculation
On Tue, 2013-05-21 at 11:12 +0200, Daniel Vetter wrote: On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we can calculate for both the clock divider for the 2MHz target rate at the same place. Afterwards we can also replace the is_cpu_edp() check with a check for port A. Signed-off-by: Imre Deak imre.d...@intel.com There's a now-dead IS_VLV case in intel_hrawclk which should be killed with this patch, too. Shouldn't it still return the correct value if someone calls it in the future? Actually it's also used in intel_dp_init_panel_power_sequencer_registers(). --Imre -Daniel --- drivers/gpu/drm/i915/intel_dp.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 90ae58a..b99da13 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -317,11 +317,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, * Note that PCH attached eDP panels should use a 125MHz input * clock divider. */ - if (is_cpu_edp(intel_dp)) { + if (IS_VALLEYVIEW(dev)) { + aux_clock_divider = 100; + } else if (intel_dig_port-port == PORT_A) { if (HAS_DDI(dev)) aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) 1; - else if (IS_VALLEYVIEW(dev)) - aux_clock_divider = 100; else if (IS_GEN6(dev) || IS_GEN7(dev)) aux_clock_divider = 200; /* SNB IVB eDP input clock at 400Mhz */ else -- 1.7.10.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 2/4] drm/i915: merge VLV eDP and DP AUX clock divider calculation
On Tue, May 21, 2013 at 12:36 PM, Imre Deak imre.d...@intel.com wrote: On Tue, 2013-05-21 at 11:12 +0200, Daniel Vetter wrote: On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we can calculate for both the clock divider for the 2MHz target rate at the same place. Afterwards we can also replace the is_cpu_edp() check with a check for port A. Signed-off-by: Imre Deak imre.d...@intel.com There's a now-dead IS_VLV case in intel_hrawclk which should be killed with this patch, too. Shouldn't it still return the correct value if someone calls it in the future? Actually it's also used in intel_dp_init_panel_power_sequencer_registers(). Indeed. I think though it's better to make this an explicit vlv case since hrawclk talks about the FSB. And that thing pretty surely doesn't exist on vlv any more ;-) So maybe a follow-up 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 2/2 V4] ALSA: hda - Add power-welll support for haswell HDA
-Original Message- From: Takashi Iwai [mailto:ti...@suse.de] Sent: Tuesday, May 21, 2013 3:19 PM To: Wang Xingchao Cc: dan...@ffwll.ch; Girdwood, Liam R; david.hennings...@canonical.com; Lin, Mengdong; Li, Jocelyn; alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org; Barnes, Jesse; Zanoni, Paulo R; Wang, Xingchao Subject: Re: [PATCH 2/2 V4] ALSA: hda - Add power-welll support for haswell HDA At Mon, 20 May 2013 19:26:58 +0800, Wang Xingchao wrote: For Intel Haswell chip, HDA controller and codec have power well dependency from GPU side. This patch added support to request/release power well in audio driver. Power save feature should be enabled to get runtime power saving. Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com --- sound/pci/hda/Kconfig | 10 ++ sound/pci/hda/Makefile|3 ++ sound/pci/hda/hda_i915.c | 75 + sound/pci/hda/hda_i915.h | 35 + sound/pci/hda/hda_intel.c | 41 +++-- 5 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 sound/pci/hda/hda_i915.c create mode 100644 sound/pci/hda/hda_i915.h diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 80a7d44..c5a872c 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -152,6 +152,16 @@ config SND_HDA_CODEC_HDMI snd-hda-codec-hdmi. This module is automatically loaded at probing. +config SND_HDA_I915 + bool Build Display HD-audio controller/codec power well support for i915 cards + depends on DRM_I915 + help + Say Y here to include full HDMI and DisplayPort HD-audio controller/codec + power-well support for Intel Haswell graphics cards based on the i915 driver. + + Note that this option must be enabled for Intel Haswell C+ stepping machines, otherwise + the GPU audio controller/codecs will not be initialized or damaged when exit from S3 mode. + config SND_HDA_CODEC_CIRRUS bool Build Cirrus Logic codec support default y diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index 24a2514..4b0a4bc 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -6,6 +6,9 @@ snd-hda-codec-$(CONFIG_PROC_FS) += hda_proc.o snd-hda-codec-$(CONFIG_SND_HDA_HWDEP) += hda_hwdep.o snd-hda-codec-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o +# for haswell power well +snd-hda-intel-$(CONFIG_SND_HDA_I915) +=hda_i915.o + # for trace-points CFLAGS_hda_codec.o := -I$(src) CFLAGS_hda_intel.o := -I$(src) diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c new file mode 100644 index 000..76c13d5 --- /dev/null +++ b/sound/pci/hda/hda_i915.c @@ -0,0 +1,75 @@ +/* + * hda_i915.c - routines for Haswell HDA controller power well +support + * + * This program is free software; you can redistribute it and/or +modify it + * under the terms of the GNU General Public License as published by +the Free + * Software Foundation; either version 2 of the License, or (at your +option) + * any later version. + * + * This program is distributed in the hope that it will be useful, +but + * WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public +License + * for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software +Foundation, + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + */ + +#include linux/init.h +#include linux/module.h +#include sound/core.h +#include drm/i915_powerwell.h +#include hda_i915.h + +static void (*get_power)(void); +static void (*put_power)(void); + +void hda_display_power(bool enable) +{ + if (!get_power || !put_power) + return; + + snd_printdd(HDA display power %s \n, + enable ? Enable : Disable); + if (enable) + get_power(); + else + put_power(); +} + +int hda_i915_init(void) +{ + int err = 0; + + get_power = symbol_request(i915_request_power_well); + if (!get_power) { + snd_printk(KERN_WARNING hda-i915: get_power symbol get fail\n); + return -ENODEV; + } + + put_power = symbol_request(i915_release_power_well); + if (!put_power) { + symbol_put(i915_request_power_well); + get_power = NULL; + return -ENODEV; + } + + snd_printd(HDA driver get symbol successfully from i915 module\n); + + return err; +} + +int hda_i915_exit(void) +{ + if (get_power) { + symbol_put(i915_request_power_well); + get_power = NULL; + } + if (put_power) { +
Re: [Intel-gfx] [PATCH 2/4] drm/i915: merge VLV eDP and DP AUX clock divider calculation
On Tue, 2013-05-21 at 12:42 +0200, Daniel Vetter wrote: On Tue, May 21, 2013 at 12:36 PM, Imre Deak imre.d...@intel.com wrote: On Tue, 2013-05-21 at 11:12 +0200, Daniel Vetter wrote: On Thu, May 16, 2013 at 02:40:35PM +0300, Imre Deak wrote: On ValleyView for both eDP and DP the AUX input clock is 200MHz, so we can calculate for both the clock divider for the 2MHz target rate at the same place. Afterwards we can also replace the is_cpu_edp() check with a check for port A. Signed-off-by: Imre Deak imre.d...@intel.com There's a now-dead IS_VLV case in intel_hrawclk which should be killed with this patch, too. Shouldn't it still return the correct value if someone calls it in the future? Actually it's also used in intel_dp_init_panel_power_sequencer_registers(). Indeed. I think though it's better to make this an explicit vlv case since hrawclk talks about the FSB. And that thing pretty surely doesn't exist on vlv any more ;-) Ok. Tbh, I haven't thought about the differences in clock topology across the platforms, but would be nice to understand it better. So maybe a follow-up patch? Yep, I think it could stand on its own. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2 V4] i915/drm: Add private api for power well usage
-Original Message- From: Takashi Iwai [mailto:ti...@suse.de] Sent: Tuesday, May 21, 2013 3:18 PM To: Wang Xingchao Cc: dan...@ffwll.ch; Girdwood, Liam R; david.hennings...@canonical.com; Lin, Mengdong; Li, Jocelyn; alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org; Barnes, Jesse; Zanoni, Paulo R; Wang, Xingchao Subject: Re: [PATCH 1/2 V4] i915/drm: Add private api for power well usage At Mon, 20 May 2013 19:26:57 +0800, Wang Xingchao wrote: Haswell Display audio depends on power well in graphic side, it should request power well before use it and release power well after use. I915 will not shutdown power well if it detects audio is using. This patch protects display audio crash for Intel Haswell C3 stepping board. Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com --- drivers/gpu/drm/i915/i915_dma.c |6 +++ drivers/gpu/drm/i915/i915_drv.h | 12 + drivers/gpu/drm/i915/intel_drv.h |4 ++ drivers/gpu/drm/i915/intel_pm.c | 98 +++--- include/drm/i915_powerwell.h | 36 ++ 5 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 include/drm/i915_powerwell.h diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a1648eb..2b9010a 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1652,6 +1652,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Start out suspended */ dev_priv-mm.suspended = 1; + if (IS_HASWELL(dev)) + i915_init_power_well(dev); + if (drm_core_check_feature(dev, DRIVER_MODESET)) { ret = i915_load_modeset_init(dev); if (ret 0) { @@ -1708,6 +1711,9 @@ int i915_driver_unload(struct drm_device *dev) intel_gpu_ips_teardown(); + if (IS_HASWELL(dev)) + i915_remove_power_well(dev); + i915_teardown_sysfs(dev); if (dev_priv-mm.inactive_shrinker.shrink) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3ac71db..a0f1a6d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -702,6 +702,15 @@ struct intel_ilk_power_mgmt { struct drm_i915_gem_object *renderctx; }; +/* Power well structure for haswell */ struct i915_power_well { + struct drm_device *device; + spinlock_t lock; + /* power well enable/disable usage count */ + int count; + int i915_request; +}; + struct i915_dri1_state { unsigned allow_batchbuffer : 1; u32 __iomem *gfx_hws_cpu_addr; @@ -1048,6 +1057,9 @@ typedef struct drm_i915_private { * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips; + /* Haswell power well */ + struct i915_power_well *hsw_pwr; + enum no_fbc_reason no_fbc_reason; struct drm_mm_node *compressed_fb; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dfcf546..efcccab 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -759,6 +759,10 @@ extern void intel_update_fbc(struct drm_device *dev); extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv); extern void intel_gpu_ips_teardown(void); +/* Power well */ +extern int i915_init_power_well(struct drm_device *dev); extern void +i915_remove_power_well(struct drm_device *dev); + extern bool intel_using_power_well(struct drm_device *dev); extern void intel_init_power_well(struct drm_device *dev); extern void intel_set_power_well(struct drm_device *dev, bool enable); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0f4b46e..eec7aa8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4344,18 +4344,12 @@ bool intel_using_power_well(struct drm_device *dev) return true; } -void intel_set_power_well(struct drm_device *dev, bool enable) +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; uint32_t tmp; - if (!HAS_POWER_WELL(dev)) - return; - - if (!i915_disable_power_well !enable) - return; - tmp = I915_READ(HSW_PWR_WELL_DRIVER); is_enabled = tmp HSW_PWR_WELL_STATE; enable_requested = tmp HSW_PWR_WELL_ENABLE; @@ -4378,6 +4372,96 @@ void intel_set_power_well(struct drm_device *dev, bool enable) } } +static struct i915_power_well *hsw_pwr; + +/* Display audio driver power well request */ void +i915_request_power_well(void) { + if (hsw_pwr == NULL) + return; + + spin_lock_irq(hsw_pwr-lock); + if (!hsw_pwr-count++ + !hsw_pwr-i915_request) +
Re: [Intel-gfx] [PATCH 2/2 V4] ALSA: hda - Add power-welll support for haswell HDA
At Tue, 21 May 2013 10:58:36 +, Wang, Xingchao wrote: -Original Message- From: Takashi Iwai [mailto:ti...@suse.de] Sent: Tuesday, May 21, 2013 3:19 PM To: Wang Xingchao Cc: dan...@ffwll.ch; Girdwood, Liam R; david.hennings...@canonical.com; Lin, Mengdong; Li, Jocelyn; alsa-de...@alsa-project.org; intel-gfx@lists.freedesktop.org; Barnes, Jesse; Zanoni, Paulo R; Wang, Xingchao Subject: Re: [PATCH 2/2 V4] ALSA: hda - Add power-welll support for haswell HDA At Mon, 20 May 2013 19:26:58 +0800, Wang Xingchao wrote: For Intel Haswell chip, HDA controller and codec have power well dependency from GPU side. This patch added support to request/release power well in audio driver. Power save feature should be enabled to get runtime power saving. Signed-off-by: Wang Xingchao xingchao.w...@linux.intel.com --- sound/pci/hda/Kconfig | 10 ++ sound/pci/hda/Makefile|3 ++ sound/pci/hda/hda_i915.c | 75 + sound/pci/hda/hda_i915.h | 35 + sound/pci/hda/hda_intel.c | 41 +++-- 5 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 sound/pci/hda/hda_i915.c create mode 100644 sound/pci/hda/hda_i915.h diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 80a7d44..c5a872c 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -152,6 +152,16 @@ config SND_HDA_CODEC_HDMI snd-hda-codec-hdmi. This module is automatically loaded at probing. +config SND_HDA_I915 + bool Build Display HD-audio controller/codec power well support for i915 cards + depends on DRM_I915 + help + Say Y here to include full HDMI and DisplayPort HD-audio controller/codec + power-well support for Intel Haswell graphics cards based on the i915 driver. + + Note that this option must be enabled for Intel Haswell C+ stepping machines, otherwise + the GPU audio controller/codecs will not be initialized or damaged when exit from S3 mode. + config SND_HDA_CODEC_CIRRUS bool Build Cirrus Logic codec support default y diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index 24a2514..4b0a4bc 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -6,6 +6,9 @@ snd-hda-codec-$(CONFIG_PROC_FS) += hda_proc.o snd-hda-codec-$(CONFIG_SND_HDA_HWDEP) += hda_hwdep.o snd-hda-codec-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o +# for haswell power well +snd-hda-intel-$(CONFIG_SND_HDA_I915) += hda_i915.o + # for trace-points CFLAGS_hda_codec.o := -I$(src) CFLAGS_hda_intel.o := -I$(src) diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c new file mode 100644 index 000..76c13d5 --- /dev/null +++ b/sound/pci/hda/hda_i915.c @@ -0,0 +1,75 @@ +/* + * hda_i915.c - routines for Haswell HDA controller power well +support + * + * This program is free software; you can redistribute it and/or +modify it + * under the terms of the GNU General Public License as published by +the Free + * Software Foundation; either version 2 of the License, or (at your +option) + * any later version. + * + * This program is distributed in the hope that it will be useful, +but + * WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public +License + * for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software +Foundation, + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + */ + +#include linux/init.h +#include linux/module.h +#include sound/core.h +#include drm/i915_powerwell.h +#include hda_i915.h + +static void (*get_power)(void); +static void (*put_power)(void); + +void hda_display_power(bool enable) +{ + if (!get_power || !put_power) + return; + + snd_printdd(HDA display power %s \n, + enable ? Enable : Disable); + if (enable) + get_power(); + else + put_power(); +} + +int hda_i915_init(void) +{ + int err = 0; + + get_power = symbol_request(i915_request_power_well); + if (!get_power) { + snd_printk(KERN_WARNING hda-i915: get_power symbol get fail\n); + return -ENODEV; + } + + put_power = symbol_request(i915_release_power_well); + if (!put_power) { + symbol_put(i915_request_power_well); + get_power = NULL; + return -ENODEV; + } + + snd_printd(HDA driver get symbol successfully from i915 module\n); + + return err; +} + +int hda_i915_exit(void) +{ + if (get_power) { +
[Intel-gfx] [PATCH 0/3] drm/i915: Trickle feed bits
This series tries to get the trickle feed settings corrected for all gen4+. Note that I've only compile tested the gen4 and vlv bits as I don't have either type of machine set up at the moment. The g4x patch (well, v1 actually) was smoke tested on real hardware at some point. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/3] drm/i915: Disable primary plane trickle feed for g4x
From: Ville Syrjälä ville.syrj...@linux.intel.com The docs say that the trickle feed disable bit is present (for primary planes only, not video sprites) on CTG, and that it must be set for ELK. Just set it for all g4x chipsets. v2: Do it in init_clock_gating too Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 9 + 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 684ab64..c8b033a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2041,6 +2041,9 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, dspcntr = ~DISPPLANE_TILED; } + if (IS_G4X(dev)) + dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; + I915_WRITE(reg, dspcntr); linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8a90cf3..cf0f658 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4390,6 +4390,7 @@ static void g4x_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; uint32_t dspclk_gate; + int pipe; I915_WRITE(RENCLK_GATE_D1, 0); I915_WRITE(RENCLK_GATE_D2, VF_UNIT_CLOCK_GATE_DISABLE | @@ -4406,6 +4407,14 @@ static void g4x_init_clock_gating(struct drm_device *dev) /* WaDisableRenderCachePipelinedFlush */ I915_WRITE(CACHE_MODE_0, _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE)); + + for_each_pipe(pipe) { + I915_WRITE(DSPCNTR(pipe), + I915_READ(DSPCNTR(pipe)) | + DISPPLANE_TRICKLE_FEED_DISABLE); + intel_flush_display_plane(dev_priv, pipe); + } + } static void crestline_init_clock_gating(struct drm_device *dev) -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Disable primary plane trickle feed for g4x
On Tue, May 21, 2013 at 03:28:32PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The docs say that the trickle feed disable bit is present (for primary planes only, not video sprites) on CTG, and that it must be set for ELK. Just set it for all g4x chipsets. v2: Do it in init_clock_gating too Actually I just noticed that we don't set up this stuff in ironlake_init_clock_gating() either. Any opinions whether I should just kill the per-plane trickle feed stuff from *_init_clock_gating(), or should I add it to ironlake_init_clock_gating() as well? Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 9 + 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 684ab64..c8b033a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2041,6 +2041,9 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb, dspcntr = ~DISPPLANE_TILED; } + if (IS_G4X(dev)) + dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE; + I915_WRITE(reg, dspcntr); linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8a90cf3..cf0f658 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4390,6 +4390,7 @@ static void g4x_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; uint32_t dspclk_gate; + int pipe; I915_WRITE(RENCLK_GATE_D1, 0); I915_WRITE(RENCLK_GATE_D2, VF_UNIT_CLOCK_GATE_DISABLE | @@ -4406,6 +4407,14 @@ static void g4x_init_clock_gating(struct drm_device *dev) /* WaDisableRenderCachePipelinedFlush */ I915_WRITE(CACHE_MODE_0, _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE)); + + for_each_pipe(pipe) { + I915_WRITE(DSPCNTR(pipe), +I915_READ(DSPCNTR(pipe)) | +DISPPLANE_TRICKLE_FEED_DISABLE); + intel_flush_display_plane(dev_priv, pipe); + } + } static void crestline_init_clock_gating(struct drm_device *dev) -- 1.8.1.5 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Disable primary plane trickle feed for g4x
On Tue, May 21, 2013 at 2:35 PM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, May 21, 2013 at 03:28:32PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The docs say that the trickle feed disable bit is present (for primary planes only, not video sprites) on CTG, and that it must be set for ELK. Just set it for all g4x chipsets. v2: Do it in init_clock_gating too Actually I just noticed that we don't set up this stuff in ironlake_init_clock_gating() either. Any opinions whether I should just kill the per-plane trickle feed stuff from *_init_clock_gating(), or should I add it to ironlake_init_clock_gating() as well? This is a bit a crazy topic since conceptually it ties into the wm/pipe-config stuff. And fastboot will make this stuff rather interesting ... I expect that we'll eventually end up with a post_modeset_fixup stage to patch up all these little bitspieces - fastboot would only call that one if possible. For now I'm not sure what to do though. Ideas highly welcome ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: replace snb_update_wm with haswell_update_wm on HSW
On Thu, May 09, 2013 at 05:13:41PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We were previously calling sandybridge_update_wm on HSW, but the SNB function didn't really match the HSW specification, so we were just writing the wrong values. For example, I was not seeing any LP watermark ever enabled. This patch implements the haswell_update_wm function as described in our specification. The values match the Haswell Watermark Calculator too. With this patch I can finally see us reaching PC7 state with screen sizes = 1920x1080. The only thing I see we're missing is to properly account for the case where the primary plane is disabled. We still don't do this and I believe we'll need some small reworks on intel_sprite.c if we want to do this correctly, so let's leave this feature for a future patch. v2: - Refactor code in the hope that it will be more useful for Ville's rework - Immpletement the 2 pieces missing on v1: sprite watermarks and support for 5/6 data buffer partitioning. Apart from I said yesterday, this is a rather large patch and could be split up a bit to make it easier to digest. - IMHO the 1/2 vs. 5/6 split stuff could be dropped completely for now since it doesn't even appear functional - the pfit downscaling pixel rate adjustment part at least could be a separate patch A few more bikesheds below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |7 + drivers/gpu/drm/i915/intel_drv.h| 12 + drivers/gpu/drm/i915/intel_pm.c | 522 +-- drivers/gpu/drm/i915/intel_sprite.c |6 +- 4 files changed, 527 insertions(+), 20 deletions(-) Hi I had some discussions with Ville based on his plans to rework the watermarks code, so I decided to do a little refactoring on the previous patch in order to make it easier for him. Now we have a clear split between the 3 steps: (i) gather the WM parameters, (ii) calculate the WM values and (iii) write the values to the registers. My idea is that he'll be able to take parts of my Haswell-specific code and make them become usable by the other hardware generations. He'll probably have to rename some of the hsw_ structs and move them around, but I hope my code can be used as a starting point for his rework. In addition to the refactoring I also added support for sprite watermarks and the 5/6 data buffer partitioning mode, so we should be feature complete. I checked the values set by the Kernel and they do match the watermarks calculator. Thanks, Paulo diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 92dcbe3..33b5de3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3055,6 +3055,10 @@ #define WM3S_LP_IVB 0x45128 #define WM1S_LP_EN (131) +#define HSW_WM_LP_VAL(lat, fbc, pri, cur) \ + (WM3_LP_EN | ((lat) WM1_LP_LATENCY_SHIFT) | \ + ((fbc) WM1_LP_FBC_SHIFT) | ((pri) WM1_LP_SR_SHIFT) | (cur)) + /* Memory latency timer register */ #define MLTR_ILK 0x11222 #define MLTR_WM1_SHIFT 0 @@ -4938,6 +4942,9 @@ #define SFUSE_STRAP_DDIC_DETECTED (11) #define SFUSE_STRAP_DDID_DETECTED (10) +#define WM_MISC 0x45260 +#define WM_MISC_DATA_PARTITION_5_6 (1 0) + #define WM_DBG 0x45280 #define WM_DBG_DISALLOW_MULTIPLE_LP (10) #define WM_DBG_DISALLOW_MAXFIFO (11) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 80b417a..b8376e1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -319,6 +319,18 @@ struct intel_plane { unsigned int crtc_w, crtc_h; uint32_t src_x, src_y; uint32_t src_w, src_h; + + /* Since we need to change the watermarks before/after + * enabling/disabling the planes, we need to store the parameters here + * as the other pieces of the struct may not reflect the values we want + * for the watermark calculations. Currently only Haswell uses this. + */ + struct plane_watermark_parameters { The type isn't used anywhere, so no need to give it a type name. + bool enable; + int horiz_pixels; + int bytes_per_pixel; + } wm; + void (*update_plane)(struct drm_plane *plane, struct drm_framebuffer *fb, struct drm_i915_gem_object *obj, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 478518d..afc4705 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2068,20 +2068,236 @@ static void ivybridge_update_wm(struct drm_device *dev) cursor_wm); } -static void -haswell_update_linetime_wm(struct drm_device
Re: [Intel-gfx] [PATCH 9/9] drm/i915: set FORCE_ARB_IDLE_PLANES workaround
2013/5/21 Daniel Vetter dan...@ffwll.ch: On Fri, May 03, 2013 at 05:23:45PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Commit 1544d9d57396d5c0c6b7644ed5ae1f4d6caad07a added a workaround inside haswell_init_clock_gating and mentioned it is a workaround for early silicon revisions and should be removed later. This workaround is documented in bit 31 of PRI_CTL. I asked Arthur and he mentioned that setting FORCE_ARB_IDLE_PLANES replaces that workaround for the newer machines. So use the new one. Also notice that there's still another workaround for PRI_CTL that involves WM_DBG, but it's not the one we're reverting. And notice that we were previously setting WM_DBG_DISALLOW_MULTIPIPE_LP which disables the LP watermarks when more than one pipe is used, and we really don't want this because we need the LP watermarks if we want to reach deeper PC states. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com I've applied all the patches in this series safe for the target_clock one. But this patch here is missing the w/a tag, can you please supply that one in a quick reply so that I can smash it into the patch? Just like most display workarounds, this one doesn't have a name. Thanks, Daniel --- drivers/gpu/drm/i915/i915_reg.h |3 +++ drivers/gpu/drm/i915/intel_pm.c | 10 ++ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index aec569f..5879f23 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3697,6 +3697,9 @@ # define CHICKEN3_DGMG_REQ_OUT_FIX_DISABLE (1 5) # define CHICKEN3_DGMG_DONE_FIX_DISABLE (1 2) +#define CHICKEN_PAR1_1 0x42080 +#define FORCE_ARB_IDLE_PLANES (1 14) + #define DISP_ARB_CTL 0x45000 #define DISP_TILE_SURFACE_SWIZZLING (113) #define DISP_FBC_WM_DIS (115) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b56de92..2297476 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4042,14 +4042,8 @@ static void haswell_init_clock_gating(struct drm_device *dev) /* WaSwitchSolVfFArbitrationPriority */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); - /* XXX: This is a workaround for early silicon revisions and should be - * removed later. - */ - I915_WRITE(WM_DBG, - I915_READ(WM_DBG) | - WM_DBG_DISALLOW_MULTIPLE_LP | - WM_DBG_DISALLOW_SPRITE | - WM_DBG_DISALLOW_MAXFIFO); + I915_WRITE(CHICKEN_PAR1_1, +I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES); lpt_init_clock_gating(dev); } -- 1.7.10.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 -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] debugging Haswell eDP black screen after S3
On Tue, May 21, 2013 at 4:00 AM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, May 17, 2013 at 09:26:18AM -0400, Ben Guthro wrote: On Fri, May 17, 2013 at 7:52 AM, Ben Guthro b...@guthro.net wrote: On Thu, May 16, 2013 at 9:24 AM, Ben Guthro b...@guthro.net wrote: On Wed, May 15, 2013 at 4:42 PM, Ben Guthro b...@guthro.net wrote: On Tue, May 14, 2013 at 5:01 PM, Ben Guthro b...@guthro.net wrote: I am attempting to debug an issue with some Haswell laptop systems which do not restore their screen after resuming from S3 when running on the stable 3.8 kernel (3.8.13) The backlight is OK, but the screen is just black. In trying to determine what was going wrong, I tried looking at the output of intel_reg_dumper, in a good, and bad case: diff -u good_reg.txt bad_reg.txt --- good_reg.txt2013-05-14 15:08:44.361997000 + +++ bad_reg.txt 2013-05-14 15:09:20.48000 + @@ -1,5 +1,4 @@ - DCC: 0x (0xf340 0xf37f 0x�� -� ) + DCC: 0x (0xf340 0xf37f 0x��= � ) CHDECMISC: 0x (none, ch2 enh disabled, ch1 enh disabled, ch0 enh disabled, flex disabled, ep not present) C0DRB0: 0x (0x) C0DRB1: 0x (0x) @@ -63,17 +62,17 @@ PIPEA_DP_LINK_N: 0x CURSOR_A_BASE: 0x01061000 CURSOR_A_CONTROL: 0x0427 - CURSOR_A_POSITION: 0x03a3032f + CURSOR_A_POSITION: 0x01bb03fb FPA0: 0x (n = 0, m1 = 0, m2 = 0) FPA1: 0x (n = 0, m1 = 0, m2 = 0) DPLL_A: 0x (disabled, non-dvo, VGA, default clock, unknown mode, p1 = 0, p2 = 0) DPLL_A_MD: 0x -HTOTAL_A: 0x0821077f (1920 active, 2082 total) -HBLANK_A: 0x0821077f (1920 start, 2082 end) - HSYNC_A: 0x081307af (1968 start, 2068 end) -VTOTAL_A: 0x045f0437 (1080 active, 1120 total) -VBLANK_A: 0x045f0437 (1080 start, 1120 end) - VSYNC_A: 0x044b0441 (1090 start, 1100 end) +HTOTAL_A: 0x (1 active, 1 total) +HBLANK_A: 0x (1 start, 1 end) + HSYNC_A: 0x (1 start, 1 end) +VTOTAL_A: 0x (1 active, 1 total) +VBLANK_A: 0x (1 start, 1 end) + VSYNC_A: 0x (1 start, 1 end) BCLRPAT_A: 0x VSYNCSHIFT_A: 0x DSPBCNTR: 0x4000 (disabled, pipe A) It appears the registers that are saved, and restored in i915_save_modeset_reg / i915_restore_modeset_reg is not working properly. When I put some debug in, I discovered that it was bailing out of i915_save_modeset_reg early since the DRIVER_MODESET bit was cleared. However, it was set at the end of i915_init() This, of course, confuses me. Am I seeing memory corruption here? It looks like I misread the code here, inversing an if statement state. That said, I don't really have any clues as to why the display is black after resuming from S3 It appears that S3 is not necessary. I can reproduce the black screen with just vbetool: vbetool dpms off vbetool dpms on Does this suggest a bios issue? This can be reliably reproduced on this machine, and worked around by saving the vbestate, and restoring it after the fact: (in a working state) vbetool vbestate save vbe.save break the system: vbetool dpms off vbetool dpms on This will break kms since now you have the vbios and the linux kms driver fighting over the same piece of hw. Does xset dpms force off xset dpms force on cause similar issues? No, these work as expected (on 3.8) I didn't realize that these broke with KMS. I'll stick with the S3 reproduction. If not please make sure that vbetool isn't badly interfering with the kernel modeset driver on suspend/resume. At least looking at your dmesg and reg dumps vbe wreaking havoc with the kms driver seems like a rather likely scenario. Also, can you please test latest 3.10-rc kernels? 3.10-rc2 doesn't seem to work at all - it boots to a black screen every time. Ben 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
Re: [Intel-gfx] debugging Haswell eDP black screen after S3
On Tue, May 21, 2013 at 9:44 AM, Ben Guthro b...@guthro.net wrote: On Tue, May 21, 2013 at 4:00 AM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, May 17, 2013 at 09:26:18AM -0400, Ben Guthro wrote: On Fri, May 17, 2013 at 7:52 AM, Ben Guthro b...@guthro.net wrote: On Thu, May 16, 2013 at 9:24 AM, Ben Guthro b...@guthro.net wrote: On Wed, May 15, 2013 at 4:42 PM, Ben Guthro b...@guthro.net wrote: On Tue, May 14, 2013 at 5:01 PM, Ben Guthro b...@guthro.net wrote: I am attempting to debug an issue with some Haswell laptop systems which do not restore their screen after resuming from S3 when running on the stable 3.8 kernel (3.8.13) The backlight is OK, but the screen is just black. In trying to determine what was going wrong, I tried looking at the output of intel_reg_dumper, in a good, and bad case: diff -u good_reg.txt bad_reg.txt --- good_reg.txt2013-05-14 15:08:44.361997000 + +++ bad_reg.txt 2013-05-14 15:09:20.48000 + @@ -1,5 +1,4 @@ - DCC: 0x (0xf340 0xf37f 0x�� -� ) + DCC: 0x (0xf340 0xf37f 0x��= � ) CHDECMISC: 0x (none, ch2 enh disabled, ch1 enh disabled, ch0 enh disabled, flex disabled, ep not present) C0DRB0: 0x (0x) C0DRB1: 0x (0x) @@ -63,17 +62,17 @@ PIPEA_DP_LINK_N: 0x CURSOR_A_BASE: 0x01061000 CURSOR_A_CONTROL: 0x0427 - CURSOR_A_POSITION: 0x03a3032f + CURSOR_A_POSITION: 0x01bb03fb FPA0: 0x (n = 0, m1 = 0, m2 = 0) FPA1: 0x (n = 0, m1 = 0, m2 = 0) DPLL_A: 0x (disabled, non-dvo, VGA, default clock, unknown mode, p1 = 0, p2 = 0) DPLL_A_MD: 0x -HTOTAL_A: 0x0821077f (1920 active, 2082 total) -HBLANK_A: 0x0821077f (1920 start, 2082 end) - HSYNC_A: 0x081307af (1968 start, 2068 end) -VTOTAL_A: 0x045f0437 (1080 active, 1120 total) -VBLANK_A: 0x045f0437 (1080 start, 1120 end) - VSYNC_A: 0x044b0441 (1090 start, 1100 end) +HTOTAL_A: 0x (1 active, 1 total) +HBLANK_A: 0x (1 start, 1 end) + HSYNC_A: 0x (1 start, 1 end) +VTOTAL_A: 0x (1 active, 1 total) +VBLANK_A: 0x (1 start, 1 end) + VSYNC_A: 0x (1 start, 1 end) BCLRPAT_A: 0x VSYNCSHIFT_A: 0x DSPBCNTR: 0x4000 (disabled, pipe A) It appears the registers that are saved, and restored in i915_save_modeset_reg / i915_restore_modeset_reg is not working properly. When I put some debug in, I discovered that it was bailing out of i915_save_modeset_reg early since the DRIVER_MODESET bit was cleared. However, it was set at the end of i915_init() This, of course, confuses me. Am I seeing memory corruption here? It looks like I misread the code here, inversing an if statement state. That said, I don't really have any clues as to why the display is black after resuming from S3 It appears that S3 is not necessary. I can reproduce the black screen with just vbetool: vbetool dpms off vbetool dpms on Does this suggest a bios issue? This can be reliably reproduced on this machine, and worked around by saving the vbestate, and restoring it after the fact: (in a working state) vbetool vbestate save vbe.save break the system: vbetool dpms off vbetool dpms on This will break kms since now you have the vbios and the linux kms driver fighting over the same piece of hw. Does xset dpms force off xset dpms force on cause similar issues? No, these work as expected (on 3.8) I didn't realize that these broke with KMS. I'll stick with the S3 reproduction. If not please make sure that vbetool isn't badly interfering with the kernel modeset driver on suspend/resume. At least looking at your dmesg and reg dumps vbe wreaking havoc with the kms driver seems like a rather likely scenario. vbetool was not installed on the system when I took those S3 dumps, so it seems unlikely to be the root of the problem, IMO. Also, can you please test latest 3.10-rc kernels? 3.10-rc2 doesn't seem to work at all - it boots to a black screen every time. Ben 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
Re: [Intel-gfx] debugging Haswell eDP black screen after S3
On Tue, May 21, 2013 at 3:44 PM, Ben Guthro b...@guthro.net wrote: This will break kms since now you have the vbios and the linux kms driver fighting over the same piece of hw. Does xset dpms force off xset dpms force on cause similar issues? No, these work as expected (on 3.8) I didn't realize that these broke with KMS. I'll stick with the S3 reproduction. Ok, so things are at least not terribly broken. If not please make sure that vbetool isn't badly interfering with the kernel modeset driver on suspend/resume. At least looking at your dmesg and reg dumps vbe wreaking havoc with the kms driver seems like a rather likely scenario. Also, can you please test latest 3.10-rc kernels? 3.10-rc2 doesn't seem to work at all - it boots to a black screen every time. That otoh is ugly. Could be that though that this is the same (or a similar bug) to your resume issue - in the last few kernel releases we've tried very hard to unify the code between initial driver load at boot-up and resume. So can you please try to bisect where the boot-up regression has been introduced between 3.8 and 3.10-rc2? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Genereal question regarding kernel development
Hi Jan, First things first, please _always_ cc relevant mailing lists. Second: I've submitted a proper patch to revert this behaviour change, but it has been nacked. I've poked Dave Airlie about it again on irc. See https://patchwork.kernel.org/patch/2402211/ Thanks, Daniel On Tue, May 21, 2013 at 3:28 PM, Jan Niggemann j...@hz6.de wrote: Hi Daniel, sorry for bothering you directly, but being a kernel dev you can probably answer my question: I read what patchwork is and what it's used for; I also understand that bugs are tracked in bugzilla and that both are separate systems. My question is: I'm affected by this: https://patchwork.kernel.org/patch/2400621/ I expected this patch to be somehow linked to a bugzilla entry I could subscribe to (in order to get updates on issue)... But I'm unable to find a bug in bugzilla regarding kworker irq storm or similar... Are patches in patchwork that correct kernel bugs in any way linked to bugzilla issues and if so, how? Thank you for your time! Regards Jan -- 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 3/9] drm/i915: use the mode-htotal to calculate linetime watermarks
2013/5/21 Daniel Vetter dan...@ffwll.ch: On Fri, May 03, 2013 at 05:23:39PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com ... instead of mode-crtc_display. The spec says pipe horizontal total number of pixels and the Haswell Watermark Calculator tool uses the Pipe H Total instead of Pipe H Src as the value. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d056bc9..4cc5f99 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) * row at the given clock rate, multiplied by 8. * */ temp |= PIPE_WM_LINETIME_TIME( - ((mode-crtc_hdisplay * 1000) / mode-clock) * 8); + ((mode-htotal * 1000) / mode-clock) * 8); Big question: What's the right value for interlaced modes? On progressive mode crtc_ mode values match the non-crtc_ prefixed ones, but not so much for interlaced modes ... So if your tool expects something resembling what we program into the pipe registers, we need to change this again. Merged for now since the spec explicitly says pixels, but I'd like someone to double-check this. The WM calculator tool says: Pipe H total is found in PIPE_HTOTAL horizontal field. But mode-htotal and mode-crtc_htotal are the same even when the mode is interlaced. So at least the values we compute are correct. I could write a follow-up patch updating this if you want. -Daniel /* IPS watermarks are only used by pipe A, and are ignored by * pipes B and C. They are calculated similarly to the common -- 1.7.10.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 -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Disable primary plane trickle feed for g4x
On Tue, May 21, 2013 at 02:52:24PM +0200, Daniel Vetter wrote: On Tue, May 21, 2013 at 2:35 PM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, May 21, 2013 at 03:28:32PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The docs say that the trickle feed disable bit is present (for primary planes only, not video sprites) on CTG, and that it must be set for ELK. Just set it for all g4x chipsets. v2: Do it in init_clock_gating too Actually I just noticed that we don't set up this stuff in ironlake_init_clock_gating() either. Any opinions whether I should just kill the per-plane trickle feed stuff from *_init_clock_gating(), or should I add it to ironlake_init_clock_gating() as well? This is a bit a crazy topic since conceptually it ties into the wm/pipe-config stuff. And fastboot will make this stuff rather interesting ... I expect that we'll eventually end up with a post_modeset_fixup stage to patch up all these little bitspieces - fastboot would only call that one if possible. I'd expect we do at least a set_base w/ fastboot, which would take care of the per-plane trickle feed bit. But I don't care much either way at this point. I'll post a quick patch for ironlake_init_clock_gating() just to keep these things at least somewhat consistent... -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Disable trickle feed in ironlake_init_clock_gating()
From: Ville Syrjälä ville.syrj...@linux.intel.com We disable trickle feed in all the (relevant) clock gating functions, except ironlake_init_clock_gating(). Copy paste the same code there as well. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 93d13d9..4ca214d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3870,6 +3870,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; + int pipe; /* Required for FBC */ dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE | @@ -3929,6 +3930,13 @@ static void ironlake_init_clock_gating(struct drm_device *dev) I915_WRITE(CACHE_MODE_0, _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE)); + for_each_pipe(pipe) { + I915_WRITE(DSPCNTR(pipe), + I915_READ(DSPCNTR(pipe)) | + DISPPLANE_TRICKLE_FEED_DISABLE); + intel_flush_display_plane(dev_priv, pipe); + } + ibx_init_clock_gating(dev); } -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: use the mode-htotal to calculate linetime watermarks
On Tue, May 21, 2013 at 4:33 PM, Paulo Zanoni przan...@gmail.com wrote: 2013/5/21 Daniel Vetter dan...@ffwll.ch: On Fri, May 03, 2013 at 05:23:39PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com ... instead of mode-crtc_display. The spec says pipe horizontal total number of pixels and the Haswell Watermark Calculator tool uses the Pipe H Total instead of Pipe H Src as the value. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d056bc9..4cc5f99 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2035,7 +2035,7 @@ haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) * row at the given clock rate, multiplied by 8. * */ temp |= PIPE_WM_LINETIME_TIME( - ((mode-crtc_hdisplay * 1000) / mode-clock) * 8); + ((mode-htotal * 1000) / mode-clock) * 8); Big question: What's the right value for interlaced modes? On progressive mode crtc_ mode values match the non-crtc_ prefixed ones, but not so much for interlaced modes ... So if your tool expects something resembling what we program into the pipe registers, we need to change this again. Merged for now since the spec explicitly says pixels, but I'd like someone to double-check this. The WM calculator tool says: Pipe H total is found in PIPE_HTOTAL horizontal field. But mode-htotal and mode-crtc_htotal are the same even when the mode is interlaced. So at least the values we compute are correct. I could write a follow-up patch updating this if you want. Oh, I've mixed up the horizontal timings with the vertical ones. You're right, everything is working correctly as-is. -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 2/6] drm/i915: Always enable the cursor right after the primary plane
On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Follow the same sequence when enabling the cursor plane during modeset. No point in doing this stuff in different order on different generations. This should also avoid a needless wait for vblank for the g4x cursor workaround when the cursor gets enabled anyway. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) @@ -3727,6 +3725,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_enable_pipe(dev_priv, pipe, false); intel_enable_plane(dev_priv, plane, pipe); + intel_crtc_update_cursor(crtc, true); if (IS_G4X(dev)) g4x_fixup_plane(dev_priv, pipe); As discussed on IRC this may interfere with commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595 Author: Egbert Eich e...@suse.com Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 I cannot test on the affected hardware as it's not available to me at the moment. If Ville's G4x doesn't show the issue I will test this next time I have access to the HW in question. Therefore: Acked-by: Egbert Eich e...@suse.com Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Genereal question regarding kernel development
On Tue, May 21, 2013 at 4:30 PM, Jan Niggemann j...@hz6.de wrote: Hi Daniel, you misunderstood my mail, I just wanted to know: Are patches in patchwork that correct kernel bugs in any way linked to bugzilla issues and if so, how? In other words: What bugzilla bug corresponds to https://patchwork.kernel.org/patch/2402211/ ? There's no bug rebort for this issue in kernel bugzilla afaik. And please don't drop cc's. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Be more informative when reporting too large for aperture error
This should help debugging the truly unexpected cases where it occurs - in particular to see which value is garbage. References: https://bugzilla.kernel.org/show_bug.cgi?id=58511 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 59349e3..38c8f11 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2993,7 +2993,10 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj, */ if (obj-base.size (map_and_fenceable ? dev_priv-gtt.mappable_end : dev_priv-gtt.total)) { - DRM_ERROR(Attempting to bind an object larger than the aperture\n); + DRM_ERROR(Attempting to bind an object larger than the aperture: object=%ld %s aperture=%ld\n, + obj-base.size, + map_and_fenceable ? mappable : total, + map_and_fenceable ? dev_priv-gtt.mappable_end : dev_priv-gtt.total); return -E2BIG; } -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Be more informative when reporting too large for aperture error
On Tue, May 21, 2013 at 04:58:49PM +0100, Chris Wilson wrote: This should help debugging the truly unexpected cases where it occurs - in particular to see which value is garbage. References: https://bugzilla.kernel.org/show_bug.cgi?id=58511 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk 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 V2] drm/i915: force full modeset if the connector is in DPMS OFF mode
On Mon, May 20, 2013 at 02:13:22PM -0700, Jesse Barnes wrote: On Fri, 3 May 2013 19:44:07 +0200 Egbert Eich e...@suse.de wrote: From: Imre Deak imre.d...@intel.com Currently the driver's assumed behavior for a modeset with an attached FB is that the corresponding connector will be switched to DPMS ON mode if it happened to be in DPMS OFF (or another power save mode). This wasn't enforced though if only the FB changed, everything else (format, connector etc.) remaining the same. In this case we only set the new FB base and left the connector in the old power save mode. Fix this by forcing a full modeset whenever there is an attached FB and any affected connector is in a power save mode. V_2: Run the test for encoders in power save mode outside the the test for fb change: user space may have just disabled the encoders but left everything else in place. Make sure the connector list is not empty before running this test. Signed-off-by: Imre Deak imre.d...@intel.com Signed-off-by: Egbert Eich e...@suse.de --- drivers/gpu/drm/i915/intel_display.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2939524..992d469 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8380,6 +8380,21 @@ static void intel_set_config_restore_state(struct drm_device *dev, } } +static bool +connector_off(struct drm_crtc *crtc, struct drm_connector *connectors, + int num_connectors) +{ + int i; + + for (i = 0; i num_connectors; i++) + if (connectors[i].encoder + connectors[i].encoder-crtc == crtc + connectors[i].dpms != DRM_MODE_DPMS_ON) + return true; + + return false; +} + static void intel_set_config_compute_mode_changes(struct drm_mode_set *set, struct intel_set_config *config) @@ -8387,7 +8402,11 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, /* We should be able to check here if the fb has the same properties * and then just flip_or_move it */ - if (set-crtc-fb != set-fb) { + if (set-connectors != NULL + connector_off(set-crtc, *set-connectors, + set-num_connectors)) { + config-mode_changed = true; + } else if (set-crtc-fb != set-fb) { /* If we have no fb then treat it as a full mode set */ if (set-crtc-fb == NULL) { DRM_DEBUG_KMS(crtc has no fb, full mode set\n); @@ -8397,8 +8416,9 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, } else if (set-fb-pixel_format != set-crtc-fb-pixel_format) { config-mode_changed = true; - } else + } else { config-fb_changed = true; + } } if (set-fb (set-x != set-crtc-x || set-y != set-crtc-y)) Acked-by: Jesse Barnes jbar...@virtuousgeek.org Picked up for -fixes, thanks for the patch. We can bikeshed this later on, but this patch here at least should patch the worst holes. And maybe we come up with saner semantics for the new atomic modeset stuff ... -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/4] drm/i915: add msecs_to_jiffies_timeout to guarantee minimum duration
We need this to avoid premature timeouts whenever scheduling a timeout based on the current jiffies value. For an explanation see [1]. The following patches will take the helper into use. Once the more generic solution proposed in the thread at [1] is accepted this patch can be reverted while keeping the follow-up patches. [1] http://marc.info/?l=linux-kernelm=136854294730957w=2 Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 639ec0b..78b6c56 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1981,4 +1981,19 @@ static inline void __user *to_user_ptr(u64 address) return (void __user *)(uintptr_t)address; } +static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) +{ + unsigned long j = msecs_to_jiffies(m); + + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1); +} + +static inline unsigned long +timespec_to_jiffies_timeout(const struct timespec *value) +{ + unsigned long j = timespec_to_jiffies(value); + + return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1); +} + #endif -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: use msecs_to_jiffies_timeout instead of open coding the same
Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 5d24503..98cd8535 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -228,7 +228,7 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv, * need to wake up periodically and check that ourselves. */ I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en); - for (i = 0; i msecs_to_jiffies(50) + 1; i++) { + for (i = 0; i msecs_to_jiffies_timeout(50); i++) { prepare_to_wait(dev_priv-gmbus_wait_queue, wait, TASK_UNINTERRUPTIBLE); -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: avoid premature DP AUX timeouts
During DP AUX communication we might time out 1 jiffy too early, because the calculated expiry jiffy value is one less than needed. This is only one reason for false DP AUX timeouts. For a complete solution we also need the following fix, which is now queued for mainline: http://marc.info/?l=linux-kernelm=136748515710837w=2 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64133 Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_i2c.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3426a61..8cdc3d7 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -276,7 +276,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq) #define C (((status = I915_READ_NOTRACE(ch_ctl)) DP_AUX_CH_CTL_SEND_BUSY) == 0) if (has_aux_irq) done = wait_event_timeout(dev_priv-gmbus_wait_queue, C, - msecs_to_jiffies(10)); + msecs_to_jiffies_timeout(10)); else done = wait_for_atomic(C, 10) == 0; if (!done) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 98cd8535..639fe19 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -263,7 +263,8 @@ gmbus_wait_idle(struct drm_i915_private *dev_priv) /* Important: The hw handles only the first bit, so set only one! */ I915_WRITE(GMBUS4 + reg_offset, GMBUS_IDLE_EN); - ret = wait_event_timeout(dev_priv-gmbus_wait_queue, C, 10); + ret = wait_event_timeout(dev_priv-gmbus_wait_queue, C, +msecs_to_jiffies_timeout(10)); I915_WRITE(GMBUS4 + reg_offset, 0); -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: avoid premature timeouts in __wait_seqno()
At the moment wait_event_timeout/wait_event_interruptible_timeout may time out 1 jiffy too early, as the calculated expiry time is 1 less than needed. Besides timing out too early this also means that the calculation of the remaining time will be incorrect and we will pass a non-zero remaining time to user space in case of a time out. This is one reason for the following bugzilla report: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64270 Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index a1a282c..2b51fa7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1003,7 +1003,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, wait_forever = false; } - timeout_jiffies = timespec_to_jiffies(wait_time); + timeout_jiffies = timespec_to_jiffies_timeout(wait_time); if (WARN_ON(!ring-irq_get(ring))) return -ENODEV; -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: use msecs_to_jiffies_timeout instead of open coding the same
We have another one of these in the wait_for register wait macro in intel_drv.h Can you please amend your patch with that fixed up, too? Thanks, Daniel On Tue, May 21, 2013 at 7:03 PM, Imre Deak imre.d...@intel.com wrote: Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 5d24503..98cd8535 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -228,7 +228,7 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv, * need to wake up periodically and check that ourselves. */ I915_WRITE(GMBUS4 + reg_offset, gmbus4_irq_en); - for (i = 0; i msecs_to_jiffies(50) + 1; i++) { + for (i = 0; i msecs_to_jiffies_timeout(50); i++) { prepare_to_wait(dev_priv-gmbus_wait_queue, wait, TASK_UNINTERRUPTIBLE); -- 1.8.1.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] debugging Haswell eDP black screen after S3
On Tue, May 21, 2013 at 10:02 AM, Daniel Vetter dan...@ffwll.ch wrote: On Tue, May 21, 2013 at 3:44 PM, Ben Guthro b...@guthro.net wrote: This will break kms since now you have the vbios and the linux kms driver fighting over the same piece of hw. Does xset dpms force off xset dpms force on cause similar issues? No, these work as expected (on 3.8) I didn't realize that these broke with KMS. I'll stick with the S3 reproduction. Ok, so things are at least not terribly broken. If not please make sure that vbetool isn't badly interfering with the kernel modeset driver on suspend/resume. At least looking at your dmesg and reg dumps vbe wreaking havoc with the kms driver seems like a rather likely scenario. Also, can you please test latest 3.10-rc kernels? 3.10-rc2 doesn't seem to work at all - it boots to a black screen every time. That otoh is ugly. Could be that though that this is the same (or a similar bug) to your resume issue - in the last few kernel releases we've tried very hard to unify the code between initial driver load at boot-up and resume. Perhaps I should qualify at all It seems that it fails somewhat late in the boot process. If I remove the boot splash cli params, I can see it transition into the high res mode, and seemingly get into init. However, even if I boot to single user mode, the screen goes black. Unfortunately, both times I tried to test this, and then reboot, I ended up at a grub rescue prompt, with an unusable system. So can you please try to bisect where the boot-up regression has been introduced between 3.8 and 3.10-rc2? I'm not sure I'll be able to do this. With the failure condition I describe above, I am unable to even ssh into this machine to debug, nevermind install a new kernel. This means I need to generate a new kernel, and install kit with that kernel for every bisection test. This may be more time than I am able to dedicate to this problem - but I'll try. Ben Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: use msecs_to_jiffies_timeout instead of open coding the same
On Tue, 2013-05-21 at 19:20 +0200, Daniel Vetter wrote: We have another one of these in the wait_for register wait macro in intel_drv.h Can you please amend your patch with that fixed up, too? I noticed it, but didn't change it since we don't need there the +1 adjustment to begin with. The time_after() check already makes sure we wait at least MS amount. So I think we should remove +1 from there.. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: use msecs_to_jiffies_timeout instead of open coding the same
On Tue, May 21, 2013 at 8:00 PM, Imre Deak imre.d...@intel.com wrote: On Tue, 2013-05-21 at 19:20 +0200, Daniel Vetter wrote: We have another one of these in the wait_for register wait macro in intel_drv.h Can you please amend your patch with that fixed up, too? I noticed it, but didn't change it since we don't need there the +1 adjustment to begin with. The time_after() check already makes sure we wait at least MS amount. So I think we should remove +1 from there.. Hm, right. Looks like the important part of commit 1d5bfac96f1e1856fbdb3f06679691e5b9c2ba8f Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Thu Mar 28 00:03:25 2013 +0100 drm/i915: fix up _wait_for macro was to just recheck the condition after the timeout expired. I agree that this is a separate 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
[Intel-gfx] [PATCH 1/9] drm/i915: split out intel_pnv_find_best_PLL
Pineview is just different. Also split out i9xx_clock from intel_clock and drop the now redundant struct device * parameter. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 92 ++-- 1 file changed, 77 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4196155..dc09c95 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -97,10 +97,13 @@ intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc, int target, int refclk, intel_clock_t *match_clock, intel_clock_t *best_clock); static bool +intel_pnv_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc, + int target, int refclk, intel_clock_t *match_clock, + intel_clock_t *best_clock); +static bool intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc, int target, int refclk, intel_clock_t *match_clock, intel_clock_t *best_clock); - static bool intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc, int target, int refclk, intel_clock_t *match_clock, @@ -246,7 +249,7 @@ static const intel_limit_t intel_limits_pineview_sdvo = { .p1 = { .min = 1, .max = 8 }, .p2 = { .dot_limit = 20, .p2_slow = 10, .p2_fast = 5 }, - .find_pll = intel_find_best_PLL, + .find_pll = intel_pnv_find_best_PLL, }; static const intel_limit_t intel_limits_pineview_lvds = { @@ -260,7 +263,7 @@ static const intel_limit_t intel_limits_pineview_lvds = { .p1 = { .min = 1, .max = 8 }, .p2 = { .dot_limit = 112000, .p2_slow = 14, .p2_fast = 14 }, - .find_pll = intel_find_best_PLL, + .find_pll = intel_pnv_find_best_PLL, }; /* Ironlake / Sandybridge @@ -512,12 +515,8 @@ static uint32_t i9xx_dpll_compute_m(struct dpll *dpll) return 5 * (dpll-m1 + 2) + (dpll-m2 + 2); } -static void intel_clock(struct drm_device *dev, int refclk, intel_clock_t *clock) +static void i9xx_clock(int refclk, intel_clock_t *clock) { - if (IS_PINEVIEW(dev)) { - pineview_clock(refclk, clock); - return; - } clock-m = i9xx_dpll_compute_m(clock); clock-p = clock-p1 * clock-p2; clock-vco = refclk * clock-m / (clock-n + 2); @@ -578,7 +577,68 @@ static bool intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc, int target, int refclk, intel_clock_t *match_clock, intel_clock_t *best_clock) +{ + struct drm_device *dev = crtc-dev; + intel_clock_t clock; + int err = target; + + if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) { + /* +* For LVDS just rely on its current settings for dual-channel. +* We haven't figured out how to reliably set up different +* single/dual channel state, if we even can. +*/ + if (intel_is_dual_link_lvds(dev)) + clock.p2 = limit-p2.p2_fast; + else + clock.p2 = limit-p2.p2_slow; + } else { + if (target limit-p2.dot_limit) + clock.p2 = limit-p2.p2_slow; + else + clock.p2 = limit-p2.p2_fast; + } + + memset(best_clock, 0, sizeof(*best_clock)); + + for (clock.m1 = limit-m1.min; clock.m1 = limit-m1.max; +clock.m1++) { + for (clock.m2 = limit-m2.min; +clock.m2 = limit-m2.max; clock.m2++) { + /* m1 is always 0 in Pineview */ + if (clock.m2 = clock.m1 !IS_PINEVIEW(dev)) + break; + for (clock.n = limit-n.min; +clock.n = limit-n.max; clock.n++) { + for (clock.p1 = limit-p1.min; + clock.p1 = limit-p1.max; clock.p1++) { + int this_err; + i9xx_clock(refclk, clock); + if (!intel_PLL_is_valid(dev, limit, + clock)) + continue; + if (match_clock + clock.p != match_clock-p) + continue; + + this_err = abs(clock.dot - target); + if (this_err err) { + *best_clock = clock; +
[Intel-gfx] [PATCH 0/9] pll limit fixes and a few other things
Hi all, So pipe config conversion progresses neatly, and this pile here contains a few more prep work things: - fix up the pll limits (or at least what the current believe about the correct values is ...) - unconfuse the meaning of adjusted_mode-clock, a side effect of this is that it makes the hsw wm patch I've rejected from Paulo obsolete (or at least it should do so). - and on top a tiny cleanup and a paranoid WARN I've promised Mika Commentsflames as usual highly welcome! Cheers, Daniel Daniel Vetter (9): drm/i915: split out intel_pnv_find_best_PLL drm/i915: move find_pll callback to dev_priv-display drm/i915: fixup i8xx pll limits drm/i915: fixup g4x pll limits drm/i915: pnv dpll doesn't use m1! drm/i915: fix ibx/cpt/ppt dpll limits drm/i915: store adjust dotclock in adjustede_mode-clock drm/i915: Drop some no longer required mode/adjusted_mode parameters drm/i915: check for strange pfit pipe assignemnt on ivb/hsw drivers/gpu/drm/i915/i915_drv.h | 7 + drivers/gpu/drm/i915/intel_ddi.c | 3 +- drivers/gpu/drm/i915/intel_display.c | 313 ++- drivers/gpu/drm/i915/intel_dp.c | 50 ++ drivers/gpu/drm/i915/intel_drv.h | 13 +- drivers/gpu/drm/i915/intel_hdmi.c| 4 +- 6 files changed, 196 insertions(+), 194 deletions(-) -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/9] drm/i915: move find_pll callback to dev_priv-display
Now that the DP madness is cleared out, this is all only per-platform. So move it out from the intel clock limits structure. While at it drop the intel prefix on the static functions, call the vtable entry find_dpll (since it's for the display pll) and rip out the now unnecessary forward declarations. Note that the parameters of -find_dpll are still unchanged, but they eventually need to be moved over to just take in a pipe configuration. But currently a lot of things are still missing from the pipe configuration (reflock, output-specific dpll limits and preferences, downclocked dotclock). So this will happen in a later step. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.h | 7 +++ drivers/gpu/drm/i915/intel_display.c | 106 ++- 2 files changed, 38 insertions(+), 75 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 14817de..b1ddc6b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -306,6 +306,8 @@ struct drm_i915_error_state { struct intel_crtc_config; struct intel_crtc; +struct intel_limit; +struct dpll; struct drm_i915_display_funcs { bool (*fbc_enabled)(struct drm_device *dev); @@ -313,6 +315,11 @@ struct drm_i915_display_funcs { void (*disable_fbc)(struct drm_device *dev); int (*get_display_clock_speed)(struct drm_device *dev); int (*get_fifo_size)(struct drm_device *dev, int plane); + bool (*find_dpll)(const struct intel_limit *limit, + struct drm_crtc *crtc, + int target, int refclk, + struct dpll *match_clock, + struct dpll *best_clock); void (*update_wm)(struct drm_device *dev); void (*update_sprite_wm)(struct drm_device *dev, int pipe, uint32_t sprite_width, int pixel_size); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dc09c95..959c2ae 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -59,24 +59,6 @@ typedef struct intel_limit intel_limit_t; struct intel_limit { intel_range_t dot, vco, n, m, m1, m2, p, p1; intel_p2_t p2; - /** -* find_pll() - Find the best values for the PLL -* @limit: limits for the PLL -* @crtc: current CRTC -* @target: target frequency in kHz -* @refclk: reference clock frequency in kHz -* @match_clock: if provided, @best_clock P divider must -* match the P divider from @match_clock -* used for LVDS downclocking -* @best_clock: best PLL values found -* -* Returns true on success, false on failure. -*/ - bool (*find_pll)(const intel_limit_t *limit, -struct drm_crtc *crtc, -int target, int refclk, -intel_clock_t *match_clock, -intel_clock_t *best_clock); }; /* FDI */ @@ -92,23 +74,6 @@ intel_pch_rawclk(struct drm_device *dev) return I915_READ(PCH_RAWCLK_FREQ) RAWCLK_FREQ_MASK; } -static bool -intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc, - int target, int refclk, intel_clock_t *match_clock, - intel_clock_t *best_clock); -static bool -intel_pnv_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc, - int target, int refclk, intel_clock_t *match_clock, - intel_clock_t *best_clock); -static bool -intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc, - int target, int refclk, intel_clock_t *match_clock, - intel_clock_t *best_clock); -static bool -intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc, - int target, int refclk, intel_clock_t *match_clock, - intel_clock_t *best_clock); - static inline u32 /* units of 100MHz */ intel_fdi_link_freq(struct drm_device *dev) { @@ -130,7 +95,6 @@ static const intel_limit_t intel_limits_i8xx_dvo = { .p1 = { .min = 2, .max = 33 }, .p2 = { .dot_limit = 165000, .p2_slow = 4, .p2_fast = 2 }, - .find_pll = intel_find_best_PLL, }; static const intel_limit_t intel_limits_i8xx_lvds = { @@ -144,7 +108,6 @@ static const intel_limit_t intel_limits_i8xx_lvds = { .p1 = { .min = 1, .max = 6 }, .p2 = { .dot_limit = 165000, .p2_slow = 14, .p2_fast = 7 }, - .find_pll = intel_find_best_PLL, }; static const intel_limit_t intel_limits_i9xx_sdvo = { @@ -158,7 +121,6 @@ static const intel_limit_t intel_limits_i9xx_sdvo = { .p1 = { .min = 1, .max = 8 }, .p2 = { .dot_limit = 20, .p2_slow =
[Intel-gfx] [PATCH 3/9] drm/i915: fixup i8xx pll limits
So apparently the pll limits in the docs are the real values, but the formula actually seems to consistenly use register values. See commit 4f7dfb6788dd022446847fbbfbe45e13bedb5be2 Author: Patrik Jakobsson patrik.r.jakobs...@gmail.com Date: Wed Feb 13 22:20:22 2013 +0100 drm/i915: Set i9xx sdvo clock limits according to specifications On gen2 the situation is a bit more messy: We reuse the code for m1/m2/n computation and register frobbing from gen3, so those limits need to be in register values (and so need to substract 2 from the doc limits). But gen2 also stores p1/p2 with 2/1 substracted in the register! For those though i8xx_update_pll does the adjustments, but the clock computation code assumes it works like on gen3. So for divisor limits we must _not_ adjust them to the register values. v2: Extract the common i8xx m/n limits into a single define. This was motivated by the mess for the g4x limits where they've all been off in different directions, apparently to fix specific bugs ... Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 959c2ae..ea8eb0c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -84,13 +84,16 @@ intel_fdi_link_freq(struct drm_device *dev) return 27; } +#define I8XX_DPLL_M_N_LIMITS \ + .n = { .min = 1, .max = 14 }, \ + .m = { .min = 96, .max = 140 }, \ + .m1 = { .min = 16, .max = 24 }, \ + .m2 = { .min = 4, .max = 14 }, + static const intel_limit_t intel_limits_i8xx_dvo = { .dot = { .min = 25000, .max = 35 }, .vco = { .min = 93, .max = 140 }, - .n = { .min = 3, .max = 16 }, - .m = { .min = 96, .max = 140 }, - .m1 = { .min = 18, .max = 26 }, - .m2 = { .min = 6, .max = 16 }, + I8XX_DPLL_M_N_LIMITS .p = { .min = 4, .max = 128 }, .p1 = { .min = 2, .max = 33 }, .p2 = { .dot_limit = 165000, @@ -100,10 +103,7 @@ static const intel_limit_t intel_limits_i8xx_dvo = { static const intel_limit_t intel_limits_i8xx_lvds = { .dot = { .min = 25000, .max = 35 }, .vco = { .min = 93, .max = 140 }, - .n = { .min = 3, .max = 16 }, - .m = { .min = 96, .max = 140 }, - .m1 = { .min = 18, .max = 26 }, - .m2 = { .min = 6, .max = 16 }, + I8XX_DPLL_M_N_LIMITS .p = { .min = 4, .max = 128 }, .p1 = { .min = 1, .max = 6 }, .p2 = { .dot_limit = 165000, -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/9] drm/i915: Drop some no longer required mode/adjusted_mode parameters
We can get at this easily through intel_crtc-config now. v2: Drop more stuff gcc spotted. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index fb481d9..25fa8cd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4619,7 +4619,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc, } static void i8xx_update_pll(struct intel_crtc *crtc, - struct drm_display_mode *adjusted_mode, intel_clock_t *reduced_clock, int num_connectors) { @@ -4674,14 +4673,15 @@ static void i8xx_update_pll(struct intel_crtc *crtc, I915_WRITE(DPLL(pipe), dpll); } -static void intel_set_pipe_timings(struct intel_crtc *intel_crtc, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) +static void intel_set_pipe_timings(struct intel_crtc *intel_crtc) { struct drm_device *dev = intel_crtc-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; enum pipe pipe = intel_crtc-pipe; enum transcoder cpu_transcoder = intel_crtc-config.cpu_transcoder; + struct drm_display_mode *adjusted_mode = + intel_crtc-config.adjusted_mode; + struct drm_display_mode *mode = intel_crtc-config.requested_mode; uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end; /* We need to be careful not to changed the adjusted mode, for otherwise @@ -4859,8 +4859,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_display_mode *adjusted_mode = - intel_crtc-config.adjusted_mode; struct drm_display_mode *mode = intel_crtc-config.requested_mode; int pipe = intel_crtc-pipe; int plane = intel_crtc-plane; @@ -4925,7 +4923,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, } if (IS_GEN2(dev)) - i8xx_update_pll(intel_crtc, adjusted_mode, + i8xx_update_pll(intel_crtc, has_reduced_clock ? reduced_clock : NULL, num_connectors); else if (IS_VALLEYVIEW(dev)) @@ -4948,7 +4946,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, DRM_DEBUG_KMS(Mode for pipe %c:\n, pipe_name(pipe)); drm_mode_debug_printmodeline(mode); - intel_set_pipe_timings(intel_crtc, mode, adjusted_mode); + intel_set_pipe_timings(intel_crtc); /* pipesrc and dspsize control the size that is scaled from, * which should always be the user's requested size. @@ -5805,7 +5803,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, } } - intel_set_pipe_timings(intel_crtc, mode, adjusted_mode); + intel_set_pipe_timings(intel_crtc); if (intel_crtc-config.has_pch_encoder) { intel_cpu_transcoder_set_m_n(intel_crtc, @@ -5975,7 +5973,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, intel_crtc-lowfreq_avail = false; - intel_set_pipe_timings(intel_crtc, mode, adjusted_mode); + intel_set_pipe_timings(intel_crtc); if (intel_crtc-config.has_pch_encoder) { intel_cpu_transcoder_set_m_n(intel_crtc, -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/9] drm/i915: store adjust dotclock in adjustede_mode-clock
... not the port clock. This allows us to kill the funny semantics around pixel_target_clock. Since the dpll code still needs the real port clock, add a new port_clock field to the pipe configuration. Handling the default case for that one is a bit tricky, since encoders might not consistently overwrite it when retrying the crtc/encoder bw arbitrage step in the compute config stage. Hence we need to always clear port_clock and update it again if the encoder hasn't put in something more specific. This can't be done in one step since the encoder might want to adjust the mode first. I was a bit on the fence whether I should subsume the pixel multiplier handling into the port_clock, too. But then decided against this since it's on an abstract level still the dotclock of the adjusted mode, and only our hw makes it a bit special due to the separate pixel mulitplier setting (which requires that the dpll runs at the non-multiplied dotclock). So after this patch the adjusted_mode accurately describes the mode we feed into the port, after the panel fitter and pixel multiplier (or line, if we ever bother with that) multiplier have done their job. Since the fdi link is between the pfit and the pixel multiplier steps we need to be careful with calculating the fdi link config. The ironlake cpu edp port is the only encoder interested in the portclock (outside of compute_config), refactor the flow a bit to avoid duplicated code. I've broken this part in an early version of the patch, hence why I've changed the code a bit more than strictly required. v2: Fix up ilk cpu pll handling. v3: Introduce an fdi_dotclock variable in ironlake_fdi_compute_config to make it clearer that we transmit the adjusted_mode without the pixel multiplier taken into account. The old code multiplied the the available link bw with the pixel multiplier, which results in the same fdi configuration, but is much more confusing. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_ddi.c | 3 ++- drivers/gpu/drm/i915/intel_display.c | 37 ++ drivers/gpu/drm/i915/intel_dp.c | 50 +--- drivers/gpu/drm/i915/intel_drv.h | 13 +- drivers/gpu/drm/i915/intel_hdmi.c| 4 +-- 5 files changed, 48 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index e668521..809dfd6 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -624,7 +624,7 @@ intel_ddi_calculate_wrpll(int clock /* in Hz */, clock, *p_out, *n2_out, *r2_out); } -bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) +bool intel_ddi_pll_mode_set(struct drm_crtc *crtc) { struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc); @@ -634,6 +634,7 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) int type = intel_encoder-type; enum pipe pipe = intel_crtc-pipe; uint32_t reg, val; + int clock = intel_crtc-config.port_clock; /* TODO: reuse PLLs when possible (compare values) */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 80698ac..fb481d9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4044,7 +4044,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, { struct drm_device *dev = intel_crtc-base.dev; struct drm_display_mode *adjusted_mode = pipe_config-adjusted_mode; - int target_clock, lane, link_bw; + int lane, link_bw, fdi_dotclock; bool setup_ok, needs_recompute = false; retry: @@ -4057,19 +4057,15 @@ retry: */ link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; - if (pipe_config-pixel_target_clock) - target_clock = pipe_config-pixel_target_clock; - else - target_clock = adjusted_mode-clock; - - lane = ironlake_get_lanes_required(target_clock, link_bw, + lane = ironlake_get_lanes_required(adjusted_mode-clock, link_bw, pipe_config-pipe_bpp); pipe_config-fdi_lanes = lane; + fdi_dotclock = adjusted_mode-clock; if (pipe_config-pixel_multiplier 1) - link_bw *= pipe_config-pixel_multiplier; - intel_link_compute_m_n(pipe_config-pipe_bpp, lane, target_clock, + fdi_dotclock /= pipe_config-pixel_multiplier; + intel_link_compute_m_n(pipe_config-pipe_bpp, lane, fdi_dotclock, link_bw, pipe_config-fdi_m_n); setup_ok = ironlake_check_fdi_lanes(intel_crtc-base.dev, @@ -4398,8 +4394,6 @@ static void vlv_update_pll(struct intel_crtc *crtc) { struct drm_device *dev = crtc-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; -
[Intel-gfx] [PATCH 9/9] drm/i915: check for strange pfit pipe assignemnt on ivb/hsw
Panel fitters on ivb/hsw are not created equal since not all of them support the new high-quality upscaling mode. To offset this the hw allows us to freely assign the pfits to pipes. Since our code currently doesn't support this we might fall over when taking over firmware state. So check for this case and WARN about it. We can then improve the code once we've hit this in the wild. Or once we decide to support the improved upscale modes, though that requires global arbitrage of modeset resources across crtcs. Suggested-by: Mika Kuoppala mika.kuopp...@intel.com Cc: Mika Kuoppala mika.kuopp...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 25fa8cd..73b6c9f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5856,6 +5856,14 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc, if (tmp PF_ENABLE) { pipe_config-pch_pfit.pos = I915_READ(PF_WIN_POS(crtc-pipe)); pipe_config-pch_pfit.size = I915_READ(PF_WIN_SZ(crtc-pipe)); + + /* We currently do not free assignements of panel fitters on +* ivb/hsw (since we don't use the higher upscaling modes which +* differentiates them) so just WARN about this case for now. */ + if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { + WARN_ON((tmp PF_PIPE_SEL_MASK_IVB) != + PF_PIPE_SEL_IVB(crtc-pipe)); + } } } -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: add basic pipe config dump support
All this pipe config abstraction adds another layer of complexity, so it's good to have better visibility into what's going on exactly. Doesn't dump out everything yet, and some bits are a bit duplicated but this should be a good start. v2: Remove a few more now redudant debug output lines. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 74 +--- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a42a7fd..5efb00d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3621,18 +3621,12 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc) if (!crtc-config.gmch_pfit.control) return; - WARN_ON(I915_READ(PFIT_CONTROL) PFIT_ENABLE); - assert_pipe_disabled(dev_priv, crtc-pipe); - /* -* Enable automatic panel scaling so that non-native modes -* fill the screen. The panel fitter should only be -* adjusted whilst the pipe is disabled, according to -* register description and PRM. +* The panel fitter should only be adjusted whilst the pipe is disabled, +* according to register description and PRM. */ - DRM_DEBUG_KMS(applying panel-fitter: %x, %x\n, - pipe_config-gmch_pfit.control, - pipe_config-gmch_pfit.pgm_ratios); + WARN_ON(I915_READ(PFIT_CONTROL) PFIT_ENABLE); + assert_pipe_disabled(dev_priv, crtc-pipe); I915_WRITE(PFIT_PGM_RATIOS, pipe_config-gmch_pfit.pgm_ratios); I915_WRITE(PFIT_CONTROL, pipe_config-gmch_pfit.control); @@ -4955,9 +4949,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, dspcntr |= DISPPLANE_SEL_PIPE_B; } - DRM_DEBUG_KMS(Mode for pipe %c:\n, pipe_name(pipe)); - drm_mode_debug_printmodeline(mode); - intel_set_pipe_timings(intel_crtc, mode, adjusted_mode); /* pipesrc and dspsize control the size that is scaled from, @@ -5759,9 +5750,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, /* Ensure that the cursor is valid for the new mode before changing... */ intel_crtc_update_cursor(crtc, true); - DRM_DEBUG_KMS(Mode for pipe %c:\n, pipe_name(pipe)); - drm_mode_debug_printmodeline(mode); - /* CPU eDP is the only output that doesn't need a PCH PLL of its own. */ if (intel_crtc-config.has_pch_encoder) { struct intel_pch_pll *pll; @@ -5972,9 +5960,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, /* Ensure that the cursor is valid for the new mode before changing... */ intel_crtc_update_cursor(crtc, true); - DRM_DEBUG_KMS(Mode for pipe %c:\n, pipe_name(pipe)); - drm_mode_debug_printmodeline(mode); - if (intel_crtc-config.has_dp_encoder) intel_dp_set_m_n(intel_crtc); @@ -7783,6 +7768,35 @@ pipe_config_set_bpp(struct drm_crtc *crtc, return bpp; } +static void intel_dump_pipe_config(struct intel_crtc *crtc, + struct intel_crtc_config *pipe_config, + const char *context) +{ + DRM_DEBUG_KMS([CRTC:%d]%s config for pipe %c\n, crtc-base.base.id, + context, pipe_name(crtc-pipe)); + + DRM_DEBUG_KMS(cpu_transcoder: %i\n, pipe_name(pipe_config-cpu_transcoder)); + DRM_DEBUG_KMS(pipe bpp: %i, dithering: %i\n, + pipe_config-pipe_bpp, pipe_config-dither); + DRM_DEBUG_KMS(fdi/pch: %i, lanes: %i, gmch_m: %i, gmch_n %i, link_m: %i, link_n: %i, tu: %i\n, + pipe_config-has_pch_encoder, + pipe_config-fdi_lanes, + pipe_config-fdi_m_n.gmch_m, pipe_config-fdi_m_n.gmch_n, + pipe_config-fdi_m_n.link_m, pipe_config-fdi_m_n.link_n, + pipe_config-fdi_m_n.tu); + DRM_DEBUG_KMS(requested mode:\n); + drm_mode_debug_printmodeline(pipe_config-requested_mode); + DRM_DEBUG_KMS(adjusted mode:\n); + drm_mode_debug_printmodeline(pipe_config-adjusted_mode); + DRM_DEBUG_KMS(gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n, + pipe_config-gmch_pfit.control, + pipe_config-gmch_pfit.pgm_ratios, + pipe_config-gmch_pfit.lvds_border_bits); + DRM_DEBUG_KMS(pch pfit: pos: 0x%08x, size: 0x%08x\n, + pipe_config-pch_pfit.pos, + pipe_config-pch_pfit.size); +} + static struct intel_crtc_config * intel_modeset_pipe_config(struct drm_crtc *crtc, struct drm_framebuffer *fb, @@ -7853,8 +7867,6 @@ encoder_retry: goto encoder_retry; } - DRM_DEBUG_KMS([CRTC:%d]\n, crtc-base.id); - pipe_config-dither =
[Intel-gfx] pipe_off wait timed out
This is new to me as of 3.10-rc2. WARNING: at drivers/gpu/drm/i915/intel_display.c:997 intel_wait_for_pipe_off+0x194/0x1a0 [i915]() pipe_off wait timed out Modules linked in: ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_LOG xt_limit ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter nf_conntrack_ipv4 nf_defrag_ipv4 ip6_tables xt_conntrack nf_conntrack microcode pcspkr r8169 mii nfsd auth_rpcgss nfs_acl lockd sunrpc i915 video backlight i2c_algo_bit drm_kms_helper drm CPU: 3 PID: 1414 Comm: kworker/3:1 Not tainted 3.10.0-rc2+ #3 Hardware name: /D510MO, BIOS MOPNV10J.86A.0175.2010.0308.0620 03/08/2010 Workqueue: events console_callback a00d1c08 8800753c7988 8151fd9f 8800753c79c8 8103ca8b 08dd 880076738000 031f 1fff 00071000 000100086e1b 8800753c7a28 Call Trace: [8151fd9f] dump_stack+0x19/0x1b [8103ca8b] warn_slowpath_common+0x6b/0xa0 [8103cb61] warn_slowpath_fmt+0x41/0x50 [a008d414] intel_wait_for_pipe_off+0x194/0x1a0 [i915] [a008d5a5] intel_disable_pipe+0x185/0x210 [i915] [a008dcef] i9xx_crtc_disable+0xcf/0x230 [i915] [a009245f] intel_crtc_update_dpms+0x6f/0xa0 [i915] [a0092515] intel_encoder_dpms+0x15/0x30 [i915] [a0097207] intel_connector_dpms+0x37/0x70 [i915] [a00520b0] drm_fb_helper_dpms.isra.17+0xc0/0x110 [drm_kms_helper] [a0052131] drm_fb_helper_blank+0x31/0x80 [drm_kms_helper] [812a6f51] fb_blank+0x51/0xc0 [812b5c3b] fbcon_blank+0x22b/0x2e0 [81526d95] ? _raw_spin_unlock_irqrestore+0x65/0x80 [8109a475] ? trace_hardirqs_on_caller+0x105/0x1d0 [8109a54d] ? trace_hardirqs_on+0xd/0x10 [81526d6d] ? _raw_spin_unlock_irqrestore+0x3d/0x80 [8104b3c6] ? try_to_del_timer_sync+0x56/0x70 [8104b48a] ? del_timer_sync+0xaa/0xd0 [8104b3e0] ? try_to_del_timer_sync+0x70/0x70 [81324618] do_blank_screen+0x1d8/0x280 [813269bf] console_callback+0x5f/0x150 [8105a7f1] process_one_work+0x1f1/0x490 [8105a78c] ? process_one_work+0x18c/0x490 [8105aef3] worker_thread+0x113/0x370 [8105ade0] ? rescuer_thread+0x310/0x310 [81062798] kthread+0xe8/0xf0 [81094732] ? get_lock_stats+0x22/0x70 [810626b0] ? kthread_create_on_node+0x150/0x150 [8152d02c] ret_from_fork+0x7c/0xb0 [810626b0] ? kthread_create_on_node+0x150/0x150 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] pipe_off wait timed out
On Wed, May 22, 2013 at 1:01 AM, Dave Jones da...@redhat.com wrote: This is new to me as of 3.10-rc2. If this is an ironlake with a DP output it's a know issue which seems to pop up every once in a while. Otherwise we need to take a look here. If so can you please boot with drm.debug=0xe, reproduce the issue and attach the complete dmesg? Also does 3.10-rc1 work (since you mention -rc2 specifically and there wasn't really a lot going into that for drm/i915). Thanks, Daniel WARNING: at drivers/gpu/drm/i915/intel_display.c:997 intel_wait_for_pipe_off+0x194/0x1a0 [i915]() pipe_off wait timed out Modules linked in: ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_LOG xt_limit ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter nf_conntrack_ipv4 nf_defrag_ipv4 ip6_tables xt_conntrack nf_conntrack microcode pcspkr r8169 mii nfsd auth_rpcgss nfs_acl lockd sunrpc i915 video backlight i2c_algo_bit drm_kms_helper drm CPU: 3 PID: 1414 Comm: kworker/3:1 Not tainted 3.10.0-rc2+ #3 Hardware name: /D510MO, BIOS MOPNV10J.86A.0175.2010.0308.0620 03/08/2010 Workqueue: events console_callback a00d1c08 8800753c7988 8151fd9f 8800753c79c8 8103ca8b 08dd 880076738000 031f 1fff 00071000 000100086e1b 8800753c7a28 Call Trace: [8151fd9f] dump_stack+0x19/0x1b [8103ca8b] warn_slowpath_common+0x6b/0xa0 [8103cb61] warn_slowpath_fmt+0x41/0x50 [a008d414] intel_wait_for_pipe_off+0x194/0x1a0 [i915] [a008d5a5] intel_disable_pipe+0x185/0x210 [i915] [a008dcef] i9xx_crtc_disable+0xcf/0x230 [i915] [a009245f] intel_crtc_update_dpms+0x6f/0xa0 [i915] [a0092515] intel_encoder_dpms+0x15/0x30 [i915] [a0097207] intel_connector_dpms+0x37/0x70 [i915] [a00520b0] drm_fb_helper_dpms.isra.17+0xc0/0x110 [drm_kms_helper] [a0052131] drm_fb_helper_blank+0x31/0x80 [drm_kms_helper] [812a6f51] fb_blank+0x51/0xc0 [812b5c3b] fbcon_blank+0x22b/0x2e0 [81526d95] ? _raw_spin_unlock_irqrestore+0x65/0x80 [8109a475] ? trace_hardirqs_on_caller+0x105/0x1d0 [8109a54d] ? trace_hardirqs_on+0xd/0x10 [81526d6d] ? _raw_spin_unlock_irqrestore+0x3d/0x80 [8104b3c6] ? try_to_del_timer_sync+0x56/0x70 [8104b48a] ? del_timer_sync+0xaa/0xd0 [8104b3e0] ? try_to_del_timer_sync+0x70/0x70 [81324618] do_blank_screen+0x1d8/0x280 [813269bf] console_callback+0x5f/0x150 [8105a7f1] process_one_work+0x1f1/0x490 [8105a78c] ? process_one_work+0x18c/0x490 [8105aef3] worker_thread+0x113/0x370 [8105ade0] ? rescuer_thread+0x310/0x310 [81062798] kthread+0xe8/0xf0 [81094732] ? get_lock_stats+0x22/0x70 [810626b0] ? kthread_create_on_node+0x150/0x150 [8152d02c] ret_from_fork+0x7c/0xb0 [810626b0] ? kthread_create_on_node+0x150/0x150 ___ 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] pipe_off wait timed out
On Wed, May 22, 2013 at 01:10:36AM +0200, Daniel Vetter wrote: On Wed, May 22, 2013 at 1:01 AM, Dave Jones da...@redhat.com wrote: This is new to me as of 3.10-rc2. If this is an ironlake with a DP output it's a know issue which seems to pop up every once in a while. 00:02.0 VGA compatible controller: Intel Corporation Atom Processor D4xx/D5xx/N4xx/N5xx Integrated Graphics Controller (rev 02) Otherwise we need to take a look here. If so can you please boot with drm.debug=0xe, reproduce the issue and attach the complete dmesg? Also does 3.10-rc1 work (since you mention -rc2 specifically and there wasn't really a lot going into that for drm/i915). It had been running rc1 for the last week or so without incident. Though I've been away, with the monitor turned off. Judging by the trace, this seems to be related to console blanking ? (this is my home firewall, so spends most of its time with the screen turned off) I've added that debug option to my command line, I'll keep an eye on it and try to reproduce over the next few days. Dave ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx