Final PC8+ patches
Subsystems (2)
cover Final PC8+ patches
From: Paulo Zanoni <[email protected]>
Hi
Quoted 1 line
From my previous series, patches 1 to 6.3 already got Reviewed-by stamps, and
patches 7+ got some comments. So this series replaces patches 7+ of the previous
series: it's on top of -nightly + patches 1 to 6.3 from the previous series.
There's nothing much new besides some bug fixing and an additional module option to allow some tuning on how often we reach PC8+.
If you want to test everything, please check the c8-wip-20130819 branch of my Kernel tree: http://cgit.freedesktop.org/~pzanoni/linux/?h=c8-wip-20130819 .
Thanks,
Paulo
Paulo Zanoni (6):
drm/i915: grab force_wake when restoring LCPLL drm/i915: fix SDEIMR assertion when disabling LCPLL drm/i915: allow package C8+ states on Haswell (disabled) drm/i915: add i915_pc8_status debugfs file drm/i915: add i915.pc8_timeout function drm/i915: enable Package C8+ by default
drivers/gpu/drm/i915/i915_debugfs.c | 25 +++++ drivers/gpu/drm/i915/i915_dma.c | 10 ++ drivers/gpu/drm/i915/i915_drv.c | 15 +++ drivers/gpu/drm/i915/i915_drv.h | 73 ++++++++++++++ drivers/gpu/drm/i915/i915_gem.c | 2 + drivers/gpu/drm/i915/i915_irq.c | 101 +++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 184 +++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/intel_dp.c | 3 + drivers/gpu/drm/i915/intel_drv.h | 8 ++ drivers/gpu/drm/i915/intel_i2c.c | 2 + drivers/gpu/drm/i915/intel_pm.c | 13 ++- 11 files changed, 428 insertions(+), 8 deletions(-)
Signature · 1 line
1.8.1.2
PATCH 1/6 drm/i915: grab force_wake when restoring LCPLL
From: Paulo Zanoni <[email protected]>
If LCPLL is disabled, there's a chance we might be in package C8 state or deeper, and we'll get a hard hang when restoring LCPLL (also, a red led lights up on my motherboard). So grab the force_wake, which will get us out of RC6 and, as a consequence, out of PC8+ (since we need RC6 to get into PC8+).
--- drivers/gpu/drm/i915/intel_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/gpu/drm/i915/intel_display.c +6 -0
@@ -6031,6 +6031,10 @@ void hsw_restore_lcpll(struct drm_i915_private *dev_priv) LCPLL_POWER_DOWN_ALLOW)) == LCPLL_PLL_LOCK)return ; +/* Make sure we're not on PC8 state before disabling PC8, otherwise +* we'll hang the machine! */ + dev_priv->uncore.funcs.force_wake_get (dev_priv); +if (val &LCPLL_POWER_DOWN_ALLOW ) { val &= ~LCPLL_POWER_DOWN_ALLOW ;I915_WRITE (LCPLL_CTL , val);
@@ -6058,6 +6062,8 @@ void hsw_restore_lcpll(struct drm_i915_private *dev_priv) LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))DRM_ERROR ("Switching back to LCPLL failed\n" ); } + + dev_priv->uncore.funcs.force_wake_put (dev_priv); }static void haswell_modeset_global_resources (struct drm_device *dev)
Signature · 1 line
1.8.1.2
Re: [PATCH 1/6] drm/i915: grab force_wake when restoring LCPLL
Quoted 18 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
If LCPLL is disabled, there's a chance we might be in package C8 state or deeper, and we'll get a hard hang when restoring LCPLL (also, a red led lights up on my motherboard). So grab the
force_wake, which will get us out of RC6 and, as a consequence, out of PC8+ (since we need RC6 to get into PC8+).Signed-off-by Paulo Zanoni <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/gpu/drm/i915/intel_display.c +0
-0
drivers/gpu/drm/i915/intel_display.c void hsw_restore_lcpll(struct drm_i915_private *dev_priv) · line 6036
Quoted 6 lines
drivers/gpu/drm/i915/intel_display.c +3
-0@@ -6031,6 +6031,10 @@ void hsw_restore_lcpll(struct drm_i915_private *dev_priv) LCPLL_POWER_DOWN_ALLOW)) == LCPLL_PLL_LOCK)return ; +/* Make sure we're not on PC8 state before disabling PC8, otherwise +* we'll hang the machine! */ + dev_priv->uncore.funcs.force_wake_get (dev_priv);
Calling the bare function pointer should be a warning that what you are doing is quite perilous and needs a phat comment beforehand.
Or it is a mistake and you just meant gen6_gt_force_wake_get(). -Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
PATCH 1/6 drm/i915: grab force_wake when restoring LCPLL
From: Paulo Zanoni <[email protected]>
If LCPLL is disabled, there's a chance we might be in package C8 state or deeper, and we'll get a hard hang when restoring LCPLL (also, a red led lights up on my motherboard). So grab the force_wake, which will get us out of RC6 and, as a consequence, out of PC8+ (since we need RC6 to get into PC8+).
v2: Call the correct force_wake interface.
--- drivers/gpu/drm/i915/intel_display.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/gpu/drm/i915/intel_display.c +6 -0
@@ -6031,6 +6031,10 @@ void hsw_restore_lcpll(struct drm_i915_private *dev_priv) LCPLL_POWER_DOWN_ALLOW)) == LCPLL_PLL_LOCK)return ; +/* Make sure we're not on PC8 state before disabling PC8, otherwise +* we'll hang the machine! */ +gen6_gt_force_wake_get (dev_priv); +if (val &LCPLL_POWER_DOWN_ALLOW ) { val &= ~LCPLL_POWER_DOWN_ALLOW ;I915_WRITE (LCPLL_CTL , val);
@@ -6058,6 +6062,8 @@ void hsw_restore_lcpll(struct drm_i915_private *dev_priv) LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))DRM_ERROR ("Switching back to LCPLL failed\n" ); } + +gen6_gt_force_wake_put (dev_priv); }static void haswell_modeset_global_resources (struct drm_device *dev)
Signature · 1 line
1.8.1.2
Re: [PATCH 1/6] drm/i915: grab force_wake when restoring LCPLL
Quoted 11 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
If LCPLL is disabled, there's a chance we might be in package C8 state or deeper, and we'll get a hard hang when restoring LCPLL (also, a red led lights up on my motherboard). So grab the
force_wake, which will get us out of RC6 and, as a consequence, out of PC8+ (since we need RC6 to get into PC8+).v2: Call the correct
force_wakeinterface.Signed-off-by Paulo Zanoni <[email protected]>
Hi Paulo. As I recall, the steps in the bspec are laid out exactly so we don't have to call forcewake. I think the explanation sounds a little fishy.
Specifically, I thought D_COMP COMP_FORCE + waiting for PLL lock was sufficient. If it's not, I worry you're papering over another bug, or incorrect bspec sequence.
Not opposed to the patch in the meanwhile, but it just feels a little unsafe to me.
Signature · 1 line
Ben Widawsky, Intel Open Source Technology Center
Re: [PATCH 1/6] drm/i915: grab force_wake when restoring LCPLL
2013/8/19 Ben Widawsky <[email protected]>:
Quoted 9 lines · Paulo Zanoni
Quoted 8 lines
From: Paulo Zanoni <[email protected]>
If LCPLL is disabled, there's a chance we might be in package C8 state or deeper, and we'll get a hard hang when restoring LCPLL (also, a red led lights up on my motherboard). So grab the
force_wake, which will get us out of RC6 and, as a consequence, out of PC8+ (since we need RC6 to get into PC8+).
Quoted 7 lines
Quoted 3 lines
v2: Call the correct
force_wakeinterface.Signed-off-by Paulo Zanoni <[email protected]>Hi Paulo. As I recall, the steps in the bspec are laid out exactly so we don't have to call forcewake. I think the explanation sounds a little fishy.
Well, we seem to do exactly what BSpec says we need to do and we still get the hard machine hang if we're in PC8 while restoring from LCPLL.
Quoted 3 lines
Specifically, I thought
D_COMPCOMP_FORCE+ waiting for PLL lock was sufficient.
Why? I don't see D_COMP documentation anywhere or any text that leads me to believe that. The spec just says "enable these bits, disable these others" without any explanations on what we're really doing.
Quoted 5 lines
If it's not, I worry you're papering over another bug, or incorrect bspec sequence.
Not opposed to the patch in the meanwhile, but it just feels a little unsafe to me.
I'll send some emails requesting for clarification. We certainly can't merge PC8 without this patch or an equivalent fix.
Quoted 3 lines
--
Ben Widawsky, Intel Open Source Technology Center
Signature · 1 line
Paulo Zanoni
PATCH 2/6 drm/i915: fix SDEIMR assertion when disabling LCPLL
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
This was causing WARNs in one machine, so instead of trying to guess exactly which hotplug bits should exist, just do the test on the non-HPD bits. We don't care about the state of the hotplug bits, we just care about the others, that need to be 1.
--- drivers/gpu/drm/i915/intel_display.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_display.c +2 -6
@@ -5930,11 +5930,7 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)struct intel_ddi_plls *plls = &dev_priv->ddi_plls;struct intel_crtc *crtc;unsigned long irqflags; -uint32_t val, pch_hpd_mask; - - pch_hpd_mask =SDE_PORTB_HOTPLUG_CPT |SDE_PORTC_HOTPLUG_CPT ; -if (!(dev_priv->pch_id ==INTEL_PCH_LPT_LP_DEVICE_ID_TYPE )) - pch_hpd_mask |=SDE_PORTD_HOTPLUG_CPT |SDE_CRT_HOTPLUG_CPT ; +uint32_t val;list_for_each_entry (crtc, &dev->mode_config.crtc_list, base.head) WARN(crtc->base.enabled, "CRTC for pipe %c enabled\n",
@@ -5960,7 +5956,7 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)WARN ((val & ~DE_PCH_EVENT_IVB ) != val,"Unexpected DEIMR bits enabled: 0x%x\n" , val); val =I915_READ (SDEIMR ); -WARN ((val & ~pch_hpd_mask) != val, +WARN ((val |SDE_HOTPLUG_MASK_CPT ) !=0xffffffff ,"Unexpected SDEIMR bits enabled: 0x%x\n" , val);spin_unlock_irqrestore (&dev_priv->irq_lock, irqflags); }
Signature · 1 line
1.8.1.2
Re: [PATCH 2/6] drm/i915: fix SDEIMR assertion when disabling LCPLL
ok, if we don't care about this hotplug bits state I liked the simplification.
Quoted 45 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
This was causing WARNs in one machine, so instead of trying to guess exactly which hotplug bits should exist, just do the test on the non-HPD bits. We don't care about the state of the hotplug bits, we just care about the others, that need to be 1.
Signed-off-by Paulo Zanoni <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/gpu/drm/i915/intel_display.c +2
-7@@ -5930,11 +5930,7 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)struct intel_ddi_plls *plls = &dev_priv->ddi_plls;struct intel_crtc *crtc;unsigned long irqflags; -uint32_t val, pch_hpd_mask; - - pch_hpd_mask =SDE_PORTB_HOTPLUG_CPT |SDE_PORTC_HOTPLUG_CPT ; -if (!(dev_priv->pch_id ==INTEL_PCH_LPT_LP_DEVICE_ID_TYPE )) - pch_hpd_mask |=SDE_PORTD_HOTPLUG_CPT |SDE_CRT_HOTPLUG_CPT ; +uint32_t val;list_for_each_entry (crtc, &dev->mode_config.crtc_list, base.head) WARN(crtc->base.enabled, "CRTC for pipe %c enabled\n",@@ -5960,7 +5956,7 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)WARN ((val & ~DE_PCH_EVENT_IVB ) != val,"Unexpected DEIMR bits enabled: 0x%x\n" , val); val =I915_READ (SDEIMR ); -WARN ((val & ~pch_hpd_mask) != val, +WARN ((val |SDE_HOTPLUG_MASK_CPT ) !=0xffffffff ,"Unexpected SDEIMR bits enabled: 0x%x\n" , val);spin_unlock_irqrestore (&dev_priv->irq_lock, irqflags); } --1.8.1.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Rodrigo Vivi Blog: http://blog.vivi.eng.br
PATCH 3/6 drm/i915: allow package C8+ states on Haswell (disabled)
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
This patch allows PC8+ states on Haswell. These states can only be reached when all the display outputs are disabled, and they allow some more power savings.
The fact that the graphics device is allowing PC8+ doesn't mean that the machine will actually enter PC8+: all the other devices also need to allow PC8+.
For now this option is disabled by default. You need i915.allow_pc8=1 if you want it.
This patch adds a big comment inside i915_drv.h explaining how it works and how it tracks things. Read it.
v2: (this is not really v2, many previous versions were already sent,
but they had different names)
- Use the new functions to enable/disable GTIMR and GEN6_PMIMR
- Rename almost all variables and functions to names suggested by Chris
- More WARNs on the IRQ handling code
- Also disable PC8 when there's GPU work to do (thanks to Ben for the help on this), so apps can run caster
- Enable PC8 on a delayed work function that is delayed for 5 seconds. This makes sure we only enable PC8+ if we're really idle
- Make sure we're not in PC8+ when suspending
v3: - WARN if IRQs are disabled on __wait_seqno
- Replace some DRM_ERRORs with WARNs
- Fix calls to restore GT and PM interrupts
- Use intel_mark_busy instead of intel_ring_advance to disable PC8
v4: - Use the force_wake, Luke!
v5: - Remove the "IIR is not zero" WARNs
- Move the force_wake chunk to its own patch
- Only restore what's missing from RC6, not everything
--- drivers/gpu/drm/i915/i915_dma.c | 10 +++ drivers/gpu/drm/i915/i915_drv.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 72 +++++++++++++++ drivers/gpu/drm/i915/i915_gem.c | 2 + drivers/gpu/drm/i915/i915_irq.c | 101 +++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 170 ++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_dp.c | 3 + drivers/gpu/drm/i915/intel_drv.h | 8 ++ drivers/gpu/drm/i915/intel_i2c.c | 2 + drivers/gpu/drm/i915/intel_pm.c | 13 ++- 10 files changed, 390 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_dma.c +10 -0
@@ -1486,6 +1486,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)mutex_init (&dev_priv->rps.hw_lock);mutex_init (&dev_priv->modeset_restore_lock); +mutex_init (&dev_priv->pc8.lock); + dev_priv->pc8.requirements_met = false; + dev_priv->pc8.gpu_idle = false; + dev_priv->pc8.irqs_disabled = false; + dev_priv->pc8.enabled = false; + dev_priv->pc8.disable_count =2 ;/* requirements_met + gpu_idle */ +INIT_DELAYED_WORK (&dev_priv->pc8.enable_work, hsw_enable_pc8_work); +i915_dump_device_info (dev_priv);/* Not all pre-production machines fall into this category, only the
@@ -1740,6 +1748,8 @@ int i915_driver_unload(struct drm_device *dev)cancel_work_sync (&dev_priv->gpu_error.work);i915_destroy_error_state (dev); +cancel_delayed_work_sync (&dev_priv->pc8.enable_work); +if (dev->pdev->msi_enabled)pci_disable_msi (dev->pdev);
drivers/gpu/drm/i915/i915_drv.c +11 -0
@@ -141,6 +141,10 @@ module_param_named(fastboot, i915_fastboot, bool, 0600);MODULE_PARM_DESC (fastboot,"Try to skip unnecessary mode sets at boot time " "(default: false)" ); +int i915_enable_pc8__read_mostly =0 ; +module_param_named (enable_pc8, i915_enable_pc8, int,0600 ); +MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: false)" ); +bool i915_prefault_disable__read_mostly ;module_param_named (prefault_disable, i915_prefault_disable, bool,0600 ); MODULE_PARM_DESC(prefault_disable,
@@ -557,6 +561,9 @@ static int i915_drm_freeze(struct drm_device *dev) dev_priv->modeset_restore =MODESET_SUSPENDED ;mutex_unlock (&dev_priv->modeset_restore_lock); +/* We do a lot of poking in a lot of registers, make sure they work +* properly. */ +hsw_disable_package_c8 (dev_priv);intel_set_power_well (dev, true);drm_kms_helper_poll_disable (dev);
@@ -713,6 +720,10 @@ static int __i915_drm_thaw(struct drm_device *dev)schedule_work (&dev_priv->console_resume_work); } +/* Undo what we did at i915_drm_freeze so the refcount goes back to the +* expected level. */ +hsw_enable_package_c8 (dev_priv); +mutex_lock (&dev_priv->modeset_restore_lock); dev_priv->modeset_restore =MODESET_DONE ;mutex_unlock (&dev_priv->modeset_restore_lock);
drivers/gpu/drm/i915/i915_drv.h +72 -0
90 lines not on this page · raw message
drivers/gpu/drm/i915/i915_gem.c +2 -0
@@ -999,6 +999,8 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,bool wait_forever = true;int ret; +WARN (dev_priv->pc8.irqs_disabled,"IRQs disabled\n" ); +if (i915_seqno_passed (ring->get_seqno (ring, true), seqno))return 0 ;
drivers/gpu/drm/i915/i915_irq.c +101 -0
134 lines not on this page · raw message
drivers/gpu/drm/i915/intel_display.c +169 -1
194 lines not on this page · raw message
drivers/gpu/drm/i915/intel_dp.c +3 -0
@@ -344,6 +344,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,else precharge =5 ; +intel_aux_display_runtime_get (dev_priv); +/* Try to wait for any previous AUX channel activity */ for (try =0 ; try <3 ; try++) { status =I915_READ_NOTRACE (ch_ctl);
@@ -434,6 +436,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, ret = recv_bytes; out:pm_qos_update_request (&dev_priv->pm_qos,PM_QOS_DEFAULT_VALUE ); +intel_aux_display_runtime_put (dev_priv);return ret; }
drivers/gpu/drm/i915/intel_drv.h +8 -0
@@ -745,6 +745,7 @@ extern void intel_set_power_well(struct drm_device *dev, bool enable);extern void intel_enable_gt_powersave (struct drm_device *dev);extern void intel_disable_gt_powersave (struct drm_device *dev);extern void ironlake_teardown_rc6 (struct drm_device *dev); +void gen6_update_ring_freq (struct drm_device *dev);extern bool intel_ddi_get_hw_state (struct intel_encoder *encoder,enum pipe *pipe);
@@ -784,5 +785,12 @@ extern void ilk_disable_gt_irq(struct drm_i915_private *dev_priv,extern void snb_enable_pm_irq (struct drm_i915_private *dev_priv,uint32_t mask);extern void snb_disable_pm_irq (struct drm_i915_private *dev_priv,uint32_t mask); +extern void hsw_enable_pc8_work (struct work_struct *__work); +extern void hsw_enable_package_c8 (struct drm_i915_private *dev_priv); +extern void hsw_disable_package_c8 (struct drm_i915_private *dev_priv); +extern void hsw_pc8_disable_interrupts (struct drm_device *dev); +extern void hsw_pc8_restore_interrupts (struct drm_device *dev); +extern void intel_aux_display_runtime_get (struct drm_i915_private *dev_priv); +extern void intel_aux_display_runtime_put (struct drm_i915_private *dev_priv);#endif /* __INTEL_DRV_H__ */
drivers/gpu/drm/i915/intel_i2c.c +2 -0
@@ -398,6 +398,7 @@ gmbus_xfer(struct i2c_adapter *adapter,int i, reg_offset;int ret =0 ; +intel_aux_display_runtime_get (dev_priv);mutex_lock (&dev_priv->gmbus_mutex);if (bus->force_bit) {
@@ -497,6 +498,7 @@ timeout: out:mutex_unlock (&dev_priv->gmbus_mutex); +intel_aux_display_runtime_put (dev_priv);return ret; }
drivers/gpu/drm/i915/intel_pm.c +12 -1
@@ -3607,7 +3607,7 @@ static void gen6_enable_rps(struct drm_device *dev)gen6_gt_force_wake_put (dev_priv); } -static void gen6_update_ring_freq (struct drm_device *dev) +void gen6_update_ring_freq (struct drm_device *dev) {struct drm_i915_private *dev_priv = dev->dev_private;int min_freq =15 ;
@@ -5433,6 +5433,17 @@ void intel_init_power_well(struct drm_device *dev)I915_WRITE (HSW_PWR_WELL_BIOS ,0 ); } +/* Disables PC8 so we can use the GMBUS and DP AUX interrupts. */ +void intel_aux_display_runtime_get (struct drm_i915_private *dev_priv) +{ +hsw_disable_package_c8 (dev_priv); +} + +void intel_aux_display_runtime_put (struct drm_i915_private *dev_priv) +{ +hsw_enable_package_c8 (dev_priv); +} +/* Set up chip specific power management-related functions */ void intel_init_pm (struct drm_device *dev) {
Signature · 1 line
1.8.1.2
Re: [PATCH 3/6] drm/i915: allow package C8+ states on Haswell (disabled)
Quoted 37 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
This patch allows PC8+ states on Haswell. These states can only be reached when all the display outputs are disabled, and they allow some more power savings.
The fact that the graphics device is allowing PC8+ doesn't mean that the machine will actually enter PC8+: all the other devices also need to allow PC8+.
For now this option is disabled by default. You need i915.allow_pc8=1 if you want it.
This patch adds a big comment inside
i915_drv.hexplaining how it works and how it tracks things. Read it.v2: (this is not really v2, many previous versions were already sent,
but they had different names)
- Use the new functions to enable/disable GTIMR andGEN6_PMIMR
- Rename almost all variables and functions to names suggested by Chris
- More WARNs on the IRQ handling code
- Also disable PC8 when there's GPU work to do (thanks to Ben for the help on this), so apps can run caster
- Enable PC8 on a delayed work function that is delayed for 5 seconds. This makes sure we only enable PC8+ if we're really idle
- Make sure we're not in PC8+ when suspending
v3: - WARN if IRQs are disabled on__wait_seqno
- Replace someDRM_ERRORswith WARNs
- Fix calls to restore GT and PM interrupts
- Useintel_mark_busyinstead ofintel_ring_advanceto disable PC8
v4: - Use theforce_wake, Luke!
v5: - Remove the "IIR is not zero" WARNs
- Move theforce_wakechunk to its own patch
- Only restore what's missing from RC6, not everything
Quoted 1 line
Signed-off-by Paulo Zanoni <[email protected]>
I think this is a nice looking patch now, and I think I understand what you need from this patch alone, so
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
PATCH 4/6 drm/i915: add i915_pc8_status debugfs file
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
Make it print the value of the variables on the PC8 struct.
v2: Update to recent renames and add the new fields.
--- drivers/gpu/drm/i915/i915_debugfs.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
drivers/gpu/drm/i915/i915_debugfs.c +25 -0
@@ -1769,6 +1769,30 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)return 0 ; } +static int i915_pc8_status (struct seq_file *m,void *unused) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; + +if (!IS_HASWELL (dev)) { +seq_puts (m,"not supported\n" ); +return 0 ; + } + +mutex_lock (&dev_priv->pc8.lock); +seq_printf (m,"Requirements met: %s\n" , +yesno (dev_priv->pc8.requirements_met)); +seq_printf (m,"GPU idle: %s\n" ,yesno (dev_priv->pc8.gpu_idle)); +seq_printf (m,"Disable count: %d\n" , dev_priv->pc8.disable_count); +seq_printf (m,"IRQs disabled: %s\n" , +yesno (dev_priv->pc8.irqs_disabled)); +seq_printf (m,"Enabled: %s\n" ,yesno (dev_priv->pc8.enabled)); +mutex_unlock (&dev_priv->pc8.lock); + +return 0 ; +} +static int i915_wedged_get (void *data,u64 *val) {
@@ -2208,6 +2232,7 @@ static struct drm_info_list i915_debugfs_list[] = { {"i915_dpio", i915_dpio_info, 0}, {"i915_llc", i915_llc, 0}, {"i915_edp_psr_status", i915_edp_psr_status, 0}, + {"i915_pc8_status", i915_pc8_status, 0}, };#define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
Signature · 1 line
1.8.1.2
Re: [PATCH 4/6] drm/i915: add i915_pc8_status debugfs file
I haven't reviewed the patches that add pc8 itself, but I can assume it is correct since it was already rv-b Chris.
So:
Quoted 61 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
Make it print the value of the variables on the PC8 struct.
v2: Update to recent renames and add the new fields.
Signed-off-by Paulo Zanoni <[email protected]>--- drivers/gpu/drm/i915/i915_debugfs.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)drivers/gpu/drm/i915/i915_debugfs.c +25
-1@@ -1769,6 +1769,30 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)return 0 ; } +static int i915_pc8_status (struct seq_file *m,void *unused) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; + +if (!IS_HASWELL (dev)) { +seq_puts (m,"not supported\n" ); +return 0 ; + } + +mutex_lock (&dev_priv->pc8.lock); +seq_printf (m,"Requirements met: %s\n" , +yesno (dev_priv->pc8.requirements_met)); +seq_printf (m,"GPU idle: %s\n" ,yesno (dev_priv->pc8.gpu_idle)); +seq_printf (m,"Disable count: %d\n" , dev_priv->pc8.disable_count); +seq_printf (m,"IRQs disabled: %s\n" , +yesno (dev_priv->pc8.irqs_disabled)); +seq_printf (m,"Enabled: %s\n" ,yesno (dev_priv->pc8.enabled)); +mutex_unlock (&dev_priv->pc8.lock); + +return 0 ; +} +static int i915_wedged_get (void *data,u64 *val) {@@ -2208,6 +2232,7 @@ static struct drm_info_list i915_debugfs_list[] = { {"i915_dpio", i915_dpio_info, 0}, {"i915_llc", i915_llc, 0}, {"i915_edp_psr_status", i915_edp_psr_status, 0}, + {"i915_pc8_status", i915_pc8_status, 0}, };#define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list) --1.8.1.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Rodrigo Vivi Blog: http://blog.vivi.eng.br
PATCH 5/6 drm/i915: add i915.pc8_timeout function
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
We currently only enter PC8+ after all its required conditions are met, there's no rendering, and we stay like that for at least 5 seconds.
I chose "5 seconds" because this value is conservative and won't make us enter/leave PC8+ thousands of times after the screen is off: some desktop environments have applications that wake up and do rendering every 1-3 seconds, even when the screen is off and the machine is completely idle.
But when I was testing my PC8+ patches I set the default value to 100ms so I could use the bad-behaving desktop environments to stress-test my patches. I also thought it would be a good idea to ask our power management team to test different values, but I'm pretty sure they would ask me for an easy way to change the timeout. So to help these 2 cases I decided to create an option that would make it easier to change the default value. I also expect people making specific products that use our driver could try to find the perfect timeout for them.
Anyway, fixing the bad-behaving applications will always lead to better power savings than just changing the timeout value: you need to stop waking the Kernel, not quickly put it back to sleep again after you wake it for nothing. Bad sleep leads to bad mood!
--- drivers/gpu/drm/i915/i915_drv.c | 4 ++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.c +4 -0
@@ -145,6 +145,10 @@ int i915_enable_pc8 __read_mostly = 0;module_param_named (enable_pc8, i915_enable_pc8, int,0600 );MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: false)" ); +int i915_pc8_timeout__read_mostly =5000 ; +module_param_named (pc8_timeout, i915_pc8_timeout, int,0600 ); +MODULE_PARM_DESC (pc8_timeout,"Number of msecs of idleness required to enter PC8+ (default: 5000)" ); +bool i915_prefault_disable__read_mostly ;module_param_named (prefault_disable, i915_prefault_disable, bool,0600 ); MODULE_PARM_DESC(prefault_disable,
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -1710,6 +1710,7 @@ extern int i915_disable_power_well __read_mostly;extern int i915_enable_ips__read_mostly ;extern bool i915_fastboot__read_mostly ;extern int i915_enable_pc8__read_mostly ; +extern int i915_pc8_timeout__read_mostly ;extern bool i915_prefault_disable__read_mostly ;extern int i915_suspend (struct drm_device *dev,pm_message_t state);
drivers/gpu/drm/i915/intel_display.c +1 -1
@@ -6099,7 +6099,7 @@ static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)return ;schedule_delayed_work (&dev_priv->pc8.enable_work, -msecs_to_jiffies (5 *1000 )); +msecs_to_jiffies (i915_pc8_timeout)); }static void __hsw_disable_package_c8 (struct drm_i915_private *dev_priv)
Signature · 1 line
1.8.1.2
Re: [PATCH 5/6] drm/i915: add i915.pc8_timeout function
Quoted 81 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
We currently only enter PC8+ after all its required conditions are met, there's no rendering, and we stay like that for at least 5 seconds.
I chose "5 seconds" because this value is conservative and won't make us enter/leave PC8+ thousands of times after the screen is off: some desktop environments have applications that wake up and do rendering every 1-3 seconds, even when the screen is off and the machine is completely idle.
But when I was testing my PC8+ patches I set the default value to 100ms so I could use the bad-behaving desktop environments to stress-test my patches. I also thought it would be a good idea to ask our power management team to test different values, but I'm pretty sure they would ask me for an easy way to change the timeout. So to help these 2 cases I decided to create an option that would make it easier to change the default value. I also expect people making specific products that use our driver could try to find the perfect timeout for them.
Anyway, fixing the bad-behaving applications will always lead to better power savings than just changing the timeout value: you need to stop waking the Kernel, not quickly put it back to sleep again after you wake it for nothing. Bad sleep leads to bad mood!
Signed-off-by Paulo Zanoni <[email protected]>--- drivers/gpu/drm/i915/i915_drv.c | 4 ++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/i915_drv.c +4
-0@@ -145,6 +145,10 @@ int i915_enable_pc8 __read_mostly = 0;module_param_named (enable_pc8, i915_enable_pc8, int,0600 );MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: false)" ); +int i915_pc8_timeout__read_mostly =5000 ; +module_param_named (pc8_timeout, i915_pc8_timeout, int,0600 ); +MODULE_PARM_DESC (pc8_timeout,"Number of msecs of idleness required to enter PC8+ (default: 5000)" ); +bool i915_prefault_disable__read_mostly ;module_param_named (prefault_disable, i915_prefault_disable, bool,0600 ); MODULE_PARM_DESC(prefault_disable,drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -1710,6 +1710,7 @@ extern int i915_disable_power_well __read_mostly;extern int i915_enable_ips__read_mostly ;extern bool i915_fastboot__read_mostly ;extern int i915_enable_pc8__read_mostly ; +extern int i915_pc8_timeout__read_mostly ;extern bool i915_prefault_disable__read_mostly ;extern int i915_suspend (struct drm_device *dev,pm_message_t state);drivers/gpu/drm/i915/intel_display.c +1
-2@@ -6099,7 +6099,7 @@ static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)return ;schedule_delayed_work (&dev_priv->pc8.enable_work, -msecs_to_jiffies (5 *1000 )); +msecs_to_jiffies (i915_pc8_timeout)); }static void __hsw_disable_package_c8 (struct drm_i915_private *dev_priv) --1.8.1.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Rodrigo Vivi Blog: http://blog.vivi.eng.br
PATCH 6/6 drm/i915: enable Package C8+ by default
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
This should be working, so enable it by default. Also easy to revert.
v2: Rebase, s/allow/enable/.
--- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_drv.c +2 -2
@@ -141,9 +141,9 @@ module_param_named(fastboot, i915_fastboot, bool, 0600);MODULE_PARM_DESC (fastboot,"Try to skip unnecessary mode sets at boot time " "(default: false)" ); -int i915_enable_pc8__read_mostly =0 ; +int i915_enable_pc8__read_mostly =1 ;module_param_named (enable_pc8, i915_enable_pc8, int,0600 ); -MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: false)" ); +MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: true)" );int i915_pc8_timeout__read_mostly =5000 ;module_param_named (pc8_timeout, i915_pc8_timeout, int,0600 );
Signature · 1 line
1.8.1.2
Re: [PATCH 6/6] drm/i915: enable Package C8+ by default
Quoted 34 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
This should be working, so enable it by default. Also easy to revert.
v2: Rebase, s/allow/enable/.
Signed-off-by Paulo Zanoni <[email protected]>--- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/i915/i915_drv.c +2
-3@@ -141,9 +141,9 @@ module_param_named(fastboot, i915_fastboot, bool, 0600);MODULE_PARM_DESC (fastboot,"Try to skip unnecessary mode sets at boot time " "(default: false)" ); -int i915_enable_pc8__read_mostly =0 ; +int i915_enable_pc8__read_mostly =1 ;module_param_named (enable_pc8, i915_enable_pc8, int,0600 ); -MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: false)" ); +MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: true)" );int i915_pc8_timeout__read_mostly =5000 ;module_param_named (pc8_timeout, i915_pc8_timeout, int,0600 ); --1.8.1.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Rodrigo Vivi Blog: http://blog.vivi.eng.br
Re: [PATCH 6/6] drm/i915: enable Package C8+ by default
Quoted 10 lines · Rodrigo Vivi
Reviewed-by Rodrigo Vivi <[email protected]>Quoted 7 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
This should be working, so enable it by default. Also easy to revert.
v2: Rebase, s/allow/enable/.
Signed-off-by Paulo Zanoni <[email protected]>
Series merged to dinq with a small note for the first patch that the discussion with hw guys is still ongoing.
-Daniel
Quoted 37 lines
Quoted 27 lines
--- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/i915/i915_drv.c +2
-3@@ -141,9 +141,9 @@ module_param_named(fastboot, i915_fastboot, bool, 0600);MODULE_PARM_DESC (fastboot,"Try to skip unnecessary mode sets at boot time " "(default: false)" ); -int i915_enable_pc8__read_mostly =0 ; +int i915_enable_pc8__read_mostly =1 ;module_param_named (enable_pc8, i915_enable_pc8, int,0600 ); -MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: false)" ); +MODULE_PARM_DESC (enable_pc8,"Enable support for low power package C states (PC8+) (default: true)" );int i915_pc8_timeout__read_mostly =5000 ;module_param_named (pc8_timeout, i915_pc8_timeout, int,0600 ); --1.8.1.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfxSignature · 6 lines
Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
PATCH 1/1 tests: add pc8
In reply to Paulo ZanoniFrom: Paulo Zanoni <[email protected]>
This test chekcs our code that enables Package C8+. The environment requirements for this test are quite complicated:
- The machine needs to be properly configured to reach PC8+ when possible, which means all the power management policies and device drivers must be properly configured.
- You need at least one output connected.
- You need the /dev/cpu/0/msr file available.
- You need the /dev/i2c-* files available.
v2: - Many many changes due to the latest review comments and rebase.
--- tests/.gitignore | 1 + tests/Makefile.am | 1 + tests/pc8.c | 804 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 806 insertions(+) create mode 100644 tests/pc8.c
tests/.gitignore +1 -0
@@ -93,6 +93,7 @@ getversion kms_flip kms_render kms_setmode +pc8 prime_nv_api prime_nv_pcopy prime_nv_test
tests/Makefile.am +1 -0
@@ -45,6 +45,7 @@ TESTS_progs_M = \ kms_render \ kms_setmode \ $(NOUVEAU_TESTS_M) \ + pc8 \ prime_self_import \ $(NULL)
tests/pc8.c new file +804 -0
804 lines not on this page · raw message
Signature · 6 lines
1.8.1.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH igt] tests: add pc8
Quoted 11 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
This test chekcs our code that enables Package C8+. The environment requirements for this test are quite complicated:
- The machine needs to be properly configured to reach PC8+ when possible, which means all the power management policies and device drivers must be properly configured.
- You need at least one output connected.
- You need the/dev/cpu/0/msrfile available.
- You need the/dev/i2c-*files available.
Quoted 3 lines
v2: - Many many changes due to the latest review comments and rebase.
Signed-off-by Paulo Zanoni <[email protected]>
Applied and pushed with a small fixup patch on top to convert your new testcase to the latest igt infrastructure changes.
Thanks, Daniel
Quoted 848 lines
--- tests/.gitignore | 1 + tests/Makefile.am | 1 + tests/pc8.c | 804 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 806 insertions(+) create mode 100644 tests/pc8.ctests/.gitignore +1
-0@@ -93,6 +93,7 @@ getversion kms_flip kms_render kms_setmode +pc8 prime_nv_api prime_nv_pcopy prime_nv_testtests/Makefile.am +1
-0@@ -45,6 +45,7 @@ TESTS_progs_M = \ kms_render \ kms_setmode \ $(NOUVEAU_TESTS_M) \ + pc8 \ prime_self_import \ $(NULL)tests/pc8.c new file +804
-0@@ -0,0 +1,804 @@ +/* +* Copyright © 2013 Intel Corporation +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +* +* Authors: +* Paulo Zanoni <[email protected]> +* +*/ + +#include <stdio.h> +#include <stdint.h> +#include <stdbool.h> +#include <string.h> + +#include <unistd.h> +#include <fcntl.h> +#include <dirent.h> +#include <sys/ioctl.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <linux/i2c.h> +#include <linux/i2c-dev.h> + +#include "drm.h" +#include "drmtest.h" +#include "intel_batchbuffer.h" +#include "intel_gpu_tools.h" +#include "i915_drm.h" + +#define MSR_PC8_RES 0x630 +#define MSR_PC9_RES 0x631 +#define MSR_PC10_RES 0x632 + +#define MAX_CONNECTORS 32 +#define MAX_ENCODERS 32 +#define MAX_CRTCS 16 + +int drm_fd, msr_fd; +struct mode_set_data ms_data; + +/* Stuff used when creating FBs and mode setting. */ +struct mode_set_data { +drmModeResPtr res; +drmModeConnectorPtr connectors[MAX_CONNECTORS ]; +drmModePropertyBlobPtr edids[MAX_CONNECTORS ]; + +drm_intel_bufmgr *bufmgr; +uint32_t devid; +}; + +/* Stuff we query at different times so we can compare. */ +struct compare_data { +drmModeResPtr res; +drmModeEncoderPtr encoders[MAX_ENCODERS ]; +drmModeConnectorPtr connectors[MAX_CONNECTORS ]; +drmModeCrtcPtr crtcs[MAX_CRTCS ]; +drmModePropertyBlobPtr edids[MAX_CONNECTORS ]; +}; + +struct compare_registers { +/* We know these are lost */ +uint32_t arb_mode; +uint32_t tilectl; + +/* Stuff touched at init_clock_gating, so we can make sure we +* don't need to call it when reiniting. */ +uint32_t gen6_ucgctl2; +uint32_t gen7_l3cntlreg1; +uint32_t transa_chicken1; + +uint32_t deier; +uint32_t gtier; + +uint32_t ddi_buf_trans_a_1; +uint32_t ddi_buf_trans_b_5; +uint32_t ddi_buf_trans_c_10; +uint32_t ddi_buf_trans_d_15; +uint32_t ddi_buf_trans_e_20; +}; + +/* If the read fails, then the machine doesn't support PC8+ residencies. */ +static bool supports_pc8_plus_residencies (void ) +{ +int rc; +uint64_t val; + + rc =pread (msr_fd, &val,sizeof (uint64_t ),MSR_PC8_RES ); +if (!rc ==sizeof (val)) +return false; + rc =pread (msr_fd, &val,sizeof (uint64_t ),MSR_PC9_RES ); +if (!rc ==sizeof (val)) +return false; + rc =pread (msr_fd, &val,sizeof (uint64_t ),MSR_PC10_RES ); +if (!rc ==sizeof (val)) +return false; + +return true; +} + +static uint64_t get_residency (uint32_t type) +{ +int rc; +uint64_t ret; + + rc =pread (msr_fd, &ret,sizeof (uint64_t ), type); +igt_assert (rc ==sizeof (ret)); + +return ret; +} + +static bool pc8_plus_residency_changed (unsigned int timeout_sec) +{ +unsigned int i; +uint64_t res_pc8, res_pc9, res_pc10; +int to_sleep =100 *1000 ; + + res_pc8 =get_residency (MSR_PC8_RES ); + res_pc9 =get_residency (MSR_PC9_RES ); + res_pc10 =get_residency (MSR_PC10_RES ); + +for (i =0 ; i < timeout_sec *1000 *1000 ; i += to_sleep) { +if (res_pc8 !=get_residency (MSR_PC8_RES ) || + res_pc9 !=get_residency (MSR_PC9_RES ) || + res_pc10 !=get_residency (MSR_PC10_RES )) { +return true; + } +usleep (to_sleep); + } + +return false; +} + +/* Checks not only if PC8+ is allowed, but also if we're reaching it. +* We call this when we expect this function to return quickly since PC8 is +* actually enabled, so the 30s timeout we use shouldn't matter. */ +static bool pc8_plus_enabled (void ) +{ +return pc8_plus_residency_changed (30 ); +} + +/* We call this when we expect PC8+ to be actually disabled, so we should not +* return until the 5s timeout expires. In other words: in the "happy case", +* every time we call this function the program will take 5s more to finish. */ +static bool pc8_plus_disabled (void ) +{ +return !pc8_plus_residency_changed (5 ); +} + +static void disable_all_screens (struct mode_set_data *data) +{ +int i, rc; + +for (i =0 ; i < data->res->count_crtcs; i++) { + rc =drmModeSetCrtc (drm_fd, data->res->crtcs[i],-1 ,0 ,0 , +NULL ,0 ,NULL ); +igt_assert (rc ==0 ); + } +} + +static uint32_t create_fb (struct mode_set_data *data,int width,int height) +{ +struct kmstest_fb fb; +cairo_t *cr; +uint32_t buffer_id; + + buffer_id =kmstest_create_fb (drm_fd, width, height,32 ,24 , false, + &fb); + cr =kmstest_get_cairo_ctx (drm_fd, &fb); +kmstest_paint_test_pattern (cr, width, height); +return buffer_id; +} + +static void enable_one_screen (struct mode_set_data *data) +{ +uint32_t crtc_id =0 , buffer_id =0 , connector_id =0 ; +drmModeModeInfoPtr mode =NULL ; +int i, rc; + +for (i =0 ; i < data->res->count_connectors; i++) { +drmModeConnectorPtr c = data->connectors[i]; + +if (c->connection ==DRM_MODE_CONNECTED && c->count_modes) { + connector_id = c->connector_id; + mode = &c->modes[0 ]; +break ; + } + } + + crtc_id = data->res->crtcs[0 ]; + buffer_id =create_fb (data, mode->hdisplay, mode->vdisplay); + +igt_assert (crtc_id); +igt_assert (buffer_id); +igt_assert (connector_id); +igt_assert (mode); + + rc =drmModeSetCrtc (drm_fd, crtc_id, buffer_id,0 ,0 , &connector_id, +1 , mode); +igt_assert (rc ==0 ); +} + +static drmModePropertyBlobPtr get_connector_edid (drmModeConnectorPtr connector, +int index) +{ +unsigned int i; +drmModeObjectPropertiesPtr props; +drmModePropertyBlobPtr ret =NULL ; + + props =drmModeObjectGetProperties (drm_fd, connector->connector_id, +DRM_MODE_OBJECT_CONNECTOR ); + +for (i =0 ; i < props->count_props; i++) { +drmModePropertyPtr prop =drmModeGetProperty (drm_fd, + props->props[i]); + +if (strcmp (prop->name,"EDID" ) ==0 ) { +igt_assert (prop->flags &DRM_MODE_PROP_BLOB ); +igt_assert (prop->count_blobs ==0 ); + ret =drmModeGetPropertyBlob (drm_fd, + props->prop_values[i]); + } + +drmModeFreeProperty (prop); + } + +drmModeFreeObjectProperties (props); +return ret; +} + +static void init_mode_set_data (struct mode_set_data *data) +{ +int i; + + data->res =drmModeGetResources (drm_fd); +igt_assert (data->res); +igt_assert (data->res->count_connectors <=MAX_CONNECTORS ); + +for (i =0 ; i < data->res->count_connectors; i++) { + data->connectors[i] =drmModeGetConnector (drm_fd, + data->res->connectors[i]); + data->edids[i] =get_connector_edid (data->connectors[i], i); + } + + data->bufmgr =drm_intel_bufmgr_gem_init (drm_fd,4096 ); + data->devid =intel_get_drm_devid (drm_fd); + +do_or_die (igt_set_vt_graphics_mode ()); +drm_intel_bufmgr_gem_enable_reuse (data->bufmgr); +} + +static void fini_mode_set_data (struct mode_set_data *data) +{ +int i; + +drm_intel_bufmgr_destroy (data->bufmgr); + +for (i =0 ; i < data->res->count_connectors; i++) { +drmModeFreeConnector (data->connectors[i]); +drmModeFreePropertyBlob (data->edids[i]); + } +drmModeFreeResources (data->res); +} + +static void get_drm_info (struct compare_data *data) +{ +int i; + + data->res =drmModeGetResources (drm_fd); +igt_assert (data->res); + +igt_assert (data->res->count_connectors <=MAX_CONNECTORS ); +igt_assert (data->res->count_encoders <=MAX_ENCODERS ); +igt_assert (data->res->count_crtcs <=MAX_CRTCS ); + +for (i =0 ; i < data->res->count_connectors; i++) { + data->connectors[i] =drmModeGetConnector (drm_fd, + data->res->connectors[i]); + data->edids[i] =get_connector_edid (data->connectors[i], i); + } +for (i =0 ; i < data->res->count_encoders; i++) + data->encoders[i] =drmModeGetEncoder (drm_fd, + data->res->encoders[i]); +for (i =0 ; i < data->res->count_crtcs; i++) + data->crtcs[i] =drmModeGetCrtc (drm_fd, data->res->crtcs[i]); +} + +static void get_registers (struct compare_registers *data) +{ +intel_register_access_init (intel_get_pci_device (),0 ); + data->arb_mode =INREG (0x4030 ); + data->tilectl =INREG (0x101000 ); + data->gen6_ucgctl2 =INREG (0x9404 ); + data->gen7_l3cntlreg1 =INREG (0xB0C1 ); + data->transa_chicken1 =INREG (0xF0060 ); + data->deier =INREG (0x4400C ); + data->gtier =INREG (0x4401C ); + data->ddi_buf_trans_a_1 =INREG (0x64E00 ); + data->ddi_buf_trans_b_5 =INREG (0x64E70 ); + data->ddi_buf_trans_c_10 =INREG (0x64EE0 ); + data->ddi_buf_trans_d_15 =INREG (0x64F58 ); + data->ddi_buf_trans_e_20 =INREG (0x64FCC ); +intel_register_access_fini (); +} + +static void free_drm_info (struct compare_data *data) +{ +int i; + +for (i =0 ; i < data->res->count_connectors; i++) { +drmModeFreeConnector (data->connectors[i]); +drmModeFreePropertyBlob (data->edids[i]); + } +for (i =0 ; i < data->res->count_encoders; i++) +drmModeFreeEncoder (data->encoders[i]); +for (i =0 ; i < data->res->count_crtcs; i++) +drmModeFreeCrtc (data->crtcs[i]); + +drmModeFreeResources (data->res); +} + +#define COMPARE (d1, d2, data) igt_assert(d1->data == d2->data) +#define COMPARE_ARRAY (d1, d2, size, data) do { \ + for (i = 0; i < size; i++) \ + igt_assert(d1->data[i] == d2->data[i]); \ +} while (0) + +static void assert_drm_resources_equal (struct compare_data *d1, +struct compare_data *d2) +{ +COMPARE (d1, d2, res->count_connectors); +COMPARE (d1, d2, res->count_encoders); +COMPARE (d1, d2, res->count_crtcs); +COMPARE (d1, d2, res->min_width); +COMPARE (d1, d2, res->max_width); +COMPARE (d1, d2, res->min_height); +COMPARE (d1, d2, res->max_height); +} + +static void assert_modes_equal (drmModeModeInfoPtr m1,drmModeModeInfoPtr m2) +{ +COMPARE (m1, m2, clock); +COMPARE (m1, m2, hdisplay); +COMPARE (m1, m2, hsync_start); +COMPARE (m1, m2, hsync_end); +COMPARE (m1, m2, htotal); +COMPARE (m1, m2, hskew); +COMPARE (m1, m2, vdisplay); +COMPARE (m1, m2, vsync_start); +COMPARE (m1, m2, vsync_end); +COMPARE (m1, m2, vtotal); +COMPARE (m1, m2, vscan); +COMPARE (m1, m2, vrefresh); +COMPARE (m1, m2, flags); +COMPARE (m1, m2, type); +igt_assert (strcmp (m1->name, m2->name) ==0 ); +} + +static void assert_drm_connectors_equal (drmModeConnectorPtr c1, +drmModeConnectorPtr c2) +{ +int i; + +COMPARE (c1, c2, connector_id); +COMPARE (c1, c2, connector_type); +COMPARE (c1, c2, connector_type_id); +COMPARE (c1, c2, mmWidth); +COMPARE (c1, c2, mmHeight); +COMPARE (c1, c2, count_modes); +COMPARE (c1, c2, count_props); +COMPARE (c1, c2, count_encoders); +COMPARE_ARRAY (c1, c2, c1->count_props, props); +COMPARE_ARRAY (c1, c2, c1->count_encoders, encoders); + +for (i =0 ; i < c1->count_modes; i++) +assert_modes_equal (&c1->modes[0 ], &c2->modes[0 ]); +} + +static void assert_drm_encoders_equal (drmModeEncoderPtr e1, +drmModeEncoderPtr e2) +{ +COMPARE (e1, e2, encoder_id); +COMPARE (e1, e2, encoder_type); +COMPARE (e1, e2, possible_crtcs); +COMPARE (e1, e2, possible_clones); +} + +static void assert_drm_crtcs_equal (drmModeCrtcPtr c1,drmModeCrtcPtr c2) +{ +COMPARE (c1, c2, crtc_id); +} + +static void assert_drm_edids_equal (drmModePropertyBlobPtr e1, +drmModePropertyBlobPtr e2) +{ +if (!e1 && !e2) +return ; +igt_assert (e1 && e2); + +COMPARE (e1, e2, id); +COMPARE (e1, e2, length); + +igt_assert (memcmp (e1->data, e2->data, e1->length) ==0 ); +} + +static void compare_registers (struct compare_registers *d1, +struct compare_registers *d2) +{ +COMPARE (d1, d2, gen6_ucgctl2); +COMPARE (d1, d2, gen7_l3cntlreg1); +COMPARE (d1, d2, transa_chicken1); +COMPARE (d1, d2, arb_mode); +COMPARE (d1, d2, tilectl); +COMPARE (d1, d2, arb_mode); +COMPARE (d1, d2, tilectl); +COMPARE (d1, d2, gtier); +COMPARE (d1, d2, ddi_buf_trans_a_1); +COMPARE (d1, d2, ddi_buf_trans_b_5); +COMPARE (d1, d2, ddi_buf_trans_c_10); +COMPARE (d1, d2, ddi_buf_trans_d_15); +COMPARE (d1, d2, ddi_buf_trans_e_20); +} + +static void assert_drm_infos_equal (struct compare_data *d1, +struct compare_data *d2) +{ +int i; + +assert_drm_resources_equal (d1, d2); + +for (i =0 ; i < d1->res->count_connectors; i++) { +assert_drm_connectors_equal (d1->connectors[i], + d2->connectors[i]); +assert_drm_edids_equal (d1->edids[i], d2->edids[i]); + } + +for (i =0 ; i < d1->res->count_encoders; i++) +assert_drm_encoders_equal (d1->encoders[i], d2->encoders[i]); + +for (i =0 ; i < d1->res->count_crtcs; i++) +assert_drm_crtcs_equal (d1->crtcs[i], d2->crtcs[i]); +} + +static void blt_color_fill (struct intel_batchbuffer *batch, +drm_intel_bo *buf, +const unsigned int pages) +{ +const unsigned short height = pages/4 ; +const unsigned short width =4096 ; + +BEGIN_BATCH (5 ); +OUT_BATCH (COLOR_BLT_CMD |COLOR_BLT_WRITE_ALPHA |COLOR_BLT_WRITE_RGB ); +OUT_BATCH ((3 <<24 ) |/* 32 Bit Color */ +0xF0 |/* Raster OP copy background register */ +0 );/* Dest pitch is 0 */ +OUT_BATCH (width <<16 | height); +OUT_RELOC (buf,I915_GEM_DOMAIN_RENDER ,I915_GEM_DOMAIN_RENDER ,0 ); +OUT_BATCH (rand ());/* random pattern */ +ADVANCE_BATCH (); +} + +static void test_batch (struct mode_set_data *data) +{ +struct intel_batchbuffer *batch; +int64_t timeout_ns =1000 *1000 *1000 *2 ; +drm_intel_bo *dst; +int i, rc; + + dst =drm_intel_bo_alloc (data->bufmgr,"dst" , (8 <<20 ),4096 ); + + batch =intel_batchbuffer_alloc (data->bufmgr, +intel_get_drm_devid (drm_fd)); + +for (i =0 ; i <1000 ; i++) +blt_color_fill (batch, dst, ((8 <<20 ) >>12 )); + + rc =drm_intel_gem_bo_wait (dst, timeout_ns); +igt_assert (!rc); + +intel_batchbuffer_free (batch); +} + +/* We could check the checksum too, but just the header is probably enough. */ +static bool edid_is_valid (const unsigned char *edid) +{ +char edid_header[] = { +0x0 ,0xFF ,0xFF ,0xFF ,0xFF ,0xFF ,0xFF ,0x0 , + }; + +return (memcmp (edid, edid_header,sizeof (edid_header)) ==0 ); +} + +static int count_drm_valid_edids (struct mode_set_data *data) +{ +int i, ret =0 ; + +for (i =0 ; i < data->res->count_connectors; i++) +if (data->edids[i] &&edid_is_valid (data->edids[i]->data)) + ret++; +return ret; +} + +static int count_drm_edp_outputs (struct mode_set_data *data) +{ +int i, ret =0 ; + +for (i =0 ; i < data->res->count_connectors; i++) +if (data->connectors[i]->connector_type == + DRM_MODE_CONNECTOR_eDP) + ret++; +return ret; +} + +static bool i2c_edid_is_valid (int fd) +{ +int rc; +unsigned char edid[128 ] = {}; +struct i2c_msg msgs[] = { + {/* Start at 0. */ + .addr =0x50 , + .flags =0 , + .len =1 , + .buf = edid, + }, {/* Now read the EDID. */ + .addr =0x50 , + .flags =I2C_M_RD , + .len =128 , + .buf = edid, + } + }; +struct i2c_rdwr_ioctl_data msgset = { + .msgs = msgs, + .nmsgs =2 , + }; + + rc =ioctl (fd,I2C_RDWR , &msgset); +return (rc >=0 ) ?edid_is_valid (edid) : false; +} + +static int count_i2c_valid_edids (void ) +{ +int fd, ret =0 ; +DIR *dir; + +struct dirent *dirent; +char full_name[32 ]; + + dir =opendir ("/dev/" ); +igt_assert (dir); + +while ((dirent =readdir (dir))) { +if (strncmp (dirent->d_name,"i2c-" ,4 ) ==0 ) { +snprintf (full_name,32 ,"/dev/%s" , dirent->d_name); + fd =open (full_name,O_RDWR ); +igt_assert (fd !=-1 ); +if (i2c_edid_is_valid (fd)) + ret++; +close (fd); + } + } + +closedir (dir); + +return 0 ; +} + +/* We run this test when all outputs are turned off. When eDP panels are turned +* off, I2C doesn't work on them and we get a nice dmesg backtrace. So count how +* many eDP panels we have and subtract them. */ +static bool test_i2c (struct mode_set_data *data) +{ +int i2c_edids =count_i2c_valid_edids (); +int drm_edids =count_drm_valid_edids (data); +int edps =count_drm_edp_outputs (data); + +return i2c_edids + edps == drm_edids; +} + +static bool setup_environment (void ) +{ +int i2c_dev_files; +DIR *dev_dir; +struct dirent *dirent; + + drm_fd =drm_open_any (); +if (drm_fd <=0 ) +return false; + +init_mode_set_data (&ms_data); + +/* Only Haswell supports the PC8 feature. */ +if (!IS_HASWELL (ms_data.devid)) { +printf ("PC8+ feature only supported on Haswell.\n" ); +return false; + } + +/* Make sure our Kernel supports MSR and the module is loaded. */ + msr_fd =open ("/dev/cpu/0/msr" ,O_RDONLY ); +if (msr_fd ==-1 ) { +printf ("Can't open /dev/cpu/0/msr.\n" ); +return false; + } + +/* Make sure the /dev/i2c-* files exist. */ + dev_dir =opendir ("/dev" ); +if (!dev_dir) { +printf ("Can't open /dev.\n" ); +return false; + } + i2c_dev_files =0 ; +while ((dirent =readdir (dev_dir))) { +if (strncmp (dirent->d_name,"i2c-" ,4 ) ==0 ) + i2c_dev_files++; + } +closedir (dev_dir); +if (!i2c_dev_files) { +printf ("No /dev/i2c-* files.\n" ); +return false; + } + +/* Non-ULT machines don't support PC8+. */ +if (!supports_pc8_plus_residencies ()) { +printf ("Machine doesn't support PC8+ residencies.\n" ); +return false; + } + +/* Make sure PC8+ residencies move! */ +disable_all_screens (&ms_data); +if (!pc8_plus_enabled ()) { +printf ("Machine is not reaching PC8+ states, please check its " +"configuration.\n" ); +return false; + } + +/* Make sure PC8+ residencies stop! */ +enable_one_screen (&ms_data); +if (!pc8_plus_disabled ()) { +printf ("PC8+ residency didn't stop with screen enabled.\n" ); +return false; + } + +return true; +} + +static void teardown_environment (void ) +{ +fini_mode_set_data (&ms_data); +drmClose (drm_fd); +close (msr_fd); +} + +/* Test of the DRM resources reported by the IOCTLs are still the same. This +* ensures we still see the monitors with the same eyes. We get the EDIDs and +* compare them, which ensures we use DP AUX or GMBUS depending on what's +* connected. */ +static void drm_resources_equal_subtest (void ) +{ +struct compare_data pre_pc8, during_pc8, post_pc8; + +printf ("Checking the if the DRM resources match.\n" ); + +enable_one_screen (&ms_data); +igt_assert (pc8_plus_disabled ()); +get_drm_info (&pre_pc8); +igt_assert (pc8_plus_disabled ()); + +disable_all_screens (&ms_data); +igt_assert (pc8_plus_enabled ()); +get_drm_info (&during_pc8); +igt_assert (pc8_plus_enabled ()); + +enable_one_screen (&ms_data); +igt_assert (pc8_plus_disabled ()); +get_drm_info (&post_pc8); +igt_assert (pc8_plus_disabled ()); + +assert_drm_infos_equal (&pre_pc8, &during_pc8); +assert_drm_infos_equal (&pre_pc8, &post_pc8); + +free_drm_info (&pre_pc8); +free_drm_info (&during_pc8); +free_drm_info (&post_pc8); +} + +/* Make sure interrupts are working. */ +static void batch_subtest (void ) +{ +printf ("Testing batchbuffers.\n" ); + +enable_one_screen (&ms_data); +igt_assert (pc8_plus_disabled ()); + +disable_all_screens (&ms_data); +igt_assert (pc8_plus_enabled ()); +test_batch (&ms_data); +igt_assert (pc8_plus_enabled ()); +} + +/* Try to use raw I2C, which also needs interrupts. */ +static void i2c_subtest (void ) +{ +printf ("Testing raw i2c protocol calls.\n" ); + +enable_one_screen (&ms_data); +igt_assert (pc8_plus_disabled ()); + +disable_all_screens (&ms_data); +igt_assert (pc8_plus_enabled ()); +igt_assert (test_i2c (&ms_data)); +igt_assert (pc8_plus_enabled ()); + +enable_one_screen (&ms_data); +} + +/* Make us enter/leave PC8+ many times. */ +static void stress_test (void ) +{ +int i; + +printf ("Stress testing.\n" ); + +for (i =0 ; i <100 ; i++) { +disable_all_screens (&ms_data); +igt_assert (pc8_plus_enabled ()); +test_batch (&ms_data); +igt_assert (pc8_plus_enabled ()); + } +} + +/* Just reading/writing registers from outside the Kernel is not really a safe +* thing to do on Haswell, so don't do this test on the default case. */ +static void register_compare_subtest (void ) +{ +struct compare_registers pre_pc8, post_pc8; + +printf ("Testing register compare.\n" ); + +enable_one_screen (&ms_data); +igt_assert (pc8_plus_disabled ()); +get_registers (&pre_pc8); +igt_assert (pc8_plus_disabled ()); + +disable_all_screens (&ms_data); +igt_assert (pc8_plus_enabled ()); +enable_one_screen (&ms_data); +igt_assert (pc8_plus_disabled ()); +/* Wait for the registers to be restored. */ +sleep (1 ); +get_registers (&post_pc8); +igt_assert (pc8_plus_disabled ()); + +compare_registers (&pre_pc8, &post_pc8); +} + +int main (int argc,char *argv[]) +{ +bool do_register_compare = false; + +if (argc >1 &&strcmp (argv[1 ],"--do-register-compare" ) ==0 ) + do_register_compare = true; + +igt_subtest_init (argc, argv); + +/* Skip instead of failing in case the machine is not prepared to reach +* PC8+. We don't want bug reports from cases where the machine is just +* not properly configured. */ +printf ("Checking if the environment is properly configured.\n" ); +if (!setup_environment ()) +return 77 ; + + igt_subtest("drm-resources-equal") +drm_resources_equal_subtest (); + igt_subtest("batch") +batch_subtest (); + igt_subtest("i2c") +i2c_subtest (); + igt_subtest("stress-test") +stress_test (); +if (do_register_compare) + igt_subtest("register-compare") +register_compare_subtest (); + +teardown_environment (); + +printf ("Done!\n" ); +return 0 ; +}Signature · 6 lines
1.8.1.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch