drm/i915/vlv: WA for Turbo and RC6 to work together.
Subsystems (2)
PATCH v2 1/1 drm/i915/vlv: WA for Turbo and RC6 to work together.
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
v2: Refactor duplicate code. (ville)
--- drivers/gpu/drm/i915/i915_drv.h | 19 ++++++ drivers/gpu/drm/i915/i915_irq.c | 146 ++++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_reg.h | 15 +++++ drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++---- 4 files changed, 213 insertions(+), 17 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +19 -0
@@ -957,6 +957,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;
@@ -969,10 +975,16 @@ struct intel_gen6_power_mgmt {u8 rp1_delay;u8 rp0_delay;u8 hw_max; +u8 hw_min;bool rp_up_masked;bool rp_down_masked; +u32 cz_freq; +u32 ei_interrupt_count; + +bool use_RC0_residency_for_turbo; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;
@@ -1531,6 +1543,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;
drivers/gpu/drm/i915/i915_irq.c +142 -4
186 lines not on this page · raw message
drivers/gpu/drm/i915/i915_reg.h +15 -0
@@ -391,6 +391,7 @@#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */
@@ -406,6 +407,11 @@#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3
@@ -4857,6 +4863,7 @@#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2
@@ -4864,6 +4871,11 @@#define ECOBUS 0xa180#define FORCEWAKE_MT_ENABLE (1<<5) +#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_GFX_CLK_STATUS_BIT (1<<3) + +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FF +#define GTFIFODBG 0x120000#define GT_FIFO_SBDROPERR (1<<6)#define GT_FIFO_BLOBDROPERR (1<<5)
@@ -4979,6 +4991,9 @@#define VLV_GFX_CLK_STATUS_BIT (1<<3)#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C +#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104#define VLV_COUNT_RANGE_HIGH (1<<15)
drivers/gpu/drm/i915/intel_pm.c +37 -13
102 lines not on this page · raw message
Signature · 1 line
1.8.5.2
Hi Ville,
Please review the patch and share the comments
Thanks
Deepak
Quoted 432 lines · On 3/3/2014 11:35 AM, [email protected] wrote:
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these valuesv2: Refactor duplicate code. (ville)
Signed-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 19 ++++++ drivers/gpu/drm/i915/i915_irq.c | 146 ++++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_reg.h | 15 +++++ drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++---- 4 files changed, 213 insertions(+), 17 deletions(-)drivers/gpu/drm/i915/i915_drv.h +19
-0@@ -957,6 +957,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;@@ -969,10 +975,16 @@ struct intel_gen6_power_mgmt {u8 rp1_delay;u8 rp0_delay;u8 hw_max; +u8 hw_min;bool rp_up_masked;bool rp_down_masked; +u32 cz_freq; +u32 ei_interrupt_count; + +bool use_RC0_residency_for_turbo; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;@@ -1531,6 +1543,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;drivers/gpu/drm/i915/i915_irq.c +142
-4@@ -1023,6 +1023,120 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv, } } +static u32 vlv_c0_residency (struct drm_i915_private *dev_priv, +struct intel_rps_ei_calc *rps_ei) +{ +u32 cz_ts, cz_freq_khz; +u32 render_count, media_count; +u32 elapsed_render, elapsed_media, elapsed_time; +u32 residency =0 ; + + cz_ts =vlv_punit_read (dev_priv,PUNIT_REG_CZ_TIMESTAMP ); + cz_freq_khz =DIV_ROUND_CLOSEST (dev_priv->mem_freq *1000 ,4 ); + + render_count =I915_READ (VLV_RENDER_C0_COUNT_REG ); + media_count =I915_READ (VLV_MEDIA_C0_COUNT_REG ); + +if (rps_ei->cz_ts_ei ==0 ) { + rps_ei->cz_ts_ei = cz_ts; + rps_ei->render_ei_c0 = render_count; + rps_ei->media_ei_c0 = media_count; + +return dev_priv->rps.cur_delay; + } + + elapsed_time = cz_ts - rps_ei->cz_ts_ei; + rps_ei->cz_ts_ei = cz_ts; + + elapsed_render = render_count - rps_ei->render_ei_c0; + rps_ei->render_ei_c0 = render_count; + + elapsed_media = media_count - rps_ei->media_ei_c0; + rps_ei->media_ei_c0 = media_count; + +/* Convert all the counters into common unit of milli sec */ + elapsed_time /=VLV_CZ_CLOCK_TO_MILLI_SEC ; + elapsed_render /= cz_freq_khz; + elapsed_media /= cz_freq_khz; + +/* Calculate overall C0 residency percentage only +* if elapsed time is non zero +*/ +if (elapsed_time) { + residency = + ((max (elapsed_render, elapsed_media) *100 ) + / elapsed_time); + } + +return residency; +} + + +/** +* vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU +* busy-ness calculated from C0 counters of render & media power wells +* @dev_priv: DRM device private +* +*/ +static u32 vlv_calc_delay_from_C0_counters (struct drm_i915_private *dev_priv) +{ +u32 residency_C0_up =0 , residency_C0_down =0 ; +u8 new_delay; + + dev_priv->rps.ei_interrupt_count++; + +WARN_ON (!mutex_is_locked (&dev_priv->rps.hw_lock)); + + +if (dev_priv->rps_up_ei.cz_ts_ei ==0 ) { +vlv_c0_residency (dev_priv, &dev_priv->rps_up_ei); +vlv_c0_residency (dev_priv, &dev_priv->rps_down_ei); +return dev_priv->rps.cur_delay; + } + + +/* To down throttle, C0 residency should be less than down threshold +* for continous EI intervals. So calculate down EI counters +* once in VLV_INT_COUNT_FOR_DOWN_EI +*/ +if (dev_priv->rps.ei_interrupt_count ==VLV_INT_COUNT_FOR_DOWN_EI ) { + + dev_priv->rps.ei_interrupt_count =0 ; + + residency_C0_down =vlv_c0_residency (dev_priv, + &dev_priv->rps_down_ei); + }else { + residency_C0_up =vlv_c0_residency (dev_priv, + &dev_priv->rps_up_ei); + } + + new_delay = dev_priv->rps.cur_delay; + +/* C0 residency is greater than UP threshold. Increase Frequency */ +if (residency_C0_up >=VLV_RP_UP_EI_THRESHOLD ) { + +if (dev_priv->rps.cur_delay < dev_priv->rps.max_delay) + new_delay = dev_priv->rps.cur_delay +1 ; + +/* +* For better performance, jump directly +* to RPe if we're below it. +*/ +if (new_delay < dev_priv->rps.rpe_delay) + new_delay = dev_priv->rps.rpe_delay; + + }else if (!dev_priv->rps.ei_interrupt_count && + (residency_C0_down <VLV_RP_DOWN_EI_THRESHOLD )) { +/* This means, C0 residency is less than down threshold over +* a period of VLV_INT_COUNT_FOR_DOWN_EI. So, reduce the freq +*/ +if (dev_priv->rps.cur_delay > dev_priv->rps.min_delay) + new_delay = dev_priv->rps.cur_delay -1 ; + } + +return new_delay; +} +static void gen6_pm_rps_work (struct work_struct *work) { drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,@@ -1034,13 +1148,16 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv->rps.pm_iir; dev_priv->rps.pm_iir =0 ;/* Make sure not to corrupt PMIMR state used by ringbuffer code */ -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) +snb_enable_pm_irq (dev_priv,GEN6_PM_RP_UP_EI_EXPIRED ); +else +snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS );spin_unlock_irq (&dev_priv->irq_lock);/* Make sure we didn't queue anything we're not going to process. */ -WARN_ON (pm_iir & ~GEN6_PM_RPS_EVENTS ); +WARN_ON (pm_iir & ~(GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED )); -if ((pm_iir &GEN6_PM_RPS_EVENTS ) ==0 ) +if ((pm_iir & (GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED )) ==0 )return ;mutex_lock (&dev_priv->rps.hw_lock);@@ -1065,6 +1182,8 @@ static void gen6_pm_rps_work(struct work_struct *work)else new_delay = dev_priv->rps.min_delay; adj =0 ; + }else if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { + new_delay =vlv_calc_delay_from_C0_counters (dev_priv); }else if (pm_iir &GEN6_PM_RP_DOWN_THRESHOLD ) {if (adj <0 ) adj *=2 ;@@ -1466,6 +1585,16 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)queue_work (dev_priv->wq, &dev_priv->rps.work); } +if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { +spin_lock (&dev_priv->irq_lock); + dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ; +snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ); +spin_unlock (&dev_priv->irq_lock); +DRM_DEBUG_DRIVER ("\nQueueing RPS Work - RC6 WA Turbo" ); + +queue_work (dev_priv->wq, &dev_priv->rps.work); + } +if (HAS_VEBOX (dev_priv->dev)) {if (pm_iir &PM_VEBOX_USER_INTERRUPT )notify_ring (dev_priv->dev, &dev_priv->ring[VECS ]);@@ -1546,7 +1675,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)if (pipe_stats[0 ] &PIPE_GMBUS_INTERRUPT_STATUS )gmbus_irq_handler (dev); -if (pm_iir) +if (pm_iir & (GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED ))gen6_rps_irq_handler (dev_priv, pm_iir);I915_WRITE (GTIIR , gt_iir);@@ -2861,6 +2990,15 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) pm_irqs |=PM_VEBOX_USER_INTERRUPT ; dev_priv->pm_irq_mask =0xffffffff ; + +if (dev_priv->rps.use_RC0_residency_for_turbo) { + dev_priv->pm_irq_mask &= ~GEN6_PM_RP_UP_EI_EXPIRED ; + pm_irqs |=GEN6_PM_RP_UP_EI_EXPIRED ; + }else { + dev_priv->pm_irq_mask &= ~GEN6_PM_RPS_EVENTS ; + pm_irqs |=GEN6_PM_RPS_EVENTS ; + } +I915_WRITE (GEN6_PMIIR ,I915_READ (GEN6_PMIIR ));I915_WRITE (GEN6_PMIMR , dev_priv->pm_irq_mask);I915_WRITE (GEN6_PMIER , pm_irqs);drivers/gpu/drm/i915/i915_reg.h +15
-0@@ -391,6 +391,7 @@#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */ @@ -406,6 +407,11 @@#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3@@ -4857,6 +4863,7 @@#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2@@ -4864,6 +4871,11 @@#define ECOBUS 0xa180#define FORCEWAKE_MT_ENABLE (1<<5) +#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_GFX_CLK_STATUS_BIT (1<<3) + +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FF +#define GTFIFODBG 0x120000#define GT_FIFO_SBDROPERR (1<<6)#define GT_FIFO_BLOBDROPERR (1<<5)@@ -4979,6 +4991,9 @@#define VLV_GFX_CLK_STATUS_BIT (1<<3)#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C +#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104#define VLV_COUNT_RANGE_HIGH (1<<15)drivers/gpu/drm/i915/intel_pm.c +37
-13@@ -3084,10 +3084,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->rps.use_RC0_residency_for_turbo) +I915_WRITE (GEN6_PMINTRMSK , ~GEN6_PM_RP_UP_EI_EXPIRED ); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)@@ -3148,7 +3152,13 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;I915_WRITE (GEN6_PMINTRMSK ,0xffffffff ); -I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) { +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~GEN6_PM_RP_UP_EI_EXPIRED ); + }else { +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~GEN6_PM_RPS_EVENTS ); + }/* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving @@ -3158,7 +3168,10 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev) dev_priv->rps.pm_iir =0 ;spin_unlock_irq (&dev_priv->irq_lock); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RP_UP_EI_EXPIRED ); +else +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); }static void gen6_disable_rps (struct drm_device *dev)@@ -3228,19 +3241,29 @@ static void gen6_enable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;u32 enabled_intrs; +/* Clear out any stale interrupts first */ spin_lock_irq (&dev_priv->irq_lock);WARN_ON (dev_priv->rps.pm_iir); -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) { +snb_enable_pm_irq (dev_priv,GEN6_PM_RP_UP_EI_EXPIRED ); +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RP_UP_EI_EXPIRED ); + }else { +snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); + }spin_unlock_irq (&dev_priv->irq_lock);/* only unmask PM interrupts we need. Mask all others. */ - enabled_intrs =GEN6_PM_RPS_EVENTS ; +if (dev_priv->rps.use_RC0_residency_for_turbo) + enabled_intrs =GEN6_PM_RP_UP_EI_EXPIRED ; +else + enabled_intrs =GEN6_PM_RPS_EVENTS ;/* IVB and SNB hard hangs on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked. */ -if (INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) +if (INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev) && + !dev_priv->rps.use_RC0_residency_for_turbo) enabled_intrs |=GEN6_PM_RP_UP_EI_EXPIRED ;I915_WRITE (GEN6_PMINTRMSK , ~enabled_intrs);@@ -3608,6 +3631,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |@@ -3627,10 +3651,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RC6_THRESHOLD ,0x557 );/* allows RC6 residency counter to work */ -I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | -VLV_MEDIA_RC6_COUNT_EN | -VLV_RENDER_RC6_COUNT_EN )); +I915_WRITE (VLV_COUNTER_CONTROL ,VLV_RC_COUNTER_CONTROL );if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;@@ -3673,6 +3694,9 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; +/* enable WA for RC6+turbo to work together */ + dev_priv->rps.use_RC0_residency_for_turbo = true; +gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );
Quoted 9 lines · On Mon, Mar 03, 2014 at 11:35:50AM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
Quoted 3 lines
v2: Refactor duplicate code. (ville)
Signed-off-by Deepak S <[email protected]>
Did we reach some conclusion about this approach? It seemed to save power in some workloads at least, but there's still the question whether it ramps up the frquency fast enoguh to provide a good user experience. Maybe we should make it optional even on VLV?
Quoted 26 lines
--- drivers/gpu/drm/i915/i915_drv.h | 19 ++++++ drivers/gpu/drm/i915/i915_irq.c | 146 ++++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_reg.h | 15 +++++ drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++---- 4 files changed, 213 insertions(+), 17 deletions(-)drivers/gpu/drm/i915/i915_drv.h +6
-0@@ -957,6 +957,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;
drivers/gpu/drm/i915/i915_drv.h struct intel_gen6_power_mgmt { · line 978
Quoted 4 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -969,10 +975,16 @@ struct intel_gen6_power_mgmt {u8 rp1_delay;u8 rp0_delay;u8 hw_max; +u8 hw_min;
Some leftover still?
drivers/gpu/drm/i915/i915_drv.h struct intel_gen6_power_mgmt { · line 983
Quoted 5 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0bool rp_up_masked;bool rp_down_masked; +u32 cz_freq;
This too seems unused.
Quoted 58 lines
drivers/gpu/drm/i915/i915_drv.h +11
-0+u32 ei_interrupt_count; + +bool use_RC0_residency_for_turbo; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;@@ -1531,6 +1543,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;drivers/gpu/drm/i915/i915_irq.c +29
-0@@ -1023,6 +1023,120 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv, } } +static u32 vlv_c0_residency (struct drm_i915_private *dev_priv, +struct intel_rps_ei_calc *rps_ei) +{ +u32 cz_ts, cz_freq_khz; +u32 render_count, media_count; +u32 elapsed_render, elapsed_media, elapsed_time; +u32 residency =0 ; + + cz_ts =vlv_punit_read (dev_priv,PUNIT_REG_CZ_TIMESTAMP ); + cz_freq_khz =DIV_ROUND_CLOSEST (dev_priv->mem_freq *1000 ,4 ); + + render_count =I915_READ (VLV_RENDER_C0_COUNT_REG ); + media_count =I915_READ (VLV_MEDIA_C0_COUNT_REG ); + +if (rps_ei->cz_ts_ei ==0 ) { + rps_ei->cz_ts_ei = cz_ts; + rps_ei->render_ei_c0 = render_count; + rps_ei->media_ei_c0 = media_count; + +return dev_priv->rps.cur_delay; + } + + elapsed_time = cz_ts - rps_ei->cz_ts_ei; + rps_ei->cz_ts_ei = cz_ts; + + elapsed_render = render_count - rps_ei->render_ei_c0; + rps_ei->render_ei_c0 = render_count; + + elapsed_media = media_count - rps_ei->media_ei_c0;
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1064
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+ rps_ei->media_ei_c0 = media_count; + +/* Convert all the counters into common unit of milli sec */ + elapsed_time /=VLV_CZ_CLOCK_TO_MILLI_SEC ; + elapsed_render /= cz_freq_khz; + elapsed_media /= cz_freq_khz; + +/* Calculate overall C0 residency percentage only +* if elapsed time is non zero +*/
Badly formatted comment.
Quoted 27 lines
drivers/gpu/drm/i915/i915_irq.c +27
-0+if (elapsed_time) { + residency = + ((max (elapsed_render, elapsed_media) *100 ) + / elapsed_time); + } + +return residency; +} + + +/** +* vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU +* busy-ness calculated from C0 counters of render & media power wells +* @dev_priv: DRM device private +* +*/ +static u32 vlv_calc_delay_from_C0_counters (struct drm_i915_private *dev_priv) +{ +u32 residency_C0_up =0 , residency_C0_down =0 ; +u8 new_delay; + + dev_priv->rps.ei_interrupt_count++; + +WARN_ON (!mutex_is_locked (&dev_priv->rps.hw_lock)); + + +if (dev_priv->rps_up_ei.cz_ts_ei ==0 ) {
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1101
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+vlv_c0_residency (dev_priv, &dev_priv->rps_up_ei); +vlv_c0_residency (dev_priv, &dev_priv->rps_down_ei); +return dev_priv->rps.cur_delay; + } + + +/* To down throttle, C0 residency should be less than down threshold +* for continous EI intervals. So calculate down EI counters +* once in VLV_INT_COUNT_FOR_DOWN_EI +*/
Badly formatted comment.
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1106
Quoted 5 lines
drivers/gpu/drm/i915/i915_irq.c +5
-0+if (dev_priv->rps.ei_interrupt_count ==VLV_INT_COUNT_FOR_DOWN_EI ) { + + dev_priv->rps.ei_interrupt_count =0 ; + + residency_C0_down = vlv_c0_residency(dev_priv,
^
Extra space we don't need
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1109
Quoted 3 lines
drivers/gpu/drm/i915/i915_irq.c +3
-0+ &dev_priv->rps_down_ei); + }else { + residency_C0_up = vlv_c0_residency(dev_priv,
^
Another
Quoted 13 lines
drivers/gpu/drm/i915/i915_irq.c +13
-0+ &dev_priv->rps_up_ei); + } + + new_delay = dev_priv->rps.cur_delay; + +/* C0 residency is greater than UP threshold. Increase Frequency */ +if (residency_C0_up >=VLV_RP_UP_EI_THRESHOLD ) { + +if (dev_priv->rps.cur_delay < dev_priv->rps.max_delay) + new_delay = dev_priv->rps.cur_delay +1 ; + +/* +* For better performance, jump directly
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1132
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+* to RPe if we're below it. +*/ +if (new_delay < dev_priv->rps.rpe_delay) + new_delay = dev_priv->rps.rpe_delay; + + }else if (!dev_priv->rps.ei_interrupt_count && + (residency_C0_down <VLV_RP_DOWN_EI_THRESHOLD )) { +/* This means, C0 residency is less than down threshold over +* a period of VLV_INT_COUNT_FOR_DOWN_EI. So, reduce the freq +*/
Comment is badly formatted.
Quoted 11 lines
drivers/gpu/drm/i915/i915_irq.c +7
-0+if (dev_priv->rps.cur_delay > dev_priv->rps.min_delay) + new_delay = dev_priv->rps.cur_delay -1 ; + } + +return new_delay; +} +static void gen6_pm_rps_work (struct work_struct *work) { drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
drivers/gpu/drm/i915/i915_irq.c static void gen6_pm_rps_work(struct work_struct *work) · line 1154
Quoted 8 lines
drivers/gpu/drm/i915/i915_irq.c +4
-1@@ -1034,13 +1148,16 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv->rps.pm_iir; dev_priv->rps.pm_iir =0 ;/* Make sure not to corrupt PMIMR state used by ringbuffer code */ -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) +snb_enable_pm_irq (dev_priv,GEN6_PM_RP_UP_EI_EXPIRED ); +else +snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS );
This pattern keeps reeating many times in the patch. Maybe it would be better to track the enabled PM interrupts in dev_priv somewhere, and use that instead of GEN6_PM_RPS_EVENTS vs. GEN6_PM_RP_UP_EI_EXPIRED everywhere. Maybe call it dev_priv->pm_rps_events to keep in line with the GEN6_PM_RPS_EVENTS name. I'd make it a separate preparation patch.
Quoted 98 lines
drivers/gpu/drm/i915/i915_irq.c +4
-2spin_unlock_irq (&dev_priv->irq_lock);/* Make sure we didn't queue anything we're not going to process. */ -WARN_ON (pm_iir & ~GEN6_PM_RPS_EVENTS ); +WARN_ON (pm_iir & ~(GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED )); -if ((pm_iir &GEN6_PM_RPS_EVENTS ) ==0 ) +if ((pm_iir & (GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED )) ==0 )return ;mutex_lock (&dev_priv->rps.hw_lock);@@ -1065,6 +1182,8 @@ static void gen6_pm_rps_work(struct work_struct *work)else new_delay = dev_priv->rps.min_delay; adj =0 ; + }else if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { + new_delay =vlv_calc_delay_from_C0_counters (dev_priv); }else if (pm_iir &GEN6_PM_RP_DOWN_THRESHOLD ) {drivers/gpu/drm/i915/i915_irq.c +11
-1if (adj <0 ) adj *=2 ;@@ -1466,6 +1585,16 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)queue_work (dev_priv->wq, &dev_priv->rps.work); } +if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { +spin_lock (&dev_priv->irq_lock); + dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ; +snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ); +spin_unlock (&dev_priv->irq_lock); +DRM_DEBUG_DRIVER ("\nQueueing RPS Work - RC6 WA Turbo" ); + +queue_work (dev_priv->wq, &dev_priv->rps.work); + } +if (HAS_VEBOX (dev_priv->dev)) {if (pm_iir &PM_VEBOX_USER_INTERRUPT )notify_ring (dev_priv->dev, &dev_priv->ring[VECS ]);@@ -1546,7 +1675,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)if (pipe_stats[0 ] &PIPE_GMBUS_INTERRUPT_STATUS )gmbus_irq_handler (dev); -if (pm_iir) +if (pm_iir & (GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED ))gen6_rps_irq_handler (dev_priv, pm_iir);drivers/gpu/drm/i915/i915_irq.c +9
-0I915_WRITE (GTIIR , gt_iir);@@ -2861,6 +2990,15 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) pm_irqs |=PM_VEBOX_USER_INTERRUPT ; dev_priv->pm_irq_mask =0xffffffff ; + +if (dev_priv->rps.use_RC0_residency_for_turbo) { + dev_priv->pm_irq_mask &= ~GEN6_PM_RP_UP_EI_EXPIRED ; + pm_irqs |=GEN6_PM_RP_UP_EI_EXPIRED ; + }else { + dev_priv->pm_irq_mask &= ~GEN6_PM_RPS_EVENTS ; + pm_irqs |=GEN6_PM_RPS_EVENTS ; + } +I915_WRITE (GEN6_PMIIR ,I915_READ (GEN6_PMIIR ));I915_WRITE (GEN6_PMIMR , dev_priv->pm_irq_mask);I915_WRITE (GEN6_PMIER , pm_irqs);drivers/gpu/drm/i915/i915_reg.h +7
-0@@ -391,6 +391,7 @@#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */ @@ -406,6 +407,11 @@#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3@@ -4857,6 +4863,7 @@#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2
drivers/gpu/drm/i915/i915_reg.h line 4875
Quoted 5 lines
drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -4864,6 +4871,11 @@#define ECOBUS 0xa180#define FORCEWAKE_MT_ENABLE (1<<5) +#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_GFX_CLK_STATUS_BIT (1<<3)
Leftovers from somewhere.
drivers/gpu/drm/i915/i915_reg.h line 4877
Quoted 2 lines
drivers/gpu/drm/i915/i915_reg.h +2
-0+ +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FF
This should be below the register define, but it would be better to use the names of the bits properly. Also do we really want to enable all of the counters when you only use the rc0 counters?
I'm also wondering why we're currently enabling the rc6 counters. I don't see that listed as a requirement for rc6 to work in any document, and we don't seem to expose those counters through debugfs either.
drivers/gpu/drm/i915/i915_reg.h line 4995
Quoted 10 lines
drivers/gpu/drm/i915/i915_reg.h +3
-0+#define GTFIFODBG 0x120000#define GT_FIFO_SBDROPERR (1<<6)#define GT_FIFO_BLOBDROPERR (1<<5)@@ -4979,6 +4991,9 @@#define VLV_GFX_CLK_STATUS_BIT (1<<3)#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C
Maybe put these in the correct numerical place between GEN6_GT_GFX_RC6pp and GEN6_PCODE_MAILBOX.
Quoted 124 lines
drivers/gpu/drm/i915/i915_reg.h +1
-0+#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104#define VLV_COUNT_RANGE_HIGH (1<<15)drivers/gpu/drm/i915/intel_pm.c +37
-13@@ -3084,10 +3084,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->rps.use_RC0_residency_for_turbo) +I915_WRITE (GEN6_PMINTRMSK , ~GEN6_PM_RP_UP_EI_EXPIRED ); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)@@ -3148,7 +3152,13 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;I915_WRITE (GEN6_PMINTRMSK ,0xffffffff ); -I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) { +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~GEN6_PM_RP_UP_EI_EXPIRED ); + }else { +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~GEN6_PM_RPS_EVENTS ); + }/* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving @@ -3158,7 +3168,10 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev) dev_priv->rps.pm_iir =0 ;spin_unlock_irq (&dev_priv->irq_lock); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RP_UP_EI_EXPIRED ); +else +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); }static void gen6_disable_rps (struct drm_device *dev)@@ -3228,19 +3241,29 @@ static void gen6_enable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;u32 enabled_intrs; +/* Clear out any stale interrupts first */ spin_lock_irq (&dev_priv->irq_lock);WARN_ON (dev_priv->rps.pm_iir); -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) { +snb_enable_pm_irq (dev_priv,GEN6_PM_RP_UP_EI_EXPIRED ); +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RP_UP_EI_EXPIRED ); + }else { +snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); + }spin_unlock_irq (&dev_priv->irq_lock);/* only unmask PM interrupts we need. Mask all others. */ - enabled_intrs =GEN6_PM_RPS_EVENTS ; +if (dev_priv->rps.use_RC0_residency_for_turbo) + enabled_intrs =GEN6_PM_RP_UP_EI_EXPIRED ; +else + enabled_intrs =GEN6_PM_RPS_EVENTS ;/* IVB and SNB hard hangs on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked. */ -if (INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) +if (INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev) && + !dev_priv->rps.use_RC0_residency_for_turbo) enabled_intrs |=GEN6_PM_RP_UP_EI_EXPIRED ;I915_WRITE (GEN6_PMINTRMSK , ~enabled_intrs);@@ -3608,6 +3631,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |@@ -3627,10 +3651,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RC6_THRESHOLD ,0x557 );/* allows RC6 residency counter to work */ -I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | -VLV_MEDIA_RC6_COUNT_EN | -VLV_RENDER_RC6_COUNT_EN )); +I915_WRITE (VLV_COUNTER_CONTROL ,VLV_RC_COUNTER_CONTROL );if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;@@ -3673,6 +3694,9 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; +/* enable WA for RC6+turbo to work together */ + dev_priv->rps.use_RC0_residency_for_turbo = true; +gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );Signature · 6 lines
1.8.5.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
Quoted 10 lines · Ville Syrjälä
Quoted 9 lines · On Mon, Mar 03, 2014 at 11:35:50AM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
Quoted 8 lines
Quoted 3 lines
v2: Refactor duplicate code. (ville)
Signed-off-by Deepak S <[email protected]>Did we reach some conclusion about this approach? It seemed to save power in some workloads at least, but there's still the question whether it ramps up the frquency fast enoguh to provide a good user experience. Maybe we should make it optional even on VLV?
I have made this as a optional for the VLV. The boost logic is enabled by default. If we need power savings then we can turn off the boost and regular turbo logic will be enabled.
I will working on other options that Dainel suggested of identifying the libva workload and controlling the boost at run time.
I will address the other comments and upload the patch for review.
Quoted 477 lines
Quoted 26 lines
--- drivers/gpu/drm/i915/i915_drv.h | 19 ++++++ drivers/gpu/drm/i915/i915_irq.c | 146 ++++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_reg.h | 15 +++++ drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++---- 4 files changed, 213 insertions(+), 17 deletions(-)drivers/gpu/drm/i915/i915_drv.h +6
-0@@ -957,6 +957,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;drivers/gpu/drm/i915/i915_drv.h struct intel_gen6_power_mgmt { · line 978
Quoted 4 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -969,10 +975,16 @@ struct intel_gen6_power_mgmt {u8 rp1_delay;u8 rp0_delay;u8 hw_max; +u8 hw_min;Some leftover still?
drivers/gpu/drm/i915/i915_drv.h struct intel_gen6_power_mgmt { · line 983
Quoted 5 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0bool rp_up_masked;bool rp_down_masked; +u32 cz_freq;This too seems unused.
Quoted 58 lines
drivers/gpu/drm/i915/i915_drv.h +11
-0+u32 ei_interrupt_count; + +bool use_RC0_residency_for_turbo; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;@@ -1531,6 +1543,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;drivers/gpu/drm/i915/i915_irq.c +29
-0@@ -1023,6 +1023,120 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv, } } +static u32 vlv_c0_residency (struct drm_i915_private *dev_priv, +struct intel_rps_ei_calc *rps_ei) +{ +u32 cz_ts, cz_freq_khz; +u32 render_count, media_count; +u32 elapsed_render, elapsed_media, elapsed_time; +u32 residency =0 ; + + cz_ts =vlv_punit_read (dev_priv,PUNIT_REG_CZ_TIMESTAMP ); + cz_freq_khz =DIV_ROUND_CLOSEST (dev_priv->mem_freq *1000 ,4 ); + + render_count =I915_READ (VLV_RENDER_C0_COUNT_REG ); + media_count =I915_READ (VLV_MEDIA_C0_COUNT_REG ); + +if (rps_ei->cz_ts_ei ==0 ) { + rps_ei->cz_ts_ei = cz_ts; + rps_ei->render_ei_c0 = render_count; + rps_ei->media_ei_c0 = media_count; + +return dev_priv->rps.cur_delay; + } + + elapsed_time = cz_ts - rps_ei->cz_ts_ei; + rps_ei->cz_ts_ei = cz_ts; + + elapsed_render = render_count - rps_ei->render_ei_c0; + rps_ei->render_ei_c0 = render_count; + + elapsed_media = media_count - rps_ei->media_ei_c0;drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1064
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+ rps_ei->media_ei_c0 = media_count; + +/* Convert all the counters into common unit of milli sec */ + elapsed_time /=VLV_CZ_CLOCK_TO_MILLI_SEC ; + elapsed_render /= cz_freq_khz; + elapsed_media /= cz_freq_khz; + +/* Calculate overall C0 residency percentage only +* if elapsed time is non zero +*/ Badly formatted comment.
Quoted 27 lines
drivers/gpu/drm/i915/i915_irq.c +27
-0+if (elapsed_time) { + residency = + ((max (elapsed_render, elapsed_media) *100 ) + / elapsed_time); + } + +return residency; +} + + +/** +* vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU +* busy-ness calculated from C0 counters of render & media power wells +* @dev_priv: DRM device private +* +*/ +static u32 vlv_calc_delay_from_C0_counters (struct drm_i915_private *dev_priv) +{ +u32 residency_C0_up =0 , residency_C0_down =0 ; +u8 new_delay; + + dev_priv->rps.ei_interrupt_count++; + +WARN_ON (!mutex_is_locked (&dev_priv->rps.hw_lock)); + + +if (dev_priv->rps_up_ei.cz_ts_ei ==0 ) {drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1101
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+vlv_c0_residency (dev_priv, &dev_priv->rps_up_ei); +vlv_c0_residency (dev_priv, &dev_priv->rps_down_ei); +return dev_priv->rps.cur_delay; + } + + +/* To down throttle, C0 residency should be less than down threshold +* for continous EI intervals. So calculate down EI counters +* once in VLV_INT_COUNT_FOR_DOWN_EI +*/ Badly formatted comment.
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1106
Quoted 5 lines
drivers/gpu/drm/i915/i915_irq.c +5
-0+if (dev_priv->rps.ei_interrupt_count ==VLV_INT_COUNT_FOR_DOWN_EI ) { + + dev_priv->rps.ei_interrupt_count =0 ; + + residency_C0_down = vlv_c0_residency(dev_priv,^Extra space we don't need
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1109
Quoted 3 lines
drivers/gpu/drm/i915/i915_irq.c +3
-0+ &dev_priv->rps_down_ei); + }else { + residency_C0_up = vlv_c0_residency(dev_priv,^Another
Quoted 13 lines
drivers/gpu/drm/i915/i915_irq.c +13
-0+ &dev_priv->rps_up_ei); + } + + new_delay = dev_priv->rps.cur_delay; + +/* C0 residency is greater than UP threshold. Increase Frequency */ +if (residency_C0_up >=VLV_RP_UP_EI_THRESHOLD ) { + +if (dev_priv->rps.cur_delay < dev_priv->rps.max_delay) + new_delay = dev_priv->rps.cur_delay +1 ; + +/* +* For better performance, jump directly drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1132
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+* to RPe if we're below it. +*/ +if (new_delay < dev_priv->rps.rpe_delay) + new_delay = dev_priv->rps.rpe_delay; + + }else if (!dev_priv->rps.ei_interrupt_count && + (residency_C0_down <VLV_RP_DOWN_EI_THRESHOLD )) { +/* This means, C0 residency is less than down threshold over +* a period of VLV_INT_COUNT_FOR_DOWN_EI. So, reduce the freq +*/ Comment is badly formatted.
Quoted 11 lines
drivers/gpu/drm/i915/i915_irq.c +7
-0+if (dev_priv->rps.cur_delay > dev_priv->rps.min_delay) + new_delay = dev_priv->rps.cur_delay -1 ; + } + +return new_delay; +} +static void gen6_pm_rps_work (struct work_struct *work) { drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,drivers/gpu/drm/i915/i915_irq.c static void gen6_pm_rps_work(struct work_struct *work) · line 1154
Quoted 8 lines
drivers/gpu/drm/i915/i915_irq.c +4
-1@@ -1034,13 +1148,16 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv->rps.pm_iir; dev_priv->rps.pm_iir =0 ;/* Make sure not to corrupt PMIMR state used by ringbuffer code */ -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) +snb_enable_pm_irq (dev_priv,GEN6_PM_RP_UP_EI_EXPIRED ); +else +snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS );This pattern keeps reeating many times in the patch. Maybe it would be better to track the enabled PM interrupts in
dev_privsomewhere, and use that instead ofGEN6_PM_RPS_EVENTSvs.GEN6_PM_RP_UP_EI_EXPIREDeverywhere. Maybe call itdev_priv->pm_rps_eventsto keep in line with theGEN6_PM_RPS_EVENTSname. I'd make it a separate preparation patch.Quoted 98 lines
drivers/gpu/drm/i915/i915_irq.c +4
-2spin_unlock_irq (&dev_priv->irq_lock);/* Make sure we didn't queue anything we're not going to process. */ -WARN_ON (pm_iir & ~GEN6_PM_RPS_EVENTS ); +WARN_ON (pm_iir & ~(GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED )); -if ((pm_iir &GEN6_PM_RPS_EVENTS ) ==0 ) +if ((pm_iir & (GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED )) ==0 )return ;mutex_lock (&dev_priv->rps.hw_lock);@@ -1065,6 +1182,8 @@ static void gen6_pm_rps_work(struct work_struct *work)else new_delay = dev_priv->rps.min_delay; adj =0 ; + }else if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { + new_delay =vlv_calc_delay_from_C0_counters (dev_priv); }else if (pm_iir &GEN6_PM_RP_DOWN_THRESHOLD ) {if (adj < 0) adj *= 2;drivers/gpu/drm/i915/i915_irq.c +11
-1@@ -1466,6 +1585,16 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)queue_work (dev_priv->wq, &dev_priv->rps.work); } +if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { +spin_lock (&dev_priv->irq_lock); + dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ; +snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ); +spin_unlock (&dev_priv->irq_lock); +DRM_DEBUG_DRIVER ("\nQueueing RPS Work - RC6 WA Turbo" ); + +queue_work (dev_priv->wq, &dev_priv->rps.work); + } +if (HAS_VEBOX (dev_priv->dev)) {if (pm_iir &PM_VEBOX_USER_INTERRUPT )notify_ring (dev_priv->dev, &dev_priv->ring[VECS ]);@@ -1546,7 +1675,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)if (pipe_stats[0 ] &PIPE_GMBUS_INTERRUPT_STATUS )gmbus_irq_handler (dev); -if (pm_iir) +if (pm_iir & (GEN6_PM_RPS_EVENTS |GEN6_PM_RP_UP_EI_EXPIRED ))gen6_rps_irq_handler (dev_priv, pm_iir);I915_WRITE(GTIIR, gt_iir);drivers/gpu/drm/i915/i915_irq.c +9
-0@@ -2861,6 +2990,15 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) pm_irqs |=PM_VEBOX_USER_INTERRUPT ; dev_priv->pm_irq_mask =0xffffffff ; + +if (dev_priv->rps.use_RC0_residency_for_turbo) { + dev_priv->pm_irq_mask &= ~GEN6_PM_RP_UP_EI_EXPIRED ; + pm_irqs |=GEN6_PM_RP_UP_EI_EXPIRED ; + }else { + dev_priv->pm_irq_mask &= ~GEN6_PM_RPS_EVENTS ; + pm_irqs |=GEN6_PM_RPS_EVENTS ; + } +I915_WRITE (GEN6_PMIIR ,I915_READ (GEN6_PMIIR ));I915_WRITE (GEN6_PMIMR , dev_priv->pm_irq_mask);I915_WRITE (GEN6_PMIER , pm_irqs);drivers/gpu/drm/i915/i915_reg.h +7
-0@@ -391,6 +391,7 @@#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */ @@ -406,6 +407,11 @@#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3@@ -4857,6 +4863,7 @@#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2drivers/gpu/drm/i915/i915_reg.h line 4875
Quoted 5 lines
drivers/gpu/drm/i915/i915_reg.h +2
-0@@ -4864,6 +4871,11 @@#define ECOBUS 0xa180#define FORCEWAKE_MT_ENABLE (1<<5) +#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_GFX_CLK_STATUS_BIT (1<<3)Leftovers from somewhere.
drivers/gpu/drm/i915/i915_reg.h line 4877
Quoted 2 lines
drivers/gpu/drm/i915/i915_reg.h +2
-0+ +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FFThis should be below the register define, but it would be better to use the names of the bits properly. Also do we really want to enable all of the counters when you only use the rc0 counters?
I'm also wondering why we're currently enabling the rc6 counters. I don't see that listed as a requirement for rc6 to work in any document, and we don't seem to expose those counters through debugfs either.
drivers/gpu/drm/i915/i915_reg.h line 4995
Quoted 10 lines
drivers/gpu/drm/i915/i915_reg.h +3
-0+#define GTFIFODBG 0x120000#define GT_FIFO_SBDROPERR (1<<6)#define GT_FIFO_BLOBDROPERR (1<<5)@@ -4979,6 +4991,9 @@#define VLV_GFX_CLK_STATUS_BIT (1<<3)#define VLV_GFX_CLK_FORCE_ON_BIT (1<<2) +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811CMaybe put these in the correct numerical place between
GEN6_GT_GFX_RC6ppandGEN6_PCODE_MAILBOX.Quoted 124 lines
drivers/gpu/drm/i915/i915_reg.h +1
-0+#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104#define VLV_COUNT_RANGE_HIGH (1<<15)drivers/gpu/drm/i915/intel_pm.c +37
-14@@ -3084,10 +3084,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->rps.use_RC0_residency_for_turbo) +I915_WRITE (GEN6_PMINTRMSK , ~GEN6_PM_RP_UP_EI_EXPIRED ); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)@@ -3148,7 +3152,13 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;I915_WRITE (GEN6_PMINTRMSK ,0xffffffff ); -I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) { +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~GEN6_PM_RP_UP_EI_EXPIRED ); + }else { +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~GEN6_PM_RPS_EVENTS ); + }/* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving @@ -3158,7 +3168,10 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev) dev_priv->rps.pm_iir =0 ;spin_unlock_irq (&dev_priv->irq_lock); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RP_UP_EI_EXPIRED ); +else +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); }static void gen6_disable_rps (struct drm_device *dev)@@ -3228,19 +3241,29 @@ static void gen6_enable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;u32 enabled_intrs; +/* Clear out any stale interrupts first */ spin_lock_irq (&dev_priv->irq_lock);WARN_ON (dev_priv->rps.pm_iir); -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +if (dev_priv->rps.use_RC0_residency_for_turbo) { +snb_enable_pm_irq (dev_priv,GEN6_PM_RP_UP_EI_EXPIRED ); +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RP_UP_EI_EXPIRED ); + }else { +snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); + }spin_unlock_irq (&dev_priv->irq_lock);/* only unmask PM interrupts we need. Mask all others. */ - enabled_intrs =GEN6_PM_RPS_EVENTS ; +if (dev_priv->rps.use_RC0_residency_for_turbo) + enabled_intrs =GEN6_PM_RP_UP_EI_EXPIRED ; +else + enabled_intrs =GEN6_PM_RPS_EVENTS ;/* IVB and SNB hard hangs on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked. */ -if (INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev)) +if (INTEL_INFO (dev)->gen <=7 && !IS_HASWELL (dev) && + !dev_priv->rps.use_RC0_residency_for_turbo) enabled_intrs |=GEN6_PM_RP_UP_EI_EXPIRED ;I915_WRITE (GEN6_PMINTRMSK , ~enabled_intrs);@@ -3608,6 +3631,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |@@ -3627,10 +3651,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RC6_THRESHOLD ,0x557 );/* allows RC6 residency counter to work */ -I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | -VLV_MEDIA_RC6_COUNT_EN | -VLV_RENDER_RC6_COUNT_EN )); +I915_WRITE (VLV_COUNTER_CONTROL ,VLV_RC_COUNTER_CONTROL );if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;@@ -3673,6 +3694,9 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; +/* enable WA for RC6+turbo to work together */ + dev_priv->rps.use_RC0_residency_for_turbo = true; +gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL ); --1.8.5.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
v3 cover WA for Turbo and RC6 to work together.
From: Deepak S <[email protected]>
This series adds WA patches to enable RC6 and Turbo to work together and also adds a patch to contol the rps boost at runtime
Deepak S (3):
drm/i915: Track the enabled PM interrupts in dev_priv. drm/i915/vlv: WA for Turbo and RC6 to work together. drm/i915: Add boot paramter to control rps boost at boot time.
drivers/gpu/drm/i915/i915_drv.h | 19 +++++ drivers/gpu/drm/i915/i915_gem.c | 16 +++- drivers/gpu/drm/i915/i915_irq.c | 147 +++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_params.c | 5 ++ drivers/gpu/drm/i915/i915_reg.h | 13 +++- drivers/gpu/drm/i915/intel_pm.c | 31 +++++--- 6 files changed, 210 insertions(+), 21 deletions(-)
Signature · 1 line
1.8.4.2
PATCH v3 1/3 drm/i915: Track the enabled PM interrupts in dev_priv.
From: Deepak S <[email protected]>
When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in dev_priv to track the enabled PM interrupts
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 14 +++++++------- drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++----- 3 files changed, 17 insertions(+), 12 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -1487,6 +1487,7 @@ typedef struct drm_i915_private { };u32 gt_irq_mask;u32 pm_irq_mask; +u32 pm_rps_events;u32 pipestat_irq_mask[I915_MAX_PIPES ];struct work_struct hotplug_work;
drivers/gpu/drm/i915/i915_irq.c +7 -7
@@ -1132,13 +1132,13 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv->rps.pm_iir; dev_priv->rps.pm_iir =0 ;/* Make sure not to corrupt PMIMR state used by ringbuffer code */ -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* Make sure we didn't queue anything we're not going to process. */ -WARN_ON (pm_iir & ~GEN6_PM_RPS_EVENTS ); +WARN_ON (pm_iir & ~dev_priv->pm_rps_events); -if ((pm_iir &GEN6_PM_RPS_EVENTS ) ==0 ) +if ((pm_iir & dev_priv->pm_rps_events) ==0 )return ;mutex_lock (&dev_priv->rps.hw_lock);
@@ -1555,10 +1555,10 @@ static void i9xx_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe)* the work queue. */ static void gen6_rps_irq_handler (struct drm_i915_private *dev_priv,u32 pm_iir) { -if (pm_iir &GEN6_PM_RPS_EVENTS ) { +if (pm_iir & dev_priv->pm_rps_events) {spin_lock (&dev_priv->irq_lock); - dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RPS_EVENTS ; -snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RPS_EVENTS ); + dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events; +snb_disable_pm_irq (dev_priv, pm_iir & dev_priv->pm_rps_events);spin_unlock (&dev_priv->irq_lock);queue_work (dev_priv->wq, &dev_priv->rps.work);
@@ -2983,7 +2983,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)POSTING_READ (GTIER );if (INTEL_INFO (dev)->gen >=6 ) { - pm_irqs |=GEN6_PM_RPS_EVENTS ; + pm_irqs |= dev_priv->pm_rps_events;if (HAS_VEBOX (dev)) pm_irqs |=PM_VEBOX_USER_INTERRUPT ;
drivers/gpu/drm/i915/intel_pm.c +9 -5
@@ -3160,7 +3160,7 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;I915_WRITE (GEN6_PMINTRMSK ,0xffffffff ); -I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~dev_priv->pm_rps_events);/* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving
@@ -3170,7 +3170,7 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev) dev_priv->rps.pm_iir =0 ;spin_unlock_irq (&dev_priv->irq_lock); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events); }static void gen6_disable_rps (struct drm_device *dev)
@@ -3232,12 +3232,12 @@ static void gen6_enable_rps_interrupts(struct drm_device *dev)spin_lock_irq (&dev_priv->irq_lock);WARN_ON (dev_priv->rps.pm_iir); -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* only unmask PM interrupts we need. Mask all others. */ - enabled_intrs =GEN6_PM_RPS_EVENTS ; + enabled_intrs = dev_priv->pm_rps_events;/* IVB and SNB hard hangs on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked.
@@ -3311,6 +3311,8 @@ static void gen8_enable_rps(struct drm_device *dev)GEN6_RP_UP_BUSY_AVG | GEN6_RP_DOWN_IDLE_AVG); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +/* 6: Ring frequency + overclocking (our driver does this later */ gen6_set_rps (dev, (I915_READ (GEN6_GT_PERF_STATUS ) &0xff00 ) >>8 );
@@ -3430,6 +3432,7 @@ static void gen6_enable_rps(struct drm_device *dev) dev_priv->rps.power =HIGH_POWER ;/* force a reset */ gen6_set_rps (dev_priv->dev, dev_priv->rps.min_delay); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev); rc6vids =0 ;
@@ -3688,6 +3691,7 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );
Signature · 1 line
1.8.4.2
Re: [PATCH 1/3] drm/i915: Track the enabled PM interrupts in dev_priv.
Quoted 15 lines · On Thu, Mar 13, 2014 at 09:30:16PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in
dev_privto track the enabled PM interruptsSigned-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 14 +++++++------- drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++----- 3 files changed, 17 insertions(+), 12 deletions(-)drivers/gpu/drm/i915/i915_drv.h +0
-0
<snip>
Quoted 18 lines
drivers/gpu/drm/i915/i915_drv.h +3
-0@@ -3311,6 +3311,8 @@ static void gen8_enable_rps(struct drm_device *dev)GEN6_RP_UP_BUSY_AVG | GEN6_RP_DOWN_IDLE_AVG); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +/* 6: Ring frequency + overclocking (our driver does this later */ gen6_set_rps (dev, (I915_READ (GEN6_GT_PERF_STATUS ) &0xff00 ) >>8 );@@ -3430,6 +3432,7 @@ static void gen6_enable_rps(struct drm_device *dev) dev_priv->rps.power =HIGH_POWER ;/* force a reset */ gen6_set_rps (dev_priv->dev, dev_priv->rps.min_delay); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev); rc6vids =0 ;
drivers/gpu/drm/i915/i915_drv.h static void valleyview_enable_rps(struct drm_device *dev) · line 3697
Quoted 7 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -3688,6 +3691,7 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );
I think we need to initialize pm_rps_events somewhere earlier since we depend on it already in irq postinstall. Othwewise the patch looks good.
Signature · 2 lines
Ville Syrjälä Intel OTC
Re: [PATCH 1/3] drm/i915: Track the enabled PM interrupts in dev_priv.
Quoted 38 lines · Ville Syrjälä
Quoted 15 lines · On Thu, Mar 13, 2014 at 09:30:16PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in
dev_privto track the enabled PM interruptsSigned-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 14 +++++++------- drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++----- 3 files changed, 17 insertions(+), 12 deletions(-)drivers/gpu/drm/i915/i915_drv.h +0
-0<snip>
Quoted 21 lines
drivers/gpu/drm/i915/i915_drv.h +3
-0@@ -3311,6 +3311,8 @@ static void gen8_enable_rps(struct drm_device *dev)GEN6_RP_UP_BUSY_AVG | GEN6_RP_DOWN_IDLE_AVG); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +/* 6: Ring frequency + overclocking (our driver does this later */ gen6_set_rps (dev, (I915_READ (GEN6_GT_PERF_STATUS ) &0xff00 ) >>8 );@@ -3430,6 +3432,7 @@ static void gen6_enable_rps(struct drm_device *dev) dev_priv->rps.power =HIGH_POWER ;/* force a reset */ gen6_set_rps (dev_priv->dev, dev_priv->rps.min_delay); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev); rc6vids =0 ;@@ -3688,6 +3691,7 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false;
Quoted 8 lines
drivers/gpu/drm/i915/i915_drv.h static void valleyview_enable_rps(struct drm_device *dev) · line 3697
Quoted 4 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0+ dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );I think we need to initialize
pm_rps_eventssomewhere earlier since we depend on it already in irq postinstall. Othwewise the patch looks good.
Adding it in functions "intel_uncore_early_sanitize" or "pm_init" as this gets executed before irq_install in driver_load?
Re: [PATCH 1/3] drm/i915: Track the enabled PM interrupts in dev_priv.
Quoted 44 lines · Deepak
Quoted 41 lines · Ville Syrjälä
Quoted 15 lines · On Thu, Mar 13, 2014 at 09:30:16PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in
dev_privto track the enabled PM interruptsSigned-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 14 +++++++------- drivers/gpu/drm/i915/intel_pm.c | 14 +++++++++----- 3 files changed, 17 insertions(+), 12 deletions(-)drivers/gpu/drm/i915/i915_drv.h +0
-0<snip>
Quoted 24 lines
drivers/gpu/drm/i915/i915_drv.h +4
-0@@ -3311,6 +3311,8 @@ static void gen8_enable_rps(struct drm_device *dev)GEN6_RP_UP_BUSY_AVG | GEN6_RP_DOWN_IDLE_AVG); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +/* 6: Ring frequency + overclocking (our driver does this later */ gen6_set_rps (dev, (I915_READ (GEN6_GT_PERF_STATUS ) &0xff00 ) >>8 );@@ -3430,6 +3432,7 @@ static void gen6_enable_rps(struct drm_device *dev) dev_priv->rps.power =HIGH_POWER ;/* force a reset */ gen6_set_rps (dev_priv->dev, dev_priv->rps.min_delay); + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev); rc6vids =0 ;@@ -3688,6 +3691,7 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ;gen6_enable_rps_interrupts (dev);
Quoted 7 lines
Quoted 5 lines
drivers/gpu/drm/i915/i915_drv.h static void valleyview_enable_rps(struct drm_device *dev) · line 3697
Quoted 1 line
drivers/gpu/drm/i915/i915_drv.h +0
-0gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );I think we need to initialize
pm_rps_eventssomewhere earlier since we depend on it already in irq postinstall. Othwewise the patch looks good.Adding it in functions "
intel_uncore_early_sanitize" or "pm_init" as this gets executed beforeirq_installindriver_load?
intel_irq_init() might be a good choice since that's where we also initialize the rps.work, and then it's clear it gets executed before any other irq setup code.
Signature · 2 lines
Ville Syrjälä Intel OTC
v4 cover WA for Turbo and RC6 to work together.
From: Deepak S <[email protected]>
This series adds WA patches to enable RC6 and Turbo to work together and also adds a patch to contol the rps boost at runtime
Deepak S (3):
drm/i915: Track the enabled PM interrupts in dev_priv. drm/i915/vlv: WA for Turbo and RC6 to work together. drm/i915: Add boot paramter to control rps boost at boot time.
drivers/gpu/drm/i915/i915_drv.h | 17 +++++ drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 141 +++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_params.c | 5 ++ drivers/gpu/drm/i915/i915_reg.h | 13 +++- drivers/gpu/drm/i915/intel_pm.c | 26 ++++--- 6 files changed, 186 insertions(+), 18 deletions(-)
Signature · 1 line
1.8.4.2
PATCH v2 1/3 drm/i915: Track the enabled PM interrupts in dev_priv.
From: Deepak S <[email protected]>
When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in dev_priv to track the enabled PM interrupts
v2: Initialize pm_rps_events in intel_irq_init() (Ville).
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 17 ++++++++++------- drivers/gpu/drm/i915/intel_pm.c | 11 ++++++----- 3 files changed, 17 insertions(+), 12 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -1487,6 +1487,7 @@ typedef struct drm_i915_private { };u32 gt_irq_mask;u32 pm_irq_mask; +u32 pm_rps_events;u32 pipestat_irq_mask[I915_MAX_PIPES ];struct work_struct hotplug_work;
drivers/gpu/drm/i915/i915_irq.c +10 -7
@@ -1132,13 +1132,13 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv->rps.pm_iir; dev_priv->rps.pm_iir =0 ;/* Make sure not to corrupt PMIMR state used by ringbuffer code */ -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* Make sure we didn't queue anything we're not going to process. */ -WARN_ON (pm_iir & ~GEN6_PM_RPS_EVENTS ); +WARN_ON (pm_iir & ~dev_priv->pm_rps_events); -if ((pm_iir &GEN6_PM_RPS_EVENTS ) ==0 ) +if ((pm_iir & dev_priv->pm_rps_events) ==0 )return ;mutex_lock (&dev_priv->rps.hw_lock);
@@ -1555,10 +1555,10 @@ static void i9xx_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe)* the work queue. */ static void gen6_rps_irq_handler (struct drm_i915_private *dev_priv,u32 pm_iir) { -if (pm_iir &GEN6_PM_RPS_EVENTS ) { +if (pm_iir & dev_priv->pm_rps_events) {spin_lock (&dev_priv->irq_lock); - dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RPS_EVENTS ; -snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RPS_EVENTS ); + dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events; +snb_disable_pm_irq (dev_priv, pm_iir & dev_priv->pm_rps_events);spin_unlock (&dev_priv->irq_lock);queue_work (dev_priv->wq, &dev_priv->rps.work);
@@ -2983,7 +2983,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)POSTING_READ (GTIER );if (INTEL_INFO (dev)->gen >=6 ) { - pm_irqs |=GEN6_PM_RPS_EVENTS ; + pm_irqs |= dev_priv->pm_rps_events;if (HAS_VEBOX (dev)) pm_irqs |=PM_VEBOX_USER_INTERRUPT ;
@@ -4030,6 +4030,9 @@ void intel_irq_init(struct drm_device *dev)INIT_WORK (&dev_priv->rps.work, gen6_pm_rps_work);INIT_WORK (&dev_priv->l3_parity.error_work, ivybridge_parity_work); +/* Let's track the enabled rps events */ + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +setup_timer (&dev_priv->gpu_error.hangcheck_timer, i915_hangcheck_elapsed, (unsigned long ) dev);
drivers/gpu/drm/i915/intel_pm.c +6 -5
@@ -3160,7 +3160,8 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;I915_WRITE (GEN6_PMINTRMSK ,0xffffffff ); -I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~dev_priv->pm_rps_events);/* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving
@@ -3170,7 +3171,7 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev) dev_priv->rps.pm_iir =0 ;spin_unlock_irq (&dev_priv->irq_lock); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events); }static void gen6_disable_rps (struct drm_device *dev)
@@ -3232,12 +3233,12 @@ static void gen6_enable_rps_interrupts(struct drm_device *dev)spin_lock_irq (&dev_priv->irq_lock);WARN_ON (dev_priv->rps.pm_iir); -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* only unmask PM interrupts we need. Mask all others. */ - enabled_intrs =GEN6_PM_RPS_EVENTS ; + enabled_intrs = dev_priv->pm_rps_events;/* IVB and SNB hard hangs on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked.
Signature · 1 line
1.8.4.2
Re: [PATCH v2 1/3] drm/i915: Track the enabled PM interrupts in dev_priv.
Quoted 9 lines · On Sat, Mar 15, 2014 at 08:23:22PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in
dev_privto track the enabled PM interruptsv2: Initialize
pm_rps_eventsinintel_irq_init()(Ville).Signed-off-by Deepak S <[email protected]>
Quoted 118 lines
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 17 ++++++++++------- drivers/gpu/drm/i915/intel_pm.c | 11 ++++++----- 3 files changed, 17 insertions(+), 12 deletions(-)drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -1487,6 +1487,7 @@ typedef struct drm_i915_private { };u32 gt_irq_mask;u32 pm_irq_mask; +u32 pm_rps_events;u32 pipestat_irq_mask[I915_MAX_PIPES ];struct work_struct hotplug_work;drivers/gpu/drm/i915/i915_irq.c +10
-7@@ -1132,13 +1132,13 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv->rps.pm_iir; dev_priv->rps.pm_iir =0 ;/* Make sure not to corrupt PMIMR state used by ringbuffer code */ -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* Make sure we didn't queue anything we're not going to process. */ -WARN_ON (pm_iir & ~GEN6_PM_RPS_EVENTS ); +WARN_ON (pm_iir & ~dev_priv->pm_rps_events); -if ((pm_iir &GEN6_PM_RPS_EVENTS ) ==0 ) +if ((pm_iir & dev_priv->pm_rps_events) ==0 )return ;mutex_lock (&dev_priv->rps.hw_lock);@@ -1555,10 +1555,10 @@ static void i9xx_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe)* the work queue. */ static void gen6_rps_irq_handler (struct drm_i915_private *dev_priv,u32 pm_iir) { -if (pm_iir &GEN6_PM_RPS_EVENTS ) { +if (pm_iir & dev_priv->pm_rps_events) {spin_lock (&dev_priv->irq_lock); - dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RPS_EVENTS ; -snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RPS_EVENTS ); + dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events; +snb_disable_pm_irq (dev_priv, pm_iir & dev_priv->pm_rps_events);spin_unlock (&dev_priv->irq_lock);queue_work (dev_priv->wq, &dev_priv->rps.work);@@ -2983,7 +2983,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)POSTING_READ (GTIER );if (INTEL_INFO (dev)->gen >=6 ) { - pm_irqs |=GEN6_PM_RPS_EVENTS ; + pm_irqs |= dev_priv->pm_rps_events;if (HAS_VEBOX (dev)) pm_irqs |=PM_VEBOX_USER_INTERRUPT ;@@ -4030,6 +4030,9 @@ void intel_irq_init(struct drm_device *dev)INIT_WORK (&dev_priv->rps.work, gen6_pm_rps_work);INIT_WORK (&dev_priv->l3_parity.error_work, ivybridge_parity_work); +/* Let's track the enabled rps events */ + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +setup_timer (&dev_priv->gpu_error.hangcheck_timer, i915_hangcheck_elapsed, (unsigned long ) dev);drivers/gpu/drm/i915/intel_pm.c +6
-5@@ -3160,7 +3160,8 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;I915_WRITE (GEN6_PMINTRMSK ,0xffffffff ); -I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~dev_priv->pm_rps_events);/* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving @@ -3170,7 +3171,7 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev) dev_priv->rps.pm_iir =0 ;spin_unlock_irq (&dev_priv->irq_lock); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events); }static void gen6_disable_rps (struct drm_device *dev)@@ -3232,12 +3233,12 @@ static void gen6_enable_rps_interrupts(struct drm_device *dev)spin_lock_irq (&dev_priv->irq_lock);WARN_ON (dev_priv->rps.pm_iir); -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* only unmask PM interrupts we need. Mask all others. */ - enabled_intrs =GEN6_PM_RPS_EVENTS ; + enabled_intrs = dev_priv->pm_rps_events;/* IVB and SNB hard hangs on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked. Signature · 6 lines
1.8.4.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
Daniel Vetter
AppliedRe: [PATCH v2 1/3] drm/i915: Track the enabled PM interrupts in dev_priv.
Quoted 6 lines · Ville Syrjälä
Quoted 5 lines · On Sat, Mar 15, 2014 at 08:23:22PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we use different rps events for different platform or due to wa, we mgiht end up doing (vs) everywahere. Insted of this, Let's use a variable in
dev_privto track the enabled PM interrupts
I didn't follow this really, so I've reworded it a bit and fixed the spelling - or tried to at least ;-)
Quoted 6 lines
Quoted 4 lines
v2: Initialize
pm_rps_eventsinintel_irq_init()(Ville).Signed-off-by Deepak S <[email protected]>Reviewed-by Ville Syrjälä <[email protected]>
Queued for -next, thanks for the patch.
-Daniel
Quoted 127 lines
Quoted 119 lines
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_irq.c | 17 ++++++++++------- drivers/gpu/drm/i915/intel_pm.c | 11 ++++++----- 3 files changed, 17 insertions(+), 12 deletions(-)drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -1487,6 +1487,7 @@ typedef struct drm_i915_private { };u32 gt_irq_mask;u32 pm_irq_mask; +u32 pm_rps_events;u32 pipestat_irq_mask[I915_MAX_PIPES ];struct work_struct hotplug_work;drivers/gpu/drm/i915/i915_irq.c +10
-7@@ -1132,13 +1132,13 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir = dev_priv->rps.pm_iir; dev_priv->rps.pm_iir =0 ;/* Make sure not to corrupt PMIMR state used by ringbuffer code */ -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* Make sure we didn't queue anything we're not going to process. */ -WARN_ON (pm_iir & ~GEN6_PM_RPS_EVENTS ); +WARN_ON (pm_iir & ~dev_priv->pm_rps_events); -if ((pm_iir &GEN6_PM_RPS_EVENTS ) ==0 ) +if ((pm_iir & dev_priv->pm_rps_events) ==0 )return ;mutex_lock (&dev_priv->rps.hw_lock);@@ -1555,10 +1555,10 @@ static void i9xx_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe)* the work queue. */ static void gen6_rps_irq_handler (struct drm_i915_private *dev_priv,u32 pm_iir) { -if (pm_iir &GEN6_PM_RPS_EVENTS ) { +if (pm_iir & dev_priv->pm_rps_events) {spin_lock (&dev_priv->irq_lock); - dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RPS_EVENTS ; -snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RPS_EVENTS ); + dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events; +snb_disable_pm_irq (dev_priv, pm_iir & dev_priv->pm_rps_events);spin_unlock (&dev_priv->irq_lock);queue_work (dev_priv->wq, &dev_priv->rps.work);@@ -2983,7 +2983,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)POSTING_READ (GTIER );if (INTEL_INFO (dev)->gen >=6 ) { - pm_irqs |=GEN6_PM_RPS_EVENTS ; + pm_irqs |= dev_priv->pm_rps_events;if (HAS_VEBOX (dev)) pm_irqs |=PM_VEBOX_USER_INTERRUPT ;@@ -4030,6 +4030,9 @@ void intel_irq_init(struct drm_device *dev)INIT_WORK (&dev_priv->rps.work, gen6_pm_rps_work);INIT_WORK (&dev_priv->l3_parity.error_work, ivybridge_parity_work); +/* Let's track the enabled rps events */ + dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +setup_timer (&dev_priv->gpu_error.hangcheck_timer, i915_hangcheck_elapsed, (unsigned long ) dev);drivers/gpu/drm/i915/intel_pm.c +6
-5@@ -3160,7 +3160,8 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev)struct drm_i915_private *dev_priv = dev->dev_private;I915_WRITE (GEN6_PMINTRMSK ,0xffffffff ); -I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & ~GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIER ,I915_READ (GEN6_PMIER ) & + ~dev_priv->pm_rps_events);/* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving @@ -3170,7 +3171,7 @@ static void gen6_disable_rps_interrupts(struct drm_device *dev) dev_priv->rps.pm_iir =0 ;spin_unlock_irq (&dev_priv->irq_lock); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events); }static void gen6_disable_rps (struct drm_device *dev)@@ -3232,12 +3233,12 @@ static void gen6_enable_rps_interrupts(struct drm_device *dev)spin_lock_irq (&dev_priv->irq_lock);WARN_ON (dev_priv->rps.pm_iir); -snb_enable_pm_irq (dev_priv,GEN6_PM_RPS_EVENTS ); -I915_WRITE (GEN6_PMIIR ,GEN6_PM_RPS_EVENTS ); +snb_enable_pm_irq (dev_priv, dev_priv->pm_rps_events); +I915_WRITE (GEN6_PMIIR , dev_priv->pm_rps_events);spin_unlock_irq (&dev_priv->irq_lock);/* only unmask PM interrupts we need. Mask all others. */ - enabled_intrs =GEN6_PM_RPS_EVENTS ; + enabled_intrs = dev_priv->pm_rps_events;/* IVB and SNB hard hangs on looping batchbuffer * if GEN6_PM_UP_EI_EXPIRED is masked. Signature · 6 lines
1.8.4.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfxSignature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ 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 v4 2/3 drm/i915/vlv: WA for Turbo and RC6 to work together.
In reply to [email protected]From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
v2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
v4: Enable required counters and remove unwanted code (Ville)
--- drivers/gpu/drm/i915/i915_drv.h | 15 +++++ drivers/gpu/drm/i915/i915_irq.c | 126 +++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_reg.h | 13 ++++- drivers/gpu/drm/i915/intel_pm.c | 15 +++-- 4 files changed, 163 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +15 -0
@@ -974,6 +974,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;
@@ -990,6 +996,8 @@ struct intel_gen6_power_mgmt {bool rp_up_masked;bool rp_down_masked; +u32 ei_interrupt_count; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;
@@ -1583,6 +1591,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;
drivers/gpu/drm/i915/i915_irq.c +125 -1
150 lines not on this page · raw message
drivers/gpu/drm/i915/i915_reg.h +12 -1
@@ -419,6 +419,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */
@@ -434,6 +435,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3
@@ -4892,6 +4898,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2
@@ -5019,13 +5026,17 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104 +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FF#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xc
drivers/gpu/drm/i915/intel_pm.c +11 -4
@@ -3096,10 +3096,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)
@@ -3618,6 +3622,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |
@@ -3638,9 +3643,11 @@ static void valleyview_enable_rps(struct drm_device *dev)/* allows RC6 residency counter to work */ I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | +_MASKED_BIT_ENABLE (VLV_MEDIA_RC0_COUNT_EN | +VLV_RENDER_RC0_COUNT_EN |VLV_MEDIA_RC6_COUNT_EN |VLV_RENDER_RC6_COUNT_EN )); +if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;
Signature · 1 line
1.8.4.2
Quoted 98 lines · On Sat, Mar 15, 2014 at 08:23:23PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these valuesv2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
v4: Enable required counters and remove unwanted code (Ville)
Signed-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 15 +++++ drivers/gpu/drm/i915/i915_irq.c | 126 +++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_reg.h | 13 ++++- drivers/gpu/drm/i915/intel_pm.c | 15 +++-- 4 files changed, 163 insertions(+), 6 deletions(-)drivers/gpu/drm/i915/i915_drv.h +15
-0@@ -974,6 +974,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;@@ -990,6 +996,8 @@ struct intel_gen6_power_mgmt {bool rp_up_masked;bool rp_down_masked; +u32 ei_interrupt_count; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;@@ -1583,6 +1591,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;drivers/gpu/drm/i915/i915_irq.c +27
-0@@ -1121,6 +1121,123 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv, } } +static u32 vlv_c0_residency (struct drm_i915_private *dev_priv, +struct intel_rps_ei_calc *rps_ei) +{ +u32 cz_ts, cz_freq_khz; +u32 render_count, media_count; +u32 elapsed_render, elapsed_media, elapsed_time; +u32 residency =0 ; + + cz_ts =vlv_punit_read (dev_priv,PUNIT_REG_CZ_TIMESTAMP ); + cz_freq_khz =DIV_ROUND_CLOSEST (dev_priv->mem_freq *1000 ,4 ); + + render_count =I915_READ (VLV_RENDER_C0_COUNT_REG ); + media_count =I915_READ (VLV_MEDIA_C0_COUNT_REG ); + +if (rps_ei->cz_ts_ei ==0 ) { + rps_ei->cz_ts_ei = cz_ts; + rps_ei->render_ei_c0 = render_count; + rps_ei->media_ei_c0 = media_count; + +return dev_priv->rps.cur_delay; + } + + elapsed_time = cz_ts - rps_ei->cz_ts_ei; + rps_ei->cz_ts_ei = cz_ts; + + elapsed_render = render_count - rps_ei->render_ei_c0; + rps_ei->render_ei_c0 = render_count;
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1160
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+ + elapsed_media = media_count - rps_ei->media_ei_c0; + rps_ei->media_ei_c0 = media_count; + +/* Convert all the counters into common unit of milli sec */ + elapsed_time /=VLV_CZ_CLOCK_TO_MILLI_SEC ; + elapsed_render /= cz_freq_khz; + elapsed_media /= cz_freq_khz; + +/**
This isn't a kernel-doc comment so the double ** shouldn't be here.
Quoted 27 lines
drivers/gpu/drm/i915/i915_irq.c +27
-0+* Calculate overall C0 residency percentage +* only if elapsed time is non zero +*/ +if (elapsed_time) { + residency = + ((max (elapsed_render, elapsed_media) *100 ) + / elapsed_time); + } + +return residency; +} + + +/** +* vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU +* busy-ness calculated from C0 counters of render & media power wells +* @dev_priv: DRM device private +* +*/ +static u32 vlv_calc_delay_from_C0_counters (struct drm_i915_private *dev_priv) +{ +u32 residency_C0_up =0 , residency_C0_down =0 ; +u8 new_delay; + + dev_priv->rps.ei_interrupt_count++; + +WARN_ON (!mutex_is_locked (&dev_priv->rps.hw_lock));
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1197
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+ + +if (dev_priv->rps_up_ei.cz_ts_ei ==0 ) { +vlv_c0_residency (dev_priv, &dev_priv->rps_up_ei); +vlv_c0_residency (dev_priv, &dev_priv->rps_down_ei); +return dev_priv->rps.cur_delay; + } + + +/**
here too
Quoted 14 lines
drivers/gpu/drm/i915/i915_irq.c +14
-0+* To down throttle, C0 residency should be less than down threshold +* for continous EI intervals. So calculate down EI counters +* once in VLV_INT_COUNT_FOR_DOWN_EI +*/ +if (dev_priv->rps.ei_interrupt_count ==VLV_INT_COUNT_FOR_DOWN_EI ) { + + dev_priv->rps.ei_interrupt_count =0 ; + + residency_C0_down =vlv_c0_residency (dev_priv, + &dev_priv->rps_down_ei); + }else { + residency_C0_up =vlv_c0_residency (dev_priv, + &dev_priv->rps_up_ei); + }
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1221
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +10
-0+ + new_delay = dev_priv->rps.cur_delay; + +/* C0 residency is greater than UP threshold. Increase Frequency */ +if (residency_C0_up >=VLV_RP_UP_EI_THRESHOLD ) { + +if (dev_priv->rps.cur_delay < dev_priv->rps.max_delay) + new_delay = dev_priv->rps.cur_delay +1 ; + +/**
and here
drivers/gpu/drm/i915/i915_irq.c void gen6_set_pm_mask(struct drm_i915_private *dev_priv, · line 1230
Quoted 9 lines
drivers/gpu/drm/i915/i915_irq.c +9
-0+* For better performance, jump directly +* to RPe if we're below it. +*/ +if (new_delay < dev_priv->rps.rpe_delay) + new_delay = dev_priv->rps.rpe_delay; + + }else if (!dev_priv->rps.ei_interrupt_count && + (residency_C0_down <VLV_RP_DOWN_EI_THRESHOLD )) { +/**
ditto
Quoted 31 lines
drivers/gpu/drm/i915/i915_irq.c +13
-0+* This means, C0 residency is less than down threshold over +* a period of VLV_INT_COUNT_FOR_DOWN_EI. So, reduce the freq +*/ +if (dev_priv->rps.cur_delay > dev_priv->rps.min_delay) + new_delay = dev_priv->rps.cur_delay -1 ; + } + +return new_delay; +} +static void gen6_pm_rps_work (struct work_struct *work) { drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,@@ -1163,6 +1280,8 @@ static void gen6_pm_rps_work(struct work_struct *work)else new_delay = dev_priv->rps.min_delay; adj =0 ; + }else if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { + new_delay =vlv_calc_delay_from_C0_counters (dev_priv); }else if (pm_iir &GEN6_PM_RP_DOWN_THRESHOLD ) {if (adj <0 ) adj *=2 ;@@ -2989,6 +3108,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) pm_irqs |=PM_VEBOX_USER_INTERRUPT ; dev_priv->pm_irq_mask =0xffffffff ; +I915_WRITE (GEN6_PMIIR ,I915_READ (GEN6_PMIIR ));I915_WRITE (GEN6_PMIMR , dev_priv->pm_irq_mask);I915_WRITE (GEN6_PMIER , pm_irqs);
drivers/gpu/drm/i915/i915_irq.c void intel_irq_init(struct drm_device *dev) · line 4155
Quoted 6 lines
drivers/gpu/drm/i915/i915_irq.c +2
-1@@ -4031,7 +4151,11 @@ void intel_irq_init(struct drm_device *dev)INIT_WORK (&dev_priv->l3_parity.error_work, ivybridge_parity_work);/* Let's track the enabled rps events */ - dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +if (IS_VALLEYVIEW (dev)) +/* WAUseRC0ResidenncyTurbo:VLV */
I can't see this name in the database (even with the typo fixed). I do see something called WaGsvRC0ResidencyMethod:vlv which I'm assuming is the right one.
So apart from the minor nits I listed, my only two remaining issues here are whether we want to make this optional or not, and whether we should apply acceleration to the up/down transitions like gen6_pm_rps_work() does (to ramp the freq up/down faster). I forget if you tested how that affects the power consumption numbers, or if you just tested the effect of the __wait_seqno() boost mechanism.
Quoted 107 lines
drivers/gpu/drm/i915/i915_irq.c +3
-0+ dev_priv->pm_rps_events =GEN6_PM_RP_UP_EI_EXPIRED ; + else + dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS; setup_timer(&dev_priv->gpu_error.hangcheck_timer, i915_hangcheck_elapsed,drivers/gpu/drm/i915/i915_reg.h +12
-1@@ -419,6 +419,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */ @@ -434,6 +435,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3@@ -4892,6 +4898,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2@@ -5019,13 +5026,17 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104 +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FF#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xcdrivers/gpu/drm/i915/intel_pm.c +11
-4@@ -3096,10 +3096,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)@@ -3618,6 +3622,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |@@ -3638,9 +3643,11 @@ static void valleyview_enable_rps(struct drm_device *dev)/* allows RC6 residency counter to work */ I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | +_MASKED_BIT_ENABLE (VLV_MEDIA_RC0_COUNT_EN | +VLV_RENDER_RC0_COUNT_EN |VLV_MEDIA_RC6_COUNT_EN |VLV_RENDER_RC6_COUNT_EN )); +if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;Signature · 6 lines
1.8.4.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
PATCH v3 3/3 drm/i915: Add boot paramter to control rps boost at boot time.
In reply to [email protected]From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
v2: Addressed rps default comment (Jani)
v3: Use bool to represent the boot parameter (Ville).
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -2120,6 +2120,7 @@ struct i915_params {bool prefault_disable;bool reset;bool disable_display; +bool enable_rps_boost; };extern struct i915_params i915__read_mostly ;
drivers/gpu/drm/i915/i915_gem.c +1 -1
@@ -1042,7 +1042,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, timeout_expire = timeout ? jiffies +timespec_to_jiffies_timeout (timeout) :0 ; -if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv)) { +if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv) && i915.enable_rps_boost) {gen6_rps_boost (dev_priv);if (file_priv) mod_delayed_work(dev_priv->wq,
drivers/gpu/drm/i915/i915_params.c +5 -0
@@ -49,6 +49,7 @@ struct i915_params i915 __read_mostly = { .invert_brightness = 0, .disable_display = 0, .enable_cmd_parser = 0, + .enable_rps_boost = true, };module_param_named (modeset, i915.modeset, int,0400 );
@@ -162,3 +163,7 @@ MODULE_PARM_DESC(disable_display, "Disable display (default: false)");module_param_named (enable_cmd_parser, i915.enable_cmd_parser, int,0600 );MODULE_PARM_DESC (enable_cmd_parser,"Enable command parsing (1=enabled, 0=disabled [default])" ); + +module_param_named (enable_rps_boost, i915.enable_rps_boost, bool,0600 ); +MODULE_PARM_DESC (enable_rps_boost, +"Enable/Disable boost RPS frequency (default: true)" );
Signature · 1 line
1.8.4.2
Re: [PATCH v3 3/3] drm/i915: Add boot paramter to control rps boost at boot time.
Quoted 11 lines · On Sat, Mar 15, 2014 at 08:23:24PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
v2: Addressed rps default comment (Jani)
v3: Use bool to represent the boot parameter (Ville).
Signed-off-by Deepak S <[email protected]>
Quoted 58 lines
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -2120,6 +2120,7 @@ struct i915_params {bool prefault_disable;bool reset;bool disable_display; +bool enable_rps_boost; };extern struct i915_params i915__read_mostly ;drivers/gpu/drm/i915/i915_gem.c +1
-1@@ -1042,7 +1042,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, timeout_expire = timeout ? jiffies +timespec_to_jiffies_timeout (timeout) :0 ; -if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv)) { +if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv) && i915.enable_rps_boost) {gen6_rps_boost (dev_priv);if (file_priv) mod_delayed_work(dev_priv->wq,drivers/gpu/drm/i915/i915_params.c +5
-0@@ -49,6 +49,7 @@ struct i915_params i915 __read_mostly = { .invert_brightness = 0, .disable_display = 0, .enable_cmd_parser = 0, + .enable_rps_boost = true, };module_param_named (modeset, i915.modeset, int,0400 );@@ -162,3 +163,7 @@ MODULE_PARM_DESC(disable_display, "Disable display (default: false)");module_param_named (enable_cmd_parser, i915.enable_cmd_parser, int,0600 );MODULE_PARM_DESC (enable_cmd_parser,"Enable command parsing (1=enabled, 0=disabled [default])" ); + +module_param_named (enable_rps_boost, i915.enable_rps_boost, bool,0600 ); +MODULE_PARM_DESC (enable_rps_boost, +"Enable/Disable boost RPS frequency (default: true)" );Signature · 6 lines
1.8.4.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
PATCH v5 1/1 drm/i915/vlv: WA for Turbo and RC6 to work together.
In reply to [email protected]From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
v2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
v4: Enable required counters and remove unwanted code (Ville)
v5: Resolved comments and remove kernel-doc style comments. (Ville)
--- drivers/gpu/drm/i915/i915_drv.h | 15 +++++ drivers/gpu/drm/i915/i915_irq.c | 135 +++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_reg.h | 12 +++- drivers/gpu/drm/i915/intel_pm.c | 13 +++- 4 files changed, 170 insertions(+), 5 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +15 -0
@@ -816,6 +816,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;
@@ -843,6 +849,8 @@ struct intel_gen6_power_mgmt {bool rp_up_masked;bool rp_down_masked; +u32 ei_interrupt_count; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;
@@ -1403,6 +1411,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;
drivers/gpu/drm/i915/i915_irq.c +134 -1
159 lines not on this page · raw message
drivers/gpu/drm/i915/i915_reg.h +11 -1
@@ -506,6 +506,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */
@@ -521,6 +522,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3
@@ -4984,6 +4990,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2
@@ -5112,12 +5119,15 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xc
drivers/gpu/drm/i915/intel_pm.c +10 -3
@@ -3097,9 +3097,13 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) ~VLV_GFX_CLK_FORCE_ON_BIT);/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_freq_softlimit); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)
@@ -3619,6 +3623,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |
@@ -3639,9 +3644,11 @@ static void valleyview_enable_rps(struct drm_device *dev)/* allows RC6 residency counter to work */ I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | +_MASKED_BIT_ENABLE (VLV_MEDIA_RC0_COUNT_EN | +VLV_RENDER_RC0_COUNT_EN |VLV_MEDIA_RC6_COUNT_EN |VLV_RENDER_RC6_COUNT_EN )); +if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;
Signature · 1 line
1.9.1
Quoted 9 lines · On Thu, Mar 27, 2014 at 12:05:01PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
Quoted 7 lines
v2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
v4: Enable required counters and remove unwanted code (Ville)
v5: Resolved comments and remove kernel-doc style comments. (Ville)
That doesn't tell me that you added the frequency change acceleration support.
Quoted 204 lines
Signed-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 15 +++++ drivers/gpu/drm/i915/i915_irq.c | 135 +++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_reg.h | 12 +++- drivers/gpu/drm/i915/intel_pm.c | 13 +++- 4 files changed, 170 insertions(+), 5 deletions(-)drivers/gpu/drm/i915/i915_drv.h +15
-0@@ -816,6 +816,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;@@ -843,6 +849,8 @@ struct intel_gen6_power_mgmt {bool rp_up_masked;bool rp_down_masked; +u32 ei_interrupt_count; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;@@ -1403,6 +1411,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;drivers/gpu/drm/i915/i915_irq.c +129
-0@@ -1121,6 +1121,132 @@ void gen6_set_pm_mask(struct drm_i915_private *dev_priv, } } +static u32 vlv_c0_residency (struct drm_i915_private *dev_priv, +struct intel_rps_ei_calc *rps_ei) +{ +u32 cz_ts, cz_freq_khz; +u32 render_count, media_count; +u32 elapsed_render, elapsed_media, elapsed_time; +u32 residency =0 ; + + cz_ts =vlv_punit_read (dev_priv,PUNIT_REG_CZ_TIMESTAMP ); + cz_freq_khz =DIV_ROUND_CLOSEST (dev_priv->mem_freq *1000 ,4 ); + + render_count =I915_READ (VLV_RENDER_C0_COUNT_REG ); + media_count =I915_READ (VLV_MEDIA_C0_COUNT_REG ); + +if (rps_ei->cz_ts_ei ==0 ) { + rps_ei->cz_ts_ei = cz_ts; + rps_ei->render_ei_c0 = render_count; + rps_ei->media_ei_c0 = media_count; + +return dev_priv->rps.cur_freq; + } + + elapsed_time = cz_ts - rps_ei->cz_ts_ei; + rps_ei->cz_ts_ei = cz_ts; + + elapsed_render = render_count - rps_ei->render_ei_c0; + rps_ei->render_ei_c0 = render_count; + + elapsed_media = media_count - rps_ei->media_ei_c0; + rps_ei->media_ei_c0 = media_count; + +/* Convert all the counters into common unit of milli sec */ + elapsed_time /=VLV_CZ_CLOCK_TO_MILLI_SEC ; + elapsed_render /= cz_freq_khz; + elapsed_media /= cz_freq_khz; + +/* +* Calculate overall C0 residency percentage +* only if elapsed time is non zero +*/ +if (elapsed_time) { + residency = + ((max (elapsed_render, elapsed_media) *100 ) + / elapsed_time); + } + +return residency; +} + + +/** +* vlv_calc_delay_from_C0_counters - Increase/Decrease freq based on GPU +* busy-ness calculated from C0 counters of render & media power wells +* @dev_priv: DRM device private +* +*/ +static u32 vlv_calc_delay_from_C0_counters (struct drm_i915_private *dev_priv) +{ +u32 residency_C0_up =0 , residency_C0_down =0 ; +u8 new_delay, adj; + + dev_priv->rps.ei_interrupt_count++; + +WARN_ON (!mutex_is_locked (&dev_priv->rps.hw_lock)); + + +if (dev_priv->rps_up_ei.cz_ts_ei ==0 ) { +vlv_c0_residency (dev_priv, &dev_priv->rps_up_ei); +vlv_c0_residency (dev_priv, &dev_priv->rps_down_ei); +return dev_priv->rps.cur_freq; + } + + +/* +* To down throttle, C0 residency should be less than down threshold +* for continous EI intervals. So calculate down EI counters +* once in VLV_INT_COUNT_FOR_DOWN_EI +*/ +if (dev_priv->rps.ei_interrupt_count ==VLV_INT_COUNT_FOR_DOWN_EI ) { + + dev_priv->rps.ei_interrupt_count =0 ; + + residency_C0_down =vlv_c0_residency (dev_priv, + &dev_priv->rps_down_ei); + }else { + residency_C0_up =vlv_c0_residency (dev_priv, + &dev_priv->rps_up_ei); + } + + new_delay = dev_priv->rps.cur_freq; + + adj = dev_priv->rps.last_adj; +/* C0 residency is greater than UP threshold. Increase Frequency */ +if (residency_C0_up >=VLV_RP_UP_EI_THRESHOLD ) { +if (adj >0 ) + adj *=2 ; +else + adj =1 ; + +if (dev_priv->rps.cur_freq < dev_priv->rps.max_freq_softlimit) + new_delay = dev_priv->rps.cur_freq + adj; + +/* +* For better performance, jump directly +* to RPe if we're below it. +*/ +if (new_delay < dev_priv->rps.efficient_freq) + new_delay = dev_priv->rps.efficient_freq; + + }else if (!dev_priv->rps.ei_interrupt_count && + (residency_C0_down <VLV_RP_DOWN_EI_THRESHOLD )) { +if (adj <0 ) + adj *=2 ; +else + adj =-1 ; +/* +* This means, C0 residency is less than down threshold over +* a period of VLV_INT_COUNT_FOR_DOWN_EI. So, reduce the freq +*/ +if (dev_priv->rps.cur_freq > dev_priv->rps.max_freq_softlimit) + new_delay = dev_priv->rps.cur_freq + adj; + } + +return new_delay; +} +static void gen6_pm_rps_work (struct work_struct *work) { drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,@@ -1163,6 +1289,8 @@ static void gen6_pm_rps_work(struct work_struct *work)else new_delay = dev_priv->rps.min_freq_softlimit; adj =0 ; + }else if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { + new_delay =vlv_calc_delay_from_C0_counters (dev_priv); }else if (pm_iir &GEN6_PM_RP_DOWN_THRESHOLD ) {if (adj <0 ) adj *=2 ;@@ -3053,6 +3181,7 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) pm_irqs |=PM_VEBOX_USER_INTERRUPT ; dev_priv->pm_irq_mask =0xffffffff ; +I915_WRITE (GEN6_PMIIR ,I915_READ (GEN6_PMIIR ));I915_WRITE (GEN6_PMIMR , dev_priv->pm_irq_mask);I915_WRITE (GEN6_PMIER , pm_irqs);
drivers/gpu/drm/i915/i915_irq.c void intel_irq_init(struct drm_device *dev) · line 4228
Quoted 6 lines
drivers/gpu/drm/i915/i915_irq.c +2
-1@@ -4095,7 +4224,11 @@ void intel_irq_init(struct drm_device *dev)INIT_WORK (&dev_priv->l3_parity.error_work, ivybridge_parity_work);/* Let's track the enabled rps events */ - dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +if (IS_VALLEYVIEW (dev)) +/* WAUseRC0ResidenncyTurbo:VLV */
Looks like you forgot to fix the w/a comment.
Otherwise the patch looks good to me, so if you fix those two small issues you can add:
Quoted 103 lines
drivers/gpu/drm/i915/i915_irq.c +3
-0+ dev_priv->pm_rps_events =GEN6_PM_RP_UP_EI_EXPIRED ; + else + dev_priv->pm_rps_events = GEN6_PM_RPS_EVENTS; setup_timer(&dev_priv->gpu_error.hangcheck_timer, i915_hangcheck_elapsed,drivers/gpu/drm/i915/i915_reg.h +11
-1@@ -506,6 +506,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */ @@ -521,6 +522,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3@@ -4984,6 +4990,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2@@ -5112,12 +5119,15 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xcdrivers/gpu/drm/i915/intel_pm.c +10
-3@@ -3097,9 +3097,13 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) ~VLV_GFX_CLK_FORCE_ON_BIT);/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_freq_softlimit); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)@@ -3619,6 +3623,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |@@ -3639,9 +3644,11 @@ static void valleyview_enable_rps(struct drm_device *dev)/* allows RC6 residency counter to work */ I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | +_MASKED_BIT_ENABLE (VLV_MEDIA_RC0_COUNT_EN | +VLV_RENDER_RC0_COUNT_EN |VLV_MEDIA_RC6_COUNT_EN |VLV_RENDER_RC6_COUNT_EN )); +if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
Quoted 10 lines · Ville Syrjälä
drivers/gpu/drm/i915/i915_drv.h typedef struct drm_i915_private { · line 1418
Quoted 9 lines · On Thu, Mar 27, 2014 at 12:05:01PM +0530, [email protected] wrote:
drivers/gpu/drm/i915/i915_drv.h +5
-0@@ -1403,6 +1411,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei;
I could have sworn there was a field for holding all the interesting rps state together.
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Quoted 6 lines · Chris Wilson
Quoted 5 lines · Ville Syrjälä
Quoted 4 lines · On Thu, Mar 27, 2014 at 12:05:01PM +0530, [email protected] wrote:
drivers/gpu/drm/i915/i915_drv.h +0
-0@@ -1403,6 +1411,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps;
Quoted 9 lines
Quoted 5 lines
drivers/gpu/drm/i915/i915_drv.h typedef struct drm_i915_private { · line 1418
Quoted 5 lines
drivers/gpu/drm/i915/i915_drv.h +5
-0+/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei;I could have sworn there was a field for holding all the interesting rps state together.
-Chris
Hi Chris,
Earlier i was using the rps structure to hold the wa rps state, but there was not of duplicated code to avoid that i created a separate structure. We can still re-factor and use rps structure to hold the wa state. Let me know if we need to created a separate patch to re-factor or add to the WA patch itself.
I a thinking of adding a new patch on top this.
Thanks
PATCH v6 1/1 drm/i915/vlv: WA for Turbo and RC6 to work together.
In reply to [email protected]From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
v2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
v4: Enable required counters and remove unwanted code (Ville)
v5: Added frequency change acceleration support and remove kernel-doc style comments. (Ville)
v6: Updated comment section and Fix w/a comment. (Ville)
--- drivers/gpu/drm/i915/i915_drv.h | 15 +++++ drivers/gpu/drm/i915/i915_irq.c | 135 +++++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/i915_reg.h | 12 +++- drivers/gpu/drm/i915/intel_pm.c | 13 +++- 4 files changed, 170 insertions(+), 5 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +15 -0
@@ -816,6 +816,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;
@@ -843,6 +849,8 @@ struct intel_gen6_power_mgmt {bool rp_up_masked;bool rp_down_masked; +u32 ei_interrupt_count; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;
@@ -1403,6 +1411,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;
drivers/gpu/drm/i915/i915_irq.c +134 -1
159 lines not on this page · raw message
drivers/gpu/drm/i915/i915_reg.h +11 -1
@@ -506,6 +506,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */
@@ -521,6 +522,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3
@@ -4984,6 +4990,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2
@@ -5112,12 +5119,15 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xc
drivers/gpu/drm/i915/intel_pm.c +10 -3
@@ -3097,9 +3097,13 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) ~VLV_GFX_CLK_FORCE_ON_BIT);/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_freq_softlimit); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)
@@ -3619,6 +3623,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |
@@ -3639,9 +3644,11 @@ static void valleyview_enable_rps(struct drm_device *dev)/* allows RC6 residency counter to work */ I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | +_MASKED_BIT_ENABLE (VLV_MEDIA_RC0_COUNT_EN | +VLV_RENDER_RC0_COUNT_EN |VLV_MEDIA_RC6_COUNT_EN |VLV_RENDER_RC6_COUNT_EN )); +if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;
Signature · 6 lines
1.9.1 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 10 lines · On Sun, 30 Mar 2014 11:58:48 +0530 [email protected] wrote:
drivers/gpu/drm/i915/i915_drv.h +2
-0@@ -843,6 +849,8 @@ struct intel_gen6_power_mgmt {bool rp_up_masked;bool rp_down_masked; +u32 ei_interrupt_count; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;
drivers/gpu/drm/i915/i915_drv.h typedef struct drm_i915_private { · line 1419
Quoted 9 lines
drivers/gpu/drm/i915/i915_drv.h +6
-0@@ -1403,6 +1411,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; +
I think Chris meant that these bits belonged in intel_gen6_power_mgmt too.
Other than that it looks like Ville has given this his r-b so it ought to be fine.
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
PATCH v3 2/3 drm/i915/vlv: WA for Turbo and RC6 to work together.
In reply to [email protected]From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values
v2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
--- drivers/gpu/drm/i915/i915_drv.h | 17 +++++ drivers/gpu/drm/i915/i915_irq.c | 133 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 13 +++- drivers/gpu/drm/i915/intel_pm.c | 19 +++--- 4 files changed, 173 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +17 -0
@@ -974,6 +974,12 @@ struct i915_suspend_saved_registers {u32 savePCH_PORT_HOTPLUG; }; +struct intel_rps_ei_calc { +u32 cz_ts_ei; +u32 render_ei_c0; +u32 media_ei_c0; +}; +struct intel_gen6_power_mgmt {/* work and pm_iir are protected by dev_priv->irq_lock */ struct work_struct work;
@@ -990,6 +996,10 @@ struct intel_gen6_power_mgmt {bool rp_up_masked;bool rp_down_masked; +u32 ei_interrupt_count; + +bool use_RC0_residency_for_turbo; +int last_adj;enum {LOW_POWER ,BETWEEN ,HIGH_POWER } power;
@@ -1583,6 +1593,13 @@ typedef struct drm_i915_private {/* gen6+ rps state */ struct intel_gen6_power_mgmt rps; +/* rps wa up ei calculation */ +struct intel_rps_ei_calc rps_up_ei; + +/* rps wa down ei calculation */ +struct intel_rps_ei_calc rps_down_ei; + +/* ilk-only ips/rps state. Everything in here is protected by the global * mchdev_lock in intel_pm.c */ struct intel_ilk_power_mgmt ips;
drivers/gpu/drm/i915/i915_irq.c +133 -0
157 lines not on this page · raw message
drivers/gpu/drm/i915/i915_reg.h +12 -1
@@ -419,6 +419,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */
@@ -434,6 +435,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3
@@ -4892,6 +4898,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2
@@ -5019,13 +5026,17 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104 +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FF#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xc
drivers/gpu/drm/i915/intel_pm.c +11 -8
@@ -3096,10 +3096,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)
@@ -3620,6 +3624,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |
@@ -3639,10 +3644,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RC6_THRESHOLD ,0x557 );/* allows RC6 residency counter to work */ -I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | -VLV_MEDIA_RC6_COUNT_EN | -VLV_RENDER_RC6_COUNT_EN )); +I915_WRITE (VLV_COUNTER_CONTROL ,VLV_RC_COUNTER_CONTROL );if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;
@@ -3691,7 +3693,8 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; - dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +/* WAUseRC0ResidenncyTurbo:VLV */ + dev_priv->pm_rps_events =GEN6_PM_RP_UP_EI_EXPIRED ;gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );
Signature · 1 line
1.8.4.2
Quoted 21 lines · On Thu, Mar 13, 2014 at 09:30:17PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these valuesv2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
Signed-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 17 +++++ drivers/gpu/drm/i915/i915_irq.c | 133 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 13 +++- drivers/gpu/drm/i915/intel_pm.c | 19 +++--- 4 files changed, 173 insertions(+), 9 deletions(-)
<snip>
drivers/gpu/drm/i915/i915_irq.c static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir) · line 1691
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +6
-0@@ -1564,6 +1683,16 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)queue_work (dev_priv->wq, &dev_priv->rps.work); } +if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { +spin_lock (&dev_priv->irq_lock); + dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ; +snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ); +spin_unlock (&dev_priv->irq_lock); +DRM_DEBUG_DRIVER ("\nQueueing RPS Work - RC6 WA Turbo" );
This debug message still seems rather pointless. Just drop it.
Oh actually isn't this entire block of code useless now that pm_rps_events is used? The previous if block already checked pm_rps_events which will include GEN6_PM_RP_UP_EI_EXPIRED on VLV, so this code here will just repeat the work already done.
Quoted 8 lines
drivers/gpu/drm/i915/i915_irq.c +4
-0+ +queue_work (dev_priv->wq, &dev_priv->rps.work); + } +if (HAS_VEBOX (dev_priv->dev)) {if (pm_iir &PM_VEBOX_USER_INTERRUPT )notify_ring (dev_priv->dev, &dev_priv->ring[VECS ]);
drivers/gpu/drm/i915/i915_irq.c static void gen5_gt_irq_postinstall(struct drm_device *dev) · line 3123
Quoted 6 lines
drivers/gpu/drm/i915/i915_irq.c +3
-0@@ -2989,6 +3118,10 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) pm_irqs |=PM_VEBOX_USER_INTERRUPT ; dev_priv->pm_irq_mask =0xffffffff ; + + dev_priv->pm_irq_mask &= ~dev_priv->pm_rps_events; + pm_irqs |= dev_priv->pm_rps_events;
What's this stuff doing here?
Quoted 37 lines
drivers/gpu/drm/i915/i915_irq.c +1
-0+I915_WRITE (GEN6_PMIIR ,I915_READ (GEN6_PMIIR ));I915_WRITE (GEN6_PMIMR , dev_priv->pm_irq_mask);I915_WRITE (GEN6_PMIER , pm_irqs);drivers/gpu/drm/i915/i915_reg.h +7
-0@@ -419,6 +419,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */ @@ -434,6 +435,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3@@ -4892,6 +4898,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2
drivers/gpu/drm/i915/i915_reg.h enum punit_power_well { · line 5029
Quoted 4 lines
drivers/gpu/drm/i915/i915_reg.h +1
-0@@ -5019,13 +5026,17 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104 +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FF
I'd still like to see names for all the bits we frob, and I'd still like to have some kind of an answer to the question whether we really need to enable them all when the w/a is only interested in the rc0 counters.
Quoted 57 lines
drivers/gpu/drm/i915/i915_reg.h +4
-1#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xcdrivers/gpu/drm/i915/intel_pm.c +9
-7@@ -3096,10 +3096,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)@@ -3620,6 +3624,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |@@ -3639,10 +3644,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RC6_THRESHOLD ,0x557 );/* allows RC6 residency counter to work */ -I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | -VLV_MEDIA_RC6_COUNT_EN | -VLV_RENDER_RC6_COUNT_EN )); +I915_WRITE (VLV_COUNTER_CONTROL ,VLV_RC_COUNTER_CONTROL );if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;
drivers/gpu/drm/i915/intel_pm.c static void valleyview_enable_rps(struct drm_device *dev) · line 3697
Quoted 6 lines
drivers/gpu/drm/i915/intel_pm.c +2
-1@@ -3691,7 +3693,8 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false; - dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +/* WAUseRC0ResidenncyTurbo:VLV */ + dev_priv->pm_rps_events =GEN6_PM_RP_UP_EI_EXPIRED ;
I'm still wondering if we should have the option of using the old fashioned method...
Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +0
-0gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL );Signature · 6 lines
1.8.4.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
Quoted 36 lines · Ville Syrjälä
Quoted 21 lines · On Thu, Mar 13, 2014 at 09:30:17PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these valuesv2: Refactor duplicate code. (Ville)
v3: Reformat the comments. (Ville)
Signed-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 17 +++++ drivers/gpu/drm/i915/i915_irq.c | 133 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 13 +++- drivers/gpu/drm/i915/intel_pm.c | 19 +++--- 4 files changed, 173 insertions(+), 9 deletions(-)<snip>
drivers/gpu/drm/i915/i915_irq.c static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir) · line 1691
Quoted 10 lines
drivers/gpu/drm/i915/i915_irq.c +6
-0@@ -1564,6 +1683,16 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)queue_work (dev_priv->wq, &dev_priv->rps.work); } +if (pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ) { +spin_lock (&dev_priv->irq_lock); + dev_priv->rps.pm_iir |= pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ; +snb_disable_pm_irq (dev_priv, pm_iir &GEN6_PM_RP_UP_EI_EXPIRED ); +spin_unlock (&dev_priv->irq_lock); +DRM_DEBUG_DRIVER ("\nQueueing RPS Work - RC6 WA Turbo" );This debug message still seems rather pointless. Just drop it.
Quoted 4 lines
Oh actually isn't this entire block of code useless now that
pm_rps_eventsis used? The previous if block already checkedpm_rps_eventswhich will includeGEN6_PM_RP_UP_EI_EXPIREDon VLV, so this code here will just repeat the work already done.
hmmm. I think i missed this in internal review. with pm_rps_events i think this is redundant.
Quoted 54 lines
Quoted 8 lines
drivers/gpu/drm/i915/i915_irq.c +4
-0+ +queue_work (dev_priv->wq, &dev_priv->rps.work); + } +if (HAS_VEBOX (dev_priv->dev)) {if (pm_iir &PM_VEBOX_USER_INTERRUPT )notify_ring (dev_priv->dev, &dev_priv->ring[VECS ]);drivers/gpu/drm/i915/i915_irq.c static void gen5_gt_irq_postinstall(struct drm_device *dev) · line 3123
Quoted 6 lines
drivers/gpu/drm/i915/i915_irq.c +3
-0@@ -2989,6 +3118,10 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev) pm_irqs |=PM_VEBOX_USER_INTERRUPT ; dev_priv->pm_irq_mask =0xffffffff ; + + dev_priv->pm_irq_mask &= ~dev_priv->pm_rps_events; + pm_irqs |= dev_priv->pm_rps_events;What's this stuff doing here?
Quoted 38 lines
drivers/gpu/drm/i915/i915_irq.c +1
-0+I915_WRITE (GEN6_PMIIR ,I915_READ (GEN6_PMIIR ));I915_WRITE (GEN6_PMIMR , dev_priv->pm_irq_mask);I915_WRITE (GEN6_PMIER , pm_irqs);drivers/gpu/drm/i915/i915_reg.h +7
-0@@ -419,6 +419,7 @@ enum punit_power_well {#define PUNIT_REG_GPU_FREQ_STS 0xd8#define GENFREQSTATUS (1<<0)#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc +#define PUNIT_REG_CZ_TIMESTAMP 0xce#define PUNIT_FUSE_BUS2 0xf6/* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5/* bits 55:48 */ @@ -434,6 +435,11 @@ enum punit_power_well {#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf8000000 +#define VLV_CZ_CLOCK_TO_MILLI_SEC 100000 +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 +#define VLV_INT_COUNT_FOR_DOWN_EI 5 +/* vlv2 north clock has */ #define CCK_FUSE_REG 0x8#define CCK_FUSE_HPLL_FREQ_MASK 0x3@@ -4892,6 +4898,7 @@ enum punit_power_well {#define VLV_GTLC_PW_STATUS 0x130094#define VLV_GTLC_PW_RENDER_STATUS_MASK 0x80#define VLV_GTLC_PW_MEDIA_STATUS_MASK 0x20 +#define VLV_GTLC_SURVIVABILITY_REG 0x130098#define FORCEWAKE_MT 0xa188/* multi-threaded */ #define FORCEWAKE_KERNEL 0x1#define FORCEWAKE_USER 0x2@@ -5019,13 +5026,17 @@ enum punit_power_well {
Quoted 8 lines
drivers/gpu/drm/i915/i915_reg.h enum punit_power_well { · line 5029
Quoted 3 lines
drivers/gpu/drm/i915/i915_reg.h +1
-0#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104 +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FFI'd still like to see names for all the bits we frob, and I'd still like to have some kind of an answer to the question whether we really need to enable them all when the w/a is only interested in the rc0 counters.
I did try with enabling only the rc0 counters, but the busyness calculation was not right. Let me do some more investigation and get back to you on this.
Quoted 60 lines
Quoted 60 lines
drivers/gpu/drm/i915/i915_reg.h +4
-1#define VLV_COUNT_RANGE_HIGH (1<<15) +#define VLV_MEDIA_RC0_COUNT_EN (1<<5) +#define VLV_RENDER_RC0_COUNT_EN (1<<4)#define VLV_MEDIA_RC6_COUNT_EN (1<<1)#define VLV_RENDER_RC6_COUNT_EN (1<<0)#define GEN6_GT_GFX_RC6 0x138108#define GEN6_GT_GFX_RC6p 0x13810C#define GEN6_GT_GFX_RC6pp 0x138110 - +#define VLV_RENDER_C0_COUNT_REG 0x138118 +#define VLV_MEDIA_C0_COUNT_REG 0x13811C#define GEN6_PCODE_MAILBOX 0x138124#define GEN6_PCODE_READY (1<<31)#define GEN6_READ_OC_PARAMS 0xcdrivers/gpu/drm/i915/intel_pm.c +9
-7@@ -3096,10 +3096,14 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)I915_READ (VLV_GTLC_SURVIVABILITY_REG ) & ~VLV_GFX_CLK_FORCE_ON_BIT); -/* Unmask Up interrupts */ - dev_priv->rps.rp_up_masked = true; -gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , +/* Unmask Turbo interrupts */ +if (dev_priv->pm_rps_events &GEN6_PM_RP_UP_EI_EXPIRED ) +I915_WRITE (GEN6_PMINTRMSK , ~dev_priv->pm_rps_events); +else { + dev_priv->rps.rp_up_masked = true; +gen6_set_pm_mask (dev_priv,GEN6_PM_RP_DOWN_THRESHOLD , dev_priv->rps.min_delay); + } }void gen6_rps_idle (struct drm_i915_private *dev_priv)@@ -3620,6 +3624,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RP_DOWN_EI ,350000 );I915_WRITE (GEN6_RP_IDLE_HYSTERSIS ,10 ); +I915_WRITE (GEN6_RP_DOWN_TIMEOUT ,0xf4240 ); I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO |@@ -3639,10 +3644,7 @@ static void valleyview_enable_rps(struct drm_device *dev)I915_WRITE (GEN6_RC6_THRESHOLD ,0x557 );/* allows RC6 residency counter to work */ -I915_WRITE (VLV_COUNTER_CONTROL , -_MASKED_BIT_ENABLE (VLV_COUNT_RANGE_HIGH | -VLV_MEDIA_RC6_COUNT_EN | -VLV_RENDER_RC6_COUNT_EN )); +I915_WRITE (VLV_COUNTER_CONTROL ,VLV_RC_COUNTER_CONTROL );if (intel_enable_rc6 (dev) &INTEL_RC6_ENABLE ) rc6_mode =GEN7_RC_CTL_TO_MODE |VLV_RC_CTL_CTX_RST_PARALLEL ;@@ -3691,7 +3693,8 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rp_up_masked = false; dev_priv->rps.rp_down_masked = false;
Quoted 6 lines
drivers/gpu/drm/i915/intel_pm.c static void valleyview_enable_rps(struct drm_device *dev) · line 3697
Quoted 3 lines
drivers/gpu/drm/i915/intel_pm.c +2
-1- dev_priv->pm_rps_events =GEN6_PM_RPS_EVENTS ; +/* WAUseRC0ResidenncyTurbo:VLV */ + dev_priv->pm_rps_events =GEN6_PM_RP_UP_EI_EXPIRED ;I'm still wondering if we should have the option of using the old fashioned method...
I think we can have a if turbo_wa, This will help us to switch by disabling a flag.
Quoted 11 lines
Quoted 10 lines
drivers/gpu/drm/i915/intel_pm.c +0
-1gen6_enable_rps_interrupts (dev);gen6_gt_force_wake_put (dev_priv,FORCEWAKE_ALL ); --1.8.4.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 4 lines · Deepak
Quoted 1 line · Ville Syrjälä
On Thu, Mar 13, 2014 at 09:30:17PM +0530, [email protected] wrote:
<snip>
Quoted 11 lines
Quoted 10 lines
drivers/gpu/drm/i915/i915_reg.h enum punit_power_well { · line 5029
Quoted 5 lines
drivers/gpu/drm/i915/i915_reg.h +1
-0@@ -5019,13 +5026,17 @@ enum punit_power_well {#define GEN6_GT_GFX_RC6_LOCKED 0x138104#define VLV_COUNTER_CONTROL 0x138104 +#define VLV_RC_COUNTER_CONTROL 0xFFFF00FFI'd still like to see names for all the bits we frob, and I'd still like to have some kind of an answer to the question whether we really need to enable them all when the w/a is only interested in the rc0 counters.
Quoted 3 lines
I did try with enabling only the rc0 counters, but the busyness calculation was not right. Let me do some more investigation and get back to you on this.
Well, if you tried it and it didn't work right, then I'm already fairly satisfied with that. It just needs a comment to make it clear why we enable them all. Of course if you can dig out more details, that's always a bonus.
Signature · 2 lines
Ville Syrjälä Intel OTC
PATCH v2 3/3 drm/i915: Add boot paramter to control rps boost at boot time.
In reply to [email protected]From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
v2: Addressed rps default comment (Jani)
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 16 +++++++++++++++- drivers/gpu/drm/i915/i915_params.c | 5 +++++ 3 files changed, 21 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -2106,6 +2106,7 @@ struct i915_params {int panel_use_ssc;int vbt_sdvo_panel_type;int enable_rc6; +int enable_rps_boost;int enable_fbc;int enable_ppgtt;int enable_psr;
drivers/gpu/drm/i915/i915_gem.c +15 -1
@@ -1002,6 +1002,17 @@ static bool can_wait_boost(struct drm_i915_file_private *file_priv)return !atomic_xchg (&file_priv->rps_wait_boost, true); } +static int intel_enable_rps_boost (struct drm_device *dev) +{ +/* No RPS Boost before Ironlake */ +if (INTEL_INFO (dev)->gen <6 ) +return 0 ; + +/* Respect the kernel parameter if it is set */ +return i915.enable_rps_boost; + +} +/** * __wait_seqno - wait until execution of seqno has finished * @ring: the ring expected to report seqno
@@ -1042,8 +1053,11 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, timeout_expire = timeout ? jiffies +timespec_to_jiffies_timeout (timeout) :0 ; -if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv)) { +if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv) && +intel_enable_rps_boost (ring->dev)) { +gen6_rps_boost (dev_priv); +if (file_priv) mod_delayed_work(dev_priv->wq, &file_priv->mm.idle_work,
drivers/gpu/drm/i915/i915_params.c +5 -0
@@ -34,6 +34,7 @@ struct i915_params i915 __read_mostly = { .panel_use_ssc = -1, .vbt_sdvo_panel_type = -1, .enable_rc6 = -1, + .enable_rps_boost = 1, .enable_fbc = -1, .enable_hangcheck = true, .enable_ppgtt = -1,
@@ -78,6 +79,10 @@ MODULE_PARM_DESC(enable_rc6,"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " "default: -1 (use per-chip default)"); +module_param_named (enable_rps_boost, i915.enable_rps_boost, int,0600 ); +MODULE_PARM_DESC (enable_rps_boost, +"Enable/Disable boost RPS frequency (default: enabled (1))" ); +module_param_named (enable_fbc, i915.enable_fbc, int,0600 );MODULE_PARM_DESC (enable_fbc, "Enable frame buffer compression for power savings "
Signature · 1 line
1.8.4.2
Re: [PATCH v2 3/3] drm/i915: Add boot paramter to control rps boost at boot time.
Quoted 20 lines · On Thu, Mar 13, 2014 at 09:30:18PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
v2: Addressed rps default comment (Jani)
Signed-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 16 +++++++++++++++- drivers/gpu/drm/i915/i915_params.c | 5 +++++ 3 files changed, 21 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/i915_drv.h +0
-0
drivers/gpu/drm/i915/i915_drv.h struct i915_params { · line 2109
Quoted 4 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -2106,6 +2106,7 @@ struct i915_params {int panel_use_ssc;int vbt_sdvo_panel_type;int enable_rc6; +int enable_rps_boost;
Should be bool like Jani said. And then it should be thrown somewhere somewhere at the end of the structure next to the other bools.
Quoted 8 lines
drivers/gpu/drm/i915/i915_drv.h +0
-0int enable_fbc;int enable_ppgtt;int enable_psr;drivers/gpu/drm/i915/i915_gem.c +0
-0
drivers/gpu/drm/i915/i915_gem.c static bool can_wait_boost(struct drm_i915_file_private *file_priv) · line 1007
Quoted 6 lines
drivers/gpu/drm/i915/i915_gem.c +3
-0@@ -1002,6 +1002,17 @@ static bool can_wait_boost(struct drm_i915_file_private *file_priv)return !atomic_xchg (&file_priv->rps_wait_boost, true); } +static int intel_enable_rps_boost (struct drm_device *dev) +{ +/* No RPS Boost before Ironlake */
This comment is still wrong. I'd just drop it, everyone should know what the gen check below means.
drivers/gpu/drm/i915/i915_gem.c static bool can_wait_boost(struct drm_i915_file_private *file_priv) · line 1011
Quoted 4 lines
drivers/gpu/drm/i915/i915_gem.c +4
-0+if (INTEL_INFO (dev)->gen <6 ) +return 0 ; + +/* Respect the kernel parameter if it is set */
This comment too seems rather obvious. I'd drop it as well.
drivers/gpu/drm/i915/i915_gem.c static bool can_wait_boost(struct drm_i915_file_private *file_priv) · line 1014
Quoted 3 lines
drivers/gpu/drm/i915/i915_gem.c +3
-0+return i915.enable_rps_boost; + +}
This function is still just a wrapper for i915.enable_rps_boost since __wait_seqno() already does the gen check. You could just check i915.enable_rps_boost directly in __wait_seqno(). The other option is to just drop the gen check from __wait_seqno() and just let this function take care of it. Hmm. Yeah that might be the nicest choice in fact.
drivers/gpu/drm/i915/i915_gem.c static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, · line 1057
Quoted 11 lines
drivers/gpu/drm/i915/i915_gem.c +3
-1+/** * __wait_seqno - wait until execution of seqno has finished * @ring: the ring expected to report seqno @@ -1042,8 +1053,11 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, timeout_expire = timeout ? jiffies +timespec_to_jiffies_timeout (timeout) :0 ; -if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv)) { +if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv) && +intel_enable_rps_boost (ring->dev)) {
Indentation is quite wrong. There's also trailing whitespace around these parts. Please run patches through checkpatch.pl before submitting.
Quoted 11 lines
drivers/gpu/drm/i915/i915_gem.c +2
-0+gen6_rps_boost (dev_priv); +if (file_priv) mod_delayed_work(dev_priv->wq, &file_priv->mm.idle_work,drivers/gpu/drm/i915/i915_params.c +0
-0
drivers/gpu/drm/i915/i915_params.c struct i915_params i915 __read_mostly = { · line 37
Quoted 4 lines
drivers/gpu/drm/i915/i915_params.c +1
-0@@ -34,6 +34,7 @@ struct i915_params i915 __read_mostly = { .panel_use_ssc = -1, .vbt_sdvo_panel_type = -1, .enable_rc6 = -1, + .enable_rps_boost = 1,
true
drivers/gpu/drm/i915/i915_params.c MODULE_PARM_DESC(enable_rc6, · line 82
Quoted 8 lines
drivers/gpu/drm/i915/i915_params.c +1
-0.enable_fbc = -1, .enable_hangcheck = true, .enable_ppgtt = -1,@@ -78,6 +79,10 @@ MODULE_PARM_DESC(enable_rc6,"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " "default: -1 (use per-chip default)"); +module_param_named (enable_rps_boost, i915.enable_rps_boost, int,0600 );
bool
drivers/gpu/drm/i915/i915_params.c MODULE_PARM_DESC(enable_rc6, · line 84
Quoted 2 lines
drivers/gpu/drm/i915/i915_params.c +2
-0+MODULE_PARM_DESC (enable_rps_boost, +"Enable/Disable boost RPS frequency (default: enabled (1))" );
default: true
Quoted 11 lines
drivers/gpu/drm/i915/i915_params.c +1
-0+module_param_named (enable_fbc, i915.enable_fbc, int,0600 );MODULE_PARM_DESC (enable_fbc, "Enable frame buffer compression for power savings "Signature · 6 lines
1.8.4.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
Re: [PATCH v2 3/3] drm/i915: Add boot paramter to control rps boost at boot time.
Quoted 16 lines · Ville Syrjälä
Quoted 15 lines · On Thu, Mar 13, 2014 at 09:30:18PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
v2: Addressed rps default comment (Jani)
Signed-off-by Deepak S <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 16 +++++++++++++++- drivers/gpu/drm/i915/i915_params.c | 5 +++++ 3 files changed, 21 insertions(+), 1 deletion(-)
Quoted 12 lines
drivers/gpu/drm/i915/i915_drv.h struct i915_params { · line 2109
Quoted 9 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -2106,6 +2106,7 @@ struct i915_params {int panel_use_ssc;int vbt_sdvo_panel_type;int enable_rc6; +int enable_rps_boost;Should be bool like Jani said. And then it should be thrown somewhere somewhere at the end of the structure next to the other bools.
I will address this.
Quoted 11 lines
Quoted 11 lines
drivers/gpu/drm/i915/i915_drv.h +0
-0int enable_fbc;int enable_ppgtt;int enable_psr;drivers/gpu/drm/i915/i915_gem.c +0
-0@@ -1002,6 +1002,17 @@ static bool can_wait_boost(struct drm_i915_file_private *file_priv)return !atomic_xchg (&file_priv->rps_wait_boost, true); }
Quoted 6 lines
drivers/gpu/drm/i915/i915_gem.c static bool can_wait_boost(struct drm_i915_file_private *file_priv) · line 1007
Quoted 3 lines
drivers/gpu/drm/i915/i915_gem.c +3
-0+static int intel_enable_rps_boost (struct drm_device *dev) +{ +/* No RPS Boost before Ironlake */ This comment is still wrong. I'd just drop it, everyone should know what the gen check below means.
Ok
Quoted 6 lines
drivers/gpu/drm/i915/i915_gem.c static bool can_wait_boost(struct drm_i915_file_private *file_priv) · line 1011
Quoted 4 lines
drivers/gpu/drm/i915/i915_gem.c +4
-0+if (INTEL_INFO (dev)->gen <6 ) +return 0 ; + +/* Respect the kernel parameter if it is set */ This comment too seems rather obvious. I'd drop it as well.
Ok
Quoted 10 lines
drivers/gpu/drm/i915/i915_gem.c static bool can_wait_boost(struct drm_i915_file_private *file_priv) · line 1014
Quoted 3 lines
drivers/gpu/drm/i915/i915_gem.c +3
-0+return i915.enable_rps_boost; + +}This function is still just a wrapper for i915.enable_rps_boost since
__wait_seqno()already does the gen check. You could just check i915.enable_rps_boost directly in__wait_seqno(). The other option is to just drop the gen check from__wait_seqno()and just let this function take care of it. Hmm. Yeah that might be the nicest choice in fact.
Agreed. Does not make sense to have multiple platform check's.
Quoted 61 lines
drivers/gpu/drm/i915/i915_gem.c static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, · line 1057
Quoted 11 lines
drivers/gpu/drm/i915/i915_gem.c +3
-1+/** * __wait_seqno - wait until execution of seqno has finished * @ring: the ring expected to report seqno @@ -1042,8 +1053,11 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, timeout_expire = timeout ? jiffies +timespec_to_jiffies_timeout (timeout) :0 ; -if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv)) { +if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv) && +intel_enable_rps_boost (ring->dev)) {Indentation is quite wrong. There's also trailing whitespace around these parts. Please run patches through checkpatch.pl before submitting.
Quoted 11 lines
drivers/gpu/drm/i915/i915_gem.c +2
-0+gen6_rps_boost (dev_priv); +if (file_priv) mod_delayed_work(dev_priv->wq, &file_priv->mm.idle_work,drivers/gpu/drm/i915/i915_params.c +0
-0drivers/gpu/drm/i915/i915_params.c struct i915_params i915 __read_mostly = { · line 37
Quoted 4 lines
drivers/gpu/drm/i915/i915_params.c +1
-0@@ -34,6 +34,7 @@ struct i915_params i915 __read_mostly = { .panel_use_ssc = -1, .vbt_sdvo_panel_type = -1, .enable_rc6 = -1, + .enable_rps_boost = 1,true
drivers/gpu/drm/i915/i915_params.c MODULE_PARM_DESC(enable_rc6, · line 82
Quoted 8 lines
drivers/gpu/drm/i915/i915_params.c +1
-0.enable_fbc = -1, .enable_hangcheck = true, .enable_ppgtt = -1,@@ -78,6 +79,10 @@ MODULE_PARM_DESC(enable_rc6,"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " "default: -1 (use per-chip default)"); +module_param_named (enable_rps_boost, i915.enable_rps_boost, int,0600 );bool
drivers/gpu/drm/i915/i915_params.c MODULE_PARM_DESC(enable_rc6, · line 84
Quoted 2 lines
drivers/gpu/drm/i915/i915_params.c +2
-0+MODULE_PARM_DESC (enable_rps_boost, +"Enable/Disable boost RPS frequency (default: enabled (1))" );default: true
Quoted 11 lines
drivers/gpu/drm/i915/i915_params.c +1
-1+module_param_named (enable_fbc, i915.enable_fbc, int,0600 );MODULE_PARM_DESC (enable_fbc, "Enable frame buffer compression for power savings " --1.8.4.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx